Hi Marc, > -----Original Message----- > From: Marc Zyngier [mailto:maz@xxxxxxxxxx] > Sent: 08 January 2021 17:12 > To: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; kvmarm@xxxxxxxxxxxxxxxxxxxxx > Cc: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@xxxxxxxxxx>; > James Morse <james.morse@xxxxxxx>; Julien Thierry > <julien.thierry.kdev@xxxxxxxxx>; Suzuki K Poulose > <suzuki.poulose@xxxxxxx>; Ard Biesheuvel <ardb@xxxxxxxxxx>; > kernel-team@xxxxxxxxxxx > Subject: [PATCH 2/2] KVM: arm64: Workaround firmware wrongly advertising > GICv2-on-v3 compatibility > > It looks like we have broken firmware out there that wrongly advertises > a GICv2 compatibility interface, despite the CPUs not being able to deal > with it. > > To work around this, check that the CPU initialising KVM is actually able > to switch to MMIO instead of system registers, and use that as a > precondition to enable GICv2 compatibility in KVM. > > Note that the detection happens on a single CPU. If the firmware is > lying *and* that the CPUs are asymetric, all hope is lost anyway. > > Reported-by: Shameerali Kolothum Thodi > <shameerali.kolothum.thodi@xxxxxxxxxx> > Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx> > --- > arch/arm64/kvm/hyp/vgic-v3-sr.c | 34 +++++++++++++++++++++++++++++++-- > arch/arm64/kvm/vgic/vgic-v3.c | 8 ++++++-- > 2 files changed, 38 insertions(+), 4 deletions(-) > > diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c > b/arch/arm64/kvm/hyp/vgic-v3-sr.c > index 005daa0c9dd7..d504499ab917 100644 > --- a/arch/arm64/kvm/hyp/vgic-v3-sr.c > +++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c > @@ -408,11 +408,41 @@ void __vgic_v3_init_lrs(void) > /* > * Return the GIC CPU configuration: > * - [31:0] ICH_VTR_EL2 > - * - [63:32] RES0 > + * - [62:32] RES0 > + * - [63] MMIO (GICv2) capable > */ > u64 __vgic_v3_get_gic_config(void) > { > - return read_gicreg(ICH_VTR_EL2); > + u64 sre = read_gicreg(ICC_SRE_EL1); > + unsigned long flags = 0; > + bool v2_capable; > + > + /* > + * To check whether we have a MMIO-based (GICv2 compatible) > + * CPU interface, we need to disable the system register > + * view. To do that safely, we have to prevent any interrupt > + * from firing (which would be deadly). > + * > + * Note that this only makes sense on VHE, as interrupts are > + * already masked for nVHE as part of the exception entry to > + * EL2. > + */ > + if (has_vhe()) > + flags = local_daif_save(); > + > + write_gicreg(0, ICC_SRE_EL1); > + isb(); > + > + v2_capable = !(read_gicreg(ICC_SRE_EL1) & ICC_SRE_EL1_SRE); > + > + write_gicreg(sre, ICC_SRE_EL1); > + isb(); > + > + if (has_vhe()) > + local_daif_restore(flags); > + > + return (read_gicreg(ICH_VTR_EL2) | > + v2_capable ? (1ULL << 63) : 0); > } Thanks for sending this out. I had a go with this series and unfortunately it didn't work on a system with faulty BIOS. It looks like the culprit here is the ?: operator. There seems to be an operator precedence at play here and it returns, vgic_v3_probe: ich_vtr_el2 0x8000000000000000 And with the below change, return (read_gicreg(ICH_VTR_EL2) | - v2_capable ? (1ULL << 63) : 0); + (v2_capable ? (1ULL << 63) : 0)); } It returns, vgic_v3_probe: ich_vtr_el2 0x90080003 and works correctly. [ 18.918738] kvm [1]: disabling GICv2 emulation Thanks, Shameer > u64 __vgic_v3_read_vmcr(void) > diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c > index 8e7bf3151057..67b27b47312b 100644 > --- a/arch/arm64/kvm/vgic/vgic-v3.c > +++ b/arch/arm64/kvm/vgic/vgic-v3.c > @@ -584,8 +584,10 @@ early_param("kvm-arm.vgic_v4_enable", > early_gicv4_enable); > int vgic_v3_probe(const struct gic_kvm_info *info) > { > u64 ich_vtr_el2 = kvm_call_hyp_ret(__vgic_v3_get_gic_config); > + bool has_v2; > int ret; > > + has_v2 = ich_vtr_el2 >> 63; > ich_vtr_el2 = (u32)ich_vtr_el2; > > /* > @@ -605,13 +607,15 @@ int vgic_v3_probe(const struct gic_kvm_info *info) > gicv4_enable ? "en" : "dis"); > } > > + kvm_vgic_global_state.vcpu_base = 0; > + > if (!info->vcpu.start) { > kvm_info("GICv3: no GICV resource entry\n"); > - kvm_vgic_global_state.vcpu_base = 0; > + } else if (!has_v2) { > + pr_warn("CPU interface incapable of MMIO access\n"); > } else if (!PAGE_ALIGNED(info->vcpu.start)) { > pr_warn("GICV physical address 0x%llx not page aligned\n", > (unsigned long long)info->vcpu.start); > - kvm_vgic_global_state.vcpu_base = 0; > } else { > kvm_vgic_global_state.vcpu_base = info->vcpu.start; > kvm_vgic_global_state.can_emulate_gicv2 = true; > -- > 2.29.2 _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
