[PATCH] KVM: arm64: nv: Respect HA when computing translated TCR

[PATCH] KVM: arm64: nv: Respect HA when computing translated TCR

[Oliver Upton]

Add the missing forwarding of TCR_EL2.HA to TCR_EL1.HA, which are
different bit indices, to ensure that FEAT_HAF actually works for an L1
hypervisor in the EL2 translation regime.

Cc: stable@vger.kernel.org
Fixes: d5bbb76f4474 ("KVM: arm64: nv: Expose hardware access flag management to NV guests")
Signed-off-by: Oliver Upton <oupton@kernel.org>
---
Spinning this off from the HAFDBS series since this fixes an issue with
FEAT_HAF which is already supported in NV.

 arch/arm64/include/asm/kvm_nested.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
index 0f899addafa8..2811ec466d88 100644
--- a/arch/arm64/include/asm/kvm_nested.h
+++ b/arch/arm64/include/asm/kvm_nested.h
@@ -29,7 +29,8 @@ static inline u64 translate_tcr_el2_to_tcr_el1(u64 tcr)
 	       (tcr & TCR_EL2_TG0_MASK) |
 	       (tcr & TCR_EL2_ORGN0_MASK) |
 	       (tcr & TCR_EL2_IRGN0_MASK) |
-	       (tcr & TCR_EL2_T0SZ_MASK);
+	       (tcr & TCR_EL2_T0SZ_MASK) |
+	       ((tcr & TCR_EL2_HA) ? TCR_HA : 0);
 }
 
 static inline u64 translate_cptr_el2_to_cpacr_el1(u64 cptr_el2)

base-commit: 9e9283e4c941e806d2735f09b66763487bc48b50
-- 
2.47.3

Re: [PATCH] KVM: arm64: nv: Respect HA when computing translated TCR

[Fuad Tabba]

Hi Oliver,

On Thu, 29 Oct 2026 09:05:22 +0000, Oliver Upton <oupton@kernel.org> wrote:
[...]
> diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
[...]
> @@ -29,7 +29,8 @@ static inline u64 translate_tcr_el2_to_tcr_el1(u64 tcr)
>                (tcr & TCR_EL2_TG0_MASK) |
>                (tcr & TCR_EL2_ORGN0_MASK) |
>                (tcr & TCR_EL2_IRGN0_MASK) |
> -              (tcr & TCR_EL2_T0SZ_MASK);
> +              (tcr & TCR_EL2_T0SZ_MASK) |
> +              ((tcr & TCR_EL2_HA) ? TCR_HA : 0);
>  }

Should you forward SH0 in this patch as well? It's missing here, so
the walks are Non-shareable, and looking at the ARM ARM, this would
make every hardware Access flag update CONSTRAINED UNPREDICTABLE
(R_WLTNL, and the IsWBShareable() check in AArch64_S1Translate()). An
L1 that programmed SH0 as Inner Shareable could then take an
unsupported atomic update fault.

HPD isn't forwarded either, although at.c reads it from TCR_EL2 for an
E2H=0 guest. Was that intentional?

Cheers,
/fuad

Re: [PATCH] KVM: arm64: nv: Respect HA when computing translated TCR

[Marc Zyngier]

On Thu, 29 Oct 2026 10:05:19 +0000,
Fuad Tabba <fuad.tabba@linux.dev> wrote:
> 
> Hi Oliver,
> 
> On Thu, 29 Oct 2026 09:05:22 +0000, Oliver Upton <oupton@kernel.org> wrote:
> [...]
> > diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
> [...]
> > @@ -29,7 +29,8 @@ static inline u64 translate_tcr_el2_to_tcr_el1(u64 tcr)
> >                (tcr & TCR_EL2_TG0_MASK) |
> >                (tcr & TCR_EL2_ORGN0_MASK) |
> >                (tcr & TCR_EL2_IRGN0_MASK) |
> > -              (tcr & TCR_EL2_T0SZ_MASK);
> > +              (tcr & TCR_EL2_T0SZ_MASK) |
> > +              ((tcr & TCR_EL2_HA) ? TCR_HA : 0);
> >  }
> 
> Should you forward SH0 in this patch as well? It's missing here, so
> the walks are Non-shareable, and looking at the ARM ARM, this would
> make every hardware Access flag update CONSTRAINED UNPREDICTABLE
> (R_WLTNL, and the IsWBShareable() check in AArch64_S1Translate()). An
> L1 that programmed SH0 as Inner Shareable could then take an
> unsupported atomic update fault.

That's a good point. However, my reading of the ARM ARM indicates that
as long as S2 shareability is Inner-Shareable, the resulting
shareability is Inner-Shareable (see R_ZNLRJ), so we're actually safe.
This should be fixed though.

> HPD isn't forwarded either, although at.c reads it from TCR_EL2 for an
> E2H=0 guest. Was that intentional?

Another bug (E2H==0 really doesn't see much love). This needs fixing,
but also relies on sanitising TCR_EL2. Thankfully, there is [1] which
requires some reviewing... ;-)

Thanks,

	M.

[1] https://lore.kernel.org/r/20260917214215.74135-1-weilin.chang@arm.com

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

Re: [PATCH] KVM: arm64: nv: Respect HA when computing translated TCR

[Fuad Tabba]

Hi Marc,

