[Qemu-devel] [PATCH for-4.0.1] q35: Revert to kernel irqchip

[Qemu-devel] [PATCH for-4.0.1] q35: Revert to kernel irqchip

[Alex Williamson]

Commit b2fc91db8447 ("q35: set split kernel irqchip as default") changed
the default for the pc-q35-4.0 machine type to use split irqchip, which
turned out to have disasterous effects on vfio-pci INTx support.  KVM
resampling irqfds are registered for handling these interrupts, but
these are non-functional in split irqchip mode.  We can't simply test
for split irqchip in QEMU as userspace handling of this interrupt is a
significant performance regression versus KVM handling (GeForce GPUs
assigned to Windows VMs are non-functional without forcing MSI mode or
re-enabling kernel irqchip).

The resolution is to revert the change in default irqchip mode with a
new pc-q35-4.0.1 machine type for qemu-stable while the development
branch makes the same change in the pc-q35-4.1 machine type.  The
qemu-q35-4.0 machine type should not be used in vfio-pci configurations
for devices requiring legacy INTx support without explicitly modifying
the VM configuration to use KVM irqchip.  This new 4.0.1 machine type
makes this change automatically.

Link: https://bugs.launchpad.net/qemu/+bug/1826422
Link: https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg03305.html
Fixes: b2fc91db8447 ("q35: set split kernel irqchip as default")
Cc: qemu-stable@nongnu.org
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

Do we want new stable versions for other archs too or only as needed?

 hw/core/machine.c    |    3 +++
 hw/i386/pc.c         |    3 +++
 hw/i386/pc_q35.c     |   16 ++++++++++++++--
 include/hw/boards.h  |    3 +++
 include/hw/i386/pc.h |    3 +++
 5 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 743fef28982c..5d046a43e3d2 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -24,6 +24,9 @@
 #include "hw/pci/pci.h"
 #include "hw/mem/nvdimm.h"
 
