From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f182.google.com (mail-pl1-f182.google.com [209.85.214.182]) by mx.groups.io with SMTP id smtpd.web11.16740.1618421211649297839 for ; Wed, 14 Apr 2021 10:26:51 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20161025 header.b=dSGCM9Cm; spf=pass (domain: gmail.com, ip: 209.85.214.182, mailfrom: kuqin12@gmail.com) Received: by mail-pl1-f182.google.com with SMTP id z22so5387857plo.3 for ; Wed, 14 Apr 2021 10:26:51 -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=ZRmn91SCuzX3uj091fD5jskHx+pD3FZD5LE1Ga9djsU=; b=dSGCM9Cm6j5V7vDiVQHRgtRzGZbqMdIzEg/DeDjhpXe5/lstkJwIdNbuTnIEZzeNyd hHUPUzEd11K6+39Dg09B/7dR/eLiYb84l1fP/R5WcETYsnTo4P/BpVjs3a8KzkduI3xO pYFidC4vqtIJu1yDVDZfb1YLvBLMrYw4m3Jv+ldmnjNqmReb/5NXd477Mtqr6nmSKx9Z JTsy1JRNdPA+TRX9wmAm+WJqIW8u5ouMu1xTJOJg8hPLYOqniml9bXE5v46FptO2oWiR yzgxNk7N3FOmW+a7EsjL+2TKPIHL/tYFZM1/haytZ8OlHyTE6V9qP7XEFnJDz7TH6GFE 1WCw== 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=ZRmn91SCuzX3uj091fD5jskHx+pD3FZD5LE1Ga9djsU=; b=SBJPbA38ONkyYahfbG0vTWtNR0uNo2XMbRZZ1ntxkOq2gvFh0eDkWxDExLDFKIQLfd J7I9OvC6eJiA2bT4u66inn4/egraOJLk9oDc0MQvWXAImWFab1tzn6mxUsuB9ANzTJSh QyF5g5V1HLBDz37esMT4RWfGFhAiYsSZvDTsYA7FWnuyuZ7jGnEzYDZ0fw+bdg08rKQe 8t4RY/hBtxKBTNEP0ao1BvDWwULbs/ibl37608UqHxS5vu6LlCv0gVghgDfRNAKQwPxa u02YipRKU36LcEnRprppYOG4GE5ghVQiPUbP2CkRmQgscoY8O/UJwffrLy3ni6R2Wdcq JEmg== X-Gm-Message-State: AOAM530+vl21Mf5N0GN4pkxcWngeItoKx/b/Wj4+oQFzbkE1QWdc066/ ln4op6/mX/3uyFKhSU9DWTs= X-Google-Smtp-Source: ABdhPJwGBdkc3ZrixmHIG5WXwqpYiQ0u89jNCPWWDRjDggFXtpYHr5t6Q5oNnKQSqwGCLR/gmth80g== X-Received: by 2002:a17:90a:1b42:: with SMTP id q60mr4875010pjq.230.1618421211123; Wed, 14 Apr 2021 10:26:51 -0700 (PDT) Return-Path: Received: from [192.168.50.18] ([50.35.88.161]) by smtp.gmail.com with ESMTPSA id l18sm137408pgh.70.2021.04.14.10.26.49 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 14 Apr 2021 10:26:50 -0700 (PDT) Subject: Re: [PATCH v1 1/1] UefiCpuPkg: PiSmmCpuDxeSmm: Not to Change Bitwidth During Static Paging To: "Ni, Ray" , "devel@edk2.groups.io" Cc: "Dong, Eric" , Laszlo Ersek , "Kumar, Rahul1" References: <20210414025922.850-1-kuqin12@gmail.com> <20210414025922.850-2-kuqin12@gmail.com> From: "Kun Qin" Message-ID: <7da47291-8e88-be85-d61d-32b917e9f9e4@gmail.com> Date: Wed, 14 Apr 2021 10:26:49 -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: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Hi Ray, I do not have a preference in using local variable or input parameter. I can change the SetStaticPageTable interface to accept bitwidth in v2. Regards, Kun On 04/14/2021 02:49, Ni, Ray wrote: > Is it possible to let SetStaticPageTable() accept another parameter "PhysicalAddressBits" so it doesn't update the global one? > Using this way, SetStaticPageTable() can avoid reference the global variable completely. > >> -----Original Message----- >> From: Kun Qin >> Sent: Wednesday, April 14, 2021 10:59 AM >> To: devel@edk2.groups.io >> Cc: Dong, Eric ; Ni, Ray ; Laszlo >> Ersek ; Kumar, Rahul1 >> Subject: [PATCH v1 1/1] UefiCpuPkg: PiSmmCpuDxeSmm: Not to Change >> Bitwidth During Static Paging >> >> 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 replaced the changed bitwidth to a stack based variable. >> >> Cc: Eric Dong >> Cc: Ray Ni >> Cc: Laszlo Ersek >> Cc: Rahul Kumar >> >> Signed-off-by: Kun Qin >> --- >> UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 25 +++++++++++--------- >> 1 file changed, 14 insertions(+), 11 deletions(-) >> >> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c >> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c >> index 6902584b1fbd..0caee8a27abe 100644 >> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c >> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c >> @@ -226,6 +226,7 @@ SetStaticPageTable ( >> UINTN IndexOfPml4Entries; >> UINTN IndexOfPdpEntries; >> UINTN IndexOfPageDirectoryEntries; >> + UINT64 PhysicalAddressBits; >> UINT64 *PageMapLevel5Entry; >> UINT64 *PageMapLevel4Entry; >> UINT64 *PageMap; >> @@ -237,26 +238,28 @@ 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; >> + PhysicalAddressBits = mPhysicalAddressBits; >> + >> + 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. >> -- >> 2.31.0.windows.1 >