[PATCH v2 0/2] KVM: arm64: Work around firmware wongly advertising GICv2 compatibility

[PATCH v2 0/2] KVM: arm64: Work around firmware wongly advertising GICv2 compatibility

[Marc Zyngier]

It appears that there is firmware out there that advertises GICv2
compatibility on GICv3, despite the CPUs not being able to actually do
it. That's a bummer, and at best creates unexpected behaviours for the
users. At worse, it will crash the machine. Awesome!

In order to mitigate this issue, try and validate whether we can
actually flip the CPU into supporting MMIO accesses instead of system
registers. If we can't, ignore the compatibility information and
shout. It's not completely foolproof, but it should cover the existing
broken platforms...

The workaround is much more invasive than Shameer's initial proposal,
as I wanted to keep it localised to KVM instead of spreading the
horror at every level (after all, only KVM is concerned with v2
compatibility).

Tested on a deliberately misconfigured FVP (DT advertising the MMIO
regions with cluster*.gicv3.cpuintf-mmap-access-level=2).

* From v1:
  - Fixed silly thinko when computing the configuration state

Marc Zyngier (2):
  KVM: arm64: Rename __vgic_v3_get_ich_vtr_el2() to
    __vgic_v3_get_gic_config()
  KVM: arm64: Workaround firmware wrongly advertising GICv2-on-v3
    compatibility

 arch/arm64/include/asm/kvm_asm.h   |  4 +--
 arch/arm64/kvm/hyp/nvhe/hyp-main.c |  6 ++---
 arch/arm64/kvm/hyp/vgic-v3-sr.c    | 40 ++++++++++++++++++++++++++++--
 arch/arm64/kvm/vgic/vgic-v3.c      | 12 ++++++---
 4 files changed, 52 insertions(+), 10 deletions(-)

-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 1/2] KVM: arm64: Rename __vgic_v3_get_ich_vtr_el2() to __vgic_v3_get_gic_config()

[Marc Zyngier]

As we are about to report a bit more information to the rest of
the kernel, rename __vgic_v3_get_ich_vtr_el2() to the more
explicit __vgic_v3_get_gic_config().

No functional change.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/include/asm/kvm_asm.h   | 4 ++--
 arch/arm64/kvm/hyp/nvhe/hyp-main.c | 6 +++---
 arch/arm64/kvm/hyp/vgic-v3-sr.c    | 7 ++++++-
 arch/arm64/kvm/vgic/vgic-v3.c      | 4 +++-
 4 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
index 8a33d83ea843..37b9cd3e458e 100644
--- a/arch/arm64/include/asm/kvm_asm.h
+++ b/arch/arm64/include/asm/kvm_asm.h
@@ -50,7 +50,7 @@
 #define __KVM_HOST_SMCCC_FUNC___kvm_tlb_flush_local_vmid	5
 #define __KVM_HOST_SMCCC_FUNC___kvm_timer_set_cntvoff		6
 #define __KVM_HOST_SMCCC_FUNC___kvm_enable_ssbs			7
-#define __KVM_HOST_SMCCC_FUNC___vgic_v3_get_ich_vtr_el2		8
+#define __KVM_HOST_SMCCC_FUNC___vgic_v3_get_gic_config		8
 #define __KVM_HOST_SMCCC_FUNC___vgic_v3_read_vmcr		9
 #define __KVM_HOST_SMCCC_FUNC___vgic_v3_write_vmcr		10
 #define __KVM_HOST_SMCCC_FUNC___vgic_v3_init_lrs		11
@@ -192,7 +192,7 @@ extern void __kvm_timer_set_cntvoff(u64 cntvoff);
 
 extern int __kvm_vcpu_run(struct kvm_vcpu *vcpu);
 
-extern u64 __vgic_v3_get_ich_vtr_el2(void);
+extern u64 __vgic_v3_get_gic_config(void);
 extern u64 __vgic_v3_read_vmcr(void);
 extern void __vgic_v3_write_vmcr(u32 vmcr);
 extern void __vgic_v3_init_lrs(void);
diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
index bde658d51404..3dc7f0c4fa94 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
@@ -67,9 +67,9 @@ static void handle___kvm_enable_ssbs(struct kvm_cpu_context *host_ctxt)
 	write_sysreg_el2(tmp, SYS_SCTLR);
 }
 
-static void handle___vgic_v3_get_ich_vtr_el2(struct kvm_cpu_context *host_ctxt)
+static void handle___vgic_v3_get_gic_config(struct kvm_cpu_context *host_ctxt)
 {
-	cpu_reg(host_ctxt, 1) = __vgic_v3_get_ich_vtr_el2();
+	cpu_reg(host_ctxt, 1) = __vgic_v3_get_gic_config();
 }
 
 static void handle___vgic_v3_read_vmcr(struct kvm_cpu_context *host_ctxt)
