Re: [PATCH] pc: piix4_pm: init legacy PCI hotplug when running on Xen

Re: [PATCH] pc: piix4_pm: init legacy PCI hotplug when running on Xen

[Stefano Stabellini]

Konrad,
this is another bug fix for QEMU: pci hotplug doesn't work when
xen_platform_pci=0 without this.

I think we should have it in 4.5. What do you think?

- Stefano

On Thu, 13 Nov 2014, Paolo Bonzini wrote:
> On 13/11/2014 03:30, Li Liang wrote:
> > If user starts QEMU with "-machine pc,accel=xen", then
> > compat property in xenfv won't work and it would cause error:
> > "Unsupported bus. Bus doesn't have property 'acpi-pcihp-bsel' set"
> > when PCI device is added with -device on QEMU CLI.
> > 
> > In case of Xen instead of using compat property, just use the fact
> > that xen doesn't use QEMU's fw_cfg/acpi tables to switch piix4_pm
> > into legacy PCI hotplug mode when Xen is enabled.
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > [liang.z.li@intel.com: use "xen_enabled()" instead of "!fw_cfg"]
> > Signed-off-by: Li Liang <liang.z.li@intel.com>
> > ---
> >  hw/acpi/piix4.c   |  4 ++++
> >  hw/i386/pc_piix.c | 11 -----------
> >  2 files changed, 4 insertions(+), 11 deletions(-)
> > 
> > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> > index 78c0a6d..481a16c 100644
> > --- a/hw/acpi/piix4.c
> > +++ b/hw/acpi/piix4.c
> > @@ -36,6 +36,7 @@
> >  #include "hw/mem/pc-dimm.h"
> >  #include "hw/acpi/memory_hotplug.h"
> >  #include "hw/acpi/acpi_dev_interface.h"
> > +#include "hw/xen/xen.h"
> >  
> >  //#define DEBUG
> >  
> > @@ -501,6 +502,9 @@ I2CBus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
> >      s->irq = sci_irq;
> >      s->smi_irq = smi_irq;
> >      s->kvm_enabled = kvm_enabled;
> > +    if (xen_enabled()) {
> > +        s->use_acpi_pci_hotplug = false;
> > +    }
> >  
> >      qdev_init_nofail(dev);
> >  
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index b559181..7bb97a4 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -916,17 +916,6 @@ static QEMUMachine xenfv_machine = {
> >      .max_cpus = HVM_MAX_VCPUS,
> >      .default_machine_opts = "accel=xen",
> >      .hot_add_cpu = pc_hot_add_cpu,
> > -    .compat_props = (GlobalProperty[]) {
> > -        /* xenfv has no fwcfg and so does not load acpi from QEMU.
> > -         * as such new acpi features don't work.
> > -         */
> > -        {
> > -            .driver   = "PIIX4_PM",
> > -            .property = "acpi-pci-hotplug-with-bridge-support",
> > -            .value    = "off",
> > -        },
> > -        { /* end of list */ }
> > -    },
> >  };
> >  #endif
> >  
> > 
> 
> Acked-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> Stefano, are you going to send the pull request yourself?
> 
> Paolo
> 

Re: [PATCH] pc: piix4_pm: init legacy PCI hotplug when running on Xen

[Konrad Rzeszutek Wilk]

On Fri, Nov 14, 2014 at 01:57:21PM +0000, Stefano Stabellini wrote:
> Konrad,
> this is another bug fix for QEMU: pci hotplug doesn't work when
> xen_platform_pci=0 without this.

Yes.
> 
> I think we should have it in 4.5. What do you think?

Do you believe we should first get an Tested-by from the Intel QA folks?

