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

["Sunil V L" <sunilvl@ventanamicro.com>]

On Thu, Jan 18, 2024 at 03:58:04PM +0000, Pedro Falcato wrote:
> On Thu, Jan 18, 2024 at 9:50 AM Dhaval <dhaval@rivosinc.com> 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
> 
> I don't agree with this patch. As I see it, the library has a simple
> contract: Do cache operation X and return. We cannot safely return if
> we don't know how to do cache operation X. Say, with a Thead core and
> Xtheadcmo.
> Any other concerns wrt DMA are, in my view, somewhat separate.
> 
> One can easily theorize a way this change can come to bite us, say, a
> storage controller writes bogus data to storage (because the platform
> needs explicit cache coherency, and we don't know how to do that) and
> causes data corruption.
> 
I understand your point. It is an issue for sure if the device is
non cache coherent but there is no way to flush the cache. However, if
CMO is not there, the driver should use NonCacheCoherent library which
uses UC memory. These interfaces will be called for UC memory in that case
which should be NO-OP instead of assertion.

For custom CMO extension, the silicon vendor needs to implement
different cache management library in edk2-platforms.

Having said that, I notice that the interface in CpuDxe needs to be
updated now. Dhaval, would you be able to add that patch also?

Thanks,
Sunil

> > a harmless logger message. Eventually platform/drivers need to have
> > better guard for such functionality.
> 
> Like an ASSERT? :)
> 


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