Re: [Qemu-devel] [PATCH v8 07/12] spapr: add an extra OV5 field to the sPAPR IRQ backend

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

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

On Tue, Dec 11, 2018 at 11:38:18PM +0100, Cédric Le Goater wrote:
> The interrupt modes supported by the hypervisor are advertised to the
> guest with new bits definitions of the option vector 5 of property
> "ibm,arch-vec-5-platform-support. The byte 23 bits 0-1 of the OV5 are
> defined as follow :
> 
>   0b00   PAPR 2.7 and earlier (Legacy systems)
>   0b01   XIVE Exploitation mode only
>   0b10   Either available
> 
> If the client/guest selects the XIVE interrupt mode, it informs the
> hypervisor by returning the value 0b01 in byte 23 bits 0-1. A 0b00
> value indicates the use of the XICS interrupt mode (Legacy systems).
> 
> The sPAPR IRQ backend is extended with these definitions and the
> values are directly used to populate the "ibm,arch-vec-5-platform-support"
> property. The interrupt mode is advertised under TCG and under KVM.
> Although a KVM XIVE device is not yet available, the machine can still
> operate with kernel_irqchip=off. However, we apply a restriction on
> the CPU which is required to be a POWER9 when a XIVE interrupt
> controller is in use.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> 
>  Changes since v7:
> 
>  - improved commit log
>  - introduced a check on CPU type.
> 
>  include/hw/ppc/spapr.h     |  6 ++++++
>  include/hw/ppc/spapr_irq.h |  1 +
>  hw/ppc/spapr.c             | 36 ++++++++++++++++++++++++++++++------
>  hw/ppc/spapr_irq.c         |  3 +++
>  4 files changed, 40 insertions(+), 6 deletions(-)
> 
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 6bf028a02fe2..06765b4e9d79 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -824,5 +824,11 @@ int spapr_caps_post_migration(sPAPRMachineState *spapr);
>  
>  void spapr_check_pagesize(sPAPRMachineState *spapr, hwaddr pagesize,
>                            Error **errp);
> +/*
> + * XIVE definitions
> + */
> +#define SPAPR_OV5_XIVE_LEGACY   0x0
> +#define SPAPR_OV5_XIVE_EXPLOIT  0x40
> +#define SPAPR_OV5_XIVE_BOTH     0x80 /* Only to advertise on the platform */
>  
>  #endif /* HW_SPAPR_H */
> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> index 63061a009b4c..b34d5a00381b 100644
> --- a/include/hw/ppc/spapr_irq.h
> +++ b/include/hw/ppc/spapr_irq.h
> @@ -33,6 +33,7 @@ void spapr_irq_msi_reset(sPAPRMachineState *spapr);
>  typedef struct sPAPRIrq {
>      uint32_t    nr_irqs;
>      uint32_t    nr_msis;
> +    uint8_t     ov5;
>  
>      void (*init)(sPAPRMachineState *spapr, Error **errp);
>      int (*claim)(sPAPRMachineState *spapr, int irq, bool lsi, Error **errp);
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 57c0066edd56..ff138f224a87 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1095,12 +1095,14 @@ static void spapr_dt_rtas(sPAPRMachineState *spapr, void *fdt)
>      spapr_dt_rtas_tokens(fdt, rtas);
>  }
>  
> -/* Prepare ibm,arch-vec-5-platform-support, which indicates the MMU features
> - * that the guest may request and thus the valid values for bytes 24..26 of
> - * option vector 5: */
> -static void spapr_dt_ov5_platform_support(void *fdt, int chosen)
> +/* Prepare ibm,arch-vec-5-platform-support, which indicates the MMU
> + * and the XIVE features that the guest may request and thus the valid
> + * values for bytes 23..26 of option vector 5: */
> +static void spapr_dt_ov5_platform_support(sPAPRMachineState *spapr, void *fdt,
> +                                          int chosen)
>  {
>      PowerPCCPU *first_ppc_cpu = POWERPC_CPU(first_cpu);
> +    sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
>  
>      char val[2 * 4] = {
>          23, 0x00, /* Xive mode, filled in below. */
> @@ -1111,9 +1113,17 @@ static void spapr_dt_ov5_platform_support(void *fdt, int chosen)
>  
>      if (!ppc_check_compat(first_ppc_cpu, CPU_POWERPC_LOGICAL_3_00, 0,
>                            first_ppc_cpu->compat_pvr)) {
> -        /* If we're in a pre POWER9 compat mode then the guest should do hash */
> +        /* If we're in a pre POWER9 compat mode then the guest should do hash
> +         * and use the legacy interrupt mode
> +         */
> +        val[1] = 0x00; /* XICS */
>          val[3] = 0x00; /* Hash */
>      } else if (kvm_enabled()) {
> +        /* If the KVM XIVE device is not available, the machine can
> +         * still operate with kernel_irqchip=off
> +         */
> +        val[1] = smc->irq->ov5;


I know it means another layer of if, but I'd prefer to rearrange this
so setting the intc value is outside the if (kvm_enabled())..
> +
>          if (kvmppc_has_cap_mmu_radix() && kvmppc_has_cap_mmu_hash_v3()) {
>              val[3] = 0x80; /* OV5_MMU_BOTH */
>          } else if (kvmppc_has_cap_mmu_radix()) {
> @@ -1122,6 +1132,9 @@ static void spapr_dt_ov5_platform_support(void *fdt, int chosen)
>              val[3] = 0x00; /* Hash */
>          }
>      } else {
> +        /* In TCG, the interrupt mode is defined by the pseries machine */
> +        val[1] = smc->irq->ov5;

..rather than duplicated in each branch.

> +
>          /* V3 MMU supports both hash and radix in tcg (with dynamic switching) */
>          val[3] = 0xC0;
>      }
> @@ -1189,7 +1202,7 @@ static void spapr_dt_chosen(sPAPRMachineState *spapr, void *fdt)
>          _FDT(fdt_setprop_string(fdt, chosen, "stdout-path", stdout_path));
>      }
>  
> -    spapr_dt_ov5_platform_support(fdt, chosen);
> +    spapr_dt_ov5_platform_support(spapr, fdt, chosen);
>  
>      g_free(stdout_path);
>      g_free(bootlist);
> @@ -2622,6 +2635,17 @@ static void spapr_machine_init(MachineState *machine)
>      /* advertise support for ibm,dyamic-memory-v2 */
>      spapr_ovec_set(spapr->ov5, OV5_DRMEM_V2);
>  
> +    /* advertise XIVE on POWER9 machines */
> +    if (smc->irq->ov5 & SPAPR_OV5_XIVE_EXPLOIT) {
> +        if (ppc_type_check_compat(machine->cpu_type, CPU_POWERPC_LOGICAL_3_00,
> +                                  0, spapr->max_compat_pvr)) {
> +            spapr_ovec_set(spapr->ov5, OV5_XIVE_EXPLOIT);
> +        } else {
> +            error_report("XIVE-only machines require a POWER9 CPU");
> +            exit(1);
> +        }
> +    }
> +
>      /* init CPUs */
>      spapr_init_cpus(spapr);
>  
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index f835ea939cbf..79f06308995b 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -216,6 +216,7 @@ static int spapr_irq_post_load_xics(sPAPRMachineState *spapr, int version_id)
>  sPAPRIrq spapr_irq_xics = {
>      .nr_irqs     = SPAPR_IRQ_XICS_NR_IRQS,
>      .nr_msis     = SPAPR_IRQ_XICS_NR_MSIS,
> +    .ov5         = SPAPR_OV5_XIVE_LEGACY,
>  
>      .init        = spapr_irq_init_xics,
>      .claim       = spapr_irq_claim_xics,
> @@ -342,6 +343,7 @@ static void spapr_irq_reset_xive(sPAPRMachineState *spapr, Error **errp)
>  sPAPRIrq spapr_irq_xive = {
>      .nr_irqs     = SPAPR_IRQ_XIVE_NR_IRQS,
>      .nr_msis     = SPAPR_IRQ_XIVE_NR_MSIS,
> +    .ov5         = SPAPR_OV5_XIVE_EXPLOIT,
>  
>      .init        = spapr_irq_init_xive,
>      .claim       = spapr_irq_claim_xive,
> @@ -466,6 +468,7 @@ int spapr_irq_find(sPAPRMachineState *spapr, int num, bool align, Error **errp)
>  sPAPRIrq spapr_irq_xics_legacy = {
>      .nr_irqs     = SPAPR_IRQ_XICS_LEGACY_NR_IRQS,
>      .nr_msis     = SPAPR_IRQ_XICS_LEGACY_NR_IRQS,
> +    .ov5         = SPAPR_OV5_XIVE_LEGACY,
>  
>      .init        = spapr_irq_init_xics,
>      .claim       = spapr_irq_claim_xics,

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