Re: [edk2-devel] [Patch V2 14/14] UefiCpuPkg: Modify UnitTest code since tested API is changed
> @@ -413,8 +422,9 @@ CompareEntrysforOnePoint (
>//
>for (Index = 0; Index < MapCount; Index++) {
> if ((Address >= Map[Index].LinearAddress) && (Address <
> (Map[Index].LinearAddress + Map[Index].Length))) {
> - AttributeInMap.Uint64=
> (Map[Index].Attribute.Uint64 &
> mSupportedBit.Uint64);
> - AttributeInMap.Bits.PageTableBaseAddress = ((Address -
> Map[Index].LinearAddress) >> 12) +
> Map[Index].Attribute.Bits.PageTableBaseAddress;
> + AttributeInMap.Uint64 = (Map[Index].Attribute.Uint64 &
> mSupportedBit.Uint64);
> + AttributeInMap.Uint64 &=
> (~IA32_MAP_ATTRIBUTE_PAGE_TABLE_BASE_ADDRESS_MASK);
> + AttributeInMap.Uint64 |= (Address - Map[Index].LinearAddress +
> IA32_MAP_ATTRIBUTE_PAGE_TABLE_BASE_ADDRESS
> (&Map[Index].Attribute)) &
> IA32_MAP_ATTRIBUTE_PAGE_TABLE_BASE_ADDRESS_MASK;
1. "& IA32_MAP_ATTRIBUTE_PAGE_TABLE_BASE_ADDRESS_MASK" is not needed.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101203): https://edk2.groups.io/g/devel/message/101203
Mute This Topic: https://groups.io/mt/97469491/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V2 13/14] UefiCpuPkg: Fix IA32 build failure in CpuPageTableLib.inf
> +UINT32PageTableBaseAddressLow : 19; // Page Table Base Address > High 1. Comments say "High". Should be "Low". > + > > - MapMask.Bits.PageTableBaseAddress = 1; > - MapMask.Bits.Present = 1; > - MapMask.Bits.ReadWrite= 1; > + MapMask.Bits.PageTableBaseAddressLow = 1; > + MapMask.Bits.Present = 1; > + MapMask.Bits.ReadWrite = 1; 2. Can you please create a separate patch to initialize MapMask to 0? Missing the initialization doesn't cause functionality issue but looks confusing. > >PageTable = 0; >PageTableBufferSize = 0; > -- > 2.31.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#101202): https://edk2.groups.io/g/devel/message/101202 Mute This Topic: https://groups.io/mt/97469489/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V2 12/14] UefiCpuPkg/CpuPageTableLib: Add RandomTest for PAE paging
Can you please check that the reserved fields in 4 PDPTE entries are set to 0
in the test?
> -Original Message-
> From: Tan, Dun
> Sent: Wednesday, March 8, 2023 6:08 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray ; Kumar,
> Rahul R ; Gerd Hoffmann
> Subject: [Patch V2 12/14] UefiCpuPkg/CpuPageTableLib: Add RandomTest
> for PAE paging
>
> Add RandomTest for PAE paging.
>
> Signed-off-by: Dun Tan
> Cc: Eric Dong
> Cc: Ray Ni
> Cc: Rahul Kumar
> Cc: Gerd Hoffmann
> ---
>
> UefiCpuPkg/Library/CpuPageTableLib/UnitTest/CpuPageTableLibUnitTestHo
> st.c | 2 ++
> UefiCpuPkg/Library/CpuPageTableLib/UnitTest/RandomTest.c |
> 3 +-
> -
> UefiCpuPkg/Library/CpuPageTableLib/UnitTest/TestHelper.c |
> 5 ++-
> --
> 3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git
> a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/CpuPageTableLibUnitTest
> Host.c
> b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/CpuPageTableLibUnitTest
> Host.c
> index 3df6436af3..06a8fd3c02 100644
> ---
> a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/CpuPageTableLibUnitTest
> Host.c
> +++
> b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/CpuPageTableLibUnitTest
> Host.c
> @@ -9,6 +9,7 @@
> #include "CpuPageTableLibUnitTest.h"
>
> // ---
> PageMode--
> TestCount-TestRangeCount---RandomOptions
> +static CPU_PAGE_TABLE_LIB_RANDOM_TEST_CONTEXT
> mTestContextPagingPae = { PagingPae, 100, 20, USE_RANDOM_ARRAY };
> static CPU_PAGE_TABLE_LIB_RANDOM_TEST_CONTEXT
> mTestContextPaging4Level= { Paging4Level, 100, 20,
> USE_RANDOM_ARRAY };
> static CPU_PAGE_TABLE_LIB_RANDOM_TEST_CONTEXT
> mTestContextPaging4Level1GB = { Paging4Level1GB, 100, 20,
> USE_RANDOM_ARRAY };
> static CPU_PAGE_TABLE_LIB_RANDOM_TEST_CONTEXT
> mTestContextPaging5Level= { Paging5Level, 100, 20,
> USE_RANDOM_ARRAY };
> @@ -865,6 +866,7 @@ UefiTestMain (
> goto EXIT;
>}
>
> + AddTestCase (RandomTestCase, "Random Test for PagingPae", "Random
> Test Case1", TestCaseforRandomTest, NULL, NULL,
> &mTestContextPagingPae);
>AddTestCase (RandomTestCase, "Random Test for Paging4Level", "Random
> Test Case1", TestCaseforRandomTest, NULL, NULL,
> &mTestContextPaging4Level);
>AddTestCase (RandomTestCase, "Random Test for Paging4Level1G",
> "Random Test Case2", TestCaseforRandomTest, NULL, NULL,
> &mTestContextPaging4Level1GB);
>AddTestCase (RandomTestCase, "Random Test for Paging5Level", "Random
> Test Case3", TestCaseforRandomTest, NULL, NULL,
> &mTestContextPaging5Level);
> diff --git a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/RandomTest.c
> b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/RandomTest.c
> index 8f8f0a5a9f..a7f45fb175 100644
> --- a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/RandomTest.c
> +++ b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/RandomTest.c
> @@ -251,10 +251,9 @@ ValidateAndRandomeModifyPageTable (
>UNIT_TEST_STATUS Status;
>IA32_PAGING_ENTRY *PagingEntry;
>
> - if ((PagingMode == Paging32bit) || (PagingMode == PagingPae) ||
> (PagingMode >= PagingModeMax)) {
> + if ((PagingMode == Paging32bit) || (PagingMode >= PagingModeMax)) {
> //
> // 32bit paging is never supported.
> -// PAE paging will be supported later.
> //
> return UNIT_TEST_ERROR_TEST_FAILED;
>}
> diff --git a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/TestHelper.c
> b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/TestHelper.c
> index 11f7e607ca..614bd6bbf1 100644
> --- a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/TestHelper.c
> +++ b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/TestHelper.c
> @@ -175,10 +175,9 @@ IsPageTableValid (
> return UNIT_TEST_PASSED;
>}
>
> - if ((PagingMode == Paging32bit) || (PagingMode == PagingPae) ||
> (PagingMode >= PagingModeMax)) {
> + if ((PagingMode == Paging32bit) || (PagingMode >= PagingModeMax)) {
> //
> // 32bit paging is never supported.
> -// PAE paging will be supported later.
> //
> return UNIT_TEST_ERROR_TEST_FAILED;
>}
> @@ -264,7 +263,7 @@ GetEntryFromPageTable (
>UINT64 Index;
>IA32_PAGING_ENTRY *PagingEntry;
>
> - if ((PagingMode == Paging32bit) || (PagingMode == PagingPae) ||
> (PagingMode >= PagingModeMax)) {
> + if ((PagingMode == Paging32bit) || (PagingMode >= PagingModeMax)) {
> //
> // 32bit paging is never supported.
> // PAE paging will be supported later.
> --
> 2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101201): https://edk2.groups.io/g/devel/message/101201
Mute This Topic: https://groups.io/mt/97469488/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V2 11/14] UefiCpuPkg/CpuPageTableLib: Enable PAE paging
> +if (PagingMode == PagingPae) {
> + //
> + // These fields of PAE paging PDPTE should be 0 according to SDM.
> + //
1. can you update comments to explain such as:
"These fields are treated as ReadWrite, by the common map logic. So they
might be set to 1."
> + TopPagingEntry.PdptePae.Bits.MustBeZero = 0;
> + TopPagingEntry.PdptePae.Bits.MustBeZero2 = 0;
2. Above code sets the CR3 value. You should update the 4 PDPTE entries.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101200): https://edk2.groups.io/g/devel/message/101200
Mute This Topic: https://groups.io/mt/97469487/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V2 10/14] UefiCpuPkg/CpuPageTableLib: Modify RandomTest to check IsModified
>
>GenerateSingleRandomMapEntry (MaxAddress, MapEntrys);
>Status = PageTableParse (*PageTable, PagingMode, NULL, &MapCount);
>
> + if (MapCount != 0) {
> +//
> +// Allocate memory for Map
> +// Note the memory is only used in this one Single MapEntry Test
> +//
> +UT_ASSERT_EQUAL (Status, RETURN_BUFFER_TOO_SMALL);
> +Map = AllocatePages (EFI_SIZE_TO_PAGES (MapCount * sizeof
> (IA32_MAP_ENTRY)));
> +ASSERT (Map != NULL);
> +Status = PageTableParse (*PageTable, PagingMode, Map, &MapCount);
> + }
> +
>//
>// Check if the generated MapEntrys->Maps[MapsIndex] contains not-
> present range.
>//
>if (MapEntrys->Maps[MapsIndex].Length > 0) {
> if (MapCount != 0) {
> - UT_ASSERT_EQUAL (Status, RETURN_BUFFER_TOO_SMALL);
> - Map = AllocatePages (EFI_SIZE_TO_PAGES (MapCount * sizeof
> (IA32_MAP_ENTRY)));
> - ASSERT (Map != NULL);
> - Status = PageTableParse (*PageTable, PagingMode, Map, &MapCount);
> -
>if (Map[MapCount - 1].LinearAddress + Map[MapCount - 1].Length <
> MapEntrys->Maps[MapsIndex].LinearAddress + MapEntrys-
> >Maps[MapsIndex].Length) {
> IsNotPresent = TRUE;
1. can you split the Parse() API call into a standalone patch?
> +if (MapCount2 == 0) {
> + UT_ASSERT_EQUAL (IsModified, FALSE);
2. no need to treat "MapCount == 0" as special. CompareMem() should be able to
accept 0-length bytes.
> +} else if (CompareMem (Map, Map2, MapCount2 * sizeof
> (IA32_MAP_ENTRY)) != 0) {
> + UT_ASSERT_EQUAL (IsModified, TRUE);
> +} else {
> + UT_ASSERT_EQUAL (IsModified, FALSE);
> +}
>}
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101199): https://edk2.groups.io/g/devel/message/101199
Mute This Topic: https://groups.io/mt/97469486/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V2 09/14] UefiCpuPkg/CpuPageTableLib: Add OUTPUT IsModified parameter.
> + IA32_PAGING_ENTRY ParentPagingEntryContent;
1. how about "OriginalParentPagingEntry"?
> + IA32_PAGING_ENTRY PrevLeafPagingEntryContent;
2. how about "OriginalCurrentPagingEntry"?
>
> + //
> + // Check if ParentPagingEntry entry is modified.
> + //
3. Can you add more comments to explain why checking parent entry content is
enough?
> + if (ParentPagingEntryContent.Uint64 != ParentPagingEntry->Uint64) {
> +if (IsModified != NULL) {
4. Can you always pass a non-NULL IsModified to MapInLevel()?
5. By the way, it's good that this patch doesn't enhance test case to test
IsModified return value.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101198): https://edk2.groups.io/g/devel/message/101198
Mute This Topic: https://groups.io/mt/97469484/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V2 07/14] UefiCpuPkg/CpuPageTableLib:Modify RandomTest to check Mask/Attr
>
> -/**
> - Return a random boolean.
> -
> - @return boolean
> -**/
> -BOOLEAN
> -RandomBoolean (
> - VOID
> - )
> -{
> - BOOLEAN Value;
> -
> - LocalRandomBytes ((UINT8 *)&Value, sizeof (BOOLEAN));
> - return Value%2;
> -}
> -
> /**
>Return a 32bit random number.
>
> @@ -139,6 +123,21 @@ Random64 (
>return (UINT64)(Value % (Limit - Start + 1)) + Start;
> }
>
> +/**
> + Returns true with the percentage of input Probability.
> +
> + @param[in] ProbabilityThe percentage to return true.
> +
> + @return boolean
> +**/
> +BOOLEAN
> +RandomBoolean (
> + UINT8 Probability
> + )
> +{
> + return ((Probability > ((UINT8)Random64 (0, 100))) ? TRUE : FALSE);
> +}
1. can you split the RandomBoolean() change in standalone patch?
2. can you simplify below code to only use for-loop?
> + //
> + if (MapEntrys->Maps[MapsIndex].Length > 0) {
> +if (MapCount != 0) {
> + UT_ASSERT_EQUAL (Status, RETURN_BUFFER_TOO_SMALL);
> + Map = AllocatePages (EFI_SIZE_TO_PAGES (MapCount * sizeof
> (IA32_MAP_ENTRY)));
> + ASSERT (Map != NULL);
> + Status = PageTableParse (*PageTable, PagingMode, Map, &MapCount);
> +
> + if (Map[MapCount - 1].LinearAddress + Map[MapCount - 1].Length <
> MapEntrys->Maps[MapsIndex].LinearAddress + MapEntrys-
> >Maps[MapsIndex].Length) {
> +IsNotPresent = TRUE;
> + } else {
> +for (Index = 0; Index < MapCount; Index++) {
> + if ((PreviousAddress < Map[Index].LinearAddress) &&
> + (MapEntrys->Maps[MapsIndex].LinearAddress <
> Map[Index].LinearAddress) &&
> + ((MapEntrys->Maps[MapsIndex].LinearAddress + MapEntrys-
> >Maps[MapsIndex].Length) > PreviousAddress))
> + {
> +//
> +// MapEntrys->Maps[MapsIndex] contains not-present range in
> exsiting page table.
> +//
> +IsNotPresent = TRUE;
> +break;
> + }
> +
> + PreviousAddress = Map[Index].LinearAddress + Map[Index].Length;
> +}
> + }
> +} else {
> + IsNotPresent = TRUE;
> +}
> + }
>
>PageTableBufferSize = 0;
>Status = PageTableMap (
> @@ -638,6 +700,25 @@ SingleMapEntryTest (
>&MapEntrys->Maps[MapsIndex].Attribute,
>&MapEntrys->Maps[MapsIndex].Mask
>);
> +
> + //
> + // Return Status should be InvalidParameter when:
> + // 1. MapEntrys->Maps[MapsIndex] contains not-present range.
> + // 2. MapEntrys->Maps[MapsIndex].Mask contains zero value field or
> Attribute->Bits.Present is 0.
> + //
> + Attribute = &MapEntrys->Maps[MapsIndex].Attribute;
> + Mask = &MapEntrys->Maps[MapsIndex].Mask;
> + if (((Attribute->Bits.Present == 0) || (Mask->Bits.Present == 0) || (Mask-
> >Bits.ReadWrite == 0) ||
> + (Mask->Bits.UserSupervisor == 0) || (Mask->Bits.WriteThrough == 0) ||
> (Mask->Bits.CacheDisabled == 0) ||
> + (Mask->Bits.Accessed == 0) || (Mask->Bits.Dirty == 0) || (Mask-
> >Bits.Pat == 0) || (Mask->Bits.Global == 0) ||
> + (Mask->Bits.PageTableBaseAddress == 0) || (Mask->Bits.ProtectionKey
> == 0) || (Mask->Bits.Nx == 0)) &&
> + IsNotPresent)
> + {
> +RemoveLastMapEntry (MapEntrys);
> +UT_ASSERT_EQUAL (Status, RETURN_INVALID_PARAMETER);
> +return UNIT_TEST_PASSED;
> + }
> +
>if (PageTableBufferSize != 0) {
> UT_ASSERT_EQUAL (Status, RETURN_BUFFER_TOO_SMALL);
>
> diff --git a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/TestHelper.c
> b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/TestHelper.c
> index 5bd70c0f65..11f7e607ca 100644
> --- a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/TestHelper.c
> +++ b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/TestHelper.c
> @@ -171,6 +171,10 @@ IsPageTableValid (
>UNIT_TEST_STATUS Status;
>IA32_PAGING_ENTRY *PagingEntry;
>
> + if (PageTable == 0) {
> +return UNIT_TEST_PASSED;
> + }
> +
>if ((PagingMode == Paging32bit) || (PagingMode == PagingPae) ||
> (PagingMode >= PagingModeMax)) {
> //
> // 32bit paging is never supported.
> --
> 2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101197): https://edk2.groups.io/g/devel/message/101197
Mute This Topic: https://groups.io/mt/97469479/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V2 08/14] UefiCpuPkg/CpuPageTableLib: Enable non-1:1 mapping in random test
Reviewed-by: Ray Ni
> -Original Message-
> From: Tan, Dun
> Sent: Wednesday, March 8, 2023 6:08 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray ; Kumar,
> Rahul R ; Gerd Hoffmann
> Subject: [Patch V2 08/14] UefiCpuPkg/CpuPageTableLib: Enable non-1:1
> mapping in random test
>
> Enable non-1:1 mapping in random test. In previous test, non-1:1
> test will fail due to the non-1:1 mapping issue in CpuPageTableLib
> and invalid Input Mask when creating new page table or mapping
> not-present range. Now these issue have been fixed.
>
> Signed-off-by: Dun Tan
> Cc: Eric Dong
> Cc: Ray Ni
> Cc: Rahul Kumar
> Cc: Gerd Hoffmann
> ---
>
> UefiCpuPkg/Library/CpuPageTableLib/UnitTest/CpuPageTableLibUnitTestHo
> st.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git
> a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/CpuPageTableLibUnitTest
> Host.c
> b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/CpuPageTableLibUnitTest
> Host.c
> index fe00a7f632..6f27411d4b 100644
> ---
> a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/CpuPageTableLibUnitTest
> Host.c
> +++
> b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/CpuPageTableLibUnitTest
> Host.c
> @@ -9,10 +9,10 @@
> #include "CpuPageTableLibUnitTest.h"
>
> // ---
> PageMode--
> TestCount-TestRangeCount---RandomOptions
> -static CPU_PAGE_TABLE_LIB_RANDOM_TEST_CONTEXT
> mTestContextPaging4Level= { Paging4Level, 100, 20,
> ONLY_ONE_ONE_MAPPING|USE_RANDOM_ARRAY };
> -static CPU_PAGE_TABLE_LIB_RANDOM_TEST_CONTEXT
> mTestContextPaging4Level1GB = { Paging4Level1GB, 100, 20,
> ONLY_ONE_ONE_MAPPING|USE_RANDOM_ARRAY };
> -static CPU_PAGE_TABLE_LIB_RANDOM_TEST_CONTEXT
> mTestContextPaging5Level= { Paging5Level, 100, 20,
> ONLY_ONE_ONE_MAPPING|USE_RANDOM_ARRAY };
> -static CPU_PAGE_TABLE_LIB_RANDOM_TEST_CONTEXT
> mTestContextPaging5Level1GB = { Paging5Level1GB, 100, 20,
> ONLY_ONE_ONE_MAPPING|USE_RANDOM_ARRAY };
> +static CPU_PAGE_TABLE_LIB_RANDOM_TEST_CONTEXT
> mTestContextPaging4Level= { Paging4Level, 100, 20,
> USE_RANDOM_ARRAY };
> +static CPU_PAGE_TABLE_LIB_RANDOM_TEST_CONTEXT
> mTestContextPaging4Level1GB = { Paging4Level1GB, 100, 20,
> USE_RANDOM_ARRAY };
> +static CPU_PAGE_TABLE_LIB_RANDOM_TEST_CONTEXT
> mTestContextPaging5Level= { Paging5Level, 100, 20,
> USE_RANDOM_ARRAY };
> +static CPU_PAGE_TABLE_LIB_RANDOM_TEST_CONTEXT
> mTestContextPaging5Level1GB = { Paging5Level1GB, 100, 20,
> USE_RANDOM_ARRAY };
>
> /**
>Check if the input parameters are not supported.
> --
> 2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101196): https://edk2.groups.io/g/devel/message/101196
Mute This Topic: https://groups.io/mt/97469483/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V2 06/14] UefiCpuPkg/CpuPageTableLib: Add manual test to check Mask and Attr
Please update test case to not expect failure when setting a non-present range
as non-present .
> -Original Message-
> From: Tan, Dun
> Sent: Wednesday, March 8, 2023 6:08 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray ; Kumar,
> Rahul R ; Gerd Hoffmann
> Subject: [Patch V2 06/14] UefiCpuPkg/CpuPageTableLib: Add manual test to
> check Mask and Attr
>
> Add manual test case to check input Mask and Attribute. When
> creating new page table or mapping not-present range in existing
> page table, all the non-reserved fields of Mask and Present bit of
> Attribute should not be 0.
>
> Signed-off-by: Dun Tan
> Cc: Eric Dong
> Cc: Ray Ni
> Cc: Rahul Kumar
> Cc: Gerd Hoffmann
> ---
>
> UefiCpuPkg/Library/CpuPageTableLib/UnitTest/CpuPageTableLibUnitTestHo
> st.c | 110
> ++
> +++-
> 1 file changed, 109 insertions(+), 1 deletion(-)
>
> diff --git
> a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/CpuPageTableLibUnitTest
> Host.c
> b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/CpuPageTableLibUnitTest
> Host.c
> index 3014a03243..fe00a7f632 100644
> ---
> a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/CpuPageTableLibUnitTest
> Host.c
> +++
> b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/CpuPageTableLibUnitTest
> Host.c
> @@ -697,6 +697,114 @@ TestCaseManualChangeNx (
>return UNIT_TEST_PASSED;
> }
>
> +/**
> + Check if the input Mask and Attribute is expected when creating new page
> table or
> + map not-present range in existing page table.
> +
> + @param[in] Context[Optional] An optional parameter that enables:
> + 1) test-case reuse with varied parameters and
> + 2) test-case re-entry for Target tests that need a
> + reboot. This parameter is a VOID* and it is the
> + responsibility of the test author to ensure that the
> + contents are well understood by all test cases that
> may
> + consume it.
> +
> + @retval UNIT_TEST_PASSED The Unit test has completed and the
> test
> +case was successful.
> + @retval UNIT_TEST_ERROR_TEST_FAILED A test case assertion has failed.
> +**/
> +UNIT_TEST_STATUS
> +EFIAPI
> +TestCaseToCheckMapMaskAndAttr (
> + IN UNIT_TEST_CONTEXT Context
> + )
> +{
> + UINTN PageTable;
> + PAGING_MODE PagingMode;
> + VOID*Buffer;
> + UINTN PageTableBufferSize;
> + IA32_MAP_ATTRIBUTE MapAttribute;
> + IA32_MAP_ATTRIBUTE ExpectedMapAttribute;
> + IA32_MAP_ATTRIBUTE MapMask;
> + RETURN_STATUS Status;
> + IA32_MAP_ENTRY *Map;
> + UINTN MapCount;
> +
> + PagingMode= Paging4Level;
> + PageTableBufferSize = 0;
> + PageTable = 0;
> + Buffer= NULL;
> + MapAttribute.Uint64 = 0;
> + MapAttribute.Bits.Present = 1;
> + MapMask.Uint64= 0;
> + MapMask.Bits.Present = 1;
> + //
> + // Create Page table to cover [0, 1G]. All fields of MapMask should be set.
> + //
> + Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, 0, SIZE_1GB, &MapAttribute, &MapMask);
> + UT_ASSERT_EQUAL (Status, RETURN_INVALID_PARAMETER);
> + MapMask.Uint64 = MAX_UINT64;
> + Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, 0, SIZE_1GB, &MapAttribute, &MapMask);
> + UT_ASSERT_EQUAL (Status, RETURN_BUFFER_TOO_SMALL);
> + Buffer = AllocatePages (EFI_SIZE_TO_PAGES (PageTableBufferSize));
> + Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, 0, SIZE_1GB, &MapAttribute, &MapMask);
> + UT_ASSERT_EQUAL (Status, RETURN_SUCCESS);
> +
> + //
> + // Update Page table to cover [1G, 2G - 8K]. All fields of MapMask should
> be set and Present bit of MapAttribute should be 1.
> + //
> + PageTableBufferSize = 0;
> + MapAttribute.Uint64 = SIZE_1GB;
> + MapAttribute.Bits.Present = 1;
> + MapMask.Uint64= 0;
> + MapMask.Bits.Present = 1;
> + Status= PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, SIZE_1GB, SIZE_1GB - SIZE_8KB, &MapAttribute,
> &MapMask);
> + UT_ASSERT_EQUAL (Status, RETURN_INVALID_PARAMETER);
> + MapMask.Uint64 = MAX_UINT64;
> + Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, SIZE_1GB, SIZE_1GB - SIZE_8KB, &MapAttribute,
> &MapMask);
> + UT_ASSERT_EQUAL (Status, RETURN_BUFFER_TOO_SMALL);
> + Buffer = AllocatePages (EFI_SIZE_TO_PAGES (PageTableBufferSize));
> + Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, SIZE_1GB, SIZE_1GB - SIZE_8KB, &MapAttribute,
> &MapMask);
> + UT_ASSERT_EQUAL (Status, RETURN_SUCCESS);
> +
> + //
> + // Update Page table to cover [2G - 8K, 2G] and
Re: [edk2-devel] [Patch V2 05/14] UefiCpuPkg/CpuPageTebleLib: Check Mask and Attr in PageTableMap
> +**/
> +RETURN_STATUS
> +CheckMaskAndAttrForNotPresentEntry (
> + IN IA32_MAP_ATTRIBUTE *Attribute,
> + IN IA32_MAP_ATTRIBUTE *Mask
> + )
> +{
> + if ((Attribute->Bits.Present == 0) || (Mask->Bits.Present == 0) || (Mask-
> >Bits.ReadWrite == 0) ||
1. I think we can allow caller to set a not-present range as not-present.
Even though it's meaningless😊
So, I think we can remove the Attribute.Present check.
2. The function name can be more readable: how about
IsAllAttributesSetForNonPresentEntry()?
> + (Mask->Bits.UserSupervisor == 0) || (Mask->Bits.WriteThrough == 0) ||
> (Mask->Bits.CacheDisabled == 0) ||
> + (Mask->Bits.Accessed == 0) || (Mask->Bits.Dirty == 0) || (Mask-
> >Bits.Pat == 0) || (Mask->Bits.Global == 0) ||
> + (Mask->Bits.PageTableBaseAddress == 0) || (Mask->Bits.ProtectionKey
> == 0) || (Mask->Bits.Nx == 0))
> + {
> +return RETURN_INVALID_PARAMETER;
> + }
> +
> + return RETURN_SUCCESS;
> +}
> +PagingEntry = (IA32_PAGING_ENTRY
> *)(UINTN)IA32_PNLE_PAGE_TABLE_BASE_ADDRESS (&ParentPagingEntry-
> >Pnle);
> +PagingEntryIndexLimit = (BitFieldRead64 (LinearAddress + Length - 1,
> BitStart + 9, 63) > BitFieldRead64 (LinearAddress + Offset, BitStart + 9,
> 63)) ?
> 511 :
3. Can you add more comments for the code here?
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101194): https://edk2.groups.io/g/devel/message/101194
Mute This Topic: https://groups.io/mt/97469476/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2] MdePkg/BaseCacheMaintenanceLib: RISC-V: Fix instruction cache not been invalidated
On Fri, Mar 10, 2023 at 01:50:19PM -0800, Tuan Phan wrote: > When the range instruction cache invalidating not supported, the whole > instruction cache should be invalidated instead. > > Signed-off-by: Tuan Phan > --- > V2: > - Format with uncrustify. > Reviewed-by: Sunil V L -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#101193): https://edk2.groups.io/g/devel/message/101193 Mute This Topic: https://groups.io/mt/97530656/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V2 04/14] UefiCpuPkg/CpuPageTableLib: Fix issue when splitting leaf entry
> + > + ParentPagingEntry->Uintn = (UINTN)(VOID *)PagingEntry; Only address field is set but attributes are cleared to zeros. Then following code sets the attributes. Still a hole at this point, right? > >// >// Set NOP attributes > @@ -363,12 +370,6 @@ PageTableLibMapInLevel ( >// will make the entire region read-only even the child entries > set the > RW bit. >// >PageTableLibSetPnle (&ParentPagingEntry->Pnle, &NopAttribute, > &AllOneMask); -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#101192): https://edk2.groups.io/g/devel/message/101192 Mute This Topic: https://groups.io/mt/97469475/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V2 02/14] UefiCpuPkg/CpuPageTableLib: Add check for input Length
Thanks Ray. Will update the corresponding function header comments in next
version.
Thanks,
Dun
-Original Message-
From: Ni, Ray
Sent: Wednesday, March 15, 2023 9:25 AM
To: Tan, Dun ; devel@edk2.groups.io
Cc: Dong, Eric ; Kumar, Rahul R ;
Gerd Hoffmann
Subject: RE: [Patch V2 02/14] UefiCpuPkg/CpuPageTableLib: Add check for input
Length
The function header comments in lib header and C file should be updated as well
to document a new condition when success is returned.
> -Original Message-
> From: Tan, Dun
> Sent: Wednesday, March 8, 2023 6:08 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray ;
> Kumar, Rahul R ; Gerd Hoffmann
>
> Subject: [Patch V2 02/14] UefiCpuPkg/CpuPageTableLib: Add check for
> input Length
>
> Add check for input Length in PageTableMap (). Return RETURN_SUCCESS
> when input Length is 0.
>
> Signed-off-by: Dun Tan
> Cc: Eric Dong
> Cc: Ray Ni
> Cc: Rahul Kumar
> Cc: Gerd Hoffmann
> ---
> UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> index 47027917d9..4c9d70fa0a 100644
> --- a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> +++ b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> @@ -567,6 +567,10 @@ PageTableMap (
>IA32_PAGE_LEVEL MaxLeafLevel;
>IA32_MAP_ATTRIBUTE ParentAttribute;
>
> + if (Length == 0) {
> +return RETURN_SUCCESS;
> + }
> +
>if ((PagingMode == Paging32bit) || (PagingMode == PagingPae) ||
> (PagingMode >= PagingModeMax)) {
> //
> // 32bit paging is never supported.
> --
> 2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101191): https://edk2.groups.io/g/devel/message/101191
Mute This Topic: https://groups.io/mt/97469467/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V2 01/14] UefiCpuPkg/CpuPageTableLib: Remove unneeded 'if' condition
Thanks Ray. Will update the copy right year in next version patch.
Thanks,
Dun
-Original Message-
From: Ni, Ray
Sent: Wednesday, March 15, 2023 9:24 AM
To: Tan, Dun ; devel@edk2.groups.io
Cc: Dong, Eric ; Kumar, Rahul R ;
Gerd Hoffmann
Subject: RE: [Patch V2 01/14] UefiCpuPkg/CpuPageTableLib: Remove unneeded 'if'
condition
You can carry my Reviewed-by in next version if you add the copy right year
change.
> -Original Message-
> From: Tan, Dun
> Sent: Wednesday, March 8, 2023 6:08 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray ;
> Kumar, Rahul R ; Gerd Hoffmann
>
> Subject: [Patch V2 01/14] UefiCpuPkg/CpuPageTableLib: Remove unneeded
> 'if' condition
>
> Remove unneeded 'if' condition in CpuPageTableLib code.
> The deleted code is in the code branch for present non-leaf parent
> entry. So the 'if' check for (ParentPagingEntry->Pnle.Bits.Present
> == 0) is always FALSE.
>
> Signed-off-by: Dun Tan
> Cc: Eric Dong
> Cc: Ray Ni
> Cc: Rahul Kumar
> Cc: Gerd Hoffmann
> ---
> UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c | 9 -
> 1 file changed, 9 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> index 37713ec659..47027917d9 100644
> --- a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> +++ b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> @@ -375,15 +375,6 @@ PageTableLibMapInLevel (
> //we need to change PDPTE[0].ReadWrite = 1 and let all PDE[0-
> 255].ReadWrite = 0 in this step.
> // when PDPTE[0].Nx = 1 but caller wants to map [0-2MB] as Nx = 0
> (PDT[0].Nx = 0)
> //we need to change PDPTE[0].Nx = 0 and let all
> PDE[0-255].Nx = 1 in
> this step.
> -if ((ParentPagingEntry->Pnle.Bits.Present == 0) && (Mask->Bits.Present
> == 1) && (Attribute->Bits.Present == 1)) {
> - if (Modify) {
> -ParentPagingEntry->Pnle.Bits.Present = 1;
> - }
> -
> - ChildAttribute.Bits.Present = 0;
> - ChildMask.Bits.Present = 1;
> -}
> -
> if ((ParentPagingEntry->Pnle.Bits.ReadWrite == 0) && (Mask-
> >Bits.ReadWrite == 1) && (Attribute->Bits.ReadWrite == 1)) {
>if (Modify) {
> ParentPagingEntry->Pnle.Bits.ReadWrite = 1;
> --
> 2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101190): https://edk2.groups.io/g/devel/message/101190
Mute This Topic: https://groups.io/mt/97469465/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V2 03/14] UefiCpuPkg/CpuPageTableLib: Fix the non-1:1 mapping issue
Okay, I'll split this patch into 2 patches in next version
Thanks,
Dun
-Original Message-
From: Ni, Ray
Sent: Wednesday, March 15, 2023 9:28 AM
To: Tan, Dun ; devel@edk2.groups.io
Cc: Dong, Eric ; Kumar, Rahul R ;
Gerd Hoffmann
Subject: RE: [Patch V2 03/14] UefiCpuPkg/CpuPageTableLib: Fix the non-1:1
mapping issue
Dun,
Can you split this patch to 2 patches?
One to move some local variable initialization to the beginning of the function.
The other to fix the bug. So the bug fix changes look smaller.
Thanks,
Ray
> -Original Message-
> From: Tan, Dun
> Sent: Wednesday, March 8, 2023 6:08 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray ;
> Kumar, Rahul R ; Gerd Hoffmann
>
> Subject: [Patch V2 03/14] UefiCpuPkg/CpuPageTableLib: Fix the non-1:1
> mapping issue
>
> In previous code logic, when splitting a leaf parent entry to smaller
> granularity child page table, if the parent entry
> Attribute&Mask(without PageTableBaseAddress field) is equal to the
> input attribute&mask(without PageTableBaseAddress field), the split
> process won't happen. This may lead to failure in non-1:1 mapping.
>
> For example, there is a page table in which [0, 1G] is mapped(Lv4[0]
> ,Lv3[0,0], a non-leaf level4 entry and a leaf level3 entry). And we
> want to remap [0, 2M] linear address range to [1G, 1G + 2M] with the
> same attibute. The expected behaviour should be: split Lv3[0,0] entry
> into 512 level2 entries and remap the first level2 entry to cover [0,
> 2M]. But the split won't happen in previous code since
> PageTableBaseAddress of input Attribute is not checked.
>
> So, when checking if a leaf parent entry needs to be splitted, we
> should also check if PageTableBaseAddress calculated by parent entry
> is equal to the value caculated by input attribute.
>
> Signed-off-by: Dun Tan
> Cc: Eric Dong
> Cc: Ray Ni
> Cc: Rahul Kumar
> Cc: Gerd Hoffmann
> ---
> UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c | 27
> +--
> 1 file changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> index 4c9d70fa0a..ee27238edb 100644
> --- a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> +++ b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> @@ -258,6 +258,7 @@ PageTableLibMapInLevel (
>UINTN BitStart;
>UINTN Index;
>IA32_PAGING_ENTRY *PagingEntry;
> + UINTN PagingEntryIndex;
>IA32_PAGING_ENTRY *CurrentPagingEntry;
>UINT64 RegionLength;
>UINT64 SubLength;
> @@ -288,6 +289,13 @@ PageTableLibMapInLevel (
>LocalParentAttribute.Uint64 = ParentAttribute->Uint64;
>ParentAttribute = &LocalParentAttribute;
>
> + //
> + // RegionLength: 256T (1 << 48) 512G (1 << 39), 1G (1 << 30), 2M (1
> + << 21)
> or 4K (1 << 12).
> + //
> + BitStart = 12 + (Level - 1) * 9;
> + PagingEntryIndex = (UINTN)BitFieldRead64 (LinearAddress + Offset,
> BitStart, BitStart + 9 - 1);
> + RegionLength = REGION_LENGTH (Level);
> +
>//
>// ParentPagingEntry ONLY is deferenced for checking Present and
> MustBeOne bits
>// when Modify is FALSE.
> @@ -325,8 +333,11 @@ PageTableLibMapInLevel (
> // the actual attributes of grand-parents when determing the
> memory type.
> //
> PleBAttribute.Uint64 = PageTableLibGetPleBMapAttribute
> (&ParentPagingEntry->PleB, ParentAttribute);
> -if ((IA32_MAP_ATTRIBUTE_ATTRIBUTES (&PleBAttribute) &
> IA32_MAP_ATTRIBUTE_ATTRIBUTES (Mask))
> -== (IA32_MAP_ATTRIBUTE_ATTRIBUTES (Attribute) &
> IA32_MAP_ATTRIBUTE_ATTRIBUTES (Mask)))
> +if IA32_MAP_ATTRIBUTE_ATTRIBUTES (&PleBAttribute) &
> IA32_MAP_ATTRIBUTE_ATTRIBUTES (Mask))
> + == (IA32_MAP_ATTRIBUTE_ATTRIBUTES (Attribute) &
> IA32_MAP_ATTRIBUTE_ATTRIBUTES (Mask &&
> +( (Mask->Bits.PageTableBaseAddress == 0)
> +|| ((IA32_MAP_ATTRIBUTE_PAGE_TABLE_BASE_ADDRESS
> (&PleBAttribute) + PagingEntryIndex * RegionLength)
> +== (IA32_MAP_ATTRIBUTE_PAGE_TABLE_BASE_ADDRESS
> + (Attribute)
> + Offset
> {
>//
>// This function is called when the memory length is less than
> the region length of the parent level.
> @@ -353,8 +364,7 @@ PageTableLibMapInLevel (
>//
>PageTableLibSetPnle (&ParentPagingEntry->Pnle, &NopAttribute,
> &AllOneMask);
>
> - RegionLength = REGION_LENGTH (Level);
> - PagingEntry = (IA32_PAGING_ENTRY
> *)(UINTN)IA32_PNLE_PAGE_TABLE_BASE_ADDRESS (&ParentPagingEntry-
> >Pnle);
> + PagingEntry = (IA32_PAGING_ENTRY
> *)(UINTN)IA32_PNLE_PAGE_TABLE_BASE_ADDRESS (&ParentPagingEntry-
> >Pnle);
>for (SubOffset = 0, Index = 0; Index < 512; Index++) {
> PagingEntry[Index].Uint64 = OneOfPagingEntry.Uint64 + SubOffset;
> SubOffset+= RegionLength;
> @@ -425,14 +435,11 @@
Re: [edk2-devel] [Patch V2 03/14] UefiCpuPkg/CpuPageTableLib: Fix the non-1:1 mapping issue
Dun,
Can you split this patch to 2 patches?
One to move some local variable initialization to the beginning of the function.
The other to fix the bug. So the bug fix changes look smaller.
Thanks,
Ray
> -Original Message-
> From: Tan, Dun
> Sent: Wednesday, March 8, 2023 6:08 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray ; Kumar,
> Rahul R ; Gerd Hoffmann
> Subject: [Patch V2 03/14] UefiCpuPkg/CpuPageTableLib: Fix the non-1:1
> mapping issue
>
> In previous code logic, when splitting a leaf parent entry to
> smaller granularity child page table, if the parent entry
> Attribute&Mask(without PageTableBaseAddress field) is equal to the
> input attribute&mask(without PageTableBaseAddress field), the split
> process won't happen. This may lead to failure in non-1:1 mapping.
>
> For example, there is a page table in which [0, 1G] is mapped(Lv4[0]
> ,Lv3[0,0], a non-leaf level4 entry and a leaf level3 entry). And we
> want to remap [0, 2M] linear address range to [1G, 1G + 2M] with the
> same attibute. The expected behaviour should be: split Lv3[0,0]
> entry into 512 level2 entries and remap the first level2 entry to
> cover [0, 2M]. But the split won't happen in previous code since
> PageTableBaseAddress of input Attribute is not checked.
>
> So, when checking if a leaf parent entry needs to be splitted, we
> should also check if PageTableBaseAddress calculated by parent entry
> is equal to the value caculated by input attribute.
>
> Signed-off-by: Dun Tan
> Cc: Eric Dong
> Cc: Ray Ni
> Cc: Rahul Kumar
> Cc: Gerd Hoffmann
> ---
> UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c | 27
> +--
> 1 file changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> index 4c9d70fa0a..ee27238edb 100644
> --- a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> +++ b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> @@ -258,6 +258,7 @@ PageTableLibMapInLevel (
>UINTN BitStart;
>UINTN Index;
>IA32_PAGING_ENTRY *PagingEntry;
> + UINTN PagingEntryIndex;
>IA32_PAGING_ENTRY *CurrentPagingEntry;
>UINT64 RegionLength;
>UINT64 SubLength;
> @@ -288,6 +289,13 @@ PageTableLibMapInLevel (
>LocalParentAttribute.Uint64 = ParentAttribute->Uint64;
>ParentAttribute = &LocalParentAttribute;
>
> + //
> + // RegionLength: 256T (1 << 48) 512G (1 << 39), 1G (1 << 30), 2M (1 << 21)
> or 4K (1 << 12).
> + //
> + BitStart = 12 + (Level - 1) * 9;
> + PagingEntryIndex = (UINTN)BitFieldRead64 (LinearAddress + Offset,
> BitStart, BitStart + 9 - 1);
> + RegionLength = REGION_LENGTH (Level);
> +
>//
>// ParentPagingEntry ONLY is deferenced for checking Present and
> MustBeOne bits
>// when Modify is FALSE.
> @@ -325,8 +333,11 @@ PageTableLibMapInLevel (
> // the actual attributes of grand-parents when determing the memory
> type.
> //
> PleBAttribute.Uint64 = PageTableLibGetPleBMapAttribute
> (&ParentPagingEntry->PleB, ParentAttribute);
> -if ((IA32_MAP_ATTRIBUTE_ATTRIBUTES (&PleBAttribute) &
> IA32_MAP_ATTRIBUTE_ATTRIBUTES (Mask))
> -== (IA32_MAP_ATTRIBUTE_ATTRIBUTES (Attribute) &
> IA32_MAP_ATTRIBUTE_ATTRIBUTES (Mask)))
> +if IA32_MAP_ATTRIBUTE_ATTRIBUTES (&PleBAttribute) &
> IA32_MAP_ATTRIBUTE_ATTRIBUTES (Mask))
> + == (IA32_MAP_ATTRIBUTE_ATTRIBUTES (Attribute) &
> IA32_MAP_ATTRIBUTE_ATTRIBUTES (Mask &&
> +( (Mask->Bits.PageTableBaseAddress == 0)
> +|| ((IA32_MAP_ATTRIBUTE_PAGE_TABLE_BASE_ADDRESS
> (&PleBAttribute) + PagingEntryIndex * RegionLength)
> +== (IA32_MAP_ATTRIBUTE_PAGE_TABLE_BASE_ADDRESS (Attribute)
> + Offset
> {
>//
>// This function is called when the memory length is less than the
> region
> length of the parent level.
> @@ -353,8 +364,7 @@ PageTableLibMapInLevel (
>//
>PageTableLibSetPnle (&ParentPagingEntry->Pnle, &NopAttribute,
> &AllOneMask);
>
> - RegionLength = REGION_LENGTH (Level);
> - PagingEntry = (IA32_PAGING_ENTRY
> *)(UINTN)IA32_PNLE_PAGE_TABLE_BASE_ADDRESS (&ParentPagingEntry-
> >Pnle);
> + PagingEntry = (IA32_PAGING_ENTRY
> *)(UINTN)IA32_PNLE_PAGE_TABLE_BASE_ADDRESS (&ParentPagingEntry-
> >Pnle);
>for (SubOffset = 0, Index = 0; Index < 512; Index++) {
> PagingEntry[Index].Uint64 = OneOfPagingEntry.Uint64 + SubOffset;
> SubOffset+= RegionLength;
> @@ -425,14 +435,11 @@ PageTableLibMapInLevel (
>}
>
>//
> - // RegionLength: 256T (1 << 48) 512G (1 << 39), 1G (1 << 30), 2M (1 << 21)
> or
> 4K (1 << 12).
>// RegionStart: points to the linear address that's aligned on
> RegionLength
> and lower than (LinearAddress + Offset).
>//
> - BitStart = 12 + (Level - 1) * 9;
> - Index= (UINTN)BitFieldRea
Re: [edk2-devel] [Patch V2 01/14] UefiCpuPkg/CpuPageTableLib: Remove unneeded 'if' condition
You can carry my Reviewed-by in next version if you add the copy right year
change.
> -Original Message-
> From: Tan, Dun
> Sent: Wednesday, March 8, 2023 6:08 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray ; Kumar,
> Rahul R ; Gerd Hoffmann
> Subject: [Patch V2 01/14] UefiCpuPkg/CpuPageTableLib: Remove unneeded
> 'if' condition
>
> Remove unneeded 'if' condition in CpuPageTableLib code.
> The deleted code is in the code branch for present non-leaf parent
> entry. So the 'if' check for (ParentPagingEntry->Pnle.Bits.Present
> == 0) is always FALSE.
>
> Signed-off-by: Dun Tan
> Cc: Eric Dong
> Cc: Ray Ni
> Cc: Rahul Kumar
> Cc: Gerd Hoffmann
> ---
> UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c | 9 -
> 1 file changed, 9 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> index 37713ec659..47027917d9 100644
> --- a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> +++ b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> @@ -375,15 +375,6 @@ PageTableLibMapInLevel (
> //we need to change PDPTE[0].ReadWrite = 1 and let all PDE[0-
> 255].ReadWrite = 0 in this step.
> // when PDPTE[0].Nx = 1 but caller wants to map [0-2MB] as Nx = 0
> (PDT[0].Nx = 0)
> //we need to change PDPTE[0].Nx = 0 and let all
> PDE[0-255].Nx = 1 in
> this step.
> -if ((ParentPagingEntry->Pnle.Bits.Present == 0) && (Mask->Bits.Present
> == 1) && (Attribute->Bits.Present == 1)) {
> - if (Modify) {
> -ParentPagingEntry->Pnle.Bits.Present = 1;
> - }
> -
> - ChildAttribute.Bits.Present = 0;
> - ChildMask.Bits.Present = 1;
> -}
> -
> if ((ParentPagingEntry->Pnle.Bits.ReadWrite == 0) && (Mask-
> >Bits.ReadWrite == 1) && (Attribute->Bits.ReadWrite == 1)) {
>if (Modify) {
> ParentPagingEntry->Pnle.Bits.ReadWrite = 1;
> --
> 2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101187): https://edk2.groups.io/g/devel/message/101187
Mute This Topic: https://groups.io/mt/97469465/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V2 02/14] UefiCpuPkg/CpuPageTableLib: Add check for input Length
The function header comments in lib header and C file should be updated as well
to document a new condition when success is returned.
> -Original Message-
> From: Tan, Dun
> Sent: Wednesday, March 8, 2023 6:08 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray ; Kumar,
> Rahul R ; Gerd Hoffmann
> Subject: [Patch V2 02/14] UefiCpuPkg/CpuPageTableLib: Add check for input
> Length
>
> Add check for input Length in PageTableMap (). Return
> RETURN_SUCCESS when input Length is 0.
>
> Signed-off-by: Dun Tan
> Cc: Eric Dong
> Cc: Ray Ni
> Cc: Rahul Kumar
> Cc: Gerd Hoffmann
> ---
> UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> index 47027917d9..4c9d70fa0a 100644
> --- a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> +++ b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> @@ -567,6 +567,10 @@ PageTableMap (
>IA32_PAGE_LEVEL MaxLeafLevel;
>IA32_MAP_ATTRIBUTE ParentAttribute;
>
> + if (Length == 0) {
> +return RETURN_SUCCESS;
> + }
> +
>if ((PagingMode == Paging32bit) || (PagingMode == PagingPae) ||
> (PagingMode >= PagingModeMax)) {
> //
> // 32bit paging is never supported.
> --
> 2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101186): https://edk2.groups.io/g/devel/message/101186
Mute This Topic: https://groups.io/mt/97469467/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V2 01/14] UefiCpuPkg/CpuPageTableLib: Remove unneeded 'if' condition
Dun,
The copyright year needs to change to 2023.
Code logic change is good to me.
Thanks,
Ray
> -Original Message-
> From: Tan, Dun
> Sent: Wednesday, March 8, 2023 6:08 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray ; Kumar,
> Rahul R ; Gerd Hoffmann
> Subject: [Patch V2 01/14] UefiCpuPkg/CpuPageTableLib: Remove unneeded
> 'if' condition
>
> Remove unneeded 'if' condition in CpuPageTableLib code.
> The deleted code is in the code branch for present non-leaf parent
> entry. So the 'if' check for (ParentPagingEntry->Pnle.Bits.Present
> == 0) is always FALSE.
>
> Signed-off-by: Dun Tan
> Cc: Eric Dong
> Cc: Ray Ni
> Cc: Rahul Kumar
> Cc: Gerd Hoffmann
> ---
> UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c | 9 -
> 1 file changed, 9 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> index 37713ec659..47027917d9 100644
> --- a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> +++ b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> @@ -375,15 +375,6 @@ PageTableLibMapInLevel (
> //we need to change PDPTE[0].ReadWrite = 1 and let all PDE[0-
> 255].ReadWrite = 0 in this step.
> // when PDPTE[0].Nx = 1 but caller wants to map [0-2MB] as Nx = 0
> (PDT[0].Nx = 0)
> //we need to change PDPTE[0].Nx = 0 and let all
> PDE[0-255].Nx = 1 in
> this step.
> -if ((ParentPagingEntry->Pnle.Bits.Present == 0) && (Mask->Bits.Present
> == 1) && (Attribute->Bits.Present == 1)) {
> - if (Modify) {
> -ParentPagingEntry->Pnle.Bits.Present = 1;
> - }
> -
> - ChildAttribute.Bits.Present = 0;
> - ChildMask.Bits.Present = 1;
> -}
> -
> if ((ParentPagingEntry->Pnle.Bits.ReadWrite == 0) && (Mask-
> >Bits.ReadWrite == 1) && (Attribute->Bits.ReadWrite == 1)) {
>if (Modify) {
> ParentPagingEntry->Pnle.Bits.ReadWrite = 1;
> --
> 2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101185): https://edk2.groups.io/g/devel/message/101185
Mute This Topic: https://groups.io/mt/97469465/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] Event: TianoCore Bug Triage - APAC / NAMO - Tuesday, March 14, 2023 #cal-reminder
*Reminder: TianoCore Bug Triage - APAC / NAMO* *When:* Tuesday, March 14, 2023 6:30pm to 7:30pm (UTC-07:00) America/Los Angeles *Where:* https://teams.microsoft.com/l/meetup-join/19%3ameeting_OTk1YzJhN2UtOGQwNi00NjY4LWEwMTktY2JiODRlYTY1NmY0%40thread.v2/0?context=%7b%22Tid%22%3a%2246c98d88-e344-4ed4-8496-4ed7712e255d%22%2c%22Oid%22%3a%226e4ce4c4-1242-431b-9a51-92cd01a5df3c%22%7d *Organizer:* Liming Gao gaolim...@byosoft.com.cn ( gaolim...@byosoft.com.cn?subject=Re:%20Event:%20TianoCore%20Bug%20Triage%20-%20APAC%20%2F%20NAMO ) View Event ( https://edk2.groups.io/g/devel/viewevent?eventid=1753764 ) *Description:* TianoCore Bug Triage - APAC / NAMO Hosted by Liming Gao Microsoft Teams meeting *Join on your computer or mobile app* Click here to join the meeting ( https://teams.microsoft.com/l/meetup-join/19%3ameeting_OTk1YzJhN2UtOGQwNi00NjY4LWEwMTktY2JiODRlYTY1NmY0%40thread.v2/0?context=%7b%22Tid%22%3a%2246c98d88-e344-4ed4-8496-4ed7712e255d%22%2c%22Oid%22%3a%226e4ce4c4-1242-431b-9a51-92cd01a5df3c%22%7d ) *Join with a video conferencing device* te...@conf.intel.com Video Conference ID: 116 062 094 0 Alternate VTC dialing instructions ( https://conf.intel.com/teams/?conf=1160620940&ivr=teams&d=conf.intel.com&test=test_call ) *Or call in (audio only)* +1 916-245-6934,,77463821# ( tel:+19162456934,,77463821# ) United States, Sacramento Phone Conference ID: 774 638 21# Find a local number ( https://dialin.teams.microsoft.com/d195d438-2daa-420e-b9ea-da26f9d1d6d5?id=77463821 ) | Reset PIN ( https://mysettings.lync.com/pstnconferencing ) Learn More ( https://aka.ms/JoinTeamsMeeting ) | Meeting options ( https://teams.microsoft.com/meetingOptions/?organizerId=b286b53a-1218-4db3-bfc9-3d4c5aa7669e&tenantId=46c98d88-e344-4ed4-8496-4ed7712e255d&threadId=19_meeting_OTUyZTg2NjgtNDhlNS00ODVlLTllYTUtYzg1OTNjNjdiZjFh@thread.v2&messageId=0&language=en-US ) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#101184): https://edk2.groups.io/g/devel/message/101184 Mute This Topic: https://groups.io/mt/97595810/21656 Mute #cal-reminder:https://edk2.groups.io/g/devel/mutehashtag/cal-reminder Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel][PATCH V1 1/1] BaseTools: Generate compile information in build report
I saw the code was merged to edk2-basetools repo 5 days ago. When will the change be in edk2 repo? I am really interested in using the json files😊 Thanks, Ray > -Original Message- > From: Palomino Sosa, Guillermo A > Sent: Monday, March 6, 2023 11:41 PM > To: Kinney, Michael D ; Feng, Bob C > ; Gao, Liming ; > devel@edk2.groups.io > Cc: Chen, Christine ; Ni, Ray ; > Oram, Isaac W ; Sean Brogan > ; a...@kernel.org > Subject: RE: [edk2-devel][PATCH V1 1/1] BaseTools: Generate compile > information in build report > > Hi guys, can we submit the pull request or do you have comments on it? > https://github.com/tianocore/edk2-basetools/pull/88 > > Thanks > > > -Original Message- > From: Palomino Sosa, Guillermo A > Sent: Tuesday, February 28, 2023 10:00 PM > To: Ni, Ray ; Kinney, Michael D > ; devel@edk2.groups.io; a...@kernel.org > Cc: Chen, Christine ; Feng, Bob C > ; Gao, Liming ; Oram, > Isaac W ; Sean Brogan > > Subject: RE: [edk2-devel][PATCH V1 1/1] BaseTools: Generate compile > information in build report > > It takes same time as original build report to be generated as it constructed > using the same data structures as build report. So I think its OK to not have > it > enabled by default. > > Patch is ready in the pull request to be reviews. > > > > -Original Message- > From: Ni, Ray > Sent: Tuesday, February 28, 2023 6:52 PM > To: Palomino Sosa, Guillermo A ; > Kinney, Michael D ; devel@edk2.groups.io; > a...@kernel.org > Cc: Chen, Christine ; Feng, Bob C > ; Gao, Liming ; Oram, > Isaac W ; Sean Brogan > > Subject: RE: [edk2-devel][PATCH V1 1/1] BaseTools: Generate compile > information in build report > > What's the status of this patch? > Does report generation take time? If no, why not generate them by default > without individual flag control. > I really like the feature to generate "compile_commands.json" > > > -Original Message- > > From: Palomino Sosa, Guillermo A > > Sent: Tuesday, February 28, 2023 7:42 AM > > To: Kinney, Michael D ; > > devel@edk2.groups.io; a...@kernel.org > > Cc: Ni, Ray ; Chen, Christine > > ; Feng, Bob C ; Gao, > > Liming ; Oram, Isaac W > > ; Sean Brogan > > Subject: RE: [edk2-devel][PATCH V1 1/1] BaseTools: Generate compile > > information in build report > > > > I have updated the pull based on Sean feedback. I added following > > fields to > > module_report.json: > > * LibraryClass > > * ModuleEntryPointList > > * ConstructorList > > * DestructorList > > > > I have also added commit from Ard based on this request to fix build issue: > > https://github.com/tianocore/edk2-basetools/pull/88 > > > > Thanks > > > > -Original Message- > > From: Kinney, Michael D > > Sent: Monday, February 27, 2023 4:36 PM > > To: devel@edk2.groups.io; a...@kernel.org; Palomino Sosa, Guillermo A > > > > Cc: Ni, Ray ; Chen, Christine > > ; Feng, Bob C ; Gao, > > Liming ; Oram, Isaac W > > ; Sean Brogan ; > > Kinney, Michael D > > Subject: RE: [edk2-devel][PATCH V1 1/1] BaseTools: Generate compile > > information in build report > > > > Hi Guillermo, > > > > Can you please look at Ards PR and make sure his fix is included in your PR. > > > > Also, please work with Christine and Bob to see what is going on with > > the Code Coverage check. We do want it to be easy for all community > > members to submit change requests. We may need support from the > > edk2-bastools maintainers to help with CI issues and help with changes to > address. > > > > Thanks, > > > > Mike > > > > > -Original Message- > > > From: devel@edk2.groups.io On Behalf Of Ard > > > Biesheuvel > > > Sent: Monday, February 27, 2023 10:58 AM > > > To: devel@edk2.groups.io; Palomino Sosa, Guillermo A > > > > > > Cc: Ni, Ray ; Kinney, Michael D > > > ; Chen, Christine > > > ; Feng, Bob C ; Gao, > > > Liming ; Oram, Isaac W > > > ; Sean Brogan > > > Subject: Re: [edk2-devel][PATCH V1 1/1] BaseTools: Generate compile > > > information in build report > > > > > > On Mon, 27 Feb 2023 at 18:40, Guillermo Antonio Palomino Sosa > > > wrote: > > > > > > > > Hi. I have submitted a pull request to edk2-basetools repository: > > > > https://github.com/tianocore/edk2-basetools/pull/88 > > > > This is the feature request for it: > > > > https://github.com/tianocore/edk2-basetools/issues/87 > > > > I'm also attaching the patch here: > > > > (0001-BaseTools-Generate-compile-information-in-build-repo.patch) > > > > > > > > On a side note, seems like tip of edk2-basetools is broken due > > > > this > > commit that makes direct import of Common package: > > > > https://github.com/tianocore/edk2- > > basetools/commit/8e6018d3ea4c1aae7 > > > > 185f589d129cea14a5d89fd > > > > edk2-basetools\edk2basetools\GenFds\SubTypeGuidSection.py: > > > > import Common.LongFilePathOs as os > > > > > > > > > > > > > > I sent a fix and a PR for this about a month ago: > > > > > > https://github.com/tianocore/edk2-basetools/pull/86 > > > > > > but CodeCov seems to take issue with it, for reaso
Re: [edk2-devel] [PATCH 2/2] MdePkg: Update code to be more C11 compliant by using __func__
Reviewed-by: Michael D Kinney
> -Original Message-
> From: devel@edk2.groups.io On Behalf Of Rebecca Cran
> Sent: Monday, March 13, 2023 7:20 PM
> To: devel@edk2.groups.io; Kinney, Michael D ;
> Gao, Liming ;
> Liu, Zhiguang
> Subject: Re: [edk2-devel] [PATCH 2/2] MdePkg: Update code to be more C11
> compliant by using __func__
>
> Mike,
>
>
> I think your concerns with this patch were addressed?
>
> Could you add a Reviewed-by please?
>
>
> Thanks.
> Rebecca
>
>
> On 2/9/23 6:01 PM, Michael D Kinney wrote:
> > Hi Rebecca,
> >
> > Did this pass EDK II CI?
> >
> > This change does break EBC compiler builds. The following has to be added
> > to
> > MdePkg/Include/Ebc/ProcessorBind.h in order to use __func__ everywhere.
> >
> > #define __func__ __FUNCTION__
> >
> > I also see __FUNCTION__ used in many packages. I am wondering if we want
> > to do this clean up if it should be its own patch series and update all
> > packages in once series.
> >
> > Mike
> >
> >> -Original Message-
> >> From: devel@edk2.groups.io On Behalf Of Rebecca Cran
> >> Sent: Thursday, February 9, 2023 7:45 AM
> >> To: devel@edk2.groups.io; Kinney, Michael D ;
> >> Gao, Liming ;
> Liu, Zhiguang
> >>
> >> Cc: Rebecca Cran
> >> Subject: [edk2-devel] [PATCH 2/2] MdePkg: Update code to be more C11
> >> compliant by using __func__
> >>
> >> __FUNCTION__ is a pre-standard extension that gcc and Visual C++ among
> >> others support, while __func__ was standardized in C99.
> >>
> >> Since it's more standard, replace __FUNCTION__ with __func__ throughout
> >> MdePkg.
> >>
> >> Signed-off-by: Rebecca Cran
> >> ---
> >> MdePkg/Include/Library/PerformanceLib.h
> >> | 12 ++--
> >> MdePkg/Include/Library/UnitTestLib.h
> >> | 18 +--
> ---
> >> MdePkg/Library/BaseCacheMaintenanceLib/LoongArchCache.c
> >> | 6 +++---
> >> MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c
> >> | 12 ++--
> >> MdePkg/Library/BaseLib/SafeString.c
> >> | 2 +-
> >>
> >> MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.c
> >> | 4 ++--
> >> MdePkg/Library/DxeRngLib/DxeRngLib.c
> >> | 14 +++---
> >> 7 files changed, 34 insertions(+), 34 deletions(-)
> >>
> >> diff --git a/MdePkg/Include/Library/PerformanceLib.h
> >> b/MdePkg/Include/Library/PerformanceLib.h
> >> index 34ec956b9c0e..d0f2dfb070d5 100644
> >> --- a/MdePkg/Include/Library/PerformanceLib.h
> >> +++ b/MdePkg/Include/Library/PerformanceLib.h
> >> @@ -526,7 +526,7 @@ LogPerformanceMeasurement (
> >> #define PERF_EVENT_SIGNAL_BEGIN(EventGuid) \
> >> do { \
> >> if (LogPerformanceMeasurementEnabled (PERF_GENERAL_TYPE)) { \
> >> - LogPerformanceMeasurement (&gEfiCallerIdGuid, EventGuid,
> >> __FUNCTION__ , 0, PERF_EVENTSIGNAL_START_ID); \
> >> + LogPerformanceMeasurement (&gEfiCallerIdGuid, EventGuid, __func__ ,
> >> 0, PERF_EVENTSIGNAL_START_ID); \
> >> } \
> >> } while (FALSE)
> >>
> >> @@ -542,7 +542,7 @@ LogPerformanceMeasurement (
> >> #define PERF_EVENT_SIGNAL_END(EventGuid) \
> >> do { \
> >> if (LogPerformanceMeasurementEnabled (PERF_GENERAL_TYPE)) { \
> >> - LogPerformanceMeasurement (&gEfiCallerIdGuid, EventGuid,
> >> __FUNCTION__ , 0, PERF_EVENTSIGNAL_END_ID); \
> >> + LogPerformanceMeasurement (&gEfiCallerIdGuid, EventGuid, __func__ ,
> >> 0, PERF_EVENTSIGNAL_END_ID); \
> >> } \
> >> } while (FALSE)
> >>
> >> @@ -558,7 +558,7 @@ LogPerformanceMeasurement (
> >> #define PERF_CALLBACK_BEGIN(TriggerGuid) \
> >> do { \
> >> if (LogPerformanceMeasurementEnabled (PERF_GENERAL_TYPE)) { \
> >> - LogPerformanceMeasurement (&gEfiCallerIdGuid, TriggerGuid,
> >> __FUNCTION__ , 0, PERF_CALLBACK_START_ID); \
> >> + LogPerformanceMeasurement (&gEfiCallerIdGuid, TriggerGuid, __func__
> >> , 0, PERF_CALLBACK_START_ID); \
> >> } \
> >> } while (FALSE)
> >>
> >> @@ -574,7 +574,7 @@ LogPerformanceMeasurement (
> >> #define PERF_CALLBACK_END(TriggerGuid) \
> >> do { \
> >> if (LogPerformanceMeasurementEnabled (PERF_GENERAL_TYPE)) { \
> >> - LogPerformanceMeasurement (&gEfiCallerIdGuid, TriggerGuid,
> >> __FUNCTION__ , 0, PERF_CALLBACK_END_ID); \
> >> + LogPerformanceMeasurement (&gEfiCallerIdGuid, TriggerGuid, __func__
> >> , 0, PERF_CALLBACK_END_ID); \
> >> } \
> >> } while (FALSE)
> >>
> >> @@ -589,7 +589,7 @@ LogPerformanceMeasurement (
> >> #define PERF_FUNCTION_BEGIN() \
> >> do { \
> >> if (LogPerformanceMeasurementEnabled (PERF_GENERAL_TYPE)) { \
> >> - LogPerformanceMeasurement (&gEfiCallerIdGuid, NULL, __FUNCTION__ ,
> >> 0, PERF_FUNCTION_STAR
Re: [edk2-devel] [PATCH tianocore-docs 1/2] Readme.md: convert links from Gitbook to Github Pages
Reviewed-by: Michael D Kinney > -Original Message- > From: Rebecca Cran > Sent: Tuesday, March 14, 2023 1:05 PM > To: devel@edk2.groups.io; Ard Biesheuvel ; Feng, > Bob C ; Abner > Chang ; Justen, Jordan L ; > Gao, Liming ; > Leif Lindholm ; Michael Kubacki > ; Kinney, Michael D > ; Demeter, Miki ; Ni, Ray > ; Zimmer, Vincent > ; Dong, Eric > Cc: Rebecca Cran > Subject: [PATCH tianocore-docs 1/2] Readme.md: convert links from Gitbook to > Github Pages > > We no longer publish documentation to gitbook.com, instead > using tianocore-docs.github.io. Update the links to point > to the correct locations. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Rebecca Cran > --- > Readme.md | 117 +--- > 1 file changed, 52 insertions(+), 65 deletions(-) > > diff --git a/Readme.md b/Readme.md > index e6d35fa41131..1e4a5d25bd6f 100644 > --- a/Readme.md > +++ b/Readme.md > @@ -38,121 +38,108 @@ available when reading the **HTML** versions of the > documents. Document issues a > be entered in [Tianocore Bugzilla](https://bugzilla.tianocore.org). > > * **_EDK II Build Specification_** \[ > -[HTML > ](https://www.gitbook.com/read/book/edk2-docs/edk-ii-build-specification), > -[PDF > ](https://www.gitbook.com/download/pdf/book/edk2-docs/edk-ii-build-specification), > -[Mobi > ](https://www.gitbook.com/download/mobi/book/edk2-docs/edk-ii-build-specification), > -[ePub > ](https://www.gitbook.com/download/epub/book/edk2-docs/edk-ii-build-specification), > -[Gitbook](https://www.gitbook.com/book/edk2-docs/edk-ii-build-specification), > +[HTML ](https://tianocore-docs.github.io/edk2-BuildSpecification/draft/), > +[PDF > ](https://tianocore-docs.github.io/edk2-BuildSpecification/draft/edk2-BuildSpecification-draft.pdf), > +[Mobi > ](https://tianocore-docs.github.io/edk2-BuildSpecification/draft/edk2-BuildSpecification-draft.mobi), > +[ePub > ](https://tianocore-docs.github.io/edk2-BuildSpecification/draft/edk2-BuildSpecification-draft.epub), > [GitHub ](https://github.com/tianocore-docs/edk2-BuildSpecification) > \] > > * **_EDK II DEC Specification_** \[ > -[HTML > ](https://www.gitbook.com/read/book/edk2-docs/edk-ii-dec-specification), > -[PDF > ](https://www.gitbook.com/download/pdf/book/edk2-docs/edk-ii-dec-specification), > -[Mobi > ](https://www.gitbook.com/download/mobi/book/edk2-docs/edk-ii-dec-specification), > -[ePub > ](https://www.gitbook.com/download/epub/book/edk2-docs/edk-ii-dec-specification), > -[Gitbook](https://www.gitbook.com/book/edk2-docs/edk-ii-dec-specification), > +[HTML ](https://tianocore-docs.github.io/edk2-DecSpecification/draft/), > +[PDF > ](https://tianocore-docs.github.io/edk2-DecSpecification/draft/edk2-DecSpecification-draft.pdf), > +[Mobi > ](https://tianocore-docs.github.io/edk2-DecSpecification/draft/edk2-DecSpecification-draft.mobi), > +[ePub > ](https://tianocore-docs.github.io/edk2-DecSpecification/draft/edk2-DecSpecification-draft.epub), > [GitHub ](https://github.com/tianocore-docs/edk2-DecSpecification) > \] > > * **_EDK II INF Specification_** \[ > -[HTML > ](https://www.gitbook.com/read/book/edk2-docs/edk-ii-inf-specification), > -[PDF > ](https://www.gitbook.com/download/pdf/book/edk2-docs/edk-ii-inf-specification), > -[Mobi > ](https://www.gitbook.com/download/mobi/book/edk2-docs/edk-ii-inf-specification), > -[ePub > ](https://www.gitbook.com/download/epub/book/edk2-docs/edk-ii-inf-specification), > -[Gitbook](https://www.gitbook.com/book/edk2-docs/edk-ii-inf-specification), > +[HTML ](https://tianocore-docs.github.io/edk2-InfSpecification/draft/), > +[PDF > ](https://tianocore-docs.github.io/edk2-InfSpecification/draft/edk2-InfSpecification-draft.pdf), > +[Mobi > ](https://tianocore-docs.github.io/edk2-InfSpecification/draft/edk2-InfSpecification-draft.mobi), > +[ePub > ](https://tianocore-docs.github.io/edk2-InfSpecification/draft/edk2-InfSpecification-draft.epub), > [GitHub ](https://github.com/tianocore-docs/edk2-InfSpecification) > \] > > * **_EDK II DSC Specification_** \[ > -[HTML > ](https://www.gitbook.com/read/book/edk2-docs/edk-ii-dsc-specification), > -[PDF > ](https://www.gitbook.com/download/pdf/book/edk2-docs/edk-ii-dsc-specification), > -[Mobi > ](https://www.gitbook.com/download/mobi/book/edk2-docs/edk-ii-dsc-specification), > -[ePub > ](https://www.gitbook.com/download/epub/book/edk2-docs/edk-ii-dsc-specification), > -[Gitbook](https://www.gitbook.com/book/edk2-docs/edk-ii-dsc-specification/details), > +[HTML ](https://tianocore-docs.github.io/edk2-DscSpecification/draft/), > +[PDF > ](https://tianocore-docs.github.io/edk2-DscSpecification/draft/edk2-DscSpecification-draft.pdf), > +[Mobi > ](https://tianocore-docs.github.io/edk2-DscSpecification/draft/edk2-DscSpecification-draft.mobi), > +[ePub > ](https://tianocore-docs.github.io/edk2-DscSpecification/draft/edk2-DscSpecification-draft
Re: [edk2-devel] [PATCH tianocore-docs 2/2] Readme.md: Update the Gitbook documentation section
> -Original Message- > From: Rebecca Cran > Sent: Tuesday, March 14, 2023 1:05 PM > To: devel@edk2.groups.io; Ard Biesheuvel ; Feng, > Bob C ; Abner > Chang ; Justen, Jordan L ; > Gao, Liming ; > Leif Lindholm ; Michael Kubacki > ; Kinney, Michael D > ; Demeter, Miki ; Ni, Ray > ; Zimmer, Vincent > ; Dong, Eric > Cc: Rebecca Cran > Subject: [PATCH tianocore-docs 2/2] Readme.md: Update the Gitbook > documentation section > > We no longer use Gitbook for publishing the documentation. > Update the section to direct users to use the mailing list to > send feedback, and fix the Gitbook URL to point to the documentation > which includes how to format the documents. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Rebecca Cran > --- > Readme.md | 7 +++ > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/Readme.md b/Readme.md > index 1e4a5d25bd6f..7f0c05710c78 100644 > --- a/Readme.md > +++ b/Readme.md > @@ -30,12 +30,11 @@ documentation. > > # EDK II GitBook Documents > > -These are the latest draft revisions published using > [Gitbook](https://legacy.gitbook.com/). > +These are the latest draft revisions published using > [Gitbook](https://docs.gitbook.com/). I don't think we should reference docs.gitbook.com. The source format of the documents are Gitbook Markdown and the tools used to generate the published docs are based on the open Source gitbook tools using a GitHub action and are published in repositories in the GitHub tianococore-docs organization. > The source content for these documents are in GIT repositories in the > [Tianocore-docs](https://github.com/tianocore-docs) organization hosted by > [GitHub](https://github.com). > -Feedback on these documents may be provided using the > [Gitbook](https://www.gitbook.com) commenting feature > -available when reading the **HTML** versions of the documents. Document > issues and feature requests can also > -be entered in [Tianocore Bugzilla](https://bugzilla.tianocore.org). > +Feedback on these documents may be provided via the mailing list. > +Document issues and feature requests can also be entered in [Tianocore > Bugzilla](https://bugzilla.tianocore.org). > > * **_EDK II Build Specification_** \[ > [HTML ](https://tianocore-docs.github.io/edk2-BuildSpecification/draft/), > -- > 2.37.1 (Apple Git-137.1) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#101178): https://edk2.groups.io/g/devel/message/101178 Mute This Topic: https://groups.io/mt/97612887/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH tianocore-docs 2/2] Readme.md: Update the Gitbook documentation section
We no longer use Gitbook for publishing the documentation. Update the section to direct users to use the mailing list to send feedback, and fix the Gitbook URL to point to the documentation which includes how to format the documents. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Rebecca Cran --- Readme.md | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Readme.md b/Readme.md index 1e4a5d25bd6f..7f0c05710c78 100644 --- a/Readme.md +++ b/Readme.md @@ -30,12 +30,11 @@ documentation. # EDK II GitBook Documents -These are the latest draft revisions published using [Gitbook](https://legacy.gitbook.com/). +These are the latest draft revisions published using [Gitbook](https://docs.gitbook.com/). The source content for these documents are in GIT repositories in the [Tianocore-docs](https://github.com/tianocore-docs) organization hosted by [GitHub](https://github.com). -Feedback on these documents may be provided using the [Gitbook](https://www.gitbook.com) commenting feature -available when reading the **HTML** versions of the documents. Document issues and feature requests can also -be entered in [Tianocore Bugzilla](https://bugzilla.tianocore.org). +Feedback on these documents may be provided via the mailing list. +Document issues and feature requests can also be entered in [Tianocore Bugzilla](https://bugzilla.tianocore.org). * **_EDK II Build Specification_** \[ [HTML ](https://tianocore-docs.github.io/edk2-BuildSpecification/draft/), -- 2.37.1 (Apple Git-137.1) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#101177): https://edk2.groups.io/g/devel/message/101177 Mute This Topic: https://groups.io/mt/97612887/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH tianocore-docs 1/2] Readme.md: convert links from Gitbook to Github Pages
We no longer publish documentation to gitbook.com, instead using tianocore-docs.github.io. Update the links to point to the correct locations. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Rebecca Cran --- Readme.md | 117 +--- 1 file changed, 52 insertions(+), 65 deletions(-) diff --git a/Readme.md b/Readme.md index e6d35fa41131..1e4a5d25bd6f 100644 --- a/Readme.md +++ b/Readme.md @@ -38,121 +38,108 @@ available when reading the **HTML** versions of the documents. Document issues a be entered in [Tianocore Bugzilla](https://bugzilla.tianocore.org). * **_EDK II Build Specification_** \[ -[HTML ](https://www.gitbook.com/read/book/edk2-docs/edk-ii-build-specification), -[PDF ](https://www.gitbook.com/download/pdf/book/edk2-docs/edk-ii-build-specification), -[Mobi ](https://www.gitbook.com/download/mobi/book/edk2-docs/edk-ii-build-specification), -[ePub ](https://www.gitbook.com/download/epub/book/edk2-docs/edk-ii-build-specification), -[Gitbook](https://www.gitbook.com/book/edk2-docs/edk-ii-build-specification), +[HTML ](https://tianocore-docs.github.io/edk2-BuildSpecification/draft/), +[PDF ](https://tianocore-docs.github.io/edk2-BuildSpecification/draft/edk2-BuildSpecification-draft.pdf), +[Mobi ](https://tianocore-docs.github.io/edk2-BuildSpecification/draft/edk2-BuildSpecification-draft.mobi), +[ePub ](https://tianocore-docs.github.io/edk2-BuildSpecification/draft/edk2-BuildSpecification-draft.epub), [GitHub ](https://github.com/tianocore-docs/edk2-BuildSpecification) \] * **_EDK II DEC Specification_** \[ -[HTML ](https://www.gitbook.com/read/book/edk2-docs/edk-ii-dec-specification), -[PDF ](https://www.gitbook.com/download/pdf/book/edk2-docs/edk-ii-dec-specification), -[Mobi ](https://www.gitbook.com/download/mobi/book/edk2-docs/edk-ii-dec-specification), -[ePub ](https://www.gitbook.com/download/epub/book/edk2-docs/edk-ii-dec-specification), -[Gitbook](https://www.gitbook.com/book/edk2-docs/edk-ii-dec-specification), +[HTML ](https://tianocore-docs.github.io/edk2-DecSpecification/draft/), +[PDF ](https://tianocore-docs.github.io/edk2-DecSpecification/draft/edk2-DecSpecification-draft.pdf), +[Mobi ](https://tianocore-docs.github.io/edk2-DecSpecification/draft/edk2-DecSpecification-draft.mobi), +[ePub ](https://tianocore-docs.github.io/edk2-DecSpecification/draft/edk2-DecSpecification-draft.epub), [GitHub ](https://github.com/tianocore-docs/edk2-DecSpecification) \] * **_EDK II INF Specification_** \[ -[HTML ](https://www.gitbook.com/read/book/edk2-docs/edk-ii-inf-specification), -[PDF ](https://www.gitbook.com/download/pdf/book/edk2-docs/edk-ii-inf-specification), -[Mobi ](https://www.gitbook.com/download/mobi/book/edk2-docs/edk-ii-inf-specification), -[ePub ](https://www.gitbook.com/download/epub/book/edk2-docs/edk-ii-inf-specification), -[Gitbook](https://www.gitbook.com/book/edk2-docs/edk-ii-inf-specification), +[HTML ](https://tianocore-docs.github.io/edk2-InfSpecification/draft/), +[PDF ](https://tianocore-docs.github.io/edk2-InfSpecification/draft/edk2-InfSpecification-draft.pdf), +[Mobi ](https://tianocore-docs.github.io/edk2-InfSpecification/draft/edk2-InfSpecification-draft.mobi), +[ePub ](https://tianocore-docs.github.io/edk2-InfSpecification/draft/edk2-InfSpecification-draft.epub), [GitHub ](https://github.com/tianocore-docs/edk2-InfSpecification) \] * **_EDK II DSC Specification_** \[ -[HTML ](https://www.gitbook.com/read/book/edk2-docs/edk-ii-dsc-specification), -[PDF ](https://www.gitbook.com/download/pdf/book/edk2-docs/edk-ii-dsc-specification), -[Mobi ](https://www.gitbook.com/download/mobi/book/edk2-docs/edk-ii-dsc-specification), -[ePub ](https://www.gitbook.com/download/epub/book/edk2-docs/edk-ii-dsc-specification), -[Gitbook](https://www.gitbook.com/book/edk2-docs/edk-ii-dsc-specification/details), +[HTML ](https://tianocore-docs.github.io/edk2-DscSpecification/draft/), +[PDF ](https://tianocore-docs.github.io/edk2-DscSpecification/draft/edk2-DscSpecification-draft.pdf), +[Mobi ](https://tianocore-docs.github.io/edk2-DscSpecification/draft/edk2-DscSpecification-draft.mobi), +[ePub ](https://tianocore-docs.github.io/edk2-DscSpecification/draft/edk2-DscSpecification-draft.epub), [GitHub ](https://github.com/tianocore-docs/edk2-DscSpecification) \] * **_EDK II FDF Specification_** \[ -[HTML ](https://www.gitbook.com/read/book/edk2-docs/edk-ii-fdf-specification), -[PDF ](https://www.gitbook.com/download/pdf/book/edk2-docs/edk-ii-fdf-specification), -[Mobi ](https://www.gitbook.com/download/mobi/book/edk2-docs/edk-ii-fdf-specification), -[ePub ](https://www.gitbook.com/download/epub/book/edk2-docs/edk-ii-fdf-specification), -[Gitbook](https://www.gitbook.com/book/edk2-docs/edk-ii-fdf-specification), +[HTML ](https://tianocore-docs.github.io/edk2-FdfSpecification/draft/), +[PDF ](https://tianocore-docs.
[edk2-devel] [PATCH tianocore-docs 0/2] Fix links to specifications, guides etc.
We stopped using Gitbook for publishing documentation a while ago, and now only use the Gitbook markdown _format_ and publish pages to Github Pages at tianocore-docs.github.io. Fix the links to the draft documentation pages, and update the text to point users to the mailing list since they can no longer provide feedback via gitbook.com. Rebecca Cran (2): Readme.md: convert links from Gitbook to Github Pages Readme.md: Update the Gitbook documentation section Readme.md | 124 +--- 1 file changed, 55 insertions(+), 69 deletions(-) -- 2.37.1 (Apple Git-137.1) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#101175): https://edk2.groups.io/g/devel/message/101175 Mute This Topic: https://groups.io/mt/97612882/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [RESEND v1 1/1] CI: Use Fedora-37 (gcc12) image for Linux jobs
Reviewed-by: Rebecca Cran On 3/14/23 9:21 AM, Oliver Steffen wrote: Switch to the new Fedora-37 CI docker image, and with it to gcc12, for Linux jobs. Signed-off-by: Oliver Steffen Reviewed-by: Sunil V L Reviewed-by: Michael Kubacki --- .azurepipelines/templates/defaults.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.azurepipelines/templates/defaults.yml b/.azurepipelines/templates/defaults.yml index 74d6b417839d..190d9a09d2a7 100644 --- a/.azurepipelines/templates/defaults.yml +++ b/.azurepipelines/templates/defaults.yml @@ -9,4 +9,4 @@ variables: default_python_version: ">=3.10.6" - default_linux_image: "ghcr.io/tianocore/containers/fedora-35-test:47addc9" + default_linux_image: "ghcr.io/tianocore/containers/fedora-37-test:14d2aba" -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#101174): https://edk2.groups.io/g/devel/message/101174 Mute This Topic: https://groups.io/mt/97606100/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v5 08/38] ArmPkg/ArmMmuLib: Avoid splitting block entries if possible
On Tue, 14 Mar 2023 at 19:13, Leif Lindholm wrote: > > On Mon, Mar 13, 2023 at 18:16:44 +0100, Ard Biesheuvel wrote: > > Currently, the ARM MMU page table logic will break down any block entry > > that overlaps with the region being mapped, even if the block entry in > > question is using the same attributes as the new region. > > > > This means that creating a non-executable mapping inside a region that > > is already mapped non-executable at a coarser granularity may trigger a > > call to AllocatePages (), which may recurse back into the page table > > code to update the attributes on the newly allocated page tables. > > > > Let's avoid this, by preserving the block entry if it already covers the > > region being mapped with the correct attributes. > > So if a later mapping is made inside the same block with conflicting > attributes? That triggers the break down at that point and because the > existing mapping did not conflict, it'll all flush out? > Indeed. The case here is simply, e.g., mapping a single page XP that is already covered by a 2 MB XP block: without this patch, we break down that 2 MB block into page mappings that all have the same attributes. If the 4k page being remapped is being allocated for a page table, we may end up with unbounded recursion. If the attributes are actually different, the split still happens. But otherwise, the block mapping is retained. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#101173): https://edk2.groups.io/g/devel/message/101173 Mute This Topic: https://groups.io/mt/97585995/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v5 08/38] ArmPkg/ArmMmuLib: Avoid splitting block entries if possible
On Mon, Mar 13, 2023 at 18:16:44 +0100, Ard Biesheuvel wrote:
> Currently, the ARM MMU page table logic will break down any block entry
> that overlaps with the region being mapped, even if the block entry in
> question is using the same attributes as the new region.
>
> This means that creating a non-executable mapping inside a region that
> is already mapped non-executable at a coarser granularity may trigger a
> call to AllocatePages (), which may recurse back into the page table
> code to update the attributes on the newly allocated page tables.
>
> Let's avoid this, by preserving the block entry if it already covers the
> region being mapped with the correct attributes.
So if a later mapping is made inside the same block with conflicting
attributes? That triggers the break down at that point and because the
existing mapping did not conflict, it'll all flush out?
/
Leif
> Signed-off-by: Ard Biesheuvel
> ---
> ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 10 ++
> ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c | 11 +++
> 2 files changed, 21 insertions(+)
>
> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> index 6d21a2e41dd1..1ce200c43c72 100644
> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> @@ -251,6 +251,16 @@ UpdateRegionMappingRecursive (
>ASSERT (Level < 3);
>
>if (!IsTableEntry (*Entry, Level)) {
> +//
> +// If the region we are trying to map is already covered by a block
> +// entry with the right attributes, don't bother splitting it up.
> +//
> +if (IsBlockEntry (*Entry, Level) &&
> +((*Entry & TT_ATTRIBUTES_MASK & ~AttributeClearMask) ==
> AttributeSetMask))
> +{
> + continue;
> +}
> +
> //
> // No table entry exists yet, so we need to allocate a page table
> // for the next level.
> diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c
> b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c
> index 247cf87bf3d3..299d38ad07e8 100644
> --- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c
> +++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c
> @@ -170,6 +170,17 @@ UpdatePageEntries (
>
> // Does this descriptor need to be converted from section entry to 4K
> pages?
> if (!TT_DESCRIPTOR_SECTION_TYPE_IS_PAGE_TABLE (Descriptor)) {
> + //
> + // If the section mapping covers the requested region with the expected
> + // attributes, splitting it is unnecessary, and should be avoided as it
> + // may result in unbounded recursion when using a strict NX policy.
> + //
> + if ((EntryValue & ~TT_DESCRIPTOR_PAGE_TYPE_MASK & EntryMask) ==
> + (ConvertSectionAttributesToPageAttributes (Descriptor) &
> EntryMask))
> + {
> +continue;
> + }
> +
>Status = ConvertSectionToPages (FirstLevelIdx <<
> TT_DESCRIPTOR_SECTION_BASE_SHIFT);
>if (EFI_ERROR (Status)) {
> // Exit for loop
> --
> 2.39.2
>
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101172): https://edk2.groups.io/g/devel/message/101172
Mute This Topic: https://groups.io/mt/97585995/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] 回复: [PATCH 1/1] MdePkg: Add new JedecJep106Lib to fetch JEDEC JEP106 manufacturer
On 3/9/23 7:53 PM, gaoliming via groups.io wrote: +/** + Looks up the JEP-106 manufacturer. + + @param Code Last non-zero byte of the manufacturer's ID code. + @param ContinuationBytes Number of continuation bytes indicated in JEP-106. + + @return The manufacturer string, or NULL if an error occurred or the + combination of Code and ContinuationBytes are not valid. + +**/ +CONST CHAR8 * +Jep106GetManufacturerName ( + IN UINT8 Code, + IN UINT8 ContinuationBytes + ); + Library API needs EFIAPI. Thanks, I'll fix it. +/** + Returns the length of the longest manufacturer name. + +@return The length of the longest manufacturer name. + +**/ +UINTN +Jep106GetLongestManufacturerName ( + VOID + ); + What usage is for this new API? I have a new SPD Type17 library I'm working on that will use this. -- Rebecca Cran -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#101171): https://edk2.groups.io/g/devel/message/101171 Mute This Topic: https://groups.io/mt/97512168/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 1/1] ArmPkg/SmbiosMiscDxe: Adjust the priority of getting firmware version
On 3/14/23 6:48 AM, Leif Lindholm wrote: No objection to that. But can we do it like this?: Change GetBiosVersion to SetBiosVersion and in MiscBiosVendor, only call SetBiosVersion (); and move the selection logic fully into SetBiosVersion? Rebecca, thoughts? Arguably, once an OemMiscLib dependency was added, the Pcd values became less useful in the core code. That sounds very reasonable. -- Rebecca Cran -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#101169): https://edk2.groups.io/g/devel/message/101169 Mute This Topic: https://groups.io/mt/97575576/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v5 04/38] ArmPkg/ArmMmuLib ARM: Isolate the access flag from AP mask
On Mon, Mar 13, 2023 at 18:16:40 +0100, Ard Biesheuvel wrote:
> Split the ARM permission fields in the short descriptors into an access
> flag and AP[2:1] as per the recommendation in the ARM ARM. This makes
> the access flag available separately, which allows us to implement
> EFI_MEMORY_RP memory analogous to how it will be implemented for
> AArch64.
>
> Signed-off-by: Ard Biesheuvel
Reviewed-by: Leif Lindholm
Comment: it seems your setup has lost the diff.orderFile setting, so
we end up with the new definitions after their first use.
> ---
> ArmPkg/Drivers/CpuDxe/Arm/Mmu.c | 47 ++--
> ArmPkg/Include/Chipset/ArmV7Mmu.h | 40 +++--
> ArmPkg/Library/ArmLib/Arm/ArmV7Support.S| 2 +
> ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibConvert.c | 1 +
> ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c | 12 -
> 5 files changed, 63 insertions(+), 39 deletions(-)
>
> diff --git a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
> index 8eb1f71395f5..07faab8216ec 100644
> --- a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
> +++ b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
> @@ -50,30 +50,27 @@ SectionToGcdAttributes (
>
>// determine protection attributes
>switch (SectionAttributes & TT_DESCRIPTOR_SECTION_AP_MASK) {
> -case TT_DESCRIPTOR_SECTION_AP_NO_NO: // no read, no write
> - // *GcdAttributes |= EFI_MEMORY_RO | EFI_MEMORY_RP;
> - break;
> -
> -case TT_DESCRIPTOR_SECTION_AP_RW_NO:
> +case TT_DESCRIPTOR_SECTION_AP_NO_RW:
> case TT_DESCRIPTOR_SECTION_AP_RW_RW:
>// normal read/write access, do not add additional attributes
>break;
>
> // read only cases map to write-protect
> -case TT_DESCRIPTOR_SECTION_AP_RO_NO:
> +case TT_DESCRIPTOR_SECTION_AP_NO_RO:
> case TT_DESCRIPTOR_SECTION_AP_RO_RO:
>*GcdAttributes |= EFI_MEMORY_RO;
>break;
> -
> -default:
> - return EFI_UNSUPPORTED;
>}
>
>// now process eXectue Never attribute
> - if ((SectionAttributes & TT_DESCRIPTOR_SECTION_XN_MASK) != 0 ) {
> + if ((SectionAttributes & TT_DESCRIPTOR_SECTION_XN_MASK) != 0) {
> *GcdAttributes |= EFI_MEMORY_XP;
>}
>
> + if ((SectionAttributes & TT_DESCRIPTOR_SECTION_AF) == 0) {
> +*GcdAttributes |= EFI_MEMORY_RP;
> + }
> +
>return EFI_SUCCESS;
> }
>
> @@ -114,30 +111,27 @@ PageToGcdAttributes (
>
>// determine protection attributes
>switch (PageAttributes & TT_DESCRIPTOR_PAGE_AP_MASK) {
> -case TT_DESCRIPTOR_PAGE_AP_NO_NO: // no read, no write
> - // *GcdAttributes |= EFI_MEMORY_RO | EFI_MEMORY_RP;
> - break;
> -
> -case TT_DESCRIPTOR_PAGE_AP_RW_NO:
> +case TT_DESCRIPTOR_PAGE_AP_NO_RW:
> case TT_DESCRIPTOR_PAGE_AP_RW_RW:
>// normal read/write access, do not add additional attributes
>break;
>
> // read only cases map to write-protect
> -case TT_DESCRIPTOR_PAGE_AP_RO_NO:
> +case TT_DESCRIPTOR_PAGE_AP_NO_RO:
> case TT_DESCRIPTOR_PAGE_AP_RO_RO:
>*GcdAttributes |= EFI_MEMORY_RO;
>break;
> -
> -default:
> - return EFI_UNSUPPORTED;
>}
>
>// now process eXectue Never attribute
> - if ((PageAttributes & TT_DESCRIPTOR_PAGE_XN_MASK) != 0 ) {
> + if ((PageAttributes & TT_DESCRIPTOR_PAGE_XN_MASK) != 0) {
> *GcdAttributes |= EFI_MEMORY_XP;
>}
>
> + if ((PageAttributes & TT_DESCRIPTOR_PAGE_AF) == 0) {
> +*GcdAttributes |= EFI_MEMORY_RP;
> + }
> +
>return EFI_SUCCESS;
> }
>
> @@ -166,6 +160,7 @@ SyncCacheConfigPage (
>// Convert SectionAttributes into PageAttributes
>NextPageAttributes =
> TT_DESCRIPTOR_CONVERT_TO_PAGE_CACHE_POLICY (*NextSectionAttributes) |
> +TT_DESCRIPTOR_CONVERT_TO_PAGE_AF (*NextSectionAttributes) |
> TT_DESCRIPTOR_CONVERT_TO_PAGE_AP (*NextSectionAttributes);
>
>// obtain page table base
> @@ -174,7 +169,7 @@ SyncCacheConfigPage (
>for (i = 0; i < TRANSLATION_TABLE_PAGE_COUNT; i++) {
> if ((SecondLevelTable[i] & TT_DESCRIPTOR_PAGE_TYPE_MASK) ==
> TT_DESCRIPTOR_PAGE_TYPE_PAGE) {
>// extract attributes (cacheability and permissions)
> - PageAttributes = SecondLevelTable[i] &
> (TT_DESCRIPTOR_PAGE_CACHE_POLICY_MASK | TT_DESCRIPTOR_PAGE_AP_MASK);
> + PageAttributes = SecondLevelTable[i] &
> (TT_DESCRIPTOR_PAGE_CACHE_POLICY_MASK | TT_DESCRIPTOR_PAGE_AP_MASK |
> TT_DESCRIPTOR_PAGE_AF);
>
>if (NextPageAttributes == 0) {
> // start on a new region
> @@ -213,6 +208,7 @@ SyncCacheConfigPage (
>// Convert back PageAttributes into SectionAttributes
>*NextSectionAttributes =
> TT_DESCRIPTOR_CONVERT_TO_SECTION_CACHE_POLICY (NextPageAttributes) |
> +TT_DESCRIPTOR_CONVERT_TO_SECTION_AF (NextPageAttributes) |
> TT_DESCRIPTOR_CONVERT_TO_SECTION_AP (NextPageAttributes);
>
>return EFI_SUCCESS;
> @@ -256,14 +252,14 @@ SyncCacheConfig (
>FirstLevelTable = (ARM_FIRST_LEVEL_DESCRIPTOR *)(ArmGetTTBR0BaseAddress
> ());
>
Re: [edk2-devel] [PATCH v5 02/38] ArmPkg/ArmMmuLib ARM: Split off XN page descriptor bit from type field
On Mon, Mar 13, 2023 at 18:16:38 +0100, Ard Biesheuvel wrote:
> With large page support out of the picture, we can treat bits 1 and 0 of
> the page descriptor as individual valid and XN bits, instead of treating
> XN as a page type. Doing so aligns the handling of the attribute with
> the section descriptor layout, as well as the XN handling on AArch64,
> and this is beneficial for maintainability.
On average, this is a big enough improvement that I'm happy for it to
go in, but it *does* take a step away from the actual architectural
definition.
I'll choose to see it as viewing aarch32 through aarch64 goggles,
which feels reasonable these days.
Reviewed-by: Leif Lindholm
> Signed-off-by: Ard Biesheuvel
> ---
> ArmPkg/Include/Chipset/ArmV7Mmu.h | 8 +++-
> ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c | 12 ++--
> 2 files changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/ArmPkg/Include/Chipset/ArmV7Mmu.h
> b/ArmPkg/Include/Chipset/ArmV7Mmu.h
> index 7501ebfdf97f..6a2584ceb303 100644
> --- a/ArmPkg/Include/Chipset/ArmV7Mmu.h
> +++ b/ArmPkg/Include/Chipset/ArmV7Mmu.h
> @@ -54,11 +54,9 @@
> #define TT_DESCRIPTOR_SECTION_TYPE_IS_PAGE_TABLE(Desc) (((Desc) & 3UL) ==
> TT_DESCRIPTOR_SECTION_TYPE_PAGE_TABLE)
>
> // Translation table descriptor types
> -#define TT_DESCRIPTOR_PAGE_TYPE_MASK (3UL << 0)
> -#define TT_DESCRIPTOR_PAGE_TYPE_FAULT (0UL << 0)
> -#define TT_DESCRIPTOR_PAGE_TYPE_PAGE (2UL << 0)
> -#define TT_DESCRIPTOR_PAGE_TYPE_PAGE_XN(3UL << 0)
> -#define TT_DESCRIPTOR_PAGE_TYPE_LARGEPAGE (1UL << 0)
> +#define TT_DESCRIPTOR_PAGE_TYPE_MASK (1UL << 1)
> +#define TT_DESCRIPTOR_PAGE_TYPE_FAULT (0UL << 1)
> +#define TT_DESCRIPTOR_PAGE_TYPE_PAGE (1UL << 1)
>
> // Section descriptor definitions
> #define TT_DESCRIPTOR_SECTION_SIZE (0x0010)
> diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c
> b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c
> index 9ca00c976d5f..12d0f4c30f8e 100644
> --- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c
> +++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c
> @@ -104,12 +104,8 @@ UpdatePageEntries (
>
>// EntryMask: bitmask of values to change (1 = change this value, 0 =
> leave alone)
>// EntryValue: values at bit positions specified by EntryMask
> - EntryMask = TT_DESCRIPTOR_PAGE_TYPE_MASK | TT_DESCRIPTOR_PAGE_AP_MASK;
> - if ((Attributes & EFI_MEMORY_XP) != 0) {
> -EntryValue = TT_DESCRIPTOR_PAGE_TYPE_PAGE_XN;
> - } else {
> -EntryValue = TT_DESCRIPTOR_PAGE_TYPE_PAGE;
> - }
> + EntryMask = TT_DESCRIPTOR_PAGE_TYPE_MASK | TT_DESCRIPTOR_PAGE_AP_MASK |
> TT_DESCRIPTOR_PAGE_XN_MASK;
> + EntryValue = TT_DESCRIPTOR_PAGE_TYPE_PAGE;
>
>// Although the PI spec is unclear on this, the GCD guarantees that only
>// one Attribute bit is set at a time, so the order of the conditionals
> below
> @@ -148,6 +144,10 @@ UpdatePageEntries (
> EntryValue |= TT_DESCRIPTOR_PAGE_AP_RW_RW;
>}
>
> + if ((Attributes & EFI_MEMORY_XP) != 0) {
> +EntryValue |= TT_DESCRIPTOR_PAGE_XN_MASK;
> + }
> +
>// Obtain page table base
>FirstLevelTable = (ARM_FIRST_LEVEL_DESCRIPTOR *)ArmGetTTBR0BaseAddress ();
>
> --
> 2.39.2
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101167): https://edk2.groups.io/g/devel/message/101167
Mute This Topic: https://groups.io/mt/97585984/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] Update edksetup.bat etc. to support building BaseTools with VS2008 and VS2010
> I am glad that you add this support. But, I want to confirm whether someone > still uses VS2008 or VS2010. Given that it's now 2023 and this message is from 2019, I guess we can assume that nobody is still using VS2008 or VS2010. Given that Microsoft has dropped documentation for anything older than VS2015, I'd like to propose also removing VS2012 and VS2013. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#101166): https://edk2.groups.io/g/devel/message/101166 Mute This Topic: https://groups.io/mt/32985108/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 0/3] BaseTools: allow users to override CC and CXX on the make command line
This is a patch series that I'd really like to get committed. It'll allow us to drop the dependency on gcc for building edk2 on FreeBSD. The Github branch is https://github.com/bcran/edk2/tree/mdepkg-c11 and there's a PR at https://github.com/tianocore/edk2/pull/4142. -- Rebecca Cran On 3/9/23 8:47 AM, Rebecca Cran wrote: Could I get some reviews on this please? Thanks. Rebecca Cran On 2/16/23 8:50 PM, Rebecca Cran wrote: Currently, the BaseTools Makefiles use BUILD_CC and BUILD_CXX, which doesn't allow users to override the compiler to use in the expected way by running e.g. "make CC=clang-17 CXX=clang++-17". clang/llvm support was added in https://bugzilla.tianocore.org/show_bug.cgi?id=2842 in a way that required users to run "make CXX=llvm" and have clang and clang++ executables under $(CLANG_BIN). As far as I know this isn't a standard way of telling a build system to use clang, and so is likely difficult to discover by users. This patch series fixes that, and as a side effect allows the clang analyzer to run via "scan-build make". Since clang 17 defaults to C++17 or newer where the 'register' keyword is deprecated and the warning turned into an error, override the version used when building C++ code via "-std=c++14". Rebecca Cran (3): BaseTools: Allow users to specify compiler to use with make CC= CXX= BaseTools: Improve detection of users wanting to build using clang BaseTools: Build against C++14 when building with clang BaseTools/Source/C/DevicePath/GNUmakefile | 7 ++- BaseTools/Source/C/LzmaCompress/GNUmakefile | 2 +- BaseTools/Source/C/Makefiles/app.makefile | 2 +- BaseTools/Source/C/Makefiles/footer.makefile | 6 +- BaseTools/Source/C/Makefiles/header.makefile | 59 ++-- BaseTools/Source/C/VfrCompile/GNUmakefile | 19 --- BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile | 20 +++ BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile | 31 +- BaseTools/Source/Python/Workspace/DscBuildData.py | 2 +- 9 files changed, 76 insertions(+), 72 deletions(-) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#101165): https://edk2.groups.io/g/devel/message/101165 Mute This Topic: https://groups.io/mt/97022151/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [RESEND v1 0/1] CI: Use Fedora 37 / GCC 12 for Linux jobs
Switch the CI (Linux) to use GCC 12 providied by a new Fedora 37 based container image. New container image: https://github.com/tianocore/containers/pull/60 Test PR: https://github.com/tianocore/edk2/pull/4108 Oliver Steffen (1): CI: Use Fedora-37 (gcc12) image for Linux jobs .azurepipelines/templates/defaults.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.39.2 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#101163): https://edk2.groups.io/g/devel/message/101163 Mute This Topic: https://groups.io/mt/97606099/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [RESEND v1 1/1] CI: Use Fedora-37 (gcc12) image for Linux jobs
Switch to the new Fedora-37 CI docker image, and with it to gcc12, for Linux jobs. Signed-off-by: Oliver Steffen Reviewed-by: Sunil V L Reviewed-by: Michael Kubacki --- .azurepipelines/templates/defaults.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.azurepipelines/templates/defaults.yml b/.azurepipelines/templates/defaults.yml index 74d6b417839d..190d9a09d2a7 100644 --- a/.azurepipelines/templates/defaults.yml +++ b/.azurepipelines/templates/defaults.yml @@ -9,4 +9,4 @@ variables: default_python_version: ">=3.10.6" - default_linux_image: "ghcr.io/tianocore/containers/fedora-35-test:47addc9" + default_linux_image: "ghcr.io/tianocore/containers/fedora-37-test:14d2aba" -- 2.39.2 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#101164): https://edk2.groups.io/g/devel/message/101164 Mute This Topic: https://groups.io/mt/97606100/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2-platforms][PATCH 1/2] Ampere: PCIe: Add support for Ampere Altra Max
Urgh, I think I forgot to reply to this - apologies.
On Fri, Feb 24, 2023 at 10:26:42 +0700, Nhi Pham wrote:
> Hi Leif,
>
> Thanks for your reviewing. Most of your feedback is valid. I will fix them.
> There is a comment that need your more explanation.
>
> Please see the inline reply for more details.
>
> > > diff --git
> > > a/Silicon/Ampere/AmpereAltraPkg/Drivers/PcieInitPei/RootComplexNVParam.c
> > > b/Silicon/Ampere/AmpereAltraPkg/Drivers/PcieInitPei/RootComplexNVParam.c
> > > index aa34a90b44c6..1346fa616ab3 100644
> > > ---
> > > a/Silicon/Ampere/AmpereAltraPkg/Drivers/PcieInitPei/RootComplexNVParam.c
> > > +++
> > > b/Silicon/Ampere/AmpereAltraPkg/Drivers/PcieInitPei/RootComplexNVParam.c
> > > @@ -37,7 +37,7 @@
> > > | Y | Y | Y | Y | 3|
> > >
> > > - Copyright (c) 2020 - 2021, Ampere Computing LLC. All rights
> > > reserved.
> > > + Copyright (c) 2020 - 2023, Ampere Computing LLC. All rights
> > > reserved.
> > > SPDX-License-Identifier: BSD-2-Clause-Patent
> > > @@ -149,6 +149,10 @@ GetDefaultDevMap (
> > > AC01_ROOT_COMPLEX *RootComplex
> > > )
> > > {
> > > + BOOLEAN IsAc01;
> > > +
> > > + IsAc01 = IsAc01Processor ();
> > > +
> > > if (RootComplex->Pcie[PcieController0].Active
> > > && RootComplex->Pcie[PcieController1].Active
> > > && RootComplex->Pcie[PcieController2].Active
> > > @@ -169,14 +173,20 @@ GetDefaultDevMap (
> > > && RootComplex->Pcie[PcieController5].Active
> > > && RootComplex->Pcie[PcieController6].Active
> > > && RootComplex->Pcie[PcieController7].Active) {
> > > -RootComplex->DefaultDevMapHigh = DevMapMode4;
> > > +if (IsAc01) {
> > > + RootComplex->DefaultDevMapHigh = DevMapMode4;
> > > +}
> >
> > Who sets RootComplex->DefaultDevMapHigh if !IsAc01?
>
> It will be the default value (0) because Ampere Altra Max does not have Root
> Complex Type B.
OK.
So, this just looks a bit like a maintenance nightmare.
There are so many places that individually check for a specific
platform.
And this is not the most readable file to begin with.
So looking at this *existing* function, it seems to be determining how
many PCIe controllers are active. Could we simplify it with some
arithmetic instead of translating truth tables to C conditionals?
If so, it looks like we could have a single IsAc01 check, covering
cases 2-4?
/
Leif
> >
> > > } else if (RootComplex->Pcie[PcieController4].Active
> > >&& RootComplex->Pcie[PcieController6].Active
> > >&& RootComplex->Pcie[PcieController7].Active) {
> > > -RootComplex->DefaultDevMapHigh = DevMapMode3;
> > > +if (IsAc01) {
> > > + RootComplex->DefaultDevMapHigh = DevMapMode3;
> > > +}
> > same
> >
> > > } else if (RootComplex->Pcie[PcieController4].Active
> > >&& RootComplex->Pcie[PcieController6].Active) {
> > > -RootComplex->DefaultDevMapHigh = DevMapMode2;
> > > +if (IsAc01) {
> > > + RootComplex->DefaultDevMapHigh = DevMapMode2;
> > > +}
> > same
> >
> > > } else {
> > > RootComplex->DefaultDevMapHigh = DevMapMode1;
> > > }
> > I feel this function in general could be rewritten more reader-friendly.
> >
> > > @@ -203,12 +213,17 @@ GetLaneAllocation (
> > > EFI_STATUS Status;
> > > INTN RPIndex;
> > > NVPARAMNvParamOffset;
> > > - UINT32 Value, Width;
> > > + UINT32 Value, Width, Controller;
> > > // Retrieve lane allocation and capabilities for each controller
> > > if (RootComplex->Type == RootComplexTypeA) {
> > > -NvParamOffset = (RootComplex->Socket == 0) ?
> > > NV_SI_RO_BOARD_S0_RCA0_CFG : NV_SI_RO_BOARD_S1_RCA0_CFG;
> > > -NvParamOffset += RootComplex->ID * NV_PARAM_ENTRYSIZE;
> > > +if (RootComplex->ID < MaxPcieControllerOfRootComplexA) {
> > > + NvParamOffset = (RootComplex->Socket == 0) ?
> > > NV_SI_RO_BOARD_S0_RCA0_CFG : NV_SI_RO_BOARD_S1_RCA0_CFG;
> > > + NvParamOffset += RootComplex->ID * NV_PARAM_ENTRYSIZE;
> > > +} else {
> > > + NvParamOffset = (RootComplex->Socket == 0) ?
> > > NV_SI_RO_BOARD_S0_RCA4_CFG : NV_SI_RO_BOARD_S1_RCA4_CFG;
> > > + NvParamOffset += (RootComplex->ID -
> > > MaxPcieControllerOfRootComplexA) * NV_PARAM_ENTRYSIZE;
> > > +}
> > Helper function.
> >
> > > } else {
> > > //
> > > // There're two NVParam entries per RootComplexTypeB
> > > @@ -222,7 +237,13 @@ GetLaneAllocation (
> > > Value = 0;
> > > }
> > > - for (RPIndex = 0; RPIndex < MaxPcieControllerOfRootComplexA;
> > > RPIndex++) {
> > > + if (IsAc01Processor ()) {
> > > +Controller = MaxPcieControllerOfRootComplexA;
> > > + } else {
> > > +Controller = RootComplex->MaxPcieController;
> > > + }
> > Straightforward enough, but could still be a helper function.
> >
> > > +
> > > + for (RPIndex = PcieController0; RPIndex < Controller; RPIndex++) {
> > >
Re: [edk2-devel] [PATCH 1/1] ArmPkg/SmbiosMiscDxe: Adjust the priority of getting firmware version
Hi Tinh, +Rebecca
On Mon, Mar 13, 2023 at 23:52:41 +0700, Tinh Nguyen via groups.io wrote:
> On 3/13/2023 10:03 PM, Leif Lindholm wrote:
> > On Mon, Mar 13, 2023 at 13:43:21 +0700, Tinh Nguyen wrote:
> > > The BIOS Firmware Version in the SMBIOS Type 0 can be fetched from
> > > the fixed PcdFirmwareVersionString or platform specific OemMiscLib.
> > > In fact, the support from OemMiscLib comes into play when the firmware
> > > version may be modified at boot time for extended information.
> > Wait. Firmware version modified at boot time for extended information?
> > What type of extended information?
>
> That could be SCP version, TF-A version, etc.
Ah, ok - so the edk2 image never knows a proper version name, so you
want to keep the Pcd as a fallback only, to provide the edk2 build
version if no other info can be retrieved?
That's valid, just not a case I would have expected.
> > > Therefore, the priority of getting the version from OemMiscLib should
> > > be higher. In case there is no modification in the OemMiscLib,
> > > we have to keep HII string STR_MISC_BIOS_VERSION empty or 'Not Specified'
> > > to indicate that the firmware version should be fetched from
> > > the PcdFirmwareVersionString.
> > >
> > > Reviewed-by: Nhi Pham
> > > Signed-off-by: Tinh Nguyen
> > > ---
> > > ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c |
> > > 36 ++--
> > > 1 file changed, 25 insertions(+), 11 deletions(-)
> > >
> > > diff --git
> > > a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c
> > > b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c
> > > index 66ead22a6e2c..31a3f6cde544 100644
> > > ---
> > > a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c
> > > +++
> > > b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c
> > > @@ -1,6 +1,6 @@
> > > /** @file
> > > - Copyright (c) 2022, Ampere Computing LLC. All rights reserved.
> > > + Copyright (c) 2022 - 2023, Ampere Computing LLC. All rights
> > > reserved.
> > > Copyright (c) 2021, NUVIA Inc. All rights reserved.
> > > Copyright (c) 2009, Intel Corporation. All rights reserved.
> > > Copyright (c) 2015, Hisilicon Limited. All rights reserved.
> > > @@ -170,6 +170,7 @@ SMBIOS_MISC_TABLE_FUNCTION (MiscBiosVendor) {
> > > EFI_STRING_ID TokenToGet;
> > > SMBIOS_TABLE_TYPE0 *SmbiosRecord;
> > > SMBIOS_TABLE_TYPE0 *InputData;
> > > + CHAR16 *DefaultVersionString;
> > > //
> > > // First check for invalid parameters.
> > > @@ -187,17 +188,30 @@ SMBIOS_MISC_TABLE_FUNCTION (MiscBiosVendor) {
> > > HiiSetString (mSmbiosMiscHiiHandle, TokenToUpdate, Vendor, NULL);
> > > }
> > > - Version = GetBiosVersion ();
> > GetBiosVersion exists as a helper function to avoid cluttering
> > higher-level functions with unnecessary details. If this change is
> > needed, that is where it should go.
>
> This change is based on existing code, and goal is to get the Firmware
> version string from OemMiscLib first, rather than PcdFirmwareVersionString.
>
> I would rather not make any changes that aren't relevant.
I see now why you want to re-jig the current logic, and that includes
the selection logic. But this function is already too long. It needs
to be simplified, not made more complex.
> > But I still don't understand the purpose of this patch, so cannot
> > comment on whether I feel this is the right approach or not.
> > Please elaborate.
>
> The firmware version is currently being updated:
>
> 1. Get the string from Fixed PcdFirmwareVersionString.
>
> 2. Check to see if this string is null; if not, update the SMBIOS firmware
> version string.
>
> 3. if PcdFirmwareVersionString is null, retrieve the string from OemMiscLib.
That's the function - I was asking for the purpose, which you
described higher up in your reply.
> As you can see, the implementation is intended to honor the
> PcdFirmwareVersionString.
>
> We can't get the extend information (which can be derived from runtime) into
> the SMBIOS firmware version string if we don't set PcdFirmwareVersionString
> null
>
> However, in some cases we don't want to set PcdFirmwareVersionString to
> null because it might be used by other modules.
Hmm. On one level I think you're highlighting a desire for separation
of "firmware version" from "edk2 version", which isn't something the
codebase generally does today.
But we don't need to solve that completely to make this change.
> I think it makes sense to prioritize OemMiscLib since it's more flexible
> than Pcd.
No objection to that.
But can we do it like this?:
Change GetBiosVersion to SetBiosVersion and in MiscBiosVendor, only call
SetBiosVersion ();
and move the selection logic fully into SetBiosVersion?
Rebecca, thoughts?
Arguably, once an OemMiscLib dependency was added, the Pcd values became
less useful in the core code.
/
Leif
> >
> > > + DefaultVersionS
Re: [edk2-devel] [PATCH 01/22] CryptoPkg/openssl: update submodule to openssl-3.0.8
Hi, > Yeah the SIGILL trapping is a bit nasty, but that is only used if no > implementation of getauxval() exists. > > So perhaps the cleanest way to approach this is to provide a dummy > implementation of getauxval() which only supports AT_HWCAP, and > returns the correct hwcap mask for what the CPU id registers report in > terms for ISA support for crypto extensions. > > I can code that up if you want. Getting crypto/armcap.c even compile on edk2 will be a challenge I think. So I'd rather exclude it, and add a OPENSSL_cpuid_setup() aarch64 implementation to edk2 and copy over the 10 lines which map HWCAP -> OPENSSL_armcap_P. That'll be alot easier than adding dummy stubs for siglongjmp, sigaction & friends. A hwcap query function would be helpful nevertheless. thanks & take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#101160): https://edk2.groups.io/g/devel/message/101160 Mute This Topic: https://groups.io/mt/97576405/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 01/22] CryptoPkg/openssl: update submodule to openssl-3.0.8
On Tue, 14 Mar 2023 at 09:16, kra...@redhat.com wrote: > > On Mon, Mar 13, 2023 at 03:13:28PM +, Li, Yi wrote: > > Hi Gerd, > > > > I also have some work on Openssl3, mainly to research how to reduce the > > binary size increase after the upgrade: > > > > https://github.com/tianocore/edk2-staging/blob/OpenSSL11_EOL/CryptoPkg/Readme-OpenSSL3.0.md > > > > > > > > I really appreciate your work in this patch series, especially the clear py > > script. > > > > But it seems that part of our work is repeated, if you don't mind, can > > I merge your work into openssl3.0 Edk2Staging branch? You can find it > > here if you're interested: > > Sure, that is the point of sharing it ;) > > github branch (which hot some updates for aarch64 meanwhile) is at > https://github.com/kraxel/edk2/commits/openssl3 > > aarch64 is not working, the cpu capability probing needs some work. > openssl seems to just try instructions and catch SIGILL. edk2 needs > something else of course. Easiest way out would be to just provide > dummy functions, but that would also mean we wouldn't use aes > instructions if available ... > > Any hints on that from the arm camp are welcome. > Yeah the SIGILL trapping is a bit nasty, but that is only used if no implementation of getauxval() exists. So perhaps the cleanest way to approach this is to provide a dummy implementation of getauxval() which only supports AT_HWCAP, and returns the correct hwcap mask for what the CPU id registers report in terms for ISA support for crypto extensions. I can code that up if you want. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#101159): https://edk2.groups.io/g/devel/message/101159 Mute This Topic: https://groups.io/mt/97576405/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2] UefiPayloadPkg: Correct MAX_LOGICAL_PROCESSORS value
Reviewed-by: James Lu Thanks, James -Original Message- From: Zhang, Xiaoqiang Sent: Monday, March 13, 2023 12:53 PM To: devel@edk2.groups.io Cc: Zhang, Xiaoqiang ; Dong, Guo ; Ni, Ray ; Lu, James Subject: [PATCH v2] UefiPayloadPkg: Correct MAX_LOGICAL_PROCESSORS value REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4367 Issue: For Server platforms, when FSP does not output mCpuInitMpLibHobGuid HOB, the code will wakeup all APs and calculate the processor count in DXE phase. But when processor thread number is above 256, will encounter Startup IPI exception in DXE phase. Root cause: MAX_LOGICAL_PROCESSORS MARCO value in UefiPayloadPkg.dsc is 256, when the actual processor thread number is above 256, will encounter data overflow exception. Solution: Align MAX_LOGICAL_PROCESSORS value with Server platform side value 1024. Signed-off-by: Xiaoqiang Zhang Cc: Guo Dong Cc: Ray Ni Cc: James Lu --- UefiPayloadPkg/UefiPayloadPkg.dsc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc index 35e3bfff35..bca5d3f335 100644 --- a/UefiPayloadPkg/UefiPayloadPkg.dsc +++ b/UefiPayloadPkg/UefiPayloadPkg.dsc @@ -59,7 +59,7 @@ # # CPU options # - DEFINE MAX_LOGICAL_PROCESSORS = 256 + DEFINE MAX_LOGICAL_PROCESSORS = 1024 # # PCI options -- 2.39.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#101158): https://edk2.groups.io/g/devel/message/101158 Mute This Topic: https://groups.io/mt/97587622/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 01/22] CryptoPkg/openssl: update submodule to openssl-3.0.8
On Mon, Mar 13, 2023 at 03:13:28PM +, Li, Yi wrote: > Hi Gerd, > > I also have some work on Openssl3, mainly to research how to reduce the > binary size increase after the upgrade: > > https://github.com/tianocore/edk2-staging/blob/OpenSSL11_EOL/CryptoPkg/Readme-OpenSSL3.0.md > > > > I really appreciate your work in this patch series, especially the clear py > script. > > But it seems that part of our work is repeated, if you don't mind, can > I merge your work into openssl3.0 Edk2Staging branch? You can find it > here if you're interested: Sure, that is the point of sharing it ;) github branch (which hot some updates for aarch64 meanwhile) is at https://github.com/kraxel/edk2/commits/openssl3 aarch64 is not working, the cpu capability probing needs some work. openssl seems to just try instructions and catch SIGILL. edk2 needs something else of course. Easiest way out would be to just provide dummy functions, but that would also mean we wouldn't use aes instructions if available ... Any hints on that from the arm camp are welcome. take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#101157): https://edk2.groups.io/g/devel/message/101157 Mute This Topic: https://groups.io/mt/97576405/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 19/22] CryptoPkg/openssl: update *.inf, add generated files
On Mon, Mar 13, 2023 at 03:26:41PM +, Li, Yi1 wrote: > >+ DEFINE OPENSSL_FLAGS_NOASM = -DSTATIC_LEGACY > > > > Why we need this macro, EDK2 does not seem to use the algorithm in the legacy > provider. Oh, that slipped into the wrong patch. The openssl Configure script adds that, and configure.py picks it up and adds it to OPENSSL_FLAGS_*. Should have been in the "update generated files" patch (which also updates the autogenerated content in *.inf). Yes, seems not be needed, but doesn't harm either and isn't worth a special case in configure.py ... take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#101156): https://edk2.groups.io/g/devel/message/101156 Mute This Topic: https://groups.io/mt/97576445/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 16/22] [hash] CryptoPkg/openssl: add OpensslLibHash.inf
On Mon, Mar 13, 2023 at 03:46:25PM +, Li, Yi1 wrote: > A bit confused here, why we need this inf, just to make it clear? > As you mentioned, it doesn't help with binary file size. As the commit message says it helps to figure where the bloat comes because you get error messages for missing symbols instead of a huge binary. That has lead to this patch series: https://github.com/tianocore/edk2/pull/4104 But we don't have to keep it, not sure how much value that has given the cause has been found and fixed. take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#101155): https://edk2.groups.io/g/devel/message/101155 Mute This Topic: https://groups.io/mt/97576423/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
