* [Patch v3 0/6] Add "test then write" mechanism
@ 2019-08-15 2:50 Dong, Eric
2019-08-15 2:50 ` [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test Then Write" Macros Dong, Eric
` (5 more replies)
0 siblings, 6 replies; 13+ messages in thread
From: Dong, Eric @ 2019-08-15 2:50 UTC (permalink / raw)
To: devel; +Cc: Ray Ni, Laszlo Ersek
v3 changes:
1. Avoid changing exist API CpuRegisterTableWrite, add new API CpuRegisterTableTestThenWrite which align new adds macros.
Only 1/6 patch been changed in v3.
V2 changes:
1. Split CR read/write action in to one discrete patch 2. Keep the old logic which continue the process if error found.
Below code is current implementation:
if (MsrRegister[ProcessorNumber].Bits.Lock == 0) {
CPU_REGISTER_TABLE_WRITE_FIELD (
ProcessorNumber,
Msr,
MSR_IA32_FEATURE_CONTROL,
MSR_IA32_FEATURE_CONTROL_REGISTER,
Bits.Lock,
1
);
}
With below steps, the Bits.Lock bit will lose its value:
1. Trig normal boot, the Bits.Lock is 0. 1 will be added
into the register table and then will set to the MSR.
2. Trig warm reboot, MSR value preserves. After normal boot phase,
the Bits.Lock is 1, so it will not be added into the register
table during the warm reboot phase.
3. Trig S3 then resume, the Bits.Lock change to 0 and Bits.Lock is
not added in register table during normal boot phase. so it's
still 0 after resume.
This is not an expect behavior. The expect result is the value should always 1 after booting or resuming from S3.
The root cause for this issue is
1. driver bases on current value to insert the "set value action" to
the register table.
2. Some MSRs may reserve their value during warm reboot. So the insert
action may be skip after warm reboot.
The solution for this issue is:
1. Always add "Test then Set" action for above referred MSRs.
2. Detect current value before set new value. Only set new value when
current value not same as new value.
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Eric Dong (6):
UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test Then Write" Macros.
UefiCpuPkg/PiSmmCpuDxeSmm: Combine CR read/write action.
UefiCpuPkg/PiSmmCpuDxeSmm: Supports test then write new value logic.
UefiCpuPkg/RegisterCpuFeaturesLib: Combine CR read/write action.
UefiCpuPkg/RegisterCpuFeaturesLib: Supports test then write new value
logic.
UefiCpuPkg/CpuCommonFeaturesLib: Use new macros.
UefiCpuPkg/Include/AcpiCpuData.h | 1 +
.../Include/Library/RegisterCpuFeaturesLib.h | 91 +++++++++++
.../CpuCommonFeaturesLib/CpuCommonFeatures.h | 15 --
.../CpuCommonFeaturesLib.c | 8 +-
.../CpuCommonFeaturesLib/FeatureControl.c | 141 ++++++------------
.../CpuCommonFeaturesLib/MachineCheck.c | 23 ++-
.../CpuFeaturesInitialize.c | 139 +++++++++++------
.../RegisterCpuFeaturesLib.c | 45 +++++-
UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 133 +++++++++++------
9 files changed, 375 insertions(+), 221 deletions(-)
--
2.21.0.windows.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test Then Write" Macros.
2019-08-15 2:50 [Patch v3 0/6] Add "test then write" mechanism Dong, Eric
@ 2019-08-15 2:50 ` Dong, Eric
2019-08-16 1:14 ` Zeng, Star
2019-08-15 2:50 ` [Patch v3 2/6] UefiCpuPkg/PiSmmCpuDxeSmm: Combine CR read/write action Dong, Eric
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Dong, Eric @ 2019-08-15 2:50 UTC (permalink / raw)
To: devel; +Cc: Ray Ni, Laszlo Ersek, Star Zeng
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2040
Add below new micros which test the current value before write
the new value. Only write new value when current value not
same as new value.
CPU_REGISTER_TABLE_TEST_THEN_WRITE32
CPU_REGISTER_TABLE_TEST_THEN_WRITE64
CPU_REGISTER_TABLE_TEST_THEN_WRITE_FIELD
Also add below API:
CpuRegisterTableTestThenWrite
Signed-off-by: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Star Zeng <star.zeng@intel.com>
---
UefiCpuPkg/Include/AcpiCpuData.h | 1 +
.../Include/Library/RegisterCpuFeaturesLib.h | 91 +++++++++++++++++++
.../RegisterCpuFeaturesLib.c | 45 ++++++++-
3 files changed, 134 insertions(+), 3 deletions(-)
diff --git a/UefiCpuPkg/Include/AcpiCpuData.h b/UefiCpuPkg/Include/AcpiCpuData.h
index b963a2f592..472a1a8070 100644
--- a/UefiCpuPkg/Include/AcpiCpuData.h
+++ b/UefiCpuPkg/Include/AcpiCpuData.h
@@ -81,6 +81,7 @@ typedef struct {
UINT16 Reserved; // offset 10 - 11
UINT32 HighIndex; // offset 12-15, only valid for MemoryMapped
UINT64 Value; // offset 16-23
+ UINT8 TestThenWrite; // 0ffset 24
} CPU_REGISTER_TABLE_ENTRY;
//
diff --git a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
index e420e7f075..5bd464b32e 100644
--- a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
+++ b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
@@ -348,6 +348,32 @@ CpuRegisterTableWrite (
IN UINT64 Value
);
+/**
+ Adds an entry in specified register table.
+
+ This function adds an entry in specified register table, with given register type,
+ register index, bit section and value.
+
+ Driver will test the current value before setting new value.
+
+ @param[in] ProcessorNumber The index of the CPU to add a register table entry
+ @param[in] RegisterType Type of the register to program
+ @param[in] Index Index of the register to program
+ @param[in] ValueMask Mask of bits in register to write
+ @param[in] Value Value to write
+
+ @note This service could be called by BSP only.
+**/
+VOID
+EFIAPI
+CpuRegisterTableTestThenWrite (
+ IN UINTN ProcessorNumber,
+ IN REGISTER_TYPE RegisterType,
+ IN UINT64 Index,
+ IN UINT64 ValueMask,
+ IN UINT64 Value
+ );
+
/**
Adds an entry in specified Pre-SMM register table.
@@ -390,6 +416,26 @@ PreSmmCpuRegisterTableWrite (
CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index, MAX_UINT32, Value); \
} while(FALSE);
+/**
+ Adds a 32-bit register write entry in specified register table.
+
+ This macro adds an entry in specified register table, with given register type,
+ register index, and value.
+
+ Driver will test the current value before setting new value.
+
+ @param[in] ProcessorNumber The index of the CPU to add a register table entry.
+ @param[in] RegisterType Type of the register to program
+ @param[in] Index Index of the register to program
+ @param[in] Value Value to write
+
+ @note This service could be called by BSP only.
+**/
+#define CPU_REGISTER_TABLE_TEST_THEN_WRITE32(ProcessorNumber, RegisterType, Index, Value) \
+ do { \
+ CpuRegisterTableTestThenWrite (ProcessorNumber, RegisterType, Index, MAX_UINT32, Value); \
+ } while(FALSE);
+
/**
Adds a 64-bit register write entry in specified register table.
@@ -408,6 +454,26 @@ PreSmmCpuRegisterTableWrite (
CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index, MAX_UINT64, Value); \
} while(FALSE);
+/**
+ Adds a 64-bit register write entry in specified register table.
+
+ This macro adds an entry in specified register table, with given register type,
+ register index, and value.
+
+ Driver will test the current value before setting new value.
+
+ @param[in] ProcessorNumber The index of the CPU to add a register table entry.
+ @param[in] RegisterType Type of the register to program
+ @param[in] Index Index of the register to program
+ @param[in] Value Value to write
+
+ @note This service could be called by BSP only.
+**/
+#define CPU_REGISTER_TABLE_TEST_THEN_WRITE64(ProcessorNumber, RegisterType, Index, Value) \
+ do { \
+ CpuRegisterTableTestThenWrite (ProcessorNumber, RegisterType, Index, MAX_UINT64, Value); \
+ } while(FALSE);
+
/**
Adds a bit field write entry in specified register table.
@@ -431,6 +497,31 @@ PreSmmCpuRegisterTableWrite (
CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index, ~ValueMask, Value); \
} while(FALSE);
+/**
+ Adds a bit field write entry in specified register table.
+
+ This macro adds an entry in specified register table, with given register type,
+ register index, bit field section, and value.
+
+ Driver will test the current value before setting new value.
+
+ @param[in] ProcessorNumber The index of the CPU to add a register table entry.
+ @param[in] RegisterType Type of the register to program.
+ @param[in] Index Index of the register to program.
+ @param[in] Type The data type name of a register structure.
+ @param[in] Field The bit fiel name in register structure to write.
+ @param[in] Value Value to write to the bit field.
+
+ @note This service could be called by BSP only.
+**/
+#define CPU_REGISTER_TABLE_TEST_THEN_WRITE_FIELD(ProcessorNumber, RegisterType, Index, Type, Field, Value) \
+ do { \
+ UINT64 ValueMask; \
+ ValueMask = MAX_UINT64; \
+ ((Type *)(&ValueMask))->Field = 0; \
+ CpuRegisterTableTestThenWrite (ProcessorNumber, RegisterType, Index, ~ValueMask, Value); \
+ } while(FALSE);
+
/**
Adds a 32-bit register write entry in specified register table.
diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
index 67885bf69b..e9769882b9 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
@@ -1025,6 +1025,8 @@ EnlargeRegisterTable (
@param[in] ValidBitStart Start of the bit section
@param[in] ValidBitLength Length of the bit section
@param[in] Value Value to write
+ @param[in] TestThenWrite Whether need to test current Value before writing.
+
**/
VOID
CpuRegisterTableWriteWorker (
@@ -1034,7 +1036,8 @@ CpuRegisterTableWriteWorker (
IN UINT64 Index,
IN UINT8 ValidBitStart,
IN UINT8 ValidBitLength,
- IN UINT64 Value
+ IN UINT64 Value,
+ IN UINT8 TestThenWrite
)
{
CPU_FEATURES_DATA *CpuFeaturesData;
@@ -1070,6 +1073,7 @@ CpuRegisterTableWriteWorker (
RegisterTableEntry[RegisterTable->TableLength].ValidBitStart = ValidBitStart;
RegisterTableEntry[RegisterTable->TableLength].ValidBitLength = ValidBitLength;
RegisterTableEntry[RegisterTable->TableLength].Value = Value;
+ RegisterTableEntry[RegisterTable->TableLength].TestThenWrite = TestThenWrite;
RegisterTable->TableLength++;
}
@@ -1105,7 +1109,42 @@ CpuRegisterTableWrite (
Start = (UINT8)LowBitSet64 (ValueMask);
End = (UINT8)HighBitSet64 (ValueMask);
Length = End - Start + 1;
- CpuRegisterTableWriteWorker (FALSE, ProcessorNumber, RegisterType, Index, Start, Length, Value);
+ CpuRegisterTableWriteWorker (FALSE, ProcessorNumber, RegisterType, Index, Start, Length, Value, FALSE);
+}
+
+/**
+ Adds an entry in specified register table.
+
+ This function adds an entry in specified register table, with given register type,
+ register index, bit section and value.
+
+ @param[in] ProcessorNumber The index of the CPU to add a register table entry
+ @param[in] RegisterType Type of the register to program
+ @param[in] Index Index of the register to program
+ @param[in] ValueMask Mask of bits in register to write
+ @param[in] Value Value to write
+ @param[in] TestThenWrite Whether need to test current Value before writing.
+
+ @note This service could be called by BSP only.
+**/
+VOID
+EFIAPI
+CpuRegisterTableTestThenWrite (
+ IN UINTN ProcessorNumber,
+ IN REGISTER_TYPE RegisterType,
+ IN UINT64 Index,
+ IN UINT64 ValueMask,
+ IN UINT64 Value
+ )
+{
+ UINT8 Start;
+ UINT8 End;
+ UINT8 Length;
+
+ Start = (UINT8)LowBitSet64 (ValueMask);
+ End = (UINT8)HighBitSet64 (ValueMask);
+ Length = End - Start + 1;
+ CpuRegisterTableWriteWorker (FALSE, ProcessorNumber, RegisterType, Index, Start, Length, Value, TRUE);
}
/**
@@ -1139,7 +1178,7 @@ PreSmmCpuRegisterTableWrite (
Start = (UINT8)LowBitSet64 (ValueMask);
End = (UINT8)HighBitSet64 (ValueMask);
Length = End - Start + 1;
- CpuRegisterTableWriteWorker (TRUE, ProcessorNumber, RegisterType, Index, Start, Length, Value);
+ CpuRegisterTableWriteWorker (TRUE, ProcessorNumber, RegisterType, Index, Start, Length, Value, FALSE);
}
/**
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Patch v3 2/6] UefiCpuPkg/PiSmmCpuDxeSmm: Combine CR read/write action.
2019-08-15 2:50 [Patch v3 0/6] Add "test then write" mechanism Dong, Eric
2019-08-15 2:50 ` [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test Then Write" Macros Dong, Eric
@ 2019-08-15 2:50 ` Dong, Eric
2019-08-15 2:50 ` [Patch v3 3/6] UefiCpuPkg/PiSmmCpuDxeSmm: Supports test then write new value logic Dong, Eric
` (3 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Dong, Eric @ 2019-08-15 2:50 UTC (permalink / raw)
To: devel; +Cc: Ray Ni, Laszlo Ersek
Signed-off-by: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 104 ++++++++++++++++++------------
1 file changed, 62 insertions(+), 42 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
index d8c6b19ead..627a3b87ac 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
@@ -159,6 +159,58 @@ S3WaitForSemaphore (
) != Value);
}
+/**
+ Read / write CR value.
+
+ @param[in] CrIndex The CR index which need to read/write.
+ @param[in] Read Read or write. TRUE is read.
+ @param[in,out] CrValue CR value.
+
+ @retval EFI_SUCCESS means read/write success, else return EFI_UNSUPPORTED.
+**/
+UINTN
+ReadWriteCr (
+ IN UINT32 CrIndex,
+ IN BOOLEAN Read,
+ IN OUT UINTN *CrValue
+ )
+{
+ switch (CrIndex) {
+ case 0:
+ if (Read) {
+ *CrValue = AsmReadCr0 ();
+ } else {
+ AsmWriteCr0 (*CrValue);
+ }
+ break;
+ case 2:
+ if (Read) {
+ *CrValue = AsmReadCr2 ();
+ } else {
+ AsmWriteCr2 (*CrValue);
+ }
+ break;
+ case 3:
+ if (Read) {
+ *CrValue = AsmReadCr3 ();
+ } else {
+ AsmWriteCr3 (*CrValue);
+ }
+ break;
+ case 4:
+ if (Read) {
+ *CrValue = AsmReadCr4 ();
+ } else {
+ AsmWriteCr4 (*CrValue);
+ }
+ break;
+ default:
+ return EFI_UNSUPPORTED;;
+ }
+
+ return EFI_SUCCESS;
+}
+
/**
Initialize the CPU registers from a register table.
@@ -188,6 +240,7 @@ ProgramProcessorRegister (
UINTN ProcessorIndex;
UINTN ValidThreadCount;
UINT32 *ValidCoreCountPerPackage;
+ EFI_STATUS Status;
//
// Traverse Register Table of this logical processor
@@ -206,50 +259,17 @@ ProgramProcessorRegister (
// The specified register is Control Register
//
case ControlRegister:
- switch (RegisterTableEntry->Index) {
- case 0:
- Value = AsmReadCr0 ();
- Value = (UINTN) BitFieldWrite64 (
- Value,
- RegisterTableEntry->ValidBitStart,
- RegisterTableEntry->ValidBitStart + RegisterTableEntry->ValidBitLength - 1,
- (UINTN) RegisterTableEntry->Value
- );
- AsmWriteCr0 (Value);
- break;
- case 2:
- Value = AsmReadCr2 ();
- Value = (UINTN) BitFieldWrite64 (
- Value,
- RegisterTableEntry->ValidBitStart,
- RegisterTableEntry->ValidBitStart + RegisterTableEntry->ValidBitLength - 1,
- (UINTN) RegisterTableEntry->Value
- );
- AsmWriteCr2 (Value);
- break;
- case 3:
- Value = AsmReadCr3 ();
- Value = (UINTN) BitFieldWrite64 (
- Value,
- RegisterTableEntry->ValidBitStart,
- RegisterTableEntry->ValidBitStart + RegisterTableEntry->ValidBitLength - 1,
- (UINTN) RegisterTableEntry->Value
- );
- AsmWriteCr3 (Value);
- break;
- case 4:
- Value = AsmReadCr4 ();
- Value = (UINTN) BitFieldWrite64 (
- Value,
- RegisterTableEntry->ValidBitStart,
- RegisterTableEntry->ValidBitStart + RegisterTableEntry->ValidBitLength - 1,
- (UINTN) RegisterTableEntry->Value
- );
- AsmWriteCr4 (Value);
- break;
- default:
+ Status = ReadWriteCr (RegisterTableEntry->Index, TRUE, &Value);
+ if (EFI_ERROR (Status)) {
break;
}
+ Value = (UINTN) BitFieldWrite64 (
+ Value,
+ RegisterTableEntry->ValidBitStart,
+ RegisterTableEntry->ValidBitStart + RegisterTableEntry->ValidBitLength - 1,
+ RegisterTableEntry->Value
+ );
+ ReadWriteCr (RegisterTableEntry->Index, FALSE, &Value);
break;
//
// The specified register is Model Specific Register
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Patch v3 3/6] UefiCpuPkg/PiSmmCpuDxeSmm: Supports test then write new value logic.
2019-08-15 2:50 [Patch v3 0/6] Add "test then write" mechanism Dong, Eric
2019-08-15 2:50 ` [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test Then Write" Macros Dong, Eric
2019-08-15 2:50 ` [Patch v3 2/6] UefiCpuPkg/PiSmmCpuDxeSmm: Combine CR read/write action Dong, Eric
@ 2019-08-15 2:50 ` Dong, Eric
2019-08-15 2:50 ` [Patch v3 4/6] UefiCpuPkg/RegisterCpuFeaturesLib: Combine CR read/write action Dong, Eric
` (2 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Dong, Eric @ 2019-08-15 2:50 UTC (permalink / raw)
To: devel; +Cc: Ray Ni, Laszlo Ersek
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2040
Supports new logic which test current value before write new value.
Only write new value when current value not same as new value.
Signed-off-by: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
index 627a3b87ac..ba5cc0194c 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
@@ -241,6 +241,7 @@ ProgramProcessorRegister (
UINTN ValidThreadCount;
UINT32 *ValidCoreCountPerPackage;
EFI_STATUS Status;
+ UINT64 CurrentValue;
//
// Traverse Register Table of this logical processor
@@ -263,6 +264,16 @@ ProgramProcessorRegister (
if (EFI_ERROR (Status)) {
break;
}
+ if (RegisterTableEntry->TestThenWrite) {
+ CurrentValue = BitFieldRead64 (
+ Value,
+ RegisterTableEntry->ValidBitStart,
+ RegisterTableEntry->ValidBitStart + RegisterTableEntry->ValidBitLength - 1
+ );
+ if (CurrentValue == RegisterTableEntry->Value) {
+ break;
+ }
+ }
Value = (UINTN) BitFieldWrite64 (
Value,
RegisterTableEntry->ValidBitStart,
@@ -275,6 +286,24 @@ ProgramProcessorRegister (
// The specified register is Model Specific Register
//
case Msr:
+ if (RegisterTableEntry->TestThenWrite) {
+ Value = (UINTN)AsmReadMsr64 (RegisterTableEntry->Index);
+ if (RegisterTableEntry->ValidBitLength >= 64) {
+ if (Value == RegisterTableEntry->Value) {
+ break;
+ }
+ } else {
+ CurrentValue = BitFieldRead64 (
+ Value,
+ RegisterTableEntry->ValidBitStart,
+ RegisterTableEntry->ValidBitStart + RegisterTableEntry->ValidBitLength - 1
+ );
+ if (CurrentValue == RegisterTableEntry->Value) {
+ break;
+ }
+ }
+ }
+
//
// If this function is called to restore register setting after INIT signal,
// there is no need to restore MSRs in register table.
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Patch v3 4/6] UefiCpuPkg/RegisterCpuFeaturesLib: Combine CR read/write action.
2019-08-15 2:50 [Patch v3 0/6] Add "test then write" mechanism Dong, Eric
` (2 preceding siblings ...)
2019-08-15 2:50 ` [Patch v3 3/6] UefiCpuPkg/PiSmmCpuDxeSmm: Supports test then write new value logic Dong, Eric
@ 2019-08-15 2:50 ` Dong, Eric
2019-08-15 2:50 ` [Patch v3 5/6] UefiCpuPkg/RegisterCpuFeaturesLib: Supports test then write new value logic Dong, Eric
2019-08-15 2:50 ` [Patch v3 6/6] UefiCpuPkg/CpuCommonFeaturesLib: Use new macros Dong, Eric
5 siblings, 0 replies; 13+ messages in thread
From: Dong, Eric @ 2019-08-15 2:50 UTC (permalink / raw)
To: devel; +Cc: Ray Ni, Laszlo Ersek
Signed-off-by: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
---
.../CpuFeaturesInitialize.c | 110 ++++++++++--------
1 file changed, 63 insertions(+), 47 deletions(-)
diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
index fb0535edd6..63bc50a55f 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
@@ -744,6 +744,58 @@ LibWaitForSemaphore (
) != Value);
}
+/**
+ Read / write CR value.
+
+ @param[in] CrIndex The CR index which need to read/write.
+ @param[in] Read Read or write. TRUE is read.
+ @param[in,out] CrValue CR value.
+
+ @retval EFI_SUCCESS means read/write success, else return EFI_UNSUPPORTED.
+**/
+UINTN
+ReadWriteCr (
+ IN UINT32 CrIndex,
+ IN BOOLEAN Read,
+ IN OUT UINTN *CrValue
+ )
+{
+ switch (CrIndex) {
+ case 0:
+ if (Read) {
+ *CrValue = AsmReadCr0 ();
+ } else {
+ AsmWriteCr0 (*CrValue);
+ }
+ break;
+ case 2:
+ if (Read) {
+ *CrValue = AsmReadCr2 ();
+ } else {
+ AsmWriteCr2 (*CrValue);
+ }
+ break;
+ case 3:
+ if (Read) {
+ *CrValue = AsmReadCr3 ();
+ } else {
+ AsmWriteCr3 (*CrValue);
+ }
+ break;
+ case 4:
+ if (Read) {
+ *CrValue = AsmReadCr4 ();
+ } else {
+ AsmWriteCr4 (*CrValue);
+ }
+ break;
+ default:
+ return EFI_UNSUPPORTED;;
+ }
+
+ return EFI_SUCCESS;
+}
+
/**
Initialize the CPU registers from a register table.
@@ -773,6 +825,7 @@ ProgramProcessorRegister (
UINTN ProcessorIndex;
UINTN ValidThreadCount;
UINT32 *ValidCoreCountPerPackage;
+ EFI_STATUS Status;
//
// Traverse Register Table of this logical processor
@@ -791,55 +844,18 @@ ProgramProcessorRegister (
// The specified register is Control Register
//
case ControlRegister:
- switch (RegisterTableEntry->Index) {
- case 0:
- Value = AsmReadCr0 ();
- Value = (UINTN) BitFieldWrite64 (
- Value,
- RegisterTableEntry->ValidBitStart,
- RegisterTableEntry->ValidBitStart + RegisterTableEntry->ValidBitLength - 1,
- RegisterTableEntry->Value
- );
- AsmWriteCr0 (Value);
- break;
- case 2:
- Value = AsmReadCr2 ();
- Value = (UINTN) BitFieldWrite64 (
- Value,
- RegisterTableEntry->ValidBitStart,
- RegisterTableEntry->ValidBitStart + RegisterTableEntry->ValidBitLength - 1,
- RegisterTableEntry->Value
- );
- AsmWriteCr2 (Value);
- break;
- case 3:
- Value = AsmReadCr3 ();
- Value = (UINTN) BitFieldWrite64 (
- Value,
- RegisterTableEntry->ValidBitStart,
- RegisterTableEntry->ValidBitStart + RegisterTableEntry->ValidBitLength - 1,
- RegisterTableEntry->Value
- );
- AsmWriteCr3 (Value);
- break;
- case 4:
- Value = AsmReadCr4 ();
- Value = (UINTN) BitFieldWrite64 (
- Value,
- RegisterTableEntry->ValidBitStart,
- RegisterTableEntry->ValidBitStart + RegisterTableEntry->ValidBitLength - 1,
- RegisterTableEntry->Value
- );
- AsmWriteCr4 (Value);
- break;
- case 8:
- //
- // Do we need to support CR8?
- //
- break;
- default:
+ Status = ReadWriteCr (RegisterTableEntry->Index, TRUE, &Value);
+ if (EFI_ERROR (Status)) {
break;
}
+
+ Value = (UINTN) BitFieldWrite64 (
+ Value,
+ RegisterTableEntry->ValidBitStart,
+ RegisterTableEntry->ValidBitStart + RegisterTableEntry->ValidBitLength - 1,
+ RegisterTableEntry->Value
+ );
+ ReadWriteCr (RegisterTableEntry->Index, FALSE, &Value);
break;
//
// The specified register is Model Specific Register
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Patch v3 5/6] UefiCpuPkg/RegisterCpuFeaturesLib: Supports test then write new value logic.
2019-08-15 2:50 [Patch v3 0/6] Add "test then write" mechanism Dong, Eric
` (3 preceding siblings ...)
2019-08-15 2:50 ` [Patch v3 4/6] UefiCpuPkg/RegisterCpuFeaturesLib: Combine CR read/write action Dong, Eric
@ 2019-08-15 2:50 ` Dong, Eric
2019-08-15 2:50 ` [Patch v3 6/6] UefiCpuPkg/CpuCommonFeaturesLib: Use new macros Dong, Eric
5 siblings, 0 replies; 13+ messages in thread
From: Dong, Eric @ 2019-08-15 2:50 UTC (permalink / raw)
To: devel; +Cc: Ray Ni, Laszlo Ersek
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2040
Supports new logic which test current value before write new value.
Only write new value when current value not same as new value.
Signed-off-by: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
---
.../CpuFeaturesInitialize.c | 31 ++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
index 63bc50a55f..0a4fcff033 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
@@ -826,6 +826,7 @@ ProgramProcessorRegister (
UINTN ValidThreadCount;
UINT32 *ValidCoreCountPerPackage;
EFI_STATUS Status;
+ UINT64 CurrentValue;
//
// Traverse Register Table of this logical processor
@@ -848,7 +849,16 @@ ProgramProcessorRegister (
if (EFI_ERROR (Status)) {
break;
}
-
+ if (RegisterTableEntry->TestThenWrite) {
+ CurrentValue = BitFieldRead64 (
+ Value,
+ RegisterTableEntry->ValidBitStart,
+ RegisterTableEntry->ValidBitStart + RegisterTableEntry->ValidBitLength - 1
+ );
+ if (CurrentValue == RegisterTableEntry->Value) {
+ break;
+ }
+ }
Value = (UINTN) BitFieldWrite64 (
Value,
RegisterTableEntry->ValidBitStart,
@@ -857,10 +867,29 @@ ProgramProcessorRegister (
);
ReadWriteCr (RegisterTableEntry->Index, FALSE, &Value);
break;
+
//
// The specified register is Model Specific Register
//
case Msr:
+ if (RegisterTableEntry->TestThenWrite) {
+ Value = (UINTN)AsmReadMsr64 (RegisterTableEntry->Index);
+ if (RegisterTableEntry->ValidBitLength >= 64) {
+ if (Value == RegisterTableEntry->Value) {
+ break;
+ }
+ } else {
+ CurrentValue = BitFieldRead64 (
+ Value,
+ RegisterTableEntry->ValidBitStart,
+ RegisterTableEntry->ValidBitStart + RegisterTableEntry->ValidBitLength - 1
+ );
+ if (CurrentValue == RegisterTableEntry->Value) {
+ break;
+ }
+ }
+ }
+
if (RegisterTableEntry->ValidBitLength >= 64) {
//
// If length is not less than 64 bits, then directly write without reading
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Patch v3 6/6] UefiCpuPkg/CpuCommonFeaturesLib: Use new macros.
2019-08-15 2:50 [Patch v3 0/6] Add "test then write" mechanism Dong, Eric
` (4 preceding siblings ...)
2019-08-15 2:50 ` [Patch v3 5/6] UefiCpuPkg/RegisterCpuFeaturesLib: Supports test then write new value logic Dong, Eric
@ 2019-08-15 2:50 ` Dong, Eric
5 siblings, 0 replies; 13+ messages in thread
From: Dong, Eric @ 2019-08-15 2:50 UTC (permalink / raw)
To: devel; +Cc: Ray Ni, Laszlo Ersek
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2040
Below code is current implementation:
if (MsrRegister[ProcessorNumber].Bits.Lock == 0) {
CPU_REGISTER_TABLE_WRITE_FIELD (
ProcessorNumber,
Msr,
MSR_IA32_FEATURE_CONTROL,
MSR_IA32_FEATURE_CONTROL_REGISTER,
Bits.Lock,
1
);
}
1. In first normal boot, the Bits.Lock is 0, 1 will be added
into the register table and then will set to the MSR.
2. Trig warm reboot, MSR value preserves. After normal boot phase,
the Bits.Lock is 1, so it will not be added into the register
table during the warm reboot phase.
3. Trig S3 then resume, the Bits.Lock change to 0 and Bits.Lock is
not added in register table, so it's still 0 after resume. This
is not an expect behavior. The expect value is the value should
always 1 after booting or resuming from S3.
The root cause for this issue is
1. driver bases on current value to insert the "set value action" to
the register table.
2. Some MSRs may reserve their value during warm reboot.
The solution for this issue is using new added macros for the MSRs which
preserve value during warm reboot.
Signed-off-by: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
---
.../CpuCommonFeaturesLib/CpuCommonFeatures.h | 15 --
.../CpuCommonFeaturesLib.c | 8 +-
.../CpuCommonFeaturesLib/FeatureControl.c | 141 ++++++------------
.../CpuCommonFeaturesLib/MachineCheck.c | 23 ++-
4 files changed, 58 insertions(+), 129 deletions(-)
diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeatures.h b/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeatures.h
index 25d0174727..b2390e6c39 100644
--- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeatures.h
+++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeatures.h
@@ -848,21 +848,6 @@ X2ApicInitialize (
IN BOOLEAN State
);
-/**
- Prepares for the data used by CPU feature detection and initialization.
-
- @param[in] NumberOfProcessors The number of CPUs in the platform.
-
- @return Pointer to a buffer of CPU related configuration data.
-
- @note This service could be called by BSP only.
-**/
-VOID *
-EFIAPI
-FeatureControlGetConfigData (
- IN UINTN NumberOfProcessors
- );
-
/**
Prepares for the data used by CPU feature detection and initialization.
diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c
index fd43b8d662..f0dd3a3b43 100644
--- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c
+++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c
@@ -91,7 +91,7 @@ CpuCommonFeaturesLibConstructor (
if (IsCpuFeatureSupported (CPU_FEATURE_LOCK_FEATURE_CONTROL_REGISTER)) {
Status = RegisterCpuFeature (
"Lock Feature Control Register",
- FeatureControlGetConfigData,
+ NULL,
LockFeatureControlRegisterSupport,
LockFeatureControlRegisterInitialize,
CPU_FEATURE_LOCK_FEATURE_CONTROL_REGISTER,
@@ -102,7 +102,7 @@ CpuCommonFeaturesLibConstructor (
if (IsCpuFeatureSupported (CPU_FEATURE_SMX)) {
Status = RegisterCpuFeature (
"SMX",
- FeatureControlGetConfigData,
+ NULL,
SmxSupport,
SmxInitialize,
CPU_FEATURE_SMX,
@@ -114,7 +114,7 @@ CpuCommonFeaturesLibConstructor (
if (IsCpuFeatureSupported (CPU_FEATURE_VMX)) {
Status = RegisterCpuFeature (
"VMX",
- FeatureControlGetConfigData,
+ NULL,
VmxSupport,
VmxInitialize,
CPU_FEATURE_VMX,
@@ -214,7 +214,7 @@ CpuCommonFeaturesLibConstructor (
if (IsCpuFeatureSupported (CPU_FEATURE_LMCE)) {
Status = RegisterCpuFeature (
"LMCE",
- FeatureControlGetConfigData,
+ NULL,
LmceSupport,
LmceInitialize,
CPU_FEATURE_LMCE,
diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/FeatureControl.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/FeatureControl.c
index 3712ef1e5c..6679df8ba4 100644
--- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/FeatureControl.c
+++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/FeatureControl.c
@@ -8,28 +8,6 @@
#include "CpuCommonFeatures.h"
-/**
- Prepares for the data used by CPU feature detection and initialization.
-
- @param[in] NumberOfProcessors The number of CPUs in the platform.
-
- @return Pointer to a buffer of CPU related configuration data.
-
- @note This service could be called by BSP only.
-**/
-VOID *
-EFIAPI
-FeatureControlGetConfigData (
- IN UINTN NumberOfProcessors
- )
-{
- VOID *ConfigData;
-
- ConfigData = AllocateZeroPool (sizeof (MSR_IA32_FEATURE_CONTROL_REGISTER) * NumberOfProcessors);
- ASSERT (ConfigData != NULL);
- return ConfigData;
-}
-
/**
Detects if VMX feature supported on current processor.
@@ -54,11 +32,6 @@ VmxSupport (
IN VOID *ConfigData OPTIONAL
)
{
- MSR_IA32_FEATURE_CONTROL_REGISTER *MsrRegister;
-
- ASSERT (ConfigData != NULL);
- MsrRegister = (MSR_IA32_FEATURE_CONTROL_REGISTER *) ConfigData;
- MsrRegister[ProcessorNumber].Uint64 = AsmReadMsr64 (MSR_IA32_FEATURE_CONTROL);
return (CpuInfo->CpuIdVersionInfoEcx.Bits.VMX == 1);
}
@@ -88,8 +61,6 @@ VmxInitialize (
IN BOOLEAN State
)
{
- MSR_IA32_FEATURE_CONTROL_REGISTER *MsrRegister;
-
//
// The scope of EnableVmxOutsideSmx bit in the MSR_IA32_FEATURE_CONTROL is core for
// below processor type, only program MSR_IA32_FEATURE_CONTROL for thread 0 in each
@@ -103,18 +74,15 @@ VmxInitialize (
}
}
- ASSERT (ConfigData != NULL);
- MsrRegister = (MSR_IA32_FEATURE_CONTROL_REGISTER *) ConfigData;
- if (MsrRegister[ProcessorNumber].Bits.Lock == 0) {
- CPU_REGISTER_TABLE_WRITE_FIELD (
- ProcessorNumber,
- Msr,
- MSR_IA32_FEATURE_CONTROL,
- MSR_IA32_FEATURE_CONTROL_REGISTER,
- Bits.EnableVmxOutsideSmx,
- (State) ? 1 : 0
- );
- }
+ CPU_REGISTER_TABLE_TEST_THEN_WRITE_FIELD (
+ ProcessorNumber,
+ Msr,
+ MSR_IA32_FEATURE_CONTROL,
+ MSR_IA32_FEATURE_CONTROL_REGISTER,
+ Bits.EnableVmxOutsideSmx,
+ (State) ? 1 : 0
+ );
+
return RETURN_SUCCESS;
}
@@ -142,11 +110,6 @@ LockFeatureControlRegisterSupport (
IN VOID *ConfigData OPTIONAL
)
{
- MSR_IA32_FEATURE_CONTROL_REGISTER *MsrRegister;
-
- ASSERT (ConfigData != NULL);
- MsrRegister = (MSR_IA32_FEATURE_CONTROL_REGISTER *) ConfigData;
- MsrRegister[ProcessorNumber].Uint64 = AsmReadMsr64 (MSR_IA32_FEATURE_CONTROL);
return TRUE;
}
@@ -176,8 +139,6 @@ LockFeatureControlRegisterInitialize (
IN BOOLEAN State
)
{
- MSR_IA32_FEATURE_CONTROL_REGISTER *MsrRegister;
-
//
// The scope of Lock bit in the MSR_IA32_FEATURE_CONTROL is core for
// below processor type, only program MSR_IA32_FEATURE_CONTROL for thread 0 in each
@@ -191,18 +152,15 @@ LockFeatureControlRegisterInitialize (
}
}
- ASSERT (ConfigData != NULL);
- MsrRegister = (MSR_IA32_FEATURE_CONTROL_REGISTER *) ConfigData;
- if (MsrRegister[ProcessorNumber].Bits.Lock == 0) {
- CPU_REGISTER_TABLE_WRITE_FIELD (
- ProcessorNumber,
- Msr,
- MSR_IA32_FEATURE_CONTROL,
- MSR_IA32_FEATURE_CONTROL_REGISTER,
- Bits.Lock,
- 1
- );
- }
+ CPU_REGISTER_TABLE_TEST_THEN_WRITE_FIELD (
+ ProcessorNumber,
+ Msr,
+ MSR_IA32_FEATURE_CONTROL,
+ MSR_IA32_FEATURE_CONTROL_REGISTER,
+ Bits.Lock,
+ 1
+ );
+
return RETURN_SUCCESS;
}
@@ -230,11 +188,6 @@ SmxSupport (
IN VOID *ConfigData OPTIONAL
)
{
- MSR_IA32_FEATURE_CONTROL_REGISTER *MsrRegister;
-
- ASSERT (ConfigData != NULL);
- MsrRegister = (MSR_IA32_FEATURE_CONTROL_REGISTER *) ConfigData;
- MsrRegister[ProcessorNumber].Uint64 = AsmReadMsr64 (MSR_IA32_FEATURE_CONTROL);
return (CpuInfo->CpuIdVersionInfoEcx.Bits.SMX == 1);
}
@@ -265,7 +218,6 @@ SmxInitialize (
IN BOOLEAN State
)
{
- MSR_IA32_FEATURE_CONTROL_REGISTER *MsrRegister;
RETURN_STATUS Status;
//
@@ -288,35 +240,32 @@ SmxInitialize (
Status = RETURN_UNSUPPORTED;
}
- ASSERT (ConfigData != NULL);
- MsrRegister = (MSR_IA32_FEATURE_CONTROL_REGISTER *) ConfigData;
- if (MsrRegister[ProcessorNumber].Bits.Lock == 0) {
- CPU_REGISTER_TABLE_WRITE_FIELD (
- ProcessorNumber,
- Msr,
- MSR_IA32_FEATURE_CONTROL,
- MSR_IA32_FEATURE_CONTROL_REGISTER,
- Bits.SenterLocalFunctionEnables,
- (State) ? 0x7F : 0
- );
-
- CPU_REGISTER_TABLE_WRITE_FIELD (
- ProcessorNumber,
- Msr,
- MSR_IA32_FEATURE_CONTROL,
- MSR_IA32_FEATURE_CONTROL_REGISTER,
- Bits.SenterGlobalEnable,
- (State) ? 1 : 0
- );
-
- CPU_REGISTER_TABLE_WRITE_FIELD (
- ProcessorNumber,
- Msr,
- MSR_IA32_FEATURE_CONTROL,
- MSR_IA32_FEATURE_CONTROL_REGISTER,
- Bits.EnableVmxInsideSmx,
- (State) ? 1 : 0
- );
- }
+ CPU_REGISTER_TABLE_TEST_THEN_WRITE_FIELD (
+ ProcessorNumber,
+ Msr,
+ MSR_IA32_FEATURE_CONTROL,
+ MSR_IA32_FEATURE_CONTROL_REGISTER,
+ Bits.SenterLocalFunctionEnables,
+ (State) ? 0x7F : 0
+ );
+
+ CPU_REGISTER_TABLE_TEST_THEN_WRITE_FIELD (
+ ProcessorNumber,
+ Msr,
+ MSR_IA32_FEATURE_CONTROL,
+ MSR_IA32_FEATURE_CONTROL_REGISTER,
+ Bits.SenterGlobalEnable,
+ (State) ? 1 : 0
+ );
+
+ CPU_REGISTER_TABLE_TEST_THEN_WRITE_FIELD (
+ ProcessorNumber,
+ Msr,
+ MSR_IA32_FEATURE_CONTROL,
+ MSR_IA32_FEATURE_CONTROL_REGISTER,
+ Bits.EnableVmxInsideSmx,
+ (State) ? 1 : 0
+ );
+
return Status;
}
diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c
index 2528e0044e..01fd6bb54d 100644
--- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c
+++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c
@@ -319,8 +319,6 @@ LmceInitialize (
IN BOOLEAN State
)
{
- MSR_IA32_FEATURE_CONTROL_REGISTER *MsrRegister;
-
//
// The scope of LcmeOn bit in the MSR_IA32_MISC_ENABLE is core for below processor type, only program
// MSR_IA32_MISC_ENABLE for thread 0 in each core.
@@ -333,17 +331,14 @@ LmceInitialize (
}
}
- ASSERT (ConfigData != NULL);
- MsrRegister = (MSR_IA32_FEATURE_CONTROL_REGISTER *) ConfigData;
- if (MsrRegister[ProcessorNumber].Bits.Lock == 0) {
- CPU_REGISTER_TABLE_WRITE_FIELD (
- ProcessorNumber,
- Msr,
- MSR_IA32_FEATURE_CONTROL,
- MSR_IA32_FEATURE_CONTROL_REGISTER,
- Bits.LmceOn,
- (State) ? 1 : 0
- );
- }
+ CPU_REGISTER_TABLE_TEST_THEN_WRITE_FIELD (
+ ProcessorNumber,
+ Msr,
+ MSR_IA32_FEATURE_CONTROL,
+ MSR_IA32_FEATURE_CONTROL_REGISTER,
+ Bits.LmceOn,
+ (State) ? 1 : 0
+ );
+
return RETURN_SUCCESS;
}
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test Then Write" Macros.
2019-08-15 2:50 ` [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test Then Write" Macros Dong, Eric
@ 2019-08-16 1:14 ` Zeng, Star
2019-08-16 1:27 ` Dong, Eric
0 siblings, 1 reply; 13+ messages in thread
From: Zeng, Star @ 2019-08-16 1:14 UTC (permalink / raw)
To: Dong, Eric, devel@edk2.groups.io; +Cc: Ni, Ray, Laszlo Ersek, Zeng, Star
> -----Original Message-----
> From: Dong, Eric
> Sent: Thursday, August 15, 2019 10:51 AM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Zeng,
> Star <star.zeng@intel.com>
> Subject: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test Then
> Write" Macros.
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2040
>
> Add below new micros which test the current value before write the new
> value. Only write new value when current value not same as new value.
> CPU_REGISTER_TABLE_TEST_THEN_WRITE32
> CPU_REGISTER_TABLE_TEST_THEN_WRITE64
> CPU_REGISTER_TABLE_TEST_THEN_WRITE_FIELD
>
> Also add below API:
> CpuRegisterTableTestThenWrite
>
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Star Zeng <star.zeng@intel.com>
> ---
> UefiCpuPkg/Include/AcpiCpuData.h | 1 +
> .../Include/Library/RegisterCpuFeaturesLib.h | 91 +++++++++++++++++++
> .../RegisterCpuFeaturesLib.c | 45 ++++++++-
> 3 files changed, 134 insertions(+), 3 deletions(-)
>
> diff --git a/UefiCpuPkg/Include/AcpiCpuData.h
> b/UefiCpuPkg/Include/AcpiCpuData.h
> index b963a2f592..472a1a8070 100644
> --- a/UefiCpuPkg/Include/AcpiCpuData.h
> +++ b/UefiCpuPkg/Include/AcpiCpuData.h
> @@ -81,6 +81,7 @@ typedef struct {
> UINT16 Reserved; // offset 10 - 11
> UINT32 HighIndex; // offset 12-15, only valid for MemoryMapped
> UINT64 Value; // offset 16-23
> + UINT8 TestThenWrite; // 0ffset 24
Could we use one byte of the Reserved field, but not add new field? And use BOOLEAN type for it?
Thanks,
Star
> } CPU_REGISTER_TABLE_ENTRY;
>
> //
> diff --git a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> index e420e7f075..5bd464b32e 100644
> --- a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> +++ b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> @@ -348,6 +348,32 @@ CpuRegisterTableWrite (
> IN UINT64 Value
> );
>
> +/**
> + Adds an entry in specified register table.
> +
> + This function adds an entry in specified register table, with given
> + register type, register index, bit section and value.
> +
> + Driver will test the current value before setting new value.
> +
> + @param[in] ProcessorNumber The index of the CPU to add a register
> table entry
> + @param[in] RegisterType Type of the register to program
> + @param[in] Index Index of the register to program
> + @param[in] ValueMask Mask of bits in register to write
> + @param[in] Value Value to write
> +
> + @note This service could be called by BSP only.
> +**/
> +VOID
> +EFIAPI
> +CpuRegisterTableTestThenWrite (
> + IN UINTN ProcessorNumber,
> + IN REGISTER_TYPE RegisterType,
> + IN UINT64 Index,
> + IN UINT64 ValueMask,
> + IN UINT64 Value
> + );
> +
> /**
> Adds an entry in specified Pre-SMM register table.
>
> @@ -390,6 +416,26 @@ PreSmmCpuRegisterTableWrite (
> CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index,
> MAX_UINT32, Value); \
> } while(FALSE);
>
> +/**
> + Adds a 32-bit register write entry in specified register table.
> +
> + This macro adds an entry in specified register table, with given
> + register type, register index, and value.
> +
> + Driver will test the current value before setting new value.
> +
> + @param[in] ProcessorNumber The index of the CPU to add a register
> table entry.
> + @param[in] RegisterType Type of the register to program
> + @param[in] Index Index of the register to program
> + @param[in] Value Value to write
> +
> + @note This service could be called by BSP only.
> +**/
> +#define CPU_REGISTER_TABLE_TEST_THEN_WRITE32(ProcessorNumber,
> RegisterType, Index, Value) \
> + do { \
> + CpuRegisterTableTestThenWrite (ProcessorNumber, RegisterType,
> +Index, MAX_UINT32, Value); \
> + } while(FALSE);
> +
> /**
> Adds a 64-bit register write entry in specified register table.
>
> @@ -408,6 +454,26 @@ PreSmmCpuRegisterTableWrite (
> CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index,
> MAX_UINT64, Value); \
> } while(FALSE);
>
> +/**
> + Adds a 64-bit register write entry in specified register table.
> +
> + This macro adds an entry in specified register table, with given
> + register type, register index, and value.
> +
> + Driver will test the current value before setting new value.
> +
> + @param[in] ProcessorNumber The index of the CPU to add a register
> table entry.
> + @param[in] RegisterType Type of the register to program
> + @param[in] Index Index of the register to program
> + @param[in] Value Value to write
> +
> + @note This service could be called by BSP only.
> +**/
> +#define CPU_REGISTER_TABLE_TEST_THEN_WRITE64(ProcessorNumber,
> RegisterType, Index, Value) \
> + do { \
> + CpuRegisterTableTestThenWrite (ProcessorNumber, RegisterType,
> +Index, MAX_UINT64, Value); \
> + } while(FALSE);
> +
> /**
> Adds a bit field write entry in specified register table.
>
> @@ -431,6 +497,31 @@ PreSmmCpuRegisterTableWrite (
> CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index,
> ~ValueMask, Value); \
> } while(FALSE);
>
> +/**
> + Adds a bit field write entry in specified register table.
> +
> + This macro adds an entry in specified register table, with given
> + register type, register index, bit field section, and value.
> +
> + Driver will test the current value before setting new value.
> +
> + @param[in] ProcessorNumber The index of the CPU to add a register
> table entry.
> + @param[in] RegisterType Type of the register to program.
> + @param[in] Index Index of the register to program.
> + @param[in] Type The data type name of a register structure.
> + @param[in] Field The bit fiel name in register structure to write.
> + @param[in] Value Value to write to the bit field.
> +
> + @note This service could be called by BSP only.
> +**/
> +#define
> CPU_REGISTER_TABLE_TEST_THEN_WRITE_FIELD(ProcessorNumber,
> RegisterType, Index, Type, Field, Value) \
> + do { \
> + UINT64 ValueMask; \
> + ValueMask = MAX_UINT64; \
> + ((Type *)(&ValueMask))->Field = 0; \
> + CpuRegisterTableTestThenWrite (ProcessorNumber, RegisterType, Index,
> ~ValueMask, Value); \
> + } while(FALSE);
> +
> /**
> Adds a 32-bit register write entry in specified register table.
>
> diff --git
> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> index 67885bf69b..e9769882b9 100644
> --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> +++
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> @@ -1025,6 +1025,8 @@ EnlargeRegisterTable (
> @param[in] ValidBitStart Start of the bit section
> @param[in] ValidBitLength Length of the bit section
> @param[in] Value Value to write
> + @param[in] TestThenWrite Whether need to test current Value before
> writing.
> +
> **/
> VOID
> CpuRegisterTableWriteWorker (
> @@ -1034,7 +1036,8 @@ CpuRegisterTableWriteWorker (
> IN UINT64 Index,
> IN UINT8 ValidBitStart,
> IN UINT8 ValidBitLength,
> - IN UINT64 Value
> + IN UINT64 Value,
> + IN UINT8 TestThenWrite
> )
> {
> CPU_FEATURES_DATA *CpuFeaturesData;
> @@ -1070,6 +1073,7 @@ CpuRegisterTableWriteWorker (
> RegisterTableEntry[RegisterTable->TableLength].ValidBitStart =
> ValidBitStart;
> RegisterTableEntry[RegisterTable->TableLength].ValidBitLength =
> ValidBitLength;
> RegisterTableEntry[RegisterTable->TableLength].Value = Value;
> + RegisterTableEntry[RegisterTable->TableLength].TestThenWrite =
> + TestThenWrite;
>
> RegisterTable->TableLength++;
> }
> @@ -1105,7 +1109,42 @@ CpuRegisterTableWrite (
> Start = (UINT8)LowBitSet64 (ValueMask);
> End = (UINT8)HighBitSet64 (ValueMask);
> Length = End - Start + 1;
> - CpuRegisterTableWriteWorker (FALSE, ProcessorNumber, RegisterType,
> Index, Start, Length, Value);
> + CpuRegisterTableWriteWorker (FALSE, ProcessorNumber, RegisterType,
> +Index, Start, Length, Value, FALSE); }
> +
> +/**
> + Adds an entry in specified register table.
> +
> + This function adds an entry in specified register table, with given
> + register type, register index, bit section and value.
> +
> + @param[in] ProcessorNumber The index of the CPU to add a register
> table entry
> + @param[in] RegisterType Type of the register to program
> + @param[in] Index Index of the register to program
> + @param[in] ValueMask Mask of bits in register to write
> + @param[in] Value Value to write
> + @param[in] TestThenWrite Whether need to test current Value before
> writing.
> +
> + @note This service could be called by BSP only.
> +**/
> +VOID
> +EFIAPI
> +CpuRegisterTableTestThenWrite (
> + IN UINTN ProcessorNumber,
> + IN REGISTER_TYPE RegisterType,
> + IN UINT64 Index,
> + IN UINT64 ValueMask,
> + IN UINT64 Value
> + )
> +{
> + UINT8 Start;
> + UINT8 End;
> + UINT8 Length;
> +
> + Start = (UINT8)LowBitSet64 (ValueMask);
> + End = (UINT8)HighBitSet64 (ValueMask);
> + Length = End - Start + 1;
> + CpuRegisterTableWriteWorker (FALSE, ProcessorNumber, RegisterType,
> + Index, Start, Length, Value, TRUE);
> }
>
> /**
> @@ -1139,7 +1178,7 @@ PreSmmCpuRegisterTableWrite (
> Start = (UINT8)LowBitSet64 (ValueMask);
> End = (UINT8)HighBitSet64 (ValueMask);
> Length = End - Start + 1;
> - CpuRegisterTableWriteWorker (TRUE, ProcessorNumber, RegisterType,
> Index, Start, Length, Value);
> + CpuRegisterTableWriteWorker (TRUE, ProcessorNumber, RegisterType,
> + Index, Start, Length, Value, FALSE);
> }
>
> /**
> --
> 2.21.0.windows.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test Then Write" Macros.
2019-08-16 1:14 ` Zeng, Star
@ 2019-08-16 1:27 ` Dong, Eric
2019-08-16 2:08 ` Zeng, Star
0 siblings, 1 reply; 13+ messages in thread
From: Dong, Eric @ 2019-08-16 1:27 UTC (permalink / raw)
To: Zeng, Star, devel@edk2.groups.io; +Cc: Ni, Ray, Laszlo Ersek
> -----Original Message-----
> From: Zeng, Star
> Sent: Friday, August 16, 2019 9:15 AM
> To: Dong, Eric <eric.dong@intel.com>; devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Zeng,
> Star <star.zeng@intel.com>
> Subject: RE: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test
> Then Write" Macros.
>
>
>
> > -----Original Message-----
> > From: Dong, Eric
> > Sent: Thursday, August 15, 2019 10:51 AM
> > To: devel@edk2.groups.io
> > Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> > Zeng, Star <star.zeng@intel.com>
> > Subject: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test
> > Then Write" Macros.
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2040
> >
> > Add below new micros which test the current value before write the new
> > value. Only write new value when current value not same as new value.
> > CPU_REGISTER_TABLE_TEST_THEN_WRITE32
> > CPU_REGISTER_TABLE_TEST_THEN_WRITE64
> > CPU_REGISTER_TABLE_TEST_THEN_WRITE_FIELD
> >
> > Also add below API:
> > CpuRegisterTableTestThenWrite
> >
> > Signed-off-by: Eric Dong <eric.dong@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > ---
> > UefiCpuPkg/Include/AcpiCpuData.h | 1 +
> > .../Include/Library/RegisterCpuFeaturesLib.h | 91 +++++++++++++++++++
> > .../RegisterCpuFeaturesLib.c | 45 ++++++++-
> > 3 files changed, 134 insertions(+), 3 deletions(-)
> >
> > diff --git a/UefiCpuPkg/Include/AcpiCpuData.h
> > b/UefiCpuPkg/Include/AcpiCpuData.h
> > index b963a2f592..472a1a8070 100644
> > --- a/UefiCpuPkg/Include/AcpiCpuData.h
> > +++ b/UefiCpuPkg/Include/AcpiCpuData.h
> > @@ -81,6 +81,7 @@ typedef struct {
> > UINT16 Reserved; // offset 10 - 11
> > UINT32 HighIndex; // offset 12-15, only valid for MemoryMapped
> > UINT64 Value; // offset 16-23
> > + UINT8 TestThenWrite; // 0ffset 24
>
> Could we use one byte of the Reserved field, but not add new field? And use
> BOOLEAN type for it?
I'm not sure whether use the Reserved field is an correct approach, do you have samples which use the reserved fields?
But I think add new field is a more safe one.
Thanks,
Eric
>
> Thanks,
> Star
>
> > } CPU_REGISTER_TABLE_ENTRY;
> >
> > //
> > diff --git a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> > b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> > index e420e7f075..5bd464b32e 100644
> > --- a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> > +++ b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> > @@ -348,6 +348,32 @@ CpuRegisterTableWrite (
> > IN UINT64 Value
> > );
> >
> > +/**
> > + Adds an entry in specified register table.
> > +
> > + This function adds an entry in specified register table, with given
> > + register type, register index, bit section and value.
> > +
> > + Driver will test the current value before setting new value.
> > +
> > + @param[in] ProcessorNumber The index of the CPU to add a register
> > table entry
> > + @param[in] RegisterType Type of the register to program
> > + @param[in] Index Index of the register to program
> > + @param[in] ValueMask Mask of bits in register to write
> > + @param[in] Value Value to write
> > +
> > + @note This service could be called by BSP only.
> > +**/
> > +VOID
> > +EFIAPI
> > +CpuRegisterTableTestThenWrite (
> > + IN UINTN ProcessorNumber,
> > + IN REGISTER_TYPE RegisterType,
> > + IN UINT64 Index,
> > + IN UINT64 ValueMask,
> > + IN UINT64 Value
> > + );
> > +
> > /**
> > Adds an entry in specified Pre-SMM register table.
> >
> > @@ -390,6 +416,26 @@ PreSmmCpuRegisterTableWrite (
> > CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index,
> > MAX_UINT32, Value); \
> > } while(FALSE);
> >
> > +/**
> > + Adds a 32-bit register write entry in specified register table.
> > +
> > + This macro adds an entry in specified register table, with given
> > + register type, register index, and value.
> > +
> > + Driver will test the current value before setting new value.
> > +
> > + @param[in] ProcessorNumber The index of the CPU to add a register
> > table entry.
> > + @param[in] RegisterType Type of the register to program
> > + @param[in] Index Index of the register to program
> > + @param[in] Value Value to write
> > +
> > + @note This service could be called by BSP only.
> > +**/
> > +#define CPU_REGISTER_TABLE_TEST_THEN_WRITE32(ProcessorNumber,
> > RegisterType, Index, Value) \
> > + do { \
> > + CpuRegisterTableTestThenWrite (ProcessorNumber, RegisterType,
> > +Index, MAX_UINT32, Value); \
> > + } while(FALSE);
> > +
> > /**
> > Adds a 64-bit register write entry in specified register table.
> >
> > @@ -408,6 +454,26 @@ PreSmmCpuRegisterTableWrite (
> > CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index,
> > MAX_UINT64, Value); \
> > } while(FALSE);
> >
> > +/**
> > + Adds a 64-bit register write entry in specified register table.
> > +
> > + This macro adds an entry in specified register table, with given
> > + register type, register index, and value.
> > +
> > + Driver will test the current value before setting new value.
> > +
> > + @param[in] ProcessorNumber The index of the CPU to add a register
> > table entry.
> > + @param[in] RegisterType Type of the register to program
> > + @param[in] Index Index of the register to program
> > + @param[in] Value Value to write
> > +
> > + @note This service could be called by BSP only.
> > +**/
> > +#define CPU_REGISTER_TABLE_TEST_THEN_WRITE64(ProcessorNumber,
> > RegisterType, Index, Value) \
> > + do { \
> > + CpuRegisterTableTestThenWrite (ProcessorNumber, RegisterType,
> > +Index, MAX_UINT64, Value); \
> > + } while(FALSE);
> > +
> > /**
> > Adds a bit field write entry in specified register table.
> >
> > @@ -431,6 +497,31 @@ PreSmmCpuRegisterTableWrite (
> > CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index,
> > ~ValueMask, Value); \
> > } while(FALSE);
> >
> > +/**
> > + Adds a bit field write entry in specified register table.
> > +
> > + This macro adds an entry in specified register table, with given
> > + register type, register index, bit field section, and value.
> > +
> > + Driver will test the current value before setting new value.
> > +
> > + @param[in] ProcessorNumber The index of the CPU to add a register
> > table entry.
> > + @param[in] RegisterType Type of the register to program.
> > + @param[in] Index Index of the register to program.
> > + @param[in] Type The data type name of a register structure.
> > + @param[in] Field The bit fiel name in register structure to write.
> > + @param[in] Value Value to write to the bit field.
> > +
> > + @note This service could be called by BSP only.
> > +**/
> > +#define
> > CPU_REGISTER_TABLE_TEST_THEN_WRITE_FIELD(ProcessorNumber,
> > RegisterType, Index, Type, Field, Value) \
> > + do { \
> > + UINT64 ValueMask; \
> > + ValueMask = MAX_UINT64; \
> > + ((Type *)(&ValueMask))->Field = 0; \
> > + CpuRegisterTableTestThenWrite (ProcessorNumber, RegisterType,
> > + Index,
> > ~ValueMask, Value); \
> > + } while(FALSE);
> > +
> > /**
> > Adds a 32-bit register write entry in specified register table.
> >
> > diff --git
> > a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> > b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> > index 67885bf69b..e9769882b9 100644
> > ---
> > a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> > +++
> > b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> > @@ -1025,6 +1025,8 @@ EnlargeRegisterTable (
> > @param[in] ValidBitStart Start of the bit section
> > @param[in] ValidBitLength Length of the bit section
> > @param[in] Value Value to write
> > + @param[in] TestThenWrite Whether need to test current Value before
> > writing.
> > +
> > **/
> > VOID
> > CpuRegisterTableWriteWorker (
> > @@ -1034,7 +1036,8 @@ CpuRegisterTableWriteWorker (
> > IN UINT64 Index,
> > IN UINT8 ValidBitStart,
> > IN UINT8 ValidBitLength,
> > - IN UINT64 Value
> > + IN UINT64 Value,
> > + IN UINT8 TestThenWrite
> > )
> > {
> > CPU_FEATURES_DATA *CpuFeaturesData;
> > @@ -1070,6 +1073,7 @@ CpuRegisterTableWriteWorker (
> > RegisterTableEntry[RegisterTable->TableLength].ValidBitStart =
> > ValidBitStart;
> > RegisterTableEntry[RegisterTable->TableLength].ValidBitLength =
> > ValidBitLength;
> > RegisterTableEntry[RegisterTable->TableLength].Value = Value;
> > + RegisterTableEntry[RegisterTable->TableLength].TestThenWrite =
> > + TestThenWrite;
> >
> > RegisterTable->TableLength++;
> > }
> > @@ -1105,7 +1109,42 @@ CpuRegisterTableWrite (
> > Start = (UINT8)LowBitSet64 (ValueMask);
> > End = (UINT8)HighBitSet64 (ValueMask);
> > Length = End - Start + 1;
> > - CpuRegisterTableWriteWorker (FALSE, ProcessorNumber, RegisterType,
> > Index, Start, Length, Value);
> > + CpuRegisterTableWriteWorker (FALSE, ProcessorNumber, RegisterType,
> > +Index, Start, Length, Value, FALSE); }
> > +
> > +/**
> > + Adds an entry in specified register table.
> > +
> > + This function adds an entry in specified register table, with given
> > + register type, register index, bit section and value.
> > +
> > + @param[in] ProcessorNumber The index of the CPU to add a register
> > table entry
> > + @param[in] RegisterType Type of the register to program
> > + @param[in] Index Index of the register to program
> > + @param[in] ValueMask Mask of bits in register to write
> > + @param[in] Value Value to write
> > + @param[in] TestThenWrite Whether need to test current Value before
> > writing.
> > +
> > + @note This service could be called by BSP only.
> > +**/
> > +VOID
> > +EFIAPI
> > +CpuRegisterTableTestThenWrite (
> > + IN UINTN ProcessorNumber,
> > + IN REGISTER_TYPE RegisterType,
> > + IN UINT64 Index,
> > + IN UINT64 ValueMask,
> > + IN UINT64 Value
> > + )
> > +{
> > + UINT8 Start;
> > + UINT8 End;
> > + UINT8 Length;
> > +
> > + Start = (UINT8)LowBitSet64 (ValueMask);
> > + End = (UINT8)HighBitSet64 (ValueMask);
> > + Length = End - Start + 1;
> > + CpuRegisterTableWriteWorker (FALSE, ProcessorNumber, RegisterType,
> > + Index, Start, Length, Value, TRUE);
> > }
> >
> > /**
> > @@ -1139,7 +1178,7 @@ PreSmmCpuRegisterTableWrite (
> > Start = (UINT8)LowBitSet64 (ValueMask);
> > End = (UINT8)HighBitSet64 (ValueMask);
> > Length = End - Start + 1;
> > - CpuRegisterTableWriteWorker (TRUE, ProcessorNumber, RegisterType,
> > Index, Start, Length, Value);
> > + CpuRegisterTableWriteWorker (TRUE, ProcessorNumber, RegisterType,
> > + Index, Start, Length, Value, FALSE);
> > }
> >
> > /**
> > --
> > 2.21.0.windows.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test Then Write" Macros.
2019-08-16 1:27 ` Dong, Eric
@ 2019-08-16 2:08 ` Zeng, Star
2019-08-16 2:20 ` Dong, Eric
0 siblings, 1 reply; 13+ messages in thread
From: Zeng, Star @ 2019-08-16 2:08 UTC (permalink / raw)
To: Dong, Eric, devel@edk2.groups.io
Cc: Ni, Ray, Laszlo Ersek, Yao, Jiewen, Gao, Liming,
Kinney, Michael D, Zeng, Star
> -----Original Message-----
> From: Dong, Eric
> Sent: Friday, August 16, 2019 9:27 AM
> To: Zeng, Star <star.zeng@intel.com>; devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: RE: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test
> Then Write" Macros.
>
> > -----Original Message-----
> > From: Zeng, Star
> > Sent: Friday, August 16, 2019 9:15 AM
> > To: Dong, Eric <eric.dong@intel.com>; devel@edk2.groups.io
> > Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> > Zeng, Star <star.zeng@intel.com>
> > Subject: RE: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add
> > "Test Then Write" Macros.
> >
> >
> >
> > > -----Original Message-----
> > > From: Dong, Eric
> > > Sent: Thursday, August 15, 2019 10:51 AM
> > > To: devel@edk2.groups.io
> > > Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> > > Zeng, Star <star.zeng@intel.com>
> > > Subject: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test
> > > Then Write" Macros.
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2040
> > >
> > > Add below new micros which test the current value before write the
> > > new value. Only write new value when current value not same as new
> value.
> > > CPU_REGISTER_TABLE_TEST_THEN_WRITE32
> > > CPU_REGISTER_TABLE_TEST_THEN_WRITE64
> > > CPU_REGISTER_TABLE_TEST_THEN_WRITE_FIELD
> > >
> > > Also add below API:
> > > CpuRegisterTableTestThenWrite
> > >
> > > Signed-off-by: Eric Dong <eric.dong@intel.com>
> > > Cc: Ray Ni <ray.ni@intel.com>
> > > Cc: Laszlo Ersek <lersek@redhat.com>
> > > Cc: Star Zeng <star.zeng@intel.com>
> > > ---
> > > UefiCpuPkg/Include/AcpiCpuData.h | 1 +
> > > .../Include/Library/RegisterCpuFeaturesLib.h | 91
> +++++++++++++++++++
> > > .../RegisterCpuFeaturesLib.c | 45 ++++++++-
> > > 3 files changed, 134 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/UefiCpuPkg/Include/AcpiCpuData.h
> > > b/UefiCpuPkg/Include/AcpiCpuData.h
> > > index b963a2f592..472a1a8070 100644
> > > --- a/UefiCpuPkg/Include/AcpiCpuData.h
> > > +++ b/UefiCpuPkg/Include/AcpiCpuData.h
> > > @@ -81,6 +81,7 @@ typedef struct {
> > > UINT16 Reserved; // offset 10 - 11
> > > UINT32 HighIndex; // offset 12-15, only valid for
> MemoryMapped
> > > UINT64 Value; // offset 16-23
> > > + UINT8 TestThenWrite; // 0ffset 24
> >
> > Could we use one byte of the Reserved field, but not add new field?
> > And use BOOLEAN type for it?
>
> I'm not sure whether use the Reserved field is an correct approach, do you
> have samples which use the reserved fields?
> But I think add new field is a more safe one.
What "more safe" means here? Adding new field extends the structure, from the structure layout view of point, it is an incompatible change, the structure size is not just from 24 to 25 bytes, but to be nature aligned, the structure size will be (24 + 8) 32. Since there is Reserved field can be reused, that size impact can be removed.
FspGlobalData.h:
SHA-1: a2e61f341d26a78751b2f19b5004c6bbfc8b4fa9
* IntelFsp2Pkg: Support FSP Dispatch mode
MemoryProfile.h
SHA-1: 072a3ca1d36a42aec97f871c808776ee7038ca06 (related to 94092aa60341a3e4b1e1ea7c362781b8404ac538)
* MdeModulePkg MemoryProfile.h:two bytes of Reserved[4] as ActionStringOffset
If needed, more comments from Ray, Laszlo, Jiewen, Liming and Mike will be better.
Thanks,
Star
>
> Thanks,
> Eric
> >
> > Thanks,
> > Star
> >
> > > } CPU_REGISTER_TABLE_ENTRY;
> > >
> > > //
> > > diff --git a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> > > b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> > > index e420e7f075..5bd464b32e 100644
> > > --- a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> > > +++ b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> > > @@ -348,6 +348,32 @@ CpuRegisterTableWrite (
> > > IN UINT64 Value
> > > );
> > >
> > > +/**
> > > + Adds an entry in specified register table.
> > > +
> > > + This function adds an entry in specified register table, with
> > > + given register type, register index, bit section and value.
> > > +
> > > + Driver will test the current value before setting new value.
> > > +
> > > + @param[in] ProcessorNumber The index of the CPU to add a
> > > + register
> > > table entry
> > > + @param[in] RegisterType Type of the register to program
> > > + @param[in] Index Index of the register to program
> > > + @param[in] ValueMask Mask of bits in register to write
> > > + @param[in] Value Value to write
> > > +
> > > + @note This service could be called by BSP only.
> > > +**/
> > > +VOID
> > > +EFIAPI
> > > +CpuRegisterTableTestThenWrite (
> > > + IN UINTN ProcessorNumber,
> > > + IN REGISTER_TYPE RegisterType,
> > > + IN UINT64 Index,
> > > + IN UINT64 ValueMask,
> > > + IN UINT64 Value
> > > + );
> > > +
> > > /**
> > > Adds an entry in specified Pre-SMM register table.
> > >
> > > @@ -390,6 +416,26 @@ PreSmmCpuRegisterTableWrite (
> > > CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index,
> > > MAX_UINT32, Value); \
> > > } while(FALSE);
> > >
> > > +/**
> > > + Adds a 32-bit register write entry in specified register table.
> > > +
> > > + This macro adds an entry in specified register table, with given
> > > + register type, register index, and value.
> > > +
> > > + Driver will test the current value before setting new value.
> > > +
> > > + @param[in] ProcessorNumber The index of the CPU to add a
> > > + register
> > > table entry.
> > > + @param[in] RegisterType Type of the register to program
> > > + @param[in] Index Index of the register to program
> > > + @param[in] Value Value to write
> > > +
> > > + @note This service could be called by BSP only.
> > > +**/
> > > +#define
> CPU_REGISTER_TABLE_TEST_THEN_WRITE32(ProcessorNumber,
> > > RegisterType, Index, Value) \
> > > + do { \
> > > + CpuRegisterTableTestThenWrite (ProcessorNumber, RegisterType,
> > > +Index, MAX_UINT32, Value); \
> > > + } while(FALSE);
> > > +
> > > /**
> > > Adds a 64-bit register write entry in specified register table.
> > >
> > > @@ -408,6 +454,26 @@ PreSmmCpuRegisterTableWrite (
> > > CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index,
> > > MAX_UINT64, Value); \
> > > } while(FALSE);
> > >
> > > +/**
> > > + Adds a 64-bit register write entry in specified register table.
> > > +
> > > + This macro adds an entry in specified register table, with given
> > > + register type, register index, and value.
> > > +
> > > + Driver will test the current value before setting new value.
> > > +
> > > + @param[in] ProcessorNumber The index of the CPU to add a
> > > + register
> > > table entry.
> > > + @param[in] RegisterType Type of the register to program
> > > + @param[in] Index Index of the register to program
> > > + @param[in] Value Value to write
> > > +
> > > + @note This service could be called by BSP only.
> > > +**/
> > > +#define
> CPU_REGISTER_TABLE_TEST_THEN_WRITE64(ProcessorNumber,
> > > RegisterType, Index, Value) \
> > > + do { \
> > > + CpuRegisterTableTestThenWrite (ProcessorNumber, RegisterType,
> > > +Index, MAX_UINT64, Value); \
> > > + } while(FALSE);
> > > +
> > > /**
> > > Adds a bit field write entry in specified register table.
> > >
> > > @@ -431,6 +497,31 @@ PreSmmCpuRegisterTableWrite (
> > > CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index,
> > > ~ValueMask, Value); \
> > > } while(FALSE);
> > >
> > > +/**
> > > + Adds a bit field write entry in specified register table.
> > > +
> > > + This macro adds an entry in specified register table, with given
> > > + register type, register index, bit field section, and value.
> > > +
> > > + Driver will test the current value before setting new value.
> > > +
> > > + @param[in] ProcessorNumber The index of the CPU to add a
> > > + register
> > > table entry.
> > > + @param[in] RegisterType Type of the register to program.
> > > + @param[in] Index Index of the register to program.
> > > + @param[in] Type The data type name of a register structure.
> > > + @param[in] Field The bit fiel name in register structure to write.
> > > + @param[in] Value Value to write to the bit field.
> > > +
> > > + @note This service could be called by BSP only.
> > > +**/
> > > +#define
> > > CPU_REGISTER_TABLE_TEST_THEN_WRITE_FIELD(ProcessorNumber,
> > > RegisterType, Index, Type, Field, Value) \
> > > + do { \
> > > + UINT64 ValueMask; \
> > > + ValueMask = MAX_UINT64; \
> > > + ((Type *)(&ValueMask))->Field = 0;
> \
> > > + CpuRegisterTableTestThenWrite (ProcessorNumber, RegisterType,
> > > + Index,
> > > ~ValueMask, Value); \
> > > + } while(FALSE);
> > > +
> > > /**
> > > Adds a 32-bit register write entry in specified register table.
> > >
> > > diff --git
> > > a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> > > b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> > > index 67885bf69b..e9769882b9 100644
> > > ---
> > > a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> > > +++
> > > b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> > > @@ -1025,6 +1025,8 @@ EnlargeRegisterTable (
> > > @param[in] ValidBitStart Start of the bit section
> > > @param[in] ValidBitLength Length of the bit section
> > > @param[in] Value Value to write
> > > + @param[in] TestThenWrite Whether need to test current Value
> before
> > > writing.
> > > +
> > > **/
> > > VOID
> > > CpuRegisterTableWriteWorker (
> > > @@ -1034,7 +1036,8 @@ CpuRegisterTableWriteWorker (
> > > IN UINT64 Index,
> > > IN UINT8 ValidBitStart,
> > > IN UINT8 ValidBitLength,
> > > - IN UINT64 Value
> > > + IN UINT64 Value,
> > > + IN UINT8 TestThenWrite
> > > )
> > > {
> > > CPU_FEATURES_DATA *CpuFeaturesData;
> > > @@ -1070,6 +1073,7 @@ CpuRegisterTableWriteWorker (
> > > RegisterTableEntry[RegisterTable->TableLength].ValidBitStart =
> > > ValidBitStart;
> > > RegisterTableEntry[RegisterTable->TableLength].ValidBitLength =
> > > ValidBitLength;
> > > RegisterTableEntry[RegisterTable->TableLength].Value = Value;
> > > + RegisterTableEntry[RegisterTable->TableLength].TestThenWrite =
> > > + TestThenWrite;
> > >
> > > RegisterTable->TableLength++;
> > > }
> > > @@ -1105,7 +1109,42 @@ CpuRegisterTableWrite (
> > > Start = (UINT8)LowBitSet64 (ValueMask);
> > > End = (UINT8)HighBitSet64 (ValueMask);
> > > Length = End - Start + 1;
> > > - CpuRegisterTableWriteWorker (FALSE, ProcessorNumber,
> > > RegisterType, Index, Start, Length, Value);
> > > + CpuRegisterTableWriteWorker (FALSE, ProcessorNumber,
> > > +RegisterType, Index, Start, Length, Value, FALSE); }
> > > +
> > > +/**
> > > + Adds an entry in specified register table.
> > > +
> > > + This function adds an entry in specified register table, with
> > > + given register type, register index, bit section and value.
> > > +
> > > + @param[in] ProcessorNumber The index of the CPU to add a
> > > + register
> > > table entry
> > > + @param[in] RegisterType Type of the register to program
> > > + @param[in] Index Index of the register to program
> > > + @param[in] ValueMask Mask of bits in register to write
> > > + @param[in] Value Value to write
> > > + @param[in] TestThenWrite Whether need to test current Value
> before
> > > writing.
> > > +
> > > + @note This service could be called by BSP only.
> > > +**/
> > > +VOID
> > > +EFIAPI
> > > +CpuRegisterTableTestThenWrite (
> > > + IN UINTN ProcessorNumber,
> > > + IN REGISTER_TYPE RegisterType,
> > > + IN UINT64 Index,
> > > + IN UINT64 ValueMask,
> > > + IN UINT64 Value
> > > + )
> > > +{
> > > + UINT8 Start;
> > > + UINT8 End;
> > > + UINT8 Length;
> > > +
> > > + Start = (UINT8)LowBitSet64 (ValueMask);
> > > + End = (UINT8)HighBitSet64 (ValueMask);
> > > + Length = End - Start + 1;
> > > + CpuRegisterTableWriteWorker (FALSE, ProcessorNumber,
> > > + RegisterType, Index, Start, Length, Value, TRUE);
> > > }
> > >
> > > /**
> > > @@ -1139,7 +1178,7 @@ PreSmmCpuRegisterTableWrite (
> > > Start = (UINT8)LowBitSet64 (ValueMask);
> > > End = (UINT8)HighBitSet64 (ValueMask);
> > > Length = End - Start + 1;
> > > - CpuRegisterTableWriteWorker (TRUE, ProcessorNumber, RegisterType,
> > > Index, Start, Length, Value);
> > > + CpuRegisterTableWriteWorker (TRUE, ProcessorNumber, RegisterType,
> > > + Index, Start, Length, Value, FALSE);
> > > }
> > >
> > > /**
> > > --
> > > 2.21.0.windows.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test Then Write" Macros.
2019-08-16 2:08 ` Zeng, Star
@ 2019-08-16 2:20 ` Dong, Eric
2019-08-16 3:44 ` Liming Gao
0 siblings, 1 reply; 13+ messages in thread
From: Dong, Eric @ 2019-08-16 2:20 UTC (permalink / raw)
To: Zeng, Star, devel@edk2.groups.io
Cc: Ni, Ray, Laszlo Ersek, Yao, Jiewen, Gao, Liming,
Kinney, Michael D
> -----Original Message-----
> From: Zeng, Star
> Sent: Friday, August 16, 2019 10:08 AM
> To: Dong, Eric <eric.dong@intel.com>; devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Gao, Liming <liming.gao@intel.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: RE: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test
> Then Write" Macros.
>
>
>
> > -----Original Message-----
> > From: Dong, Eric
> > Sent: Friday, August 16, 2019 9:27 AM
> > To: Zeng, Star <star.zeng@intel.com>; devel@edk2.groups.io
> > Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>
> > Subject: RE: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add
> > "Test Then Write" Macros.
> >
> > > -----Original Message-----
> > > From: Zeng, Star
> > > Sent: Friday, August 16, 2019 9:15 AM
> > > To: Dong, Eric <eric.dong@intel.com>; devel@edk2.groups.io
> > > Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> > > Zeng, Star <star.zeng@intel.com>
> > > Subject: RE: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add
> > > "Test Then Write" Macros.
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Dong, Eric
> > > > Sent: Thursday, August 15, 2019 10:51 AM
> > > > To: devel@edk2.groups.io
> > > > Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> > > > Zeng, Star <star.zeng@intel.com>
> > > > Subject: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add
> > > > "Test Then Write" Macros.
> > > >
> > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2040
> > > >
> > > > Add below new micros which test the current value before write the
> > > > new value. Only write new value when current value not same as new
> > value.
> > > > CPU_REGISTER_TABLE_TEST_THEN_WRITE32
> > > > CPU_REGISTER_TABLE_TEST_THEN_WRITE64
> > > > CPU_REGISTER_TABLE_TEST_THEN_WRITE_FIELD
> > > >
> > > > Also add below API:
> > > > CpuRegisterTableTestThenWrite
> > > >
> > > > Signed-off-by: Eric Dong <eric.dong@intel.com>
> > > > Cc: Ray Ni <ray.ni@intel.com>
> > > > Cc: Laszlo Ersek <lersek@redhat.com>
> > > > Cc: Star Zeng <star.zeng@intel.com>
> > > > ---
> > > > UefiCpuPkg/Include/AcpiCpuData.h | 1 +
> > > > .../Include/Library/RegisterCpuFeaturesLib.h | 91
> > +++++++++++++++++++
> > > > .../RegisterCpuFeaturesLib.c | 45 ++++++++-
> > > > 3 files changed, 134 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/UefiCpuPkg/Include/AcpiCpuData.h
> > > > b/UefiCpuPkg/Include/AcpiCpuData.h
> > > > index b963a2f592..472a1a8070 100644
> > > > --- a/UefiCpuPkg/Include/AcpiCpuData.h
> > > > +++ b/UefiCpuPkg/Include/AcpiCpuData.h
> > > > @@ -81,6 +81,7 @@ typedef struct {
> > > > UINT16 Reserved; // offset 10 - 11
> > > > UINT32 HighIndex; // offset 12-15, only valid for
> > MemoryMapped
> > > > UINT64 Value; // offset 16-23
> > > > + UINT8 TestThenWrite; // 0ffset 24
> > >
> > > Could we use one byte of the Reserved field, but not add new field?
> > > And use BOOLEAN type for it?
> >
> > I'm not sure whether use the Reserved field is an correct approach, do
> > you have samples which use the reserved fields?
> > But I think add new field is a more safe one.
>
> What "more safe" means here? Adding new field extends the structure, from
> the structure layout view of point, it is an incompatible change, the structure
> size is not just from 24 to 25 bytes, but to be nature aligned, the structure size
> will be (24 + 8) 32. Since there is Reserved field can be reused, that size impact
> can be removed.
Yes, agree the size impact. I'm just not clear whether the Reserved field can be used. Whether it has compatible impact.
>
> FspGlobalData.h:
> SHA-1: a2e61f341d26a78751b2f19b5004c6bbfc8b4fa9
> * IntelFsp2Pkg: Support FSP Dispatch mode
>
> MemoryProfile.h
> SHA-1: 072a3ca1d36a42aec97f871c808776ee7038ca06 (related to
> 94092aa60341a3e4b1e1ea7c362781b8404ac538)
> * MdeModulePkg MemoryProfile.h:two bytes of Reserved[4] as
> ActionStringOffset
>
If so many examples already did it. I think we can also do it.
>
> If needed, more comments from Ray, Laszlo, Jiewen, Liming and Mike will be
> better.
Yes, I think we need get more comments from them.
Thanks,
Eric
>
>
> Thanks,
> Star
>
> >
> > Thanks,
> > Eric
> > >
> > > Thanks,
> > > Star
> > >
> > > > } CPU_REGISTER_TABLE_ENTRY;
> > > >
> > > > //
> > > > diff --git a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> > > > b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> > > > index e420e7f075..5bd464b32e 100644
> > > > --- a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> > > > +++ b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> > > > @@ -348,6 +348,32 @@ CpuRegisterTableWrite (
> > > > IN UINT64 Value
> > > > );
> > > >
> > > > +/**
> > > > + Adds an entry in specified register table.
> > > > +
> > > > + This function adds an entry in specified register table, with
> > > > + given register type, register index, bit section and value.
> > > > +
> > > > + Driver will test the current value before setting new value.
> > > > +
> > > > + @param[in] ProcessorNumber The index of the CPU to add a
> > > > + register
> > > > table entry
> > > > + @param[in] RegisterType Type of the register to program
> > > > + @param[in] Index Index of the register to program
> > > > + @param[in] ValueMask Mask of bits in register to write
> > > > + @param[in] Value Value to write
> > > > +
> > > > + @note This service could be called by BSP only.
> > > > +**/
> > > > +VOID
> > > > +EFIAPI
> > > > +CpuRegisterTableTestThenWrite (
> > > > + IN UINTN ProcessorNumber,
> > > > + IN REGISTER_TYPE RegisterType,
> > > > + IN UINT64 Index,
> > > > + IN UINT64 ValueMask,
> > > > + IN UINT64 Value
> > > > + );
> > > > +
> > > > /**
> > > > Adds an entry in specified Pre-SMM register table.
> > > >
> > > > @@ -390,6 +416,26 @@ PreSmmCpuRegisterTableWrite (
> > > > CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index,
> > > > MAX_UINT32, Value); \
> > > > } while(FALSE);
> > > >
> > > > +/**
> > > > + Adds a 32-bit register write entry in specified register table.
> > > > +
> > > > + This macro adds an entry in specified register table, with
> > > > + given register type, register index, and value.
> > > > +
> > > > + Driver will test the current value before setting new value.
> > > > +
> > > > + @param[in] ProcessorNumber The index of the CPU to add a
> > > > + register
> > > > table entry.
> > > > + @param[in] RegisterType Type of the register to program
> > > > + @param[in] Index Index of the register to program
> > > > + @param[in] Value Value to write
> > > > +
> > > > + @note This service could be called by BSP only.
> > > > +**/
> > > > +#define
> > CPU_REGISTER_TABLE_TEST_THEN_WRITE32(ProcessorNumber,
> > > > RegisterType, Index, Value) \
> > > > + do { \
> > > > + CpuRegisterTableTestThenWrite (ProcessorNumber, RegisterType,
> > > > +Index, MAX_UINT32, Value); \
> > > > + } while(FALSE);
> > > > +
> > > > /**
> > > > Adds a 64-bit register write entry in specified register table.
> > > >
> > > > @@ -408,6 +454,26 @@ PreSmmCpuRegisterTableWrite (
> > > > CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index,
> > > > MAX_UINT64, Value); \
> > > > } while(FALSE);
> > > >
> > > > +/**
> > > > + Adds a 64-bit register write entry in specified register table.
> > > > +
> > > > + This macro adds an entry in specified register table, with
> > > > + given register type, register index, and value.
> > > > +
> > > > + Driver will test the current value before setting new value.
> > > > +
> > > > + @param[in] ProcessorNumber The index of the CPU to add a
> > > > + register
> > > > table entry.
> > > > + @param[in] RegisterType Type of the register to program
> > > > + @param[in] Index Index of the register to program
> > > > + @param[in] Value Value to write
> > > > +
> > > > + @note This service could be called by BSP only.
> > > > +**/
> > > > +#define
> > CPU_REGISTER_TABLE_TEST_THEN_WRITE64(ProcessorNumber,
> > > > RegisterType, Index, Value) \
> > > > + do { \
> > > > + CpuRegisterTableTestThenWrite (ProcessorNumber, RegisterType,
> > > > +Index, MAX_UINT64, Value); \
> > > > + } while(FALSE);
> > > > +
> > > > /**
> > > > Adds a bit field write entry in specified register table.
> > > >
> > > > @@ -431,6 +497,31 @@ PreSmmCpuRegisterTableWrite (
> > > > CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index,
> > > > ~ValueMask, Value); \
> > > > } while(FALSE);
> > > >
> > > > +/**
> > > > + Adds a bit field write entry in specified register table.
> > > > +
> > > > + This macro adds an entry in specified register table, with
> > > > + given register type, register index, bit field section, and value.
> > > > +
> > > > + Driver will test the current value before setting new value.
> > > > +
> > > > + @param[in] ProcessorNumber The index of the CPU to add a
> > > > + register
> > > > table entry.
> > > > + @param[in] RegisterType Type of the register to program.
> > > > + @param[in] Index Index of the register to program.
> > > > + @param[in] Type The data type name of a register structure.
> > > > + @param[in] Field The bit fiel name in register structure to write.
> > > > + @param[in] Value Value to write to the bit field.
> > > > +
> > > > + @note This service could be called by BSP only.
> > > > +**/
> > > > +#define
> > > > CPU_REGISTER_TABLE_TEST_THEN_WRITE_FIELD(ProcessorNumber,
> > > > RegisterType, Index, Type, Field, Value) \
> > > > + do { \
> > > > + UINT64 ValueMask; \
> > > > + ValueMask = MAX_UINT64;
> \
> > > > + ((Type *)(&ValueMask))->Field = 0;
> > \
> > > > + CpuRegisterTableTestThenWrite (ProcessorNumber, RegisterType,
> > > > + Index,
> > > > ~ValueMask, Value); \
> > > > + } while(FALSE);
> > > > +
> > > > /**
> > > > Adds a 32-bit register write entry in specified register table.
> > > >
> > > > diff --git
> > > > a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib
> > > > .c
> > > > b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib
> > > > .c index 67885bf69b..e9769882b9 100644
> > > > ---
> > > > a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib
> > > > .c
> > > > +++
> > > > b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib
> > > > .c @@ -1025,6 +1025,8 @@ EnlargeRegisterTable (
> > > > @param[in] ValidBitStart Start of the bit section
> > > > @param[in] ValidBitLength Length of the bit section
> > > > @param[in] Value Value to write
> > > > + @param[in] TestThenWrite Whether need to test current Value
> > before
> > > > writing.
> > > > +
> > > > **/
> > > > VOID
> > > > CpuRegisterTableWriteWorker (
> > > > @@ -1034,7 +1036,8 @@ CpuRegisterTableWriteWorker (
> > > > IN UINT64 Index,
> > > > IN UINT8 ValidBitStart,
> > > > IN UINT8 ValidBitLength,
> > > > - IN UINT64 Value
> > > > + IN UINT64 Value,
> > > > + IN UINT8 TestThenWrite
> > > > )
> > > > {
> > > > CPU_FEATURES_DATA *CpuFeaturesData;
> > > > @@ -1070,6 +1073,7 @@ CpuRegisterTableWriteWorker (
> > > > RegisterTableEntry[RegisterTable->TableLength].ValidBitStart =
> > > > ValidBitStart;
> > > > RegisterTableEntry[RegisterTable->TableLength].ValidBitLength =
> > > > ValidBitLength;
> > > > RegisterTableEntry[RegisterTable->TableLength].Value = Value;
> > > > + RegisterTableEntry[RegisterTable->TableLength].TestThenWrite =
> > > > + TestThenWrite;
> > > >
> > > > RegisterTable->TableLength++;
> > > > }
> > > > @@ -1105,7 +1109,42 @@ CpuRegisterTableWrite (
> > > > Start = (UINT8)LowBitSet64 (ValueMask);
> > > > End = (UINT8)HighBitSet64 (ValueMask);
> > > > Length = End - Start + 1;
> > > > - CpuRegisterTableWriteWorker (FALSE, ProcessorNumber,
> > > > RegisterType, Index, Start, Length, Value);
> > > > + CpuRegisterTableWriteWorker (FALSE, ProcessorNumber,
> > > > +RegisterType, Index, Start, Length, Value, FALSE); }
> > > > +
> > > > +/**
> > > > + Adds an entry in specified register table.
> > > > +
> > > > + This function adds an entry in specified register table, with
> > > > + given register type, register index, bit section and value.
> > > > +
> > > > + @param[in] ProcessorNumber The index of the CPU to add a
> > > > + register
> > > > table entry
> > > > + @param[in] RegisterType Type of the register to program
> > > > + @param[in] Index Index of the register to program
> > > > + @param[in] ValueMask Mask of bits in register to write
> > > > + @param[in] Value Value to write
> > > > + @param[in] TestThenWrite Whether need to test current Value
> > before
> > > > writing.
> > > > +
> > > > + @note This service could be called by BSP only.
> > > > +**/
> > > > +VOID
> > > > +EFIAPI
> > > > +CpuRegisterTableTestThenWrite (
> > > > + IN UINTN ProcessorNumber,
> > > > + IN REGISTER_TYPE RegisterType,
> > > > + IN UINT64 Index,
> > > > + IN UINT64 ValueMask,
> > > > + IN UINT64 Value
> > > > + )
> > > > +{
> > > > + UINT8 Start;
> > > > + UINT8 End;
> > > > + UINT8 Length;
> > > > +
> > > > + Start = (UINT8)LowBitSet64 (ValueMask);
> > > > + End = (UINT8)HighBitSet64 (ValueMask);
> > > > + Length = End - Start + 1;
> > > > + CpuRegisterTableWriteWorker (FALSE, ProcessorNumber,
> > > > + RegisterType, Index, Start, Length, Value, TRUE);
> > > > }
> > > >
> > > > /**
> > > > @@ -1139,7 +1178,7 @@ PreSmmCpuRegisterTableWrite (
> > > > Start = (UINT8)LowBitSet64 (ValueMask);
> > > > End = (UINT8)HighBitSet64 (ValueMask);
> > > > Length = End - Start + 1;
> > > > - CpuRegisterTableWriteWorker (TRUE, ProcessorNumber,
> > > > RegisterType, Index, Start, Length, Value);
> > > > + CpuRegisterTableWriteWorker (TRUE, ProcessorNumber,
> > > > + RegisterType, Index, Start, Length, Value, FALSE);
> > > > }
> > > >
> > > > /**
> > > > --
> > > > 2.21.0.windows.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test Then Write" Macros.
2019-08-16 2:20 ` Dong, Eric
@ 2019-08-16 3:44 ` Liming Gao
2019-08-16 3:58 ` Dong, Eric
0 siblings, 1 reply; 13+ messages in thread
From: Liming Gao @ 2019-08-16 3:44 UTC (permalink / raw)
To: Dong, Eric, Zeng, Star, devel@edk2.groups.io
Cc: Ni, Ray, Laszlo Ersek, Yao, Jiewen, Kinney, Michael D
Eric:
>-----Original Message-----
>From: Dong, Eric
>Sent: Friday, August 16, 2019 10:20 AM
>To: Zeng, Star <star.zeng@intel.com>; devel@edk2.groups.io
>Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Yao,
>Jiewen <jiewen.yao@intel.com>; Gao, Liming <liming.gao@intel.com>;
>Kinney, Michael D <michael.d.kinney@intel.com>
>Subject: RE: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test
>Then Write" Macros.
>
>
>
>> -----Original Message-----
>> From: Zeng, Star
>> Sent: Friday, August 16, 2019 10:08 AM
>> To: Dong, Eric <eric.dong@intel.com>; devel@edk2.groups.io
>> Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Yao,
>> Jiewen <jiewen.yao@intel.com>; Gao, Liming <liming.gao@intel.com>;
>Kinney,
>> Michael D <michael.d.kinney@intel.com>; Zeng, Star <star.zeng@intel.com>
>> Subject: RE: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test
>> Then Write" Macros.
>>
>>
>>
>> > -----Original Message-----
>> > From: Dong, Eric
>> > Sent: Friday, August 16, 2019 9:27 AM
>> > To: Zeng, Star <star.zeng@intel.com>; devel@edk2.groups.io
>> > Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>
>> > Subject: RE: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add
>> > "Test Then Write" Macros.
>> >
>> > > -----Original Message-----
>> > > From: Zeng, Star
>> > > Sent: Friday, August 16, 2019 9:15 AM
>> > > To: Dong, Eric <eric.dong@intel.com>; devel@edk2.groups.io
>> > > Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>;
>> > > Zeng, Star <star.zeng@intel.com>
>> > > Subject: RE: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add
>> > > "Test Then Write" Macros.
>> > >
>> > >
>> > >
>> > > > -----Original Message-----
>> > > > From: Dong, Eric
>> > > > Sent: Thursday, August 15, 2019 10:51 AM
>> > > > To: devel@edk2.groups.io
>> > > > Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>;
>> > > > Zeng, Star <star.zeng@intel.com>
>> > > > Subject: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add
>> > > > "Test Then Write" Macros.
>> > > >
>> > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2040
>> > > >
>> > > > Add below new micros which test the current value before write the
>> > > > new value. Only write new value when current value not same as new
>> > value.
>> > > > CPU_REGISTER_TABLE_TEST_THEN_WRITE32
>> > > > CPU_REGISTER_TABLE_TEST_THEN_WRITE64
>> > > > CPU_REGISTER_TABLE_TEST_THEN_WRITE_FIELD
>> > > >
>> > > > Also add below API:
>> > > > CpuRegisterTableTestThenWrite
>> > > >
>> > > > Signed-off-by: Eric Dong <eric.dong@intel.com>
>> > > > Cc: Ray Ni <ray.ni@intel.com>
>> > > > Cc: Laszlo Ersek <lersek@redhat.com>
>> > > > Cc: Star Zeng <star.zeng@intel.com>
>> > > > ---
>> > > > UefiCpuPkg/Include/AcpiCpuData.h | 1 +
>> > > > .../Include/Library/RegisterCpuFeaturesLib.h | 91
>> > +++++++++++++++++++
>> > > > .../RegisterCpuFeaturesLib.c | 45 ++++++++-
>> > > > 3 files changed, 134 insertions(+), 3 deletions(-)
>> > > >
>> > > > diff --git a/UefiCpuPkg/Include/AcpiCpuData.h
>> > > > b/UefiCpuPkg/Include/AcpiCpuData.h
>> > > > index b963a2f592..472a1a8070 100644
>> > > > --- a/UefiCpuPkg/Include/AcpiCpuData.h
>> > > > +++ b/UefiCpuPkg/Include/AcpiCpuData.h
>> > > > @@ -81,6 +81,7 @@ typedef struct {
>> > > > UINT16 Reserved; // offset 10 - 11
>> > > > UINT32 HighIndex; // offset 12-15, only valid for
>> > MemoryMapped
>> > > > UINT64 Value; // offset 16-23
>> > > > + UINT8 TestThenWrite; // 0ffset 24
>> > >
>> > > Could we use one byte of the Reserved field, but not add new field?
>> > > And use BOOLEAN type for it?
>> >
>> > I'm not sure whether use the Reserved field is an correct approach, do
>> > you have samples which use the reserved fields?
>> > But I think add new field is a more safe one.
>>
>> What "more safe" means here? Adding new field extends the structure,
>from
>> the structure layout view of point, it is an incompatible change, the structure
>> size is not just from 24 to 25 bytes, but to be nature aligned, the structure
>size
>> will be (24 + 8) 32. Since there is Reserved field can be reused, that size
>impact
>> can be removed.
>
>Yes, agree the size impact. I'm just not clear whether the Reserved field can
>be used. Whether it has compatible impact.
>
>>
>> FspGlobalData.h:
>> SHA-1: a2e61f341d26a78751b2f19b5004c6bbfc8b4fa9
>> * IntelFsp2Pkg: Support FSP Dispatch mode
>>
>> MemoryProfile.h
>> SHA-1: 072a3ca1d36a42aec97f871c808776ee7038ca06 (related to
>> 94092aa60341a3e4b1e1ea7c362781b8404ac538)
>> * MdeModulePkg MemoryProfile.h:two bytes of Reserved[4] as
>> ActionStringOffset
>>
>
>If so many examples already did it. I think we can also do it.
>
>>
>> If needed, more comments from Ray, Laszlo, Jiewen, Liming and Mike will
>be
>> better.
>
>Yes, I think we need get more comments from them.
I think to reuse the reserved field is a good solution.
Thanks
Liming
>
>Thanks,
>Eric
>>
>>
>> Thanks,
>> Star
>>
>> >
>> > Thanks,
>> > Eric
>> > >
>> > > Thanks,
>> > > Star
>> > >
>> > > > } CPU_REGISTER_TABLE_ENTRY;
>> > > >
>> > > > //
>> > > > diff --git a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
>> > > > b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
>> > > > index e420e7f075..5bd464b32e 100644
>> > > > --- a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
>> > > > +++ b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
>> > > > @@ -348,6 +348,32 @@ CpuRegisterTableWrite (
>> > > > IN UINT64 Value
>> > > > );
>> > > >
>> > > > +/**
>> > > > + Adds an entry in specified register table.
>> > > > +
>> > > > + This function adds an entry in specified register table, with
>> > > > + given register type, register index, bit section and value.
>> > > > +
>> > > > + Driver will test the current value before setting new value.
>> > > > +
>> > > > + @param[in] ProcessorNumber The index of the CPU to add a
>> > > > + register
>> > > > table entry
>> > > > + @param[in] RegisterType Type of the register to program
>> > > > + @param[in] Index Index of the register to program
>> > > > + @param[in] ValueMask Mask of bits in register to write
>> > > > + @param[in] Value Value to write
>> > > > +
>> > > > + @note This service could be called by BSP only.
>> > > > +**/
>> > > > +VOID
>> > > > +EFIAPI
>> > > > +CpuRegisterTableTestThenWrite (
>> > > > + IN UINTN ProcessorNumber,
>> > > > + IN REGISTER_TYPE RegisterType,
>> > > > + IN UINT64 Index,
>> > > > + IN UINT64 ValueMask,
>> > > > + IN UINT64 Value
>> > > > + );
>> > > > +
>> > > > /**
>> > > > Adds an entry in specified Pre-SMM register table.
>> > > >
>> > > > @@ -390,6 +416,26 @@ PreSmmCpuRegisterTableWrite (
>> > > > CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index,
>> > > > MAX_UINT32, Value); \
>> > > > } while(FALSE);
>> > > >
>> > > > +/**
>> > > > + Adds a 32-bit register write entry in specified register table.
>> > > > +
>> > > > + This macro adds an entry in specified register table, with
>> > > > + given register type, register index, and value.
>> > > > +
>> > > > + Driver will test the current value before setting new value.
>> > > > +
>> > > > + @param[in] ProcessorNumber The index of the CPU to add a
>> > > > + register
>> > > > table entry.
>> > > > + @param[in] RegisterType Type of the register to program
>> > > > + @param[in] Index Index of the register to program
>> > > > + @param[in] Value Value to write
>> > > > +
>> > > > + @note This service could be called by BSP only.
>> > > > +**/
>> > > > +#define
>> > CPU_REGISTER_TABLE_TEST_THEN_WRITE32(ProcessorNumber,
>> > > > RegisterType, Index, Value) \
>> > > > + do { \
>> > > > + CpuRegisterTableTestThenWrite (ProcessorNumber, RegisterType,
>> > > > +Index, MAX_UINT32, Value); \
>> > > > + } while(FALSE);
>> > > > +
>> > > > /**
>> > > > Adds a 64-bit register write entry in specified register table.
>> > > >
>> > > > @@ -408,6 +454,26 @@ PreSmmCpuRegisterTableWrite (
>> > > > CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index,
>> > > > MAX_UINT64, Value); \
>> > > > } while(FALSE);
>> > > >
>> > > > +/**
>> > > > + Adds a 64-bit register write entry in specified register table.
>> > > > +
>> > > > + This macro adds an entry in specified register table, with
>> > > > + given register type, register index, and value.
>> > > > +
>> > > > + Driver will test the current value before setting new value.
>> > > > +
>> > > > + @param[in] ProcessorNumber The index of the CPU to add a
>> > > > + register
>> > > > table entry.
>> > > > + @param[in] RegisterType Type of the register to program
>> > > > + @param[in] Index Index of the register to program
>> > > > + @param[in] Value Value to write
>> > > > +
>> > > > + @note This service could be called by BSP only.
>> > > > +**/
>> > > > +#define
>> > CPU_REGISTER_TABLE_TEST_THEN_WRITE64(ProcessorNumber,
>> > > > RegisterType, Index, Value) \
>> > > > + do { \
>> > > > + CpuRegisterTableTestThenWrite (ProcessorNumber, RegisterType,
>> > > > +Index, MAX_UINT64, Value); \
>> > > > + } while(FALSE);
>> > > > +
>> > > > /**
>> > > > Adds a bit field write entry in specified register table.
>> > > >
>> > > > @@ -431,6 +497,31 @@ PreSmmCpuRegisterTableWrite (
>> > > > CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index,
>> > > > ~ValueMask, Value); \
>> > > > } while(FALSE);
>> > > >
>> > > > +/**
>> > > > + Adds a bit field write entry in specified register table.
>> > > > +
>> > > > + This macro adds an entry in specified register table, with
>> > > > + given register type, register index, bit field section, and value.
>> > > > +
>> > > > + Driver will test the current value before setting new value.
>> > > > +
>> > > > + @param[in] ProcessorNumber The index of the CPU to add a
>> > > > + register
>> > > > table entry.
>> > > > + @param[in] RegisterType Type of the register to program.
>> > > > + @param[in] Index Index of the register to program.
>> > > > + @param[in] Type The data type name of a register structure.
>> > > > + @param[in] Field The bit fiel name in register structure to
>write.
>> > > > + @param[in] Value Value to write to the bit field.
>> > > > +
>> > > > + @note This service could be called by BSP only.
>> > > > +**/
>> > > > +#define
>> > > > CPU_REGISTER_TABLE_TEST_THEN_WRITE_FIELD(ProcessorNumber,
>> > > > RegisterType, Index, Type, Field, Value) \
>> > > > + do { \
>> > > > + UINT64 ValueMask; \
>> > > > + ValueMask = MAX_UINT64;
>> \
>> > > > + ((Type *)(&ValueMask))->Field = 0;
>> > \
>> > > > + CpuRegisterTableTestThenWrite (ProcessorNumber, RegisterType,
>> > > > + Index,
>> > > > ~ValueMask, Value); \
>> > > > + } while(FALSE);
>> > > > +
>> > > > /**
>> > > > Adds a 32-bit register write entry in specified register table.
>> > > >
>> > > > diff --git
>> > > >
>a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib
>> > > > .c
>> > > >
>b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib
>> > > > .c index 67885bf69b..e9769882b9 100644
>> > > > ---
>> > > >
>a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib
>> > > > .c
>> > > > +++
>> > > >
>b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib
>> > > > .c @@ -1025,6 +1025,8 @@ EnlargeRegisterTable (
>> > > > @param[in] ValidBitStart Start of the bit section
>> > > > @param[in] ValidBitLength Length of the bit section
>> > > > @param[in] Value Value to write
>> > > > + @param[in] TestThenWrite Whether need to test current Value
>> > before
>> > > > writing.
>> > > > +
>> > > > **/
>> > > > VOID
>> > > > CpuRegisterTableWriteWorker (
>> > > > @@ -1034,7 +1036,8 @@ CpuRegisterTableWriteWorker (
>> > > > IN UINT64 Index,
>> > > > IN UINT8 ValidBitStart,
>> > > > IN UINT8 ValidBitLength,
>> > > > - IN UINT64 Value
>> > > > + IN UINT64 Value,
>> > > > + IN UINT8 TestThenWrite
>> > > > )
>> > > > {
>> > > > CPU_FEATURES_DATA *CpuFeaturesData;
>> > > > @@ -1070,6 +1073,7 @@ CpuRegisterTableWriteWorker (
>> > > > RegisterTableEntry[RegisterTable->TableLength].ValidBitStart =
>> > > > ValidBitStart;
>> > > > RegisterTableEntry[RegisterTable->TableLength].ValidBitLength =
>> > > > ValidBitLength;
>> > > > RegisterTableEntry[RegisterTable->TableLength].Value = Value;
>> > > > + RegisterTableEntry[RegisterTable->TableLength].TestThenWrite =
>> > > > + TestThenWrite;
>> > > >
>> > > > RegisterTable->TableLength++;
>> > > > }
>> > > > @@ -1105,7 +1109,42 @@ CpuRegisterTableWrite (
>> > > > Start = (UINT8)LowBitSet64 (ValueMask);
>> > > > End = (UINT8)HighBitSet64 (ValueMask);
>> > > > Length = End - Start + 1;
>> > > > - CpuRegisterTableWriteWorker (FALSE, ProcessorNumber,
>> > > > RegisterType, Index, Start, Length, Value);
>> > > > + CpuRegisterTableWriteWorker (FALSE, ProcessorNumber,
>> > > > +RegisterType, Index, Start, Length, Value, FALSE); }
>> > > > +
>> > > > +/**
>> > > > + Adds an entry in specified register table.
>> > > > +
>> > > > + This function adds an entry in specified register table, with
>> > > > + given register type, register index, bit section and value.
>> > > > +
>> > > > + @param[in] ProcessorNumber The index of the CPU to add a
>> > > > + register
>> > > > table entry
>> > > > + @param[in] RegisterType Type of the register to program
>> > > > + @param[in] Index Index of the register to program
>> > > > + @param[in] ValueMask Mask of bits in register to write
>> > > > + @param[in] Value Value to write
>> > > > + @param[in] TestThenWrite Whether need to test current Value
>> > before
>> > > > writing.
>> > > > +
>> > > > + @note This service could be called by BSP only.
>> > > > +**/
>> > > > +VOID
>> > > > +EFIAPI
>> > > > +CpuRegisterTableTestThenWrite (
>> > > > + IN UINTN ProcessorNumber,
>> > > > + IN REGISTER_TYPE RegisterType,
>> > > > + IN UINT64 Index,
>> > > > + IN UINT64 ValueMask,
>> > > > + IN UINT64 Value
>> > > > + )
>> > > > +{
>> > > > + UINT8 Start;
>> > > > + UINT8 End;
>> > > > + UINT8 Length;
>> > > > +
>> > > > + Start = (UINT8)LowBitSet64 (ValueMask);
>> > > > + End = (UINT8)HighBitSet64 (ValueMask);
>> > > > + Length = End - Start + 1;
>> > > > + CpuRegisterTableWriteWorker (FALSE, ProcessorNumber,
>> > > > + RegisterType, Index, Start, Length, Value, TRUE);
>> > > > }
>> > > >
>> > > > /**
>> > > > @@ -1139,7 +1178,7 @@ PreSmmCpuRegisterTableWrite (
>> > > > Start = (UINT8)LowBitSet64 (ValueMask);
>> > > > End = (UINT8)HighBitSet64 (ValueMask);
>> > > > Length = End - Start + 1;
>> > > > - CpuRegisterTableWriteWorker (TRUE, ProcessorNumber,
>> > > > RegisterType, Index, Start, Length, Value);
>> > > > + CpuRegisterTableWriteWorker (TRUE, ProcessorNumber,
>> > > > + RegisterType, Index, Start, Length, Value, FALSE);
>> > > > }
>> > > >
>> > > > /**
>> > > > --
>> > > > 2.21.0.windows.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test Then Write" Macros.
2019-08-16 3:44 ` Liming Gao
@ 2019-08-16 3:58 ` Dong, Eric
0 siblings, 0 replies; 13+ messages in thread
From: Dong, Eric @ 2019-08-16 3:58 UTC (permalink / raw)
To: Gao, Liming, Zeng, Star, devel@edk2.groups.io
Cc: Ni, Ray, Laszlo Ersek, Yao, Jiewen, Kinney, Michael D
Hi Liming & Star,
> -----Original Message-----
> From: Gao, Liming
> Sent: Friday, August 16, 2019 11:44 AM
> To: Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>;
> devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: RE: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test
> Then Write" Macros.
>
> Eric:
>
> >-----Original Message-----
> >From: Dong, Eric
> >Sent: Friday, August 16, 2019 10:20 AM
> >To: Zeng, Star <star.zeng@intel.com>; devel@edk2.groups.io
> >Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Yao,
> >Jiewen <jiewen.yao@intel.com>; Gao, Liming <liming.gao@intel.com>;
> >Kinney, Michael D <michael.d.kinney@intel.com>
> >Subject: RE: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add
> >"Test Then Write" Macros.
> >
> >
> >
> >> -----Original Message-----
> >> From: Zeng, Star
> >> Sent: Friday, August 16, 2019 10:08 AM
> >> To: Dong, Eric <eric.dong@intel.com>; devel@edk2.groups.io
> >> Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> >> Yao, Jiewen <jiewen.yao@intel.com>; Gao, Liming
> >> <liming.gao@intel.com>;
> >Kinney,
> >> Michael D <michael.d.kinney@intel.com>; Zeng, Star
> >> <star.zeng@intel.com>
> >> Subject: RE: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add
> >> "Test Then Write" Macros.
> >>
> >>
> >>
> >> > -----Original Message-----
> >> > From: Dong, Eric
> >> > Sent: Friday, August 16, 2019 9:27 AM
> >> > To: Zeng, Star <star.zeng@intel.com>; devel@edk2.groups.io
> >> > Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>
> >> > Subject: RE: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add
> >> > "Test Then Write" Macros.
> >> >
> >> > > -----Original Message-----
> >> > > From: Zeng, Star
> >> > > Sent: Friday, August 16, 2019 9:15 AM
> >> > > To: Dong, Eric <eric.dong@intel.com>; devel@edk2.groups.io
> >> > > Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> >> > > Zeng, Star <star.zeng@intel.com>
> >> > > Subject: RE: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib:
> >> > > Add "Test Then Write" Macros.
> >> > >
> >> > >
> >> > >
> >> > > > -----Original Message-----
> >> > > > From: Dong, Eric
> >> > > > Sent: Thursday, August 15, 2019 10:51 AM
> >> > > > To: devel@edk2.groups.io
> >> > > > Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek
> >> > > > <lersek@redhat.com>; Zeng, Star <star.zeng@intel.com>
> >> > > > Subject: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add
> >> > > > "Test Then Write" Macros.
> >> > > >
> >> > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2040
> >> > > >
> >> > > > Add below new micros which test the current value before write
> >> > > > the new value. Only write new value when current value not same
> >> > > > as new
> >> > value.
> >> > > > CPU_REGISTER_TABLE_TEST_THEN_WRITE32
> >> > > > CPU_REGISTER_TABLE_TEST_THEN_WRITE64
> >> > > > CPU_REGISTER_TABLE_TEST_THEN_WRITE_FIELD
> >> > > >
> >> > > > Also add below API:
> >> > > > CpuRegisterTableTestThenWrite
> >> > > >
> >> > > > Signed-off-by: Eric Dong <eric.dong@intel.com>
> >> > > > Cc: Ray Ni <ray.ni@intel.com>
> >> > > > Cc: Laszlo Ersek <lersek@redhat.com>
> >> > > > Cc: Star Zeng <star.zeng@intel.com>
> >> > > > ---
> >> > > > UefiCpuPkg/Include/AcpiCpuData.h | 1 +
> >> > > > .../Include/Library/RegisterCpuFeaturesLib.h | 91
> >> > +++++++++++++++++++
> >> > > > .../RegisterCpuFeaturesLib.c | 45 ++++++++-
> >> > > > 3 files changed, 134 insertions(+), 3 deletions(-)
> >> > > >
> >> > > > diff --git a/UefiCpuPkg/Include/AcpiCpuData.h
> >> > > > b/UefiCpuPkg/Include/AcpiCpuData.h
> >> > > > index b963a2f592..472a1a8070 100644
> >> > > > --- a/UefiCpuPkg/Include/AcpiCpuData.h
> >> > > > +++ b/UefiCpuPkg/Include/AcpiCpuData.h
> >> > > > @@ -81,6 +81,7 @@ typedef struct {
> >> > > > UINT16 Reserved; // offset 10 - 11
> >> > > > UINT32 HighIndex; // offset 12-15, only valid for
> >> > MemoryMapped
> >> > > > UINT64 Value; // offset 16-23
> >> > > > + UINT8 TestThenWrite; // 0ffset 24
> >> > >
> >> > > Could we use one byte of the Reserved field, but not add new field?
> >> > > And use BOOLEAN type for it?
> >> >
> >> > I'm not sure whether use the Reserved field is an correct approach,
> >> > do you have samples which use the reserved fields?
> >> > But I think add new field is a more safe one.
> >>
> >> What "more safe" means here? Adding new field extends the structure,
> >from
> >> the structure layout view of point, it is an incompatible change, the
> >> structure size is not just from 24 to 25 bytes, but to be nature
> >> aligned, the structure
> >size
> >> will be (24 + 8) 32. Since there is Reserved field can be reused,
> >> that size
> >impact
> >> can be removed.
> >
> >Yes, agree the size impact. I'm just not clear whether the Reserved
> >field can be used. Whether it has compatible impact.
> >
> >>
> >> FspGlobalData.h:
> >> SHA-1: a2e61f341d26a78751b2f19b5004c6bbfc8b4fa9
> >> * IntelFsp2Pkg: Support FSP Dispatch mode
> >>
> >> MemoryProfile.h
> >> SHA-1: 072a3ca1d36a42aec97f871c808776ee7038ca06 (related to
> >> 94092aa60341a3e4b1e1ea7c362781b8404ac538)
> >> * MdeModulePkg MemoryProfile.h:two bytes of Reserved[4] as
> >> ActionStringOffset
> >>
> >
> >If so many examples already did it. I think we can also do it.
> >
> >>
> >> If needed, more comments from Ray, Laszlo, Jiewen, Liming and Mike
> >> will
> >be
> >> better.
> >
> >Yes, I think we need get more comments from them.
>
> I think to reuse the reserved field is a good solution.
>
Thanks for this valuable input, I will send V4 patch to use Reserved field.
Thanks,
Eric
> Thanks
> Liming
> >
> >Thanks,
> >Eric
> >>
> >>
> >> Thanks,
> >> Star
> >>
> >> >
> >> > Thanks,
> >> > Eric
> >> > >
> >> > > Thanks,
> >> > > Star
> >> > >
> >> > > > } CPU_REGISTER_TABLE_ENTRY;
> >> > > >
> >> > > > //
> >> > > > diff --git
> >> > > > a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> >> > > > b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> >> > > > index e420e7f075..5bd464b32e 100644
> >> > > > --- a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> >> > > > +++ b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
> >> > > > @@ -348,6 +348,32 @@ CpuRegisterTableWrite (
> >> > > > IN UINT64 Value
> >> > > > );
> >> > > >
> >> > > > +/**
> >> > > > + Adds an entry in specified register table.
> >> > > > +
> >> > > > + This function adds an entry in specified register table,
> >> > > > + with given register type, register index, bit section and value.
> >> > > > +
> >> > > > + Driver will test the current value before setting new value.
> >> > > > +
> >> > > > + @param[in] ProcessorNumber The index of the CPU to add a
> >> > > > + register
> >> > > > table entry
> >> > > > + @param[in] RegisterType Type of the register to program
> >> > > > + @param[in] Index Index of the register to program
> >> > > > + @param[in] ValueMask Mask of bits in register to write
> >> > > > + @param[in] Value Value to write
> >> > > > +
> >> > > > + @note This service could be called by BSP only.
> >> > > > +**/
> >> > > > +VOID
> >> > > > +EFIAPI
> >> > > > +CpuRegisterTableTestThenWrite (
> >> > > > + IN UINTN ProcessorNumber,
> >> > > > + IN REGISTER_TYPE RegisterType,
> >> > > > + IN UINT64 Index,
> >> > > > + IN UINT64 ValueMask,
> >> > > > + IN UINT64 Value
> >> > > > + );
> >> > > > +
> >> > > > /**
> >> > > > Adds an entry in specified Pre-SMM register table.
> >> > > >
> >> > > > @@ -390,6 +416,26 @@ PreSmmCpuRegisterTableWrite (
> >> > > > CpuRegisterTableWrite (ProcessorNumber, RegisterType,
> >> > > > Index, MAX_UINT32, Value); \
> >> > > > } while(FALSE);
> >> > > >
> >> > > > +/**
> >> > > > + Adds a 32-bit register write entry in specified register table.
> >> > > > +
> >> > > > + This macro adds an entry in specified register table, with
> >> > > > + given register type, register index, and value.
> >> > > > +
> >> > > > + Driver will test the current value before setting new value.
> >> > > > +
> >> > > > + @param[in] ProcessorNumber The index of the CPU to add a
> >> > > > + register
> >> > > > table entry.
> >> > > > + @param[in] RegisterType Type of the register to program
> >> > > > + @param[in] Index Index of the register to program
> >> > > > + @param[in] Value Value to write
> >> > > > +
> >> > > > + @note This service could be called by BSP only.
> >> > > > +**/
> >> > > > +#define
> >> > CPU_REGISTER_TABLE_TEST_THEN_WRITE32(ProcessorNumber,
> >> > > > RegisterType, Index, Value) \
> >> > > > + do { \
> >> > > > + CpuRegisterTableTestThenWrite (ProcessorNumber,
> >> > > > +RegisterType, Index, MAX_UINT32, Value); \
> >> > > > + } while(FALSE);
> >> > > > +
> >> > > > /**
> >> > > > Adds a 64-bit register write entry in specified register table.
> >> > > >
> >> > > > @@ -408,6 +454,26 @@ PreSmmCpuRegisterTableWrite (
> >> > > > CpuRegisterTableWrite (ProcessorNumber, RegisterType,
> >> > > > Index, MAX_UINT64, Value); \
> >> > > > } while(FALSE);
> >> > > >
> >> > > > +/**
> >> > > > + Adds a 64-bit register write entry in specified register table.
> >> > > > +
> >> > > > + This macro adds an entry in specified register table, with
> >> > > > + given register type, register index, and value.
> >> > > > +
> >> > > > + Driver will test the current value before setting new value.
> >> > > > +
> >> > > > + @param[in] ProcessorNumber The index of the CPU to add a
> >> > > > + register
> >> > > > table entry.
> >> > > > + @param[in] RegisterType Type of the register to program
> >> > > > + @param[in] Index Index of the register to program
> >> > > > + @param[in] Value Value to write
> >> > > > +
> >> > > > + @note This service could be called by BSP only.
> >> > > > +**/
> >> > > > +#define
> >> > CPU_REGISTER_TABLE_TEST_THEN_WRITE64(ProcessorNumber,
> >> > > > RegisterType, Index, Value) \
> >> > > > + do { \
> >> > > > + CpuRegisterTableTestThenWrite (ProcessorNumber,
> >> > > > +RegisterType, Index, MAX_UINT64, Value); \
> >> > > > + } while(FALSE);
> >> > > > +
> >> > > > /**
> >> > > > Adds a bit field write entry in specified register table.
> >> > > >
> >> > > > @@ -431,6 +497,31 @@ PreSmmCpuRegisterTableWrite (
> >> > > > CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index,
> >> > > > ~ValueMask, Value); \
> >> > > > } while(FALSE);
> >> > > >
> >> > > > +/**
> >> > > > + Adds a bit field write entry in specified register table.
> >> > > > +
> >> > > > + This macro adds an entry in specified register table, with
> >> > > > + given register type, register index, bit field section, and value.
> >> > > > +
> >> > > > + Driver will test the current value before setting new value.
> >> > > > +
> >> > > > + @param[in] ProcessorNumber The index of the CPU to add a
> >> > > > + register
> >> > > > table entry.
> >> > > > + @param[in] RegisterType Type of the register to program.
> >> > > > + @param[in] Index Index of the register to program.
> >> > > > + @param[in] Type The data type name of a register structure.
> >> > > > + @param[in] Field The bit fiel name in register structure to
> >write.
> >> > > > + @param[in] Value Value to write to the bit field.
> >> > > > +
> >> > > > + @note This service could be called by BSP only.
> >> > > > +**/
> >> > > > +#define
> >> > > > CPU_REGISTER_TABLE_TEST_THEN_WRITE_FIELD(ProcessorNumber,
> >> > > > RegisterType, Index, Type, Field, Value) \
> >> > > > + do { \
> >> > > > + UINT64 ValueMask; \
> >> > > > + ValueMask = MAX_UINT64;
> >> \
> >> > > > + ((Type *)(&ValueMask))->Field = 0;
> >> > \
> >> > > > + CpuRegisterTableTestThenWrite (ProcessorNumber,
> >> > > > + RegisterType, Index,
> >> > > > ~ValueMask, Value); \
> >> > > > + } while(FALSE);
> >> > > > +
> >> > > > /**
> >> > > > Adds a 32-bit register write entry in specified register table.
> >> > > >
> >> > > > diff --git
> >> > > >
> >a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib
> >> > > > .c
> >> > > >
> >b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib
> >> > > > .c index 67885bf69b..e9769882b9 100644
> >> > > > ---
> >> > > >
> >a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib
> >> > > > .c
> >> > > > +++
> >> > > >
> >b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib
> >> > > > .c @@ -1025,6 +1025,8 @@ EnlargeRegisterTable (
> >> > > > @param[in] ValidBitStart Start of the bit section
> >> > > > @param[in] ValidBitLength Length of the bit section
> >> > > > @param[in] Value Value to write
> >> > > > + @param[in] TestThenWrite Whether need to test current Value
> >> > before
> >> > > > writing.
> >> > > > +
> >> > > > **/
> >> > > > VOID
> >> > > > CpuRegisterTableWriteWorker (
> >> > > > @@ -1034,7 +1036,8 @@ CpuRegisterTableWriteWorker (
> >> > > > IN UINT64 Index,
> >> > > > IN UINT8 ValidBitStart,
> >> > > > IN UINT8 ValidBitLength,
> >> > > > - IN UINT64 Value
> >> > > > + IN UINT64 Value,
> >> > > > + IN UINT8 TestThenWrite
> >> > > > )
> >> > > > {
> >> > > > CPU_FEATURES_DATA *CpuFeaturesData;
> >> > > > @@ -1070,6 +1073,7 @@ CpuRegisterTableWriteWorker (
> >> > > > RegisterTableEntry[RegisterTable->TableLength].ValidBitStart
> >> > > > = ValidBitStart;
> >> > > >
> >> > > > RegisterTableEntry[RegisterTable->TableLength].ValidBitLength =
> ValidBitLength;
> >> > > > RegisterTableEntry[RegisterTable->TableLength].Value = Value;
> >> > > > + RegisterTableEntry[RegisterTable->TableLength].TestThenWrite
> >> > > > + = TestThenWrite;
> >> > > >
> >> > > > RegisterTable->TableLength++; } @@ -1105,7 +1109,42 @@
> >> > > > CpuRegisterTableWrite (
> >> > > > Start = (UINT8)LowBitSet64 (ValueMask);
> >> > > > End = (UINT8)HighBitSet64 (ValueMask);
> >> > > > Length = End - Start + 1;
> >> > > > - CpuRegisterTableWriteWorker (FALSE, ProcessorNumber,
> >> > > > RegisterType, Index, Start, Length, Value);
> >> > > > + CpuRegisterTableWriteWorker (FALSE, ProcessorNumber,
> >> > > > +RegisterType, Index, Start, Length, Value, FALSE); }
> >> > > > +
> >> > > > +/**
> >> > > > + Adds an entry in specified register table.
> >> > > > +
> >> > > > + This function adds an entry in specified register table,
> >> > > > + with given register type, register index, bit section and value.
> >> > > > +
> >> > > > + @param[in] ProcessorNumber The index of the CPU to add a
> >> > > > + register
> >> > > > table entry
> >> > > > + @param[in] RegisterType Type of the register to program
> >> > > > + @param[in] Index Index of the register to program
> >> > > > + @param[in] ValueMask Mask of bits in register to write
> >> > > > + @param[in] Value Value to write
> >> > > > + @param[in] TestThenWrite Whether need to test current Value
> >> > before
> >> > > > writing.
> >> > > > +
> >> > > > + @note This service could be called by BSP only.
> >> > > > +**/
> >> > > > +VOID
> >> > > > +EFIAPI
> >> > > > +CpuRegisterTableTestThenWrite (
> >> > > > + IN UINTN ProcessorNumber,
> >> > > > + IN REGISTER_TYPE RegisterType,
> >> > > > + IN UINT64 Index,
> >> > > > + IN UINT64 ValueMask,
> >> > > > + IN UINT64 Value
> >> > > > + )
> >> > > > +{
> >> > > > + UINT8 Start;
> >> > > > + UINT8 End;
> >> > > > + UINT8 Length;
> >> > > > +
> >> > > > + Start = (UINT8)LowBitSet64 (ValueMask);
> >> > > > + End = (UINT8)HighBitSet64 (ValueMask);
> >> > > > + Length = End - Start + 1;
> >> > > > + CpuRegisterTableWriteWorker (FALSE, ProcessorNumber,
> >> > > > + RegisterType, Index, Start, Length, Value, TRUE);
> >> > > > }
> >> > > >
> >> > > > /**
> >> > > > @@ -1139,7 +1178,7 @@ PreSmmCpuRegisterTableWrite (
> >> > > > Start = (UINT8)LowBitSet64 (ValueMask);
> >> > > > End = (UINT8)HighBitSet64 (ValueMask);
> >> > > > Length = End - Start + 1;
> >> > > > - CpuRegisterTableWriteWorker (TRUE, ProcessorNumber,
> >> > > > RegisterType, Index, Start, Length, Value);
> >> > > > + CpuRegisterTableWriteWorker (TRUE, ProcessorNumber,
> >> > > > + RegisterType, Index, Start, Length, Value, FALSE);
> >> > > > }
> >> > > >
> >> > > > /**
> >> > > > --
> >> > > > 2.21.0.windows.1
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-08-16 3:58 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-08-15 2:50 [Patch v3 0/6] Add "test then write" mechanism Dong, Eric
2019-08-15 2:50 ` [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test Then Write" Macros Dong, Eric
2019-08-16 1:14 ` Zeng, Star
2019-08-16 1:27 ` Dong, Eric
2019-08-16 2:08 ` Zeng, Star
2019-08-16 2:20 ` Dong, Eric
2019-08-16 3:44 ` Liming Gao
2019-08-16 3:58 ` Dong, Eric
2019-08-15 2:50 ` [Patch v3 2/6] UefiCpuPkg/PiSmmCpuDxeSmm: Combine CR read/write action Dong, Eric
2019-08-15 2:50 ` [Patch v3 3/6] UefiCpuPkg/PiSmmCpuDxeSmm: Supports test then write new value logic Dong, Eric
2019-08-15 2:50 ` [Patch v3 4/6] UefiCpuPkg/RegisterCpuFeaturesLib: Combine CR read/write action Dong, Eric
2019-08-15 2:50 ` [Patch v3 5/6] UefiCpuPkg/RegisterCpuFeaturesLib: Supports test then write new value logic Dong, Eric
2019-08-15 2:50 ` [Patch v3 6/6] UefiCpuPkg/CpuCommonFeaturesLib: Use new macros Dong, Eric
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox