On 25/09/2019 08:45, David Gibson wrote:
> SpaprIrq::ov5 stores the value for a particular byte in PAPR option vector
> 5 which indicates whether XICS, XIVE or both interrupt controllers are
> available. As usual for PAPR, the encoding is kind of overly complicated
> and confusing (though to be fair there are some backwards compat things it
> has to handle).
>
> But to make our internal code clearer, have SpaprIrq encode more directly
> which backends are available as two booleans, and derive the OV5 value from
> that at the point we need it.
OK. It looks nice.
> Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
Reviewed-by: Cédric Le Goater <c...@kaod.org>
> ---
> hw/ppc/spapr.c | 15 ++++++++++++---
> hw/ppc/spapr_hcall.c | 6 +++---
> hw/ppc/spapr_irq.c | 12 ++++++++----
> include/hw/ppc/spapr_irq.h | 3 ++-
> 4 files changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 3742a8cf06..795f6ab505 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1136,19 +1136,28 @@ static void
> spapr_dt_ov5_platform_support(SpaprMachineState *spapr, void *fdt,
> PowerPCCPU *first_ppc_cpu = POWERPC_CPU(first_cpu);
>
> char val[2 * 4] = {
> - 23, spapr->irq->ov5, /* Xive mode. */
> + 23, 0x00, /* XICS / XIVE mode */
> 24, 0x00, /* Hash/Radix, filled in below. */
> 25, 0x00, /* Hash options: Segment Tables == no, GTSE == no. */
> 26, 0x40, /* Radix options: GTSE == yes. */
> };
>
> + if (spapr->irq->xics && spapr->irq->xive) {
> + val[1] = SPAPR_OV5_XIVE_BOTH;
> + } else if (spapr->irq->xive) {
> + val[1] = SPAPR_OV5_XIVE_EXPLOIT;
> + } else {
> + assert(spapr->irq->xics);
> + val[1] = SPAPR_OV5_XIVE_LEGACY;
> + }
> +
> 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 and use the legacy interrupt mode
> */
> - val[1] = 0x00; /* XICS */
> + val[1] = SPAPR_OV5_XIVE_LEGACY; /* XICS */
> val[3] = 0x00; /* Hash */
> } else if (kvm_enabled()) {
> if (kvmppc_has_cap_mmu_radix() && kvmppc_has_cap_mmu_hash_v3()) {
> @@ -2837,7 +2846,7 @@ static void spapr_machine_init(MachineState *machine)
> spapr_ovec_set(spapr->ov5, OV5_DRMEM_V2);
>
> /* advertise XIVE on POWER9 machines */
> - if (spapr->irq->ov5 & (SPAPR_OV5_XIVE_EXPLOIT | SPAPR_OV5_XIVE_BOTH)) {
> + if (spapr->irq->xive) {
> spapr_ovec_set(spapr->ov5, OV5_XIVE_EXPLOIT);
> }
>
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 3d3a67149a..140f05c1c6 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1784,13 +1784,13 @@ static target_ulong
> h_client_architecture_support(PowerPCCPU *cpu,
> * terminate the boot.
> */
> if (guest_xive) {
> - if (spapr->irq->ov5 == SPAPR_OV5_XIVE_LEGACY) {
> + if (!spapr->irq->xive) {
> error_report(
> "Guest requested unavailable interrupt mode (XIVE), try the ic-mode=xive or
> ic-mode=dual machine property");
> exit(EXIT_FAILURE);
> }
> } else {
> - if (spapr->irq->ov5 == SPAPR_OV5_XIVE_EXPLOIT) {
> + if (!spapr->irq->xics) {
> error_report(
> "Guest requested unavailable interrupt mode (XICS), either don't set the
> ic-mode machine property or try ic-mode=xics or ic-mode=dual");
> exit(EXIT_FAILURE);
> @@ -1804,7 +1804,7 @@ static target_ulong
> h_client_architecture_support(PowerPCCPU *cpu,
> */
> if (!spapr->cas_reboot) {
> spapr->cas_reboot = spapr_ovec_test(ov5_updates, OV5_XIVE_EXPLOIT)
> - && spapr->irq->ov5 & SPAPR_OV5_XIVE_BOTH;
> + && spapr->irq->xics && spapr->irq->xive;
> }
>
> spapr_ovec_cleanup(ov5_updates);
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index f53544e45e..073f375ba2 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -209,7 +209,8 @@ static void spapr_irq_init_kvm_xics(SpaprMachineState
> *spapr, Error **errp)
> SpaprIrq spapr_irq_xics = {
> .nr_xirqs = SPAPR_NR_XIRQS,
> .nr_msis = SPAPR_NR_MSIS,
> - .ov5 = SPAPR_OV5_XIVE_LEGACY,
> + .xics = true,
> + .xive = false,
>
> .init = spapr_irq_init_xics,
> .claim = spapr_irq_claim_xics,
> @@ -357,7 +358,8 @@ static void spapr_irq_init_kvm_xive(SpaprMachineState
> *spapr, Error **errp)
> SpaprIrq spapr_irq_xive = {
> .nr_xirqs = SPAPR_NR_XIRQS,
> .nr_msis = SPAPR_NR_MSIS,
> - .ov5 = SPAPR_OV5_XIVE_EXPLOIT,
> + .xics = false,
> + .xive = true,
>
> .init = spapr_irq_init_xive,
> .claim = spapr_irq_claim_xive,
> @@ -515,7 +517,8 @@ static void spapr_irq_set_irq_dual(void *opaque, int irq,
> int val)
> SpaprIrq spapr_irq_dual = {
> .nr_xirqs = SPAPR_NR_XIRQS,
> .nr_msis = SPAPR_NR_MSIS,
> - .ov5 = SPAPR_OV5_XIVE_BOTH,
> + .xics = true,
> + .xive = true,
>
> .init = spapr_irq_init_dual,
> .claim = spapr_irq_claim_dual,
> @@ -751,7 +754,8 @@ int spapr_irq_find(SpaprMachineState *spapr, int num,
> bool align, Error **errp)
> SpaprIrq spapr_irq_xics_legacy = {
> .nr_xirqs = SPAPR_IRQ_XICS_LEGACY_NR_XIRQS,
> .nr_msis = SPAPR_IRQ_XICS_LEGACY_NR_XIRQS,
> - .ov5 = SPAPR_OV5_XIVE_LEGACY,
> + .xics = true,
> + .xive = false,
>
> .init = spapr_irq_init_xics,
> .claim = spapr_irq_claim_xics,
> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> index 75279ca137..6816cb0500 100644
> --- a/include/hw/ppc/spapr_irq.h
> +++ b/include/hw/ppc/spapr_irq.h
> @@ -39,7 +39,8 @@ void spapr_irq_msi_free(SpaprMachineState *spapr, int irq,
> uint32_t num);
> typedef struct SpaprIrq {
> uint32_t nr_xirqs;
> uint32_t nr_msis;
> - uint8_t ov5;
> + bool xics;
> + bool xive;
>
> void (*init)(SpaprMachineState *spapr, Error **errp);
> void (*claim)(SpaprMachineState *spapr, int irq, bool lsi, Error **errp);
>
