[PATCH 2/2] KVM: arm64: Workaround firmware wrongly advertising GICv2-on-v3 compatibility

[Ard Biesheuvel]

On Fri, 8 Jan 2021 at 18:12, Marc Zyngier <maz at kernel.org> wrote:
>
> 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 at huawei.com>
> Signed-off-by: Marc Zyngier <maz at kernel.org>
> ---
>  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);
>  }
>

Is it necessary to perform this check unconditionally? We only care
about this if the firmware claims v2 compat support.

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