linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] KVM: arm64: VHE: Compute fgt traps before activating them
@ 2025-11-12 10:28 Alexandru Elisei
  2025-11-12 10:40 ` Oliver Upton
  2025-11-12 10:54 ` Marc Zyngier
  0 siblings, 2 replies; 4+ messages in thread
From: Alexandru Elisei @ 2025-11-12 10:28 UTC (permalink / raw)
  To: maz, oliver.upton, joey.gouly, suzuki.poulose, yuzenghui,
	linux-arm-kernel, kvmarm

On VHE, the Fine Grain Traps registers are written to hardware in
kvm_arch_vcpu_load()->..->__activate_traps_hfgxtr(), but the fgt array is
computed later, in kvm_vcpu_load_fgt(). This can lead to zero being written
to the FGT registers the first time a VCPU is loaded. Also, any changes to
the fgt array will be visible only after the VCPU is scheduled out, and
then back in, which is not the intended behaviour.

Fix it by computing the fgt array just before the fgt traps are written
to hardware.

Fixes: fb10ddf35c1c ("KVM: arm64: Compute per-vCPU FGTs at vcpu_load()")
Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---

Stumbled upon this when running a Linux guest on FVP with FEAT_S1PIE
enabled.  Linux touches PIRE0_EL1 very early during boot, in __cpu_setup().
HFGWTR_EL2 was 0 the first time the VCPU was run, KVM would then trap
the access to PIR0_EL1 (PIRE0_EL1 is an inverted trap) and trigger the
BUG_ON(!r->access) from perform_access().

I hacked __activate_traps_hfgxtr() to print the register value for
HFGWTR_EL2. Before this patch, during the first vcpu_load(),
HFGWTR_EL2 is 0, then it has the correct value. After this patch, it
always has the correct value.

If I were to venture a shot in the dark, it might be that the name is a bit
misleading - it's kvm_vpcu_load_fgt(), but it doesn't load anything onto
hardware, it just computes values. Might be worth renaming to avoid
similar ordering issues in the future.

 arch/arm64/kvm/arm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 870953b4a8a7..052bf0d4d0b0 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -624,6 +624,7 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 	kvm_timer_vcpu_load(vcpu);
 	kvm_vgic_load(vcpu);
 	kvm_vcpu_load_debug(vcpu);
+	kvm_vcpu_load_fgt(vcpu);
 	if (has_vhe())
 		kvm_vcpu_load_vhe(vcpu);
 	kvm_arch_vcpu_load_fp(vcpu);
@@ -642,7 +643,6 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 		vcpu->arch.hcr_el2 |= HCR_TWI;
 
 	vcpu_set_pauth_traps(vcpu);
