Re: [PATCH] UefiCpuPkg RegisterCpuFeaturesLib: Use %08x to print CacheControl Index


Zeng, Star
 

-----Original Message-----
From: Laszlo Ersek <lersek@...>
Sent: Monday, February 3, 2020 4:47 PM
To: Zeng, Star <star.zeng@...>; devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@...>; Ni, Ray <ray.ni@...>
Subject: Re: [PATCH] UefiCpuPkg RegisterCpuFeaturesLib: Use %08x to print
CacheControl Index

Hello Star,

On 02/03/20 08:06, Star Zeng wrote:
Instead of %08lx, use %08x to print CacheControl Index as it is UINT32
type.

Cc: Eric Dong <eric.dong@...>
Cc: Ray Ni <ray.ni@...>
Cc: Laszlo Ersek <lersek@...>
Signed-off-by: Star Zeng <star.zeng@...>
---
.../Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git
a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
index 0a4fcff033a3..1a02809b0e7c 100644
---
a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.
+++ c
@@ -465,7 +465,7 @@ DumpRegisterTableOnProcessor (
case CacheControl:
DEBUG ((
DebugPrintErrorLevel,
- "Processor: %04d: Index %04d, CACHE: %08lx, Bit Start: %02d,
Bit Length: %02d, Value: %016lx\r\n",
+ "Processor: %04d: Index %04d, CACHE: %08x, Bit Start: %02d,
+ Bit Length: %02d, Value: %016lx\r\n",
ProcessorNumber,
FeatureIndex,
RegisterTableEntry->Index,
if you are already touching this DEBUG invocation, can you please fix the
rest of the issues with the format string?

- ProcessorNumber is UINTN. If we know for sure it can be represented in a
UINT32, then it should be cast to UINT32 explicitly, and logged with "%04u".
(Otherwise, UINTN needs to be cast to UINT64, and logged with %lu or %lx.)

- Ditto for FeatureIndex.
%04u or %04d is not enough for UINT32 which needs %08x.
I thought the code is just taking assumption about their value should be not > 9999u. It is not a real issue.

This patch is to fix a real issue, without it, the print for ValidBitStart, ValidBitLength and Value will be wrong because the parameter for them are shifted for Index to fetch UINT64 value.

I found another real issue is MMIO : %08lx should be MMIO : %016lx as the code is on purpose to get UINT64 MMIO address.

I prefer to just handle the real issue in this patch. How do you think? 😊


Thanks,
Star


The rest of the format specifications (including the now-fixed
CPU_REGISTER_TABLE_ENTRY.Index) are OK.

Thanks
Laszlo

Join devel@edk2.groups.io to automatically receive all group messages.