1. devel@edk2.groups.io
  2. Messages
   Date   
Re: [Patch V2] MinPlatformPkg: Fix the incompatible change about SecureBootVariableLib

Chiu, Chasel
 

Reviewed-by: Chasel Chiu <chasel.chiu@...>

-----Original Message-----
From: Tan, Dun <dun.tan@...>
Sent: Monday, August 9, 2021 11:00 PM
To: devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@...>; Desimone, Nathaniel L
<nathaniel.l.desimone@...>; Liming Gao <gaoliming@...>;
Dong, Eric <eric.dong@...>; Tan, Dun <dun.tan@...>
Subject: [Patch V2] MinPlatformPkg: Fix the incompatible change about
SecureBootVariableLib

V1: The newly created lib will be consumed by SecureBootConfigDxe.inf in
CoreDxeInclude.dsc
V2: Add SecureBootVariableProvisionLib in CoreDxeInclude.dsc

Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Liming Gao <gaoliming@...>
Cc: Eric Dong <eric.dong@...>
Signed-off-by: DunTan <dun.tan@...>
---
Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc | 2 ++
1 file changed, 2 insertions(+)

diff --git a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
index b154f9615d..c3d05fc913 100644
--- a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
+++ b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
@@ -139,6 +139,8 @@

!if gMinPlatformPkgTokenSpaceGuid.PcdUefiSecureBootEnable == TRUE
AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
+
+ SecureBootVariableLib|SecurityPkg/Library/SecureBootVariableLib/Secure
+ BootVariableLib.inf
+ SecureBootVariableProvisionLib|SecurityPkg/Library/SecureBootVariableP
+ rovisionLib/SecureBootVariableProvisionLib.inf
!endif

SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
--
2.31.1.windows.1
Re: [PATCH 1/1] OvmfPkg PlatformBootManagerLib: Move TryRunningQemuKernel()

Christoph Willing
 

On 10/8/21 12:52 am, James Bottomley wrote:
On Mon, 2021-08-09 at 22:53 +1000, Christoph Willing wrote:
With soft feature freeze started, I wonder if this patch could be
reviewed and pushed for edk2-stable202108 tag? I think it has
languished because I didn't initially Cc appropriately - pls add
others as necessary.

This patch is a trivial (I think) change which fixes a long standing
and annoying bug for those booting Qemu with UEFI using external
kernel & initrd.
I'm with Ard on this one: -kernel is working just fine for me and the
team at IBM working on Kata containers. It sounds like this might be a
problem local to your environment, so we need to debug it to understand
the issue rather than blindly reverse existing commits.
Thanks for responding James & Ard.

Below is the script I'm using to create, then run, the VM. To verify
that it works normally with UEFI boot, it initially uses the internal
kernel & initrd.

The OVMF_CODE & my_VARS lines contain git hash to identify the build
from which OVMF_CODE.fd & OVMF_VARS.fd were taken; 97fdcg is from a
build of yesterday's git master.

After the OS has been installed, I can run the VM multiple times to
verify that it boots under UEFI OK (I see the TianoCore splash screen)
with internal kernel.


#!/bin/bash

/usr/bin/qemu-kvm \
-name "UEFI Testing" \
-enable-kvm \
-cpu kvm64 \
-smp cores=4 \
-boot once=c \
-m 8192 \
-device intel-hda \
-device hda-duplex \
-vga virtio \
-drive if=pflash,format=raw,file=OVMF_CODE_97fdcb.fd,readonly=on \
-drive if=pflash,format=raw,file=my_VARS_97fdcb.fd \
-drive file=disk.img,format=raw,cache=none,index=0,media=disk \
-cdrom
/storage/iso/slackware/slackware64-15.0/slackware64-15.0-20210807.iso \
-daemonize \
"$@"


To now use external kernel, I add the lines:

-kernel /var/cache/vmbuilder/boot/15.0/x86_64/vmlinuz \
-initrd /var/cache/vmbuilder/boot/15.0/x86_64/initrd \
-append "root=/dev/sda2 rootfstype=ext4 ro vga=0x386" \

to the script just after "-boot once=c" (but I doubt the exact
positioning makes any difference).

In this case, I see the kernel running and initrd unpacked and its
modules loaded but the root partition is unable to be mounted - the disk
is not visible (running 'ls -l /dev/sd*' in recovery shell gives 'ls:
/dev/sd*: No such file or directory').

The last lines of the Qemu screen are:

/boot/initrd-5.13.8.gz: Loading kernel modules from initrd image:
insmod /lib/modules/5.13.8/kernel/fs/jbd2/jbd2.ko
insmod /lib/modules/5.13.8/kernel/fs/mbcache.ko
insmod /lib/modules/5.13.8/kernel/fs/ext4/ext4.ko
mount: mounting /dev/sda2 on /mnt failed: No such file or directory
ERROR: No /sbin/init found on rootdev (or not mounted). Trouble ahead.
You can try to fix it. Type 'exit' when things are done.

At that point I'm dropped into a recovery shell to try fixing something
but there's nothing that can be done since the disk containing the OS is
not visible.


However if I now change the script's OVMF files to those built from a
patched git master, the VM boots all the way to login prompt.

I'm using qemu-6.0.0 on SLackware64 but I've found exactly the same
behaviour using other OS's (Ubuntu 20.04 with 4.2-3ubuntu6.17 and Clear
Linux with 5.2.0)

I've also tried using OVMF files from Ubuntu hirsute's ovmf package
(2020.11-4) with same bad result. Of course, in this case, I was unable
to use a patched version.

From the above, I think I've done everything possible to verify the
problem and a possible fix. Is there something fundamentally wrong in
the way I'm going about this?

chris
Re: [edk2-platforms][PATCH v1 1/1] IntelSiliconPkg/PeiSmmAccessLib: Remove S3 requirement

Chaganty, Rangasai V
 

Reviewed-by: Sai Chaganty <rangasai.v.chaganty@...>

-----Original Message-----
From: mikuback@... <mikuback@...>
Sent: Monday, August 09, 2021 6:40 AM
To: devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@...>; Chaganty, Rangasai V <rangasai.v.chaganty@...>
Subject: [edk2-platforms][PATCH v1 1/1] IntelSiliconPkg/PeiSmmAccessLib: Remove S3 requirement

From: Michael Kubacki <michael.kubacki@...>

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3539

PeiInstallSmmAccessPpi() currently requires the boot mode be set to S3 to actually install gEfiPeiMmAccessPpiGuid.

This change removes this requirement in the function implementation for two reasons:

1. Practical use cases exist to require this PPI in cases other than
the boot mode being set to BOOT_ON_S3_RESUME.

2. It is poor API design to implicitly bury this requirement within
a function whose responsibility is to install the PPI. The caller
can easily place arbitrary constraints around whether to call
based on conditions such as the boot mode being
BOOT_ON_S3_RESUME.

Cc: Ray Ni <ray.ni@...>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>
---
Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLib/PeiSmmAccessLib.c | 12 ------------
1 file changed, 12 deletions(-)

diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLib/PeiSmmAccessLib.c b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLib/PeiSmmAccessLib.c
index d9bf4fba983e..4df0d695fdaf 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLib/PeiSmmAccessLib.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAcce
+++ ssLib/PeiSmmAccessLib.c
@@ -252,19 +252,7 @@ PeiInstallSmmAccessPpi (
EFI_SMRAM_HOB_DESCRIPTOR_BLOCK *DescriptorBlock;
SMM_ACCESS_PRIVATE_DATA *SmmAccessPrivate;
VOID *HobList;
- EFI_BOOT_MODE BootMode;

- Status = PeiServicesGetBootMode (&BootMode);
- if (EFI_ERROR (Status)) {
- //
- // If not in S3 boot path. do nothing
- //
- return EFI_SUCCESS;
- }
-
- if (BootMode != BOOT_ON_S3_RESUME) {
- return EFI_SUCCESS;
- }
//
// Initialize private data
//
--
2.28.0.windows.1
Re: [edk2-platforms][PATCH v1 1/1] IntelSiliconPkg: Add BaseSmmAccessLibNull

Chaganty, Rangasai V
 

Reviewed-by: Sai Chaganty <rangasai.v.chaganty@...>

-----Original Message-----
From: mikuback@... <mikuback@...>
Sent: Monday, August 09, 2021 7:16 AM
To: devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@...>; Chaganty, Rangasai V <rangasai.v.chaganty@...>
Subject: [edk2-platforms][PATCH v1 1/1] IntelSiliconPkg: Add BaseSmmAccessLibNull

From: Michael Kubacki <michael.kubacki@...>

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3540

Adds a NULL instance of SmmAccessLib.

Cc: Ray Ni <ray.ni@...>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>
---
Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/BaseSmmAccessLibNull/BaseSmmAccessLibNull.c | 33 ++++++++++++++++++++
Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/BaseSmmAccessLibNull/BaseSmmAccessLibNull.inf | 26 +++++++++++++++
Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc | 1 +
3 files changed, 60 insertions(+)

diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/BaseSmmAccessLibNull/BaseSmmAccessLibNull.c b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/BaseSmmAccessLibNull/BaseSmmAccessLibNull.c
new file mode 100644
index 000000000000..f5ad306b380b
--- /dev/null
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/BaseSmmAcc
+++ essLibNull/BaseSmmAccessLibNull.c
@@ -0,0 +1,33 @@
+/** @file
+ A NULL library instance of SmmAccessLib.
+
+ Copyright (c) 2019 - 2020, Intel Corporation. All rights
+ reserved.<BR> Copyright (c) Microsoft Corporation.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Uefi.h>
+#include <Library/DebugLib.h>
+#include <Library/SmmAccessLib.h>
+
+/**
+ This function is to install an SMM Access PPI
+
+ @retval EFI_SUCCESS - Ppi successfully started and installed.
+ @retval EFI_NOT_FOUND - Ppi can't be found.
+ @retval EFI_OUT_OF_RESOURCES - Ppi does not have enough resources to initialize the driver.
+ @retval EFI_UNSUPPORTED - The PPI was not installed and installation is unsupported in
+ this instance of function implementation.
+
+**/
+EFI_STATUS
+EFIAPI
+PeiInstallSmmAccessPpi (
+ VOID
+ )
+{
+ ASSERT (FALSE);
+ return EFI_UNSUPPORTED;
+}
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/BaseSmmAccessLibNull/BaseSmmAccessLibNull.inf b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/BaseSmmAccessLibNull/BaseSmmAccessLibNull.inf
new file mode 100644
index 000000000000..7fd3b0b89655
--- /dev/null
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/BaseSmmAcc
+++ essLibNull/BaseSmmAccessLibNull.inf
@@ -0,0 +1,26 @@
+## @file
+# A NULL library instance of SmmAccessLib.
+#
+# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR> #
+Copyright (c) Microsoft Corporation.<BR> # SPDX-License-Identifier:
+BSD-2-Clause-Patent # ##
+
+[Defines]
+INF_VERSION = 0x00010017
+BASE_NAME = BaseSmmAccessLibNull
+FILE_GUID = C1A14AB6-B757-4046-9B92-9DCE1A2154C6
+VERSION_STRING = 1.0
+MODULE_TYPE = BASE
+LIBRARY_CLASS = SmmAccessLib
+
+[Packages]
+ MdePkg/MdePkg.dec
+ IntelSiliconPkg/IntelSiliconPkg.dec
+
+[LibraryClasses]
+ DebugLib
+
+[Sources]
+ BaseSmmAccessLibNull.c
diff --git a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc
index 1092371d848e..dd0928ec58f3 100644
--- a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc
+++ b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc
@@ -90,6 +90,7 @@ [Components]
IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdateDxe.inf
IntelSiliconPkg/Feature/Capsule/Library/MicrocodeFlashAccessLibNull/MicrocodeFlashAccessLibNull.inf
IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocodePei.inf
+
+ IntelSiliconPkg/Feature/SmmAccess/Library/BaseSmmAccessLibNull/BaseSmm
+ AccessLibNull.inf
IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/PeiFirmwareBootMediaLib.inf
IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/DxeSmmFirmwareBootMediaLib.inf
IntelSiliconPkg/Library/DxeAslUpdateLib/DxeAslUpdateLib.inf
--
2.28.0.windows.1
Re: [PATCH v2 1/2] MdePkg/BaseLib: Fix unaligned API prototypes

Andrew Fish
 



On Aug 9, 2021, at 9:15 AM, Michael D Kinney <michael.d.kinney@...> wrote:

Hi Marvin,

Can you provide an example of which C compiler is flagging this as
an error and what error message is generated.

Please enter a BZ with this background information and add link to the
BZ in the commit message.

This is a change to the BaseLib class, so we need to make sure there
are no impacts to any existing code.  I looks like a safe change
because changing from a pointer to a fixed size type to VOID * 
should be compatible.  Please add that analysis to the background
in the BZ as well.


MIke,

I want to say we had a discussion about this years ago? I don’t remember the outcome. 

Dereferencing a misaligned pointer is UB (Undefined Behavior) in C [1], but historically x86 compilers have let it slide.

I think the situation we are in is the BaseLib functions don’t contain UB, but it is UB for the caller to use the returned pointer directly. 

Here is a simple example with clang UndefinedBehaviorSanitizer (UBSan) . 

~/work/Compiler>cat ub.c
#include <stdlib.h>

#define EFIAPI
#define IN
#define OUT

typedef unsigned char UINT8;
typedef unsigned short UINT16;

UINT16
EFIAPI
WriteUnaligned16 (
  OUT UINT16                    *Buffer,
  IN  UINT16                    Value
  )
{
  // ASSERT (Buffer != NULL);

  ((volatile UINT8*)Buffer)[0] = (UINT8)Value;
  ((volatile UINT8*)Buffer)[1] = (UINT8)(Value >> 8);

  return Value;
}


int main()
{
UINT8 *buffer = malloc(64);
UINT16 *pointer = (UINT16 *)(buffer + 1);


WriteUnaligned16 (pointer, 42);


// *pointer = 42; // Error: misaligned integer pointer assignment
return *pointer;
}
~/work/Compiler>clang -fsanitize=undefined  ub.c
~/work/Compiler>./a.out
ub.c:34:9: runtime error: load of misaligned address 0x7feac6405aa1 for type 'UINT16' (aka 'unsigned short'), which requires 2 byte alignment
0x7feac6405aa1: note: pointer points here
 00 00 00  64 2a 00 79 6d 28 52 54  4c 44 5f 44 45 46 41 55  4c 54 2c 20 73 77 69 66  74 5f 64 65 6d
              ^ 
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ub.c:34:9 in 

FYI line 39 is `return *pointer`and 42 is 0x2A. So reading an writing to *pointer is UB. 


As you can see in [1] the general advice is to take code that looks like:
int8_t *buffer = malloc(64);int32_t *pointer = (int32_t *)(buffer + 1);*pointer = 42; // Error: misaligned integer pointer assignment
And replace it with;
int8_t *buffer = malloc(64);int32_t value = 42;memcpy(buffer + 1, &value, sizeof(int32_t)); // Correct

But in these cases the result is in a byte aligned buffer….


Thanks,

Andrew Fish

Thanks,

Mike


-----Original Message-----
From: Marvin Häuser <mhaeuser@...>
Sent: Monday, August 9, 2021 2:51 AM
To: devel@edk2.groups.io
Cc: Kinney, Michael D <michael.d.kinney@...>; Liming Gao <gaoliming@...>; Liu, Zhiguang
<zhiguang.liu@...>; Vitaly Cheptsov <vit9696@...>
Subject: [PATCH v2 1/2] MdePkg/BaseLib: Fix unaligned API prototypes

C prohibits not only dereferencing but also casting to unaligned
pointers. Thus, the current set of unaligned APIs cannot be called
safely. Update their prototypes to take VOID * pointers, which must
be able to represent any valid pointer.

Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Cc: Vitaly Cheptsov <vit9696@...>
Signed-off-by: Marvin Häuser <mhaeuser@...>
---
MdePkg/Library/BaseLib/Arm/Unaligned.c | 14 ++++-----
MdePkg/Library/BaseLib/Unaligned.c     | 32 ++++++++++----------
MdePkg/Include/Library/BaseLib.h       | 16 +++++-----
3 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/MdePkg/Library/BaseLib/Arm/Unaligned.c b/MdePkg/Library/BaseLib/Arm/Unaligned.c
index e9934e7003cb..57f19fc44e0b 100644
--- a/MdePkg/Library/BaseLib/Arm/Unaligned.c
+++ b/MdePkg/Library/BaseLib/Arm/Unaligned.c
@@ -59,7 +59,7 @@ ReadUnaligned16 (
UINT16

EFIAPI

WriteUnaligned16 (

-  OUT UINT16                    *Buffer,

+  OUT VOID                      *Buffer,

  IN  UINT16                    Value

  )

{

@@ -87,7 +87,7 @@ WriteUnaligned16 (
UINT32

EFIAPI

ReadUnaligned24 (

-  IN CONST UINT32              *Buffer

+  IN CONST VOID                *Buffer

  )

{

  ASSERT (Buffer != NULL);

@@ -116,7 +116,7 @@ ReadUnaligned24 (
UINT32

EFIAPI

WriteUnaligned24 (

-  OUT UINT32                    *Buffer,

+  OUT VOID                      *Buffer,

  IN  UINT32                    Value

  )

{

@@ -143,7 +143,7 @@ WriteUnaligned24 (
UINT32

EFIAPI

ReadUnaligned32 (

-  IN CONST UINT32              *Buffer

+  IN CONST VOID                *Buffer

  )

{

  UINT16  LowerBytes;

@@ -175,7 +175,7 @@ ReadUnaligned32 (
UINT32

EFIAPI

WriteUnaligned32 (

-  OUT UINT32                    *Buffer,

+  OUT VOID                      *Buffer,

  IN  UINT32                    Value

  )

{

@@ -202,7 +202,7 @@ WriteUnaligned32 (
UINT64

EFIAPI

ReadUnaligned64 (

-  IN CONST UINT64              *Buffer

+  IN CONST VOID                *Buffer

  )

{

  UINT32  LowerBytes;

@@ -234,7 +234,7 @@ ReadUnaligned64 (
UINT64

EFIAPI

WriteUnaligned64 (

-  OUT UINT64                    *Buffer,

+  OUT VOID                      *Buffer,

  IN  UINT64                    Value

  )

{

diff --git a/MdePkg/Library/BaseLib/Unaligned.c b/MdePkg/Library/BaseLib/Unaligned.c
index a419cb85e53c..3041adcde606 100644
--- a/MdePkg/Library/BaseLib/Unaligned.c
+++ b/MdePkg/Library/BaseLib/Unaligned.c
@@ -26,12 +26,12 @@
UINT16

EFIAPI

ReadUnaligned16 (

-  IN CONST UINT16              *Buffer

+  IN CONST VOID                *Buffer

  )

{

  ASSERT (Buffer != NULL);



-  return *Buffer;

+  return *(CONST UINT16 *) Buffer;

}



/**

@@ -52,13 +52,13 @@ ReadUnaligned16 (
UINT16

EFIAPI

WriteUnaligned16 (

-  OUT UINT16                    *Buffer,

+  OUT VOID                      *Buffer,

  IN  UINT16                    Value

  )

{

  ASSERT (Buffer != NULL);



-  return *Buffer = Value;

+  return *(UINT16 *) Buffer = Value;

}



/**

@@ -77,12 +77,12 @@ WriteUnaligned16 (
UINT32

EFIAPI

ReadUnaligned24 (

-  IN CONST UINT32              *Buffer

+  IN CONST VOID                *Buffer

  )

{

  ASSERT (Buffer != NULL);



-  return *Buffer & 0xffffff;

+  return *(CONST UINT32 *) Buffer & 0xffffff;

}



/**

@@ -103,13 +103,13 @@ ReadUnaligned24 (
UINT32

EFIAPI

WriteUnaligned24 (

-  OUT UINT32                    *Buffer,

+  OUT VOID                      *Buffer,

  IN  UINT32                    Value

  )

{

  ASSERT (Buffer != NULL);



-  *Buffer = BitFieldWrite32 (*Buffer, 0, 23, Value);

+  *(UINT32 *) Buffer = BitFieldWrite32 (*(CONST UINT32 *) Buffer, 0, 23, Value);

  return Value;

}



@@ -129,12 +129,12 @@ WriteUnaligned24 (
UINT32

EFIAPI

ReadUnaligned32 (

-  IN CONST UINT32              *Buffer

+  IN CONST VOID                *Buffer

  )

{

  ASSERT (Buffer != NULL);



-  return *Buffer;

+  return *(CONST UINT32 *) Buffer;

}



/**

@@ -155,13 +155,13 @@ ReadUnaligned32 (
UINT32

EFIAPI

WriteUnaligned32 (

-  OUT UINT32                    *Buffer,

+  OUT VOID                      *Buffer,

  IN  UINT32                    Value

  )

{

  ASSERT (Buffer != NULL);



-  return *Buffer = Value;

+  return *(UINT32 *) Buffer = Value;

}



/**

@@ -180,12 +180,12 @@ WriteUnaligned32 (
UINT64

EFIAPI

ReadUnaligned64 (

-  IN CONST UINT64              *Buffer

+  IN CONST VOID                *Buffer

  )

{

  ASSERT (Buffer != NULL);



-  return *Buffer;

+  return *(CONST UINT64 *) Buffer;

}



/**

@@ -206,11 +206,11 @@ ReadUnaligned64 (
UINT64

EFIAPI

WriteUnaligned64 (

-  OUT UINT64                    *Buffer,

+  OUT VOID                      *Buffer,

  IN  UINT64                    Value

  )

{

  ASSERT (Buffer != NULL);



-  return *Buffer = Value;

+  return *(UINT64 *) Buffer = Value;

}

diff --git a/MdePkg/Include/Library/BaseLib.h b/MdePkg/Include/Library/BaseLib.h
index 2452c1d92e51..4d30f0539c6b 100644
--- a/MdePkg/Include/Library/BaseLib.h
+++ b/MdePkg/Include/Library/BaseLib.h
@@ -3420,7 +3420,7 @@ DivS64x64Remainder (
UINT16

EFIAPI

ReadUnaligned16 (

-  IN CONST UINT16              *Buffer

+  IN CONST VOID                *Buffer

  );





@@ -3442,7 +3442,7 @@ ReadUnaligned16 (
UINT16

EFIAPI

WriteUnaligned16 (

-  OUT UINT16                    *Buffer,

+  OUT VOID                      *Buffer,

  IN  UINT16                    Value

  );



@@ -3463,7 +3463,7 @@ WriteUnaligned16 (
UINT32

EFIAPI

ReadUnaligned24 (

-  IN CONST UINT32              *Buffer

+  IN CONST VOID                *Buffer

  );





@@ -3485,7 +3485,7 @@ ReadUnaligned24 (
UINT32

EFIAPI

WriteUnaligned24 (

-  OUT UINT32                    *Buffer,

+  OUT VOID                      *Buffer,

  IN  UINT32                    Value

  );



@@ -3506,7 +3506,7 @@ WriteUnaligned24 (
UINT32

EFIAPI

ReadUnaligned32 (

-  IN CONST UINT32              *Buffer

+  IN CONST VOID                *Buffer

  );





@@ -3528,7 +3528,7 @@ ReadUnaligned32 (
UINT32

EFIAPI

WriteUnaligned32 (

-  OUT UINT32                    *Buffer,

+  OUT VOID                      *Buffer,

  IN  UINT32                    Value

  );



@@ -3549,7 +3549,7 @@ WriteUnaligned32 (
UINT64

EFIAPI

ReadUnaligned64 (

-  IN CONST UINT64              *Buffer

+  IN CONST VOID                *Buffer

  );





@@ -3571,7 +3571,7 @@ ReadUnaligned64 (
UINT64

EFIAPI

WriteUnaligned64 (

-  OUT UINT64                    *Buffer,

+  OUT VOID                      *Buffer,

  IN  UINT64                    Value

  );



--
2.31.1
Re: [PATCH v2 4/7] ArmPkg/DefaultExceptionHandlerLib: Check DebugImageInfoTable type safely

Marvin Häuser <mhaeuser@...>
 

On 09/08/2021 14:40, Marvin Häuser wrote:
On 09/08/2021 13:55, Ard Biesheuvel wrote:
On Mon, 9 Aug 2021 at 11:51, Marvin Häuser <mhaeuser@...> wrote:
C does not allow casting to or dereferencing incompatible pointer
types. Use the ImageInfoType member of the union first to determine
the data type before dereferencing NormalImage.

Cc: Leif Lindholm <leif@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Vitaly Cheptsov <vit9696@...>
Signed-off-by: Marvin Häuser <mhaeuser@...>
Hi Marvin,

Could you please organize your patches into a consistent series,
include a cover letter and cc me on everything?
Hey Ard,

It's a series and there is a cover letter at: https://edk2.groups.io/g/devel/topic/patch_v2_0_7_fix_various/84764899?p=,,,20,0,0,0::recentpostdate%2Fsticky,,,20,2,0,84764899
The mails from yesterday can certainly be discarded, for some reason format-patch did not number the patches without the argument.
The mails from today are numbered and there is a cover letter, but for some reason the threading is all wrong in Thunderbird for me. All subsequent patches have the "In-Reply-To" header in the patch files, I think it is supposed to work off of that? Is threading broken for you as well? Any idea what could have gone wrong?
Today I learned two things.

1) Both format-patch and send-email support threading individually, and they don't cooperate [1].

2) Groups.io does not like patch sets [2].

*Sigh*. Sorry.

Best regards,
Marvin


[1] "It is up to the user to ensure that no In-Reply-To header already exists when git send-email is asked to add it (especially note that git format-patch can be configured to do the threading itself). Failure to do so may not produce the expected result in the recipient’s MUA.", https://git-scm.com/docs/git-send-email

[2] "Note: This checkbox is selected by default in new Groups.io accounts. If you do not want to see copies of your own messages, clear this checkbox. [...] (For those interested in the technical details: When this checkbox is selected, Groups.io replaces the Message-Id header with a new, system-generated one and renames the original Message-Id header to X-Orig-Message-Id.)", https://groups.io/helpcenter/membersmanual?single=true


I will create a V3 with you CC'd on all patches once I understand everything that went wrong. Is it normal to CC all people from each patch on all patches of a series?

Thanks and so sorry for the hassle!

Best regards,
Marvin

I am going to disregard anything you sent yesterday and today, as it
is a bit of a jumble.

Thanks,
Ard.


---
ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerUefi.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerUefi.c b/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerUefi.c
index e9fea4038252..9befb6d4db9b 100644
---
a/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerUefi.c
+++ b/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerUefi.c
@@ -51,8 +51,8 @@ GetImageName (

    Address = (CHAR8 *)(UINTN)FaultAddress;
    for (Entry = 0; Entry < DebugTableHeader->TableSize; Entry++, DebugTable++) {
-    if (DebugTable->NormalImage != NULL) {
-      if ((DebugTable->NormalImage->ImageInfoType == EFI_DEBUG_IMAGE_INFO_TYPE_NORMAL) &&
+    if (DebugTable->ImageInfoType != NULL) {
+      if ((*DebugTable->ImageInfoType == EFI_DEBUG_IMAGE_INFO_TYPE_NORMAL) &&
(DebugTable->NormalImage->LoadedImageProtocolInstance != NULL)) {
          if ((Address >= (CHAR8 *)DebugTable->NormalImage->LoadedImageProtocolInstance->ImageBase) &&
              (Address <= ((CHAR8 *)DebugTable->NormalImage->LoadedImageProtocolInstance->ImageBase + DebugTable->NormalImage->LoadedImageProtocolInstance->ImageSize))) {
--
2.31.1
[PATCH v3 2/2] BaseTools/CommonLib: Fix unaligned API prototypes

Marvin Häuser <mhaeuser@...>
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3542

C prohibits not only dereferencing but also casting to unaligned
pointers. Thus, the current set of unaligned APIs cannot be called
safely. Update their prototypes to take VOID * pointers, which must
be able to represent any valid pointer.

Cc: Bob Feng <bob.c.feng@...>
Cc: Liming Gao <gaoliming@...>
Cc: Yuwei Chen <yuwei.chen@...>
Cc: Vitaly Cheptsov <vit9696@...>
Signed-off-by: Marvin Häuser <mhaeuser@...>
---
BaseTools/Source/C/Common/CommonLib.c | 16 ++++++++--------
BaseTools/Source/C/Common/CommonLib.h | 8 ++++----
2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/BaseTools/Source/C/Common/CommonLib.c b/BaseTools/Source/C/Common/CommonLib.c
index 7fb4ab764fcd..f1223fb2ae0a 100644
--- a/BaseTools/Source/C/Common/CommonLib.c
+++ b/BaseTools/Source/C/Common/CommonLib.c
@@ -1154,23 +1154,23 @@ StrSize (

UINT64
ReadUnaligned64 (
- CONST UINT64 *Buffer
+ CONST VOID *Buffer
)
{
ASSERT (Buffer != NULL);

- return *Buffer;
+ return *(CONST UINT64 *) Buffer;
}

UINT64
WriteUnaligned64 (
- UINT64 *Buffer,
+ VOID *Buffer,
UINT64 Value
)
{
ASSERT (Buffer != NULL);

- return *Buffer = Value;
+ return *(UINT64 *) Buffer = Value;
}


@@ -2018,23 +2018,23 @@ AllocatePool (

UINT16
WriteUnaligned16 (
- UINT16 *Buffer,
+ VOID *Buffer,
UINT16 Value
)
{
ASSERT (Buffer != NULL);

- return *Buffer = Value;
+ return *(UINT16 *) Buffer = Value;
}

UINT16
ReadUnaligned16 (
- CONST UINT16 *Buffer
+ CONST VOID *Buffer
)
{
ASSERT (Buffer != NULL);

- return *Buffer;
+ return *(CONST UINT16 *) Buffer;
}
/**
Return whether the integer string is a hex string.
diff --git a/BaseTools/Source/C/Common/CommonLib.h b/BaseTools/Source/C/Common/CommonLib.h
index 0f05d88db206..67c42a91765d 100644
--- a/BaseTools/Source/C/Common/CommonLib.h
+++ b/BaseTools/Source/C/Common/CommonLib.h
@@ -238,13 +238,13 @@ CopyGuid (

UINT64
WriteUnaligned64 (
- UINT64 *Buffer,
+ VOID *Buffer,
UINT64 Value
);

UINT64
ReadUnaligned64 (
- CONST UINT64 *Buffer
+ CONST VOID *Buffer
);

UINTN
@@ -363,13 +363,13 @@ AllocatePool (

UINT16
WriteUnaligned16 (
- UINT16 *Buffer,
+ VOID *Buffer,
UINT16 Value
);

UINT16
ReadUnaligned16 (
- CONST UINT16 *Buffer
+ CONST VOID *Buffer
);

VOID *
--
2.31.1
[PATCH v3 1/2] MdePkg/BaseLib: Fix unaligned API prototypes

Marvin Häuser <mhaeuser@...>
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3542

C prohibits not only dereferencing but also casting to unaligned
pointers. Thus, the current set of unaligned APIs cannot be called
safely. Update their prototypes to take VOID * pointers, which must
be able to represent any valid pointer.

Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Cc: Vitaly Cheptsov <vit9696@...>
Signed-off-by: Marvin Häuser <mhaeuser@...>
---
MdePkg/Library/BaseLib/Arm/Unaligned.c | 14 ++++-----
MdePkg/Library/BaseLib/Unaligned.c | 32 ++++++++++----------
MdePkg/Include/Library/BaseLib.h | 16 +++++-----
3 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/MdePkg/Library/BaseLib/Arm/Unaligned.c b/MdePkg/Library/BaseLib/Arm/Unaligned.c
index e9934e7003cb..57f19fc44e0b 100644
--- a/MdePkg/Library/BaseLib/Arm/Unaligned.c
+++ b/MdePkg/Library/BaseLib/Arm/Unaligned.c
@@ -59,7 +59,7 @@ ReadUnaligned16 (
UINT16
EFIAPI
WriteUnaligned16 (
- OUT UINT16 *Buffer,
+ OUT VOID *Buffer,
IN UINT16 Value
)
{
@@ -87,7 +87,7 @@ WriteUnaligned16 (
UINT32
EFIAPI
ReadUnaligned24 (
- IN CONST UINT32 *Buffer
+ IN CONST VOID *Buffer
)
{
ASSERT (Buffer != NULL);
@@ -116,7 +116,7 @@ ReadUnaligned24 (
UINT32
EFIAPI
WriteUnaligned24 (
- OUT UINT32 *Buffer,
+ OUT VOID *Buffer,
IN UINT32 Value
)
{
@@ -143,7 +143,7 @@ WriteUnaligned24 (
UINT32
EFIAPI
ReadUnaligned32 (
- IN CONST UINT32 *Buffer
+ IN CONST VOID *Buffer
)
{
UINT16 LowerBytes;
@@ -175,7 +175,7 @@ ReadUnaligned32 (
UINT32
EFIAPI
WriteUnaligned32 (
- OUT UINT32 *Buffer,
+ OUT VOID *Buffer,
IN UINT32 Value
)
{
@@ -202,7 +202,7 @@ WriteUnaligned32 (
UINT64
EFIAPI
ReadUnaligned64 (
- IN CONST UINT64 *Buffer
+ IN CONST VOID *Buffer
)
{
UINT32 LowerBytes;
@@ -234,7 +234,7 @@ ReadUnaligned64 (
UINT64
EFIAPI
WriteUnaligned64 (
- OUT UINT64 *Buffer,
+ OUT VOID *Buffer,
IN UINT64 Value
)
{
diff --git a/MdePkg/Library/BaseLib/Unaligned.c b/MdePkg/Library/BaseLib/Unaligned.c
index a419cb85e53c..3041adcde606 100644
--- a/MdePkg/Library/BaseLib/Unaligned.c
+++ b/MdePkg/Library/BaseLib/Unaligned.c
@@ -26,12 +26,12 @@
UINT16
EFIAPI
ReadUnaligned16 (
- IN CONST UINT16 *Buffer
+ IN CONST VOID *Buffer
)
{
ASSERT (Buffer != NULL);

- return *Buffer;
+ return *(CONST UINT16 *) Buffer;
}

/**
@@ -52,13 +52,13 @@ ReadUnaligned16 (
UINT16
EFIAPI
WriteUnaligned16 (
- OUT UINT16 *Buffer,
+ OUT VOID *Buffer,
IN UINT16 Value
)
{
ASSERT (Buffer != NULL);

- return *Buffer = Value;
+ return *(UINT16 *) Buffer = Value;
}

/**
@@ -77,12 +77,12 @@ WriteUnaligned16 (
UINT32
EFIAPI
ReadUnaligned24 (
- IN CONST UINT32 *Buffer
+ IN CONST VOID *Buffer
)
{
ASSERT (Buffer != NULL);

- return *Buffer & 0xffffff;
+ return *(CONST UINT32 *) Buffer & 0xffffff;
}

/**
@@ -103,13 +103,13 @@ ReadUnaligned24 (
UINT32
EFIAPI
WriteUnaligned24 (
- OUT UINT32 *Buffer,
+ OUT VOID *Buffer,
IN UINT32 Value
)
{
ASSERT (Buffer != NULL);

- *Buffer = BitFieldWrite32 (*Buffer, 0, 23, Value);
+ *(UINT32 *) Buffer = BitFieldWrite32 (*(CONST UINT32 *) Buffer, 0, 23, Value);
return Value;
}

@@ -129,12 +129,12 @@ WriteUnaligned24 (
UINT32
EFIAPI
ReadUnaligned32 (
- IN CONST UINT32 *Buffer
+ IN CONST VOID *Buffer
)
{
ASSERT (Buffer != NULL);

- return *Buffer;
+ return *(CONST UINT32 *) Buffer;
}

/**
@@ -155,13 +155,13 @@ ReadUnaligned32 (
UINT32
EFIAPI
WriteUnaligned32 (
- OUT UINT32 *Buffer,
+ OUT VOID *Buffer,
IN UINT32 Value
)
{
ASSERT (Buffer != NULL);

- return *Buffer = Value;
+ return *(UINT32 *) Buffer = Value;
}

/**
@@ -180,12 +180,12 @@ WriteUnaligned32 (
UINT64
EFIAPI
ReadUnaligned64 (
- IN CONST UINT64 *Buffer
+ IN CONST VOID *Buffer
)
{
ASSERT (Buffer != NULL);

- return *Buffer;
+ return *(CONST UINT64 *) Buffer;
}

/**
@@ -206,11 +206,11 @@ ReadUnaligned64 (
UINT64
EFIAPI
WriteUnaligned64 (
- OUT UINT64 *Buffer,
+ OUT VOID *Buffer,
IN UINT64 Value
)
{
ASSERT (Buffer != NULL);

- return *Buffer = Value;
+ return *(UINT64 *) Buffer = Value;
}
diff --git a/MdePkg/Include/Library/BaseLib.h b/MdePkg/Include/Library/BaseLib.h
index 2452c1d92e51..4d30f0539c6b 100644
--- a/MdePkg/Include/Library/BaseLib.h
+++ b/MdePkg/Include/Library/BaseLib.h
@@ -3420,7 +3420,7 @@ DivS64x64Remainder (
UINT16
EFIAPI
ReadUnaligned16 (
- IN CONST UINT16 *Buffer
+ IN CONST VOID *Buffer
);


@@ -3442,7 +3442,7 @@ ReadUnaligned16 (
UINT16
EFIAPI
WriteUnaligned16 (
- OUT UINT16 *Buffer,
+ OUT VOID *Buffer,
IN UINT16 Value
);

@@ -3463,7 +3463,7 @@ WriteUnaligned16 (
UINT32
EFIAPI
ReadUnaligned24 (
- IN CONST UINT32 *Buffer
+ IN CONST VOID *Buffer
);


@@ -3485,7 +3485,7 @@ ReadUnaligned24 (
UINT32
EFIAPI
WriteUnaligned24 (
- OUT UINT32 *Buffer,
+ OUT VOID *Buffer,
IN UINT32 Value
);

@@ -3506,7 +3506,7 @@ WriteUnaligned24 (
UINT32
EFIAPI
ReadUnaligned32 (
- IN CONST UINT32 *Buffer
+ IN CONST VOID *Buffer
);


@@ -3528,7 +3528,7 @@ ReadUnaligned32 (
UINT32
EFIAPI
WriteUnaligned32 (
- OUT UINT32 *Buffer,
+ OUT VOID *Buffer,
IN UINT32 Value
);

@@ -3549,7 +3549,7 @@ WriteUnaligned32 (
UINT64
EFIAPI
ReadUnaligned64 (
- IN CONST UINT64 *Buffer
+ IN CONST VOID *Buffer
);


@@ -3571,7 +3571,7 @@ ReadUnaligned64 (
UINT64
EFIAPI
WriteUnaligned64 (
- OUT UINT64 *Buffer,
+ OUT VOID *Buffer,
IN UINT64 Value
);

--
2.31.1
Re: [Patch V2] MinPlatformPkg: Fix the incompatible change about SecureBootVariableLib

Nate DeSimone
 

Reviewed-by: Nate DeSimone <nathaniel.l.desimone@...>

-----Original Message-----
From: Tan, Dun <dun.tan@...>
Sent: Monday, August 9, 2021 8:00 AM
To: devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@...>; Desimone, Nathaniel L
<nathaniel.l.desimone@...>; Liming Gao
<gaoliming@...>; Dong, Eric <eric.dong@...>; Tan, Dun
<dun.tan@...>
Subject: [Patch V2] MinPlatformPkg: Fix the incompatible change about
SecureBootVariableLib

V1: The newly created lib will be consumed by SecureBootConfigDxe.inf in
CoreDxeInclude.dsc
V2: Add SecureBootVariableProvisionLib in CoreDxeInclude.dsc

Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Liming Gao <gaoliming@...>
Cc: Eric Dong <eric.dong@...>
Signed-off-by: DunTan <dun.tan@...>
---
Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc | 2 ++
1 file changed, 2 insertions(+)

diff --git a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
index b154f9615d..c3d05fc913 100644
--- a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
+++ b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
@@ -139,6 +139,8 @@

!if gMinPlatformPkgTokenSpaceGuid.PcdUefiSecureBootEnable == TRUE
AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
+
+ SecureBootVariableLib|SecurityPkg/Library/SecureBootVariableLib/Secure
+ BootVariableLib.inf
+ SecureBootVariableProvisionLib|SecurityPkg/Library/SecureBootVariableP
+ rovisionLib/SecureBootVariableProvisionLib.inf
!endif

SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
--
2.31.1.windows.1
Re: [edk2-platforms][PATCH v1 1/1] MinPlatformPkg/AcpiTables: Update structures for ACPI 6.3

Nate DeSimone
 

Reviewed-by: Nate DeSimone <nathaniel.l.desimone@...>

-----Original Message-----
From: mikuback@... <mikuback@...>
Sent: Friday, August 6, 2021 12:54 PM
To: devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@...>; Desimone, Nathaniel L
<nathaniel.l.desimone@...>; Liming Gao
<gaoliming@...>; Dong, Eric <eric.dong@...>; Maddy,
Daniel <danmad@...>; Michael Kubacki
<michael.kubacki@...>
Subject: [edk2-platforms][PATCH v1 1/1] MinPlatformPkg/AcpiTables:
Update structures for ACPI 6.3

From: Daniel Maddy <danmad@...>

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3535

Updates ACPI table structures in MinPlatformPkg for ACPI 6.3.

Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Liming Gao <gaoliming@...>
Cc: Eric Dong <eric.dong@...>
Cc: Daniel Maddy <danmad@...>
Co-authored-by: Michael Kubacki <michael.kubacki@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>
---
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 203
++++++++++----------
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/Facs/Facs.c | 11 +-
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/Fadt/Fadt.c | 74 ++++---
3 files changed, 150 insertions(+), 138 deletions(-)

diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index 2b51c34ef2fd..5e3c4c0672f9 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -2,6 +2,7 @@
ACPI Platform Driver

Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) Microsoft Corporation.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -13,7 +14,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #pragma
pack(1)

typedef struct {
- UINT32 AcpiProcessorId;
+ UINT32 AcpiProcessorUid;
UINT32 ApicId;
UINT32 Flags;
UINT32 SwProcApicId;
@@ -27,9 +28,9 @@ typedef struct {
// Define Union of IO APIC & Local APIC structure; // typedef union {
- EFI_ACPI_4_0_PROCESSOR_LOCAL_APIC_STRUCTURE AcpiLocalApic;
- EFI_ACPI_4_0_IO_APIC_STRUCTURE AcpiIoApic;
- EFI_ACPI_4_0_PROCESSOR_LOCAL_X2APIC_STRUCTURE AcpiLocalx2Apic;
+ EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC_STRUCTURE AcpiLocalApic;
+ EFI_ACPI_6_3_IO_APIC_STRUCTURE AcpiIoApic;
+ EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC_STRUCTURE AcpiLocalx2Apic;
struct {
UINT8 Type;
UINT8 Length;
@@ -38,9 +39,9 @@ typedef union {

#pragma pack()

-extern EFI_ACPI_5_0_FIRMWARE_ACPI_CONTROL_STRUCTURE Facs; -
extern EFI_ACPI_5_0_FIXED_ACPI_DESCRIPTION_TABLE Fadt; -extern
EFI_ACPI_HIGH_PRECISION_EVENT_TIMER_TABLE_HEADER Hpet;
+extern EFI_ACPI_6_3_FIRMWARE_ACPI_CONTROL_STRUCTURE Facs;
+extern EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE Fadt;
+extern EFI_ACPI_HIGH_PRECISION_EVENT_TIMER_TABLE_HEADER Hpet;
extern EFI_ACPI_WSMT_TABLE Wsmt;

VOID *mLocalTable[] = {
@@ -217,7 +218,7 @@ DebugDisplayReOrderTable(
DEBUG ((EFI_D_ERROR, "Index AcpiProcId ApicId Flags SwApicId Skt\n"));
for (Index=0; Index<MAX_CPU_NUM; Index++) {
DEBUG ((EFI_D_ERROR, " %02d 0x%02X 0x%02X %d 0x%02X
%d\n",
- Index, mCpuApicIdOrderTable[Index].AcpiProcessorId,
+ Index,
+ mCpuApicIdOrderTable[Index].AcpiProcessorUid,
mCpuApicIdOrderTable[Index].ApicId,
mCpuApicIdOrderTable[Index].Flags,
mCpuApicIdOrderTable[Index].SwProcApicId,
@@ -232,31 +233,31 @@ AppendCpuMapTableEntry (
)
{
EFI_STATUS Status;
- EFI_ACPI_4_0_PROCESSOR_LOCAL_APIC_STRUCTURE *LocalApicPtr;
- EFI_ACPI_4_0_PROCESSOR_LOCAL_X2APIC_STRUCTURE *LocalX2ApicPtr;
+ EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC_STRUCTURE *LocalApicPtr;
+ EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC_STRUCTURE *LocalX2ApicPtr;
UINT8 Type;

Status = EFI_SUCCESS;
Type = ((ACPI_APIC_STRUCTURE_PTR *)ApicPtr)->AcpiApicCommon.Type;
- LocalApicPtr = (EFI_ACPI_4_0_PROCESSOR_LOCAL_APIC_STRUCTURE
*)(&((ACPI_APIC_STRUCTURE_PTR *)ApicPtr)->AcpiLocalApic);
- LocalX2ApicPtr = (EFI_ACPI_4_0_PROCESSOR_LOCAL_X2APIC_STRUCTURE
*)(&((ACPI_APIC_STRUCTURE_PTR *)ApicPtr)->AcpiLocalx2Apic);
+ LocalApicPtr = (EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC_STRUCTURE
+ *)(&((ACPI_APIC_STRUCTURE_PTR *)ApicPtr)->AcpiLocalApic);
+ LocalX2ApicPtr = (EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC_STRUCTURE
+ *)(&((ACPI_APIC_STRUCTURE_PTR *)ApicPtr)->AcpiLocalx2Apic);

- if(Type == EFI_ACPI_4_0_PROCESSOR_LOCAL_APIC) {
+ if(Type == EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC) {
if(!mX2ApicEnabled) {
- LocalApicPtr->Flags =
(UINT8)mCpuApicIdOrderTable[LocalApicCounter].Flags;
- LocalApicPtr->ApicId =
(UINT8)mCpuApicIdOrderTable[LocalApicCounter].ApicId;
- LocalApicPtr->AcpiProcessorId =
(UINT8)mCpuApicIdOrderTable[LocalApicCounter].AcpiProcessorId;
+ LocalApicPtr->Flags =
(UINT8)mCpuApicIdOrderTable[LocalApicCounter].Flags;
+ LocalApicPtr->ApicId =
(UINT8)mCpuApicIdOrderTable[LocalApicCounter].ApicId;
+ LocalApicPtr->AcpiProcessorUid =
+ (UINT8)mCpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
} else {
- LocalApicPtr->Flags = 0;
- LocalApicPtr->ApicId = 0xFF;
- LocalApicPtr->AcpiProcessorId = (UINT8)0xFF;
+ LocalApicPtr->Flags = 0;
+ LocalApicPtr->ApicId = 0xFF;
+ LocalApicPtr->AcpiProcessorUid = (UINT8)0xFF;
Status = EFI_UNSUPPORTED;
}
- } else if(Type == EFI_ACPI_4_0_PROCESSOR_LOCAL_X2APIC) {
+ } else if(Type == EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC) {
if(mX2ApicEnabled) {
LocalX2ApicPtr->Flags =
(UINT8)mCpuApicIdOrderTable[LocalApicCounter].Flags;
LocalX2ApicPtr->X2ApicId =
mCpuApicIdOrderTable[LocalApicCounter].ApicId;
- LocalX2ApicPtr->AcpiProcessorUid =
mCpuApicIdOrderTable[LocalApicCounter].AcpiProcessorId;
+ LocalX2ApicPtr->AcpiProcessorUid =
+ mCpuApicIdOrderTable[LocalApicCounter].AcpiProcessorUid;
} else {
LocalX2ApicPtr->Flags = 0;
LocalX2ApicPtr->X2ApicId = (UINT32)-1;
@@ -311,8 +312,8 @@ SortCpuLocalApicInTable (
CpuIdMapPtr->ApicId = (UINT32)ProcessorInfoBuffer.ProcessorId;
CpuIdMapPtr->Flags = ((ProcessorInfoBuffer.StatusFlag &
PROCESSOR_ENABLED_BIT) != 0);
CpuIdMapPtr->SocketNum =
(UINT32)ProcessorInfoBuffer.Location.Package;
- CpuIdMapPtr->AcpiProcessorId = (CpuIdMapPtr->SocketNum *
FixedPcdGet32(PcdMaxCpuCoreCount) *
FixedPcdGet32(PcdMaxCpuThreadCount)) +
GetIndexFromApicId(CpuIdMapPtr->ApicId); //CpuIdMapPtr->ApicId;
- CpuIdMapPtr->SwProcApicId =
((UINT32)(ProcessorInfoBuffer.Location.Package << mNumOfBitShift) +
(((UINT32)ProcessorInfoBuffer.ProcessorId) & CoreThreadMask));
+ CpuIdMapPtr->AcpiProcessorUid = (CpuIdMapPtr->SocketNum *
FixedPcdGet32(PcdMaxCpuCoreCount) *
FixedPcdGet32(PcdMaxCpuThreadCount)) +
GetIndexFromApicId(CpuIdMapPtr->ApicId); //CpuIdMapPtr->ApicId;
+ CpuIdMapPtr->SwProcApicId =
((UINT32)(ProcessorInfoBuffer.Location.Package << mNumOfBitShift) +
(((UINT32)ProcessorInfoBuffer.ProcessorId) & CoreThreadMask));
if(mX2ApicEnabled) { //if X2Apic, re-order the socket # so it starts from
base 0 and contiguous
//may not necessory!!!!!
}
@@ -321,18 +322,18 @@ SortCpuLocalApicInTable (
if (CpuIdMapPtr->Flags == 1) {

if(mForceX2ApicId) {
- CpuIdMapPtr->SocketNum &= 0x7;
- CpuIdMapPtr->AcpiProcessorId &= 0xFF; //keep lower 8bit due to use
Proc obj in dsdt
- CpuIdMapPtr->SwProcApicId &= 0xFF;
+ CpuIdMapPtr->SocketNum &= 0x7;
+ CpuIdMapPtr->AcpiProcessorUid &= 0xFF; //keep lower 8bit due to
use Proc obj in dsdt
+ CpuIdMapPtr->SwProcApicId &= 0xFF;
}
}
} else { //not enabled
- CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP
*)&mCpuApicIdOrderTable[Index];
- CpuIdMapPtr->ApicId = (UINT32)-1;
- CpuIdMapPtr->Flags = 0;
- CpuIdMapPtr->AcpiProcessorId = (UINT32)-1;
- CpuIdMapPtr->SwProcApicId = (UINT32)-1;
- CpuIdMapPtr->SocketNum = (UINT32)-1;
+ CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP
*)&mCpuApicIdOrderTable[Index];
+ CpuIdMapPtr->ApicId = (UINT32)-1;
+ CpuIdMapPtr->Flags = 0;
+ CpuIdMapPtr->AcpiProcessorUid = (UINT32)-1;
+ CpuIdMapPtr->SwProcApicId = (UINT32)-1;
+ CpuIdMapPtr->SocketNum = (UINT32)-1;
} //end if PROC ENABLE
} //end for CurrentProcessor

@@ -366,9 +367,9 @@ SortCpuLocalApicInTable (
mCpuApicIdOrderTable[Index].SwProcApicId =
mCpuApicIdOrderTable[0].SwProcApicId;
mCpuApicIdOrderTable[0].SwProcApicId = TempVal;
//swap AcpiProcId
- TempVal = mCpuApicIdOrderTable[Index].AcpiProcessorId;
- mCpuApicIdOrderTable[Index].AcpiProcessorId =
mCpuApicIdOrderTable[0].AcpiProcessorId;
- mCpuApicIdOrderTable[0].AcpiProcessorId = TempVal;
+ TempVal = mCpuApicIdOrderTable[Index].AcpiProcessorUid;
+ mCpuApicIdOrderTable[Index].AcpiProcessorUid =
mCpuApicIdOrderTable[0].AcpiProcessorUid;
+ mCpuApicIdOrderTable[0].AcpiProcessorUid = TempVal;

}

@@ -377,23 +378,23 @@ SortCpuLocalApicInTable (

if(mCpuApicIdOrderTable[CurrProcessor].Flags == 0) {
//make sure disabled entry has ProcId set to FFs
- mCpuApicIdOrderTable[CurrProcessor].ApicId = (UINT32)-1;
- mCpuApicIdOrderTable[CurrProcessor].AcpiProcessorId = (UINT32)-1;
- mCpuApicIdOrderTable[CurrProcessor].SwProcApicId = (UINT32)-1;
+ mCpuApicIdOrderTable[CurrProcessor].ApicId = (UINT32)-1;
+ mCpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid = (UINT32)-1;
+ mCpuApicIdOrderTable[CurrProcessor].SwProcApicId = (UINT32)-1;

for(Index = CurrProcessor+1; Index < MAX_CPU_NUM; Index++) {
if(mCpuApicIdOrderTable[Index].Flags == 1) {
//move enabled entry up
- mCpuApicIdOrderTable[CurrProcessor].Flags = 1;
- mCpuApicIdOrderTable[CurrProcessor].ApicId =
mCpuApicIdOrderTable[Index].ApicId;
- mCpuApicIdOrderTable[CurrProcessor].AcpiProcessorId =
mCpuApicIdOrderTable[Index].AcpiProcessorId;
- mCpuApicIdOrderTable[CurrProcessor].SwProcApicId =
mCpuApicIdOrderTable[Index].SwProcApicId;
- mCpuApicIdOrderTable[CurrProcessor].SocketNum =
mCpuApicIdOrderTable[Index].SocketNum;
+ mCpuApicIdOrderTable[CurrProcessor].Flags = 1;
+ mCpuApicIdOrderTable[CurrProcessor].ApicId =
mCpuApicIdOrderTable[Index].ApicId;
+ mCpuApicIdOrderTable[CurrProcessor].AcpiProcessorUid =
mCpuApicIdOrderTable[Index].AcpiProcessorUid;
+ mCpuApicIdOrderTable[CurrProcessor].SwProcApicId =
mCpuApicIdOrderTable[Index].SwProcApicId;
+ mCpuApicIdOrderTable[CurrProcessor].SocketNum =
mCpuApicIdOrderTable[Index].SocketNum;
//disable moved entry
- mCpuApicIdOrderTable[Index].Flags = 0;
- mCpuApicIdOrderTable[Index].ApicId = (UINT32)-1;
- mCpuApicIdOrderTable[Index].AcpiProcessorId = (UINT32)-1;
- mCpuApicIdOrderTable[Index].SwProcApicId = (UINT32)-1;
+ mCpuApicIdOrderTable[Index].Flags = 0;
+ mCpuApicIdOrderTable[Index].ApicId = (UINT32)-1;
+ mCpuApicIdOrderTable[Index].AcpiProcessorUid = (UINT32)-1;
+ mCpuApicIdOrderTable[Index].SwProcApicId = (UINT32)-1;
break;
}
}
@@ -422,17 +423,17 @@ typedef struct {
} STRUCTURE_HEADER;

STRUCTURE_HEADER mMadtStructureTable[] = {
- {EFI_ACPI_4_0_PROCESSOR_LOCAL_APIC, sizeof
(EFI_ACPI_4_0_PROCESSOR_LOCAL_APIC_STRUCTURE)},
- {EFI_ACPI_4_0_IO_APIC, sizeof
(EFI_ACPI_4_0_IO_APIC_STRUCTURE)},
- {EFI_ACPI_4_0_INTERRUPT_SOURCE_OVERRIDE, sizeof
(EFI_ACPI_4_0_INTERRUPT_SOURCE_OVERRIDE_STRUCTURE)},
- {EFI_ACPI_4_0_NON_MASKABLE_INTERRUPT_SOURCE, sizeof
(EFI_ACPI_4_0_NON_MASKABLE_INTERRUPT_SOURCE_STRUCTURE)},
- {EFI_ACPI_4_0_LOCAL_APIC_NMI, sizeof
(EFI_ACPI_4_0_LOCAL_APIC_NMI_STRUCTURE)},
- {EFI_ACPI_4_0_LOCAL_APIC_ADDRESS_OVERRIDE, sizeof
(EFI_ACPI_4_0_LOCAL_APIC_ADDRESS_OVERRIDE_STRUCTURE)},
- {EFI_ACPI_4_0_IO_SAPIC, sizeof
(EFI_ACPI_4_0_IO_SAPIC_STRUCTURE)},
- {EFI_ACPI_4_0_LOCAL_SAPIC, sizeof
(EFI_ACPI_4_0_PROCESSOR_LOCAL_SAPIC_STRUCTURE)},
- {EFI_ACPI_4_0_PLATFORM_INTERRUPT_SOURCES, sizeof
(EFI_ACPI_4_0_PLATFORM_INTERRUPT_SOURCES_STRUCTURE)},
- {EFI_ACPI_4_0_PROCESSOR_LOCAL_X2APIC, sizeof
(EFI_ACPI_4_0_PROCESSOR_LOCAL_X2APIC_STRUCTURE)},
- {EFI_ACPI_4_0_LOCAL_X2APIC_NMI, sizeof
(EFI_ACPI_4_0_LOCAL_X2APIC_NMI_STRUCTURE)}
+ {EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC, sizeof
(EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC_STRUCTURE)},
+ {EFI_ACPI_6_3_IO_APIC, sizeof
(EFI_ACPI_6_3_IO_APIC_STRUCTURE)},
+ {EFI_ACPI_6_3_INTERRUPT_SOURCE_OVERRIDE, sizeof
(EFI_ACPI_6_3_INTERRUPT_SOURCE_OVERRIDE_STRUCTURE)},
+ {EFI_ACPI_6_3_NON_MASKABLE_INTERRUPT_SOURCE, sizeof
(EFI_ACPI_6_3_NON_MASKABLE_INTERRUPT_SOURCE_STRUCTURE)},
+ {EFI_ACPI_6_3_LOCAL_APIC_NMI, sizeof
(EFI_ACPI_6_3_LOCAL_APIC_NMI_STRUCTURE)},
+ {EFI_ACPI_6_3_LOCAL_APIC_ADDRESS_OVERRIDE, sizeof
(EFI_ACPI_6_3_LOCAL_APIC_ADDRESS_OVERRIDE_STRUCTURE)},
+ {EFI_ACPI_6_3_IO_SAPIC, sizeof
(EFI_ACPI_6_3_IO_SAPIC_STRUCTURE)},
+ {EFI_ACPI_6_3_LOCAL_SAPIC, sizeof
(EFI_ACPI_6_3_PROCESSOR_LOCAL_SAPIC_STRUCTURE)},
+ {EFI_ACPI_6_3_PLATFORM_INTERRUPT_SOURCES, sizeof
(EFI_ACPI_6_3_PLATFORM_INTERRUPT_SOURCES_STRUCTURE)},
+ {EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC, sizeof
(EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC_STRUCTURE)},
+ {EFI_ACPI_6_3_LOCAL_X2APIC_NMI, sizeof
(EFI_ACPI_6_3_LOCAL_X2APIC_NMI_STRUCTURE)}
};

/**
@@ -591,7 +592,7 @@ InitializeHeader (
**/
EFI_STATUS
InitializeMadtHeader (
- IN OUT EFI_ACPI_4_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER
*MadtHeader
+ IN OUT EFI_ACPI_6_3_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER
+ *MadtHeader
)
{
EFI_STATUS Status;
@@ -603,8 +604,8 @@ InitializeMadtHeader (

Status = InitializeHeader (
&MadtHeader->Header,
- EFI_ACPI_4_0_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
- EFI_ACPI_4_0_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION,
+ EFI_ACPI_6_3_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
+ EFI_ACPI_6_3_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION,
0
);
if (EFI_ERROR (Status)) {
@@ -612,7 +613,7 @@ InitializeMadtHeader (
}

MadtHeader->LocalApicAddress = PcdGet32(PcdLocalApicAddress);
- MadtHeader->Flags = EFI_ACPI_4_0_PCAT_COMPAT;
+ MadtHeader->Flags = EFI_ACPI_6_3_PCAT_COMPAT;

return EFI_SUCCESS;
}
@@ -649,7 +650,7 @@ CopyStructure (
//
// Initialize the number of table entries and the table based on the table
header passed in.
//
- if (Header->Signature ==
EFI_ACPI_4_0_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE) {
+ if (Header->Signature ==
+ EFI_ACPI_6_3_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE) {
TableNumEntries = sizeof (mMadtStructureTable) / sizeof
(STRUCTURE_HEADER);
StructureTable = mMadtStructureTable;
} else {
@@ -759,7 +760,7 @@ BuildAcpiTable (
return EFI_INVALID_PARAMETER;
}

- if (AcpiHeader->Signature !=
EFI_ACPI_4_0_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE) {
+ if (AcpiHeader->Signature !=
+ EFI_ACPI_6_3_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE) {
DEBUG ((
DEBUG_ERROR,
"MADT header signature is expected, actually 0x%08x\n", @@ -850,15
+851,15 @@ InstallMadtFromScratch ( {
EFI_STATUS Status;
UINTN Index;
- EFI_ACPI_4_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER
*NewMadtTable;
+ EFI_ACPI_6_3_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER
*NewMadtTable;
UINTN TableHandle;
- EFI_ACPI_4_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER
MadtTableHeader;
- EFI_ACPI_4_0_PROCESSOR_LOCAL_APIC_STRUCTURE
ProcLocalApicStruct;
- EFI_ACPI_4_0_IO_APIC_STRUCTURE IoApicStruct;
- EFI_ACPI_4_0_INTERRUPT_SOURCE_OVERRIDE_STRUCTURE
IntSrcOverrideStruct;
- EFI_ACPI_4_0_LOCAL_APIC_NMI_STRUCTURE LocalApciNmiStruct;
- EFI_ACPI_4_0_PROCESSOR_LOCAL_X2APIC_STRUCTURE
ProcLocalX2ApicStruct;
- EFI_ACPI_4_0_LOCAL_X2APIC_NMI_STRUCTURE
LocalX2ApicNmiStruct;
+ EFI_ACPI_6_3_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER
MadtTableHeader;
+ EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC_STRUCTURE
ProcLocalApicStruct;
+ EFI_ACPI_6_3_IO_APIC_STRUCTURE IoApicStruct;
+ EFI_ACPI_6_3_INTERRUPT_SOURCE_OVERRIDE_STRUCTURE
IntSrcOverrideStruct;
+ EFI_ACPI_6_3_LOCAL_APIC_NMI_STRUCTURE LocalApciNmiStruct;
+ EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC_STRUCTURE
ProcLocalX2ApicStruct;
+ EFI_ACPI_6_3_LOCAL_X2APIC_NMI_STRUCTURE
LocalX2ApicNmiStruct;
STRUCTURE_HEADER **MadtStructs;
UINTN MaxMadtStructCount;
UINTN MadtStructsIndex;
@@ -915,11 +916,11 @@ InstallMadtFromScratch (
//
// Build Processor Local APIC Structures and Processor Local X2APIC
Structures
//
- ProcLocalApicStruct.Type = EFI_ACPI_4_0_PROCESSOR_LOCAL_APIC;
- ProcLocalApicStruct.Length = sizeof
(EFI_ACPI_4_0_PROCESSOR_LOCAL_APIC_STRUCTURE);
+ ProcLocalApicStruct.Type = EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC;
+ ProcLocalApicStruct.Length = sizeof
+ (EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC_STRUCTURE);

- ProcLocalX2ApicStruct.Type = EFI_ACPI_4_0_PROCESSOR_LOCAL_X2APIC;
- ProcLocalX2ApicStruct.Length = sizeof
(EFI_ACPI_4_0_PROCESSOR_LOCAL_X2APIC_STRUCTURE);
+ ProcLocalX2ApicStruct.Type = EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC;
+ ProcLocalX2ApicStruct.Length = sizeof
+ (EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC_STRUCTURE);
ProcLocalX2ApicStruct.Reserved[0] = 0;
ProcLocalX2ApicStruct.Reserved[1] = 0;

@@ -930,9 +931,9 @@ InstallMadtFromScratch (
// use a processor local x2APIC structure.
//
if (!mX2ApicEnabled && mCpuApicIdOrderTable[Index].ApicId <
MAX_UINT8) {
- ProcLocalApicStruct.Flags = (UINT8)
mCpuApicIdOrderTable[Index].Flags;
- ProcLocalApicStruct.ApicId = (UINT8)
mCpuApicIdOrderTable[Index].ApicId;
- ProcLocalApicStruct.AcpiProcessorId = (UINT8)
mCpuApicIdOrderTable[Index].AcpiProcessorId;
+ ProcLocalApicStruct.Flags = (UINT8)
mCpuApicIdOrderTable[Index].Flags;
+ ProcLocalApicStruct.ApicId = (UINT8)
mCpuApicIdOrderTable[Index].ApicId;
+ ProcLocalApicStruct.AcpiProcessorUid = (UINT8)
+ mCpuApicIdOrderTable[Index].AcpiProcessorUid;

ASSERT (MadtStructsIndex < MaxMadtStructCount);
Status = CopyStructure (
@@ -943,7 +944,7 @@ InstallMadtFromScratch (
} else if (mCpuApicIdOrderTable[Index].ApicId != 0xFFFFFFFF) {
ProcLocalX2ApicStruct.Flags = (UINT8)
mCpuApicIdOrderTable[Index].Flags;
ProcLocalX2ApicStruct.X2ApicId =
mCpuApicIdOrderTable[Index].ApicId;
- ProcLocalX2ApicStruct.AcpiProcessorUid =
mCpuApicIdOrderTable[Index].AcpiProcessorId;
+ ProcLocalX2ApicStruct.AcpiProcessorUid =
+ mCpuApicIdOrderTable[Index].AcpiProcessorUid;

ASSERT (MadtStructsIndex < MaxMadtStructCount);
Status = CopyStructure (
@@ -961,8 +962,8 @@ InstallMadtFromScratch (
//
// Build I/O APIC Structures
//
- IoApicStruct.Type = EFI_ACPI_4_0_IO_APIC;
- IoApicStruct.Length = sizeof (EFI_ACPI_4_0_IO_APIC_STRUCTURE);
+ IoApicStruct.Type = EFI_ACPI_6_3_IO_APIC; IoApicStruct.Length =
+ sizeof (EFI_ACPI_6_3_IO_APIC_STRUCTURE);
IoApicStruct.Reserved = 0;

PcIoApicEnable = PcdGet32(PcdPcIoApicEnable); @@ -1008,8 +1009,8 @@
InstallMadtFromScratch (
//
// Build Interrupt Source Override Structures
//
- IntSrcOverrideStruct.Type =
EFI_ACPI_4_0_INTERRUPT_SOURCE_OVERRIDE;
- IntSrcOverrideStruct.Length = sizeof
(EFI_ACPI_4_0_INTERRUPT_SOURCE_OVERRIDE_STRUCTURE);
+ IntSrcOverrideStruct.Type =
EFI_ACPI_6_3_INTERRUPT_SOURCE_OVERRIDE;
+ IntSrcOverrideStruct.Length = sizeof
+ (EFI_ACPI_6_3_INTERRUPT_SOURCE_OVERRIDE_STRUCTURE);

//
// IRQ0=>IRQ2 Interrupt Source Override Structure @@ -1052,11 +1053,11
@@ InstallMadtFromScratch (
//
// Build Local APIC NMI Structures
//
- LocalApciNmiStruct.Type = EFI_ACPI_4_0_LOCAL_APIC_NMI;
- LocalApciNmiStruct.Length = sizeof
(EFI_ACPI_4_0_LOCAL_APIC_NMI_STRUCTURE);
- LocalApciNmiStruct.AcpiProcessorId = 0xFF; // Applies to all processors
- LocalApciNmiStruct.Flags = 0x0005; // Flags - Edge-tiggered, Active
High
- LocalApciNmiStruct.LocalApicLint = 0x1;
+ LocalApciNmiStruct.Type = EFI_ACPI_6_3_LOCAL_APIC_NMI;
+ LocalApciNmiStruct.Length = sizeof
(EFI_ACPI_6_3_LOCAL_APIC_NMI_STRUCTURE);
+ LocalApciNmiStruct.AcpiProcessorUid = 0xFF; // Applies to all processors
+ LocalApciNmiStruct.Flags = 0x0005; // Flags - Edge-tiggered, Active
High
+ LocalApciNmiStruct.LocalApicLint = 0x1;

ASSERT (MadtStructsIndex < MaxMadtStructCount);
Status = CopyStructure (
@@ -1073,8 +1074,8 @@ InstallMadtFromScratch (
// Build Local x2APIC NMI Structure
//
if (mX2ApicEnabled) {
- LocalX2ApicNmiStruct.Type = EFI_ACPI_4_0_LOCAL_X2APIC_NMI;
- LocalX2ApicNmiStruct.Length = sizeof
(EFI_ACPI_4_0_LOCAL_X2APIC_NMI_STRUCTURE);
+ LocalX2ApicNmiStruct.Type = EFI_ACPI_6_3_LOCAL_X2APIC_NMI;
+ LocalX2ApicNmiStruct.Length = sizeof
+ (EFI_ACPI_6_3_LOCAL_X2APIC_NMI_STRUCTURE);
LocalX2ApicNmiStruct.Flags = 0x000D; // Flags - Level-tiggered,
Active High
LocalX2ApicNmiStruct.AcpiProcessorUid = 0xFFFFFFFF; // Applies to all
processors
LocalX2ApicNmiStruct.LocalX2ApicLint = 0x01; @@ -1099,7 +1100,7 @@
InstallMadtFromScratch (
//
Status = BuildAcpiTable (
(EFI_ACPI_DESCRIPTION_HEADER *) &MadtTableHeader,
- sizeof (EFI_ACPI_4_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER),
+ sizeof (EFI_ACPI_6_3_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER),
MadtStructs,
MadtStructsIndex,
(UINT8 **)&NewMadtTable
@@ -1222,7 +1223,7 @@ PlatformUpdateTables (
EFI_ACPI_DESCRIPTION_HEADER *TableHeader;
UINT8 *TempOemId;
UINT64 TempOemTableId;
- EFI_ACPI_5_0_FIXED_ACPI_DESCRIPTION_TABLE *FadtHeader;
+ EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE *FadtHeader;
EFI_ACPI_HIGH_PRECISION_EVENT_TIMER_TABLE_HEADER *HpetTable;
UINT32 HpetBaseAddress;
EFI_ACPI_HIGH_PRECISION_EVENT_TIMER_BLOCK_ID HpetBlockId;
@@ -1279,12 +1280,12 @@ PlatformUpdateTables (
//
switch (Table->Signature) {

- case EFI_ACPI_4_0_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE:
+ case EFI_ACPI_6_3_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE:
ASSERT(FALSE);
break;

- case EFI_ACPI_5_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE:
- FadtHeader = (EFI_ACPI_5_0_FIXED_ACPI_DESCRIPTION_TABLE *)
Table;
+ case EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE:
+ FadtHeader = (EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE *) Table;

FadtHeader->PreferredPmProfile = PcdGet8
(PcdFadtPreferredPmProfile);
FadtHeader->IaPcBootArch = PcdGet16 (PcdFadtIaPcBootArch);
@@ -1329,7 +1330,7 @@ PlatformUpdateTables (
DEBUG(( EFI_D_ERROR, " Flags 0x%x\n", FadtHeader->Flags ));
break;

- case EFI_ACPI_3_0_HIGH_PRECISION_EVENT_TIMER_TABLE_SIGNATURE:
+ case EFI_ACPI_6_3_HIGH_PRECISION_EVENT_TIMER_TABLE_SIGNATURE:
HpetTable = (EFI_ACPI_HIGH_PRECISION_EVENT_TIMER_TABLE_HEADER
*)Table;
HpetBaseAddress = PcdGet32 (PcdHpetBaseAddress);
HpetTable->BaseAddressLower32Bit.Address = HpetBaseAddress; @@ -
1381,8 +1382,8 @@ IsHardwareChange (
UINTN HWChangeSize;
UINT32 PciId;
UINTN Handle;
- EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *FacsPtr;
- EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *pFADT;
+ EFI_ACPI_6_3_FIRMWARE_ACPI_CONTROL_STRUCTURE *FacsPtr;
+ EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE *pFADT;

HandleCount = 0;
HandleBuffer = NULL;
@@ -1428,7 +1429,7 @@ IsHardwareChange (
//
Handle = 0;
Status = LocateAcpiTableBySignature (
- EFI_ACPI_1_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
+ EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
(EFI_ACPI_DESCRIPTION_HEADER **) &pFADT,
&Handle
);
@@ -1450,7 +1451,7 @@ IsHardwareChange (
//
// Set HardwareSignature value based on CRC value.
//
- FacsPtr = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
*)(UINTN)pFADT->FirmwareCtrl;
+ FacsPtr = (EFI_ACPI_6_3_FIRMWARE_ACPI_CONTROL_STRUCTURE
+ *)(UINTN)pFADT->FirmwareCtrl;
FacsPtr->HardwareSignature = CRC;
FreePool( HWChange );
}
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/Facs/Facs.c
b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/Facs/Facs.c
index cde6e478c6b9..8700c44e633d 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/Facs/Facs.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/Facs/Facs.c
@@ -1,9 +1,10 @@
/** @file
- This file contains a structure definition for the ACPI 5.0 Firmware ACPI
+ This file contains a structure definition for the ACPI 6.3 Firmware
+ ACPI
Control Structure (FACS). The contents of this file should only be modified
for bug fixes, no porting is required.

Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) Microsoft Corporation.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -35,9 +36,9 @@ SPDX-License-Identifier: BSD-2-Clause-Patent // Please
modify all values in Facs.h only.
//

-EFI_ACPI_5_0_FIRMWARE_ACPI_CONTROL_STRUCTURE Facs = {
- EFI_ACPI_5_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE,
- sizeof (EFI_ACPI_5_0_FIRMWARE_ACPI_CONTROL_STRUCTURE),
+EFI_ACPI_6_3_FIRMWARE_ACPI_CONTROL_STRUCTURE Facs = {
+ EFI_ACPI_6_3_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE,
+ sizeof (EFI_ACPI_6_3_FIRMWARE_ACPI_CONTROL_STRUCTURE),

//
// Hardware Signature will be updated at runtime @@ -48,7 +49,7 @@
EFI_ACPI_5_0_FIRMWARE_ACPI_CONTROL_STRUCTURE Facs = {
EFI_ACPI_GLOBAL_LOCK,
EFI_ACPI_FIRMWARE_CONTROL_STRUCTURE_FLAGS,
EFI_ACPI_X_FIRMWARE_WAKING_VECTOR,
- EFI_ACPI_5_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_VERSION,
+ EFI_ACPI_6_3_FIRMWARE_ACPI_CONTROL_STRUCTURE_VERSION,
{
EFI_ACPI_RESERVED_BYTE,
EFI_ACPI_RESERVED_BYTE,
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/Fadt/Fadt.c
b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/Fadt/Fadt.c
index 6efb38cda40d..38e767856de7 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/Fadt/Fadt.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/Fadt/Fadt.c
@@ -1,9 +1,10 @@
/** @file
- This file contains a structure definition for the ACPI 5.0 Fixed ACPI
+ This file contains a structure definition for the ACPI 6.3 Fixed ACPI
Description Table (FADT). The contents of this file should only be modified
for bug fixes, no porting is required.

Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) Microsoft Corporation.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -47,6 +48,12 @@ SPDX-License-Identifier: BSD-2-Clause-Patent

#define EFI_ACPI_IAPC_BOOT_ARCH 0 // To be fixed

+//
+// ARM Boot Architecture Flags
+//
+
+#define EFI_ACPI_ARM_BOOT_ARCH 0 // To be fixed
+
//
// Fixed Feature Flags
//
@@ -55,7 +62,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent // //
PM1A Event Register Block Generic Address Information // -#define
EFI_ACPI_PM1A_EVT_BLK_ADDRESS_SPACE_ID EFI_ACPI_2_0_SYSTEM_IO
+#define EFI_ACPI_PM1A_EVT_BLK_ADDRESS_SPACE_ID
EFI_ACPI_6_3_SYSTEM_IO
#define EFI_ACPI_PM1A_EVT_BLK_BIT_WIDTH 0x20
#define EFI_ACPI_PM1A_EVT_BLK_BIT_OFFSET 0x00
#define EFI_ACPI_PM1A_EVT_BLK_ADDRESS 0 // To be fixed
@@ -63,7 +70,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent // //
PM1B Event Register Block Generic Address Information // -#define
EFI_ACPI_PM1B_EVT_BLK_ADDRESS_SPACE_ID EFI_ACPI_2_0_SYSTEM_IO
+#define EFI_ACPI_PM1B_EVT_BLK_ADDRESS_SPACE_ID
EFI_ACPI_6_3_SYSTEM_IO
#define EFI_ACPI_PM1B_EVT_BLK_BIT_WIDTH 0x00
#define EFI_ACPI_PM1B_EVT_BLK_BIT_OFFSET 0x00
#define EFI_ACPI_PM1B_EVT_BLK_ADDRESS 0 // To be fixed
@@ -71,7 +78,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent // //
PM1A Control Register Block Generic Address Information // -#define
EFI_ACPI_PM1A_CNT_BLK_ADDRESS_SPACE_ID EFI_ACPI_2_0_SYSTEM_IO
+#define EFI_ACPI_PM1A_CNT_BLK_ADDRESS_SPACE_ID
EFI_ACPI_6_3_SYSTEM_IO
#define EFI_ACPI_PM1A_CNT_BLK_BIT_WIDTH 0x10
#define EFI_ACPI_PM1A_CNT_BLK_BIT_OFFSET 0x00
#define EFI_ACPI_PM1A_CNT_BLK_ADDRESS 0 // To be fixed
@@ -79,7 +86,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent // //
PM1B Control Register Block Generic Address Information // -#define
EFI_ACPI_PM1B_CNT_BLK_ADDRESS_SPACE_ID EFI_ACPI_2_0_SYSTEM_IO
+#define EFI_ACPI_PM1B_CNT_BLK_ADDRESS_SPACE_ID
EFI_ACPI_6_3_SYSTEM_IO
#define EFI_ACPI_PM1B_CNT_BLK_BIT_WIDTH 0x00
#define EFI_ACPI_PM1B_CNT_BLK_BIT_OFFSET 0x00
#define EFI_ACPI_PM1B_CNT_BLK_ADDRESS 0 // To be fixed
@@ -87,7 +94,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent // //
PM2 Control Register Block Generic Address Information // -#define
EFI_ACPI_PM2_CNT_BLK_ADDRESS_SPACE_ID EFI_ACPI_2_0_SYSTEM_IO
+#define EFI_ACPI_PM2_CNT_BLK_ADDRESS_SPACE_ID
EFI_ACPI_6_3_SYSTEM_IO
#define EFI_ACPI_PM2_CNT_BLK_BIT_WIDTH 0x08
#define EFI_ACPI_PM2_CNT_BLK_BIT_OFFSET 0x00
#define EFI_ACPI_PM2_CNT_BLK_ADDRESS 0 // To be fixed
@@ -96,7 +103,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent //
Power Management Timer Control Register Block Generic Address //
Information // -#define EFI_ACPI_PM_TMR_BLK_ADDRESS_SPACE_ID
EFI_ACPI_2_0_SYSTEM_IO
+#define EFI_ACPI_PM_TMR_BLK_ADDRESS_SPACE_ID
EFI_ACPI_6_3_SYSTEM_IO
#define EFI_ACPI_PM_TMR_BLK_BIT_WIDTH 0x20
#define EFI_ACPI_PM_TMR_BLK_BIT_OFFSET 0x00
#define EFI_ACPI_PM_TMR_BLK_ADDRESS 0 // To be fixed
@@ -105,7 +112,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent //
General Purpose Event 0 Register Block Generic Address // Information // -
#define EFI_ACPI_GPE0_BLK_ADDRESS_SPACE_ID
EFI_ACPI_2_0_SYSTEM_IO
+#define EFI_ACPI_GPE0_BLK_ADDRESS_SPACE_ID
EFI_ACPI_6_3_SYSTEM_IO
#define EFI_ACPI_GPE0_BLK_BIT_WIDTH 0 // size of
R_PCH_ACPI_GPE0_STS_127_96 + R_PCH_ACPI_GPE0_EN_127_96
#define EFI_ACPI_GPE0_BLK_BIT_OFFSET 0x00
#define EFI_ACPI_GPE0_BLK_ADDRESS 0 // To be fixed
@@ -114,14 +121,14 @@ SPDX-License-Identifier: BSD-2-Clause-Patent //
General Purpose Event 1 Register Block Generic Address // Information // -
#define EFI_ACPI_GPE1_BLK_ADDRESS_SPACE_ID
EFI_ACPI_2_0_SYSTEM_IO
+#define EFI_ACPI_GPE1_BLK_ADDRESS_SPACE_ID
EFI_ACPI_6_3_SYSTEM_IO
#define EFI_ACPI_GPE1_BLK_BIT_WIDTH 0x0
#define EFI_ACPI_GPE1_BLK_BIT_OFFSET 0x0
#define EFI_ACPI_GPE1_BLK_ADDRESS 0 // To be fixed
//
// Reset Register Generic Address Information // -#define
EFI_ACPI_RESET_REG_ADDRESS_SPACE_ID EFI_ACPI_2_0_SYSTEM_IO
+#define EFI_ACPI_RESET_REG_ADDRESS_SPACE_ID
EFI_ACPI_6_3_SYSTEM_IO
#define EFI_ACPI_RESET_REG_BIT_WIDTH 0x08
#define EFI_ACPI_RESET_REG_BIT_OFFSET 0x00
#define EFI_ACPI_RESET_REG_ADDRESS 0x00000CF9
@@ -162,11 +169,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent //
Please modify all values in Fadt.h only.
//

-EFI_ACPI_5_0_FIXED_ACPI_DESCRIPTION_TABLE Fadt = {
+EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE Fadt = {
{
- EFI_ACPI_5_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
- sizeof (EFI_ACPI_5_0_FIXED_ACPI_DESCRIPTION_TABLE),
- EFI_ACPI_5_0_FIXED_ACPI_DESCRIPTION_TABLE_REVISION,
+ EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
+ sizeof (EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE),
+ EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE_REVISION,

//
// Checksum will be updated at runtime @@ -187,9 +194,9 @@
EFI_ACPI_5_0_FIXED_ACPI_DESCRIPTION_TABLE Fadt = {
//
// These addresses will be updated at runtime
//
- 0x00000000,
0x00000000,
-
+ 0x00000000,
+
EFI_ACPI_RESERVED_BYTE,
EFI_ACPI_PREFERRED_PM_PROFILE,
EFI_ACPI_SCI_INT,
@@ -198,7 +205,7 @@ EFI_ACPI_5_0_FIXED_ACPI_DESCRIPTION_TABLE
Fadt = {
EFI_ACPI_ACPI_DISABLE,
EFI_ACPI_S4_BIOS_REQ,
EFI_ACPI_PSTATE_CNT,
-
+
EFI_ACPI_PM1A_EVT_BLK_ADDRESS,
EFI_ACPI_PM1B_EVT_BLK_ADDRESS,
EFI_ACPI_PM1A_CNT_BLK_ADDRESS,
@@ -240,15 +247,13 @@ EFI_ACPI_5_0_FIXED_ACPI_DESCRIPTION_TABLE
Fadt = {
EFI_ACPI_RESET_REG_ADDRESS_SPACE_ID,
EFI_ACPI_RESET_REG_BIT_WIDTH,
EFI_ACPI_RESET_REG_BIT_OFFSET,
- EFI_ACPI_5_0_BYTE,
+ EFI_ACPI_6_3_BYTE,
EFI_ACPI_RESET_REG_ADDRESS
},
EFI_ACPI_RESET_VALUE,
- {
- EFI_ACPI_RESERVED_BYTE,
- EFI_ACPI_RESERVED_BYTE,
- EFI_ACPI_RESERVED_BYTE
- },
+
+ EFI_ACPI_ARM_BOOT_ARCH,
+ EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE_MINOR_REVISION,

//
// These addresses will be updated at runtime @@ -263,7 +268,7 @@
EFI_ACPI_5_0_FIXED_ACPI_DESCRIPTION_TABLE Fadt = {
EFI_ACPI_PM1A_EVT_BLK_ADDRESS_SPACE_ID,
EFI_ACPI_PM1A_EVT_BLK_BIT_WIDTH,
EFI_ACPI_PM1A_EVT_BLK_BIT_OFFSET,
- EFI_ACPI_5_0_WORD,
+ EFI_ACPI_6_3_WORD,
EFI_ACPI_PM1A_EVT_BLK_ADDRESS
},
{
@@ -273,7 +278,7 @@ EFI_ACPI_5_0_FIXED_ACPI_DESCRIPTION_TABLE
Fadt = {
EFI_ACPI_PM1B_EVT_BLK_ADDRESS_SPACE_ID,
EFI_ACPI_PM1B_EVT_BLK_BIT_WIDTH,
EFI_ACPI_PM1B_EVT_BLK_BIT_OFFSET,
- EFI_ACPI_5_0_WORD,
+ EFI_ACPI_6_3_WORD,
EFI_ACPI_PM1B_EVT_BLK_ADDRESS
},
{
@@ -283,7 +288,7 @@ EFI_ACPI_5_0_FIXED_ACPI_DESCRIPTION_TABLE
Fadt = {
EFI_ACPI_PM1A_CNT_BLK_ADDRESS_SPACE_ID,
EFI_ACPI_PM1A_CNT_BLK_BIT_WIDTH,
EFI_ACPI_PM1A_CNT_BLK_BIT_OFFSET,
- EFI_ACPI_5_0_WORD,
+ EFI_ACPI_6_3_WORD,
EFI_ACPI_PM1A_CNT_BLK_ADDRESS
},
{
@@ -293,7 +298,7 @@ EFI_ACPI_5_0_FIXED_ACPI_DESCRIPTION_TABLE
Fadt = {
EFI_ACPI_PM1B_CNT_BLK_ADDRESS_SPACE_ID,
EFI_ACPI_PM1B_CNT_BLK_BIT_WIDTH,
EFI_ACPI_PM1B_CNT_BLK_BIT_OFFSET,
- EFI_ACPI_5_0_WORD,
+ EFI_ACPI_6_3_WORD,
EFI_ACPI_PM1B_CNT_BLK_ADDRESS
},
{
@@ -303,7 +308,7 @@ EFI_ACPI_5_0_FIXED_ACPI_DESCRIPTION_TABLE
Fadt = {
EFI_ACPI_PM2_CNT_BLK_ADDRESS_SPACE_ID,
EFI_ACPI_PM2_CNT_BLK_BIT_WIDTH,
EFI_ACPI_PM2_CNT_BLK_BIT_OFFSET,
- EFI_ACPI_5_0_BYTE,
+ EFI_ACPI_6_3_BYTE,
EFI_ACPI_PM2_CNT_BLK_ADDRESS
},
{
@@ -313,7 +318,7 @@ EFI_ACPI_5_0_FIXED_ACPI_DESCRIPTION_TABLE
Fadt = {
EFI_ACPI_PM_TMR_BLK_ADDRESS_SPACE_ID,
EFI_ACPI_PM_TMR_BLK_BIT_WIDTH,
EFI_ACPI_PM_TMR_BLK_BIT_OFFSET,
- EFI_ACPI_5_0_DWORD,
+ EFI_ACPI_6_3_DWORD,
EFI_ACPI_PM_TMR_BLK_ADDRESS
},
{
@@ -323,7 +328,7 @@ EFI_ACPI_5_0_FIXED_ACPI_DESCRIPTION_TABLE
Fadt = {
EFI_ACPI_GPE0_BLK_ADDRESS_SPACE_ID,
EFI_ACPI_GPE0_BLK_BIT_WIDTH,
EFI_ACPI_GPE0_BLK_BIT_OFFSET,
- EFI_ACPI_5_0_BYTE,
+ EFI_ACPI_6_3_BYTE,
EFI_ACPI_GPE0_BLK_ADDRESS
},
{
@@ -333,7 +338,7 @@ EFI_ACPI_5_0_FIXED_ACPI_DESCRIPTION_TABLE
Fadt = {
EFI_ACPI_GPE1_BLK_ADDRESS_SPACE_ID,
EFI_ACPI_GPE1_BLK_BIT_WIDTH,
EFI_ACPI_GPE1_BLK_BIT_OFFSET,
- EFI_ACPI_5_0_BYTE,
+ EFI_ACPI_6_3_BYTE,
EFI_ACPI_GPE1_BLK_ADDRESS
},
{
@@ -355,5 +360,10 @@ EFI_ACPI_5_0_FIXED_ACPI_DESCRIPTION_TABLE
Fadt = {
0,
0,
0
- }
+ },
+
+ //
+ // Hypervisor Vendor Identity
+ //
+ 0x0000000000000000,
};
--
2.28.0.windows.1
Re: [edk2-platforms][PATCH v2 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements

Nate DeSimone
 

The series has been pushed as 5b257da~..89fb75a

Thanks,
Nate

-----Original Message-----
From: mikuback@... <mikuback@...>
Sent: Thursday, August 5, 2021 6:33 PM
To: devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@...>; Desimone, Nathaniel L
<nathaniel.l.desimone@...>; Liming Gao
<gaoliming@...>; Dong, Eric <eric.dong@...>
Subject: [edk2-platforms][PATCH v2 0/5] MinPlatformPkg: TestPointCheckLib
bug fixes and improvements

From: Michael Kubacki <michael.kubacki@...>

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3531
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3518
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3520
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3521

This patch series groups together several bug fixes and improvements to
TestPointCheckLib. The first patch is required for the others since it fixes a
MinPlatformPkg build issue that occurs with the current edk2/master branch.

V2 changes:
1. Added Reviewed-by replies received for v1 series 2. [v1 2/5]: Added a
ZeroMem() for the ProtocolCapability buffer 3. [v1 3/5]: Added a #define for
the byte index 6 parameter

Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Liming Gao <gaoliming@...>
Cc: Eric Dong <eric.dong@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>

Michael Kubacki (5):
MinPlatformPkg/TestPointCheckLib: Fix MessageLength cast issue
MinPlatformPkg/TestPointCheckLib: Set required size field in protocol
MinPlatformPkg/TestPointCheckLib: Fix incorrect array index
MinPlatformPkg/TestPointCheckLib: Improve adjacent region checking
MinPlatformPkg/TestPointCheckLib: Make OutTable parameter optional


Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckA
cpi.c | 32 +++++------

Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckS
miHandlerInstrument.c | 4 +-

Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckS
mmInfo.c | 56 ++++++++++----------

Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckTc
gTrustedBoot.c | 3 ++

Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPoi
ntCheckLib.c | 15 +++++-

Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTestPo
intCheckLib.c | 26 +++++----
Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPointStubDxe.c
| 10 ++--
Platform/Intel/MinPlatformPkg/Include/Library/TestPointCheckLib.h
| 1 +

Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPoi
ntCheckLib.inf | 2 +
9 files changed, 87 insertions(+), 62 deletions(-)

--
2.28.0.windows.1
Re: [edk2-platforms][PATCH v2 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements

Nate DeSimone
 

For the series...

Reviewed-by: Nate DeSimone <nathaniel.l.desimone@...>

-----Original Message-----
From: mikuback@... <mikuback@...>
Sent: Thursday, August 5, 2021 6:33 PM
To: devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@...>; Desimone, Nathaniel L
<nathaniel.l.desimone@...>; Liming Gao
<gaoliming@...>; Dong, Eric <eric.dong@...>
Subject: [edk2-platforms][PATCH v2 0/5] MinPlatformPkg: TestPointCheckLib
bug fixes and improvements

From: Michael Kubacki <michael.kubacki@...>

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3531
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3518
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3520
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3521

This patch series groups together several bug fixes and improvements to
TestPointCheckLib. The first patch is required for the others since it fixes a
MinPlatformPkg build issue that occurs with the current edk2/master branch.

V2 changes:
1. Added Reviewed-by replies received for v1 series 2. [v1 2/5]: Added a
ZeroMem() for the ProtocolCapability buffer 3. [v1 3/5]: Added a #define for
the byte index 6 parameter

Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Liming Gao <gaoliming@...>
Cc: Eric Dong <eric.dong@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>

Michael Kubacki (5):
MinPlatformPkg/TestPointCheckLib: Fix MessageLength cast issue
MinPlatformPkg/TestPointCheckLib: Set required size field in protocol
MinPlatformPkg/TestPointCheckLib: Fix incorrect array index
MinPlatformPkg/TestPointCheckLib: Improve adjacent region checking
MinPlatformPkg/TestPointCheckLib: Make OutTable parameter optional


Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckA
cpi.c | 32 +++++------

Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckS
miHandlerInstrument.c | 4 +-

Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckS
mmInfo.c | 56 ++++++++++----------

Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckTc
gTrustedBoot.c | 3 ++

Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPoi
ntCheckLib.c | 15 +++++-

Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTestPo
intCheckLib.c | 26 +++++----
Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPointStubDxe.c
| 10 ++--
Platform/Intel/MinPlatformPkg/Include/Library/TestPointCheckLib.h
| 1 +

Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPoi
ntCheckLib.inf | 2 +
9 files changed, 87 insertions(+), 62 deletions(-)

--
2.28.0.windows.1
[edk2-platforms][PATCH v1 1/1] MinPlatformPkg/TestPointCheckLib: Add support for BME device exemption

Michael Kubacki
 

From: Chris Ruffin <v-cruffin@...>

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D3541

Some platforms have devices which do not expose any additional
risk of DMA attacks but the BME bit cannot be disabled.

To allow MinPlatformPkg consumers to selectively exempt certain
devices from the PCI bus master test point, this change adds a
PCD to MinPlatformPkg.dec that allows those packages to specify
a list of PCI devices by S/B/D/F that should be excluded from
testing.

Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Liming Gao <gaoliming@...>
Cc: Eric Dong <eric.dong@...>
Cc: Chris Ruffin <v-cruffin@...>
Co-authored-by: Michael Kubacki <michael.kubacki@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>
---
Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckPci=
.c | 37 ++++++++++++++++++--
Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/PeiCheckPci=
.c | 35 ++++++++++++++++++
Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec =
| 4 +++
Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPoin=
tCheckLib.inf | 1 +
Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/PeiTestPoin=
tCheckLib.inf | 1 +
5 files changed, 75 insertions(+), 3 deletions(-)

diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib=
/DxeCheckPci.c b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointChec=
kLib/DxeCheckPci.c
index 514003944758..95f4fb8b7c7e 100644
--- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeChe=
ckPci.c
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeChe=
ckPci.c
@@ -44,6 +44,13 @@ typedef struct {
UINT32 Data[48];
} PCI_CONFIG_SPACE;
=20
+typedef struct {
+ UINT8 Segment;
+ UINT8 Bus;
+ UINT8 Device;
+ UINT8 Function;
+} EXEMPT_DEVICE;
+
#pragma pack()
=20
VOID
@@ -256,7 +263,7 @@ TestPointCheckPciResource (
UINT16 MinBus;
UINT16 MaxBus;
BOOLEAN IsEnd;
- =20
+
DEBUG ((DEBUG_INFO, "=3D=3D=3D=3D TestPointCheckPciResource - Enter\n"=
));
HandleBuf =3D NULL;
Status =3D gBS->LocateHandleBuffer (
@@ -338,7 +345,7 @@ TestPointCheckPciResource (
// Device
DumpPciDevice ((UINT8)Bus, (UINT8)Device, (UINT8)Func, &=
PciData);
}
- =20
+
//
// If this is not a multi-function device, we can leave th=
e loop
// to deal with the next device.
@@ -360,7 +367,7 @@ TestPointCheckPciResource (
}
}
}
- =20
+
Done:
if (HandleBuf !=3D NULL) {
FreePool (HandleBuf);
@@ -396,6 +403,9 @@ TestPointCheckPciBusMaster (
UINT8 HeaderType;
EFI_STATUS Status;
PCI_SEGMENT_INFO *PciSegmentInfo;
+ EXEMPT_DEVICE *ExemptDevicePcdPtr;
+ BOOLEAN ExemptDeviceFound;
+ UINTN Index;
=20
PciSegmentInfo =3D GetPciSegmentInfo (&SegmentCount);
if (PciSegmentInfo =3D=3D NULL) {
@@ -407,6 +417,27 @@ TestPointCheckPciBusMaster (
for (Bus =3D PciSegmentInfo[Segment].StartBusNumber; Bus <=3D PciSeg=
mentInfo[Segment].EndBusNumber; Bus++) {
for (Device =3D 0; Device <=3D 0x1F; Device++) {
for (Function =3D 0; Function <=3D 0x7; Function++) {
+ //
+ // Some platforms have devices which do not expose any additio=
nal
+ // risk of DMA attacks but are not able to be turned off. All=
ow
+ // the platform to define these devices and do not record erro=
rs
+ // for these devices.
+ //
+ ExemptDevicePcdPtr =3D (EXEMPT_DEVICE *) PcdGetPtr (PcdTestPoi=
ntIbvPlatformExemptPciBme);
+ ExemptDeviceFound =3D FALSE;
+ for (Index =3D 0; Index < (PcdGetSize (PcdTestPointIbvPlatform=
ExemptPciBme) / sizeof (EXEMPT_DEVICE)); Index++) {
+ if (Segment =3D=3D ExemptDevicePcdPtr[Index].Segment
+ && Bus =3D=3D ExemptDevicePcdPtr[Index].Bus
+ && Device =3D=3D ExemptDevicePcdPtr[Index].Device
+ && Function =3D=3D ExemptDevicePcdPtr[Index].Function) {
+ ExemptDeviceFound =3D TRUE;
+ }
+ }
+
+ if (ExemptDeviceFound) {
+ continue;
+ }
+
VendorId =3D PciSegmentRead16 (PCI_SEGMENT_LIB_ADDRESS(PciSegm=
entInfo[Segment].SegmentNumber, Bus, Device, Function, PCI_VENDOR_ID_OFFS=
ET));
//
// If VendorId =3D 0xffff, there does not exist a device at th=
is
diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib=
/PeiCheckPci.c b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointChec=
kLib/PeiCheckPci.c
index 1061f8ac1c62..25c3caba6eed 100644
--- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/PeiChe=
ckPci.c
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/PeiChe=
ckPci.c
@@ -14,6 +14,17 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/PciSegmentInfoLib.h>
#include <IndustryStandard/Pci.h>
=20
+#pragma pack(1)
+
+ typedef struct EXEMPT_DEVICE_STRUCT {
+ UINT8 Segment;
+ UINT8 Bus;
+ UINT8 Device;
+ UINT8 Function;
+} EXEMPT_DEVICE;
+
+#pragma pack()
+
EFI_STATUS
TestPointCheckPciBusMaster (
VOID
@@ -29,6 +40,9 @@ TestPointCheckPciBusMaster (
UINT8 HeaderType;
EFI_STATUS Status;
PCI_SEGMENT_INFO *PciSegmentInfo;
+ EXEMPT_DEVICE *ExemptDevicePcdPtr;
+ BOOLEAN ExemptDeviceFound;
+ UINTN Index;
=20
PciSegmentInfo =3D GetPciSegmentInfo (&SegmentCount);
if (PciSegmentInfo =3D=3D NULL) {
@@ -40,6 +54,27 @@ TestPointCheckPciBusMaster (
for (Bus =3D PciSegmentInfo[Segment].StartBusNumber; Bus <=3D PciSeg=
mentInfo[Segment].EndBusNumber; Bus++) {
for (Device =3D 0; Device <=3D 0x1F; Device++) {
for (Function =3D 0; Function <=3D 0x7; Function++) {
+ //
+ // Some platforms have devices which do not expose any additio=
nal
+ // risk of DMA attacks but are not able to be turned off. All=
ow
+ // the platform to define these devices and do not record erro=
rs
+ // for these devices.
+ //
+ ExemptDevicePcdPtr =3D (EXEMPT_DEVICE *) PcdGetPtr (PcdTestPoi=
ntIbvPlatformExemptPciBme);
+ ExemptDeviceFound =3D FALSE;
+ for (Index =3D 0; Index < (PcdGetSize (PcdTestPointIbvPlatform=
ExemptPciBme) / sizeof (EXEMPT_DEVICE)); Index++) {
+ if (Segment =3D=3D ExemptDevicePcdPtr[Index].Segment
+ && Bus =3D=3D ExemptDevicePcdPtr[Index].Bus
+ && Device =3D=3D ExemptDevicePcdPtr[Index].Device
+ && Function =3D=3D ExemptDevicePcdPtr[Index].Function) {
+ ExemptDeviceFound =3D TRUE;
+ }
+ }
+
+ if (ExemptDeviceFound) {
+ continue;
+ }
+
VendorId =3D PciSegmentRead16 (PCI_SEGMENT_LIB_ADDRESS(PciSegm=
entInfo[Segment].SegmentNumber, Bus, Device, Function, PCI_VENDOR_ID_OFFS=
ET));
//
// If VendorId =3D 0xffff, there does not exist a device at th=
is
diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec b/Platform/=
Intel/MinPlatformPkg/MinPlatformPkg.dec
index bcb42f0ef9e6..259038dde4df 100644
--- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
+++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
@@ -160,6 +160,10 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
# Stage Advanced: {0x03,=
0x0F, 0x03, 0x1D, 0x3F, 0x0F, 0x0F, 0x07, 0x03, 0x00, 0x00, 0x00, 0x00, =
0x00, 0x00, 0x00}
gMinPlatformPkgTokenSpaceGuid.PcdTestPointIbvPlatformFeature|{0x03, 0x=
0F, 0x03, 0x1D, 0x3F, 0x0F, 0x0F, 0x07, 0x03, 0x00, 0x00, 0x00, 0x00, 0x0=
0, 0x00, 0x00}|VOID*|0x00100302
=20
+ # The platform may define a list of devices that are exempt from PCI B=
ME testing.
+ # PCD Format is {SegmentNumber1, BusNumber1, DeviceNumber1, FunctionNu=
mber1, SegmentNumber2, BusNumber2, DeviceNumber2, FunctionNumber2, ...}
+ gMinPlatformPkgTokenSpaceGuid.PcdTestPointIbvPlatformExemptPciBme|{0}|=
VOID*|0x00100303
+
##
## The Flash relevant PCD are ineffective and will be patched basing o=
n FDF definitions during build.
## Set all of them to 0 here to prevent from confusion.
diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib=
/DxeTestPointCheckLib.inf b/Platform/Intel/MinPlatformPkg/Test/Library/Te=
stPointCheckLib/DxeTestPointCheckLib.inf
index 2ae1db4ee483..15779eb9b6de 100644
--- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTes=
tPointCheckLib.inf
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTes=
tPointCheckLib.inf
@@ -106,3 +106,4 @@ [Protocols]
=20
[Pcd]
gMinPlatformPkgTokenSpaceGuid.PcdTestPointIbvPlatformFeature
+ gMinPlatformPkgTokenSpaceGuid.PcdTestPointIbvPlatformExemptPciBme
diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib=
/PeiTestPointCheckLib.inf b/Platform/Intel/MinPlatformPkg/Test/Library/Te=
stPointCheckLib/PeiTestPointCheckLib.inf
index 51369fcedc1e..ea6dc6b8ba34 100644
--- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/PeiTes=
tPointCheckLib.inf
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/PeiTes=
tPointCheckLib.inf
@@ -47,6 +47,7 @@ [Sources]
=20
[Pcd]
gMinPlatformPkgTokenSpaceGuid.PcdTestPointIbvPlatformFeature
+ gMinPlatformPkgTokenSpaceGuid.PcdTestPointIbvPlatformExemptPciBme
=20
[Guids]
gEfiHobMemoryAllocStackGuid
--=20
2.28.0.windows.1
Re: [PATCH v2 0/4] Ovmf: Disable the TPM2 platform hierarchy

Stefan Berger
 

On 8/9/21 1:54 PM, James Bottomley wrote:
On Mon, 2021-08-09 at 12:37 -0400, Stefan Berger wrote:
This series imports code from the edk2-platforms project related to
changing the password of the TPM2 platform hierarchy and uses it to
disable the TPM2 platform hierarchy in Ovmf. It addresses the Ovmf
aspects of the following bugs:

https://bugzilla.tianocore.org/show_bug.cgi?id=3510
https://bugzilla.tianocore.org/show_bug.cgi?id=3499
This raises a couple of issues:

1. Since OVMF is for all x86 virtual platforms not just the PC ones,
should it be following the PC client spec for everything? I notice
you left out Xen and Bhyve ... should they never follow this?
I am not sure how to build Bhyve but one part of the patch is already there for it in this series:


If this is how you build Bhyve I am getting a build failure already before these patches here are applied.

build -p OvmfPkg/Bhyve/BhyveX64.dsc -b DEBUG -a X64 -t GCC5 -D TPM_ENABLE -D TPM_CONFIG_ENABLE -D SECURE_BOOT_ENABLE -D NETWORK_TLS_ENABLE 2>&1 | tee build.log
Build environment: Linux-5.12.14-300.fc34.x86_64-x86_64-with-glibc2.33
Build start time: 14:21:41, Aug.09 2021

WORKSPACE        = /home/stefanb/dev/edk2
EDK_TOOLS_PATH   = /home/stefanb/dev/edk2/BaseTools
CONF_PATH        = /home/stefanb/dev/edk2/Conf
PYTHON_COMMAND   = /usr/bin/python3.9


Processing meta-data .
Architecture(s)  = X64
Build target     = DEBUG
Toolchain        = GCC5

Active Platform          = /home/stefanb/dev/edk2/OvmfPkg/Bhyve/BhyveX64.dsc


build.py...
/home/stefanb/dev/edk2/OvmfPkg/Bhyve/BhyveX64.dsc(198): error 000E: File/directory not found in workspace
/home/stefanb/dev/edk2/OvmfPkg/Bhyve/Library/PlatformSecureLib/PlatformSecureLib.inf


2. Since OVMF is effectively both the platform and the firmware, what
attitude should we take to code in edk2-platforms? There are
arguments for pulling all the necessary components into OVMF, but it
could also be argued that the VMM should take care of all the edk2-
platforms pieces and OVMF should be strictly firmware.
That's what I had been wondering about in V1 as well. This import here now followed the option 2 in that discussion and I cut out basically only the function that disables the platform hierarchy rather than setting a random password, which I kept since it didn't seem to require further dependencies. to be imported from edk2-platforms.



Getting 2. sorted out is probably the more pressing policy issue for
us.

James
Re: [PATCH v2 0/4] Ovmf: Disable the TPM2 platform hierarchy

James Bottomley
 

On Mon, 2021-08-09 at 12:37 -0400, Stefan Berger wrote:
This series imports code from the edk2-platforms project related to
changing the password of the TPM2 platform hierarchy and uses it to
disable the TPM2 platform hierarchy in Ovmf. It addresses the Ovmf
aspects of the following bugs:

https://bugzilla.tianocore.org/show_bug.cgi?id=3510
https://bugzilla.tianocore.org/show_bug.cgi?id=3499
This raises a couple of issues:

1. Since OVMF is for all x86 virtual platforms not just the PC ones,
should it be following the PC client spec for everything? I notice
you left out Xen and Bhyve ... should they never follow this?
2. Since OVMF is effectively both the platform and the firmware, what
attitude should we take to code in edk2-platforms? There are
arguments for pulling all the necessary components into OVMF, but it
could also be argued that the VMM should take care of all the edk2-
platforms pieces and OVMF should be strictly firmware.

Getting 2. sorted out is probably the more pressing policy issue for
us.

James
Re: [PATCH 2/3] OvmfPkg/ResetVector: update SEV support to use new work area format

Brijesh Singh
 

On 8/9/21 11:54 AM, Tom Lendacky wrote:
On 8/4/21 3:20 PM, Brijesh Singh wrote:
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3429

Update the SEV support to switch to using the newer work area format.

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Erdem Aktas <erdemaktas@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
OvmfPkg/ResetVector/ResetVector.inf | 1 +
OvmfPkg/Sec/SecMain.inf | 1 +
OvmfPkg/Sec/SecMain.c | 25 ++++++++++++++++++++++-
OvmfPkg/ResetVector/Ia32/AmdSev.asm | 8 ++++++++
OvmfPkg/ResetVector/Ia32/PageTables64.asm | 4 ++++
OvmfPkg/ResetVector/ResetVector.nasmb | 1 +
6 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/ResetVector/ResetVector.inf b/OvmfPkg/ResetVector/ResetVector.inf
index d028c92d8cfa..6ec9cca40c3a 100644
--- a/OvmfPkg/ResetVector/ResetVector.inf
+++ b/OvmfPkg/ResetVector/ResetVector.inf
@@ -34,6 +34,7 @@ [BuildOptions]
*_*_X64_NASMB_FLAGS = -I$(WORKSPACE)/UefiCpuPkg/ResetVector/Vtf0/
[Pcd]
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase
Laszlo was trying to keep things sorted, so you should move this down to
the end of the list.
Yes, I will try to keep it sorted.



gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize
diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf
index 7f78dcee2772..82910dcbd5c2 100644
--- a/OvmfPkg/Sec/SecMain.inf
+++ b/OvmfPkg/Sec/SecMain.inf
@@ -56,6 +56,7 @@ [Ppis]
gEfiTemporaryRamSupportPpiGuid # PPI ALWAYS_PRODUCED
[Pcd]
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase
Ditto here, even though the list isn't truly sorted to begin with.
Noted.


gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize
diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
index 9db67e17b2aa..dda572c7ad7d 100644
--- a/OvmfPkg/Sec/SecMain.c
+++ b/OvmfPkg/Sec/SecMain.c
@@ -807,6 +807,29 @@ SevEsProtocolCheck (
Ghcb->GhcbUsage = GHCB_STANDARD_USAGE;
}
+/**
+ Determine if the SEV is active.
+
+ During the early booting, GuestType is set in the work area. Verify that it
+ is an SEV guest.
+
+ @retval TRUE SEV is enabled
+ @retval FALSE SEV is not enabled
+
+**/
+STATIC
+BOOLEAN
+IsSevGuest (
+ VOID
+ )
+{
+ OVMF_WORK_AREA *WorkArea;
+
+ WorkArea = (OVMF_WORK_AREA *) FixedPcdGet32 (PcdOvmfWorkAreaBase);
+
+ return ((WorkArea != NULL) && (WorkArea->GuestType == GUEST_TYPE_AMD_SEV));
+}
+
/**
Determine if SEV-ES is active.
@@ -828,7 +851,7 @@ SevEsIsEnabled (
SevEsWorkArea = (SEC_SEV_ES_WORK_AREA *) FixedPcdGet32 (PcdSevEsWorkAreaBase);
- return ((SevEsWorkArea != NULL) && (SevEsWorkArea->SevEsEnabled != 0));
+ return (((IsSevGuest()) && SevEsWorkArea != NULL) && (SevEsWorkArea->SevEsEnabled != 0));
The IsSevGuest() function checks for a NULL work area, so there's no need
to check for SevEsWorkArea being non-NULL now. I think it would read
better, though, to do:
if (!IsSevGuest ()) {
return FALSE;
}
SevEsWorkArea = ...
return (SevEsWorkArea->SevEsEnabled != 0);

}
Sure, it makes it a bit more readiable and avoids unessary checks.

VOID
diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
index aa95d06eaddb..87d81b01e263 100644
--- a/OvmfPkg/ResetVector/Ia32/AmdSev.asm
+++ b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
@@ -171,6 +171,9 @@ CheckSevFeatures:
bt eax, 0
jnc NoSev
+ ; Set the work area header to indicate that the SEV is enabled
s/the SEV/SEV/
Noted.

+ mov byte[WORK_AREA_GUEST_TYPE], 1
The "1" should probably be defined in ResetVector.nasmb as a %define.
Sure, I will define the constant


+
; Check for SEV-ES memory encryption feature:
; CPUID Fn8000_001F[EAX] - Bit 3
; CPUID raises a #VC exception if running as an SEV-ES guest
@@ -257,6 +260,11 @@ SevExit:
IsSevEsEnabled:
xor eax, eax
+ ; During CheckSevFeatures, the WORK_AREA_GUEST_TYPE is set
+ ; to 1 if SEV is enabled.
+ cmp byte[WORK_AREA_GUEST_TYPE], 1
+ jne SevEsDisabled
+
; During CheckSevFeatures, the SEV_ES_WORK_AREA was set to 1 if
; SEV-ES is enabled.
cmp byte[SEV_ES_WORK_AREA], 1
diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
index eacdb69ddb9f..f688909f1c7d 100644
--- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
+++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
@@ -42,6 +42,10 @@ BITS 32
;
SetCr3ForPageTables64:
+ ; Clear the WorkArea header. The SEV probe routines will populate the
How about:
; Initialize the WorkArea header to indicate a legacy guest. The ...

+ ; work area when detected.
+ mov byte[WORK_AREA_GUEST_TYPE], 0
And then use a %define here for the '0'
I will make the required changes.

+
OneTimeCall CheckSevFeatures
xor edx, edx
test eax, eax
diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb b/OvmfPkg/ResetVector/ResetVector.nasmb
index acec46a32450..d1d800c56745 100644
--- a/OvmfPkg/ResetVector/ResetVector.nasmb
+++ b/OvmfPkg/ResetVector/ResetVector.nasmb
@@ -72,6 +72,7 @@
%define GHCB_PT_ADDR (FixedPcdGet32 (PcdOvmfSecGhcbPageTableBase))
%define GHCB_BASE (FixedPcdGet32 (PcdOvmfSecGhcbBase))
%define GHCB_SIZE (FixedPcdGet32 (PcdOvmfSecGhcbSize))
+ %define WORK_AREA_GUEST_TYPE (FixedPcdGet32 (PcdOvmfWorkAreaBase))
Create %defines for each of the defined enum types from the first patch.
Got it. thanks
Re: [PATCH 1/3] OvmfPkg: introduce a common work area

Brijesh Singh
 

On 8/9/21 11:40 AM, Tom Lendacky wrote:
On 8/4/21 3:20 PM, Brijesh Singh wrote:
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3429

Both the TDX and SEV support needs to reserve a page in MEMFD as a work
area. The page will contain meta data specific to the guest type.
Currently, the SEV-ES support reserves a page in MEMFD
(PcdSevEsWorkArea) for the work area. This page can be reused as a TDX
work area when Intel TDX is enabled.

Based on the discussion [1], it was agreed to rename the SevEsWorkArea
to the OvmfWorkArea, and add a header that can be used to indicate the
work area type.

[1] https://edk2.groups.io/g/devel/message/78262?p=,,,20,0,0,0::\
created,0,SNP,20,2,0,84476064

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Erdem Aktas <erdemaktas@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
OvmfPkg/OvmfPkg.dec | 6 +++
OvmfPkg/OvmfPkgX64.fdf | 9 +++-
OvmfPkg/PlatformPei/PlatformPei.inf | 4 +-
OvmfPkg/Include/Library/MemEncryptSevLib.h | 21 +--------
OvmfPkg/Include/WorkArea.h | 53 ++++++++++++++++++++++
OvmfPkg/PlatformPei/MemDetect.c | 32 ++++++-------
6 files changed, 85 insertions(+), 40 deletions(-)
create mode 100644 OvmfPkg/Include/WorkArea.h

diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 2ab27f0c73c2..9d31ec45c78a 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -330,6 +330,12 @@ [PcdsFixedAtBuild]
gUefiOvmfPkgTokenSpaceGuid.PcdQemuHashTableBase|0x0|UINT32|0x47
gUefiOvmfPkgTokenSpaceGuid.PcdQemuHashTableSize|0x0|UINT32|0x48
+ ## The base address and size of the work area used during the SEC
+ # phase by the SEV and TDX supports.
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase|0|UINT32|0x49
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize|0|UINT32|0x50
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaHeaderSize|4|UINT32|0x51
+
[PcdsDynamic, PcdsDynamicEx]
gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent|0|UINT64|2
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable|FALSE|BOOLEAN|0x10
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index 5fa8c0895808..418e0ea5add4 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -83,7 +83,7 @@ [FD.MEMFD]
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize
0x00B000|0x001000
-gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize
+gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize
0x00C000|0x001000
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupSize
@@ -99,6 +99,13 @@ [FD.MEMFD]
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvSize
FV = DXEFV
+##########################################################################################
+# SEV specific PCD settings
+SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaHeaderSize = 0x4
+SET gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase = $(MEMFD_BASE_ADDRESS) + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaHeaderSize
+SET gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize - gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaHeaderSize
+##########################################################################################
+
################################################################################
[FV.SECFV]
diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index 89d1f7636870..67eb7aa7166b 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -116,8 +116,8 @@ [FixedPcd]
gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesData
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupSize
- gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase
- gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize
[FeaturePcd]
gUefiOvmfPkgTokenSpaceGuid.PcdCsmEnable
diff --git a/OvmfPkg/Include/Library/MemEncryptSevLib.h b/OvmfPkg/Include/Library/MemEncryptSevLib.h
index 76d06c206c8b..adc490e466ec 100644
--- a/OvmfPkg/Include/Library/MemEncryptSevLib.h
+++ b/OvmfPkg/Include/Library/MemEncryptSevLib.h
@@ -12,6 +12,7 @@
#define _MEM_ENCRYPT_SEV_LIB_H_
#include <Base.h>
+#include <WorkArea.h>
//
// Define the maximum number of #VCs allowed (e.g. the level of nesting
@@ -36,26 +37,6 @@ typedef struct {
VOID *GhcbBackupPages;
} SEV_ES_PER_CPU_DATA;
-//
-// Internal structure for holding SEV-ES information needed during SEC phase
-// and valid only during SEC phase and early PEI during platform
-// initialization.
-//
-// This structure is also used by assembler files:
-// OvmfPkg/ResetVector/ResetVector.nasmb
-// OvmfPkg/ResetVector/Ia32/PageTables64.asm
-// OvmfPkg/ResetVector/Ia32/Flat32ToFlat64.asm
-// any changes must stay in sync with its usage.
-//
-typedef struct _SEC_SEV_ES_WORK_AREA {
- UINT8 SevEsEnabled;
- UINT8 Reserved1[7];
-
- UINT64 RandomData;
-
- UINT64 EncryptionMask;
-} SEC_SEV_ES_WORK_AREA;
-
//
// Memory encryption address range states.
//
diff --git a/OvmfPkg/Include/WorkArea.h b/OvmfPkg/Include/WorkArea.h
new file mode 100644
index 000000000000..0aaad7e1da67
--- /dev/null
+++ b/OvmfPkg/Include/WorkArea.h
@@ -0,0 +1,53 @@
+/** @file
+
+ Work Area structure definition
+
+ Copyright (c) 2021, AMD Inc.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef __OVMF_WORK_AREA_H__
+#define __OVMF_WORK_AREA_H__
+
+//
+// Internal structure for holding SEV-ES information needed during SEC phase
+// and valid only during SEC phase and early PEI during platform
+// initialization.
+//
+// This structure is also used by assembler files:
+// OvmfPkg/ResetVector/ResetVector.nasmb
+// OvmfPkg/ResetVector/Ia32/PageTables64.asm
+// OvmfPkg/ResetVector/Ia32/Flat32ToFlat64.asm
+// any changes must stay in sync with its usage.
+//
+typedef struct _SEC_SEV_ES_WORK_AREA {
+ UINT8 SevEsEnabled;
+ UINT8 Reserved1[7];
+
+ UINT64 RandomData;
+
+ UINT64 EncryptionMask;
+} SEC_SEV_ES_WORK_AREA;
+
+//
+// Guest type for the work area
+//
+typedef enum {
+ GUEST_TYPE_NON_ENCRYPTED,
+ GUEST_TYPE_AMD_SEV,
+ GUEST_TYPE_INTEL_TDX,
+
+} GUEST_TYPE;
+
+//
+// The work area structure header definition.
+//
+typedef struct _OVMF_WORK_AREA {
+ UINT8 GuestType;
+ UINT8 Reserved1[3];
+
+ SEC_SEV_ES_WORK_AREA SevEsWorkArea;
+} OVMF_WORK_AREA;
+
+#endif
diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
index 2deec128f464..4c53b0fdf2fe 100644
--- a/OvmfPkg/PlatformPei/MemDetect.c
+++ b/OvmfPkg/PlatformPei/MemDetect.c
@@ -939,23 +939,21 @@ InitializeRamRegions (
}
#ifdef MDE_CPU_X64
- if (MemEncryptSevEsIsEnabled ()) {
- //
- // If SEV-ES is enabled, reserve the SEV-ES work area.
- //
- // Since this memory range will be used by the Reset Vector on S3
- // resume, it must be reserved as ACPI NVS.
- //
- // If S3 is unsupported, then various drivers might still write to the
- // work area. We ought to prevent DXE from serving allocation requests
- // such that they would overlap the work area.
- //
- BuildMemoryAllocationHob (
- (EFI_PHYSICAL_ADDRESS)(UINTN) FixedPcdGet32 (PcdSevEsWorkAreaBase),
- (UINT64)(UINTN) FixedPcdGet32 (PcdSevEsWorkAreaSize),
- mS3Supported ? EfiACPIMemoryNVS : EfiBootServicesData
- );
- }
+ //
+ // Reserve the work area.
+ //
+ // Since this memory range will be used by the Reset Vector on S3
+ // resume, it must be reserved as ACPI NVS.
+ //
+ // If S3 is unsupported, then various drivers might still write to the
+ // work area. We ought to prevent DXE from serving allocation requests
+ // such that they would overlap the work area.
+ //
+ BuildMemoryAllocationHob (
+ (EFI_PHYSICAL_ADDRESS)(UINTN) FixedPcdGet32 (PcdOvmfWorkAreaBase),
+ (UINT64)(UINTN) FixedPcdGet32 (PcdOvmfWorkAreaSize),
+ mS3Supported ? EfiACPIMemoryNVS : EfiBootServicesData
+ );
If SEV-ES is enabled, then we previously had already verified that the
work area was present. Without that check now, it may not be. Just for
safety, it is probably worth replacing the:
if (MemEncryptSevEsIsEnabled ()) {
with
if (FixedPcdGet32 (PcdOvmfWorkAreaSize) != 0) {
Noted.


thanks
Re: [PATCH 3/3] OvmfPkg/ResetVector: move the GHCB page setup in AmdSev.asm

Lendacky, Thomas
 

On 8/4/21 3:20 PM, Brijesh Singh wrote:
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3429

While build the initial page table, the SetCr3ForPageTables64 checks
whether SEV-ES is enabled. If so, clear the page encryption mask from the
GHCB page. Move the logic to clear the page encryption mask in the
AmdSev.asm.

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Erdem Aktas <erdemaktas@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
OvmfPkg/ResetVector/Ia32/AmdSev.asm | 113 +++++++++++++++++-----
OvmfPkg/ResetVector/Ia32/PageTables64.asm | 53 ++--------
2 files changed, 94 insertions(+), 72 deletions(-)

diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
index 87d81b01e263..fd2e6abcd4a0 100644
--- a/OvmfPkg/ResetVector/Ia32/AmdSev.asm
+++ b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
@@ -44,6 +44,27 @@ BITS 32
; The unexpected response code
%define TERM_UNEXPECTED_RESP_CODE 2

+%define PAGE_PRESENT 0x01
+%define PAGE_READ_WRITE 0x02
+%define PAGE_USER_SUPERVISOR 0x04
+%define PAGE_WRITE_THROUGH 0x08
+%define PAGE_CACHE_DISABLE 0x010
+%define PAGE_ACCESSED 0x020
+%define PAGE_DIRTY 0x040
+%define PAGE_PAT 0x080
+%define PAGE_GLOBAL 0x0100
+%define PAGE_2M_MBO 0x080
+%define PAGE_2M_PAT 0x01000
+
+%define PAGE_4K_PDE_ATTR (PAGE_ACCESSED + \
+ PAGE_DIRTY + \
+ PAGE_READ_WRITE + \
+ PAGE_PRESENT)
+
+%define PAGE_PDP_ATTR (PAGE_ACCESSED + \
+ PAGE_READ_WRITE + \
+ PAGE_PRESENT)
+

; Macro is used to issue the MSR protocol based VMGEXIT. The caller is
; responsible to populate values in the EDX:EAX registers. After the vmmcall
@@ -117,6 +138,72 @@ BITS 32
SevEsUnexpectedRespTerminate:
TerminateVmgExit TERM_UNEXPECTED_RESP_CODE

+; If SEV-ES is enabled then initialize the make the GHCB page shared
s/the make/and make/ ?

+SevClearPageEncMaskFromGHCBPage:
Just a nit, maybe SevClearPageEncMaskForGhcbPage?

+ ; Check if SEV is enabled
+ cmp byte[WORK_AREA_GUEST_TYPE], 1
+ jnz SevClearPageEncMaskFromGHCBPageExit
+
+ ; Check if SEV-ES is enabled
+ cmp byte[SEV_ES_WORK_AREA], 1
+ jnz SevClearPageEncMaskFromGHCBPageExit
+
+ ;
+ ; The initial GHCB will live at GHCB_BASE and needs to be un-encrypted.
+ ; This requires the 2MB page for this range be broken down into 512 4KB
+ ; pages. All will be marked encrypted, except for the GHCB.
+ ;
+ mov ecx, (GHCB_BASE >> 21)
+ mov eax, GHCB_PT_ADDR + PAGE_PDP_ATTR
+ mov [ecx * 8 + PT_ADDR (0x2000)], eax
+
+ ;
+ ; Page Table Entries (512 * 4KB entries => 2MB)
+ ;
+ mov ecx, 512
+pageTableEntries4kLoop:
+ mov eax, ecx
+ dec eax
+ shl eax, 12
+ add eax, GHCB_BASE & 0xFFE0_0000
+ add eax, PAGE_4K_PDE_ATTR
+ mov [ecx * 8 + GHCB_PT_ADDR - 8], eax
+ mov [(ecx * 8 + GHCB_PT_ADDR - 8) + 4], edx
+ loop pageTableEntries4kLoop
+
+ ;
+ ; Clear the encryption bit from the GHCB entry
+ ;
+ mov ecx, (GHCB_BASE & 0x1F_FFFF) >> 12
+ mov [ecx * 8 + GHCB_PT_ADDR + 4], strict dword 0
+
+ mov ecx, GHCB_SIZE / 4
+ xor eax, eax
+clearGhcbMemoryLoop:
+ mov dword[ecx * 4 + GHCB_BASE - 4], eax
+ loop clearGhcbMemoryLoop
+
+SevClearPageEncMaskFromGHCBPageExit:
+ OneTimeCallRet SevClearPageEncMaskFromGHCBPage
+
+; Check if SEV is enabled, and get the C-bit mask above 31.
+; Modified: EDX
+;
+; The value is returned in the EDX
+GetSevCBitMaskAbove31:
+ ; Check if SEV is enabled
+ cmp byte[WORK_AREA_GUEST_TYPE], 1
+ jnz NoCbitValue
+
+ mov edx, dword[SEV_ES_WORK_AREA_ENC_MASK + 4]
+ jmp GetSevCBitMaskAbove31Exit
+
+NoCbitValue:
+ xor edx, edx
How about moving the xor as the first line of this routine and jumping to
GetSevCBitMaskAbove31Exit if the first cmp is non-zero. Then you can just
do the move from SEV_ES_WORK_AREA_ENC_MASK + 4 and eliminate the extra jmp
statement and NoCbitValue label.

Thanks,
Tom

+
+GetSevCBitMaskAbove31Exit:
+ OneTimeCallRet GetSevCBitMaskAbove31
+
; Check if Secure Encrypted Virtualization (SEV) features are enabled.
;
; Register usage is tight in this routine, so multiple calls for the
@@ -249,32 +336,6 @@ SevExit:

OneTimeCallRet CheckSevFeatures

-; Check if Secure Encrypted Virtualization - Encrypted State (SEV-ES) feature
-; is enabled.
-;
-; Modified: EAX
-;
-; If SEV-ES is enabled then EAX will be non-zero.
-; If SEV-ES is disabled then EAX will be zero.
-;
-IsSevEsEnabled:
- xor eax, eax
-
- ; During CheckSevFeatures, the WORK_AREA_GUEST_TYPE is set
- ; to 1 if SEV is enabled.
- cmp byte[WORK_AREA_GUEST_TYPE], 1
- jne SevEsDisabled
-
- ; During CheckSevFeatures, the SEV_ES_WORK_AREA was set to 1 if
- ; SEV-ES is enabled.
- cmp byte[SEV_ES_WORK_AREA], 1
- jne SevEsDisabled
-
- mov eax, 1
-
-SevEsDisabled:
- OneTimeCallRet IsSevEsEnabled
-
; Start of #VC exception handling routines
;

diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
index f688909f1c7d..0e8ba4dde534 100644
--- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
+++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
@@ -46,16 +46,13 @@ SetCr3ForPageTables64:
; work area when detected.
mov byte[WORK_AREA_GUEST_TYPE], 0

+ ; Check whether the SEV is active and populate the SevEsWorkArea
OneTimeCall CheckSevFeatures
- xor edx, edx
- test eax, eax
- jz SevNotActive

- ; If SEV is enabled, C-bit is always above 31
- sub eax, 32
- bts edx, eax
-
-SevNotActive:
+ ; If SEV is enabled, the C-bit position is always above 31.
+ ; The mask will be saved in the EDX and applied during the
+ ; the page table build below.
+ OneTimeCall GetSevCBitMaskAbove31

;
; For OVMF, build some initial page tables at
@@ -105,44 +102,8 @@ pageTableEntriesLoop:
mov [(ecx * 8 + PT_ADDR (0x2000 - 8)) + 4], edx
loop pageTableEntriesLoop

- OneTimeCall IsSevEsEnabled
- test eax, eax
- jz SetCr3
-
- ;
- ; The initial GHCB will live at GHCB_BASE and needs to be un-encrypted.
- ; This requires the 2MB page for this range be broken down into 512 4KB
- ; pages. All will be marked encrypted, except for the GHCB.
- ;
- mov ecx, (GHCB_BASE >> 21)
- mov eax, GHCB_PT_ADDR + PAGE_PDP_ATTR
- mov [ecx * 8 + PT_ADDR (0x2000)], eax
-
- ;
- ; Page Table Entries (512 * 4KB entries => 2MB)
- ;
- mov ecx, 512
-pageTableEntries4kLoop:
- mov eax, ecx
- dec eax
- shl eax, 12
- add eax, GHCB_BASE & 0xFFE0_0000
- add eax, PAGE_4K_PDE_ATTR
- mov [ecx * 8 + GHCB_PT_ADDR - 8], eax
- mov [(ecx * 8 + GHCB_PT_ADDR - 8) + 4], edx
- loop pageTableEntries4kLoop
-
- ;
- ; Clear the encryption bit from the GHCB entry
- ;
- mov ecx, (GHCB_BASE & 0x1F_FFFF) >> 12
- mov [ecx * 8 + GHCB_PT_ADDR + 4], strict dword 0
-
- mov ecx, GHCB_SIZE / 4
- xor eax, eax
-clearGhcbMemoryLoop:
- mov dword[ecx * 4 + GHCB_BASE - 4], eax
- loop clearGhcbMemoryLoop
+ ; Clear the C-bit from the GHCB page if the SEV-ES is enabled.
+ OneTimeCall SevClearPageEncMaskFromGHCBPage

SetCr3:
;
Re: [PATCH 2/3] OvmfPkg/ResetVector: update SEV support to use new work area format

Lendacky, Thomas
 

On 8/4/21 3:20 PM, Brijesh Singh wrote:
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3429

Update the SEV support to switch to using the newer work area format.

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Erdem Aktas <erdemaktas@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
OvmfPkg/ResetVector/ResetVector.inf | 1 +
OvmfPkg/Sec/SecMain.inf | 1 +
OvmfPkg/Sec/SecMain.c | 25 ++++++++++++++++++++++-
OvmfPkg/ResetVector/Ia32/AmdSev.asm | 8 ++++++++
OvmfPkg/ResetVector/Ia32/PageTables64.asm | 4 ++++
OvmfPkg/ResetVector/ResetVector.nasmb | 1 +
6 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/ResetVector/ResetVector.inf b/OvmfPkg/ResetVector/ResetVector.inf
index d028c92d8cfa..6ec9cca40c3a 100644
--- a/OvmfPkg/ResetVector/ResetVector.inf
+++ b/OvmfPkg/ResetVector/ResetVector.inf
@@ -34,6 +34,7 @@ [BuildOptions]
*_*_X64_NASMB_FLAGS = -I$(WORKSPACE)/UefiCpuPkg/ResetVector/Vtf0/

[Pcd]
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase
Laszlo was trying to keep things sorted, so you should move this down to
the end of the list.

gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize
diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf
index 7f78dcee2772..82910dcbd5c2 100644
--- a/OvmfPkg/Sec/SecMain.inf
+++ b/OvmfPkg/Sec/SecMain.inf
@@ -56,6 +56,7 @@ [Ppis]
gEfiTemporaryRamSupportPpiGuid # PPI ALWAYS_PRODUCED

[Pcd]
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase
Ditto here, even though the list isn't truly sorted to begin with.

gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize
diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
index 9db67e17b2aa..dda572c7ad7d 100644
--- a/OvmfPkg/Sec/SecMain.c
+++ b/OvmfPkg/Sec/SecMain.c
@@ -807,6 +807,29 @@ SevEsProtocolCheck (
Ghcb->GhcbUsage = GHCB_STANDARD_USAGE;
}

+/**
+ Determine if the SEV is active.
+
+ During the early booting, GuestType is set in the work area. Verify that it
+ is an SEV guest.
+
+ @retval TRUE SEV is enabled
+ @retval FALSE SEV is not enabled
+
+**/
+STATIC
+BOOLEAN
+IsSevGuest (
+ VOID
+ )
+{
+ OVMF_WORK_AREA *WorkArea;
+
+ WorkArea = (OVMF_WORK_AREA *) FixedPcdGet32 (PcdOvmfWorkAreaBase);
+
+ return ((WorkArea != NULL) && (WorkArea->GuestType == GUEST_TYPE_AMD_SEV));
+}
+
/**
Determine if SEV-ES is active.

@@ -828,7 +851,7 @@ SevEsIsEnabled (

SevEsWorkArea = (SEC_SEV_ES_WORK_AREA *) FixedPcdGet32 (PcdSevEsWorkAreaBase);

- return ((SevEsWorkArea != NULL) && (SevEsWorkArea->SevEsEnabled != 0));
+ return (((IsSevGuest()) && SevEsWorkArea != NULL) && (SevEsWorkArea->SevEsEnabled != 0));
The IsSevGuest() function checks for a NULL work area, so there's no need
to check for SevEsWorkArea being non-NULL now. I think it would read
better, though, to do:

if (!IsSevGuest ()) {
return FALSE;
}

SevEsWorkArea = ...

return (SevEsWorkArea->SevEsEnabled != 0);

}

VOID
diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
index aa95d06eaddb..87d81b01e263 100644
--- a/OvmfPkg/ResetVector/Ia32/AmdSev.asm
+++ b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
@@ -171,6 +171,9 @@ CheckSevFeatures:
bt eax, 0
jnc NoSev

+ ; Set the work area header to indicate that the SEV is enabled
s/the SEV/SEV/

+ mov byte[WORK_AREA_GUEST_TYPE], 1
The "1" should probably be defined in ResetVector.nasmb as a %define.

+
; Check for SEV-ES memory encryption feature:
; CPUID Fn8000_001F[EAX] - Bit 3
; CPUID raises a #VC exception if running as an SEV-ES guest
@@ -257,6 +260,11 @@ SevExit:
IsSevEsEnabled:
xor eax, eax

+ ; During CheckSevFeatures, the WORK_AREA_GUEST_TYPE is set
+ ; to 1 if SEV is enabled.
+ cmp byte[WORK_AREA_GUEST_TYPE], 1
+ jne SevEsDisabled
+
; During CheckSevFeatures, the SEV_ES_WORK_AREA was set to 1 if
; SEV-ES is enabled.
cmp byte[SEV_ES_WORK_AREA], 1
diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
index eacdb69ddb9f..f688909f1c7d 100644
--- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
+++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
@@ -42,6 +42,10 @@ BITS 32
;
SetCr3ForPageTables64:

+ ; Clear the WorkArea header. The SEV probe routines will populate the
How about:
; Initialize the WorkArea header to indicate a legacy guest. The ...

+ ; work area when detected.
+ mov byte[WORK_AREA_GUEST_TYPE], 0
And then use a %define here for the '0'

+
OneTimeCall CheckSevFeatures
xor edx, edx
test eax, eax
diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb b/OvmfPkg/ResetVector/ResetVector.nasmb
index acec46a32450..d1d800c56745 100644
--- a/OvmfPkg/ResetVector/ResetVector.nasmb
+++ b/OvmfPkg/ResetVector/ResetVector.nasmb
@@ -72,6 +72,7 @@
%define GHCB_PT_ADDR (FixedPcdGet32 (PcdOvmfSecGhcbPageTableBase))
%define GHCB_BASE (FixedPcdGet32 (PcdOvmfSecGhcbBase))
%define GHCB_SIZE (FixedPcdGet32 (PcdOvmfSecGhcbSize))
+ %define WORK_AREA_GUEST_TYPE (FixedPcdGet32 (PcdOvmfWorkAreaBase))
Create %defines for each of the defined enum types from the first patch.

Thanks,
Tom

%define SEV_ES_WORK_AREA (FixedPcdGet32 (PcdSevEsWorkAreaBase))
%define SEV_ES_WORK_AREA_RDRAND (FixedPcdGet32 (PcdSevEsWorkAreaBase) + 8)
%define SEV_ES_WORK_AREA_ENC_MASK (FixedPcdGet32 (PcdSevEsWorkAreaBase) + 16)
[PATCH v2 0/4] Ovmf: Disable the TPM2 platform hierarchy

Stefan Berger <stefanb@...>
 

This series imports code from the edk2-platforms project related to
changing the password of the TPM2 platform hierarchy and uses it to
disable the TPM2 platform hierarchy in Ovmf. It addresses the Ovmf
aspects of the following bugs:

https://bugzilla.tianocore.org/show_bug.cgi?id=3510
https://bugzilla.tianocore.org/show_bug.cgi?id=3499

Regards,
Stefan

Stefan Berger (4):
OvmfPkg/TPM: Import PeiDxeTpmPlatformHierarchyLib.c from
edk2-platforms
OvmfPkg/TPM: Add a NULL implementation of TpmPlatformHierarchyLib
OvmfPkg: Reference new TPM classes in the build system for compilation
OvmfPkg: Disable the TPM2 platform hierarchy

OvmfPkg/AmdSev/AmdSevX64.dsc | 3 +
.../Include/Library/TpmPlatformHierarchyLib.h | 27 +++
.../PeiDxeTpmPlatformHierarchyLib.c | 210 ++++++++++++++++++
.../PeiDxeTpmPlatformHierarchyLib.inf | 40 ++++
.../PeiDxeTpmPlatformHierarchyLib.c | 19 ++
.../PeiDxeTpmPlatformHierarchyLib.inf | 31 +++
.../PlatformBootManagerLib/BdsPlatform.c | 6 +
.../PlatformBootManagerLib.inf | 1 +
.../PlatformBootManagerLibBhyve/BdsPlatform.c | 6 +
.../PlatformBootManagerLibGrub/BdsPlatform.c | 6 +
OvmfPkg/OvmfPkgIa32.dsc | 3 +
OvmfPkg/OvmfPkgIa32X64.dsc | 3 +
OvmfPkg/OvmfPkgX64.dsc | 3 +
13 files changed, 358 insertions(+)
create mode 100644 OvmfPkg/Include/Library/TpmPlatformHierarchyLib.h
create mode 100644 OvmfPkg/Library/PeiDxeTpmPlatformHierarchyLib/PeiDxeTpmPlatformHierarchyLib.c
create mode 100644 OvmfPkg/Library/PeiDxeTpmPlatformHierarchyLib/PeiDxeTpmPlatformHierarchyLib.inf
create mode 100644 OvmfPkg/Library/PeiDxeTpmPlatformHierarchyLibNull/PeiDxeTpmPlatformHierarchyLib.c
create mode 100644 OvmfPkg/Library/PeiDxeTpmPlatformHierarchyLibNull/PeiDxeTpmPlatformHierarchyLib.inf

--
2.31.1
12001 - 12020 of 90922