Re: [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors

2021-01-21 Thread Will Deacon
On Thu, Jan 21, 2021 at 06:15:06PM +, Marc Zyngier wrote:
> On 2021-01-21 17:55, Will Deacon wrote:
> > On Thu, Jan 21, 2021 at 04:25:54PM +, Marc Zyngier wrote:
> > > On 2021-01-21 15:12, Mohamed Mediouni wrote:
> > > > 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.
> > > > So, we need to use nGnRnE for on-chip MMIO and nGnRE for PCIe BARs.
> > > >
> > > > The MAIR approach isn’t adequate for such a thing, so we’ll have to
> > > > look elsewhere.
> > > 
> > > Well, there isn't many alternative to having a memory type defined
> > > in MAIR if you want to access your PCIe devices with specific
> > > semantics.
> > > 
> > > It probably means defining a memory type for PCI only, but:
> > > - we only have a single free MT entry, and I'm not sure we can
> > >   afford to waste this on a specific platform (can we re-purpose
> > >   GRE instead?),
> > 
> > We already have an nGnRnE MAIR for config space accesses.
> 
> I'm confused. If M1 needs nGnRE for PCI, and overrides nGnRE to nE
> for its in-SoC accesses, where does nGnRE goes?
> 
> Or do you propose that it is the page tables that get a different
> MT index?

Right, I'm just saying that we already have an nGnRnE MAIR configuration
so there's no need to worry about running out of entries if we need both
nGnRE and nGnRnE to co-exist. The nasty part is how to plumb this into
the mappings only for on-chip MMIO; I guess either a new API or we get
ioremap() to pick the memory type based on the address :/

Will


Re: [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors

2021-01-21 Thread Mohamed Mediouni


> On 21 Jan 2021, at 19:15, Marc Zyngier  wrote:
> 
> On 2021-01-21 17:55, Will Deacon wrote:
>> On Thu, Jan 21, 2021 at 04:25:54PM +, Marc Zyngier wrote:
>>> On 2021-01-21 15:12, Mohamed Mediouni wrote:
 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.
 So, we need to use nGnRnE for on-chip MMIO and nGnRE for PCIe BARs.
 
 The MAIR approach isn’t adequate for such a thing, so we’ll have to
 look elsewhere.
>>> Well, there isn't many alternative to having a memory type defined
>>> in MAIR if you want to access your PCIe devices with specific
>>> semantics.
>>> It probably means defining a memory type for PCI only, but:
>>> - we only have a single free MT entry, and I'm not sure we can
>>> afford to waste this on a specific platform (can we re-purpose
>>> GRE instead?),
>> We already have an nGnRnE MAIR for config space accesses.
> 
> I'm confused. If M1 needs nGnRE for PCI, and overrides nGnRE to nE
> for its in-SoC accesses, where does nGnRE goes?
> 
> Or do you propose that it is the page tables that get a different
> MT index?
> 

That MAIR patch that I added overrides nGnRE accesses to nGnRnE.

Linux tries to access to those SoC devices using nGnRE as the device
memory type without that workaround.

Maybe have a device tree property to override the used device memory type
for a given device on the SoC? Or that’s too big for what’s at the end just one 
particular set of SoCs?

But what the hardware wants is accesses to in-SoC devices being nGnRnE
and access to the PCIe BARs being nGnRE.

So both have to be supported…

>   M.
> -- 
> Jazz is not dead. It just smells funny...



Re: [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors

2021-01-21 Thread Marc Zyngier

On 2021-01-21 17:55, Will Deacon wrote:

On Thu, Jan 21, 2021 at 04:25:54PM +, Marc Zyngier wrote:

On 2021-01-21 15:12, Mohamed Mediouni wrote:
> 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.
> So, we need to use nGnRnE for on-chip MMIO and nGnRE for PCIe BARs.
>
> The MAIR approach isn’t adequate for such a thing, so we’ll have to
> look elsewhere.

Well, there isn't many alternative to having a memory type defined
in MAIR if you want to access your PCIe devices with specific
semantics.

It probably means defining a memory type for PCI only, but:
- we only have a single free MT entry, and I'm not sure we can
  afford to waste this on a specific platform (can we re-purpose
  GRE instead?),


We already have an nGnRnE MAIR for config space accesses.


I'm confused. If M1 needs nGnRE for PCI, and overrides nGnRE to nE
for its in-SoC accesses, where does nGnRE goes?

Or do you propose that it is the page tables that get a different
MT index?

M.
--
Jazz is not dead. It just smells funny...


Re: [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors

2021-01-21 Thread Will Deacon
On Thu, Jan 21, 2021 at 04:25:54PM +, Marc Zyngier wrote:
> On 2021-01-21 15:12, Mohamed Mediouni wrote:
> > 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.
> > So, we need to use nGnRnE for on-chip MMIO and nGnRE for PCIe BARs.
> > 
> > The MAIR approach isn’t adequate for such a thing, so we’ll have to
> > look elsewhere.
> 
> Well, there isn't many alternative to having a memory type defined
> in MAIR if you want to access your PCIe devices with specific
> semantics.
> 
> It probably means defining a memory type for PCI only, but:
> - we only have a single free MT entry, and I'm not sure we can
>   afford to waste this on a specific platform (can we re-purpose
>   GRE instead?),

We already have an nGnRnE MAIR for config space accesses.

Will


Re: [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors

2021-01-21 Thread Marc Zyngier

On 2021-01-21 15:12, Mohamed Mediouni wrote:

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.
So, we need to use nGnRnE for on-chip MMIO and nGnRE for PCIe BARs.

The MAIR approach isn’t adequate for such a thing, so we’ll have to
look elsewhere.


Well, there isn't many alternative to having a memory type defined
in MAIR if you want to access your PCIe devices with specific
semantics.

It probably means defining a memory type for PCI only, but:
- we only have a single free MT entry, and I'm not sure we can
  afford to waste this on a specific platform (can we re-purpose
  GRE instead?),
- we'd need to teach the PCI code to use this...

Thanks,

M.
--
Jazz is not dead. It just smells funny...


Re: [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors

2021-01-21 Thread Mohamed Mediouni



> On 21 Jan 2021, at 13:47, Will Deacon  wrote:
> 
> On Thu, Jan 21, 2021 at 11:44:23AM +, 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.
 
 On Apple processors, writes using the nGnRE device memory type get
 dropped in flight,
 getting to nowhere.
 
 Signed-off-by: Stan Skowronek 
 Signed-off-by: Mohamed Mediouni 
 ---
 arch/arm64/mm/proc.S | 26 ++
 1 file changed, 26 insertions(+)
 
 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
 
 +#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 = Implementer: Apple
 +  cselx5, x1, x5, eq
>>> 
>>> 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.
>> 
>> 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?
> 
> 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.
> 
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. 
So, we need to use nGnRnE for on-chip MMIO and nGnRE for PCIe BARs.

The MAIR approach isn’t adequate for such a thing, so we’ll have to look 
elsewhere.

Thank you,
> Will



Re: [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors

2021-01-21 Thread Will Deacon
On Thu, Jan 21, 2021 at 11:44:23AM +, 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.
> > > 
> > > On Apple processors, writes using the nGnRE device memory type get
> > > dropped in flight,
> > > getting to nowhere.
> > > 
> > > Signed-off-by: Stan Skowronek 
> > > Signed-off-by: Mohamed Mediouni 
> > > ---
> > >  arch/arm64/mm/proc.S | 26 ++
> > >  1 file changed, 26 insertions(+)
> > > 
> > > 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
> > > 
> > > +#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 = Implementer: Apple
> > > + cselx5, x1, x5, eq
> > 
> > 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.
> 
> 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?

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.

Will


Re: [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors

2021-01-21 Thread Marc Zyngier

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.


On Apple processors, writes using the nGnRE device memory type get 
dropped in flight,

getting to nowhere.

Signed-off-by: Stan Skowronek 
Signed-off-by: Mohamed Mediouni 
---
 arch/arm64/mm/proc.S | 26 ++
 1 file changed, 26 insertions(+)

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

+#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 = Implementer: Apple
+   cselx5, x1, x5, eq


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.


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?

M.
--
Jazz is not dead. It just smells funny...


Re: [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors

2021-01-21 Thread Arnd Bergmann
On Thu, Jan 21, 2021 at 12:32 PM 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.
> >  /*
> >   * 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 = Implementer: Apple
> > + cselx5, x1, x5, eq
>
> 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.

There is (at least) the custom SMP startup code that uses device
mappings. If that's the only thing that needs the modified MAIR
to be used early, I'd consider that one more reason against doing the
custom cpu_operations for now.

   Arnd


Re: [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors

2021-01-21 Thread Will Deacon
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.
> 
> On Apple processors, writes using the nGnRE device memory type get dropped in 
> flight,
> getting to nowhere.
> 
> Signed-off-by: Stan Skowronek 
> Signed-off-by: Mohamed Mediouni 
> ---
>  arch/arm64/mm/proc.S | 26 ++
>  1 file changed, 26 insertions(+)
> 
> 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
> 
> +#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 = Implementer: Apple
> + cselx5, x1, x5, eq

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.

Will


Re: [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors

2021-01-20 Thread Alexander Graf




On 20.01.21 19:06, Mohamed Mediouni wrote:



On 20 Jan 2021, at 17:47, Alexander Graf  wrote:

On 20.01.21 14:27, Mohamed Mediouni wrote:

Use nGnRnE instead of nGnRE on Apple SoCs to workaround a serious hardware 
quirk.
On Apple processors, writes using the nGnRE device memory type get dropped in 
flight,
getting to nowhere.
Signed-off-by: Stan Skowronek 
Signed-off-by: Mohamed Mediouni 
---
  arch/arm64/mm/proc.S | 26 ++
  1 file changed, 26 insertions(+)
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
+#ifdef CONFIG_ARCH_APPLE


Is there any particular reason for this #ifdef?


Alex


Not a particular reason, as we explicitly check for the implementer ID. However,
without CONFIG_ARCH_APPLE, other parts of the support for Apple CPUs
will not be available anyway.


The ifdef below for code looks ok to me, I'm explicitly wondering why 
you're guarding the #define :)


Alex


+
+/*
+ * 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 = Implementer: Apple
+cselx5, x1, x5, eq
+#endif
  #ifdef CONFIG_ARM64_MTE
  mte_tcr .reqx20
--
2.29.2





Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879





Re: [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors

2021-01-20 Thread Mohamed Mediouni



> On 20 Jan 2021, at 17:47, Alexander Graf  wrote:
> 
> On 20.01.21 14:27, Mohamed Mediouni wrote:
>> Use nGnRnE instead of nGnRE on Apple SoCs to workaround a serious hardware 
>> quirk.
>> On Apple processors, writes using the nGnRE device memory type get dropped 
>> in flight,
>> getting to nowhere.
>> Signed-off-by: Stan Skowronek 
>> Signed-off-by: Mohamed Mediouni 
>> ---
>>  arch/arm64/mm/proc.S | 26 ++
>>  1 file changed, 26 insertions(+)
>> 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
>> +#ifdef CONFIG_ARCH_APPLE
> 
> Is there any particular reason for this #ifdef?
> 
> 
> Alex
> 
Not a particular reason, as we explicitly check for the implementer ID. However,
without CONFIG_ARCH_APPLE, other parts of the support for Apple CPUs
will not be available anyway.
>> +
>> +/*
>> + * 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 = Implementer: Apple
>> +cselx5, x1, x5, eq
>> +#endif
>>  #ifdef CONFIG_ARM64_MTE
>>  mte_tcr .reqx20
>> --
>> 2.29.2
> 
> 
> 
> 
> Amazon Development Center Germany GmbH
> Krausenstr. 38
> 10117 Berlin
> Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
> Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
> Sitz: Berlin
> Ust-ID: DE 289 237 879



Re: [RFC PATCH 3/7] arm64: mm: use nGnRnE instead of nGnRE on Apple processors

2021-01-20 Thread Alexander Graf

On 20.01.21 14:27, Mohamed Mediouni wrote:

Use nGnRnE instead of nGnRE on Apple SoCs to workaround a serious hardware 
quirk.

On Apple processors, writes using the nGnRE device memory type get dropped in 
flight,
getting to nowhere.

Signed-off-by: Stan Skowronek 
Signed-off-by: Mohamed Mediouni 
---
  arch/arm64/mm/proc.S | 26 ++
  1 file changed, 26 insertions(+)

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

+#ifdef CONFIG_ARCH_APPLE


Is there any particular reason for this #ifdef?


Alex


+
+/*
+ * 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 = Implementer: Apple
+   cselx5, x1, x5, eq
+#endif
  #ifdef CONFIG_ARM64_MTE
mte_tcr .reqx20

--
2.29.2






Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879