Re: [PATCH 3/8] spapr/xive: Query the characteristics of a source in KVM

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 4714 bytes --]

On Wed, Aug 19, 2020 at 03:08:38PM +0200, Cédric Le Goater wrote:
> When running a guest with a kernel IRQ chip enabled, the XIVE
> characteristics of the interrupts are advertised to the guest in the
> H_INT_GET_SOURCE_INFO hcall. These characteristics depend on the
> underlying HW interrupts but today, QEMU simply advertises its own
> without checking what the host supports. It is not a problem for the
> moment, but POWER10 will (re)add support for StoreEOI and we need a
> way to in sync with the host.
> 
> The KVM_DEV_XIVE_GRP_SOURCE_INFO command lets QEMU query the XIVE
> characteristics of the underlying HW interrupts and override any
> previous setting done by QEMU. This allows the fallback mode, when the
> XIVE device is emulated by QEMU, to use its own custom settings on the
> sources but makes sure that we don't let a guest run with features
> incompatible with KVM.
> 
> It only applies to the StoreEOI feature for the moment.

Urgh.  This means that the source characteristics can change across a
migration, that's kind of a problem.

> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  include/hw/ppc/spapr_xive.h |  2 ++
>  hw/intc/spapr_xive.c        | 20 ++++++++++++++++++++
>  hw/intc/spapr_xive_kvm.c    | 26 ++++++++++++++++++++++++++
>  3 files changed, 48 insertions(+)
> 
> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
> index 1dddcbcb9cdd..3f325723ea74 100644
> --- a/include/hw/ppc/spapr_xive.h
> +++ b/include/hw/ppc/spapr_xive.h
> @@ -84,6 +84,8 @@ void kvmppc_xive_disconnect(SpaprInterruptController *intc);
>  void kvmppc_xive_reset(SpaprXive *xive, Error **errp);
>  int kvmppc_xive_set_source_config(SpaprXive *xive, uint32_t lisn, XiveEAS *eas,
>                                    Error **errp);
> +int kvmppc_xive_get_source_info(SpaprXive *xive, uint32_t lisn, uint64_t *flags,
> +                                 Error **errp);
>  void kvmppc_xive_sync_source(SpaprXive *xive, uint32_t lisn, Error **errp);
>  uint64_t kvmppc_xive_esb_rw(XiveSource *xsrc, int srcno, uint32_t offset,
>                              uint64_t data, bool write);
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index 1fa09f287ac0..943b9958a68b 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -932,6 +932,26 @@ static target_ulong h_int_get_source_info(PowerPCCPU *cpu,
>          args[0] |= SPAPR_XIVE_SRC_STORE_EOI;
>      }
>  
> +    if (kvm_irqchip_in_kernel()) {
> +        Error *local_err = NULL;
> +        uint64_t flags = 0;
> +
> +        kvmppc_xive_get_source_info(xive, lisn, &flags, &local_err);
> +        if (local_err) {
> +            error_report_err(local_err);
> +            return H_HARDWARE;
> +        }
> +
> +        /*
> +         * Override QEMU settings with KVM values
> +         */
> +        if (flags & XIVE_SRC_STORE_EOI) {
> +            args[0] |= SPAPR_XIVE_SRC_STORE_EOI;
> +        } else {
> +            args[0] &= ~SPAPR_XIVE_SRC_STORE_EOI;
> +        }
> +    }
> +
>      /*
>       * Force the use of the H_INT_ESB hcall in case of an LSI
>       * interrupt. This is necessary under KVM to re-trigger the
> diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c
> index e8667ce5f621..90f4509e6959 100644
> --- a/hw/intc/spapr_xive_kvm.c
> +++ b/hw/intc/spapr_xive_kvm.c
> @@ -217,6 +217,32 @@ int kvmppc_xive_set_source_config(SpaprXive *xive, uint32_t lisn, XiveEAS *eas,
>                               &kvm_src, true, errp);
>  }
>  
> +int kvmppc_xive_get_source_info(SpaprXive *xive, uint32_t lisn, uint64_t *flags,
> +                                 Error **errp)
> +{
> +    struct kvm_ppc_xive_src kvm_src = { 0 };
> +    int ret;
> +
> +    /*
> +     * Check that KVM supports the new attribute to query source
> +     * characteristics.
> +     */
> +    if (!kvm_device_check_attr(xive->fd, KVM_DEV_XIVE_GRP_SOURCE_INFO, 0)) {
> +        return 0;
> +    }
> +
> +    ret = kvm_device_access(xive->fd, KVM_DEV_XIVE_GRP_SOURCE_INFO, lisn,
> +                            &kvm_src, false, errp);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +
> +    if (kvm_src.flags & KVM_XIVE_SOURCE_FLAG_STORE_EOI) {
> +        *flags |= XIVE_SRC_STORE_EOI;
> +    }
> +    return 0;
> +}
> +
>  void kvmppc_xive_sync_source(SpaprXive *xive, uint32_t lisn, Error **errp)
>  {
>      kvm_device_access(xive->fd, KVM_DEV_XIVE_GRP_SOURCE_SYNC, lisn,

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]