> -----Original Message----- > From: Paolo Bonzini [mailto:pbonz...@redhat.com] > Sent: 16 October 2014 08:37 > To: Peter Maydell; Paul Durrant > Cc: QEMU Developers; xen-de...@lists.xenproject.org; Stefano Stabellini; > 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 > > Il 15/10/2014 19:30, Peter Maydell ha scritto: > > On 15 October 2014 11:16, Paul Durrant <paul.durr...@citrix.com> 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. > > > > This commit message doesn't mention it, but presumably this is > > all x86-specific given it's in a file which is only used for > > x86 Xen? > > > >> +static void xen_hvm_pre_save(void *opaque) > >> +{ > >> + XenIOState *state = opaque; > >> + > >> + /* Stop servicing emulation requests */ > >> + xen_set_ioreq_server_state(xen_xc, xen_domid, state->ioservid, 0); > >> + xen_destroy_ioreq_server(xen_xc, xen_domid, state->ioservid); > >> +} > >> + > >> +static const VMStateDescription vmstate_xen_hvm = { > >> + .name = "xen-hvm", > >> + .version_id = 4, > >> + .minimum_version_id = 4, > > > > This is new in upstream so why's it starting at version 4? > > > >> + .pre_save = xen_hvm_pre_save, > >> + .fields = (VMStateField[]) { > >> + VMSTATE_END_OF_LIST() > >> + }, > > > > A vmstate which doesn't actually save any state? This seems > > rather suspicious... > > > >> @@ -1060,12 +1185,19 @@ int xen_hvm_init(ram_addr_t > *below_4g_mem_size, ram_addr_t *above_4g_mem_size, > >> xen_ram_init(below_4g_mem_size, above_4g_mem_size, ram_size, > ram_memory); > >> > >> > qemu_add_vm_change_state_handler(xen_hvm_change_state_handler, > state); > >> + vmstate_register(NULL, 0, &vmstate_xen_hvm, state); > > > > Is the new use of vmstate_register() really necessary? > > Usually the state you're saving corresponds to some QOM > > device whose vmsd field you can use instead. > > In this case, it seems like a job for a vmstate change handler. >
I looked at that but it did not seem to give me the right semantic, whereas the pre-save callback gave me exactly the right semantic. Paul > Paolo