All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] KVM: arm64: Fix CPU interface MMIO compatibility detection
@ 2021-03-23 16:23 ` Marc Zyngier
  0 siblings, 0 replies; 6+ messages in thread
From: Marc Zyngier @ 2021-03-23 16:23 UTC (permalink / raw)
  To: kvmarm, kvm, linux-arm-kernel
  Cc: Ard Biesheuvel, kernel-team, Shameerali Kolothum Thodi

In order to detect whether a GICv3 CPU interface is MMIO capable,
we switch ICC_SRE_EL1.SRE to 0 and check whether it sticks.

However, this is only possible if *ALL* of the HCR_EL2 interrupt
overrides are set, and the CPU is perfectly allowed to ignore
the write to ICC_SRE_EL1 otherwise. This leads KVM to pretend
that a whole bunch of ARMv8.0 CPUs aren't MMIO-capable, and
breaks VMs that should work correctly otherwise.

Fix this by setting IMO/FMO/IMO before touching ICC_SRE_EL1,
and clear them afterwards. This allows us to reliably detect
the CPU interface capabilities.

Cc: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
Fixes: 9739f6ef053f ("KVM: arm64: Workaround firmware wrongly advertising GICv2-on-v3 compatibility")
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/hyp/vgic-v3-sr.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c b/arch/arm64/kvm/hyp/vgic-v3-sr.c
index ee3682b9873c..39f8f7f9227c 100644
--- a/arch/arm64/kvm/hyp/vgic-v3-sr.c
+++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c
@@ -429,6 +429,13 @@ u64 __vgic_v3_get_gic_config(void)
 	if (has_vhe())
 		flags = local_daif_save();
 
+	/*
+	 * Table 11-2 "Permitted ICC_SRE_ELx.SRE settings" indicates
+	 * that to be able to set ICC_SRE_EL1.SRE to 0, all the
+	 * interrupt overrides must be set. You've got to love this.
+	 */
+	sysreg_clear_set(hcr_el2, 0, HCR_AMO | HCR_FMO | HCR_IMO);
+	isb();
 	write_gicreg(0, ICC_SRE_EL1);
 	isb();
 
@@ -436,6 +443,8 @@ u64 __vgic_v3_get_gic_config(void)
 
 	write_gicreg(sre, ICC_SRE_EL1);
 	isb();
+	sysreg_clear_set(hcr_el2, HCR_AMO | HCR_FMO | HCR_IMO, 0);
+	isb();
 
 	if (has_vhe())
 		local_daif_restore(flags);
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH] KVM: arm64: Fix CPU interface MMIO compatibility detection
@ 2021-03-23 16:23 ` Marc Zyngier
  0 siblings, 0 replies; 6+ messages in thread
From: Marc Zyngier @ 2021-03-23 16:23 UTC (permalink / raw)
  To: kvmarm, kvm, linux-arm-kernel; +Cc: kernel-team

In order to detect whether a GICv3 CPU interface is MMIO capable,
we switch ICC_SRE_EL1.SRE to 0 and check whether it sticks.

However, this is only possible if *ALL* of the HCR_EL2 interrupt
overrides are set, and the CPU is perfectly allowed to ignore
the write to ICC_SRE_EL1 otherwise. This leads KVM to pretend
that a whole bunch of ARMv8.0 CPUs aren't MMIO-capable, and
breaks VMs that should work correctly otherwise.

Fix this by setting IMO/FMO/IMO before touching ICC_SRE_EL1,
and clear them afterwards. This allows us to reliably detect
the CPU interface capabilities.

Cc: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
Fixes: 9739f6ef053f ("KVM: arm64: Workaround firmware wrongly advertising GICv2-on-v3 compatibility")
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/hyp/vgic-v3-sr.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c b/arch/arm64/kvm/hyp/vgic-v3-sr.c
index ee3682b9873c..39f8f7f9227c 100644
--- a/arch/arm64/kvm/hyp/vgic-v3-sr.c
+++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c
@@ -429,6 +429,13 @@ u64 __vgic_v3_get_gic_config(void)
 	if (has_vhe())
 		flags = local_daif_save();
 
+	/*
+	 * Table 11-2 "Permitted ICC_SRE_ELx.SRE settings" indicates
+	 * that to be able to set ICC_SRE_EL1.SRE to 0, all the
+	 * interrupt overrides must be set. You've got to love this.
+	 */
+	sysreg_clear_set(hcr_el2, 0, HCR_AMO | HCR_FMO | HCR_IMO);
+	isb();
 	write_gicreg(0, ICC_SRE_EL1);
 	isb();
 
