| Message ID | 20220512151457.3899052-2-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | gicv3: Use right number of prio bits for the CPU | expand |
On 5/12/22 08:14, Peter Maydell wrote: > We allow a GICv3 to be connected to any CPU, but we don't do anything > to handle the case where the CPU type doesn't in hardware have a > GICv3 CPU interface and so the various GIC configuration fields > (gic_num_lrs, vprebits, vpribits) are not specified. > > The current behaviour is that we will add the EL1 CPU interface > registers, but will not put in the EL2 CPU interface registers, even > if the CPU has EL2, which will leave the GIC in a broken state and > probably result in the guest crashing as it tries to set it up. This > only affects the virt board when using the cortex-a15 or cortex-a7 > CPU types (both 32-bit) with -machine gic-version=3 (or 'max') > and -machine virtualization=on. > > Instead of failing to set up the EL2 registers, if the CPU doesn't > define the GIC configuration set it to a reasonable default, matching > the standard configuration for most Arm CPUs. > > Signed-off-by: Peter Maydell<peter.maydell@linaro.org> > --- > The other approach would be to make the GIC fail realize if the > CPU type doesn't officially have a GICv3 interface, and make the > virt board check for mismatches and handle 'gic-version' accordingly, > but this seems like less effort. I don't think anybody's likely > using this corner case anyway: the only reason I ran into it is > that with my patches to add cpu->gic_prebits one of the tests > in 'make check' now runs into it because it unintentionally and > unnecessarily asks for gicv3 and cortex-a15. > --- > hw/intc/arm_gicv3_cpuif.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c index 9efba798f82..df2f8583564 100644 --- a/hw/intc/arm_gicv3_cpuif.c +++ b/hw/intc/arm_gicv3_cpuif.c @@ -2755,6 +2755,15 @@ void gicv3_init_cpuif(GICv3State *s) ARMCPU *cpu = ARM_CPU(qemu_get_cpu(i)); GICv3CPUState *cs = &s->cpu[i]; + /* + * If the CPU doesn't define a GICv3 configuration, probably because + * in real hardware it doesn't have one, then we use default values + * matching the one used by most Arm CPUs. This applies to: + * cpu->gic_num_lrs + * cpu->gic_vpribits + * cpu->gic_vprebits + */ + /* Note that we can't just use the GICv3CPUState as an opaque pointer * in define_arm_cp_regs_with_opaque(), because when we're called back * it might be with code translated by CPU 0 but run by CPU 1, in @@ -2763,13 +2772,12 @@ void gicv3_init_cpuif(GICv3State *s) * get back to the GICv3CPUState from the CPUARMState. */ define_arm_cp_regs(cpu, gicv3_cpuif_reginfo); - if (arm_feature(&cpu->env, ARM_FEATURE_EL2) - && cpu->gic_num_lrs) { + if (arm_feature(&cpu->env, ARM_FEATURE_EL2)) { int j; - cs->num_list_regs = cpu->gic_num_lrs; - cs->vpribits = cpu->gic_vpribits; - cs->vprebits = cpu->gic_vprebits; + cs->num_list_regs = cpu->gic_num_lrs ?: 4; + cs->vpribits = cpu->gic_vpribits ?: 5; + cs->vprebits = cpu->gic_vprebits ?: 5; /* Check against architectural constraints: getting these * wrong would be a bug in the CPU code defining these,
We allow a GICv3 to be connected to any CPU, but we don't do anything to handle the case where the CPU type doesn't in hardware have a GICv3 CPU interface and so the various GIC configuration fields (gic_num_lrs, vprebits, vpribits) are not specified. The current behaviour is that we will add the EL1 CPU interface registers, but will not put in the EL2 CPU interface registers, even if the CPU has EL2, which will leave the GIC in a broken state and probably result in the guest crashing as it tries to set it up. This only affects the virt board when using the cortex-a15 or cortex-a7 CPU types (both 32-bit) with -machine gic-version=3 (or 'max') and -machine virtualization=on. Instead of failing to set up the EL2 registers, if the CPU doesn't define the GIC configuration set it to a reasonable default, matching the standard configuration for most Arm CPUs. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- The other approach would be to make the GIC fail realize if the CPU type doesn't officially have a GICv3 interface, and make the virt board check for mismatches and handle 'gic-version' accordingly, but this seems like less effort. I don't think anybody's likely using this corner case anyway: the only reason I ran into it is that with my patches to add cpu->gic_prebits one of the tests in 'make check' now runs into it because it unintentionally and unnecessarily asks for gicv3 and cortex-a15. --- hw/intc/arm_gicv3_cpuif.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)