Re: [edk2-devel] [PATCH v1 1/1] MdePkg/BaseCacheMaintenanceLib: RV64 replace asserts with logs

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

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

Sunil,
I thought "WriteBackDataCacheRange not supported" is more explicit over
"CMO not available".

@Pedro Falcato <pedro.falcato@gmail.com> For the example you mentioned, is
your concern more about someone not being able to notice the problem (that
the system is non-coherent) at the time of development and later ending up
with corrupted data during production? And you are suggesting that an
Assert helps address that problem by making that problem more visible to
the developer and a verbose warning does not?

I can create a patch for CpuFlushCpuDataCache but I think we should avoid
CMO based return in there. Because in case of InvalidateDataCacheRange we
have an alternate implementation of fence in the absence of CMO. So it is
better to let riscvcache decide the right implementation.

=D


On Fri, Jan 19, 2024 at 11:06 AM Sunil V L <sunilvl@ventanamicro.com> wrote:

> On Thu, Jan 18, 2024 at 03:20:18PM +0530, Dhaval wrote:
> > Some platforms do not implement cache management operations. Especially
> > for DMA drivers have code to manage data cache. The code seem to depend
> > on the underlying CPU/cache drivers to enact functionality and simply
> > return if such functionality is not implemented. However this causes
> > issue with CMO implementation which has an assert causing flow to
> > hang within debug environment. While it is not an issue in production
> > environment there is a recommendation to conver this assert in to
> > a harmless logger message. Eventually platform/drivers need to have
> > better guard for such functionality.
> >
> > Signed-off-by: Dhaval Sharma <dhaval@rivosinc.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> > Cc: Sunil V L <sunilvl@ventanamicro.com>
> > Cc: Andrei Warkentin <andrei.warkentin@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Pedro Falcato <pedro.falcato@gmail.com>
> > Cc: Yang Cheng <yangcheng.work@foxmail.com>
> > ---
> >  MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c | 11 ++++++-----
> >  1 file changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c
> b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c
> > index 73a5a6b6b5d6..d99515bcf38b 100644
> > --- a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c
> > +++ b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c
> > @@ -183,9 +183,8 @@ WriteBackInvalidateDataCache (
> >    VOID
> >    )
> >  {
> > -  ASSERT (FALSE);
> >    DEBUG ((
> > -    DEBUG_ERROR,
> > +    DEBUG_VERBOSE,
> >      "WriteBackInvalidateDataCache: RISC-V unsupported function.\n"
> >      ));
> >  }
> > @@ -226,7 +225,9 @@ WriteBackInvalidateDataCacheRange (
> >    if (RiscVIsCMOEnabled ()) {
> >      CacheOpCacheRange (Address, Length, CacheOpFlush);
> >    } else {
> > -    ASSERT (FALSE);
> > +    DEBUG (
> > +      (DEBUG_VERBOSE, "WriteBackInvalidateDataCacheRange not supported
> \n")
>
> Should this be CMO not enabled?
>
> > +      );
> >    }
> >
> >    return Address;
> > @@ -248,7 +249,7 @@ WriteBackDataCache (
> >    VOID
> >    )
> >  {
> > -  ASSERT (FALSE);
> > +  DEBUG ((DEBUG_VERBOSE, "WriteBackDataCache not supported \n"));
> >  }
> >
> >  /**
> > @@ -283,7 +284,7 @@ WriteBackDataCacheRange (
> >    if (RiscVIsCMOEnabled ()) {
> >      CacheOpCacheRange (Address, Length, CacheOpClean);
> >    } else {
> > -    ASSERT (FALSE);
> > +    DEBUG ((DEBUG_VERBOSE, "WriteBackDataCacheRange not supported \n"));
> Same comment as earlier.
>
> >    }
> >
> >    return Address;
> > --
> > 2.39.2
> >
>


-- 
Thanks!
=D


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114179): https://edk2.groups.io/g/devel/message/114179
Mute This Topic: https://groups.io/mt/103805230/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: 6543 bytes --]