From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43034) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XeQ8r-00044E-M3 for qemu-devel@nongnu.org; Wed, 15 Oct 2014 11:05:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XeQ8n-0003XM-Oy for qemu-devel@nongnu.org; Wed, 15 Oct 2014 11:05:33 -0400 Received: from smtp02.citrix.com ([66.165.176.63]:31898) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XeQ8n-0003Wq-If for qemu-devel@nongnu.org; Wed, 15 Oct 2014 11:05:29 -0400 Message-ID: <543E8D0A.4000500@citrix.com> Date: Wed, 15 Oct 2014 16:04:42 +0100 From: Andrew Cooper MIME-Version: 1.0 References: <1413364599-7582-1-git-send-email-paul.durrant@citrix.com> <1413364599-7582-3-git-send-email-paul.durrant@citrix.com> <9AAE0902D5BC7E449B7C8E4E778ABCD011102581@AMSPEX01CL01.citrite.net> In-Reply-To: <9AAE0902D5BC7E449B7C8E4E778ABCD011102581@AMSPEX01CL01.citrite.net> Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [Xen-devel] [PATCH v3 2/2] Xen: Use the ioreq-server API when available List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paul Durrant , Stefano Stabellini Cc: Peter Maydell , Olaf Hering , Alexey Kardashevskiy , Stefan Weil , Michael Tokarev , "qemu-devel@nongnu.org" , Alexander Graf , Gerd Hoffmann , Stefan Hajnoczi , "xen-devel@lists.xenproject.org" , Paolo Bonzini On 15/10/14 15:51, Paul Durrant wrote: >> -----Original Message----- >> From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] >> Sent: 15 October 2014 15:38 >> To: Paul Durrant >> Cc: qemu-devel@nongnu.org; xen-devel@lists.xenproject.org; Stefano >> Stabellini; Peter Maydell; Paolo Bonzini; Michael Tokarev; Stefan Hajn= oczi; >> Stefan Weil; Olaf Hering; Gerd Hoffmann; Alexey Kardashevskiy; Alexand= er >> Graf >> Subject: Re: [PATCH v3 2/2] Xen: Use the ioreq-server API when availab= le >> >> 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 >> 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 >>> Cc: Peter Maydell >>> Cc: Paolo Bonzini >>> Cc: Michael Tokarev >>> Cc: Stefan Hajnoczi >>> Cc: Stefan Weil >>> Cc: Olaf Hering >>> Cc: Gerd Hoffmann >>> Cc: Alexey Kardashevskiy >>> Cc: Alexander Graf >>> --- >>> 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 =3D memory_region_is_logging(section->mr); >>> hvmmem_type_t mem_type; >>> >>> + if (section->mr =3D=3D &ram_memory) { >>> + return; >>> + } else { >>> + if (add) { >>> + xen_map_memory_section(xen_xc, xen_domid, state->ioservi= d, >>> + section); >>> + } else { >>> + xen_unmap_memory_section(xen_xc, xen_domid, state->ioser= vid, >>> + section); >>> + } >>> + } >>> if (!memory_region_is_ram(section->mr)) { >>> return; >>> } >>> >>> - if (!(section->mr !=3D &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. ~Andrew