Re: [Qemu-ppc] [PATCH 3/3] xics/spapr: Detect old KVM XICS on POWER9 hosts

[David Gibson]

On Thu, Jun 13, 2019 at 06:45:05PM +0200, Greg Kurz wrote:
> Older KVMs on POWER9 don't support destroying/recreating a KVM XICS
> device, which is required by 'dual' interrupt controller mode. This
> causes QEMU to emit a warning when the guest is rebooted and to fall
> back on XICS emulation:
> 
> qemu-system-ppc64: warning: kernel_irqchip allowed but unavailable:
>  Error on KVM_CREATE_DEVICE for XICS: File exists
> 
> If kernel irqchip is required, QEMU will thus exit when the guest is
> first rebooted. Failing QEMU this late may be a painful experience
> for the user.
> 
> Detect that and exit at machine init instead.
> 
> Signed-off-by: Greg Kurz <address@hidden>

Applied, thanks.

> ---
>  docs/specs/ppc-spapr-xive.rst |    4 ++--
>  hw/intc/xics_kvm.c            |   30 ++++++++++++++++++++++++++++++
>  hw/ppc/spapr_irq.c            |   13 +++++++++++++
>  include/hw/ppc/xics_spapr.h   |    1 +
>  4 files changed, 46 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/specs/ppc-spapr-xive.rst b/docs/specs/ppc-spapr-xive.rst
> index 7a64c9d04951..6159bc6eed62 100644
> --- a/docs/specs/ppc-spapr-xive.rst
> +++ b/docs/specs/ppc-spapr-xive.rst
> @@ -142,8 +142,8 @@ xics            XICS KVM       XICS emul.     XICS KVM
>  (3) QEMU fails at CAS with ``Guest requested unavailable interrupt
>      mode (XICS), either don't set the ic-mode machine property or try
>      ic-mode=xics or ic-mode=dual``
> -(4) QEMU/KVM incompatibility due to device destruction in reset. This
> -    needs to be addressed more cleanly with an error.
> +(4) QEMU/KVM incompatibility due to device destruction in reset. QEMU fails
> +    with ``KVM is too old to support ic-mode=dual,kernel-irqchip=on``
>  
>  
>  XIVE Device tree properties
> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
> index 5c4208f43008..c7f8f5edd257 100644
> --- a/hw/intc/xics_kvm.c
> +++ b/hw/intc/xics_kvm.c
> @@ -452,3 +452,33 @@ void xics_kvm_disconnect(SpaprMachineState *spapr, Error 
> **errp)
>      /* Clear the presenter from the VCPUs */
>      kvm_disable_icps();
>  }
> +
> +/*
> + * This is a heuristic to detect older KVMs on POWER9 hosts that don't
> + * support destruction of a KVM XICS device while the VM is running.
> + * Required to start a spapr machine with ic-mode=dual,kernel-irqchip=on.
> + */
> +bool xics_kvm_has_broken_disconnect(SpaprMachineState *spapr)
> +{
> +    int rc;
> +
> +    rc = kvm_create_device(kvm_state, KVM_DEV_TYPE_XICS, false);
> +    if (rc < 0) {
> +        /*
> +         * The error is ignored on purpose. The KVM XICS setup code
> +         * will catch it again anyway. The goal here is to see if
> +         * close() actually destroys the device or not.
> +         */
> +        return false;
> +    }
> +
> +    close(rc);
> +
> +    rc = kvm_create_device(kvm_state, KVM_DEV_TYPE_XICS, false);
> +    if (rc >= 0) {
> +        close(rc);
> +        return false;
> +    }
> +
> +    return errno == EEXIST;
> +}
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index dfb99f35ea00..75654fc67aba 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -669,6 +669,19 @@ static void spapr_irq_check(SpaprMachineState *spapr, 
> Error **errp)
>              return;
>          }
>      }
> +
> +    /*
> +     * On a POWER9 host, some older KVM XICS devices cannot be destroyed and
> +     * re-created. Detect that early to avoid QEMU to exit later when the
> +     * guest reboots.
> +     */
> +    if (kvm_enabled() &&
> +        spapr->irq == &spapr_irq_dual &&
> +        machine_kernel_irqchip_required(machine) &&
> +        xics_kvm_has_broken_disconnect(spapr)) {
> +        error_setg(errp, "KVM is too old to support 
> ic-mode=dual,kernel-irqchip=on");
> +        return;
> +    }
>  }
>  
>  /*
> diff --git a/include/hw/ppc/xics_spapr.h b/include/hw/ppc/xics_spapr.h
> index 6c1d9ee55945..d968f2499ca7 100644
> --- a/include/hw/ppc/xics_spapr.h
> +++ b/include/hw/ppc/xics_spapr.h
> @@ -35,6 +35,7 @@ void spapr_dt_xics(SpaprMachineState *spapr, uint32_t 
> nr_servers, void *fdt,
>                     uint32_t phandle);
>  int xics_kvm_init(SpaprMachineState *spapr, Error **errp);
>  void xics_kvm_disconnect(SpaprMachineState *spapr, Error **errp);
> +bool xics_kvm_has_broken_disconnect(SpaprMachineState *spapr);
>  void xics_spapr_init(SpaprMachineState *spapr);
>  void xics_spapr_connect(SpaprMachineState *spapr);
>  
> 

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature