Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 10 Jul 2023 10:31:19 +0100
Message-ID: <874jmc8654.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     "Aiqun(Maria) Yu" <quic_aiquny@quicinc.com>
Cc:     <will@kernel.org>, <corbet@lwn.net>, <catalin.marinas@arm.com>,
        <quic_pkondeti@quicinc.com>, <quic_kaushalk@quicinc.com>,
        <quic_satyap@quicinc.com>, <quic_shashim@quicinc.com>,
        <quic_songxue@quicinc.com>, <linux-doc@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>,
        <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] arm64: Add the arm64.nolse_atomics command line option
In-Reply-To: <32f442e3-3d5c-4cec-9791-0da039f88287@quicinc.com>
References: <20230710055955.36551-1-quic_aiquny@quicinc.com>
        <875y6s8bwb.wl-maz@kernel.org>
        <32f442e3-3d5c-4cec-9791-0da039f88287@quicinc.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/28.2
 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Mon, 10 Jul 2023 09:19:54 +0100,
"Aiqun(Maria) Yu" <quic_aiquny@quicinc.com> wrote:
> 
> On 7/10/2023 3:27 PM, Marc Zyngier wrote:
> > On Mon, 10 Jul 2023 06:59:55 +0100,
> > Maria Yu <quic_aiquny@quicinc.com> wrote:
> >> 
> >> In order to be able to disable lse_atomic even if cpu
> >> support it, most likely because of memory controller
> >> cannot deal with the lse atomic instructions, use a
> >> new idreg override to deal with it.
> > 
> > In general, the idreg overrides are *not* there to paper over HW bugs.
> > They are there to force the kernel to use or disable a feature for
> > performance reason or to guide the *enabling* of a feature, but not
> > because the HW is broken.
> > 
> > The broken status of a HW platform must also be documented so that we
> > know what to expect when we look at, for example, a bad case of memory
> > corruption (something I'd expect to see on a system that only
> > partially implements atomic memory operations).
> > 
> 
> good idea. A noc error would be happened if the lse atomic instruction
> happened during a memory controller doesn't support lse atomic
> instructions.
> I can put the information in next patchset comment message. Pls feel
> free to let know if there is other place to have this kind of
> information with.

For a start, Documentation/arch/arm64/silicon-errata.rst should
contain an entry for the actual erratum, and a description of the
symptoms of the issue (you're mentioning a "noc error": how is that
reported to the CPU?).

The workaround should also be detected at runtime -- we cannot rely on
the user to provide a command-line argument to disable an essential
feature that anyone has taken for granted for most of a decade...

[...]

> >> @@ -185,6 +195,7 @@ static const struct {
> >>   	{ "arm64.nomops",		"id_aa64isar2.mops=0" },
> >>   	{ "arm64.nomte",		"id_aa64pfr1.mte=0" },
> >>   	{ "nokaslr",			"arm64_sw.nokaslr=1" },
> >> +	{ "arm64.nolse_atomic",         "id_aa64isar0.atomic=0" },
> > 
> > And what of 32bit?

This particular question still stands, as it is likely to affect VMs.

	M.

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