Re: [edk2-devel] AARCH64 Cacheability Attributes

["Ard Biesheuvel via groups.io" <ardb=kernel.org@groups.io>]

On Thu, 5 Jun 2025 at 19:55, Oliver Smith-Denny
<osde@linux.microsoft.com> wrote:
>
> On 6/5/2025 10:27 AM, Ard Biesheuvel via groups.io wrote:
> > Hi,
...
> >
> > Simply dropping UC from the code that declares the DRAM in the
> > platform startup code should fix the issue entirely, afaict. (Note
> > that ArmVirtQemu et al declare their system memory as only WB capable
> > and nothing else - this is needed because KVM is fundamentally
> > coherent, as the VMM uses cacheable mappings to access guest memory)
>
> That's a good point and we'll test with that. This would work for this
> case because NonDiscoverablePciDxe toggles between WC and UC and chooses
> WC if UC isn't supported, but there is nothing that specifies that
> behavior globally. It would be valid for a driver to say I need to do
> non-coherent DMA (or some other use case) and to say so I will set the
> attributes to UC. On ARM, you would have to set that to WC, but now the
> driver needs to know architecture level details, which is not ideal. For
> x86, you would want to set UC.
>

No. Whether DMA is coherent or not is /not/ a property of the device,
it is a property of how that device was integrated into the system.

So a device driver should never have to reason about the difference
between coherent or non-coherent DMA, unless it is tightly coupled
with the platform, in which case this would not be an issue.

Note that the NonCoherentDmaLib in EmbeddedPkg already prefers WC over
UC. I don't remember why I didn't do the same when I wrote
NonDiscoverablePciDxe, because it is the obvious correct thing to do
on ARM (and x86 doesn't use this driver at all afaik)


> >
> > Adding more memory types to the spec is not going to solve anything,
> > I'm afraid. We already have some dubious ISA mask types that were
> > added without proper motivation, and I'd rather have fewer types
> > (e.g., on ARM, WT is also just normal non-cacheable under the hood in
> > most cases and I don't have a clue how it is intended to be used).
> > Instead, we should stop using UC for conventional memory.
> >
>
> Yeah, I agree that already the invalid cacheability types are troubling
> and adding more may just make the situation worse. We'll think on this
> some more, I still believe this is a spec problem, that the spec does
> not envision the ARM64 architecture and very explicitly envisions the
> x86 architecture, which leads to issues like this (of course in many
> areas).
>
> I worry about relying on code workarounds for spec issues creates an
> untenable position where a driver thinks it is doing the right thing,
> per spec, but then the juggling under the hood to figure out how to
> translate it to an aarch64 concept breaks down somewhere.
>

The driver should not care about the difference. If there are
currently drivers that need to, we should fix the abstractions
instead.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#121401): https://edk2.groups.io/g/devel/message/121401
Mute This Topic: https://groups.io/mt/113471239/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-