[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] -device xen-platform crashes
From: |
Stefano Stabellini |
Subject: |
Re: [Qemu-devel] -device xen-platform crashes |
Date: |
Wed, 4 Feb 2015 15:37:12 +0000 |
User-agent: |
Alpine 2.02 (DEB 1266 2009-07-14) |
On Mon, 2 Feb 2015, Markus Armbruster wrote:
> Stefano Stabellini <address@hidden> writes:
>
> > On Fri, 30 Jan 2015, Markus Armbruster wrote:
> >> Stefano Stabellini <address@hidden> writes:
> >>
> >> > On Thu, 29 Jan 2015, Markus Armbruster wrote:
> >> >> Stefano Stabellini <address@hidden> 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.
>