From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Tue, 13 Aug 2019 03:28:00 -0700 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A0E4EEC525; Tue, 13 Aug 2019 10:27:59 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-117-193.ams2.redhat.com [10.36.117.193]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9316E600C1; Tue, 13 Aug 2019 10:27:58 +0000 (UTC) Subject: Re: [edk2-devel] [Patch v2 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test Then Write" Macros. To: devel@edk2.groups.io, eric.dong@intel.com Cc: Ray Ni References: <20190812103152.35164-1-eric.dong@intel.com> <20190812103152.35164-2-eric.dong@intel.com> From: "Laszlo Ersek" Message-ID: Date: Tue, 13 Aug 2019 12:27:57 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20190812103152.35164-2-eric.dong@intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Tue, 13 Aug 2019 10:27:59 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 08/12/19 12:31, Dong, Eric wrote: > 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 > > Signed-off-by: Eric Dong > Cc: Ray Ni > Cc: Laszlo Ersek > --- > UefiCpuPkg/Include/AcpiCpuData.h | 1 + > .../Include/Library/RegisterCpuFeaturesLib.h | 77 +++++++++++++++++-- > .../RegisterCpuFeaturesLib.c | 14 +++- > 3 files changed, 80 insertions(+), 12 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..7e613d883e 100644 > --- a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h > +++ b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h > @@ -335,6 +335,7 @@ SwitchBspAfterFeaturesInitialize ( > @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. > **/ > @@ -345,7 +346,8 @@ CpuRegisterTableWrite ( > IN REGISTER_TYPE RegisterType, > IN UINT64 Index, > IN UINT64 ValueMask, > - IN UINT64 Value > + IN UINT64 Value, > + IN UINT8 TestThenWrite > ); > > /** > @@ -385,9 +387,45 @@ PreSmmCpuRegisterTableWrite ( > > @note This service could be called by BSP only. > **/ > -#define CPU_REGISTER_TABLE_WRITE32(ProcessorNumber, RegisterType, Index, Value) \ > - do { \ > - CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index, MAX_UINT32, Value); \ > +#define CPU_REGISTER_TABLE_WRITE32(ProcessorNumber, RegisterType, Index, Value) \ > + do { \ > + CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index, MAX_UINT32, Value, FALSE); \ > + } 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. > + > + @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 { \ > + CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index, MAX_UINT32, Value, TRUE); \ > + } 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. > + > + @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_WRITE64(ProcessorNumber, RegisterType, Index, Value) \ > + do { \ > + CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index, MAX_UINT64, Value, FALSE); \ > } while(FALSE); > > /** > @@ -403,9 +441,9 @@ PreSmmCpuRegisterTableWrite ( > > @note This service could be called by BSP only. > **/ > -#define CPU_REGISTER_TABLE_WRITE64(ProcessorNumber, RegisterType, Index, Value) \ > - do { \ > - CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index, MAX_UINT64, Value); \ > +#define CPU_REGISTER_TABLE_TEST_THEN_WRITE64(ProcessorNumber, RegisterType, Index, Value) \ > + do { \ > + CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index, MAX_UINT64, Value, TRUE); \ > } while(FALSE); > > /** > @@ -428,7 +466,30 @@ PreSmmCpuRegisterTableWrite ( > UINT64 ValueMask; \ > ValueMask = MAX_UINT64; \ > ((Type *)(&ValueMask))->Field = 0; \ > - CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index, ~ValueMask, Value); \ > + CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index, ~ValueMask, Value, FALSE); \ > + } 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. > + > + @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; \ > + CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index, ~ValueMask, Value, TRUE); \ > } while(FALSE); > > /** > diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c > index 67885bf69b..5d65b897ee 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++; > } > @@ -1085,6 +1089,7 @@ CpuRegisterTableWriteWorker ( > @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. > **/ > @@ -1095,7 +1100,8 @@ CpuRegisterTableWrite ( > IN REGISTER_TYPE RegisterType, > IN UINT64 Index, > IN UINT64 ValueMask, > - IN UINT64 Value > + IN UINT64 Value, > + IN UINT8 TestThenWrite > ) > { > UINT8 Start; > @@ -1105,7 +1111,7 @@ 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, TestThenWrite); > } > > /** > @@ -1139,7 +1145,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); > } > > /** > Acked-by: Laszlo Ersek