On 08/12/19 12:31, Eric Dong wrote: > 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...@intel.com> > Cc: Laszlo Ersek <ler...@redhat.com> > > Eric Dong (6): > UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test Then Write" Macros. > UefiCpuPkg/PiSmmCpuDxeSmm: Combine CR read/write action in one > function. > UefiCpuPkg/PiSmmCpuDxeSmm: Supports test then write new value logic. > UefiCpuPkg/RegisterCpuFeaturesLib: Combine CR read/write action in one > function. > UefiCpuPkg/RegisterCpuFeaturesLib: Supports test then write new value > logic. > UefiCpuPkg/CpuCommonFeaturesLib: Use new macros. > > UefiCpuPkg/Include/AcpiCpuData.h | 1 + > .../Include/Library/RegisterCpuFeaturesLib.h | 77 +++++++++- > .../CpuCommonFeaturesLib/CpuCommonFeatures.h | 15 -- > .../CpuCommonFeaturesLib.c | 8 +- > .../CpuCommonFeaturesLib/FeatureControl.c | 141 ++++++------------ > .../CpuCommonFeaturesLib/MachineCheck.c | 23 ++- > .../CpuFeaturesInitialize.c | 141 ++++++++++++------ > .../RegisterCpuFeaturesLib.c | 14 +- > UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 135 +++++++++++------ > 9 files changed, 323 insertions(+), 232 deletions(-) >
Please don't forget to build-test this series for IA32 too. Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#45442): https://edk2.groups.io/g/devel/message/45442 Mute This Topic: https://groups.io/mt/32839204/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-