On 08/05/19 23:57, Krzysztof Rusocki wrote:
> From: Damian Nikodem <damian.niko...@intel.com>
>
> Reclaim may free page table pages that are required to handle current page
> fault. This causes a page leak, and, after sufficent number of specific
> page fault+reclaim pairs, we run out of reclaimable pages and hit:
>
> ASSERT (MinAcc != (UINT64)-1);
>
> To remedy, prevent pages essential to handling current page fault:
> (1) from being considered as reclaim candidates (first reclaim phase)
> (2) from being freed as part of "branch cleanup" (second reclaim phase)
>
> Signed-off-by: Damian Nikodem <damian.niko...@intel.com>
> Cc: Eric Dong <eric.d...@intel.com>
> Cc: Ray Ni <ray...@intel.com>
> Cc: Jian J Wang <jian.j.w...@intel.com>
> Cc: Laszlo Ersek <ler...@redhat.com>
> Cc: Krzysztof Rusocki <krzysztof.ruso...@intel.com>
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> index a3b62f7787..f11323f439 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> @@ -543,6 +543,11 @@ ReclaimPages (
> UINT64 *ReleasePageAddress;
> IA32_CR4 Cr4;
> BOOLEAN Enable5LevelPaging;
> + UINT64 PFAddress;
> + UINT64 PFAddressPml5Index;
> + UINT64 PFAddressPml4Index;
> + UINT64 PFAddressPdptIndex;
> + UINT64 PFAddressPdtIndex;
>
> Pml4 = NULL;
> Pdpt = NULL;
> @@ -554,6 +559,11 @@ ReclaimPages (
> MinPdt = (UINTN)-1;
> Acc = 0;
> ReleasePageAddress = 0;
> + PFAddress = AsmReadCr2 ();
> + PFAddressPml5Index = BitFieldRead64 (PFAddress, 48, 48 + 8);
> + PFAddressPml4Index = BitFieldRead64 (PFAddress, 39, 39 + 8);
> + PFAddressPdptIndex = BitFieldRead64 (PFAddress, 30, 30 + 8);
> + PFAddressPdtIndex = BitFieldRead64 (PFAddress, 21, 21 + 8);
>
> Cr4.UintN = AsmReadCr4 ();
> Enable5LevelPaging = (BOOLEAN) (Cr4.Bits.LA57 == 1);
> @@ -628,40 +638,46 @@ ReclaimPages (
> // we will find the entry has the smallest access record value
> //
> PDPTEIgnore = TRUE;
> - Acc = GetAndUpdateAccNum (Pdt + PdtIndex);
> + if (PdtIndex != PFAddressPdtIndex || PdptIndex !=
> PFAddressPdptIndex ||
> + Pml4Index != PFAddressPml4Index || Pml5Index !=
> PFAddressPml5Index) {
> + Acc = GetAndUpdateAccNum (Pdt + PdtIndex);
> + if (Acc < MinAcc) {
> + //
> + // If the PD entry has the smallest access record value,
> + // save the Page address to be released
> + //
> + MinAcc = Acc;
> + MinPml5 = Pml5Index;
> + MinPml4 = Pml4Index;
> + MinPdpt = PdptIndex;
> + MinPdt = PdtIndex;
> + ReleasePageAddress = Pdt + PdtIndex;
> + }
> + }
> + }
> + }
> + if (!PDPTEIgnore) {
> + //
> + // If this PDPT entry has no PDT entries pointer to 4 KByte
> pages,
> + // it should only has the entries point to 2 MByte Pages
> + //
> + if (PdptIndex != PFAddressPdptIndex || Pml4Index !=
> PFAddressPml4Index ||
> + Pml5Index != PFAddressPml5Index) {
> + Acc = GetAndUpdateAccNum (Pdpt + PdptIndex);
> if (Acc < MinAcc) {
> //
> - // If the PD entry has the smallest access record value,
> + // If the PDPT entry has the smallest access record value,
> // save the Page address to be released
> //
> MinAcc = Acc;
> MinPml5 = Pml5Index;
> MinPml4 = Pml4Index;
> MinPdpt = PdptIndex;
> - MinPdt = PdtIndex;
> - ReleasePageAddress = Pdt + PdtIndex;
> + MinPdt = (UINTN)-1;
> + ReleasePageAddress = Pdpt + PdptIndex;
> }
> }
> }
> - if (!PDPTEIgnore) {
> - //
> - // If this PDPT entry has no PDT entries pointer to 4 KByte
> pages,
> - // it should only has the entries point to 2 MByte Pages
> - //
> - Acc = GetAndUpdateAccNum (Pdpt + PdptIndex);
> - if (Acc < MinAcc) {
> - //
> - // If the PDPT entry has the smallest access record value,
> - // save the Page address to be released
> - //
> - MinAcc = Acc;
> - MinPml5 = Pml5Index;
> - MinPml4 = Pml4Index;
> - MinPdpt = PdptIndex;
> - MinPdt = (UINTN)-1;
> - ReleasePageAddress = Pdpt + PdptIndex;
> - }
> - }
> }
> }
> if (!PML4EIgnore) {
> @@ -669,18 +685,20 @@ ReclaimPages (
> // If PML4 entry has no the PDPT entry pointer to 2 MByte pages,
> // it should only has the entries point to 1 GByte Pages
> //
> - Acc = GetAndUpdateAccNum (Pml4 + Pml4Index);
> - if (Acc < MinAcc) {
> - //
> - // If the PML4 entry has the smallest access record value,
> - // save the Page address to be released
> - //
> - MinAcc = Acc;
> - MinPml5 = Pml5Index;
> - MinPml4 = Pml4Index;
> - MinPdpt = (UINTN)-1;
> - MinPdt = (UINTN)-1;
> - ReleasePageAddress = Pml4 + Pml4Index;
> + if (Pml4Index != PFAddressPml4Index || Pml5Index !=
> PFAddressPml5Index) {
> + Acc = GetAndUpdateAccNum (Pml4 + Pml4Index);
> + if (Acc < MinAcc) {
> + //
> + // If the PML4 entry has the smallest access record value,
> + // save the Page address to be released
> + //
> + MinAcc = Acc;
> + MinPml5 = Pml5Index;
> + MinPml4 = Pml4Index;
> + MinPdpt = (UINTN)-1;
> + MinPdt = (UINTN)-1;
> + ReleasePageAddress = Pml4 + Pml4Index;
> + }
> }
> }
> }
> @@ -708,7 +726,8 @@ ReclaimPages (
> Pml4 = (UINT64 *) (UINTN) (Pml5[MinPml5] & gPhyMask);
> Pdpt = (UINT64*)(UINTN)(Pml4[MinPml4] & ~mAddressEncMask & gPhyMask);
> SubEntriesNum = GetSubEntriesNum(Pdpt + MinPdpt);
> - if (SubEntriesNum == 0) {
> + if (SubEntriesNum == 0 &&
> + (MinPdpt != PFAddressPdptIndex || MinPml4 != PFAddressPml4Index ||
> MinPml5 != PFAddressPml5Index)) {
> //
> // Release the empty Page Directory table if there was no more 4
> KByte Page Table entry
> // clear the Page directory entry
> @@ -724,7 +743,7 @@ ReclaimPages (
> //
> // Update the sub-entries filed in PDPT entry and exit
> //
> - SetSubEntriesNum (Pdpt + MinPdpt, SubEntriesNum - 1);
> + SetSubEntriesNum (Pdpt + MinPdpt, (SubEntriesNum - 1) & 0x1FF);
> break;
> }
> if (MinPdpt != (UINTN)-1) {
> @@ -732,7 +751,7 @@ ReclaimPages (
> // One 2MB Page Table is released or Page Directory table is released,
> check the PML4 entry
> //
> SubEntriesNum = GetSubEntriesNum (Pml4 + MinPml4);
> - if (SubEntriesNum == 0) {
> + if (SubEntriesNum == 0 && (MinPml4 != PFAddressPml4Index || MinPml5 !=
> PFAddressPml5Index)) {
> //
> // Release the empty PML4 table if there was no more 1G KByte Page
> Table entry
> // clear the Page directory entry
> @@ -745,7 +764,7 @@ ReclaimPages (
> //
> // Update the sub-entries filed in PML4 entry and exit
> //
> - SetSubEntriesNum (Pml4 + MinPml4, SubEntriesNum - 1);
> + SetSubEntriesNum (Pml4 + MinPml4, (SubEntriesNum - 1) & 0x1FF);
> break;
> }
> //
> @@ -918,7 +937,7 @@ SmiDefaultPFHandler (
> PageTable[PTIndex] = ((PFAddress | mAddressEncMask) & gPhyMask & ~((1ull
> << EndBit) - 1)) |
> PageAttribute | IA32_PG_A | PAGE_ATTRIBUTE_BITS;
> if (UpperEntry != NULL) {
> - SetSubEntriesNum (UpperEntry, GetSubEntriesNum (UpperEntry) + 1);
> + SetSubEntriesNum (UpperEntry, (GetSubEntriesNum (UpperEntry) + 1) &
> 0x1FF);
> }
> //
> // Get the next page address if we need to create more page tables
> --------------------------------------------------------------------
>
> Intel Technology Poland sp. z o.o.
> ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII
> Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP
> 957-07-52-316 | Kapital zakladowy 200.000 PLN.
>
> Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i
> moze zawierac informacje poufne. W razie przypadkowego otrzymania tej
> wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie;
> jakiekolwiek
> przegladanie lub rozpowszechnianie jest zabronione.
> This e-mail and any attachments may contain confidential material for the
> sole use of the intended recipient(s). If you are not the intended recipient,
> please contact the sender and delete all copies; any review or distribution by
> others is strictly prohibited.
>
got no capacity left for this; please go ahead with the reviews you
already got
thanks
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#45021): https://edk2.groups.io/g/devel/message/45021
Mute This Topic: https://groups.io/mt/32737146/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
