[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aJ9OA/cHk1iFUPyH@e129823.arm.com>
Date: Fri, 15 Aug 2025 16:10:59 +0100
From: Yeoreum Yun <yeoreum.yun@....com>
To: Catalin Marinas <catalin.marinas@....com>
Cc: ryabinin.a.a@...il.com, glider@...gle.com, andreyknvl@...il.com,
dvyukov@...gle.com, vincenzo.frascino@....com, corbet@....net,
will@...nel.org, akpm@...ux-foundation.org,
scott@...amperecomputing.com, jhubbard@...dia.com,
pankaj.gupta@....com, leitao@...ian.org, kaleshsingh@...gle.com,
maz@...nel.org, broonie@...nel.org, oliver.upton@...ux.dev,
james.morse@....com, ardb@...nel.org,
hardevsinh.palaniya@...iconsignals.io, david@...hat.com,
yang@...amperecomputing.com, kasan-dev@...glegroups.com,
workflows@...r.kernel.org, linux-doc@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-mm@...ck.org
Subject: Re: [PATCH v2 1/2] kasan/hw-tags: introduce kasan.store_only option
[...]
> >
> > > + * Not marked as __init as a CPU can be hot-plugged after boot.
> > > + */
> > > +void kasan_late_init_hw_tags_cpu(void)
> > > +{
> > > + /*
> > > + * Enable stonly mode only when explicitly requested through the command line.
> > > + * If system doesn't support, kasan checks all operation.
> > > + */
> > > + kasan_enable_store_only();
> > > +}
> >
> > There's nothing late about this. We have kasan_init_hw_tags_cpu()
> > already and I'd rather have it all handled via this function. It's not
> > that different from how we added asymmetric support, though store-only
> > is complementary to the sync vs async checking.
> >
> > Like we do in mte_enable_kernel_asymm(), if the feature is not available
> > just fall back to checking both reads and writes in the chosen
> > async/sync/asymm way. You can add some pr_info() to inform the user of
> > the chosen kasan mode. It's really mostly an performance choice.
>
> But MTE_STORE_ONLY is defined as a SYSTEM_FEATURE.
> This means that when it is called from kasan_init_hw_tags_cpu(),
> the store_only mode is never set in system_capability,
> so it cannot be checked using cpus_have_cap().
>
> Although the MTE_STORE_ONLY capability is verified by
> directly reading the ID register (seems ugly),
> my concern is the potential for an inconsistent state across CPUs.
>
> For example, in the case of ASYMM, which is a BOOT_CPU_FEATURE,
> all CPUs operate in the same mode —
> if ASYMM is not supported, either
> all CPUs run in synchronous mode, or all run in asymmetric mode.
>
> However, for MTE_STORE_ONLY, CPUs that support the feature will run in store-only mode,
> while those that do not will run with full checking for all operations.
>
> If we want to enable MTE_STORE_ONLY in kasan_init_hw_tags_cpu(),
> I believe it should be reclassified as a BOOT_CPU_FEATURE.x
> Otherwise, the cpu_enable_mte_store_only() function should still be called
> as the enable callback for the MTE_STORE_ONLY feature.
> In that case, kasan_enable_store_only() should be invoked (remove late init),
> and if it returns an error, stop_machine() should be called to disable
> the STORE_ONLY feature on all other CPUs
> if any CPU is found to lack support for MTE_STORE_ONLY.
>
> Am I missing something?
So, IMHO like the ASYMM feature, it would be good to change
MTE_STORE_ONLY as BOOT_CPU_FEATURE.
That would makes everything as easiler and clear.
--
Sincerely,
Yeoreum Yun
Powered by blists - more mailing lists