Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Content-Type: text/plain;
        charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 14.0 \(3654.60.0.2.5\))
Subject: Re: [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple
 processors
From:   Mohamed Mediouni <mohamed.mediouni@caramail.com>
In-Reply-To: <20210121124742.GA22123@willie-the-truck>
Date:   Thu, 21 Jan 2021 16:12:04 +0100
Cc:     Marc Zyngier <maz@kernel.org>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Hector Martin <marcan@marcan.st>, linux-kernel@vger.kernel.org,
        Stan Skowronek <stan@corellium.com>
Content-Transfer-Encoding: quoted-printable
Message-Id: <39067FDD-F7D3-4F37-8B43-1A9FCF80EEE5@caramail.com>
References: <20210120132717.395873-1-mohamed.mediouni@caramail.com>
 <20210120132717.395873-4-mohamed.mediouni@caramail.com>
 <20210121112725.GA21750@willie-the-truck>
 <fdc49d125ef9f520254196509f6c0aa2@kernel.org>
 <20210121124742.GA22123@willie-the-truck>
To:     Will Deacon <will@kernel.org>
Precedence: bulk


> On 21 Jan 2021, at 13:47, Will Deacon <will@kernel.org> wrote:
>=20
> On Thu, Jan 21, 2021 at 11:44:23AM +0000, Marc Zyngier wrote:
>> On 2021-01-21 11:27, Will Deacon wrote:
>>> On Wed, Jan 20, 2021 at 02:27:13PM +0100, Mohamed Mediouni wrote:
>>>> Use nGnRnE instead of nGnRE on Apple SoCs to workaround a serious
>>>> hardware quirk.
>>>>=20
>>>> On Apple processors, writes using the nGnRE device memory type get
>>>> dropped in flight,
>>>> getting to nowhere.
>>>>=20
>>>> Signed-off-by: Stan Skowronek <stan@corellium.com>
>>>> Signed-off-by: Mohamed Mediouni <mohamed.mediouni@caramail.com>
>>>> ---
>>>> arch/arm64/mm/proc.S | 26 ++++++++++++++++++++++++++
>>>> 1 file changed, 26 insertions(+)
>>>>=20
>>>> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
>>>> index 1f7ee8c8b7b8..06436916f137 100644
>>>> --- a/arch/arm64/mm/proc.S
>>>> +++ b/arch/arm64/mm/proc.S
>>>> @@ -51,6 +51,25 @@
>>>> #define TCR_KASAN_HW_FLAGS 0
>>>> #endif
>>>>=20
>>>> +#ifdef CONFIG_ARCH_APPLE
>>>> +
>>>> +/*
>>>> + * Apple cores appear to black-hole writes done with nGnRE.
>>>> + * We settled on a work-around that uses MAIR vs changing every
>>>> single user of
>>>> + * nGnRE across the arm64 code.
>>>> + */
>>>> +
>>>> +#define MAIR_EL1_SET_APPLE						=
\
>>>> +	(MAIR_ATTRIDX(MAIR_ATTR_DEVICE_nGnRnE, MT_DEVICE_nGnRnE) |	=
\
>>>> +	 MAIR_ATTRIDX(MAIR_ATTR_DEVICE_nGnRnE, MT_DEVICE_nGnRE) |	=
\
>>>> +	 MAIR_ATTRIDX(MAIR_ATTR_DEVICE_GRE, MT_DEVICE_GRE) |		=
\
>>>> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL_NC, MT_NORMAL_NC) |		=
\
>>>> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL, MT_NORMAL) |			=
\
>>>> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL_WT, MT_NORMAL_WT) |		=
\
>>>> +	 MAIR_ATTRIDX(MAIR_ATTR_NORMAL, MT_NORMAL_TAGGED))
>>>> +
>>>> +#endif
>>>> +
>>>> /*
>>>>  * Default MAIR_EL1. MT_NORMAL_TAGGED is initially mapped as Normal
>>>> memory and
>>>>  * changed during __cpu_setup to Normal Tagged if the system
>>>> supports MTE.
>>>> @@ -432,6 +451,13 @@ SYM_FUNC_START(__cpu_setup)
>>>> 	 * Memory region attributes
>>>> 	 */
>>>> 	mov_q	x5, MAIR_EL1_SET
>>>> +#ifdef CONFIG_ARCH_APPLE
>>>> +	mrs	x0, MIDR_EL1
>>>> +	lsr	w0, w0, #24
>>>> +	mov_q	x1, MAIR_EL1_SET_APPLE
>>>> +	cmp	x0, #0x61			// 0x61 =3D Implementer: =
Apple
>>>> +	csel	x5, x1, x5, eq
>>>=20
>>> Why does this need to be done so early? It would be a lot cleaner if =
we
>>> could detect this in a similar fashion to other errata and update =
the
>>> MAIR
>>> appropriately. If that's not possible because of early IO mappings
>>> (which
>>> ones?), then we could instead initialise to nGnRnE unconditionally, =
but
>>> relax it to nGnRE if we detect that we _don't_ have the erratum.
>>=20
>> Would that imply another round-trip into the idmap, much like we do
>> when we switch to non-global mappings? Or do you expect that we can =
change
>> the MAIR with live mappings?
>=20
> I think we should be able to change it live and then invalidate the =
TLB. At
> least, my reading of the BBM requirements suggests that it isn't =
required
> for changing between different types of device memory. I can seek
> clarification from Arm if necessary.
>=20
Please ignore that patch.

It turns out that the PCIe controller on Apple M1 expects posted writes =
and so the memory range for it ought to be set nGnRE.=20
So, we need to use nGnRnE for on-chip MMIO and nGnRE for PCIe BARs.

The MAIR approach isn=E2=80=99t adequate for such a thing, so we=E2=80=99l=
l have to look elsewhere.

Thank you,
> Will