-	kvm_vcpu_load_fgt(vcpu);
 
 	if (is_protected_kvm_enabled()) {
 		kvm_call_hyp_nvhe(__pkvm_vcpu_load,

base-commit: e9a6fb0bcdd7609be6969112f3fbfcce3b1d4a7c
-- 
2.51.2



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] KVM: arm64: VHE: Compute fgt traps before activating them
  2025-11-12 10:28 [PATCH] KVM: arm64: VHE: Compute fgt traps before activating them Alexandru Elisei
@ 2025-11-12 10:40 ` Oliver Upton
  2025-11-12 10:56   ` Alexandru Elisei
  2025-11-12 10:54 ` Marc Zyngier
  1 sibling, 1 reply; 4+ messages in thread
From: Oliver Upton @ 2025-11-12 10:40 UTC (permalink / raw)
  To: Alexandru Elisei
  Cc: maz, oliver.upton, joey.gouly, suzuki.poulose, yuzenghui,
	linux-arm-kernel, kvmarm

Hi Alex,

On Wed, Nov 12, 2025 at 10:28:53AM +0000, Alexandru Elisei wrote:
> On VHE, the Fine Grain Traps registers are written to hardware in
> kvm_arch_vcpu_load()->..->__activate_traps_hfgxtr(), but the fgt array is
> computed later, in kvm_vcpu_load_fgt(). This can lead to zero being written
> to the FGT registers the first time a VCPU is loaded.

Yikes! This is no good, thank you for spotting it.

> Also, any changes to
> the fgt array will be visible only after the VCPU is scheduled out, and
> then back in, which is not the intended behaviour.
> 
> Fix it by computing the fgt array just before the fgt traps are written
> to hardware.
> 
> Fixes: fb10ddf35c1c ("KVM: arm64: Compute per-vCPU FGTs at vcpu_load()")
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>

Reviewed-by: Oliver Upton <oupton@kernel.org>

> ---
> 
> Stumbled upon this when running a Linux guest on FVP with FEAT_S1PIE
> enabled.  Linux touches PIRE0_EL1 very early during boot, in __cpu_setup().
> HFGWTR_EL2 was 0 the first time the VCPU was run, KVM would then trap
> the access to PIR0_EL1 (PIRE0_EL1 is an inverted trap) and trigger the
> BUG_ON(!r->access) from perform_access().
> 
> I hacked __activate_traps_hfgxtr() to print the register value for
> HFGWTR_EL2. Before this patch, during the first vcpu_load(),
> HFGWTR_EL2 is 0, then it has the correct value. After this patch, it
> always has the correct value.
> 
> If I were to venture a shot in the dark, it might be that the name is a bit
> misleading - it's kvm_vpcu_load_fgt(), but it doesn't load anything onto
> hardware, it just computes values. Might be worth renaming to avoid
> similar ordering issues in the future.

Ack, naming isn't quite the best here. The idea was for the name to make
it obvious that it is meant to be used at vcpu_load().

Thanks,
Oliver


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] KVM: arm64: VHE: Compute fgt traps before activating them
  2025-11-12 10:28 [PATCH] KVM: arm64: VHE: Compute fgt traps before activating them Alexandru Elisei
  2025-11-12 10:40 ` Oliver Upton
@ 2025-11-12 10:54 ` Marc Zyngier
  1 sibling, 0 replies; 4+ messages in thread
From: Marc Zyngier @ 2025-11-12 10:54 UTC (permalink / raw)
  To: joey.gouly, suzuki.poulose, yuzenghui, linux-arm-kernel, kvmarm,
	Oliver Upton, Alexandru Elisei

On Wed, 12 Nov 2025 10:28:53 +0000, Alexandru Elisei wrote:
> On VHE, the Fine Grain Traps registers are written to hardware in
> kvm_arch_vcpu_load()->..->__activate_traps_hfgxtr(), but the fgt array is
> computed later, in kvm_vcpu_load_fgt(). This can lead to zero being written
> to the FGT registers the first time a VCPU is loaded. Also, any changes to
> the fgt array will be visible only after the VCPU is scheduled out, and
> then back in, which is not the intended behaviour.
> 
> [...]

Applied to fixes, thanks!

[1/1] KVM: arm64: VHE: Compute fgt traps before activating them
      commit: 85592114ffda568b507bc2b04f5e9afbe7c13b62

Cheers,

	M.
-- 
Without deviation from the norm, progress is not possible.




^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] KVM: arm64: VHE: Compute fgt traps before activating them
  2025-11-12 10:40 ` Oliver Upton
@ 2025-11-12 10:56   ` Alexandru Elisei
  0 siblings, 0 replies; 4+ messages in thread
From: Alexandru Elisei @ 2025-11-12 10:56 UTC (permalink / raw)
  To: Oliver Upton
  Cc: maz, oliver.upton, joey.gouly, suzuki.poulose, yuzenghui,
	linux-arm-kernel, kvmarm

Hi Oliver,

On Wed, Nov 12, 2025 at 02:40:15AM -0800, Oliver Upton wrote:
> Hi Alex,
> 
> On Wed, Nov 12, 2025 at 10:28:53AM +0000, Alexandru Elisei wrote:
> > On VHE, the Fine Grain Traps registers are written to hardware in
> > kvm_arch_vcpu_load()->..->__activate_traps_hfgxtr(), but the fgt array is
> > computed later, in kvm_vcpu_load_fgt(). This can lead to zero being written
> > to the FGT registers the first time a VCPU is loaded.
> 
> Yikes! This is no good, thank you for spotting it.
> 
> > Also, any changes to
> > the fgt array will be visible only after the VCPU is scheduled out, and
> > then back in, which is not the intended behaviour.
> > 
> > Fix it by computing the fgt array just before the fgt traps are written
> > to hardware.
> > 
> > Fixes: fb10ddf35c1c ("KVM: arm64: Compute per-vCPU FGTs at vcpu_load()")
> > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> 
> Reviewed-by: Oliver Upton <oupton@kernel.org>
> 
> > ---
> > 
> > Stumbled upon this when running a Linux guest on FVP with FEAT_S1PIE
> > enabled.  Linux touches PIRE0_EL1 very early during boot, in __cpu_setup().
> > HFGWTR_EL2 was 0 the first time the VCPU was run, KVM would then trap
> > the access to PIR0_EL1 (PIRE0_EL1 is an inverted trap) and trigger the
> > BUG_ON(!r->access) from perform_access().
> > 
> > I hacked __activate_traps_hfgxtr() to print the register value for
> > HFGWTR_EL2. Before this patch, during the first vcpu_load(),
> > HFGWTR_EL2 is 0, then it has the correct value. After this patch, it
> > always has the correct value.
> > 
> > If I were to venture a shot in the dark, it might be that the name is a bit
> > misleading - it's kvm_vpcu_load_fgt(), but it doesn't load anything onto
> > hardware, it just computes values. Might be worth renaming to avoid
> > similar ordering issues in the future.
> 
> Ack, naming isn't quite the best here. The idea was for the name to make
> it obvious that it is meant to be used at vcpu_load().

Actually, having a look at the load functions from kvm_arch_vcpu_load(), the
vast majority don't actually write anything to hardware, so the name is actually
in keeping with the convention, and it was just me being confused by the naming
:)

Thanks,
Alex

> 
> Thanks,
> Oliver


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-11-12 10:56 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-12 10:28 [PATCH] KVM: arm64: VHE: Compute fgt traps before activating them Alexandru Elisei
2025-11-12 10:40 ` Oliver Upton
2025-11-12 10:56   ` Alexandru Elisei
2025-11-12 10:54 ` Marc Zyngier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).