|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 2/2] ioreq-server: write protected range and forwarding
> -----Original Message----- > From: Tian, Kevin > Sent: Friday, September 5, 2014 7:11 AM > To: Paul Durrant; Ye, Wei; xen-devel@xxxxxxxxxxxxx > Cc: JBeulich@xxxxxxxx; Ian Campbell; Ian Jackson; Stefano Stabellini; Dugger, > Donald D; Zhang, Yang Z; Lv, Zhiyuan; konrad.wilk@xxxxxxxxxx; Keir (Xen.org); > Tim (Xen.org) > Subject: RE: [PATCH v3 2/2] ioreq-server: write protected range and > forwarding > > > From: Paul Durrant [mailto:Paul.Durrant@xxxxxxxxxx] > > Sent: Wednesday, September 03, 2014 3:12 AM > > > > > -----Original Message----- > > > From: Wei Ye [mailto:wei.ye@xxxxxxxxx] > > > Sent: 03 September 2014 22:53 > > > To: xen-devel@xxxxxxxxxxxxx > > > Cc: JBeulich@xxxxxxxx; Ian Campbell; Paul Durrant; Ian Jackson; > > > Stefano Stabellini; donald.d.dugger@xxxxxxxxx; Kevin Tian; > > > yang.z.zhang@xxxxxxxxx; zhiyuan.lv@xxxxxxxxx; > > > konrad.wilk@xxxxxxxxxx; Keir (Xen.org); Tim (Xen.org); Wei Ye > > > Subject: [PATCH v3 2/2] ioreq-server: write protected range and > > > forwarding > > > > > > Extend the ioreq-server to write protect pages and forward the write > > > access to the corresponding device model. > > > > > > > Using rangesets did make the patch somewhat smaller then :-) > > > > > Signed-off-by: Wei Ye <wei.ye@xxxxxxxxx> > > > --- > > > tools/libxc/xc_domain.c | 33 > > +++++++++++++++++++++++++++ > > > tools/libxc/xenctrl.h | 18 +++++++++++++++ > > > xen/arch/x86/hvm/hvm.c | 46 > > > ++++++++++++++++++++++++++++++++++++++ > > > xen/include/asm-x86/hvm/domain.h | 2 +- > > > xen/include/public/hvm/hvm_op.h | 1 + > > > 5 files changed, 99 insertions(+), 1 deletion(-) > > > > > > diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c index > > > 37ed141..34e6309 100644 > > > --- a/tools/libxc/xc_domain.c > > > +++ b/tools/libxc/xc_domain.c > > > @@ -1485,6 +1485,39 @@ int > > > xc_hvm_unmap_pcidev_from_ioreq_server(xc_interface *xch, domid_t > > > domid, > > > return rc; > > > } > > > > > > +int xc_hvm_map_wp_range_to_ioreq_server(xc_interface *xch, > domid_t > > > domid, > > > + ioservid_t id, uint16_t set, > > > + uint64_t start, uint64_t > > end) > > > +{ > > > > Given what I said about the notion of MMIO ranges that need read > > emulation but no write emulation, and the ranges you're concerned > > about which require write emulation but no read emulation, I wonder > > whether this could collapse into the existing > > xc_hvm_map_io_range_to_ioreq_server() by adding an extra parameter > to say whether, for a given range, you want read emulation, write > > emulation or both. > > Agree. Add a parameter to indicate r/w permission seems cleaner. > > > > > > + DECLARE_HYPERCALL; > > > + DECLARE_HYPERCALL_BUFFER(xen_hvm_io_range_t, arg); > > > + int rc = -1; > > > + > > > + if ( arg == NULL ) > > > + { > > > + errno = -EFAULT; > > > + return -1; > > > + } > > > + > > > + hypercall.op = __HYPERVISOR_hvm_op; > > > + if ( set ) > > > + hypercall.arg[0] = HVMOP_map_io_range_to_ioreq_server; > > > + else > > > + hypercall.arg[0] = HVMOP_unmap_io_range_from_ioreq_server; > > > + hypercall.arg[1] = HYPERCALL_BUFFER_AS_ARG(arg); > > > + > > > + arg->domid = domid; > > > + arg->id = id; > > > + arg->type = HVMOP_IO_RANGE_WP; > > > + arg->start = start; > > > + arg->end = end; > > > + > > > + rc = do_xen_hypercall(xch, &hypercall); > > > + > > > + xc_hypercall_buffer_free(xch, arg); > > > + return rc; > > > +} > > > + > > > int xc_hvm_destroy_ioreq_server(xc_interface *xch, > > > domid_t domid, > > > ioservid_t id) diff --git > > > a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h index > > > b55d857..b261602 100644 > > > --- a/tools/libxc/xenctrl.h > > > +++ b/tools/libxc/xenctrl.h > > > @@ -1943,6 +1943,24 @@ int > > > xc_hvm_unmap_pcidev_from_ioreq_server(xc_interface *xch, > > > uint8_t function); > > > > > > /** > > > + * This function registers/deregisters a set of pages to be write > protected. > > > + * > > > + * @parm xch a handle to an open hypervisor interface. > > > + * @parm domid the domain id to be serviced > > > + * @parm id the IOREQ Server id. > > > + * @parm set whether registers or deregisters: 1 for register, 0 > > > + for > > > deregister > > > + * @parm start start of range > > > + * @parm end end of range (inclusive). > > > + * @return 0 on success, -1 on failure. > > > + */ > > > +int xc_hvm_map_wp_range_to_ioreq_server(xc_interface *xch, > > > + domid_t domid, > > > + ioservid_t id, > > > + uint16_t set, > > > + uint64_t start, > > > + uint64_t end); > > > + > > > +/** > > > * This function destroys an IOREQ Server. > > > * > > > * @parm xch a handle to an open hypervisor interface. > > > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index > > > 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. Wei. > for now READ can simply fail until we see a real usage. > > One side-effect though, is for default r/w emulated case we would double > rangesets here. > > > > > > ""); > > > if ( rc ) > > > goto fail; > > > @@ -1138,6 +1139,37 @@ static int hvm_get_ioreq_server_info(struct > > > domain *d, ioservid_t id, > > > return rc; > > > } > > > > > > +static int hvm_change_p2m_type_ioreq_server(struct domain *d, > > > +uint16_t > > > set, > > > + uint64_t start, > > uint64_t end) > > > +{ > > > + int rc = -EINVAL; > > > + uint64_t gpfn_s, gpfn_e, gpfn; > > > + p2m_type_t ot, nt; > > > + > > > + if ( set ) > > > + { > > > + ot = p2m_ram_rw; > > > + nt = p2m_mmio_write_dm; > > > + } > > > + else > > > + { > > > + ot = p2m_mmio_write_dm; > > > + nt = p2m_ram_rw; > > > + } > > > + > > > + gpfn_s = start >> PAGE_SHIFT; > > > + gpfn_e = end >> PAGE_SHIFT; > > > + > > > + for ( gpfn = gpfn_s; gpfn <= gpfn_e; gpfn++) > > > + { > > > + rc = p2m_change_type_one(d, gpfn, ot, nt); > > > + if ( !rc ) > > > + break; > > > + } > > > + > > > + return rc; > > > +} > > > + > > > static int hvm_map_io_range_to_ioreq_server(struct domain *d, > > ioservid_t > > > id, > > > uint32_t type, > > uint64_t start, uint64_t end) > > > { > > > @@ -1163,6 +1195,7 @@ static int > > > hvm_map_io_range_to_ioreq_server(struct domain *d, ioservid_t id, > > > case HVMOP_IO_RANGE_PORT: > > > case HVMOP_IO_RANGE_MEMORY: > > > case HVMOP_IO_RANGE_PCI: > > > + case HVMOP_IO_RANGE_WP: > > > r = s->range[type]; > > > break; > > > > > > @@ -1180,6 +1213,10 @@ static int > > > hvm_map_io_range_to_ioreq_server(struct domain *d, ioservid_t id, > > > break; > > > > > > rc = rangeset_add_range(r, start, end); > > > + > > > + if ( type == HVMOP_IO_RANGE_WP ) > > > + rc = hvm_change_p2m_type_ioreq_server(d, 1, start, > > end); > > > + > > > > Here, for memory ranges, you'd then change the p2m types to > > p2m_mmio_write_dm, p2m_mmio_read_dm or p2m_mmio_dm > depending on > > whether the a range was being added to the MEMORY_WRITE set, the > > MEMORY_READ set or both. Obviously you'd need to be careful about > > ranges with distinct semantics that fall into the same page - but that > > could be dealt with. E.g. if you had two ranges in the same page, one > > needing full emulation and one needing write-only emulation then you > > could set the p2m type to p2m_mmio_dm and handle the read emulation > > for the second range directly in hvm_send_assist_req_to_ioreq_server(). > > I prefer to not introducing such complexity now. Just focus on existing > usages, > i.e. either rw-emulation or w-emulation. We can reserve the type in > hypercall path, but p2m_mmio_read_dm can wait until a real usage is > required. > > > > > Paul > > > > > break; > > > } > > > } > > > @@ -1214,6 +1251,7 @@ static int > > > hvm_unmap_io_range_from_ioreq_server(struct domain *d, ioservid_t > id, > > > case HVMOP_IO_RANGE_PORT: > > > case HVMOP_IO_RANGE_MEMORY: > > > case HVMOP_IO_RANGE_PCI: > > > + case HVMOP_IO_RANGE_WP: > > > r = s->range[type]; > > > break; > > > > > > @@ -1231,6 +1269,10 @@ static int > > > hvm_unmap_io_range_from_ioreq_server(struct domain *d, ioservid_t > id, > > > break; > > > > > > rc = rangeset_remove_range(r, start, end); > > > + > > > + if ( type == HVMOP_IO_RANGE_WP ) > > > + rc = hvm_change_p2m_type_ioreq_server(d, 0, start, > > end); > > > + > > > break; > > > } > > > } > > > @@ -2361,6 +2403,10 @@ static struct hvm_ioreq_server > > > *hvm_select_ioreq_server(struct domain *d, > > > if ( rangeset_contains_range(r, addr, end) ) > > > return s; > > > > > > + r = s->range[HVMOP_IO_RANGE_WP]; > > > + if ( rangeset_contains_range(r, addr, end) ) > > > + return s; > > > + > > > break; > > > case IOREQ_TYPE_PCI_CONFIG: > > > if ( rangeset_contains_singleton(r, addr >> 32) ) diff > > > --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm- > > > x86/hvm/domain.h index 291a2e0..b194f9a 100644 > > > --- a/xen/include/asm-x86/hvm/domain.h > > > +++ b/xen/include/asm-x86/hvm/domain.h > > > @@ -48,7 +48,7 @@ struct hvm_ioreq_vcpu { > > > evtchn_port_t ioreq_evtchn; > > > }; > > > > > > -#define NR_IO_RANGE_TYPES (HVMOP_IO_RANGE_PCI + 1) > > > +#define NR_IO_RANGE_TYPES (HVMOP_IO_RANGE_WP + 1) > > > #define MAX_NR_IO_RANGES 256 > > > > > > struct hvm_ioreq_server { > > > diff --git a/xen/include/public/hvm/hvm_op.h > > > b/xen/include/public/hvm/hvm_op.h index eeb0a60..aedb97f 100644 > > > --- a/xen/include/public/hvm/hvm_op.h > > > +++ b/xen/include/public/hvm/hvm_op.h > > > @@ -323,6 +323,7 @@ struct xen_hvm_io_range { > > > # define HVMOP_IO_RANGE_PORT 0 /* I/O port range */ > > > # define HVMOP_IO_RANGE_MEMORY 1 /* MMIO range */ > > > # define HVMOP_IO_RANGE_PCI 2 /* PCI segment/bus/dev/func > > range */ > > > +# define HVMOP_IO_RANGE_WP 3 /* Write protetion range */ > > > uint64_aligned_t start, end; /* IN - inclusive start and end of > > > range */ }; typedef struct xen_hvm_io_range xen_hvm_io_range_t; > > > -- > > > 1.7.9.5 > > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |