Eric: Is this a bug fix or new feature? Dose it catch to this 201908 stable tag?
Thanks Liming >-----Original Message----- >From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of >Dong, Eric >Sent: Tuesday, August 13, 2019 10:29 AM >To: devel@edk2.groups.io; ler...@redhat.com >Cc: Ni, Ray <ray...@intel.com> >Subject: Re: [edk2-devel] [Patch v2 0/6] Add "test then write" mechanism. > >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 (#45589): https://edk2.groups.io/g/devel/message/45589 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] -=-=-=-=-=-=-=-=-=-=-=-
