From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.93, mailfrom: star.zeng@intel.com) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by groups.io with SMTP; Thu, 15 Aug 2019 18:14:57 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 15 Aug 2019 18:14:56 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,391,1559545200"; d="scan'208";a="167910601" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by orsmga007.jf.intel.com with ESMTP; 15 Aug 2019 18:14:56 -0700 Received: from shsmsx108.ccr.corp.intel.com (10.239.4.97) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 15 Aug 2019 18:14:55 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.19]) by SHSMSX108.ccr.corp.intel.com ([169.254.8.163]) with mapi id 14.03.0439.000; Fri, 16 Aug 2019 09:14:54 +0800 From: "Zeng, Star" To: "Dong, Eric" , "devel@edk2.groups.io" CC: "Ni, Ray" , Laszlo Ersek , "Zeng, Star" Subject: Re: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test Then Write" Macros. Thread-Topic: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test Then Write" Macros. Thread-Index: AQHVUxQ9kwBsvrRy3UiDtjlrIrWkGqb8+K7w Date: Fri, 16 Aug 2019 01:14:53 +0000 Message-ID: <0C09AFA07DD0434D9E2A0C6AEB0483104039C6A0@shsmsx102.ccr.corp.intel.com> References: <20190815025036.6780-1-eric.dong@intel.com> <20190815025036.6780-2-eric.dong@intel.com> In-Reply-To: <20190815025036.6780-2-eric.dong@intel.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: star.zeng@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > -----Original Message----- > From: Dong, Eric > Sent: Thursday, August 15, 2019 10:51 AM > To: devel@edk2.groups.io > Cc: Ni, Ray ; Laszlo Ersek ; Zeng, > Star > Subject: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test Then > Write" Macros. >=20 > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2040 >=20 > 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 >=20 > Also add below API: > CpuRegisterTableTestThenWrite >=20 > Signed-off-by: Eric Dong > Cc: Ray Ni > Cc: Laszlo Ersek > Cc: Star Zeng > --- > UefiCpuPkg/Include/AcpiCpuData.h | 1 + > .../Include/Library/RegisterCpuFeaturesLib.h | 91 +++++++++++++++++++ > .../RegisterCpuFeaturesLib.c | 45 ++++++++- > 3 files changed, 134 insertions(+), 3 deletions(-) >=20 > 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; >=20 > // > 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 > ); >=20 > +/** > + 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. >=20 > @@ -390,6 +416,26 @@ PreSmmCpuRegisterTableWrite ( > CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index, > MAX_UINT32, Value); \ > } while(FALSE); >=20 > +/** > + 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. >=20 > @@ -408,6 +454,26 @@ PreSmmCpuRegisterTableWrite ( > CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index, > MAX_UINT64, Value); \ > } while(FALSE); >=20 > +/** > + 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. >=20 > @@ -431,6 +497,31 @@ PreSmmCpuRegisterTableWrite ( > CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index, > ~ValueMask, Value); \ > } while(FALSE); >=20 > +/** > + 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 structur= e. > + @param[in] Field The bit fiel name in register structure t= o 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 =3D MAX_UINT64; = \ > + ((Type *)(&ValueMask))->Field =3D 0; = \ > + CpuRegisterTableTestThenWrite (ProcessorNumber, RegisterType, Index, > ~ValueMask, Value); \ > + } while(FALSE); > + > /** > Adds a 32-bit register write entry in specified register table. >=20 > 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 =3D > ValidBitStart; > RegisterTableEntry[RegisterTable->TableLength].ValidBitLength =3D > ValidBitLength; > RegisterTableEntry[RegisterTable->TableLength].Value =3D Valu= e; > + RegisterTableEntry[RegisterTable->TableLength].TestThenWrite =3D > + TestThenWrite; >=20 > RegisterTable->TableLength++; > } > @@ -1105,7 +1109,42 @@ CpuRegisterTableWrite ( > Start =3D (UINT8)LowBitSet64 (ValueMask); > End =3D (UINT8)HighBitSet64 (ValueMask); > Length =3D 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 =3D (UINT8)LowBitSet64 (ValueMask); > + End =3D (UINT8)HighBitSet64 (ValueMask); > + Length =3D End - Start + 1; > + CpuRegisterTableWriteWorker (FALSE, ProcessorNumber, RegisterType, > + Index, Start, Length, Value, TRUE); > } >=20 > /** > @@ -1139,7 +1178,7 @@ PreSmmCpuRegisterTableWrite ( > Start =3D (UINT8)LowBitSet64 (ValueMask); > End =3D (UINT8)HighBitSet64 (ValueMask); > Length =3D End - Start + 1; > - CpuRegisterTableWriteWorker (TRUE, ProcessorNumber, RegisterType, > Index, Start, Length, Value); > + CpuRegisterTableWriteWorker (TRUE, ProcessorNumber, RegisterType, > + Index, Start, Length, Value, FALSE); > } >=20 > /** > -- > 2.21.0.windows.1