[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v4 1/2] Differentiate IO/mem resources tracked by ioreq server



> -----Original Message-----
> From: Yu, Zhang [mailto:yu.c.zhang@xxxxxxxxxxxxxxx]
> Sent: 14 August 2015 08:41
> To: Paul Durrant; xen-devel@xxxxxxxxxxxxx; Ian Jackson; Stefano Stabellini; 
> Ian
> Campbell; Wei Liu; Keir (Xen.org); jbeulich@xxxxxxxx; Andrew Cooper
> Cc: Kevin Tian; zhiyuan.lv@xxxxxxxxx
> Subject: Re: [Xen-devel] [PATCH v4 1/2] Differentiate IO/mem resources
> tracked by ioreq server
> 
> Hi Paul,
> 
> On 8/13/2015 6:39 PM, Yu, Zhang wrote:
> >
> >
> > On 8/13/2015 6:16 PM, Paul Durrant wrote:
> >>> -----Original Message-----
> >>> From: Yu Zhang [mailto:yu.c.zhang@xxxxxxxxxxxxxxx]
> >>> Sent: 13 August 2015 11:06
> >>> To: xen-devel@xxxxxxxxxxxxx; Paul Durrant; Ian Jackson; Stefano
> >>> Stabellini; Ian
> >>> Campbell; Wei Liu; Keir (Xen.org); jbeulich@xxxxxxxx; Andrew Cooper
> >>> Cc: Kevin Tian; zhiyuan.lv@xxxxxxxxx
> >>> Subject: [PATCH v4 1/2] Differentiate IO/mem resources tracked by
> ioreq
> >>> server
> >>>
> >>> Currently in ioreq server, guest write-protected ram pages are
> >>> tracked in the same rangeset with device mmio resources. Yet
> >>> unlike device mmio, which can be in big chunks, the guest write-
> >>> protected pages may be discrete ranges with 4K bytes each.
> >>
> >> Would the interfaces be better using xen_pfn_t rather than using
> >> uint64_t to pass addresses round where the bottom 12 bits will always
> >> be zero?
> >>
> >>    Paul
> >
> > Thank you, Paul.
> > Well, I'm not quite sure if the bottom 12 bits of the address are zero.
> > I had thought these addresses are just guest physical ones causing
> > the ept violation, and not aligned down to its page address, like the
> > MMIO. So, if our handler code has already done that, using xen_pfn_t
> > could be better(my developing machine encountered some hardware
> > problem, will check the code tomorrow) :)
> >
> > Yu
> 
> I just checked the code in hvm_select_ioreq_server(), and found value
> of address is not aligned down, and the lower 12 bits are not zero.
> So I wonder, is it necessary to do the shift to get a gfn, and what's
> the benefit?
> 

Well, you can only make whole pages mmio_dm_write and I was assuming an 
emulator that did so would be interested in writes anywhere in the page. Maybe 
that assumption is wrong?

  Paul

> Thanks
> Yu
> >>
> >>>
> >>> This patch uses a seperate rangeset for the guest ram pages.
> >>> And a new ioreq type, IOREQ_TYPE_WP_MEM, is defined.
> >>>
> >>> Note: Previously, a new hypercall or subop was suggested to map
> >>> write-protected pages into ioreq server. However, it turned out
> >>> handler of this new hypercall would be almost the same with the
> >>> existing pair - HVMOP_[un]map_io_range_to_ioreq_server, and there's
> >>> already a type parameter in this hypercall. So no new hypercall
> >>> defined, only a new type is introduced.
> >>>
> >>> Signed-off-by: Yu Zhang <yu.c.zhang@xxxxxxxxxxxxxxx>
> >>> ---
> >>>   tools/libxc/include/xenctrl.h    | 31 ++++++++++++++++++++++
> >>>   tools/libxc/xc_domain.c          | 55
> >>> ++++++++++++++++++++++++++++++++++++++++
> >>>   xen/arch/x86/hvm/hvm.c           | 26 ++++++++++++++++++-
> >>>   xen/include/asm-x86/hvm/domain.h |  4 +--
> >>>   xen/include/public/hvm/hvm_op.h  |  1 +
> >>>   xen/include/public/hvm/ioreq.h   |  1 +
> >>>   6 files changed, 115 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/tools/libxc/include/xenctrl.h
> >>> b/tools/libxc/include/xenctrl.h
> >>> index de3c0ad..53f196d 100644
> >>> --- a/tools/libxc/include/xenctrl.h
> >>> +++ b/tools/libxc/include/xenctrl.h
> >>> @@ -2010,6 +2010,37 @@ int
> >>> xc_hvm_unmap_io_range_from_ioreq_server(xc_interface *xch,
> >>>                                               int is_mmio,
> >>>                                               uint64_t start,
> >>>                                               uint64_t end);
> >>> +/**
> >>> + * This function registers a range of write-protected memory for
> >>> emulation.
> >>> + *
> >>> + * @parm xch a handle to an open hypervisor interface.
> >>> + * @parm domid the domain id to be serviced
> >>> + * @parm id the IOREQ Server id.
> >>> + * @parm start start of range
> >>> + * @parm end end of range (inclusive).
> >>> + * @return 0 on success, -1 on failure.
> >>> + */
> >>> +int xc_hvm_map_mem_range_to_ioreq_server(xc_interface *xch,
> >>> +                                        domid_t domid,
> >>> +                                        ioservid_t id,
> >>> +                                        uint64_t start,
> >>> +                                        uint64_t end);
> >>> +
> >>> +/**
> >>> + * This function deregisters a range of write-protected memory for
> >>> emulation.
> >>> + *
> >>> + * @parm xch a handle to an open hypervisor interface.
> >>> + * @parm domid the domain id to be serviced
> >>> + * @parm id the IOREQ Server id.
> >>> + * @parm start start of range
> >>> + * @parm end end of range (inclusive).
> >>> + * @return 0 on success, -1 on failure.
> >>> + */
> >>> +int xc_hvm_unmap_mem_range_from_ioreq_server(xc_interface
> *xch,
> >>> +                                            domid_t domid,
> >>> +                                            ioservid_t id,
> >>> +                                            uint64_t start,
> >>> +                                            uint64_t end);
> >>>
> >>>   /**
> >>>    * This function registers a PCI device for config space emulation.
> >>> diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
> >>> index 2ee26fb..42aeba9 100644
> >>> --- a/tools/libxc/xc_domain.c
> >>> +++ b/tools/libxc/xc_domain.c
> >>> @@ -1552,6 +1552,61 @@ int
> >>> xc_hvm_unmap_io_range_from_ioreq_server(xc_interface *xch,
> domid_t
> >>> domid,
> >>>       return rc;
> >>>   }
> >>>
> >>> +int xc_hvm_map_mem_range_to_ioreq_server(xc_interface *xch,
> >>> domid_t domid,
> >>> +                                        ioservid_t id, uint64_t
> >>> start, uint64_t end)
> >>> +{
> >>> +    DECLARE_HYPERCALL;
> >>> +    DECLARE_HYPERCALL_BUFFER(xen_hvm_io_range_t, arg);
> >>> +    int rc;
> >>> +
> >>> +    arg = xc_hypercall_buffer_alloc(xch, arg, sizeof(*arg));
> >>> +    if ( arg == NULL )
> >>> +        return -1;
> >>> +
> >>> +    hypercall.op     = __HYPERVISOR_hvm_op;
> >>> +    hypercall.arg[0] = HVMOP_map_io_range_to_ioreq_server;
> >>> +    hypercall.arg[1] = HYPERCALL_BUFFER_AS_ARG(arg);
> >>> +
> >>> +    arg->domid = domid;
> >>> +    arg->id = id;
> >>> +    arg->type = HVMOP_IO_RANGE_WP_MEM;
> >>> +    arg->start = start;
> >>> +    arg->end = end;
> >>> +
> >>> +    rc = do_xen_hypercall(xch, &hypercall);
> >>> +
> >>> +    xc_hypercall_buffer_free(xch, arg);
> >>> +    return rc;
> >>> +}
> >>> +
> >>> +int xc_hvm_unmap_mem_range_from_ioreq_server(xc_interface
> *xch,
> >>> domid_t domid,
> >>> +                                            ioservid_t id, uint64_t
> >>> start, uint64_t end)
> >>> +{
> >>> +    DECLARE_HYPERCALL;
> >>> +    DECLARE_HYPERCALL_BUFFER(xen_hvm_io_range_t, arg);
> >>> +    int rc;
> >>> +
> >>> +    arg = xc_hypercall_buffer_alloc(xch, arg, sizeof(*arg));
> >>> +    if ( arg == NULL )
> >>> +        return -1;
> >>> +
> >>> +    hypercall.op     = __HYPERVISOR_hvm_op;
> >>> +    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_MEM;
> >>> +    arg->start = start;
> >>> +    arg->end = end;
> >>> +
> >>> +    rc = do_xen_hypercall(xch, &hypercall);
> >>> +
> >>> +    xc_hypercall_buffer_free(xch, arg);
> >>> +    return rc;
> >>> +
> >>> +}
> >>> +
> >>>   int xc_hvm_map_pcidev_to_ioreq_server(xc_interface *xch, domid_t
> >>> domid,
> >>>                                         ioservid_t id, uint16_t segment,
> >>>                                         uint8_t bus, uint8_t device,
> >>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> >>> index c957610..0389c0a 100644
> >>> --- a/xen/arch/x86/hvm/hvm.c
> >>> +++ b/xen/arch/x86/hvm/hvm.c
> >>> @@ -938,8 +938,9 @@ static int
> hvm_ioreq_server_alloc_rangesets(struct
> >>> hvm_ioreq_server *s,
> >>>
> >>>           rc = asprintf(&name, "ioreq_server %d %s", s->id,
> >>>                         (i == HVMOP_IO_RANGE_PORT) ? "port" :
> >>> -                      (i == HVMOP_IO_RANGE_MEMORY) ? "memory" :
> >>> +                      (i == HVMOP_IO_RANGE_MEMORY) ? "mmio" :
> >>>                         (i == HVMOP_IO_RANGE_PCI) ? "pci" :
> >>> +                      (i == HVMOP_IO_RANGE_WP_MEM) ? "wp-ed memory" :
> >>>                         "");
> >>>           if ( rc )
> >>>               goto fail;
> >>> @@ -1258,6 +1259,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_MEM:
> >>>                   r = s->range[type];
> >>>                   break;
> >>>
> >>> @@ -1309,6 +1311,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_MEM:
> >>>                   r = s->range[type];
> >>>                   break;
> >>>
> >>> @@ -2523,6 +2526,8 @@ struct hvm_ioreq_server
> >>> *hvm_select_ioreq_server(struct domain *d,
> >>>       uint32_t cf8;
> >>>       uint8_t type;
> >>>       uint64_t addr;
> >>> +    p2m_type_t p2mt;
> >>> +    struct page_info *ram_page;
> >>>
> >>>       if ( list_empty(&d->arch.hvm_domain.ioreq_server.list) )
> >>>           return NULL;
> >>> @@ -2565,6 +2570,17 @@ struct hvm_ioreq_server
> >>> *hvm_select_ioreq_server(struct domain *d,
> >>>       {
> >>>           type = p->type;
> >>>           addr = p->addr;
> >>> +
> >>> +        if ( p->type == IOREQ_TYPE_COPY )
> >>> +        {
> >>> +            ram_page = get_page_from_gfn(d, p->addr >> PAGE_SHIFT,
> >>> +                                         &p2mt, P2M_UNSHARE);
> >>> +            if ( p2mt == p2m_mmio_write_dm )
> >>> +                type = IOREQ_TYPE_WP_MEM;
> >>> +
> >>> +            if ( ram_page )
> >>> +                put_page(ram_page);
> >>> +        }
> >>>       }
> >>>
> >>>       list_for_each_entry ( s,
> >>> @@ -2582,6 +2598,7 @@ struct hvm_ioreq_server
> >>> *hvm_select_ioreq_server(struct domain *d,
> >>>           BUILD_BUG_ON(IOREQ_TYPE_PIO != HVMOP_IO_RANGE_PORT);
> >>>           BUILD_BUG_ON(IOREQ_TYPE_COPY !=
> HVMOP_IO_RANGE_MEMORY);
> >>>           BUILD_BUG_ON(IOREQ_TYPE_PCI_CONFIG !=
> >>> HVMOP_IO_RANGE_PCI);
> >>> +        BUILD_BUG_ON(IOREQ_TYPE_WP_MEM !=
> >>> HVMOP_IO_RANGE_WP_MEM);
> >>>           r = s->range[type];
> >>>
> >>>           switch ( type )
> >>> @@ -2609,6 +2626,13 @@ struct hvm_ioreq_server
> >>> *hvm_select_ioreq_server(struct domain *d,
> >>>               }
> >>>
> >>>               break;
> >>> +
> >>> +        case IOREQ_TYPE_WP_MEM:
> >>> +            end = addr + (p->size * p->count) - 1;
> >>> +            if ( rangeset_contains_range(r, addr, end) )
> >>> +                return s;
> >>> +
> >>> +            break;
> >>>           }
> >>>       }
> >>>
> >>> diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-
> >>> x86/hvm/domain.h
> >>> index 992d5d1..b2e4234 100644
> >>> --- a/xen/include/asm-x86/hvm/domain.h
> >>> +++ b/xen/include/asm-x86/hvm/domain.h
> >>> @@ -48,8 +48,8 @@ struct hvm_ioreq_vcpu {
> >>>       bool_t           pending;
> >>>   };
> >>>
> >>> -#define NR_IO_RANGE_TYPES (HVMOP_IO_RANGE_PCI + 1)
> >>> -#define MAX_NR_IO_RANGES  256
> >>> +#define NR_IO_RANGE_TYPES (HVMOP_IO_RANGE_WP_MEM + 1)
> >>> +#define MAX_NR_IO_RANGES  8192
> >>>
> >>>   struct hvm_ioreq_server {
> >>>       struct list_head       list_entry;
> >>> diff --git a/xen/include/public/hvm/hvm_op.h
> >>> b/xen/include/public/hvm/hvm_op.h
> >>> index 014546a..c02c91a 100644
> >>> --- a/xen/include/public/hvm/hvm_op.h
> >>> +++ b/xen/include/public/hvm/hvm_op.h
> >>> @@ -331,6 +331,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_MEM 3 /* Write-protected memory
> >>> range */
> >>>       uint64_aligned_t start, end; /* IN - inclusive start and end of
> >>> range */
> >>>   };
> >>>   typedef struct xen_hvm_io_range xen_hvm_io_range_t;
> >>> diff --git a/xen/include/public/hvm/ioreq.h
> >>> b/xen/include/public/hvm/ioreq.h
> >>> index 2e5809b..2b712ac 100644
> >>> --- a/xen/include/public/hvm/ioreq.h
> >>> +++ b/xen/include/public/hvm/ioreq.h
> >>> @@ -35,6 +35,7 @@
> >>>   #define IOREQ_TYPE_PIO          0 /* pio */
> >>>   #define IOREQ_TYPE_COPY         1 /* mmio ops */
> >>>   #define IOREQ_TYPE_PCI_CONFIG   2
> >>> +#define IOREQ_TYPE_WP_MEM       3
> >>>   #define IOREQ_TYPE_TIMEOFFSET   7
> >>>   #define IOREQ_TYPE_INVALIDATE   8 /* mapcache */
> >>>
> >>> --
> >>> 1.9.1
> >>
> >>
> >> _______________________________________________
> >> Xen-devel mailing list
> >> Xen-devel@xxxxxxxxxxxxx
> >> http://lists.xen.org/xen-devel
> >>
> >>
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@xxxxxxxxxxxxx
> > http://lists.xen.org/xen-devel
> >
> >

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.