Thanks.
> 
> - Stefano
> 
> On Thu, 13 Nov 2014, Paolo Bonzini wrote:
> > On 13/11/2014 03:30, Li Liang wrote:
> > > If user starts QEMU with "-machine pc,accel=xen", then
> > > compat property in xenfv won't work and it would cause error:
> > > "Unsupported bus. Bus doesn't have property 'acpi-pcihp-bsel' set"
> > > when PCI device is added with -device on QEMU CLI.
> > > 
> > > In case of Xen instead of using compat property, just use the fact
> > > that xen doesn't use QEMU's fw_cfg/acpi tables to switch piix4_pm
> > > into legacy PCI hotplug mode when Xen is enabled.
> > > 
> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > [liang.z.li@intel.com: use "xen_enabled()" instead of "!fw_cfg"]
> > > Signed-off-by: Li Liang <liang.z.li@intel.com>
> > > ---
> > >  hw/acpi/piix4.c   |  4 ++++
> > >  hw/i386/pc_piix.c | 11 -----------
> > >  2 files changed, 4 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> > > index 78c0a6d..481a16c 100644
> > > --- a/hw/acpi/piix4.c
> > > +++ b/hw/acpi/piix4.c
> > > @@ -36,6 +36,7 @@
> > >  #include "hw/mem/pc-dimm.h"
> > >  #include "hw/acpi/memory_hotplug.h"
> > >  #include "hw/acpi/acpi_dev_interface.h"
> > > +#include "hw/xen/xen.h"
> > >  
> > >  //#define DEBUG
> > >  
> > > @@ -501,6 +502,9 @@ I2CBus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
> > >      s->irq = sci_irq;
> > >      s->smi_irq = smi_irq;
> > >      s->kvm_enabled = kvm_enabled;
> > > +    if (xen_enabled()) {
> > > +        s->use_acpi_pci_hotplug = false;
> > > +    }
> > >  
> > >      qdev_init_nofail(dev);
> > >  
> > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > > index b559181..7bb97a4 100644
> > > --- a/hw/i386/pc_piix.c
> > > +++ b/hw/i386/pc_piix.c
> > > @@ -916,17 +916,6 @@ static QEMUMachine xenfv_machine = {
> > >      .max_cpus = HVM_MAX_VCPUS,
> > >      .default_machine_opts = "accel=xen",
> > >      .hot_add_cpu = pc_hot_add_cpu,
> > > -    .compat_props = (GlobalProperty[]) {
> > > -        /* xenfv has no fwcfg and so does not load acpi from QEMU.
> > > -         * as such new acpi features don't work.
> > > -         */
> > > -        {
> > > -            .driver   = "PIIX4_PM",
> > > -            .property = "acpi-pci-hotplug-with-bridge-support",
> > > -            .value    = "off",
> > > -        },
> > > -        { /* end of list */ }
> > > -    },
> > >  };
> > >  #endif
> > >  
> > > 
> > 
> > Acked-by: Paolo Bonzini <pbonzini@redhat.com>
> > 
> > Stefano, are you going to send the pull request yourself?
> > 
> > Paolo
> > 

Re: [PATCH] pc: piix4_pm: init legacy PCI hotplug when running on Xen

[Stefano Stabellini]

On Fri, 14 Nov 2014, Konrad Rzeszutek Wilk wrote:
> On Fri, Nov 14, 2014 at 01:57:21PM +0000, Stefano Stabellini wrote:
> > Konrad,
> > this is another bug fix for QEMU: pci hotplug doesn't work when
> > xen_platform_pci=0 without this.
> 
> Yes.
> > 
> > I think we should have it in 4.5. What do you think?
> 
> Do you believe we should first get an Tested-by from the Intel QA folks?
 
Liang at Intel was the one to fix and resend. Liang, could you please
test this patch on qemu-xen on xen-unstable? Thanks!


