From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paul Durrant Subject: Re: [PATCH v3 2/2] ioreq-server: write protected range and forwarding Date: Wed, 3 Sep 2014 10:11:41 +0000 Message-ID: <9AAE0902D5BC7E449B7C8E4E778ABCD0446678@AMSPEX01CL01.citrite.net> References: <1409781199-12052-1-git-send-email-wei.ye@intel.com> <1409781199-12052-3-git-send-email-wei.ye@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1409781199-12052-3-git-send-email-wei.ye@intel.com> Content-Language: en-US List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Wei Ye , "xen-devel@lists.xen.org" Cc: Kevin Tian , "Keir (Xen.org)" , Ian Campbell , "Tim (Xen.org)" , "donald.d.dugger@intel.com" , Stefano Stabellini , "zhiyuan.lv@intel.com" , "JBeulich@suse.com" , "yang.z.zhang@intel.com" , Ian Jackson List-Id: xen-devel@lists.xenproject.org > -----Original Message----- > From: Wei Ye [mailto:wei.ye@intel.com] > Sent: 03 September 2014 22:53 > To: xen-devel@lists.xen.org > Cc: JBeulich@suse.com; Ian Campbell; Paul Durrant; Ian Jackson; Stefano > Stabellini; donald.d.dugger@intel.com; Kevin Tian; yang.z.zhang@intel.com; > zhiyuan.lv@intel.com; konrad.wilk@oracle.com; 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 > --- > 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. > + 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 > ""); > 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(). 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