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

2019-05-15 Thread 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  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 

Thanks,

-- 
Peter Xu



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

2019-05-14 Thread Alex Williamson
On Tue, 14 May 2019 13:03:31 -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.
> 
> 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-sta...@nongnu.org
> Signed-off-by: Alex Williamson 
> ---
> 
> 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;
> +
>  

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

2019-05-14 Thread Alex Williamson
On Tue, 14 May 2019 20:22:32 +0100
Daniel P. Berrangé  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-sta...@nongnu.org
> > Signed-off-by: Alex Williamson 
> > ---
> > 
> > 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

2019-05-14 Thread 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-sta...@nongnu.org
> Signed-off-by: Alex Williamson 
> ---
> 
> 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 :|