Re: [edk2-devel] [PATCH v1 1/1] MdePkg/BaseCacheMaintenanceLib: RV64 replace asserts with logs
On Wed, Jan 24, 2024 at 07:57:27PM +0530, Dhaval Sharma wrote: > "The CpuDxe interface will be the wrapper." Yes, of course. It needs to be > added. I was just saying that maybe any CMO checking is not required there > as cmo library will take care of it. > That's correct. > On Tue, Jan 23, 2024 at 10:24 PM Sunil V L wrote: > > > On Tue, Jan 23, 2024 at 11:42:57AM +0530, Dhaval Sharma wrote: > > > Sunil, > > > I thought "WriteBackDataCacheRange not supported" is more explicit over > > > "CMO not available". > > > > > Okay. > > > > > @Pedro Falcato 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. > > > > > The CpuDxe interface will be the wrapper. See Arm's implementation. > > Since CMO support is added now, the CpuDxe interface should be updated. > > > > Thanks, > > Sunil > > > > > -- > Thanks! > =D -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#114303): https://edk2.groups.io/g/devel/message/114303 Mute This Topic: https://groups.io/mt/103805230/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 1/1] MdePkg/BaseCacheMaintenanceLib: RV64 replace asserts with logs
"The CpuDxe interface will be the wrapper." Yes, of course. It needs to be added. I was just saying that maybe any CMO checking is not required there as cmo library will take care of it. On Tue, Jan 23, 2024 at 10:24 PM Sunil V L wrote: > On Tue, Jan 23, 2024 at 11:42:57AM +0530, Dhaval Sharma wrote: > > Sunil, > > I thought "WriteBackDataCacheRange not supported" is more explicit over > > "CMO not available". > > > Okay. > > > @Pedro Falcato 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. > > > The CpuDxe interface will be the wrapper. See Arm's implementation. > Since CMO support is added now, the CpuDxe interface should be updated. > > Thanks, > Sunil > -- Thanks! =D -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#114302): https://edk2.groups.io/g/devel/message/114302 Mute This Topic: https://groups.io/mt/103805230/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 1/1] MdePkg/BaseCacheMaintenanceLib: RV64 replace asserts with logs
On Tue, Jan 23, 2024 at 11:42:57AM +0530, Dhaval Sharma wrote: > Sunil, > I thought "WriteBackDataCacheRange not supported" is more explicit over > "CMO not available". > Okay. > @Pedro Falcato 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. > The CpuDxe interface will be the wrapper. See Arm's implementation. Since CMO support is added now, the CpuDxe interface should be updated. Thanks, Sunil -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#114218): https://edk2.groups.io/g/devel/message/114218 Mute This Topic: https://groups.io/mt/103805230/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 1/1] MdePkg/BaseCacheMaintenanceLib: RV64 replace asserts with logs
On Tue, Jan 23, 2024 at 6:13 AM Dhaval Sharma wrote: > > Sunil, > I thought "WriteBackDataCacheRange not supported" is more explicit over "CMO > not available". > > @Pedro Falcato 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? Correct. And simply logging breaks the interface's contract by *not* doing what is specified in the library's interface. -- Pedro -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#114195): https://edk2.groups.io/g/devel/message/114195 Mute This Topic: https://groups.io/mt/103805230/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 1/1] MdePkg/BaseCacheMaintenanceLib: RV64 replace asserts with logs
Sunil,
I thought "WriteBackDataCacheRange not supported" is more explicit over
"CMO not available".
@Pedro Falcato 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 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
> > Cc: Liming Gao
> > Cc: Michael D Kinney
> > Cc: Zhiguang Liu
> > Cc: Sunil V L
> > Cc: Andrei Warkentin
> > Cc: Laszlo Ersek
> > Cc: Pedro Falcato
> > Cc: Yang Cheng
> > ---
> > 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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 1/1] MdePkg/BaseCacheMaintenanceLib: RV64 replace asserts with logs
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
> Cc: Liming Gao
> Cc: Michael D Kinney
> Cc: Zhiguang Liu
> Cc: Sunil V L
> Cc: Andrei Warkentin
> Cc: Laszlo Ersek
> Cc: Pedro Falcato
> Cc: Yang Cheng
> ---
> 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
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114027): https://edk2.groups.io/g/devel/message/114027
Mute This Topic: https://groups.io/mt/103805230/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 1/1] MdePkg/BaseCacheMaintenanceLib: RV64 replace asserts with logs
On Thu, Jan 18, 2024 at 03:58:04PM +, Pedro Falcato wrote: > On Thu, Jan 18, 2024 at 9:50 AM 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 > > 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/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 1/1] MdePkg/BaseCacheMaintenanceLib: RV64 replace asserts with logs
What's very confusing about the current situation is that we have a Pcd that can set whether I support CMO instructions. When I correctly set up my platform to not support CMO instructions and hope that everything goes well, I will trigger Assert in the debug version. But I also can't set the Pcd to support CMO instructions because that would cause me to trigger an illegal instruction exception. If we decide that this library can only be used by RISCV platforms that support CMO instructions, then we should use some stronger prompts than ASSERT, such as reporting an error at compile time so that developers can find other library alternatives. After all, ASSERT is being released version will lose its functionality which may lead to some unexpected errors. If we decide that this library can be used by platforms that do not support CMO instructions as before, then we should still use logs instead of ASSERT, otherwise it is meaningless to set whether CMO instructions are supported in Pcd and check Pcd at runtime. Yang Cheng -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#114000): https://edk2.groups.io/g/devel/message/114000 Mute This Topic: https://groups.io/mt/103805230/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 1/1] MdePkg/BaseCacheMaintenanceLib: RV64 replace asserts with logs
Hi Pedro, Agree Assert is slightly more enforcing over logs, but you could still get away with even Assert in release mode. One alternative is to convert VERBOSE into WARNING? =D On Thu, Jan 18, 2024 at 9:28 PM Pedro Falcato wrote: > On Thu, Jan 18, 2024 at 9:50 AM 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 > > 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. > > > a harmless logger message. Eventually platform/drivers need to have > > better guard for such functionality. > > Like an ASSERT? :) > > -- > Pedro > -- Thanks! =D -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113999): https://edk2.groups.io/g/devel/message/113999 Mute This Topic: https://groups.io/mt/103805230/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 1/1] MdePkg/BaseCacheMaintenanceLib: RV64 replace asserts with logs
On Thu, Jan 18, 2024 at 9:50 AM 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 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. > a harmless logger message. Eventually platform/drivers need to have > better guard for such functionality. Like an ASSERT? :) -- Pedro -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113997): https://edk2.groups.io/g/devel/message/113997 Mute This Topic: https://groups.io/mt/103805230/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 1/1] MdePkg/BaseCacheMaintenanceLib: RV64 replace asserts with logs
On 1/18/24 10:50, 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
> Cc: Liming Gao
> Cc: Michael D Kinney
> Cc: Zhiguang Liu
> Cc: Sunil V L
> Cc: Andrei Warkentin
> Cc: Laszlo Ersek
> Cc: Pedro Falcato
> Cc: Yang Cheng
> ---
> 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")
> + );
>}
>
>return Address;
This formatting looks very strange, normally you'd write
DEBUG ((
));
LGTM otherwise:
Acked-by: Laszlo Ersek
> @@ -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"));
>}
>
>return Address;
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113993): https://edk2.groups.io/g/devel/message/113993
Mute This Topic: https://groups.io/mt/103805230/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v1 1/1] MdePkg/BaseCacheMaintenanceLib: RV64 replace asserts with logs
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
Cc: Liming Gao
Cc: Michael D Kinney
Cc: Zhiguang Liu
Cc: Sunil V L
Cc: Andrei Warkentin
Cc: Laszlo Ersek
Cc: Pedro Falcato
Cc: Yang Cheng
---
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")
+ );
}
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"));
}
return Address;
--
2.39.2
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113990): https://edk2.groups.io/g/devel/message/113990
Mute This Topic: https://groups.io/mt/103805230/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
