For now, this is really something that ought to be hidden by DmaLib abstraction (Map/Unmap). This would allow the driver to be minimally aware of how the IP is integrated into the SoC.

A

> -----Original Message-----
> From: Sunil V L <sunilvl@ventanamicro.com>
> Sent: Monday, January 8, 2024 11:32 PM
> To: Pedro Falcato <pedro.falcato@gmail.com>
> Cc: devel@edk2.groups.io; dhaval@rivosinc.com; yorange
> <yangcheng.work@foxmail.com>; Warkentin, Andrei
> <andrei.warkentin@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
> Leif Lindholm <quic_llindhol@quicinc.com>
> Subject: Re: [edk2-devel] [PATCH v10 4/5] MdePkg: Utilize Cache
> Management Operations Implementation For RISC-V
>
> On Mon, Jan 08, 2024 at 09:53:46PM +0000, Pedro Falcato wrote:
> > On Mon, Jan 8, 2024 at 4:23 PM Dhaval Sharma <dhaval@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