@@ -436,6 +443,8 @@ u64 __vgic_v3_get_gic_config(void)
 
 	write_gicreg(sre, ICC_SRE_EL1);
 	isb();
+	sysreg_clear_set(hcr_el2, HCR_AMO | HCR_FMO | HCR_IMO, 0);
+	isb();
 
 	if (has_vhe())
 		local_daif_restore(flags);
-- 
2.30.0

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH] KVM: arm64: Fix CPU interface MMIO compatibility detection
@ 2021-03-23 16:23 ` Marc Zyngier
  0 siblings, 0 replies; 6+ messages in thread
From: Marc Zyngier @ 2021-03-23 16:23 UTC (permalink / raw)
  To: kvmarm, kvm, linux-arm-kernel
  Cc: Ard Biesheuvel, kernel-team, Shameerali Kolothum Thodi

In order to detect whether a GICv3 CPU interface is MMIO capable,
we switch ICC_SRE_EL1.SRE to 0 and check whether it sticks.

However, this is only possible if *ALL* of the HCR_EL2 interrupt
overrides are set, and the CPU is perfectly allowed to ignore
the write to ICC_SRE_EL1 otherwise. This leads KVM to pretend
that a whole bunch of ARMv8.0 CPUs aren't MMIO-capable, and
breaks VMs that should work correctly otherwise.

Fix this by setting IMO/FMO/IMO before touching ICC_SRE_EL1,
and clear them afterwards. This allows us to reliably detect
the CPU interface capabilities.

Cc: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
Fixes: 9739f6ef053f ("KVM: arm64: Workaround firmware wrongly advertising GICv2-on-v3 compatibility")
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/hyp/vgic-v3-sr.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c b/arch/arm64/kvm/hyp/vgic-v3-sr.c
index ee3682b9873c..39f8f7f9227c 100644
--- a/arch/arm64/kvm/hyp/vgic-v3-sr.c
+++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c
@@ -429,6 +429,13 @@ u64 __vgic_v3_get_gic_config(void)
 	if (has_vhe())
 		flags = local_daif_save();
 
+	/*
+	 * Table 11-2 "Permitted ICC_SRE_ELx.SRE settings" indicates
+	 * that to be able to set ICC_SRE_EL1.SRE to 0, all the
+	 * interrupt overrides must be set. You've got to love this.
+	 */
+	sysreg_clear_set(hcr_el2, 0, HCR_AMO | HCR_FMO | HCR_IMO);
+	isb();
 	write_gicreg(0, ICC_SRE_EL1);
 	isb();
 
@@ -436,6 +443,8 @@ u64 __vgic_v3_get_gic_config(void)
 
 	write_gicreg(sre, ICC_SRE_EL1);
 	isb();
+	sysreg_clear_set(hcr_el2, HCR_AMO | HCR_FMO | HCR_IMO, 0);
+	isb();
 
 	if (has_vhe())
 		local_daif_restore(flags);
-- 
2.30.0


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

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* RE: [PATCH] KVM: arm64: Fix CPU interface MMIO compatibility detection
  2021-03-23 16:23 ` Marc Zyngier
  (?)
