Hi Gerd,

I explain the reason why 47 here is since virtual addresses are sign-extended 
in the commit message.
About the technical background, I also mentioned in the commit message " When 
5-Level Paging is disabled and the PhysicalAddressBits retrived  from CPU HOB 
or CpuId is bigger than 47". Could you please provide more detailed suggestion 
about the commit message?

Or can we merge the code firstly? Then I'll raise another PR to make the 
comments around the code more detailed as we want.

Thanks,
Dun

-----Original Message-----
From: Gerd Hoffmann <kra...@redhat.com> 
Sent: Thursday, January 11, 2024 6:21 PM
To: Tan, Dun <dun....@intel.com>
Cc: devel@edk2.groups.io; Ni, Ray <ray...@intel.com>; Laszlo Ersek 
<ler...@redhat.com>; Kumar, Rahul R <rahul.r.ku...@intel.com>
Subject: Re: [Patch V3] UefiCpuPkg:Limit PhysicalAddressBits in special case

On Thu, Jan 11, 2024 at 04:59:47PM +0800, Dun Tan wrote:
> When creating smm page table, limit maximum supported physical address 
> bits returned by
> CalculateMaximumSupportAddress() to 47 if 5-Level Paging is disabled.
> When 5-Level Paging is disabled and the PhysicalAddressBits retrived 
> from CPU HOB or CpuId is bigger than 47, and since virtual addresses 
> are sign-extended, only [0, 2^47-1] range in 52-bit physical address 
> is mapped in page table.

> +  //
> +  // Only [0, 2^47 -1] in 52-bit physical addresses is mapped in page 
> + table  // when 5-Level Paging is disabled.
> +  //
> +  ASSERT (PhysicalAddressBits <= 52);  if (!Is5LevelPagingNeeded && 
> + (PhysicalAddressBits > 47)) {
> +    PhysicalAddressBits = 47;
> +  }

The code change is fine but the comment should be more verbose and explain the 
why 47 not 48 is used here.  The discussion on the patch clearly showed that 
the technical background is not obvious ...

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113636): https://edk2.groups.io/g/devel/message/113636
Mute This Topic: https://groups.io/mt/103658816/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to