Re: [edk2-devel] [PATCH 0/6] Substract TME-MK KEY_ID_BITS from CPU max PA

Re: [edk2-devel] [PATCH 0/6] Substract TME-MK KEY_ID_BITS from CPU max PA

[Ni, Ray]

[-- Attachment #1: Type: text/plain, Size: 1333 bytes --]

Gerd, Tom,
Can you please review this patch series?

thanks,
ray
________________________________
From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Ni, Ray <ray.ni@intel.com>
Sent: Wednesday, March 22, 2023 7:56:44 AM
To: devel@edk2.groups.io <devel@edk2.groups.io>
Subject: [edk2-devel] [PATCH 0/6] Substract TME-MK KEY_ID_BITS from CPU max PA


Ray Ni (6):
  MdePkg: Add TME-MK related CPUID and MSR definitions
  UefiCpuPkg/MtrrTest: Only claim CPUID max leaf as 1
  UefiCpuPkg/MtrrLib: Substract TME-MK KEY_ID_BITS from CPU max PA
  UefiCpuPkg/CpuDxe: Refactor to use CPUID definitions
  UefiCpuPkg/CpuDxe: Substract TME-MK KEY_ID_BITS from CPU max PA
  UefiCpuPkg/MtrrTest: Add test cases for TME-MK enable case

 .../Include/Register/Intel/ArchitecturalMsr.h | 106 ++++++++++++++-
 MdePkg/Include/Register/Intel/Cpuid.h         |   9 +-
 UefiCpuPkg/CpuDxe/CpuDxe.c                    |  38 ++++--
 UefiCpuPkg/CpuDxe/CpuDxe.h                    |   3 +-
 UefiCpuPkg/Library/MtrrLib/MtrrLib.c          |  24 +++-
 .../MtrrLib/UnitTest/MtrrLibUnitTest.c        |  18 +--
 .../MtrrLib/UnitTest/MtrrLibUnitTest.h        |   3 +-
 UefiCpuPkg/Library/MtrrLib/UnitTest/Support.c | 126 ++++++++++++++----
 8 files changed, 278 insertions(+), 49 deletions(-)

--
2.39.1.windows.1







[-- Attachment #2: Type: text/html, Size: 2676 bytes --]

Re: [edk2-devel] [PATCH 0/6] Substract TME-MK KEY_ID_BITS from CPU max PA

[Ni, Ray]

[-- Attachment #1: Type: text/plain, Size: 1315 bytes --]

Mike, Eric,
Ca you kindly review?

thanks,
ray
________________________________
From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Ni, Ray <ray.ni@intel.com>
Sent: Wednesday, March 22, 2023 7:56:44 AM
To: devel@edk2.groups.io <devel@edk2.groups.io>
Subject: [edk2-devel] [PATCH 0/6] Substract TME-MK KEY_ID_BITS from CPU max PA


Ray Ni (6):
  MdePkg: Add TME-MK related CPUID and MSR definitions
  UefiCpuPkg/MtrrTest: Only claim CPUID max leaf as 1
  UefiCpuPkg/MtrrLib: Substract TME-MK KEY_ID_BITS from CPU max PA
  UefiCpuPkg/CpuDxe: Refactor to use CPUID definitions
  UefiCpuPkg/CpuDxe: Substract TME-MK KEY_ID_BITS from CPU max PA
  UefiCpuPkg/MtrrTest: Add test cases for TME-MK enable case

 .../Include/Register/Intel/ArchitecturalMsr.h | 106 ++++++++++++++-
 MdePkg/Include/Register/Intel/Cpuid.h         |   9 +-
 UefiCpuPkg/CpuDxe/CpuDxe.c                    |  38 ++++--
 UefiCpuPkg/CpuDxe/CpuDxe.h                    |   3 +-
 UefiCpuPkg/Library/MtrrLib/MtrrLib.c          |  24 +++-
 .../MtrrLib/UnitTest/MtrrLibUnitTest.c        |  18 +--
 .../MtrrLib/UnitTest/MtrrLibUnitTest.h        |   3 +-
 UefiCpuPkg/Library/MtrrLib/UnitTest/Support.c | 126 ++++++++++++++----
 8 files changed, 278 insertions(+), 49 deletions(-)

--
2.39.1.windows.1







[-- Attachment #2: Type: text/html, Size: 2584 bytes --]

Re: [edk2-devel] [PATCH 0/6] Substract TME-MK KEY_ID_BITS from CPU max PA

[Ni, Ray]

[-- Attachment #1: Type: text/plain, Size: 2027 bytes --]

Tom,
I would assume this won't break any AMD system because the CPUID bit doesn't declare this feature.

Gerd,
This is needed by Intel platform for TME-MK. Can you help to analyze if it has any impact to OVMF? I assume no.


Thanks,
Ray

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
Sent: Tuesday, March 28, 2023 10:10 PM
To: devel@edk2.groups.io
Cc: 'Gerd Hoffmann' <kraxel@redhat.com>; Tom Lendacky <thomas.lendacky@amd.com>
Subject: Re: [edk2-devel] [PATCH 0/6] Substract TME-MK KEY_ID_BITS from CPU max PA

Gerd, Tom,
Can you please review this patch series?

thanks,
ray
________________________________
From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> on behalf of Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: Wednesday, March 22, 2023 7:56:44 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>
Subject: [edk2-devel] [PATCH 0/6] Substract TME-MK KEY_ID_BITS from CPU max PA


Ray Ni (6):
  MdePkg: Add TME-MK related CPUID and MSR definitions
  UefiCpuPkg/MtrrTest: Only claim CPUID max leaf as 1
  UefiCpuPkg/MtrrLib: Substract TME-MK KEY_ID_BITS from CPU max PA
  UefiCpuPkg/CpuDxe: Refactor to use CPUID definitions
  UefiCpuPkg/CpuDxe: Substract TME-MK KEY_ID_BITS from CPU max PA
  UefiCpuPkg/MtrrTest: Add test cases for TME-MK enable case

 .../Include/Register/Intel/ArchitecturalMsr.h | 106 ++++++++++++++-
 MdePkg/Include/Register/Intel/Cpuid.h         |   9 +-
 UefiCpuPkg/CpuDxe/CpuDxe.c                    |  38 ++++--
 UefiCpuPkg/CpuDxe/CpuDxe.h                    |   3 +-
 UefiCpuPkg/Library/MtrrLib/MtrrLib.c          |  24 +++-
 .../MtrrLib/UnitTest/MtrrLibUnitTest.c        |  18 +--
 .../MtrrLib/UnitTest/MtrrLibUnitTest.h        |   3 +-
 UefiCpuPkg/Library/MtrrLib/UnitTest/Support.c | 126 ++++++++++++++----
 8 files changed, 278 insertions(+), 49 deletions(-)

--
2.39.1.windows.1







[-- Attachment #2: Type: text/html, Size: 5972 bytes --]

Re: [edk2-devel] [PATCH 0/6] Substract TME-MK KEY_ID_BITS from CPU max PA

[Gerd Hoffmann]

On Thu, Mar 30, 2023 at 02:26:25AM +0000, Ni, Ray wrote:
> Gerd,
> This is needed by Intel platform for TME-MK. Can you help to analyze if it has any impact to OVMF? I assume no.

How does that feature interact with vt and tdx?

take care,
  Gerd

Re: [edk2-devel] [PATCH 0/6] Substract TME-MK KEY_ID_BITS from CPU max PA

[Ni, Ray]

I don' t think vt is related.

For tdx, the actual max physical address bits is decreased by the KEY_ID_BITS bits.
But the max physical address bits reported from CPUID instruction don't change.

Details in https://cdrdv2-public.intel.com/679154/multi-key-total-memory-encryption-spec-1.4.pdf



> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd
> Hoffmann
> Sent: Thursday, March 30, 2023 3:25 PM
> To: Ni, Ray <ray.ni@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>; devel@edk2.groups.io
> Subject: Re: [edk2-devel] [PATCH 0/6] Substract TME-MK KEY_ID_BITS from
> CPU max PA
> 
> On Thu, Mar 30, 2023 at 02:26:25AM +0000, Ni, Ray wrote:
> > Gerd,
> > This is needed by Intel platform for TME-MK. Can you help to analyze if it
> has any impact to OVMF? I assume no.
> 
> How does that feature interact with vt and tdx?
> 
> take care,
>   Gerd
> 
> 
> 
> 
> 

Re: [edk2-devel] [PATCH 0/6] Substract TME-MK KEY_ID_BITS from CPU max PA

[Gerd Hoffmann]

  Hi,

> For tdx, the actual max physical address bits is decreased by the KEY_ID_BITS bits.
> But the max physical address bits reported from CPUID instruction don't change.

I guess the physical address bits calculation for tdx needs adjustment
then.  Right now we have:

  if (TdIsEnabled ()) {
    if (TdSharedPageMask () == (1ULL << 47)) {
      PhysMemAddressWidth = 48;
    } else {
      PhysMemAddressWidth = 52;
    }
  }


See PlatformAddressWidthInitialization() in PlatformInitLib/MemDetect.c

Which would be the third place needing the same logic.  Maybe worth
thinking about a helper function in a library somewhere, so we don't
cut&paste the same code snippet again and again ...

take care,
  Gerd

Re: [edk2-devel] [PATCH 0/6] Substract TME-MK KEY_ID_BITS from CPU max PA

[Ni, Ray]

@Xu, Min M, can you check if Gerd's comment is valid?

Gerd,
This Bugzilla captures the same idea: https://bugzilla.tianocore.org/show_bug.cgi?id=3394
Originally from Laszlo. +@Liu, Zhiguang who will work on patch submission later.

Thanks,
Ray

> -----Original Message-----
> From: kraxel@redhat.com <kraxel@redhat.com>
> Sent: Thursday, March 30, 2023 5:03 PM
> To: Ni, Ray <ray.ni@intel.com>
> Cc: devel@edk2.groups.io; Tom Lendacky <thomas.lendacky@amd.com>
> Subject: Re: [edk2-devel] [PATCH 0/6] Substract TME-MK KEY_ID_BITS from CPU max PA
> 
>   Hi,
> 
> > For tdx, the actual max physical address bits is decreased by the KEY_ID_BITS bits.
> > But the max physical address bits reported from CPUID instruction don't change.
> 
> I guess the physical address bits calculation for tdx needs adjustment
> then.  Right now we have:
> 
>   if (TdIsEnabled ()) {
>     if (TdSharedPageMask () == (1ULL << 47)) {
>       PhysMemAddressWidth = 48;
>     } else {
>       PhysMemAddressWidth = 52;
>     }
>   }
> 
> 
> See PlatformAddressWidthInitialization() in PlatformInitLib/MemDetect.c
> 
> Which would be the third place needing the same logic.  Maybe worth
> thinking about a helper function in a library somewhere, so we don't
> cut&paste the same code snippet again and again ...
> 
> take care,
>   Gerd

Re: [edk2-devel] [PATCH 0/6] Substract TME-MK KEY_ID_BITS from CPU max PA

[Ni, Ray]

Gerd,
Can you give a Reviewed-by/Acked-by for the patch series?

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
> Sent: Friday, March 31, 2023 3:25 PM
> To: kraxel@redhat.com; Xu, Min M <min.m.xu@intel.com>; Liu, Zhiguang
> <zhiguang.liu@intel.com>
> Cc: devel@edk2.groups.io; Tom Lendacky <thomas.lendacky@amd.com>
> Subject: Re: [edk2-devel] [PATCH 0/6] Substract TME-MK KEY_ID_BITS from
> CPU max PA
> 
> @Xu, Min M, can you check if Gerd's comment is valid?
> 
> Gerd,
> This Bugzilla captures the same idea:
> https://bugzilla.tianocore.org/show_bug.cgi?id=3394
> Originally from Laszlo. +@Liu, Zhiguang who will work on patch submission
> later.
> 
> Thanks,
> Ray
> 
> > -----Original Message-----
> > From: kraxel@redhat.com <kraxel@redhat.com>
> > Sent: Thursday, March 30, 2023 5:03 PM
> > To: Ni, Ray <ray.ni@intel.com>
> > Cc: devel@edk2.groups.io; Tom Lendacky <thomas.lendacky@amd.com>
> > Subject: Re: [edk2-devel] [PATCH 0/6] Substract TME-MK KEY_ID_BITS
> from CPU max PA
> >
> >   Hi,
> >
> > > For tdx, the actual max physical address bits is decreased by the
> KEY_ID_BITS bits.
> > > But the max physical address bits reported from CPUID instruction don't
> change.
> >
> > I guess the physical address bits calculation for tdx needs adjustment
> > then.  Right now we have:
> >
> >   if (TdIsEnabled ()) {
> >     if (TdSharedPageMask () == (1ULL << 47)) {
> >       PhysMemAddressWidth = 48;
> >     } else {
> >       PhysMemAddressWidth = 52;
> >     }
> >   }
> >
> >
> > See PlatformAddressWidthInitialization() in PlatformInitLib/MemDetect.c
> >
> > Which would be the third place needing the same logic.  Maybe worth
> > thinking about a helper function in a library somewhere, so we don't
> > cut&paste the same code snippet again and again ...
> >
> > take care,
> >   Gerd
> 
> 
> 
> 
> 

Re: [edk2-devel] [PATCH 0/6] Substract TME-MK KEY_ID_BITS from CPU max PA

[Gerd Hoffmann]

On Fri, Mar 31, 2023 at 08:02:00AM +0000, Ni, Ray wrote:
> Gerd,
> Can you give a Reviewed-by/Acked-by for the patch series?

> > This Bugzilla captures the same idea:
> > https://bugzilla.tianocore.org/show_bug.cgi?id=3394

Oh, the bug lists even more places which care about
the physical address width.

Sure you want merge the series in this apparently
incomplete state?

It should have no bad effects on OVMF though, so if you
prefer to merge this as-is and implement the suggested
library solution as separate patch series:

Acked-by: Gerd Hoffmann <kraxel@redhat.com>

take care,
  Gerd

Re: [edk2-devel] [PATCH 0/6] Substract TME-MK KEY_ID_BITS from CPU max PA

[Lendacky, Thomas]

On 3/29/23 21:26, Ni, Ray wrote:
> Tom,
> I would assume this won't break any AMD system because the CPUID bit doesn't declare this feature.

Right, shouldn't be an issue on AMD systems.

Though it does bring to mind that we should probably do something similar 
for AMD at some point. With SEV there is a 1-bit physical address 
reduction (reported via CPUID) in the guest (but it should only be 
reported in CPUID if the encryption bit is part of the reported max 
physical address).

Thanks,
Tom

> 
> Gerd,
> This is needed by Intel platform for TME-MK. Can you help to analyze if it has any impact to OVMF? I assume no.
> 
> 
> Thanks,
> Ray
> 
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
> Sent: Tuesday, March 28, 2023 10:10 PM
> To: devel@edk2.groups.io
> Cc: 'Gerd Hoffmann' <kraxel@redhat.com>; Tom Lendacky <thomas.lendacky@amd.com>
> Subject: Re: [edk2-devel] [PATCH 0/6] Substract TME-MK KEY_ID_BITS from CPU max PA
> 
> Gerd, Tom,
> Can you please review this patch series?
> 
> thanks,
> ray
> ________________________________
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> on behalf of Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
> Sent: Wednesday, March 22, 2023 7:56:44 AM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>
> Subject: [edk2-devel] [PATCH 0/6] Substract TME-MK KEY_ID_BITS from CPU max PA
> 
> 
> Ray Ni (6):
>    MdePkg: Add TME-MK related CPUID and MSR definitions
>    UefiCpuPkg/MtrrTest: Only claim CPUID max leaf as 1
>    UefiCpuPkg/MtrrLib: Substract TME-MK KEY_ID_BITS from CPU max PA
>    UefiCpuPkg/CpuDxe: Refactor to use CPUID definitions
>    UefiCpuPkg/CpuDxe: Substract TME-MK KEY_ID_BITS from CPU max PA
>    UefiCpuPkg/MtrrTest: Add test cases for TME-MK enable case
> 
>   .../Include/Register/Intel/ArchitecturalMsr.h | 106 ++++++++++++++-
>   MdePkg/Include/Register/Intel/Cpuid.h         |   9 +-
>   UefiCpuPkg/CpuDxe/CpuDxe.c                    |  38 ++++--
>   UefiCpuPkg/CpuDxe/CpuDxe.h                    |   3 +-
>   UefiCpuPkg/Library/MtrrLib/MtrrLib.c          |  24 +++-
>   .../MtrrLib/UnitTest/MtrrLibUnitTest.c        |  18 +--
>   .../MtrrLib/UnitTest/MtrrLibUnitTest.h        |   3 +-
>   UefiCpuPkg/Library/MtrrLib/UnitTest/Support.c | 126 ++++++++++++++----
>   8 files changed, 278 insertions(+), 49 deletions(-)
> 
> --
> 2.39.1.windows.1
> 
> 
> 
> 
> 
> 
>