@ 2021-03-24 15:47   ` Shameerali Kolothum Thodi
  -1 siblings, 0 replies; 6+ messages in thread
From: Shameerali Kolothum Thodi @ 2021-03-24 15:47 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm, kvm, linux-arm-kernel; +Cc: Ard Biesheuvel, kernel-team



> -----Original Message-----
> From: Marc Zyngier [mailto:maz@kernel.org]
> Sent: 23 March 2021 16:23
> To: kvmarm@lists.cs.columbia.edu; kvm@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org
> Cc: Ard Biesheuvel <ardb@kernel.org>; kernel-team@android.com; Shameerali
> Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Subject: [PATCH] KVM: arm64: Fix CPU interface MMIO compatibility detection
> 
> In order to detect whether a GICv3 CPU interface is MMIO capable,
> we switch ICC_SRE_EL1.SRE to 0 and check whether it sticks.
> 
> However, this is only possible if *ALL* of the HCR_EL2 interrupt
> overrides are set, and the CPU is perfectly allowed to ignore
> the write to ICC_SRE_EL1 otherwise. This leads KVM to pretend
> that a whole bunch of ARMv8.0 CPUs aren't MMIO-capable, and
> breaks VMs that should work correctly otherwise.
> 
> Fix this by setting IMO/FMO/IMO before touching ICC_SRE_EL1,
> and clear them afterwards. This allows us to reliably detect
> the CPU interface capabilities.
> 

Tested on HiSilicon D06 platform where the original issue(firmware wrongly
advertising GICv2-on-v3 compatibility) was reported and all seems to be fine.

Though not sure whether this fix is relevant to this particular platform,
FWIW,

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

Thanks,
Shameer

> Cc: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Fixes: 9739f6ef053f ("KVM: arm64: Workaround firmware wrongly advertising
> GICv2-on-v3 compatibility")
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/hyp/vgic-v3-sr.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c
> b/arch/arm64/kvm/hyp/vgic-v3-sr.c
> index ee3682b9873c..39f8f7f9227c 100644
> --- a/arch/arm64/kvm/hyp/vgic-v3-sr.c
> +++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c
> @@ -429,6 +429,13 @@ u64 __vgic_v3_get_gic_config(void)
>  	if (has_vhe())
>  		flags = local_daif_save();
> 
> +	/*
> +	 * Table 11-2 "Permitted ICC_SRE_ELx.SRE settings" indicates
> +	 * that to be able to set ICC_SRE_EL1.SRE to 0, all the
> +	 * interrupt overrides must be set. You've got to love this.
> +	 */
> +	sysreg_clear_set(hcr_el2, 0, HCR_AMO | HCR_FMO | HCR_IMO);
> +	isb();
>  	write_gicreg(0, ICC_SRE_EL1);
>  	isb();
> 
> @@ -436,6 +443,8 @@ u64 __vgic_v3_get_gic_config(void)
> 
>  	write_gicreg(sre, ICC_SRE_EL1);
>  	isb();
> +	sysreg_clear_set(hcr_el2, HCR_AMO | HCR_FMO | HCR_IMO, 0);
> +	isb();
> 
>  	if (has_vhe())
>  		local_daif_restore(flags);
> --
> 2.30.0


^ permalink raw reply	[flat|nested] 6+ messages in thread
* RE: [PATCH] KVM: arm64: Fix CPU interface MMIO compatibility detection
@ 2021-03-24 15:47   ` Shameerali Kolothum Thodi
  0 siblings, 0 replies; 6+ messages in thread
From: Shameerali Kolothum Thodi @ 2021-03-24 15:47 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm, kvm, linux-arm-kernel; +Cc: kernel-team



> -----Original Message-----
> From: Marc Zyngier [mailto:maz@kernel.org]
> Sent: 23 March 2021 16:23
> To: kvmarm@lists.cs.columbia.edu; kvm@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org
> Cc: Ard Biesheuvel <ardb@kernel.org>; kernel-team@android.com; Shameerali
> Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Subject: [PATCH] KVM: arm64: Fix CPU interface MMIO compatibility detection
> 
> In order to detect whether a GICv3 CPU interface is MMIO capable,
> we switch ICC_SRE_EL1.SRE to 0 and check whether it sticks.
> 
> However, this is only possible if *ALL* of the HCR_EL2 interrupt
> overrides are set, and the CPU is perfectly allowed to ignore
> the write to ICC_SRE_EL1 otherwise. This leads KVM to pretend
> that a whole bunch of ARMv8.0 CPUs aren't MMIO-capable, and
> breaks VMs that should work correctly otherwise.
> 
> Fix this by setting IMO/FMO/IMO before touching ICC_SRE_EL1,
> and clear them afterwards. This allows us to reliably detect
> the CPU interface capabilities.
> 

Tested on HiSilicon D06 platform where the original issue(firmware wrongly
advertising GICv2-on-v3 compatibility) was reported and all seems to be fine.

Though not sure whether this fix is relevant to this particular platform,
FWIW,

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

Thanks,
Shameer