+GlobalProperty hw_compat_4_0[] = {};
+const size_t hw_compat_4_0_len = G_N_ELEMENTS(hw_compat_4_0);
+
 GlobalProperty hw_compat_3_1[] = {
     { "pcie-root-port", "x-speed", "2_5" },
     { "pcie-root-port", "x-width", "1" },
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index f2c15bf1f2c3..d98b737b8f3b 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -115,6 +115,9 @@ struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
 /* Physical Address of PVH entry point read from kernel ELF NOTE */
 static size_t pvh_start_addr;
 
+GlobalProperty pc_compat_4_0[] = {};
+const size_t pc_compat_4_0_len = G_N_ELEMENTS(pc_compat_4_0);
+
 GlobalProperty pc_compat_3_1[] = {
     { "intel-iommu", "dma-drain", "off" },
     { "Opteron_G3" "-" TYPE_X86_CPU, "rdtscp", "off" },
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 372c6b73bebd..45cc29d1adb7 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -357,7 +357,7 @@ static void pc_q35_machine_options(MachineClass *m)
     m->units_per_default_bus = 1;
     m->default_machine_opts = "firmware=bios-256k.bin";
     m->default_display = "std";
-    m->default_kernel_irqchip_split = true;
+    m->default_kernel_irqchip_split = false;
     m->no_floppy = 1;
     machine_class_allow_dynamic_sysbus_dev(m, TYPE_AMD_IOMMU_DEVICE);
     machine_class_allow_dynamic_sysbus_dev(m, TYPE_INTEL_IOMMU_DEVICE);
@@ -365,12 +365,24 @@ static void pc_q35_machine_options(MachineClass *m)
     m->max_cpus = 288;
 }
 
-static void pc_q35_4_0_machine_options(MachineClass *m)
+static void pc_q35_4_0_1_machine_options(MachineClass *m)
 {
     pc_q35_machine_options(m);
     m->alias = "q35";
 }
 
+DEFINE_Q35_MACHINE(v4_0_1, "pc-q35-4.0.1", NULL,
+                   pc_q35_4_0_1_machine_options);
+
+static void pc_q35_4_0_machine_options(MachineClass *m)
+{
+    pc_q35_4_0_1_machine_options(m);
+    m->default_kernel_irqchip_split = true;
+    m->alias = NULL;
+    compat_props_add(m->compat_props, hw_compat_4_0, hw_compat_4_0_len);
+    compat_props_add(m->compat_props, pc_compat_4_0, pc_compat_4_0_len);
+}
+
 DEFINE_Q35_MACHINE(v4_0, "pc-q35-4.0", NULL,
                    pc_q35_4_0_machine_options);
 
diff --git a/include/hw/boards.h b/include/hw/boards.h
index e231860666a1..fe1885cbffa0 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -293,6 +293,9 @@ struct MachineState {
     } \
     type_init(machine_initfn##_register_types)
 
+extern GlobalProperty hw_compat_4_0[];
+extern const size_t hw_compat_4_0_len;
+
 extern GlobalProperty hw_compat_3_1[];
 extern const size_t hw_compat_3_1_len;
 
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index ca65ef18afb4..43df7230a22b 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -293,6 +293,9 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
 int e820_get_num_entries(void);
 bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
 
+extern GlobalProperty pc_compat_4_0[];
+extern const size_t pc_compat_4_0_len;
+
 extern GlobalProperty pc_compat_3_1[];
 extern const size_t pc_compat_3_1_len;
 

Re: [Qemu-devel] [PATCH for-4.0.1] q35: Revert to kernel irqchip

[Daniel P. Berrangé]

On Tue, May 14, 2019 at 01:03:31PM -0600, Alex Williamson wrote:
> Commit b2fc91db8447 ("q35: set split kernel irqchip as default") changed
> the default for the pc-q35-4.0 machine type to use split irqchip, which
> turned out to have disasterous effects on vfio-pci INTx support.  KVM
> resampling irqfds are registered for handling these interrupts, but
> these are non-functional in split irqchip mode.  We can't simply test
> for split irqchip in QEMU as userspace handling of this interrupt is a
> significant performance regression versus KVM handling (GeForce GPUs
> assigned to Windows VMs are non-functional without forcing MSI mode or
> re-enabling kernel irqchip).
> 
> The resolution is to revert the change in default irqchip mode with a
> new pc-q35-4.0.1 machine type for qemu-stable while the development
> branch makes the same change in the pc-q35-4.1 machine type.  The
> qemu-q35-4.0 machine type should not be used in vfio-pci configurations
> for devices requiring legacy INTx support without explicitly modifying
> the VM configuration to use KVM irqchip.  This new 4.0.1 machine type
> makes this change automatically.

If we introduce a pc-q35-4.0.1 machine type in -stable, then VMs
created in stable won't be migratable to future 4.1 unless we also
create this same machine type in master.

If we really want to create a new 4.0.1 machine type, then this
patch needs to go to git master before stable IMHO to guarantee
no regression to master.

> Link: https://bugs.launchpad.net/qemu/+bug/1826422
> Link: https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg03305.html
> Fixes: b2fc91db8447 ("q35: set split kernel irqchip as default")
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
> 
> Do we want new stable versions for other archs too or only as needed?



Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH for-4.0.1] q35: Revert to kernel irqchip

[Alex Williamson]

On Tue, 14 May 2019 20:22:32 +0100
Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Tue, May 14, 2019 at 01:03:31PM -0600, Alex Williamson wrote:
> > Commit b2fc91db8447 ("q35: set split kernel irqchip as default") changed
> > the default for the pc-q35-4.0 machine type to use split irqchip, which
> > turned out to have disasterous effects on vfio-pci INTx support.  KVM
> > resampling irqfds are registered for handling these interrupts, but
> > these are non-functional in split irqchip mode.  We can't simply test
> > for split irqchip in QEMU as userspace handling of this interrupt is a
> > significant performance regression versus KVM handling (GeForce GPUs
> > assigned to Windows VMs are non-functional without forcing MSI mode or
> > re-enabling kernel irqchip).
> > 
> > The resolution is to revert the change in default irqchip mode with a
> > new pc-q35-4.0.1 machine type for qemu-stable while the development
> > branch makes the same change in the pc-q35-4.1 machine type.  The
> > qemu-q35-4.0 machine type should not be used in vfio-pci configurations
> > for devices requiring legacy INTx support without explicitly modifying
> > the VM configuration to use KVM irqchip.  This new 4.0.1 machine type
> > makes this change automatically.  
> 
> If we introduce a pc-q35-4.0.1 machine type in -stable, then VMs
> created in stable won't be migratable to future 4.1 unless we also
> create this same machine type in master.

Yes, I overlooked 4.0.1 on 4.1, I'm working on that now.  Reposting a
v2 of the 4.1 version shortly.

> If we really want to create a new 4.0.1 machine type, then this
> patch needs to go to git master before stable IMHO to guarantee
> no regression to master.

Yes, I'm doing both in parallel, I expect the 4.0.1 support to be
contingent on be the master branch change, thus the second link below.
Thanks,

Alex

> > Link: https://bugs.launchpad.net/qemu/+bug/1826422
> > Link: https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg03305.html
> > Fixes: b2fc91db8447 ("q35: set split kernel irqchip as default")
> > Cc: qemu-stable@nongnu.org
> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > ---
> > 
> > Do we want new stable versions for other archs too or only as needed?  
> 
> 
> 
> Regards,
> Daniel

Re: [Qemu-devel] [PATCH for-4.0.1] q35: Revert to kernel irqchip

[Alex Williamson]

On Tue, 14 May 2019 13:03:31 -0600
Alex Williamson <alex.williamson@redhat.com> wrote:

> Commit b2fc91db8447 ("q35: set split kernel irqchip as default") changed
> the default for the pc-q35-4.0 machine type to use split irqchip, which
> turned out to have disasterous effects on vfio-pci INTx support.  KVM
> resampling irqfds are registered for handling these interrupts, but
> these are non-functional in split irqchip mode.  We can't simply test
> for split irqchip in QEMU as userspace handling of this interrupt is a
> significant performance regression versus KVM handling (GeForce GPUs
> assigned to Windows VMs are non-functional without forcing MSI mode or
> re-enabling kernel irqchip).
> 
> The resolution is to revert the change in default irqchip mode with a
> new pc-q35-4.0.1 machine type for qemu-stable while the development
> branch makes the same change in the pc-q35-4.1 machine type.  The
> qemu-q35-4.0 machine type should not be used in vfio-pci configurations
> for devices requiring legacy INTx support without explicitly modifying
> the VM configuration to use KVM irqchip.  This new 4.0.1 machine type
> makes this change automatically.
> 
> Link: https://bugs.launchpad.net/qemu/+bug/1826422
> Link: https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg03305.html

This link is superseded by a v2 of the mainline patch:

Link: https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg03338.html

I believe this patch is still the proper stable backport though.  Also
to clarify, this patch should be gated on mainline acceptance of the
link above, but clearly there's no clean cherry-pick between mainline
and stable for this, so I'm proposing them in parallel.  Thanks,

Alex

> Fixes: b2fc91db8447 ("q35: set split kernel irqchip as default")
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
> 
> Do we want new stable versions for other archs too or only as needed?
> 
>  hw/core/machine.c    |    3 +++
>  hw/i386/pc.c         |    3 +++
>  hw/i386/pc_q35.c     |   16 ++++++++++++++--
>  include/hw/boards.h  |    3 +++
>  include/hw/i386/pc.h |    3 +++
>  5 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 743fef28982c..5d046a43e3d2 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -24,6 +24,9 @@
>  #include "hw/pci/pci.h"
>  #include "hw/mem/nvdimm.h"
>  
> +GlobalProperty hw_compat_4_0[] = {};
> +const size_t hw_compat_4_0_len = G_N_ELEMENTS(hw_compat_4_0);
> +
>  GlobalProperty hw_compat_3_1[] = {
>      { "pcie-root-port", "x-speed", "2_5" },
>      { "pcie-root-port", "x-width", "1" },
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index f2c15bf1f2c3..d98b737b8f3b 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -115,6 +115,9 @@ struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
>  /* Physical Address of PVH entry point read from kernel ELF NOTE */
>  static size_t pvh_start_addr;
>  
> +GlobalProperty pc_compat_4_0[] = {};
> +const size_t pc_compat_4_0_len = G_N_ELEMENTS(pc_compat_4_0);
> +
>  GlobalProperty pc_compat_3_1[] = {
>      { "intel-iommu", "dma-drain", "off" },
>      { "Opteron_G3" "-" TYPE_X86_CPU, "rdtscp", "off" },
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 372c6b73bebd..45cc29d1adb7 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -357,7 +357,7 @@ static void pc_q35_machine_options(MachineClass *m)
>      m->units_per_default_bus = 1;
>      m->default_machine_opts = "firmware=bios-256k.bin";
>      m->default_display = "std";
> -    m->default_kernel_irqchip_split = true;
> +    m->default_kernel_irqchip_split = false;
>      m->no_floppy = 1;
>      machine_class_allow_dynamic_sysbus_dev(m, TYPE_AMD_IOMMU_DEVICE);
>      machine_class_allow_dynamic_sysbus_dev(m, TYPE_INTEL_IOMMU_DEVICE);
> @@ -365,12 +365,24 @@ static void pc_q35_machine_options(MachineClass *m)
>      m->max_cpus = 288;
>  }
>  
> -static void pc_q35_4_0_machine_options(MachineClass *m)
> +static void pc_q35_4_0_1_machine_options(MachineClass *m)
>  {
>      pc_q35_machine_options(m);
>      m->alias = "q35";
>  }
>  
> +DEFINE_Q35_MACHINE(v4_0_1, "pc-q35-4.0.1", NULL,
> +                   pc_q35_4_0_1_machine_options);
> +
> +static void pc_q35_4_0_machine_options(MachineClass *m)
> +{
> +    pc_q35_4_0_1_machine_options(m);
> +    m->default_kernel_irqchip_split = true;
> +    m->alias = NULL;
> +    compat_props_add(m->compat_props, hw_compat_4_0, hw_compat_4_0_len);
> +    compat_props_add(m->compat_props, pc_compat_4_0, pc_compat_4_0_len);
> +}
> +
>  DEFINE_Q35_MACHINE(v4_0, "pc-q35-4.0", NULL,
>                     pc_q35_4_0_machine_options);
>  
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index e231860666a1..fe1885cbffa0 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -293,6 +293,9 @@ struct MachineState {
>      } \
>      type_init(machine_initfn##_register_types)
>  
> +extern GlobalProperty hw_compat_4_0[];
> +extern const size_t hw_compat_4_0_len;
> +
>  extern GlobalProperty hw_compat_3_1[];
>  extern const size_t hw_compat_3_1_len;
>  
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index ca65ef18afb4..43df7230a22b 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -293,6 +293,9 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
>  int e820_get_num_entries(void);
>  bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
>  
> +extern GlobalProperty pc_compat_4_0[];
> +extern const size_t pc_compat_4_0_len;
> +
>  extern GlobalProperty pc_compat_3_1[];
>  extern const size_t pc_compat_3_1_len;
>  
> 

Re: [Qemu-devel] [PATCH for-4.0.1] q35: Revert to kernel irqchip

[Peter Xu]

On Tue, May 14, 2019 at 02:22:03PM -0600, Alex Williamson wrote:
> On Tue, 14 May 2019 13:03:31 -0600
> Alex Williamson <alex.williamson@redhat.com> wrote:
> 
> > Commit b2fc91db8447 ("q35: set split kernel irqchip as default") changed
> > the default for the pc-q35-4.0 machine type to use split irqchip, which
> > turned out to have disasterous effects on vfio-pci INTx support.  KVM
> > resampling irqfds are registered for handling these interrupts, but
> > these are non-functional in split irqchip mode.  We can't simply test
> > for split irqchip in QEMU as userspace handling of this interrupt is a
> > significant performance regression versus KVM handling (GeForce GPUs
> > assigned to Windows VMs are non-functional without forcing MSI mode or
> > re-enabling kernel irqchip).
> > 
> > The resolution is to revert the change in default irqchip mode with a
> > new pc-q35-4.0.1 machine type for qemu-stable while the development
> > branch makes the same change in the pc-q35-4.1 machine type.  The
> > qemu-q35-4.0 machine type should not be used in vfio-pci configurations
> > for devices requiring legacy INTx support without explicitly modifying
> > the VM configuration to use KVM irqchip.  This new 4.0.1 machine type
> > makes this change automatically.
> > 
> > Link: https://bugs.launchpad.net/qemu/+bug/1826422
> > Link: https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg03305.html
> 
> This link is superseded by a v2 of the mainline patch:
> 
> Link: https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg03338.html
> 
> I believe this patch is still the proper stable backport though.  Also
> to clarify, this patch should be gated on mainline acceptance of the
> link above, but clearly there's no clean cherry-pick between mainline
> and stable for this, so I'm proposing them in parallel.  Thanks,

Agreed.  As long as the 4.1 patch can be accepted, this should be the
correct patch for 4.0-stable AFAICT:

Reviewed-by: Peter Xu <peterx@redhat.com>

Thanks,

-- 
Peter Xu