> Thanks.
> > 
> > - Stefano
> > 
> > On Thu, 13 Nov 2014, Paolo Bonzini wrote:
> > > On 13/11/2014 03:30, Li Liang wrote:
> > > > If user starts QEMU with "-machine pc,accel=xen", then
> > > > compat property in xenfv won't work and it would cause error:
> > > > "Unsupported bus. Bus doesn't have property 'acpi-pcihp-bsel' set"
> > > > when PCI device is added with -device on QEMU CLI.
> > > > 
> > > > In case of Xen instead of using compat property, just use the fact
> > > > that xen doesn't use QEMU's fw_cfg/acpi tables to switch piix4_pm
> > > > into legacy PCI hotplug mode when Xen is enabled.
> > > > 
> > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > > [liang.z.li@intel.com: use "xen_enabled()" instead of "!fw_cfg"]
> > > > Signed-off-by: Li Liang <liang.z.li@intel.com>
> > > > ---
> > > >  hw/acpi/piix4.c   |  4 ++++
> > > >  hw/i386/pc_piix.c | 11 -----------
> > > >  2 files changed, 4 insertions(+), 11 deletions(-)
> > > > 
> > > > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> > > > index 78c0a6d..481a16c 100644
> > > > --- a/hw/acpi/piix4.c
> > > > +++ b/hw/acpi/piix4.c
> > > > @@ -36,6 +36,7 @@
> > > >  #include "hw/mem/pc-dimm.h"
> > > >  #include "hw/acpi/memory_hotplug.h"
> > > >  #include "hw/acpi/acpi_dev_interface.h"
> > > > +#include "hw/xen/xen.h"
> > > >  
> > > >  //#define DEBUG
> > > >  
> > > > @@ -501,6 +502,9 @@ I2CBus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
> > > >      s->irq = sci_irq;
> > > >      s->smi_irq = smi_irq;
> > > >      s->kvm_enabled = kvm_enabled;
> > > > +    if (xen_enabled()) {
> > > > +        s->use_acpi_pci_hotplug = false;
> > > > +    }
> > > >  
> > > >      qdev_init_nofail(dev);
> > > >  
> > > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > > > index b559181..7bb97a4 100644
> > > > --- a/hw/i386/pc_piix.c
> > > > +++ b/hw/i386/pc_piix.c
> > > > @@ -916,17 +916,6 @@ static QEMUMachine xenfv_machine = {
> > > >      .max_cpus = HVM_MAX_VCPUS,
> > > >      .default_machine_opts = "accel=xen",
> > > >      .hot_add_cpu = pc_hot_add_cpu,
> > > > -    .compat_props = (GlobalProperty[]) {
> > > > -        /* xenfv has no fwcfg and so does not load acpi from QEMU.
> > > > -         * as such new acpi features don't work.
> > > > -         */
> > > > -        {
> > > > -            .driver   = "PIIX4_PM",
> > > > -            .property = "acpi-pci-hotplug-with-bridge-support",
> > > > -            .value    = "off",
> > > > -        },
> > > > -        { /* end of list */ }
> > > > -    },
> > > >  };
> > > >  #endif
> > > >  
> > > > 
> > > 
> > > Acked-by: Paolo Bonzini <pbonzini@redhat.com>
> > > 
> > > Stefano, are you going to send the pull request yourself?
> > > 
> > > Paolo
> > > 
> 

Re: [PATCH] pc: piix4_pm: init legacy PCI hotplug when running on Xen

[Li, Liang Z]

> > > Konrad,
> > > this is another bug fix for QEMU: pci hotplug doesn't work when
> > > xen_platform_pci=0 without this.
> >
> > Yes.
> > >
> > >I think we should have it in 4.5. What do yo  think?
> >
> > Do you believe we should first get an Tested-by from the Intel QA folks?
 
> Liang at Intel was the one to fix and resend. Liang, could you please test this patch on qemu-xen on xen-unstable? Thanks! 

I have verified this patch can fix the bug  for QEMU: pci hotplug doesn't work when  xen_platform_pci=0,  my original intention  was to fix this bug, so I resent the patch.

Liang

Re: [PATCH] pc: piix4_pm: init legacy PCI hotplug when running on Xen

[Stefano Stabellini]

On Sun, 16 Nov 2014, Li, Liang Z wrote:
> > > > Konrad,
> > > > this is another bug fix for QEMU: pci hotplug doesn't work when
> > > > xen_platform_pci=0 without this.
> > >
> > > Yes.
> > > >
> > > >I think we should have it in 4.5. What do yo  think?
> > >
> > > Do you believe we should first get an Tested-by from the Intel QA folks?
>  
> > Liang at Intel was the one to fix and resend. Liang, could you please test this patch on qemu-xen on xen-unstable? Thanks! 
> 
> I have verified this patch can fix the bug  for QEMU: pci hotplug doesn't work when  xen_platform_pci=0,  my original intention  was to fix this bug, so I resent the patch.

In that case I'll go ahead and backport it to qemu-xen for 4.5.

Re: [PATCH] pc: piix4_pm: init legacy PCI hotplug when running on Xen

[Konrad Rzeszutek Wilk]

On Mon, Nov 17, 2014 at 11:17:45AM +0000, Stefano Stabellini wrote:
> On Sun, 16 Nov 2014, Li, Liang Z wrote:
> > > > > Konrad,
> > > > > this is another bug fix for QEMU: pci hotplug doesn't work when
> > > > > xen_platform_pci=0 without this.
> > > >
> > > > Yes.
> > > > >
> > > > >I think we should have it in 4.5. What do yo  think?
> > > >
> > > > Do you believe we should first get an Tested-by from the Intel QA folks?
> >  
> > > Liang at Intel was the one to fix and resend. Liang, could you please test this patch on qemu-xen on xen-unstable? Thanks! 
> > 
> > I have verified this patch can fix the bug  for QEMU: pci hotplug doesn't work when  xen_platform_pci=0,  my original intention  was to fix this bug, so I resent the patch.
> 
> In that case I'll go ahead and backport it to qemu-xen for 4.5.

Thank you!