> Cc: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Fixes: 9739f6ef053f ("KVM: arm64: Workaround firmware wrongly advertising
> GICv2-on-v3 compatibility")
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/hyp/vgic-v3-sr.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c
> b/arch/arm64/kvm/hyp/vgic-v3-sr.c
> index ee3682b9873c..39f8f7f9227c 100644
> --- a/arch/arm64/kvm/hyp/vgic-v3-sr.c
> +++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c
> @@ -429,6 +429,13 @@ u64 __vgic_v3_get_gic_config(void)
>  	if (has_vhe())
>  		flags = local_daif_save();
> 
> +	/*
> +	 * Table 11-2 "Permitted ICC_SRE_ELx.SRE settings" indicates
> +	 * that to be able to set ICC_SRE_EL1.SRE to 0, all the
> +	 * interrupt overrides must be set. You've got to love this.
> +	 */
> +	sysreg_clear_set(hcr_el2, 0, HCR_AMO | HCR_FMO | HCR_IMO);
> +	isb();
>  	write_gicreg(0, ICC_SRE_EL1);
>  	isb();
> 
> @@ -436,6 +443,8 @@ u64 __vgic_v3_get_gic_config(void)
> 
>  	write_gicreg(sre, ICC_SRE_EL1);
>  	isb();
> +	sysreg_clear_set(hcr_el2, HCR_AMO | HCR_FMO | HCR_IMO, 0);
> +	isb();
> 
>  	if (has_vhe())
>  		local_daif_restore(flags);
> --
> 2.30.0

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 6+ messages in thread
* RE: [PATCH] KVM: arm64: Fix CPU interface MMIO compatibility detection
@ 2021-03-24 15:47   ` Shameerali Kolothum Thodi
  0 siblings, 0 replies; 6+ messages in thread
From: Shameerali Kolothum Thodi @ 2021-03-24 15:47 UTC (permalink / raw)
  To: Marc Zyngier, kvmarm, kvm, linux-arm-kernel; +Cc: Ard Biesheuvel, kernel-team



> -----Original Message-----
> From: Marc Zyngier [mailto:maz@kernel.org]
> Sent: 23 March 2021 16:23
> To: kvmarm@lists.cs.columbia.edu; kvm@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org
> Cc: Ard Biesheuvel <ardb@kernel.org>; kernel-team@android.com; Shameerali
> Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Subject: [PATCH] KVM: arm64: Fix CPU interface MMIO compatibility detection
> 
> In order to detect whether a GICv3 CPU interface is MMIO capable,
> we switch ICC_SRE_EL1.SRE to 0 and check whether it sticks.
> 
> However, this is only possible if *ALL* of the HCR_EL2 interrupt
> overrides are set, and the CPU is perfectly allowed to ignore
> the write to ICC_SRE_EL1 otherwise. This leads KVM to pretend
> that a whole bunch of ARMv8.0 CPUs aren't MMIO-capable, and
> breaks VMs that should work correctly otherwise.
> 
> Fix this by setting IMO/FMO/IMO before touching ICC_SRE_EL1,
> and clear them afterwards. This allows us to reliably detect
> the CPU interface capabilities.
> 

Tested on HiSilicon D06 platform where the original issue(firmware wrongly
advertising GICv2-on-v3 compatibility) was reported and all seems to be fine.

Though not sure whether this fix is relevant to this particular platform,
FWIW,

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

Thanks,
Shameer

> Cc: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Fixes: 9739f6ef053f ("KVM: arm64: Workaround firmware wrongly advertising
> GICv2-on-v3 compatibility")
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/hyp/vgic-v3-sr.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c
> b/arch/arm64/kvm/hyp/vgic-v3-sr.c
> index ee3682b9873c..39f8f7f9227c 100644
> --- a/arch/arm64/kvm/hyp/vgic-v3-sr.c
> +++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c
> @@ -429,6 +429,13 @@ u64 __vgic_v3_get_gic_config(void)
>  	if (has_vhe())
>  		flags = local_daif_save();
> 
> +	/*
> +	 * Table 11-2 "Permitted ICC_SRE_ELx.SRE settings" indicates
> +	 * that to be able to set ICC_SRE_EL1.SRE to 0, all the
> +	 * interrupt overrides must be set. You've got to love this.
> +	 */
> +	sysreg_clear_set(hcr_el2, 0, HCR_AMO | HCR_FMO | HCR_IMO);
> +	isb();
>  	write_gicreg(0, ICC_SRE_EL1);
>  	isb();
> 
> @@ -436,6 +443,8 @@ u64 __vgic_v3_get_gic_config(void)
> 
>  	write_gicreg(sre, ICC_SRE_EL1);
>  	isb();
> +	sysreg_clear_set(hcr_el2, HCR_AMO | HCR_FMO | HCR_IMO, 0);
> +	isb();
> 
>  	if (has_vhe())
>  		local_daif_restore(flags);
> --
> 2.30.0


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

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-03-24 15:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-23 16:23 [PATCH] KVM: arm64: Fix CPU interface MMIO compatibility detection Marc Zyngier
2021-03-23 16:23 ` Marc Zyngier
2021-03-23 16:23 ` Marc Zyngier
2021-03-24 15:47 ` Shameerali Kolothum Thodi
2021-03-24 15:47   ` Shameerali Kolothum Thodi
2021-03-24 15:47   ` Shameerali Kolothum Thodi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.