Re: [edk2-devel] [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test Then Write" Macros.
Eric:
>-Original Message-
>From: Dong, Eric
>Sent: Friday, August 16, 2019 10:20 AM
>To: Zeng, Star ; devel@edk2.groups.io
>Cc: Ni, Ray ; Laszlo Ersek ; Yao,
>Jiewen ; Gao, Liming ;
>Kinney, Michael D
>Subject: RE: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test
>Then Write" Macros.
>
>
>
>> -Original Message-
>> From: Zeng, Star
>> Sent: Friday, August 16, 2019 10:08 AM
>> To: Dong, Eric ; devel@edk2.groups.io
>> Cc: Ni, Ray ; Laszlo Ersek ; Yao,
>> Jiewen ; Gao, Liming ;
>Kinney,
>> Michael D ; Zeng, Star
>> Subject: RE: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test
>> Then Write" Macros.
>>
>>
>>
>> > -Original Message-
>> > From: Dong, Eric
>> > Sent: Friday, August 16, 2019 9:27 AM
>> > To: Zeng, Star ; devel@edk2.groups.io
>> > Cc: Ni, Ray ; Laszlo Ersek
>> > Subject: RE: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add
>> > "Test Then Write" Macros.
>> >
>> > > -Original Message-
>> > > From: Zeng, Star
>> > > Sent: Friday, August 16, 2019 9:15 AM
>> > > 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.
>> > >
>> > >
>> > >
>> > > > -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.
>> > > >
>> > > > 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
>> > > >
>> > > > Also add below API:
>> > > > CpuRegisterTableTestThenWrite
>> > > >
>> > > > 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(-)
>> > > >
>> > > > 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?
>> >
>> > I'm not sure whether use the Reserved field is an correct approach, do
>> > you have samples which use the reserved fields?
>> > But I think add new field is a more safe one.
>>
>> What "more safe" means here? Adding new field extends the structure,
>from
>> the structure layout view of point, it is an incompatible change, the
>> structure
>> size is not just from 24 to 25 bytes, but to be nature aligned, the structure
>size
>> will be (24 + 8) 32. Since there is Reserved field can be reused, that size
>impact
>> can be removed.
>
>Yes, agree the size impact. I'm just not clear whether the Reserved field can
>be used. Whether it has compatible impact.
>
>>
>> FspGlobalData.h:
>> SHA-1: a2e61f341d26a78751b2f19b5004c6bbfc8b4fa9
>> * IntelFsp2Pkg: Support FSP Dispatch mode
>>
>> MemoryProfile.h
>> SHA-1: 072a3ca1d36a42aec97f871c808776ee7038ca06 (related to
>> 94092aa60341a3e4b1e1ea7c362781b8404ac538)
>> * MdeModulePkg MemoryProfile.h:two bytes of Reserved[4] as
>> ActionStringOffset
>>
>
>If so many examples already did it. I think we can also do it.
>
>>
>> If needed, more comments from Ray, Laszlo, Jiewen, Liming and Mike will
>be
>> better.
>
>Yes, I think we need get more comments from them.
I think to reuse the reserved field is a good solution.
Thanks
Liming
>
>Thanks,
>Eric
>>
>>
>> Thanks,
>> Star
>>
>> >
>> > Thanks,
>> > Eric
>> > >
>> > > Thanks,
>> > > Star
>> > >
>> > > > } CPU_REGISTER_TABLE_ENTRY;
>> > > >
>> > > > //
>> > > > 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 @@
Re: [edk2-devel] [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test Then Write" Macros.
> -Original Message-
> From: Zeng, Star
> Sent: Friday, August 16, 2019 10:08 AM
> To: Dong, Eric ; devel@edk2.groups.io
> Cc: Ni, Ray ; Laszlo Ersek ; Yao,
> Jiewen ; Gao, Liming ; Kinney,
> Michael D ; Zeng, Star
> Subject: RE: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test
> Then Write" Macros.
>
>
>
> > -Original Message-
> > From: Dong, Eric
> > Sent: Friday, August 16, 2019 9:27 AM
> > To: Zeng, Star ; devel@edk2.groups.io
> > Cc: Ni, Ray ; Laszlo Ersek
> > Subject: RE: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add
> > "Test Then Write" Macros.
> >
> > > -Original Message-
> > > From: Zeng, Star
> > > Sent: Friday, August 16, 2019 9:15 AM
> > > 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.
> > >
> > >
> > >
> > > > -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.
> > > >
> > > > 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
> > > >
> > > > Also add below API:
> > > > CpuRegisterTableTestThenWrite
> > > >
> > > > 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(-)
> > > >
> > > > 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?
> >
> > I'm not sure whether use the Reserved field is an correct approach, do
> > you have samples which use the reserved fields?
> > But I think add new field is a more safe one.
>
> What "more safe" means here? Adding new field extends the structure, from
> the structure layout view of point, it is an incompatible change, the
> structure
> size is not just from 24 to 25 bytes, but to be nature aligned, the structure
> size
> will be (24 + 8) 32. Since there is Reserved field can be reused, that size
> impact
> can be removed.
Yes, agree the size impact. I'm just not clear whether the Reserved field can
be used. Whether it has compatible impact.
>
> FspGlobalData.h:
> SHA-1: a2e61f341d26a78751b2f19b5004c6bbfc8b4fa9
> * IntelFsp2Pkg: Support FSP Dispatch mode
>
> MemoryProfile.h
> SHA-1: 072a3ca1d36a42aec97f871c808776ee7038ca06 (related to
> 94092aa60341a3e4b1e1ea7c362781b8404ac538)
> * MdeModulePkg MemoryProfile.h:two bytes of Reserved[4] as
> ActionStringOffset
>
If so many examples already did it. I think we can also do it.
>
> If needed, more comments from Ray, Laszlo, Jiewen, Liming and Mike will be
> better.
Yes, I think we need get more comments from them.
Thanks,
Eric
>
>
> Thanks,
> Star
>
> >
> > Thanks,
> > Eric
> > >
> > > Thanks,
> > > Star
> > >
> > > > } CPU_REGISTER_TABLE_ENTRY;
> > > >
> > > > //
> > > > 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
> > > >);
> > > >
> > > > +/**
> > > > + 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
> > >
Re: [edk2-devel] [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test Then Write" Macros.
> -Original Message-
> From: Dong, Eric
> Sent: Friday, August 16, 2019 9:27 AM
> To: Zeng, Star ; devel@edk2.groups.io
> Cc: Ni, Ray ; Laszlo Ersek
> Subject: RE: [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test
> Then Write" Macros.
>
> > -Original Message-
> > From: Zeng, Star
> > Sent: Friday, August 16, 2019 9:15 AM
> > 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.
> >
> >
> >
> > > -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.
> > >
> > > 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
> > >
> > > Also add below API:
> > > CpuRegisterTableTestThenWrite
> > >
> > > 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(-)
> > >
> > > 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?
>
> I'm not sure whether use the Reserved field is an correct approach, do you
> have samples which use the reserved fields?
> But I think add new field is a more safe one.
What "more safe" means here? Adding new field extends the structure, from the
structure layout view of point, it is an incompatible change, the structure
size is not just from 24 to 25 bytes, but to be nature aligned, the structure
size will be (24 + 8) 32. Since there is Reserved field can be reused, that
size impact can be removed.
FspGlobalData.h:
SHA-1: a2e61f341d26a78751b2f19b5004c6bbfc8b4fa9
* IntelFsp2Pkg: Support FSP Dispatch mode
MemoryProfile.h
SHA-1: 072a3ca1d36a42aec97f871c808776ee7038ca06 (related to
94092aa60341a3e4b1e1ea7c362781b8404ac538)
* MdeModulePkg MemoryProfile.h:two bytes of Reserved[4] as ActionStringOffset
If needed, more comments from Ray, Laszlo, Jiewen, Liming and Mike will be
better.
Thanks,
Star
>
> Thanks,
> Eric
> >
> > Thanks,
> > Star
> >
> > > } CPU_REGISTER_TABLE_ENTRY;
> > >
> > > //
> > > 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
> > >);
> > >
> > > +/**
> > > + 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] IndexIndex of the register to program
> > > + @param[in] ValueMaskMask of bits in register to write
> > > + @param[in] ValueValue 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.
> > >
> > > @@ -390,6 +416,26 @@ PreSmmCpuRegisterTableWrite (
> > > CpuRegisterTableWrite (ProcessorNumber,
Re: [edk2-devel] [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test Then Write" Macros.
> -Original Message-
> From: Zeng, Star
> Sent: Friday, August 16, 2019 9:15 AM
> 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.
>
>
>
> > -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.
> >
> > 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
> >
> > Also add below API:
> > CpuRegisterTableTestThenWrite
> >
> > 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(-)
> >
> > 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?
I'm not sure whether use the Reserved field is an correct approach, do you have
samples which use the reserved fields?
But I think add new field is a more safe one.
Thanks,
Eric
>
> Thanks,
> Star
>
> > } CPU_REGISTER_TABLE_ENTRY;
> >
> > //
> > 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
> >);
> >
> > +/**
> > + 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] IndexIndex of the register to program
> > + @param[in] ValueMaskMask of bits in register to write
> > + @param[in] ValueValue 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.
> >
> > @@ -390,6 +416,26 @@ PreSmmCpuRegisterTableWrite (
> > CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index,
> > MAX_UINT32, Value); \
> >} 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.
> > +
> > + 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] IndexIndex of the register to program
> > + @param[in] ValueValue 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.
> >
> > @@ -408,6 +454,26 @@ PreSmmCpuRegisterTableWrite (
> > CpuRegisterTableWrite
Re: [edk2-devel] [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test Then Write" Macros.
> -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.
>
> 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
>
> Also add below API:
> CpuRegisterTableTestThenWrite
>
> 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(-)
>
> 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;
>
> //
> 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
>);
>
> +/**
> + 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] IndexIndex of the register to program
> + @param[in] ValueMaskMask of bits in register to write
> + @param[in] ValueValue 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.
>
> @@ -390,6 +416,26 @@ PreSmmCpuRegisterTableWrite (
> CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index,
> MAX_UINT32, Value); \
>} 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.
> +
> + 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] IndexIndex of the register to program
> + @param[in] ValueValue 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.
>
> @@ -408,6 +454,26 @@ PreSmmCpuRegisterTableWrite (
> CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index,
> MAX_UINT64, Value); \
>} 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.
> +
> + 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] IndexIndex of the register to program
> + @param[in] ValueValue to write
> +
> + @note This service could be called by BSP only.
> +**/
> +#define
[edk2-devel] [Patch v3 1/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add "Test Then Write" Macros.
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
Also add below API:
CpuRegisterTableTestThenWrite
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(-)
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..5bd464b32e 100644
--- a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
+++ b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
@@ -348,6 +348,32 @@ CpuRegisterTableWrite (
IN UINT64 Value
);
+/**
+ 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] IndexIndex of the register to program
+ @param[in] ValueMaskMask of bits in register to write
+ @param[in] ValueValue 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.
@@ -390,6 +416,26 @@ PreSmmCpuRegisterTableWrite (
CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index, MAX_UINT32,
Value); \
} 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.
+
+ 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] IndexIndex of the register to program
+ @param[in] ValueValue 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.
@@ -408,6 +454,26 @@ PreSmmCpuRegisterTableWrite (
CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index, MAX_UINT64,
Value); \
} 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.
+
+ 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] IndexIndex of the register to program
+ @param[in] ValueValue 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.
@@ -431,6 +497,31 @@ PreSmmCpuRegisterTableWrite (
CpuRegisterTableWrite (ProcessorNumber, RegisterType, Index, ~ValueMask,
Value); \
} while(FALSE);
+/**
+ Adds a bit field write entry in specified register table.
+
+
