All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] KVM: arm64: nv: Respect HA when computing translated TCR
@ 2026-10-29  9:05 Oliver Upton
  2026-10-29 10:05 ` Fuad Tabba
  0 siblings, 1 reply; 4+ messages in thread
From: Oliver Upton @ 2026-10-29  9:05 UTC (permalink / raw)
  To: kvmarm
  Cc: Marc Zyngier, Joey Gouly, Suzuki K Poulose, Zenghui Yu,
	Wei-Lin Chang, Steffen Eiden, Fuad Tabba, Oliver Upton, stable

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


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

* Re: [PATCH] KVM: arm64: nv: Respect HA when computing translated TCR
  2026-10-29  9:05 [PATCH] KVM: arm64: nv: Respect HA when computing translated TCR Oliver Upton
@ 2026-10-29 10:05 ` Fuad Tabba
  2026-10-29 10:58   ` Marc Zyngier
  0 siblings, 1 reply; 4+ messages in thread
From: Fuad Tabba @ 2026-10-29 10:05 UTC (permalink / raw)
  To: Oliver Upton
  Cc: kvmarm, Marc Zyngier, Joey Gouly, Suzuki K Poulose, Zenghui Yu,
	Wei-Lin Chang, Steffen Eiden, stable

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

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

* Re: [PATCH] KVM: arm64: nv: Respect HA when computing translated TCR
  2026-10-29 10:05 ` Fuad Tabba
@ 2026-10-29 10:58   ` Marc Zyngier
  2026-10-29 11:41     ` Fuad Tabba
  0 siblings, 1 reply; 4+ messages in thread
From: Marc Zyngier @ 2026-10-29 10:58 UTC (permalink / raw)
  To: Fuad Tabba
  Cc: Oliver Upton, kvmarm, Joey Gouly, Suzuki K Poulose, Zenghui Yu,
	Wei-Lin Chang, Steffen Eiden, stable

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.

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

* Re: [PATCH] KVM: arm64: nv: Respect HA when computing translated TCR
  2026-10-29 10:58   ` Marc Zyngier
@ 2026-10-29 11:41     ` Fuad Tabba
  0 siblings, 0 replies; 4+ messages in thread
From: Fuad Tabba @ 2026-10-29 11:41 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Oliver Upton, kvmarm, Joey Gouly, Suzuki K Poulose, Zenghui Yu,
	Wei-Lin Chang, Steffen Eiden, stable

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

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

end of thread, other threads:[~2026-10-29 11:42 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-29  9:05 [PATCH] KVM: arm64: nv: Respect HA when computing translated TCR Oliver Upton
2026-10-29 10:05 ` Fuad Tabba
2026-10-29 10:58   ` Marc Zyngier
2026-10-29 11:41     ` Fuad Tabba

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.