EL3 firmware may implement PSCI interface on aarch64 platforms,
including qemu-tcg-aarch64. However, EL3 firmware does not usually own
pulling and deploying ACPI tables from qemu fw_cfg. Thus the only way
EL3 can advertise PSCI on qemu is in FDT. One such EL3 fw is ARM trusted
firmware. Qemu itself also won't advertise PSCI in either FDT or ACPI if
EL3 firmware is present.

PSCI can be advertised in both FDT and ACPI, and Hyper-V/NT kernel
expect to see all information published in ACPI. To better support
running Hyper-V/NT on qemu-tcg-aarch64 with EDK2 as UEFI implementation
and ARM trusted firmware as EL3 PSCI implementation we can patch in PSCI
bits in ACPI FADT when pulling tables from fw_cfg if PSCI node is
advertised in FDT. EDK2 owns ACPI table publishing and is also aware of
FDT on arm, so it is ideally poised to handle this.

This change illustrates how it could potentially be done. I am looking
for comments on overall validity of the idea to patch FADT and whether
or not this particular approach of handling it in AcpiPlatformDxe is the
way to do it or maybe it is better to handle it via
gQemuAcpiTableNotifyProtocolGuid somehow.

Signed-off-by: Evgeny Iakovlev <eiakovlev@...>
---
OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf | 4 +-
OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c | 108 ++++++++++++++++++++
2 files changed, 111 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf b/OvmfPkg/AcpiPl=
atformDxe/AcpiPlatformDxe.inf
index 8939dde425..a011a43743 100644
--- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
+++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
@@ -30,6 +30,7 @@
QemuFwCfgAcpi.c
=20
[Packages]
+ EmbeddedPkg/EmbeddedPkg.dec
MdeModulePkg/MdeModulePkg.dec
MdePkg/MdePkg.dec
OvmfPkg/OvmfPkg.dec
@@ -50,6 +51,7 @@
[Protocols]
gEfiAcpiTableProtocolGuid # PROTOCOL ALWAYS_CONSUM=
ED
gEfiPciIoProtocolGuid # PROTOCOL SOMETIMES_CON=
SUMED
+ gFdtClientProtocolGuid # PROTOCOL SOMETIMES_CON=
SUMED
gQemuAcpiTableNotifyProtocolGuid # PROTOCOL PRODUCES
=20
[Guids]
@@ -64,4 +66,4 @@
gEfiMdePkgTokenSpaceGuid.PcdConfidentialComputingGuestAttr
=20
[Depex]
- gEfiAcpiTableProtocolGuid
+ gEfiAcpiTableProtocolGuid AND gFdtClientProtocolGuid
diff --git a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c b/OvmfPkg/AcpiPlatfo=
rmDxe/QemuFwCfgAcpi.c
index c8dee17c13..31d8665d2d 100644
--- a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
+++ b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
@@ -9,6 +9,7 @@
**/
=20
#include <IndustryStandard/Acpi.h> // EFI_ACPI_DESCRIPTION_HE=
ADER
+#include <IndustryStandard/Acpi51.h> // EFI_ACPI_5_1_FIXED_ACPI=
_DESCRIPTION_TABLE
#include <IndustryStandard/QemuLoader.h> // QEMU_LOADER_FNAME_SIZE
#include <Library/BaseLib.h> // AsciiStrCmp()
#include <Library/BaseMemoryLib.h> // CopyMem()
@@ -20,6 +21,7 @@
#include <Library/UefiBootServicesTableLib.h> // gBS
=20
#include <Protocol/QemuAcpiTableNotify.h>
+#include <Protocol/FdtClient.h>
#include "AcpiPlatform.h"
EFI_HANDLE mQemuAcpiHandle =3D NULL;
QEMU_ACPI_TABLE_NOTIFY_PROTOCOL mAcpiNotifyProtocol;
@@ -815,6 +817,92 @@ UndoCmdWritePointer (
));
}
=20
+/**
+ Locate a PSCI node in DTB published by EL3 firmware that implemented i=
t
+ and use information in it to patch ACPI FADT PSCI bits.
+
+ The reason for it is that EL3 PSCI implementation, which is not EDK2,
+ doesn't own ACPI tables and cannot advertise it there itself, it is us=
ing DTB instead.
+
+ Qemu also won't advertise PSCI if EL3 firmware is present.
+ A real example of that is running ARM trusted firmware in EL3 with PSC=
I enabled.
+
+ @param[in] PonterValue FADT base address
+
+ @param[in] TableSize Size of table in bytes
+
+ @return EFI_SUCCESS if FAST was patched or if patchi=
ng was not needed,
+ error status returned by FDT client protocol=
otherwise.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+PatchFadtPsciBits (
+ IN UINT64 PointerValue,
+ IN UINTN TableSize
+ )
+{
+ EFI_STATUS Status;
+ FDT_CLIENT_PROTOCOL *FdtClient;
+ CONST VOID *Prop;
+ EFI_ACPI_5_1_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
+ UINT8 MajorMinorRev;
+
+ Status =3D gBS->LocateProtocol (
+ &gFdtClientProtocolGuid,
+ NULL,
+ (VOID **)&FdtClient
+ );
+ if (Status =3D=3D EFI_NOT_FOUND) {
+ goto NoPatchingNeeded;
+ } else if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ Status =3D FdtClient->FindCompatibleNodeProperty (
+ FdtClient,
+ "arm,psci-0.2",
+ "method",
+ &Prop,
+ NULL
+ );
+ if (Status =3D=3D EFI_NOT_FOUND) {
+ goto NoPatchingNeeded;
+ } else if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ //
+ // Make sure FADT is large enough for 5.1
+ //
+ if (TableSize < sizeof (*Fadt)) {
+ goto NoPatchingNeeded;
+ }
+
+ Fadt =3D (EFI_ACPI_5_1_FIXED_ACPI_DESCRIPTION_TABLE *)(UINTN)PointerVa=
lue;
+
+ //
+ // Check that FADT revision is at least 5.1 now that it's safe to read=
fields
+ //
+ MajorMinorRev =3D (Fadt->Header.Revision << 4) | (Fadt->MinorVersion &=
0x0F);
+ if (MajorMinorRev < 0x51) {
+ goto NoPatchingNeeded;
+ }
+
+ //
+ // Patch FADT PSCI bits
+ //
+ Fadt->ArmBootArch =3D EFI_ACPI_5_1_ARM_PSCI_COMPLIANT;
+ if (AsciiStrnCmp (Prop, "hvc", 3) =3D=3D 0) {
+ Fadt->ArmBootArch |=3D EFI_ACPI_5_1_ARM_PSCI_USE_HVC;
+ }
+
+ return EFI_SUCCESS;
+
+NoPatchingNeeded:
+ return EFI_SUCCESS;
+}
+
//
// We'll be saving the keys of installed tables so that we can roll them=
back
// in case of failure. 128 tables should be enough for anyone (TM).
@@ -901,6 +989,7 @@ Process2ndPassCmdAddPointer (
UINT64 PointerValue;
UINTN Blob2Remaining;
UINTN TableSize;
+ UINT32 Signature;
CONST EFI_ACPI_1_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *Facs;
CONST EFI_ACPI_DESCRIPTION_HEADER *Header;
EFI_STATUS Status;
@@ -960,6 +1049,7 @@ Process2ndPassCmdAddPointer (
));
=20
TableSize =3D 0;
+ Signature =3D 0;
=20
//
// To make our job simple, the FACS has a custom header. Sigh.
@@ -979,6 +1069,7 @@ Process2ndPassCmdAddPointer (
Facs->Length
));
TableSize =3D Facs->Length;
+ Signature =3D Facs->Signature;
}
}
=20
@@ -1004,6 +1095,7 @@ Process2ndPassCmdAddPointer (
Header->Length
));
TableSize =3D Header->Length;
+ Signature =3D Header->Signature;
=20
//
// Skip RSDT and XSDT because those are handled by
@@ -1035,6 +1127,22 @@ Process2ndPassCmdAddPointer (
goto RollbackSeenPointer;
}
=20
+ //
+ // For FADT also patch PSCI flag if DTB exposes it.
+ //
+ if (Signature =3D=3D EFI_ACPI_1_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATU=
RE) {
+ Status =3D PatchFadtPsciBits(PointerValue, TableSize);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: PatchFadtPsciBits(): %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ goto RollbackSeenPointer;
+ }
+ }
+
Status =3D AcpiProtocol->InstallAcpiTable (
AcpiProtocol,
(VOID *)(UINTN)PointerValue,
--=20
2.38.1.vfs.0.0

EL3 firmware may implement PSCI interface on aarch64 platforms,
including qemu-tcg-aarch64. However, EL3 firmware does not usually own
pulling and deploying ACPI tables from qemu fw_cfg. Thus the only way
EL3 can advertise PSCI on qemu is in FDT. One such EL3 fw is ARM trusted
firmware. Qemu itself also won't advertise PSCI in either FDT or ACPI if
EL3 firmware is present.

PSCI can be advertised in both FDT and ACPI, and Hyper-V/NT kernel
expect to see all information published in ACPI. To better support
running Hyper-V/NT on qemu-tcg-aarch64 with EDK2 as UEFI implementation
and ARM trusted firmware as EL3 PSCI implementation we can patch in PSCI
bits in ACPI FADT when pulling tables from fw_cfg if PSCI node is
advertised in FDT. EDK2 owns ACPI table publishing and is also aware of
FDT on arm, so it is ideally poised to handle this.

This change illustrates how it could potentially be done. I am looking
for comments on overall validity of the idea to patch FADT and whether
or not this particular approach of handling it in AcpiPlatformDxe is the
way to do it or maybe it is better to handle it via
gQemuAcpiTableNotifyProtocolGuid somehow.

Signed-off-by: Evgeny Iakovlev <eiakovlev@...>
---
Resending the message because i wasn't subscribed to the rfc group the
last time, sorry.
---
OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf | 4 +-
OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c | 108 ++++++++++++++++++++
2 files changed, 111 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf b/OvmfPkg/AcpiPl=
atformDxe/AcpiPlatformDxe.inf
index 8939dde425..a011a43743 100644
--- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
+++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
@@ -30,6 +30,7 @@
QemuFwCfgAcpi.c
=20
[Packages]
+ EmbeddedPkg/EmbeddedPkg.dec
MdeModulePkg/MdeModulePkg.dec
MdePkg/MdePkg.dec
OvmfPkg/OvmfPkg.dec
@@ -50,6 +51,7 @@
[Protocols]
gEfiAcpiTableProtocolGuid # PROTOCOL ALWAYS_CONSUM=
ED
gEfiPciIoProtocolGuid # PROTOCOL SOMETIMES_CON=
SUMED
+ gFdtClientProtocolGuid # PROTOCOL SOMETIMES_CON=
SUMED
gQemuAcpiTableNotifyProtocolGuid # PROTOCOL PRODUCES
=20
[Guids]
@@ -64,4 +66,4 @@
gEfiMdePkgTokenSpaceGuid.PcdConfidentialComputingGuestAttr
=20
[Depex]
- gEfiAcpiTableProtocolGuid
+ gEfiAcpiTableProtocolGuid AND gFdtClientProtocolGuid
diff --git a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c b/OvmfPkg/AcpiPlatfo=
rmDxe/QemuFwCfgAcpi.c
index c8dee17c13..31d8665d2d 100644
--- a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
+++ b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
@@ -9,6 +9,7 @@
**/
=20
#include <IndustryStandard/Acpi.h> // EFI_ACPI_DESCRIPTION_HE=
ADER
+#include <IndustryStandard/Acpi51.h> // EFI_ACPI_5_1_FIXED_ACPI=
_DESCRIPTION_TABLE
#include <IndustryStandard/QemuLoader.h> // QEMU_LOADER_FNAME_SIZE
#include <Library/BaseLib.h> // AsciiStrCmp()
#include <Library/BaseMemoryLib.h> // CopyMem()
@@ -20,6 +21,7 @@
#include <Library/UefiBootServicesTableLib.h> // gBS
=20
#include <Protocol/QemuAcpiTableNotify.h>
+#include <Protocol/FdtClient.h>
#include "AcpiPlatform.h"
EFI_HANDLE mQemuAcpiHandle =3D NULL;
QEMU_ACPI_TABLE_NOTIFY_PROTOCOL mAcpiNotifyProtocol;
@@ -815,6 +817,92 @@ UndoCmdWritePointer (
));
}
=20
+/**
+ Locate a PSCI node in DTB published by EL3 firmware that implemented i=
t
+ and use information in it to patch ACPI FADT PSCI bits.
+
+ The reason for it is that EL3 PSCI implementation, which is not EDK2,
+ doesn't own ACPI tables and cannot advertise it there itself, it is us=
ing DTB instead.
+
+ Qemu also won't advertise PSCI if EL3 firmware is present.
+ A real example of that is running ARM trusted firmware in EL3 with PSC=
I enabled.
+
+ @param[in] PonterValue FADT base address
+
+ @param[in] TableSize Size of table in bytes
+
+ @return EFI_SUCCESS if FAST was patched or if patchi=
ng was not needed,
+ error status returned by FDT client protocol=
otherwise.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+PatchFadtPsciBits (
+ IN UINT64 PointerValue,
+ IN UINTN TableSize
+ )
+{
+ EFI_STATUS Status;
+ FDT_CLIENT_PROTOCOL *FdtClient;
+ CONST VOID *Prop;
+ EFI_ACPI_5_1_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
+ UINT8 MajorMinorRev;
+
+ Status =3D gBS->LocateProtocol (
+ &gFdtClientProtocolGuid,
+ NULL,
+ (VOID **)&FdtClient
+ );
+ if (Status =3D=3D EFI_NOT_FOUND) {
+ goto NoPatchingNeeded;
+ } else if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ Status =3D FdtClient->FindCompatibleNodeProperty (
+ FdtClient,
+ "arm,psci-0.2",
+ "method",
+ &Prop,
+ NULL
+ );
+ if (Status =3D=3D EFI_NOT_FOUND) {
+ goto NoPatchingNeeded;
+ } else if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ //
+ // Make sure FADT is large enough for 5.1
+ //
+ if (TableSize < sizeof (*Fadt)) {
+ goto NoPatchingNeeded;
+ }
+
+ Fadt =3D (EFI_ACPI_5_1_FIXED_ACPI_DESCRIPTION_TABLE *)(UINTN)PointerVa=
lue;
+
+ //
+ // Check that FADT revision is at least 5.1 now that it's safe to read=
fields
+ //
+ MajorMinorRev =3D (Fadt->Header.Revision << 4) | (Fadt->MinorVersion &=
0x0F);
+ if (MajorMinorRev < 0x51) {
+ goto NoPatchingNeeded;
+ }
+
+ //
+ // Patch FADT PSCI bits
+ //
+ Fadt->ArmBootArch =3D EFI_ACPI_5_1_ARM_PSCI_COMPLIANT;
+ if (AsciiStrnCmp (Prop, "hvc", 3) =3D=3D 0) {
+ Fadt->ArmBootArch |=3D EFI_ACPI_5_1_ARM_PSCI_USE_HVC;
+ }
+
+ return EFI_SUCCESS;
+
+NoPatchingNeeded:
+ return EFI_SUCCESS;
+}
+
//
// We'll be saving the keys of installed tables so that we can roll them=
back
// in case of failure. 128 tables should be enough for anyone (TM).
@@ -901,6 +989,7 @@ Process2ndPassCmdAddPointer (
UINT64 PointerValue;
UINTN Blob2Remaining;
UINTN TableSize;
+ UINT32 Signature;
CONST EFI_ACPI_1_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *Facs;
CONST EFI_ACPI_DESCRIPTION_HEADER *Header;
EFI_STATUS Status;
@@ -960,6 +1049,7 @@ Process2ndPassCmdAddPointer (
));
=20
TableSize =3D 0;
+ Signature =3D 0;
=20
//
// To make our job simple, the FACS has a custom header. Sigh.
@@ -979,6 +1069,7 @@ Process2ndPassCmdAddPointer (
Facs->Length
));
TableSize =3D Facs->Length;
+ Signature =3D Facs->Signature;
}
}
=20
@@ -1004,6 +1095,7 @@ Process2ndPassCmdAddPointer (
Header->Length
));
TableSize =3D Header->Length;
+ Signature =3D Header->Signature;
=20
//
// Skip RSDT and XSDT because those are handled by
@@ -1035,6 +1127,22 @@ Process2ndPassCmdAddPointer (
goto RollbackSeenPointer;
}
=20
+ //
+ // For FADT also patch PSCI flag if DTB exposes it.
+ //
+ if (Signature =3D=3D EFI_ACPI_1_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATU=
RE) {
+ Status =3D PatchFadtPsciBits(PointerValue, TableSize);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: PatchFadtPsciBits(): %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ goto RollbackSeenPointer;
+ }
+ }
+
Status =3D AcpiProtocol->InstallAcpiTable (
AcpiProtocol,
(VOID *)(UINTN)PointerValue,
--=20
2.38.1.vfs.0.0