Hi Laszlo, Yes, I already checked IA32 build.
As Ray is leaving these days, can you help to review this serial? Thanks, Eric > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Laszlo Ersek > Sent: Monday, August 12, 2019 10:15 PM > To: Dong, Eric <eric.d...@intel.com>; devel@edk2.groups.io > Cc: Ni, Ray <ray...@intel.com> > Subject: Re: [edk2-devel] [Patch v2 0/6] Add "test then write" mechanism. > > 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 (#45489): https://edk2.groups.io/g/devel/message/45489 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] -=-=-=-=-=-=-=-=-=-=-=-
