Re: [Xen-devel] [PATCH v3 2/2] Xen: Use the ioreq-server API when available

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Thu, 16 Oct 2014, Paul Durrant wrote:
> > -----Original Message-----
> > From: Stefano Stabellini [mailto:stefano.stabellini@xxxxxxxxxxxxx]
> > Sent: 15 October 2014 15:38
> > To: Paul Durrant
> > Cc: qemu-devel@xxxxxxxxxx; xen-devel@xxxxxxxxxxxxxxxxxxxx; Stefano
> > Stabellini; Peter Maydell; Paolo Bonzini; Michael Tokarev; Stefan Hajnoczi;
> > Stefan Weil; Olaf Hering; Gerd Hoffmann; Alexey Kardashevskiy; Alexander
> > Graf
> > Subject: Re: [PATCH v3 2/2] Xen: Use the ioreq-server API when available
> > 
> > On Wed, 15 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.
> > >
> > > Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> > 
> > The patch is OK, so you can add my Acked-by.
> > I have a couple of minor comments below. If you need to repost it then
> > would be nice if you could address them.
> > 
> > 
> > > Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> > > Cc: Peter Maydell <peter.maydell@xxxxxxxxxx>
> > > Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> > > Cc: Michael Tokarev <mjt@xxxxxxxxxx>
> > > Cc: Stefan Hajnoczi <stefanha@xxxxxxxxxx>
> > > Cc: Stefan Weil <sw@xxxxxxxxxxx>
> > > Cc: Olaf Hering <olaf@xxxxxxxxx>
> > > Cc: Gerd Hoffmann <kraxel@xxxxxxxxxx>
> > > Cc: Alexey Kardashevskiy <aik@xxxxxxxxx>
> > > Cc: Alexander Graf <agraf@xxxxxxx>
> > > ---
> > >  configure                   |   29 ++++++
> > >  include/hw/xen/xen_common.h |  222
> > +++++++++++++++++++++++++++++++++++++++++++
> > >  trace-events                |    8 ++
> > >  xen-hvm.c                   |  174 +++++++++++++++++++++++++++++----
> > >  4 files changed, 412 insertions(+), 21 deletions(-)
> > >
> > 
> > [...]
> > 
> > > diff --git a/xen-hvm.c b/xen-hvm.c
> > > index 05e522c..0bbbf2a 100644
> > > --- a/xen-hvm.c
> > > +++ b/xen-hvm.c
> > > @@ -62,9 +62,6 @@ static inline ioreq_t
> > *xen_vcpu_ioreq(shared_iopage_t *shared_page, int vcpu)
> > >  }
> > >  #  define FMT_ioreq_size "u"
> > >  #endif
> > > -#ifndef HVM_PARAM_BUFIOREQ_EVTCHN
> > > -#define HVM_PARAM_BUFIOREQ_EVTCHN 26
> > > -#endif
> > >
> > >  #define BUFFER_IO_MAX_DELAY  100
> > >
> > > @@ -78,6 +75,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 +90,8 @@ typedef struct XenIOState {
> > >
> > >      struct xs_handle *xenstore;
> > >      MemoryListener memory_listener;
> > > +    MemoryListener io_listener;
> > > +    DeviceListener device_listener;
> > >      QLIST_HEAD(, XenPhysmap) physmap;
> > >      hwaddr free_phys_offset;
> > >      const XenPhysmap *log_for_dirtybit;
> > > @@ -442,12 +442,23 @@ static void xen_set_memory(struct
> > MemoryListener *listener,
> > >      bool log_dirty = memory_region_is_logging(section->mr);
> > >      hvmmem_type_t mem_type;
> > >
> > > +    if (section->mr == &ram_memory) {
> > > +        return;
> > > +    } else {
> > > +        if (add) {
> > > +            xen_map_memory_section(xen_xc, xen_domid, state->ioservid,
> > > +                                   section);
> > > +        } else {
> > > +            xen_unmap_memory_section(xen_xc, xen_domid, state->ioservid,
> > > +                                     section);
> > > +        }
> > > +    }
> > >      if (!memory_region_is_ram(section->mr)) {
> > >          return;
> > >      }
> > >
> > > -    if (!(section->mr != &ram_memory
> > > -          && ( (log_dirty && add) || (!log_dirty && !add)))) {
> > > +    if (!(log_dirty && add) && !(!log_dirty && !add)) {
> > >          return;
> > 
> > if (!((log_dirty && add) || (!log_dirty && !add)))
> > 
> 
> Thinking some more about what Andrew said, this is even more simply expressed 
> as
> 
> if (add != log_dirty)
> 
> is it not?

Yes, I think that should work.

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