@@ -118,7 +118,7 @@ static const hcall_t *host_hcall[] = {
 	HANDLE_FUNC(__kvm_tlb_flush_local_vmid),
 	HANDLE_FUNC(__kvm_timer_set_cntvoff),
 	HANDLE_FUNC(__kvm_enable_ssbs),
-	HANDLE_FUNC(__vgic_v3_get_ich_vtr_el2),
+	HANDLE_FUNC(__vgic_v3_get_gic_config),
 	HANDLE_FUNC(__vgic_v3_read_vmcr),
 	HANDLE_FUNC(__vgic_v3_write_vmcr),
 	HANDLE_FUNC(__vgic_v3_init_lrs),
diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c b/arch/arm64/kvm/hyp/vgic-v3-sr.c
index 80406f463c28..005daa0c9dd7 100644
--- a/arch/arm64/kvm/hyp/vgic-v3-sr.c
+++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c
@@ -405,7 +405,12 @@ void __vgic_v3_init_lrs(void)
 		__gic_v3_set_lr(0, i);
 }
 
-u64 __vgic_v3_get_ich_vtr_el2(void)
+/*
+ * Return the GIC CPU configuration:
+ * - [31:0]  ICH_VTR_EL2
+ * - [63:32] RES0
+ */
+u64 __vgic_v3_get_gic_config(void)
 {
 	return read_gicreg(ICH_VTR_EL2);
 }
diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index 9cdf39a94a63..8e7bf3151057 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -583,9 +583,11 @@ early_param("kvm-arm.vgic_v4_enable", early_gicv4_enable);
  */
 int vgic_v3_probe(const struct gic_kvm_info *info)
 {
-	u32 ich_vtr_el2 = kvm_call_hyp_ret(__vgic_v3_get_ich_vtr_el2);
+	u64 ich_vtr_el2 = kvm_call_hyp_ret(__vgic_v3_get_gic_config);
 	int ret;
 
+	ich_vtr_el2 = (u32)ich_vtr_el2;
+
 	/*
 	 * The ListRegs field is 5 bits, but there is an architectural
 	 * maximum of 16 list registers. Just ignore bit 4...
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Marc Zyngier]

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@huawei.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/hyp/vgic-v3-sr.c | 35 +++++++++++++++++++++++++++++++--
 arch/arm64/kvm/vgic/vgic-v3.c   |  8 ++++++--
 2 files changed, 39 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..ee3682b9873c 100644
--- a/arch/arm64/kvm/hyp/vgic-v3-sr.c
+++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c
@@ -408,11 +408,42 @@ 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 val, sre = read_gicreg(ICC_SRE_EL1);
+	unsigned long flags = 0;
+
+	/*
+	 * 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();
+
+	val = read_gicreg(ICC_SRE_EL1);
+
+	write_gicreg(sre, ICC_SRE_EL1);
+	isb();
+
+	if (has_vhe())
+		local_daif_restore(flags);
+
+	val  = (val & ICC_SRE_EL1_SRE) ? 0 : (1ULL << 63);
+	val |= read_gicreg(ICH_VTR_EL2);
+
+	return val;
 }
 
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Ard Biesheuvel]

On Fri, 15 Jan 2021 at 15:03, Marc Zyngier <maz@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@huawei.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/hyp/vgic-v3-sr.c | 35 +++++++++++++++++++++++++++++++--
>  arch/arm64/kvm/vgic/vgic-v3.c   |  8 ++++++--
>  2 files changed, 39 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..ee3682b9873c 100644
> --- a/arch/arm64/kvm/hyp/vgic-v3-sr.c
> +++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c
> @@ -408,11 +408,42 @@ 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 val, sre = read_gicreg(ICC_SRE_EL1);
> +       unsigned long flags = 0;
> +
> +       /*
> +        * 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();
> +
> +       val = read_gicreg(ICC_SRE_EL1);
> +
> +       write_gicreg(sre, ICC_SRE_EL1);
> +       isb();
> +
> +       if (has_vhe())
> +               local_daif_restore(flags);
> +
> +       val  = (val & ICC_SRE_EL1_SRE) ? 0 : (1ULL << 63);
> +       val |= read_gicreg(ICH_VTR_EL2);
> +
> +       return val;
>  }
>
>  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");

Could we include FW_BUG here to stress that this is a firmware problem?

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Marc Zyngier]

On 2021-01-15 14:08, Ard Biesheuvel wrote:
> On Fri, 15 Jan 2021 at 15:03, Marc Zyngier <maz@kernel.org> wrote:

[...]

>> 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");
> 
> Could we include FW_BUG here to stress that this is a firmware problem?

Absolutely! That's what it now looks like:

[    2.648452] kvm [1]: IPA Size Limit: 40 bits
[    2.649259] [Firmware Bug]: CPU interface incapable of MMIO access
[    2.649620] kvm [1]: disabling GICv2 emulation
[    2.650227] kvm [1]: GIC system register CPU interface enabled
[    2.652004] kvm [1]: vgic interrupt IRQ9
[    2.655623] kvm [1]: VHE mode initialized successfully

Updated version pushed out.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: Marc Zyngier [mailto:maz@kernel.org]
> Sent: 15 January 2021 14:15
> To: Ard Biesheuvel <ardb@kernel.org>
> Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>; kvmarm
> <kvmarm@lists.cs.columbia.edu>; Shameerali Kolothum Thodi
> <shameerali.kolothum.thodi@huawei.com>; James Morse
> <james.morse@arm.com>; Julien Thierry <julien.thierry.kdev@gmail.com>;
> Suzuki K Poulose <suzuki.poulose@arm.com>; Android Kernel Team
> <kernel-team@android.com>
> Subject: Re: [PATCH v2 2/2] KVM: arm64: Workaround firmware wrongly
> advertising GICv2-on-v3 compatibility
> 
> On 2021-01-15 14:08, Ard Biesheuvel wrote:
> > On Fri, 15 Jan 2021 at 15:03, Marc Zyngier <maz@kernel.org> wrote:
> 
> [...]
> 
> >> 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");
> >
> > Could we include FW_BUG here to stress that this is a firmware problem?
> 
> Absolutely! That's what it now looks like:
> 
> [    2.648452] kvm [1]: IPA Size Limit: 40 bits
> [    2.649259] [Firmware Bug]: CPU interface incapable of MMIO access
> [    2.649620] kvm [1]: disabling GICv2 emulation
> [    2.650227] kvm [1]: GIC system register CPU interface enabled
> [    2.652004] kvm [1]: vgic interrupt IRQ9
> [    2.655623] kvm [1]: VHE mode initialized successfully
> 
> Updated version pushed out.

Is there a v3 for this series? I couldn't find one.

Anyways, tested this series on a D06 with faulty firmware and it is working as expected.
FWIW,

   Tested-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>

Thanks,
Shameer

> Thanks,
> 
>          M.
> --
> Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Marc Zyngier]

On 2021-01-19 10:09, Shameerali Kolothum Thodi wrote:
>> -----Original Message-----
>> From: Marc Zyngier [mailto:maz@kernel.org]
>> Sent: 15 January 2021 14:15
>> To: Ard Biesheuvel <ardb@kernel.org>
>> Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>; kvmarm
>> <kvmarm@lists.cs.columbia.edu>; Shameerali Kolothum Thodi
>> <shameerali.kolothum.thodi@huawei.com>; James Morse
>> <james.morse@arm.com>; Julien Thierry <julien.thierry.kdev@gmail.com>;
>> Suzuki K Poulose <suzuki.poulose@arm.com>; Android Kernel Team
>> <kernel-team@android.com>
>> Subject: Re: [PATCH v2 2/2] KVM: arm64: Workaround firmware wrongly
>> advertising GICv2-on-v3 compatibility
>> 
>> On 2021-01-15 14:08, Ard Biesheuvel wrote:
>> > On Fri, 15 Jan 2021 at 15:03, Marc Zyngier <maz@kernel.org> wrote:
>> 
>> [...]
>> 
>> >> 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");
>> >
>> > Could we include FW_BUG here to stress that this is a firmware problem?
>> 
>> Absolutely! That's what it now looks like:
>> 
>> [    2.648452] kvm [1]: IPA Size Limit: 40 bits
>> [    2.649259] [Firmware Bug]: CPU interface incapable of MMIO access
>> [    2.649620] kvm [1]: disabling GICv2 emulation
>> [    2.650227] kvm [1]: GIC system register CPU interface enabled
>> [    2.652004] kvm [1]: vgic interrupt IRQ9
>> [    2.655623] kvm [1]: VHE mode initialized successfully
>> 
>> Updated version pushed out.
> 
> Is there a v3 for this series? I couldn't find one.

Nope, I didn't think it was useful to send another series for such
a minor change.

> 
> Anyways, tested this series on a D06 with faulty firmware and it is
> working as expected.
> FWIW,
> 
>    Tested-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> 

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel