mbox series

[0/2] KVM: arm64: nv: Fix sysreg RESx-ication

Message ID 20250112165029.1181056-1-maz@kernel.org (mailing list archive)
Headers show
Series KVM: arm64: nv: Fix sysreg RESx-ication | expand

Message

Marc Zyngier Jan. 12, 2025, 4:50 p.m. UTC
Joey recently reported that some rather basic tests were failing on
NV, and managed to track it down to critical register fields (such as
HCR_EL2.E2H) not having their expect value.

Further investigation has outlined a couple of critical issues:

- Evaluating HCR_EL2.E2H must always be done with a sanitising
  accessor, no ifs, no buts. Given that KVM assumes a fixed value for
  this bit, we cannot leave it to the guest to mess with.

- Resetting the sysreg file must result in the RESx bits taking
  effect. Otherwise, we may end-up making the wrong decision (see
  above), and we definitely expose invalid values to the guest. Note
  that because we compute the RESx masks very late in the VM setup, we
  need to apply these masks at that particular point as well.

The two patches in this series are enough to fix the current set of
issues, but __vcpu_sys_reg() needs some extra work as it is doing the
wrong thing when used as a lvalue. I'll post a separate series for
that, as the two problems are fairly orthogonal, and this results in a
significant amount of churn.

All kudos to Joey for patiently tracking that one down. This was
hidden behind a myriad of other issues, and nailing this sucker down
is nothing short of a debugging lesson. Drinks on me next time.

Unless someone shouts, I'll take this in for 6.14.

Marc Zyngier (2):
  KVM: arm64: nv: Always evaluate HCR_EL2 using sanitising accessors
  KVM: arm64: nv: Apply RESx settings to sysreg reset values

 arch/arm64/include/asm/kvm_emulate.h | 36 ++++++++++++----------------
 arch/arm64/include/asm/kvm_nested.h  |  2 +-
 arch/arm64/kvm/hyp/vhe/sysreg-sr.c   |  4 ++--
 arch/arm64/kvm/nested.c              |  9 +++++--
 arch/arm64/kvm/sys_regs.c            |  5 +++-
 5 files changed, 29 insertions(+), 27 deletions(-)

Comments

Joey Gouly Jan. 14, 2025, 11:13 a.m. UTC | #1
On Sun, Jan 12, 2025 at 04:50:27PM +0000, Marc Zyngier wrote:
> Joey recently reported that some rather basic tests were failing on
> NV, and managed to track it down to critical register fields (such as
> HCR_EL2.E2H) not having their expect value.
> 
> Further investigation has outlined a couple of critical issues:
> 
> - Evaluating HCR_EL2.E2H must always be done with a sanitising
>   accessor, no ifs, no buts. Given that KVM assumes a fixed value for
>   this bit, we cannot leave it to the guest to mess with.
> 
> - Resetting the sysreg file must result in the RESx bits taking
>   effect. Otherwise, we may end-up making the wrong decision (see
>   above), and we definitely expose invalid values to the guest. Note
>   that because we compute the RESx masks very late in the VM setup, we
>   need to apply these masks at that particular point as well.
> 
> The two patches in this series are enough to fix the current set of
> issues, but __vcpu_sys_reg() needs some extra work as it is doing the
> wrong thing when used as a lvalue. I'll post a separate series for
> that, as the two problems are fairly orthogonal, and this results in a
> significant amount of churn.
> 
> All kudos to Joey for patiently tracking that one down. This was
> hidden behind a myriad of other issues, and nailing this sucker down
> is nothing short of a debugging lesson. Drinks on me next time.
> 
> Unless someone shouts, I'll take this in for 6.14.
> 

Testing using the updated kvm-arm64/nv-next branch:
Tested-by: Joey Gouly <joey.gouly@arm.com>
Reviewed-by: Joey Gouly <joey.gouly@arm.com>

Thanks!
Joey

> Marc Zyngier (2):
>   KVM: arm64: nv: Always evaluate HCR_EL2 using sanitising accessors
>   KVM: arm64: nv: Apply RESx settings to sysreg reset values
> 
>  arch/arm64/include/asm/kvm_emulate.h | 36 ++++++++++++----------------
>  arch/arm64/include/asm/kvm_nested.h  |  2 +-
>  arch/arm64/kvm/hyp/vhe/sysreg-sr.c   |  4 ++--
>  arch/arm64/kvm/nested.c              |  9 +++++--
>  arch/arm64/kvm/sys_regs.c            |  5 +++-
>  5 files changed, 29 insertions(+), 27 deletions(-)
> 
> -- 
> 2.39.2
>
Marc Zyngier Jan. 14, 2025, 11:37 a.m. UTC | #2
On Sun, 12 Jan 2025 16:50:27 +0000, Marc Zyngier wrote:
> Joey recently reported that some rather basic tests were failing on
> NV, and managed to track it down to critical register fields (such as
> HCR_EL2.E2H) not having their expect value.
> 
> Further investigation has outlined a couple of critical issues:
> 
> - Evaluating HCR_EL2.E2H must always be done with a sanitising
>   accessor, no ifs, no buts. Given that KVM assumes a fixed value for
>   this bit, we cannot leave it to the guest to mess with.
> 
> [...]

Applied to next, thanks!

[1/2] KVM: arm64: nv: Always evaluate HCR_EL2 using sanitising accessors
      commit: c139b6d1b4d27724987af5071177fb5f3d60c1e4
[2/2] KVM: arm64: nv: Apply RESx settings to sysreg reset values
      commit: 36f998de853cfad60508dfdfb41c9c40a2245f19

Cheers,

	M.