From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f45.google.com (mail-pj1-f45.google.com [209.85.216.45]) by mx.groups.io with SMTP id smtpd.web08.1812.1618853094347916672 for ; Mon, 19 Apr 2021 10:24:54 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20161025 header.b=EZxYOxhb; spf=pass (domain: gmail.com, ip: 209.85.216.45, mailfrom: kuqin12@gmail.com) Received: by mail-pj1-f45.google.com with SMTP id s14so13488771pjl.5 for ; Mon, 19 Apr 2021 10:24:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=fCx/0muIO4VEuVVfGgKCce0O0u2EQ39DSl4HuJO6lKo=; b=EZxYOxhbF4pH7sukALgBB98/0qqd0i1HP2IKsj3BYt8eKc6+C5NaRBdA1O9+cw4JOy kaKDX9WBE9C66PAjuVlGkU0pdmqnc2cIvntWVSvXPULoxwUANjx0fWf+gYUOKh/ZQP8w 5WnHDN3M20o3x8D7uFWsRQy9pCTFRiVfeGmKpyopWbcZGteXa2Og/PPocJDCfPeU9YUa 5JfBuECqoriZW6kqJR8A+zy+Sr1a01+XrpozCIHIl62p/YexK516Gj11dnPqvxdo2ZKp pb73uQjYz+K4qPDSLjXtYBzlF8Pvf0Spa2/MmZ1JPIBofA32RA/ljiKXLxQHwNQ2nAbw tkfg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=fCx/0muIO4VEuVVfGgKCce0O0u2EQ39DSl4HuJO6lKo=; b=gqIftA8gKicKufBnJTdLPl4jiQTtl3nXs2QuZd4B7nxOaFLzz9zQn456Gnve7pWO0X /+9TulSWw+BRQfwkMcUcmI41ewZNy8jOAHDlPdSHDy/pcvuwbMWAH2rM5IkAE28yM4CY 8kqM5qhiyIm3vuTiZV+VUpRFUZq2BZKD2loaM+KG0xWm09rgrV9MHYqKbV0zCU0OypJb PqBhMs4C6zyrQuFl1m/J56vnk5FJkijo9yfoi7bK0slsdeIlKGgWnYQH/avfCIgtbU7o LhpDY76y1HRXdPijJbmqa6z2jcV77WI1cQrL7mamOSFuPSKgCok9eIUcy4t9qXwQdKu+ CYrA== X-Gm-Message-State: AOAM530C44GWBvx8Ifcl43cFZ8piymYT1FngC37yB+mEyk94Mw2M33TM K4jeUaIIncnksbaOv6wKO9k= X-Google-Smtp-Source: ABdhPJy9aY1GOSTjauI8P7f+z4pShqbGnB0v8kg8XIfbNmLF39fwdDMN082/CU0PFElMXMD9RryDlQ== X-Received: by 2002:a17:90b:33c6:: with SMTP id lk6mr156663pjb.37.1618853093997; Mon, 19 Apr 2021 10:24:53 -0700 (PDT) Return-Path: Received: from [192.168.50.18] ([50.35.88.161]) by smtp.gmail.com with ESMTPSA id i10sm9657560pfo.37.2021.04.19.10.24.53 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 19 Apr 2021 10:24:53 -0700 (PDT) Subject: Re: [edk2-devel] [PATCH v2 1/1] UefiCpuPkg: PiSmmCpuDxeSmm: Not to Change Bitwidth During Static Paging To: devel@edk2.groups.io, Ray Ni Cc: Eric Dong , Laszlo Ersek , Rahul Kumar References: <20210414202547.394-1-kuqin12@gmail.com> <1675D34B1BADE93C.32393@groups.io> From: "Kun Qin" Message-ID: Date: Mon, 19 Apr 2021 10:24:52 -0700 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.9.1 MIME-Version: 1.0 In-Reply-To: <1675D34B1BADE93C.32393@groups.io> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Hi Ray, Would you please provide feedback for this patch? It is already reviewed by Laszlo here: https://edk2.groups.io/g/devel/message/74122. Could you please let me know what it takes after your review? I can send a v3 for reviewed-by tags if necessary. Thanks in advance, Kun On 04/14/2021 13:25, Kun Qin via groups.io wrote: > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3300 > > Current implementation of SetStaticPageTable routine in PiSmmCpuDxeSmm > driver will check a global variable mPhysicalAddressBits, and eventually > cap any value larger than 39 at 39. > > This global variable is used in ConvertMemoryPageAttributes, which backs > SmmSetMemoryAttributes and SmmClearMemoryAttributes. Thus for a processor > that supports more than 39 bits width, trying to mark page table regions > higher than 39-bit will always return EFI_UNSUPPROTED. > > This change updated the interface of SetStaticPageTable function to take > PhysicalAddressBits as an input parameter, in order to avoid changing/ > accessing the global variable. > > Cc: Eric Dong > Cc: Ray Ni > Cc: Laszlo Ersek > Cc: Rahul Kumar > > Fixes: 4eee0cc7cc0db74489b99c19eba056b53eda6358 > Signed-off-by: Kun Qin > --- > > Notes: > v2: > - SetStaticPageTable interface update [Ray] > - Commit message updates, variable type change [Laszlo] > > UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 30 +++++++++++--------- > 1 file changed, 16 insertions(+), 14 deletions(-) > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > index 6902584b1fbd..d6f8dd94d303 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > @@ -211,11 +211,13 @@ CalculateMaximumSupportAddress ( > /** > Set static page table. > > - @param[in] PageTable Address of page table. > + @param[in] PageTable Address of page table. > + @param[in] PhysicalAddressBits The maximum physical address bits supported. > **/ > VOID > SetStaticPageTable ( > - IN UINTN PageTable > + IN UINTN PageTable, > + IN UINT8 PhysicalAddressBits > ) > { > UINT64 PageAddress; > @@ -237,26 +239,26 @@ SetStaticPageTable ( > // IA-32e paging translates 48-bit linear addresses to 52-bit physical addresses > // when 5-Level Paging is disabled. > // > - ASSERT (mPhysicalAddressBits <= 52); > - if (!m5LevelPagingNeeded && mPhysicalAddressBits > 48) { > - mPhysicalAddressBits = 48; > + ASSERT (PhysicalAddressBits <= 52); > + if (!m5LevelPagingNeeded && PhysicalAddressBits > 48) { > + PhysicalAddressBits = 48; > } > > NumberOfPml5EntriesNeeded = 1; > - if (mPhysicalAddressBits > 48) { > - NumberOfPml5EntriesNeeded = (UINTN) LShiftU64 (1, mPhysicalAddressBits - 48); > - mPhysicalAddressBits = 48; > + if (PhysicalAddressBits > 48) { > + NumberOfPml5EntriesNeeded = (UINTN) LShiftU64 (1, PhysicalAddressBits - 48); > + PhysicalAddressBits = 48; > } > > NumberOfPml4EntriesNeeded = 1; > - if (mPhysicalAddressBits > 39) { > - NumberOfPml4EntriesNeeded = (UINTN) LShiftU64 (1, mPhysicalAddressBits - 39); > - mPhysicalAddressBits = 39; > + if (PhysicalAddressBits > 39) { > + NumberOfPml4EntriesNeeded = (UINTN) LShiftU64 (1, PhysicalAddressBits - 39); > + PhysicalAddressBits = 39; > } > > NumberOfPdpEntriesNeeded = 1; > - ASSERT (mPhysicalAddressBits > 30); > - NumberOfPdpEntriesNeeded = (UINTN) LShiftU64 (1, mPhysicalAddressBits - 30); > + ASSERT (PhysicalAddressBits > 30); > + NumberOfPdpEntriesNeeded = (UINTN) LShiftU64 (1, PhysicalAddressBits - 30); > > // > // By architecture only one PageMapLevel4 exists - so lets allocate storage for it. > @@ -438,7 +440,7 @@ SmmInitPageTable ( > // When access to non-SMRAM memory is restricted, create page table > // that covers all memory space. > // > - SetStaticPageTable ((UINTN)PTEntry); > + SetStaticPageTable ((UINTN)PTEntry, mPhysicalAddressBits); > } else { > // > // Add pages to page pool >