Re: [Qemu-devel] [PATCH] Xen: Use the ioreq-server API when available

[Paul Durrant]

> -----Original Message-----
> From: Stefano Stabellini [mailto:address@hidden
> Sent: 10 October 2014 15:40
> To: Paul Durrant
> Cc: address@hidden; address@hidden; Michael S.
> Tsirkin; Stefano Stabellini; Peter Maydell; Paolo Bonzini; Michael Tokarev;
> Stefan Hajnoczi; Stefan Weil; Andreas Faerber"; Thomas Huth; Peter
> Crosthwaite; Eduardo Habkost; Gerd Hoffmann; Alexey Kardashevskiy;
> Alexander Graf
> Subject: Re: [PATCH] Xen: Use the ioreq-server API when available
> 
> On Fri, 10 Oct 2014, Paul Durrant wrote:
> > The ioreq-server API added to Xen 4.5 offers better security than
> > the existing Xen/QEMU interface because the shared pages that are
> > used to pass emulation request/results back and forth are removed
> > from the guest's memory space before any requests are serviced.
> > This prevents the guest from mapping these pages (they are in a
> > well known location) and attempting to attack QEMU by synthesizing
> > its own request structures. Hence, this patch modifies configure
> > to detect whether the API is available, and adds the necessary
> > code to use the API if it is.
> >
> > The ioreq-server API does require that PCI device models explicitly
> > register with Xen for config space accesses, so to use the API the
> > code in xen-hvm.c needs to be informed as PCI devices are added or
> > removed from PCI buses. This patch therefore also adds a PCI bus
> > listener interface akin to the memory listener interface to fulfil
> > this need.
> >
> > Signed-off-by: Paul Durrant <address@hidden>
> > Cc: Michael S. Tsirkin <address@hidden>
> > Cc: Stefano Stabellini <address@hidden>
> > Cc: Peter Maydell <address@hidden>
> > Cc: Paolo Bonzini <address@hidden>
> > Cc: Michael Tokarev <address@hidden>
> > Cc: Stefan Hajnoczi <address@hidden>
> > Cc: Stefan Weil <address@hidden>
> > Cc: Andreas Faerber" <address@hidden>
> > Cc: Thomas Huth <address@hidden>
> > Cc: Peter Crosthwaite <address@hidden>
> > Cc: Eduardo Habkost <address@hidden>
> > Cc: Gerd Hoffmann <address@hidden>
> > Cc: Alexey Kardashevskiy <address@hidden>
> > Cc: Alexander Graf <address@hidden>
> > ---
> >  configure               |   29 ++++++
> >  hw/pci/pci.c            |   65 ++++++++++++
> >  include/hw/pci/pci.h    |    9 ++
> >  include/qemu/typedefs.h |    1 +
> >  trace-events            |    8 ++
> >  xen-hvm.c               |  251
> +++++++++++++++++++++++++++++++++++++++++++----
> >  6 files changed, 345 insertions(+), 18 deletions(-)
> 
> Please split this patch into two: one to introduce the PCI listener
> stuff and another for the Xen specific changes.
> 

Originally it was, but I was uneasy about introducing an interface with no 
consumers. Is that generally acceptable?

[snip]
> > diff --git a/xen-hvm.c b/xen-hvm.c
> > index 05e522c..031edd9 100644
> > --- a/xen-hvm.c
> > +++ b/xen-hvm.c
> > @@ -78,6 +78,7 @@ typedef struct XenPhysmap {
> >  } XenPhysmap;
> >
> >  typedef struct XenIOState {
> > +    ioservid_t ioservid;
> >      shared_iopage_t *shared_page;
> >      buffered_iopage_t *buffered_io_page;
> >      QEMUTimer *buffered_io_timer;
> > @@ -92,6 +93,8 @@ typedef struct XenIOState {
> >
> >      struct xs_handle *xenstore;
> >      MemoryListener memory_listener;
> > +    MemoryListener io_listener;
> > +    PCIListener pci_listener;
> >      QLIST_HEAD(, XenPhysmap) physmap;
> >      hwaddr free_phys_offset;
> >      const XenPhysmap *log_for_dirtybit;
> > @@ -480,6 +483,20 @@ static void xen_region_add(MemoryListener
> *listener,
> >                             MemoryRegionSection *section)
> >  {
> >      memory_region_ref(section->mr);
> > +
> > +#if CONFIG_XEN_CTRL_INTERFACE_VERSION >= 450
> 
> Please use include/hw/xen/xen_common.h for compat stuff.  In this case
> you could provide an empty implementation of
> xc_hvm_map_io_range_to_ioreq_server. Try to remove ifdefs from
> xen-hvm.c.
>

Ok, I can do it that way; I was only following suit with the already #ifdefed 
code in xen-hvm.c.

  Paul