On Mon, Jan 08, 2024 at 09:53:46PM +0000, Pedro Falcato wrote:
> On Mon, Jan 8, 2024 at 4:23 PM Dhaval Sharma <dha...@rivosinc.com> wrote:
> >
> > Hi yangcheng/Pedro,
>
> +CC a bunch of relevant people
>
> Hi, (FYI you did not CC me)
>
> Looking at yangcheng's example:
>
> Status = PrepareDmaData (gpIdmacDesc, Length, Buffer); <-- We write
> to the IDMAC desc
> if (EFI_ERROR (Status)) {
> goto out;
> }
>
> WriteBackDataCacheRange (gpIdmacDesc, DescPages * EFI_PAGE_SIZE);
> <-- Make sure it's DMA-coherent
> StartDma (Length); <-- We've flushed the cache, everything is now in
> DRAM and DMA-coherent, start DMA
>
> which screams of "bad abstractions" because you don't actually need to
> write data back, if the device and platform are DMA coherent.
>
> So what we want here really depends. My local "Volume I: RISC-V
> Unprivileged ISA V20191213" says, section A.5:
>
> "Table A.5 provides a mapping of Linux memory ordering macros onto
> RISC-V memory instructions.
> The Linux fences dma rmb() and dma wmb() map onto FENCE R,R and FENCE
> W,W, respectively,
> since the RISC-V Unix Platform requires coherent DMA, but would be
> mapped onto FENCE RI,RI
> and FENCE WO,WO, respectively, on a platform with non-coherent DMA.
> Platforms with non-
> coherent DMA may also require a mechanism by which cache lines can be
> flushed and/or invalidated.
> Such mechanisms will be device-specific and/or standardized in a
> future extension to the ISA."
>
> The (current date) RISCV Platform Spec also says: "Memory accesses by
> I/O masters can be coherent or non-coherent with respect to all
> hart-related caches."
> which is brilliantly useless.
>
> so I think the best solution here is to:
>
> 1) Add a new PCD for platform DMA coherency, and test that on
> WriteBackDataCacheRange's ASSERT (if (!Coherent) ASSERT() else
> return;)
> 2) Add a more abstracting API that doesn't necessarily map to
> WriteBackDataCache when all we wanted was to assert DMA coherency
>
> but, alas, I've seen a lot less funky platforms than many of you, and
> DMA/cache-coherency is not really my thing, so I'll defer to others..
>
My preference is just remove the assertion and add the debug verbose
message instead of changing drivers/introduce new interfaces. It is a
nop in linux as well if CMO is not present.
Thanks,
Sunil
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113422): https://edk2.groups.io/g/devel/message/113422
Mute This Topic: https://groups.io/mt/103150435/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
