Hi Olivier, Soby,
Yes, thank you for the discussion. I will submit a patch with this change.
Best regards, Andrei
From: Olivier Deprez Olivier.Deprez@arm.com Sent: Tuesday, May 13, 2025 4:59 PM To: Soby Mathew Soby.Mathew@arm.com; Andrei Stefanescu andrei.stefanescu@nxp.com; tf-a@lists.trustedfirmware.org Cc: Ciprian Marian Costea ciprianmarian.costea@nxp.com; Alexandru-Catalin Ionita alexandru-catalin.ionita@nxp.com Subject: [EXT] Re: Question about memory mapping attributes for MT_DEVICE
You don't often get email from olivier.deprez@arm.commailto:olivier.deprez@arm.com. Learn why this is importanthttps://aka.ms/LearnAboutSenderIdentification
Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button
Hi Soby,
On second thought, yes I agree it is probably best fixed for all unilaterally.
Hi Andrei,
To conclude about,
* Would it be ok to send a patch which changes the ATTR_DEVICE to nGnRnE?
I believe it is ok then to submit such change. We'll make sure to confirm with a full CI run at least to cover Arm FVP's and few physical boards.
Regards, Olivier.
________________________________ From: Soby Mathew <Soby.Mathew@arm.commailto:Soby.Mathew@arm.com> Sent: 13 May 2025 14:59 To: Olivier Deprez <Olivier.Deprez@arm.commailto:Olivier.Deprez@arm.com>; Andrei Stefanescu <andrei.stefanescu@nxp.commailto:andrei.stefanescu@nxp.com>; tf-a@lists.trustedfirmware.orgmailto:tf-a@lists.trustedfirmware.org <tf-a@lists.trustedfirmware.orgmailto:tf-a@lists.trustedfirmware.org> Cc: Ciprian Marian Costea <ciprianmarian.costea@nxp.commailto:ciprianmarian.costea@nxp.com>; Alexandru-Catalin Ionita <alexandru-catalin.ionita@nxp.commailto:alexandru-catalin.ionita@nxp.com> Subject: RE: Question about memory mapping attributes for MT_DEVICE
Ack, I could see your concern :) .
One way to avoid globally affecting the Device attribute could be to define a new Device attribute, “MT_DEVICE_STRONG”, as nGnRnE and use that for the coherent region needed for Bakery Lock. But because MMU settings are done in the platform layer, unless the platform uses the new macro, the inconsistency may remain.
On the other hand, it is worth to get this fixed properly for all platforms and changing MT_DEVICE globally will achieve that.
Best Regards
Soby Mathew
From: Olivier Deprez <Olivier.Deprez@arm.commailto:Olivier.Deprez@arm.com> Sent: Tuesday, May 13, 2025 1:43 PM To: Soby Mathew <Soby.Mathew@arm.commailto:Soby.Mathew@arm.com>; Andrei Stefanescu <andrei.stefanescu@nxp.commailto:andrei.stefanescu@nxp.com>; tf-a@lists.trustedfirmware.orgmailto:tf-a@lists.trustedfirmware.org Cc: Ciprian Marian Costea <ciprianmarian.costea@nxp.commailto:ciprianmarian.costea@nxp.com>; Alexandru-Catalin Ionita <alexandru-catalin.ionita@nxp.commailto:alexandru-catalin.ionita@nxp.com> Subject: Re: Question about memory mapping attributes for MT_DEVICE
Hi Soby,
* We need to use the MT_Device memory type.
MT_DEVICE xlat lib. option matches nGnRE presently.
I wanted to point out that changing MT_DEVICE definition unilaterally in xlat lib. to nGnRnE changes the behavior for all peripherals MMIO (not just the region concerned by the bakery lock), and also changes the behavior to all platforms. This might come as a breaking change eventually as we don't know the effect of changing this memory type affecting all downstream platforms. But I agree this risk is limited as you mention.
I was suggesting changing the memory type only for the region concerned by the bakery lock (if possible) as opposed to changing the MT_DEVICE definition globally.
Regards,
Olivier.
________________________________
From: Soby Mathew <Soby.Mathew@arm.commailto:Soby.Mathew@arm.com> Sent: 13 May 2025 14:25 To: Olivier Deprez <Olivier.Deprez@arm.commailto:Olivier.Deprez@arm.com>; Andrei Stefanescu <andrei.stefanescu@nxp.commailto:andrei.stefanescu@nxp.com>; tf-a@lists.trustedfirmware.orgmailto:tf-a@lists.trustedfirmware.org <tf-a@lists.trustedfirmware.orgmailto:tf-a@lists.trustedfirmware.org> Cc: Ciprian Marian Costea <ciprianmarian.costea@nxp.commailto:ciprianmarian.costea@nxp.com>; Alexandru-Catalin Ionita <alexandru-catalin.ionita@nxp.commailto:alexandru-catalin.ionita@nxp.com> Subject: RE: Question about memory mapping attributes for MT_DEVICE
Hi Andrei,
I think you are right about this inconsistency. AFAIK, AArch64 memory model says that when S1 MMU is off, all Data accesses are Device_nGnRnE. And this means that there is inconsistent memory attributes when USE_COHERENT_MEM=1 in current TF-A.
My opinion is that this is probably an oversight and need to be corrected. Since nGnRnE has stronger ordering than nGnRE, it is unlikely to cause any problem.
I think MT_NON_CACHEABLE does not have the required properties for being used in the Bakery Lock algorithm and we will still have the inconsistent memory attributes when accessed with S1 MMU off. We need to use the MT_Device memory type.
Best Regards
Soby Mathew
From: Olivier Deprez via TF-A <tf-a@lists.trustedfirmware.orgmailto:tf-a@lists.trustedfirmware.org> Sent: Tuesday, May 13, 2025 1:00 PM To: Andrei Stefanescu <andrei.stefanescu@nxp.commailto:andrei.stefanescu@nxp.com>; tf-a@lists.trustedfirmware.orgmailto:tf-a@lists.trustedfirmware.org Cc: Ciprian Marian Costea <ciprianmarian.costea@nxp.commailto:ciprianmarian.costea@nxp.com>; Alexandru-Catalin Ionita <alexandru-catalin.ionita@nxp.commailto:alexandru-catalin.ionita@nxp.com> Subject: [TF-A] Re: Question about memory mapping attributes for MT_DEVICE
Hi,
On peripheral regions mapped as nGnRE, and by browsing git history, I can only really tell that this mapping exists since first TF-A commit in 2013 (!).
https://git.trustedfirmware.org/plugins/gitiles/TF-A/trusted-firmware-a/+/4f...
I'm a bit nervous about changing this attribute globally which will affect all peripherals mapped as MT_DEVICE.
For the use case you mention, I appreciate you're intending to map a 'normal' memory region e.g. DRAM or SRAM (as opposed to a peripheral MMIO), so I'd be inclined to say you could map this region as MT_NON_CACHEABLE achieving the same purpose?
Regards,
Olivier.
________________________________
From: Olivier Deprez <Olivier.Deprez@arm.commailto:Olivier.Deprez@arm.com> Sent: 13 May 2025 13:42 To: Andrei Stefanescu <andrei.stefanescu@nxp.commailto:andrei.stefanescu@nxp.com>; tf-a@lists.trustedfirmware.orgmailto:tf-a@lists.trustedfirmware.org <tf-a@lists.trustedfirmware.orgmailto:tf-a@lists.trustedfirmware.org> Cc: Ghennadi Procopciuc <ghennadi.procopciuc@nxp.commailto:ghennadi.procopciuc@nxp.com>; Ciprian Marian Costea <ciprianmarian.costea@nxp.commailto:ciprianmarian.costea@nxp.com>; Alexandru-Catalin Ionita <alexandru-catalin.ionita@nxp.commailto:alexandru-catalin.ionita@nxp.com> Subject: Re: Question about memory mapping attributes for MT_DEVICE
The email was moderated, re-sending on behalf of Andrei.
________________________________
From: Andrei Stefanescu <andrei.stefanescu@nxp.commailto:andrei.stefanescu@nxp.com> Sent: 13 May 2025 12:35 To: tf-a@lists.trustedfirmware.orgmailto:tf-a@lists.trustedfirmware.org <tf-a@lists.trustedfirmware.orgmailto:tf-a@lists.trustedfirmware.org> Cc: Ghennadi Procopciuc <ghennadi.procopciuc@nxp.commailto:ghennadi.procopciuc@nxp.com>; Ciprian Marian Costea <ciprianmarian.costea@nxp.commailto:ciprianmarian.costea@nxp.com>; Alexandru-Catalin Ionita <alexandru-catalin.ionita@nxp.commailto:alexandru-catalin.ionita@nxp.com> Subject: Question about memory mapping attributes for MT_DEVICE
Hi,
I noticed that the memory mapping attributes for MT_DEVICE memory are defined to: nGnRE [1]. Why was nGnRE selected instead of nGnRnE?
Platforms which have USE_COHERENT_MEM set to 1 will map the coherent memory area as MT_DEVICE. This area is helpful for cases where a backery lock is shared between cores which have MMU enabled and cores which don’t (whose access is equivalent to nGnRnE). This would generate an access attributes mismatch for the coherent memory area.
Would it be ok to send a patch which changes the ATTR_DEVICE to nGnRnE?
Best regards, Andrei Stefanescu
[1] - https://git.trustedfirmware.org/plugins/gitiles/TF-A/trusted-firmware-a.git/...