From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id AC9047803DF for ; Fri, 12 Jan 2024 07:15:57 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=XhFREmKnYtuCs0qVIX5TajEL7ka0S4qJrEqwYlDxe7I=; c=relaxed/simple; d=groups.io; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject:To:Cc:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type; s=20140610; t=1705043756; v=1; b=coP5MfxomwUNUksil7haSgX8b+6vaeDiEsFE1bqPynaUUlA9xBuVucSScGXbsKMrNPwljU19 tHbS/rZCOjn8R+Dtn7j6M5b//nN9Gc2XMmHrv749VgbkWOR9puIPd7XZrLYC+v5qgwkADF0djNL QteOPsPaoqK6uS17NcySrLr4= X-Received: by 127.0.0.2 with SMTP id tAYVYY7687511xOVEpxkiv2g; Thu, 11 Jan 2024 23:15:56 -0800 X-Received: from mail-qv1-f51.google.com (mail-qv1-f51.google.com [209.85.219.51]) by mx.groups.io with SMTP id smtpd.web10.2365.1705043755168247308 for ; Thu, 11 Jan 2024 23:15:55 -0800 X-Received: by mail-qv1-f51.google.com with SMTP id 6a1803df08f44-6814502e528so1881726d6.2 for ; Thu, 11 Jan 2024 23:15:54 -0800 (PST) X-Gm-Message-State: A0681Yrdz1mj1bMX5O2AWURAx7686176AA= X-Google-Smtp-Source: AGHT+IH/2AKLkGVGZSsLVUj2KJYGlPzNqwC2ItapGletN0JQf7Vk1s5OLzAOIuz6o12nPGQ7IReT/taGUkhDUPL8B7g= X-Received: by 2002:ad4:5d6a:0:b0:680:b48c:b54b with SMTP id fn10-20020ad45d6a000000b00680b48cb54bmr581035qvb.102.1705043754097; Thu, 11 Jan 2024 23:15:54 -0800 (PST) MIME-Version: 1.0 References: <17828.1704727587183697949@groups.io> <6131.1704730982570913230@groups.io> In-Reply-To: From: "Dhaval Sharma" Date: Fri, 12 Jan 2024 12:45:42 +0530 Message-ID: Subject: Re: [edk2-devel] [PATCH v10 4/5] MdePkg: Utilize Cache Management Operations Implementation For RISC-V To: "Warkentin, Andrei" Cc: Sunil V L , Pedro Falcato , "devel@edk2.groups.io" , yorange , Ard Biesheuvel , Leif Lindholm Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,dhaval@rivosinc.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: multipart/alternative; boundary="0000000000004a332f060eba6f58" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=coP5Mfxo; dmarc=none; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io --0000000000004a332f060eba6f58 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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=E2=80=AFPM Warkentin, Andrei 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 awar= e > of how the IP is integrated into the SoC. > > A > > > -----Original Message----- > > From: Sunil V L > > Sent: Monday, January 8, 2024 11:32 PM > > To: Pedro Falcato > > Cc: devel@edk2.groups.io; dhaval@rivosinc.com; yorange > > ; Warkentin, Andrei > > ; Ard Biesheuvel >; > > Leif Lindholm > > 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=E2=80=AFPM Dhaval Sharma > > wrote: > > > > > > > > Hi yangcheng/Pedro, > > > > > > +CC a bunch of relevant people > > > > > > Hi, (FYI you did not CC me) > > > > > > Looking at yangcheng's example: > > > > > > Status =3D PrepareDmaData (gpIdmacDesc, Length, Buffer); <-- We wri= te > > > 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 i= n > > > DRAM and DMA-coherent, start DMA > > > > > > which screams of "bad abstractions" because you don't actually need t= o > > > 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 > --=20 Thanks! =3DD -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- 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] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- --0000000000004a332f060eba6f58 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
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=C2=A0this code when it= is known that platform does not support such operations. If there are no o= bjections to this we can go ahead.

On Tue, Jan 9, 2024 at 9:50=E2=80=AFPM Wa= rkentin, Andrei <andrei.warkentin@intel.com> wrote:
For now, this is really something that o= ught to be hidden by DmaLib abstraction (Map/Unmap). This would allow the d= river 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@ed= k2.groups.io; = dhaval@rivosinc.com; yorange
> <ya= ngcheng.work@foxmail.com>; Warkentin, Andrei
> <an= drei.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=E2=80=AFPM Dhaval Sharma <dhaval@rivosinc.com&g= t;
> wrote:
> > >
> > > Hi yangcheng/Pedro,
> >
> > +CC a bunch of relevant people
> >
> > Hi, (FYI you did not CC me)
> >
> > Looking at yangcheng's example:
> >
> >=C2=A0 =C2=A0Status =3D PrepareDmaData (gpIdmacDesc, Length, Buffe= r); <-- We write
> > to the IDMAC desc
> >=C2=A0 =C2=A0if (EFI_ERROR (Status)) {
> >=C2=A0 =C2=A0 =C2=A0goto out;
> >=C2=A0 =C2=A0}
> >
> >=C2=A0 =C2=A0WriteBackDataCacheRange (gpIdmacDesc, DescPages * EFI= _PAGE_SIZE);
> > <-- Make sure it's DMA-coherent
> >=C2=A0 =C2=A0StartDma (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: RIS= C-V
> > Unprivileged ISA V20191213" says, section A.5:
> >
> > "Table A.5 provides a mapping of Linux memory ordering macro= s onto
> > RISC-V memory instructions.
> > The Linux fences dma rmb() and dma wmb() map onto FENCE R,R and F= ENCE
> > W,W, respectively, since the RISC-V Unix Platform requires cohere= nt
> > 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 ca= n be
> > flushed and/or invalidated.
> > Such mechanisms will be device-specific and/or standardized in a<= br> > > future extension to the ISA."
> >
> > The (current date) RISCV Platform Spec also says: "Memory ac= cesses 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() els= e
> > 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!
=3DD
_._,_._,_
Groups.io Links:

=20 You receive all messages sent to this group. =20 =20

View/Reply Online (#113652) | =20 | Mute= This Topic | New Topic
Your Subscriptio= n | Contact Group Owner | Unsubscribe [rebecca@openfw.io]

_._,_._,_
--0000000000004a332f060eba6f58--