Re: [Xen-devel] [PATCH v3 2/2] ioreq-server: write protected range and forwarding

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Ye, Wei
> Sent: Tuesday, September 09, 2014 11:10 PM
> > > > > > adc0f93..f8c4db8 100644
> > > > > > --- a/xen/arch/x86/hvm/hvm.c
> > > > > > +++ b/xen/arch/x86/hvm/hvm.c
> > > > > > @@ -853,6 +853,7 @@ static int
> > > > > > hvm_ioreq_server_alloc_rangesets(struct
> > > > > > hvm_ioreq_server *s,
> > > > > >                        (i == HVMOP_IO_RANGE_PORT) ?
> "port" :
> > > > > >                        (i == HVMOP_IO_RANGE_MEMORY) ?
> > > > > "memory" :
> > > > > >                        (i == HVMOP_IO_RANGE_PCI) ? "pci" :
> > > > > > +                      (i == HVMOP_IO_RANGE_WP) ? "write
> > > > > protection" :
> > > > >
> > > > > Continuing along my train of thought from above, perhaps the
> > > > > rangesets should now be
> > > > >
> > > > > HVMOP_IO_RANGE_PORT_READ
> > > > > HMVOP_IO_RANGE_PORT_WRITE
> > > > > HVMOP_IO_RANGE_MEMORY_READ
> > > > > HVMOP_IO_RANGE_MEMORY_WRITE
> > > > > HVMOP_IO_RANGE_PCI
> > > >
> > > > this looks cleaner. Once we start considering the permission, WP
> > > > becomes only an attribute which can be attached to either port or
> > memory resource.
> > > > So better not define a new type here.
> > > >
> > >
> > > Shall we also need a type of HVMOP_IO_RANGE_MEMORY_BOTH?
> > > Otherwise, if want to set
> > > both r/w emulation, user needs to call the interface
> > > xc_hvm_map_io_range_to_ioreq_server() twice. One is with the
> > parameter
> > > HVMOP_IO_RANGE_MEMORY_READ and the other with parameter
> > > HVMOP_IO_RANGE_MEMORY_WRITE.
> > >
> >
> > That's true, so messing with the types directly is probably not the correct
> > answer. Instead, break direct association between type and rangeset index
> > and then hvmop_map_io_range_to_ioreq_server() can call
> > hvm_map_io_range_to_ioreq_server() multiple times if necessary.
> > You could declare the array ins struct hvm_ioreq_server as:
> >
> > struct rangeset     *range[NR_IO_RANGE_TYPES * 2];
> >
> > and then calculate index as something like:
> >
> > if (flags & READ)
> >   index = type;
> >
> > if (flags & WRITE)
> >   index = type + NR_IO_RANGE_TYPES;
> >
> > Yes, it does mean double the number of rangesets if you want both read and
> > write, but that's a very small amount of extra memory. If necessary it could
> > be avoided by having a third set of rangesets for READ|WRITE but it would
> > mean potentially having to do two lookups in hvm_select_ioreq_server().
> >
> >   Paul
> >
> I'm a little bit confused that like write protection for a true mmio range. 
> If the
> true
> mmio range is readable but write is porected, then where is the data can be
> read from
> If the read is not forwarded to device model. Current write protection for
> normal pages
> treat write as mmio is a special case. For read, data comes from the normal
> page, but
> wirte go to device model for emulation. And, we also can write directly to the
> normal page but forward read to device model. So I think the write protection
> or
> read protection is meaningfull only for normal page protection, not for port 
> or
> mmio resources.
> So, double the rangeset that split READ|WRITE sets for existing resource is 
> not
> very reasonable,  am I right?
> 
> Wei

Ideally there could be such usage of a special device assignment style. For
example, have all the writes from the VM forwarded to an agent in Dom0, so
writes can be logged and replayed for introspection or high availability 
purpose,
while having most reads from device directly for performance reason if no
side-effect. 

However it's only an ideal case, so likely the change we made now is incomplete
and then anyway more changes are required to make it complete when the 
ideal case becomes real. Regarding to that, maybe it's better to just introduce 
a new type, specifically for this write-protected memory page usage:

>>HVMOP_IO_RANGE_WP_MEMORY

It has effect only on normal memory (add a check on that), w/o impact on
existing interfaces.

Paul, how about your thoughts?

Thanks
Kevin

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel