On Mon, 2 Feb 2015, Markus Armbruster wrote: > Stefano Stabellini <stefano.stabell...@eu.citrix.com> writes: > > > On Fri, 30 Jan 2015, Markus Armbruster wrote: > >> Stefano Stabellini <stefano.stabell...@eu.citrix.com> writes: > >> > >> > On Thu, 29 Jan 2015, Markus Armbruster wrote: > >> >> Stefano Stabellini <stefano.stabell...@eu.citrix.com> writes: > >> >> > >> >> > On Thu, 29 Jan 2015, Markus Armbruster wrote: > >> >> >> Reproducer: qemu -nodefaults -S -display none -device xen-platform > >> >> >> > >> >> >> Yes, xen-platform makes no sense without Xen, but it shouldn't crash. > >> >> > > >> >> > Is it just a matter of doing the following? > >> >> > > >> >> > diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c > >> >> > index 28b324a..40ae1f3 100644 > >> >> > --- a/hw/i386/xen/xen_platform.c > >> >> > +++ b/hw/i386/xen/xen_platform.c > >> >> > @@ -177,6 +177,10 @@ static void platform_fixed_ioport_writeb(void > >> >> > *opaque, uint32_t addr, uint32_t v > >> >> > { > >> >> > PCIXenPlatformState *s = opaque; > >> >> > > >> >> > + if (!xen_enabled()) { > >> >> > + return; > >> >> > + } > >> >> > + > >> >> > switch (addr) { > >> >> > case 0: /* Platform flags */ { > >> >> > hvmmem_type_t mem_type = (val & PFFLAG_ROM_LOCK) ? > >> >> > >> >> Fixes the crash for me. > >> >> > >> >> Should Xen-only devices fail to realize when !xen_enabled()? > >> > > >> > Accelerators are configured after device registration unfortunately. > >> > >> Realization happens even later, doesn't it? > > > > Yes, you are right. > > > > > >> With this patch: > >> > >> @@ -389,6 +393,11 @@ static int xen_platform_initfn(PCIDevice *dev) > >> PCIXenPlatformState *d = XEN_PLATFORM(dev); > >> uint8_t *pci_conf; > >> > >> + if (!xen_enabled()) { > >> + error_report("Nope"); > >> + return -1; > >> + } > >> + > >> pci_conf = dev->config; > >> > >> pci_set_word(pci_conf + PCI_COMMAND, PCI_COMMAND_IO | > >> PCI_COMMAND_MEMORY); > >> > >> I get: > >> > >> $ upstream-qemu -nodefaults -S -display none -device xen-platform > >> upstream-qemu: -device xen-platform: Nope > >> upstream-qemu: -device xen-platform: Device initialization failed. > >> upstream-qemu: -device xen-platform: Device 'xen-platform' could not > >> be initialized > >> [Exit 1 ] > >> > >> Note that error_report() is a bit problematic in init methods. The best > >> solution is to convert the device to realize (requires my "pci: Partial > >> conversion to realize" series) and use error_setg(). > > > > I agree that this is much better than breaking at runtime for no clear > > reason. Are you going to submit a proper patch? > > Can do. Best for all the xen-only PCI devices, not just this one.
Uhm.. what are the other Xen-only PCI devices? All the other Xen "devices" are actually PV backends. > I'd prefer to do it on top of a conversion to realize. Makes the task > depend on [PATCH 00/10] pci: Partial conversion to realize. >