On Thu, 29 Oct 2026 10:58:54 +0000, Marc Zyngier <maz@kernel.org> wrote:
[...]
> That's a good point. However, my reading of the ARM ARM indicates that
> as long as S2 shareability is Inner-Shareable, the resulting
> shareability is Inner-Shareable (see R_ZNLRJ), so we're actually safe.
> This should be fixed though.

If my reading is correct, I don't think R_ZNLRJ is enough here.
AArch64_S1Translate() (J1.2.4.57 in M.d) checks the walk attributes
before it translates the update through stage 2:

    // Either the access flag was clear or AP[2]/nDirty was set
    if new_desc != descriptor then
        if (!IsWBShareable(descipaddr.memattrs) &&
            ConstrainUnpredictableBool(
                Unpredictable_Unsupported_Atomic_HW_Update)) then
            fault.statuscode = Fault_UnsupportedAtomicHWUpdate;
            return (fault, ARBITRARY : AddressDescriptor);
        end;
        ...
        if regime == Regime_EL10 && EL2Enabled() then
            ...
            (s2fault, descpaddr) = AArch64_S2Translate(fault,
                    descipaddr, s1aarch64, s2aligned, descaccess);

descipaddr is what AArch64_S1Walk() returns, and it holds the stage 1
walk attributes (walkaddress.memattrs = walkstate.memattrs, with SH0
from TCR_EL1), never combined with stage 2. R_VSXXT also covers an
update "not supported in the stage 1 translation due to the defined
memory type": that one is a stage 1 abort, taken to EL1.

But it's not like the ARM ARM is an easy read :P

Cheers,
/fuad

Re: [PATCH] KVM: arm64: nv: Respect HA when computing translated TCR

[Oliver Upton]

On Thu, Oct 29, 2026 at 11:41:22AM +0000, Fuad Tabba wrote:
> Hi Marc,
> 
> On Thu, 29 Oct 2026 10:58:54 +0000, Marc Zyngier <maz@kernel.org> wrote:
> [...]
> > That's a good point. However, my reading of the ARM ARM indicates that
> > as long as S2 shareability is Inner-Shareable, the resulting
> > shareability is Inner-Shareable (see R_ZNLRJ), so we're actually safe.
> > This should be fixed though.
> 
> If my reading is correct, I don't think R_ZNLRJ is enough here.
> AArch64_S1Translate() (J1.2.4.57 in M.d) checks the walk attributes
> before it translates the update through stage 2:
> 
>     // Either the access flag was clear or AP[2]/nDirty was set
>     if new_desc != descriptor then
>         if (!IsWBShareable(descipaddr.memattrs) &&
>             ConstrainUnpredictableBool(
>                 Unpredictable_Unsupported_Atomic_HW_Update)) then
>             fault.statuscode = Fault_UnsupportedAtomicHWUpdate;
>             return (fault, ARBITRARY : AddressDescriptor);
>         end;
>         ...
>         if regime == Regime_EL10 && EL2Enabled() then
>             ...
>             (s2fault, descpaddr) = AArch64_S2Translate(fault,
>                     descipaddr, s1aarch64, s2aligned, descaccess);
> 
> descipaddr is what AArch64_S1Walk() returns, and it holds the stage 1
> walk attributes (walkaddress.memattrs = walkstate.memattrs, with SH0
> from TCR_EL1), never combined with stage 2. R_VSXXT also covers an
> update "not supported in the stage 1 translation due to the defined
> memory type": that one is a stage 1 abort, taken to EL1.
> 
> But it's not like the ARM ARM is an easy read :P

This is definitely helpful but my understanding is that the pseudocode
is *not* authoritative. I think the line "If a translation table hardware
update is not atomic as observed by other agents ..." in R_NNNMB gives
an implementation enough freedom to decide something Non-Shareable at S1
is actually atomic after the result of S2 translation (e.g. S2FWB).

So if the implementation decides it is atomic, no MMU fault is generated
and the update happens like normal. Now, with that being IMP DEF there's
a good chance the behavior can vary, like we saw on the output of
translation on N3 [*].

I'll add another patch to propagate SH0 and HPD, thanks Fuad.

[*]: c8631ea59b65 ("KVM: arm64: selftests: Explicitly set the page attrs to Inner-Shareable")

Best,
Oliver

Re: [PATCH] KVM: arm64: nv: Respect HA when computing translated TCR

[Fuad Tabba]

On Thu, 29 Oct 2026 at 18:37, Oliver Upton <oupton@kernel.org> wrote:
[...]
>
> This is definitely helpful but my understanding is that the pseudocode
> is *not* authoritative. I think the line "If a translation table hardware
> update is not atomic as observed by other agents ..." in R_NNNMB gives
> an implementation enough freedom to decide something Non-Shareable at S1
> is actually atomic after the result of S2 translation (e.g. S2FWB).

I was under the impression that the pseudocode was more authoritative
than the textual description (bugs aside, which the text can also
have). Maybe someone at ARM could help settle this? :)

> So if the implementation decides it is atomic, no MMU fault is generated
> and the update happens like normal. Now, with that being IMP DEF there's
> a good chance the behavior can vary, like we saw on the output of
> translation on N3 [*].
>
> I'll add another patch to propagate SH0 and HPD, thanks Fuad.

Thanks!
/fuad