Re: [edk2-devel] [PATCH v10 4/5] MdePkg: Utilize Cache Management Operations Implementation For RISC-V

["Dhaval Sharma" <dhaval@rivosinc.com>]

[-- Attachment #1: Type: text/plain, Size: 4196 bytes --]

Based on the above discussion, and some more thoughts I am thinking it is
okay to at least replace ASSERT from CMO code and let other platform code
place its own guards to avoid calling this code when it is known that
platform does not support such operations. If there are no objections to
this we can go ahead.

On Tue, Jan 9, 2024 at 9:50 PM Warkentin, Andrei <andrei.warkentin@intel.com>
wrote:

> 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
>


-- 
Thanks!
=D


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



[-- Attachment #2: Type: text/html, Size: 6272 bytes --]