Message-ID: <aYtkbezCx9vW8SHz@e129823.arm.com>
Date: Tue, 10 Feb 2026 17:01:33 +0000
From: Yeoreum Yun <yeoreum.yun@....com>
To: Catalin Marinas <catalin.marinas@....com>
Cc: linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	kvmarm@...ts.linux.dev, kvm@...r.kernel.org,
	linux-kselftest@...r.kernel.org, will@...nel.org, maz@...nel.org,
	broonie@...nel.org, oliver.upton@...ux.dev, miko.lenczewski@....com,
	kevin.brodsky@....com, ardb@...nel.org, suzuki.poulose@....com,
	lpieralisi@...nel.org, scott@...amperecomputing.com,
	joey.gouly@....com, yuzenghui@...wei.com, pbonzini@...hat.com,
	shuah@...nel.org, mark.rutland@....com, arnd@...db.de
Subject: Re: [PATCH v12 2/7] arm64: cpufeature: add FEAT_LSUI

Hi Catalin,

> Hi Levi,
>
> On Tue, Feb 10, 2026 at 09:54:49AM +0000, Yeoreum Yun wrote:
> > > On Fri, Feb 06, 2026 at 06:42:19PM +0000, Catalin Marinas wrote:
> > > > On Wed, Jan 21, 2026 at 07:06:17PM +0000, Yeoreum Yun wrote:
> > > > > +#ifdef CONFIG_ARM64_LSUI
> > > > > +static bool has_lsui(const struct arm64_cpu_capabilities *entry, int scope)
> > > > > +{
> > > > > +	if (!has_cpuid_feature(entry, scope))
> > > > > +		return false;
> > > > > +
> > > > > +	/*
> > > > > +	 * A CPU that supports LSUI should also support FEAT_PAN,
> > > > > +	 * so that SW_PAN handling is not required.
> > > > > +	 */
> > > > > +	if (WARN_ON(!__system_matches_cap(ARM64_HAS_PAN)))
> > > > > +		return false;
> > > > > +
> > > > > +	return true;
> > > > > +}
> > > > > +#endif
> > > >
> > > > I still find this artificial dependency a bit strange. Maybe one doesn't
> > > > want any PAN at all (software or hardware) and won't get LSUI either
> > > > (it's unlikely but possible).
> > > > We have the uaccess_ttbr0_*() calls already for !LSUI, so maybe
> > > > structuring the macros in a way that they also take effect with LSUI.
> > > > For futex, we could add some new functions like uaccess_enable_futex()
> > > > which wouldn't do anything if LSUI is enabled with hw PAN.
> > >
> > > Hmm, I forgot that we removed CONFIG_ARM64_PAN for 7.0, so it makes it
> > > harder to disable. Give it a try but if the macros too complicated, we
> > > can live with the additional check in has_lsui().
> > >
> > > However, for completeness, we need to check the equivalent of
> > > !system_uses_ttbr0_pan() but probing early, something like:
> > >
> > > 	if (IS_ENABLED(CONFIG_ARM64_SW_TTBR0_PAN) &&
> > > 	    !__system_matches_cap(ARM64_HAS_PAN)) {
> > > 		pr_info_once("TTBR0 PAN incompatible with FEAT_LSUI; disabling FEAT_LSUI");
> > > 		return false;
> > > 	}
> >
> > TBH, I'm not sure whether it's a artifical dependency or not.
> > AFAIK, FEAT_PAN is mandatory from Armv8.1 and the FEAT_LSUI seems to
> > implements based on the present of "FEAT_PAN".
> >
> > So, for a hardware which doesn't have FEAT_PAN but has FEAT_LSUI
> > sounds like "wrong" hardware and I'm not sure whether it's right
> > to enable FEAT_LSUI in this case.
>
> In principle we shouldn't have such hardware but, as Will pointed out,
> we might have such combination due to other reasons like virtualisation,
> id reg override.
>
> It's not that FEAT_LSUI requires FEAT_PAN but rather that the way you
> implemented it, the FEAT_LSUI futex code is incompatible with SW_PAN
> because you no longer call uaccess_enable_privileged(). So I suggested a
> small tweak above to make this more obvious. I would also remove the
> WARN_ON, or at least make it WARN_ON_ONCE() if you still want the stack
> dump.
>
> However...
>
> > SW_PAN case is the same problem. Since If system uses SW_PAN,
> > that means this hardware doesn't have a "FEAT_PAN"
> > So this question seems to ultimately boil down to whether
> > it is appropriate to allow the use of FEAT_LSUI
> > even when FEAT_PAN is not supported.
> >
> > That's why I think the purpose of "has_lsui()" is not for artifical
> > dependency but to disable for unlike case which have !FEAT_PAN and FEAT_LSUI
> > and IMHO it's enough to check only check with "ARM64_HAS_PAN" instead of
> > making a new function like uaccess_enable_futex().
>
> Why not keep uaccess_enable_privileged() in
> arch_futex_atomic_op_inuser() and cmpxchg for all cases and make it a
> no-op if FEAT_LSUI is implemented together with FEAT_PAN?

This is because I had a assumption FEAT_PAN must be present
when FEAT_LSUI is presented and this was not considering the virtualisation case.
and  FEAT_PAN is present uaccess_ttbr0_enable() becomes nop and
following feedback you gave - https://lore.kernel.org/all/aJ9oIes7LLF3Nsp1@arm.com/
and the reason you mention last, It doesn't need to call mte_enable_tco().

That's why I thought it doesn't need to call uaccess_enable_privileged().

But for a compatibility with SW_PAN, I think we can put only
uaccess_ttbr0_enable() in arch_futex_atomic_op_inuser() and cmpxchg simply
instead of adding a new APIs uaccess_enable_futex() and
by doing this I think has_lsui() can be removed with its WRAN.

Am I missing something?

> A quick grep shows a recent addition in __lse_swap_desc() (and the llsc equivalent)
> but this one can also use CAST with FEAT_LSUI.

Thanks. I'll apply this with FEAT_LSUI in next round.

>
> BTW, with the removal of uaccess_enable_privileged(), we now get MTE tag
> checks for the futex operations. I think that's good as it matches the
> other uaccess ops, though it's a slight ABI change. If we want to
> preserve the old behaviour, we definitely need
> uaccess_enable_privileged() that only does mte_enable_tco().

I think we don't need to preserve the old behaviour. so we can skip
mte_enable_tco() in case of FEAT_LSUI is presented.


Thanks.

--
Sincerely,
Yeoreum Yun

Powered by blists - more mailing lists