| Message ID | 1445250224-5788-2-git-send-email-stefano.stabellini@eu.citrix.com |
|---|---|
| State | New |
| Headers | show |
On 19/10/2015 12:23, Stefano Stabellini wrote: > The xen-platform code crashes on reset if the xen backend is not > initialized, because it calls xc_hvm_set_mem_type(). Ensure xen-platform > won't be created without initializing the xen backend. > > The assert can't be triggered by the user because the device is not > hotpluggable, and the only code creating it (at pc_xen_hvm_init()) > already checks xen_enabled(). The device is not hotpluggable, but it is accessible with -device. This is by design because IIUC, we want -M xenfv to be phased out in favor of the PC machines plus -device xen-platform. Thus, k->init should be changed to k->realize instead. I guess it can be done on top of this pull request, so I'm not blocking it. Paolo > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > --- > hw/i386/xen/xen_platform.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c > index ee45f03..8682c42 100644 > --- a/hw/i386/xen/xen_platform.c > +++ b/hw/i386/xen/xen_platform.c > @@ -387,6 +387,9 @@ static int xen_platform_initfn(PCIDevice *dev) > PCIXenPlatformState *d = XEN_PLATFORM(dev); > uint8_t *pci_conf; > > + /* Device will crash on reset if xen is not initialized */ > + assert(xen_enabled()); > +
On Mon, Oct 19, 2015 at 12:48:11PM +0200, Paolo Bonzini wrote: > > > On 19/10/2015 12:23, Stefano Stabellini wrote: > > The xen-platform code crashes on reset if the xen backend is not > > initialized, because it calls xc_hvm_set_mem_type(). Ensure xen-platform > > won't be created without initializing the xen backend. > > > > The assert can't be triggered by the user because the device is not > > hotpluggable, and the only code creating it (at pc_xen_hvm_init()) > > already checks xen_enabled(). > > The device is not hotpluggable, but it is accessible with -device. This > is by design because IIUC, we want -M xenfv to be phased out in favor of > the PC machines plus -device xen-platform. Oops, that means the assert() needs to be replaced with proper error reporting. I will do it. > > Thus, k->init should be changed to k->realize instead. I guess it can > be done on top of this pull request, so I'm not blocking it. xen_platform_initfn() is PCIDeviceClass::init, which is called from realize.
diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c index ee45f03..8682c42 100644 --- a/hw/i386/xen/xen_platform.c +++ b/hw/i386/xen/xen_platform.c @@ -387,6 +387,9 @@ static int xen_platform_initfn(PCIDevice *dev) PCIXenPlatformState *d = XEN_PLATFORM(dev); uint8_t *pci_conf; + /* Device will crash on reset if xen is not initialized */ + assert(xen_enabled()); + pci_conf = dev->config; pci_set_word(pci_conf + PCI_COMMAND, PCI_COMMAND_IO | PCI_COMMAND_MEMORY);