Re: [PATCH 3/6] KVM: arm64: Consider SCTLR_EL2.M when mapping the L1 VNCR page
Joey Gouly <[email protected]> Tue, 4 Aug 2026 12:09:56 +0100
| Newsgroups | dev.linux.lists.kvmarm,org.infradead.lists.linux-arm-kernel,org.kernel.vger.kvm,org.kernel.vger.stable |
|---|---|
| Message-ID | <[email protected]> |
Another thing I forgot..
On Sat, Aug 01, 2026 at 01:48:15PM +0100, Marc Zyngier wrote:
> We record a VNCR TLB even when SCTLR_EL2.M is 0 in order to make
> our life easier. But this is not something that the architecture
> anticipate.
>
> As a consequence, a hypervisor is free to set VNCR_EL2 to
> some PA when SCTLR_EL2.M==0, use it to run a guest which indirectly
> accesses the VNCR page, then eventually set SCTLR_EL2.M==1 with
> the same VA. Yes, this is odd, but apparently legal.
>
> A common trick in HW is to invalidate the TLBs on SCTLR_ELx.M being
> flipped. But doing this is a not a good idea for us (we'd need to
> trap SCTLR accesses), and wouldn't scale as we nest deeper.
>
> Instead, use the fact that the S1 MMU being off at the point of
> translation is cached in our TLB, and simply ignore it if it
> doesn't match the current MMU state.
>
> Fixes: 2a359e072596f ("KVM: arm64: nv: Handle mapping of VNCR_EL2 at EL2")
> Signed-off-by: Marc Zyngier <[email protected]>
> Cc: [email protected]
> ---
> arch/arm64/include/asm/kvm_nested.h | 7 +++++++
> arch/arm64/kvm/at.c | 2 --
> arch/arm64/kvm/nested.c | 3 +++
> 3 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
> index 012d711034d17..cc48817a5bf7a 100644
> --- a/arch/arm64/include/asm/kvm_nested.h
> +++ b/arch/arm64/include/asm/kvm_nested.h
> @@ -388,6 +388,8 @@ struct s1_walk_result {
> bool failed;
> };
>
> +#define S1_MMU_DISABLED (-127)
> +
> static inline void fail_s1_walk(struct s1_walk_result *wr, u8 fst, bool s1ptw)
> {
> wr->fst = fst;
> @@ -396,6 +398,11 @@ static inline void fail_s1_walk(struct s1_walk_result *wr, u8 fst, bool s1ptw)
> wr->failed = true;
> }
>
> +static inline bool s1_mmu_disabled(struct s1_walk_result *wr)
> +{
> + return wr->level == S1_MMU_DISABLED;
> +}
> +
> int __kvm_translate_va(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
> struct s1_walk_result *wr, u64 va);
> int __kvm_find_s1_desc_level(struct kvm_vcpu *vcpu, u64 va, u64 ipa,
> diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c
> index 640f2dc00a8ba..0926426b87989 100644
> --- a/arch/arm64/kvm/at.c
> +++ b/arch/arm64/kvm/at.c
> @@ -11,8 +11,6 @@
> #include <asm/kvm_mmu.h>
> #include <asm/lsui.h>
>
> -#define S1_MMU_DISABLED (-127)
> -
> static int get_ia_size(struct s1_walk_info *wi)
> {
> return 64 - wi->txsz;
> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index 035cda256e2a5..d7dba02dc84fe 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c
> @@ -1578,6 +1578,9 @@ static void kvm_map_l1_vncr(struct kvm_vcpu *vcpu)
> if (!vt->valid)
> return;
>
> + if (!(vcpu_read_sys_reg(vcpu, SCTLR_EL2) & SCTLR_ELx_M) != s1_mmu_disabled(&vt->wr))
> + return;
Another thing, should we set valid=false? (Haven't really looked into
what that implies..)
Thanks,
Joey