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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Wed, 15 Oct 2014, Andrew Cooper wrote:
> On 15/10/14 15:51, 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)))
> >>
> > I'm not sure which is better TBH.
> 
> I set simplification problems like this to my Part 1a digital
> electronics supervisees.
> 
> This is "if (!(log_dirty ^ add))" as they are both booleans, and reads
> rather more easily that either of the above.
 
you are funny

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