Re: [edk2-devel] [Patch V3] UefiCpuPkg:Limit PhysicalAddressBits in special case

["duntan" <dun.tan@intel.com>]

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 <kraxel@redhat.com> 
Sent: Thursday, January 11, 2024 6:21 PM
To: Tan, Dun <dun.tan@intel.com>
Cc: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Kumar, Rahul R <rahul.r.kumar@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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-