[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH] spapr/irq: Add XIVE sanity checks on non-P9 machi
From: |
Greg Kurz |
Subject: |
Re: [Qemu-ppc] [PATCH] spapr/irq: Add XIVE sanity checks on non-P9 machines |
Date: |
Thu, 28 Mar 2019 14:09:50 +0100 |
On Thu, 28 Mar 2019 11:00:44 +0100
Cédric Le Goater <address@hidden> wrote:
> On non-P9 machines, the XIVE interrupt mode is not advertised, see
> spapr_dt_ov5_platform_support(). Add a couple of checks on the machine
> configuration to filter bogus setups and prevent OS failures :
>
> Interrupt modes
>
> CPU/Compat XICS XIVE dual
>
> P8/P8 OK QEMU failure (1) OK (3)
> P9/P8 OK QEMU failure (2) OK (3)
> P9/P9 OK OK OK
>
> (1) CPU exception model is incompatible with XIVE and the presenters
> will fail to realize.
>
> (2) CPU exception model is compatible with XIVE, but the XIVE CAS
> advertisement is dropped when in POWER8 mode. So we could ended up
> booting with the XIVE DT properties but without the HCALLs. Avoid
> confusing Linux with such settings and fail under QEMU.
>
> (3) force XICS in machine init
>
> Remove the check on XIVE-only machines in spapr_machine_init(), which
> has now become redundant.
>
> Signed-off-by: Cédric Le Goater <address@hidden>
> ---
LGTM
Reviewed-by: Greg Kurz <address@hidden>
> hw/ppc/spapr.c | 8 +-------
> hw/ppc/spapr_irq.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 51 insertions(+), 7 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index adde36a01da3..33ab7b683ba2 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2806,13 +2806,7 @@ static void spapr_machine_init(MachineState *machine)
>
> /* advertise XIVE on POWER9 machines */
> if (spapr->irq->ov5 & (SPAPR_OV5_XIVE_EXPLOIT | SPAPR_OV5_XIVE_BOTH)) {
> - 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 if (spapr->irq->ov5 & SPAPR_OV5_XIVE_EXPLOIT) {
> - error_report("XIVE-only machines require a POWER9 CPU");
> - exit(1);
> - }
> + spapr_ovec_set(spapr->ov5, OV5_XIVE_EXPLOIT);
> }
>
> /* init CPUs */
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 5809c955501c..b1f79ea9def6 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -16,6 +16,7 @@
> #include "hw/ppc/spapr_xive.h"
> #include "hw/ppc/xics.h"
> #include "hw/ppc/xics_spapr.h"
> +#include "cpu-models.h"
> #include "sysemu/kvm.h"
>
> #include "trace.h"
> @@ -566,12 +567,55 @@ SpaprIrq spapr_irq_dual = {
> .get_nodename = spapr_irq_get_nodename_dual,
> };
>
> +
> +static void spapr_irq_check(SpaprMachineState *spapr, Error **errp)
> +{
> + MachineState *machine = MACHINE(spapr);
> +
> + /*
> + * Sanity checks on non-P9 machines. On these, XIVE is not
> + * advertised, see spapr_dt_ov5_platform_support()
> + */
> + if (!ppc_type_check_compat(machine->cpu_type, CPU_POWERPC_LOGICAL_3_00,
> + 0, spapr->max_compat_pvr)) {
> + /*
> + * If the 'dual' interrupt mode is selected, force XICS as CAS
> + * negotiation is useless.
> + */
> + if (spapr->irq == &spapr_irq_dual) {
> + spapr->irq = &spapr_irq_xics;
> + return;
> + }
> +
> + /*
> + * Non-P9 machines using only XIVE is a bogus setup. We have two
> + * scenarios to take into account because of the compat mode:
> + *
> + * 1. POWER7/8 machines should fail to init later on when creating
> + * the XIVE interrupt presenters because a POWER9 exception
> + * model is required.
> +
> + * 2. POWER9 machines using the POWER8 compat mode won't fail and
> + * will let the OS boot with a partial XIVE setup : DT
> + * properties but no hcalls.
> + *
> + * To cover both and not confuse the OS, add an early failure in
> + * QEMU.
> + */
> + if (spapr->irq == &spapr_irq_xive) {
> + error_setg(errp, "XIVE-only machines require a POWER9 CPU");
> + return;
> + }
> + }
> +}
> +
> /*
> * sPAPR IRQ frontend routines for devices
> */
> void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
> {
> MachineState *machine = MACHINE(spapr);
> + Error *local_err = NULL;
>
> if (machine_kernel_irqchip_split(machine)) {
> error_setg(errp, "kernel_irqchip split mode not supported on
> pseries");
> @@ -584,6 +628,12 @@ void spapr_irq_init(SpaprMachineState *spapr, Error
> **errp)
> return;
> }
>
> + spapr_irq_check(spapr, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> +
> /* Initialize the MSI IRQ allocator. */
> if (!SPAPR_MACHINE_GET_CLASS(spapr)->legacy_irq_allocation) {
> spapr_irq_msi_init(spapr, spapr->irq->nr_msis);