> -----Original Message----- > From: Ming Huang <huangm...@linux.alibaba.com> > Sent: Tuesday, August 13, 2024 7:05 PM > To: Kinney, Michael D <michael.d.kin...@intel.com>; devel@edk2.groups.io; > gaolim...@byosoft.com.cn; Liu, Zhiguang <zhiguang....@intel.com> > Cc: ming.hua...@outlook.com > Subject: Re: [PATCH v1 v1 1/1] MdePkg: Add error output for IoLib.c > > > > On 8/13/24 11:29 PM, Kinney, Michael D wrote: > > Hi, > > > > We have moved to a PR based review process. Can you change this to a > PR? > > OK, I will change this to a PR. > > > > > https://github.com/tianocore/tianocore.github.io/wiki/EDK-II- > Development-Process > > > > Also, this change may impact the code size and performance of these > APIs. > > When ASSERTS() are disabled, an optimizing compiler can inline these > operations. > > By adding conditional and DEBUG(), that optimization may only occur if > both > > ASSERT() and DEBUG_ERROR messages are disabled and the optimization > compiler > > can optimize away the conditional statement as well. > > > > Another option is to use a different DEBUG_X debug log level that is > not normally > > enabled and a platform can choose to enable. > > Are you referring to using DEBUG_WARN/DEBUG_INFO ? Which is better for > DEBUG_WARN/DEBUG_INFO?
Perhaps DEBUG_VERBOSE. > > > > > The DEBUG_CODE() macros can also be used to conditionally include the > conditional > > and DEBUG() message. > > > > This patch is also incomplete because there are other APIs in the IoLib > that > > ASSERT() on unaligned access. > > I will modify other ASSERT() in IoLib except ASSERT (FALSE). > > Thanks, > > Ming > > > > > Thanks, > > > > Mike > > > >> -----Original Message----- > >> From: Ming Huang <huangm...@linux.alibaba.com> > >> Sent: Tuesday, August 13, 2024 2:42 AM > >> To: devel@edk2.groups.io; gaolim...@byosoft.com.cn; Kinney, Michael D > >> <michael.d.kin...@intel.com>; Liu, Zhiguang <zhiguang....@intel.com> > >> Cc: ming.hua...@outlook.com; Ming Huang <huangm...@linux.alibaba.com> > >> Subject: [PATCH v1 v1 1/1] MdePkg: Add error output for IoLib.c > >> > >> It is better to output error address information When Address is > >> invalid before ASSERT. > >> > >> Signed-off-by: Ming Huang <huangm...@linux.alibaba.com> > >> --- > >> MdePkg/Library/BaseIoLibIntrinsic/IoLib.c | 24 ++++++++++++++++++++ > >> 1 file changed, 24 insertions(+) > >> > >> diff --git a/MdePkg/Library/BaseIoLibIntrinsic/IoLib.c > >> b/MdePkg/Library/BaseIoLibIntrinsic/IoLib.c > >> index 5bd02b56a1..57d05af5a9 100644 > >> --- a/MdePkg/Library/BaseIoLibIntrinsic/IoLib.c > >> +++ b/MdePkg/Library/BaseIoLibIntrinsic/IoLib.c > >> @@ -174,6 +174,10 @@ MmioRead16 ( > >> UINT16 Value; > >> BOOLEAN Flag; > >> > >> + if ((Address & 1) != 0) { > >> + DEBUG ((DEBUG_ERROR, "MmioRead16 invalid address: %lx\n", > Address)); > >> + } > >> + > >> ASSERT ((Address & 1) == 0); > >> Flag = FilterBeforeMmIoRead (FilterWidth16, Address, &Value); > >> if (Flag) { > >> @@ -220,6 +224,10 @@ MmioWrite16 ( > >> { > >> BOOLEAN Flag; > >> > >> + if ((Address & 1) != 0) { > >> + DEBUG ((DEBUG_ERROR, "MmioWrite16 invalid address: %lx\n", > Address)); > >> + } > >> + > >> ASSERT ((Address & 1) == 0); > >> > >> Flag = FilterBeforeMmIoWrite (FilterWidth16, Address, &Value); > >> @@ -266,6 +274,10 @@ MmioRead32 ( > >> UINT32 Value; > >> BOOLEAN Flag; > >> > >> + if ((Address & 3) != 0) { > >> + DEBUG ((DEBUG_ERROR, "MmioRead32 invalid address: %lx\n", > Address)); > >> + } > >> + > >> ASSERT ((Address & 3) == 0); > >> > >> Flag = FilterBeforeMmIoRead (FilterWidth32, Address, &Value); > >> @@ -313,6 +325,10 @@ MmioWrite32 ( > >> { > >> BOOLEAN Flag; > >> > >> + if ((Address & 3) != 0) { > >> + DEBUG ((DEBUG_ERROR, "MmioWrite32 invalid address: %lx\n", > Address)); > >> + } > >> + > >> ASSERT ((Address & 3) == 0); > >> > >> Flag = FilterBeforeMmIoWrite (FilterWidth32, Address, &Value); > >> @@ -359,6 +375,10 @@ MmioRead64 ( > >> UINT64 Value; > >> BOOLEAN Flag; > >> > >> + if ((Address & 7) != 0) { > >> + DEBUG ((DEBUG_ERROR, "MmioRead64 invalid address: %lx\n", > Address)); > >> + } > >> + > >> ASSERT ((Address & 7) == 0); > >> > >> Flag = FilterBeforeMmIoRead (FilterWidth64, Address, &Value); > >> @@ -404,6 +424,10 @@ MmioWrite64 ( > >> { > >> BOOLEAN Flag; > >> > >> + if ((Address & 7) != 0) { > >> + DEBUG ((DEBUG_ERROR, "MmioWrite64 invalid address: %lx\n", > Address)); > >> + } > >> + > >> ASSERT ((Address & 7) == 0); > >> > >> Flag = FilterBeforeMmIoWrite (FilterWidth64, Address, &Value); > >> -- > >> 2.17.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#120336): https://edk2.groups.io/g/devel/message/120336 Mute This Topic: https://groups.io/mt/107873366/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-