Re: [edk2-devel] [PATCH edk2-platforms v5 4/4] SbsaQemu: disable XHCI in DSDT if not present
Couple of minor comments on this patch, rest of set good to go:
On Wed, Oct 18, 2023 at 13:55:41 +0200, Marcin Juszkiewicz wrote:
> We need platform version to be at least 0.3 to have XHCI
> in virtual hardware. On older platforms there is non-working
> EHCI which we ignore.
>
> Set DSDT node to be disabled so operating system will not try
> to initialize not-existing hardware.
>
> Signed-off-by: Marcin Juszkiewicz
> ---
> .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf | 4 ++
> .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c| 68
> +++-
> Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl| 3 +-
> 3 files changed, 73 insertions(+), 2 deletions(-)
>
> diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> index fd849ca1594b..464119de1457 100644
> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> @@ -10,6 +10,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -682,6 +683,71 @@ AddGtdtTable (
>return Status;
> }
>
> +/*
> + * A function to disable XHCI node on Platform Version lower than 0.3
> + */
STATIC
> +EFI_STATUS
> +DisableXhciOnOlderPlatVer (
> + VOID
> + )
> +{
> + EFI_STATUS Status;
> + EFI_ACPI_SDT_PROTOCOL*AcpiSdtProtocol;
> + EFI_ACPI_DESCRIPTION_HEADER *Table;
> + UINTNTableKey;
> + UINTNTableIndex;
> + EFI_ACPI_HANDLE TableHandle;
> +
> + Status = EFI_SUCCESS;
> +
> + if ( PLATFORM_VERSION_LESS_THAN (0, 3)) {
> +DEBUG ((DEBUG_ERROR, "Platform Version < 0.3 - disabling XHCI\n"));
> +Status = gBS->LocateProtocol (
> + ,
> + NULL,
> + (VOID **)
> + );
> +if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "Unable to locate ACPI table protocol\n"));
> + return Status;
> +}
> +
> +Status = AcpiLocateTableBySignature (
> + AcpiSdtProtocol,
> +
> EFI_ACPI_6_3_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE,
> + ,
> + ,
> +
> + );
> +if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "ACPI DSDT table not found!\n"));
> + ASSERT_EFI_ERROR (Status);
> + return Status;
> +}
> +
> +Status = AcpiSdtProtocol->OpenSdt (TableKey, );
> +if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + AcpiSdtProtocol->Close (TableHandle);
> + return Status;
> +}
> +
> +Status = AcpiAmlObjectUpdateInteger (AcpiSdtProtocol, TableHandle,
> "\\_SB.USB0.XHCI", 0x0);
> +if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "Failed to disable XHCI!\n"));
> + ASSERT_EFI_ERROR (Status);
> + AcpiSdtProtocol->Close (TableHandle);
> + return Status;
> +}
> +
> +AcpiSdtProtocol->Close (TableHandle);
> +AcpiUpdateChecksum ((UINT8 *)Table, Table->Length);
> + }
> +
> + return Status;
> +}
> +
> +
> EFI_STATUS
> EFIAPI
> InitializeSbsaQemuAcpiDxe (
> @@ -738,5 +804,5 @@ InitializeSbsaQemuAcpiDxe (
> DEBUG ((DEBUG_ERROR, "Failed to add GTDT table\n"));
>}
>
> - return EFI_SUCCESS;
> + return DisableXhciOnOlderPlatVer ();
Replacing the normal exit path like this unnecessarily ties this call
to the end of the function.
Please add another boring "if (EFI_ERROR ..." stanza instead.
/
Leif
> }
> diff --git a/Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl
> b/Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl
> index 543b5782580a..ba3eefc975a5 100644
> --- a/Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl
> +++ b/Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl
> @@ -73,8 +73,9 @@ DefinitionBlock ("DsdtTable.aml", "DSDT",
> Name (_HID, "PNP0D10") // _HID: Hardware ID
> Name (_UID, 0x00)// _UID: Unique ID
> Name (_CCA, 0x01)// _CCA: Cache Coherency Attribute
> +Name (XHCI, 0xF)// will be set using AcpiLib
> Method (_STA) {
> - Return (0xF)
> + Return (XHCI)
> }
> Method (_CRS, 0x0, Serialized) {
> Name (RBUF, ResourceTemplate() {
>
> --
> 2.41.0
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110129): https://edk2.groups.io/g/devel/message/110129
Mute This Topic: https://groups.io/mt/102037246/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH edk2-platforms v5 4/4] SbsaQemu: disable XHCI in DSDT if not present
We need platform version to be at least 0.3 to have XHCI
in virtual hardware. On older platforms there is non-working
EHCI which we ignore.
Set DSDT node to be disabled so operating system will not try
to initialize not-existing hardware.
Signed-off-by: Marcin Juszkiewicz
---
.../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf | 4 ++
.../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c| 68 +++-
Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl| 3 +-
3 files changed, 73 insertions(+), 2 deletions(-)
diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
index 7c7e08e0fd3a..291743b19115 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
@@ -29,6 +29,7 @@ [Packages]
Silicon/Qemu/SbsaQemu/SbsaQemu.dec
[LibraryClasses]
+ AcpiLib
ArmLib
BaseMemoryLib
BaseLib
@@ -49,6 +50,8 @@ [Pcd]
gArmTokenSpaceGuid.PcdGicDistributorBase
gArmTokenSpaceGuid.PcdGicRedistributorsBase
gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdGicItsBase
+ gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformVersionMajor
+ gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformVersionMinor
gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdSmmuBase
[Depex]
@@ -58,6 +61,7 @@ [Guids]
gEdkiiPlatformHasAcpiGuid
[Protocols]
+ gEfiAcpiSdtProtocolGuid
gEfiAcpiTableProtocolGuid ## CONSUMES
[FixedPcd]
diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
index fd849ca1594b..464119de1457 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -682,6 +683,71 @@ AddGtdtTable (
return Status;
}
+/*
+ * A function to disable XHCI node on Platform Version lower than 0.3
+ */
+EFI_STATUS
+DisableXhciOnOlderPlatVer (
+ VOID
+ )
+{
+ EFI_STATUS Status;
+ EFI_ACPI_SDT_PROTOCOL*AcpiSdtProtocol;
+ EFI_ACPI_DESCRIPTION_HEADER *Table;
+ UINTNTableKey;
+ UINTNTableIndex;
+ EFI_ACPI_HANDLE TableHandle;
+
+ Status = EFI_SUCCESS;
+
+ if ( PLATFORM_VERSION_LESS_THAN (0, 3)) {
+DEBUG ((DEBUG_ERROR, "Platform Version < 0.3 - disabling XHCI\n"));
+Status = gBS->LocateProtocol (
+ ,
+ NULL,
+ (VOID **)
+ );
+if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Unable to locate ACPI table protocol\n"));
+ return Status;
+}
+
+Status = AcpiLocateTableBySignature (
+ AcpiSdtProtocol,
+
EFI_ACPI_6_3_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE,
+ ,
+ ,
+
+ );
+if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "ACPI DSDT table not found!\n"));
+ ASSERT_EFI_ERROR (Status);
+ return Status;
+}
+
+Status = AcpiSdtProtocol->OpenSdt (TableKey, );
+if (EFI_ERROR (Status)) {
+ ASSERT_EFI_ERROR (Status);
+ AcpiSdtProtocol->Close (TableHandle);
+ return Status;
+}
+
+Status = AcpiAmlObjectUpdateInteger (AcpiSdtProtocol, TableHandle,
"\\_SB.USB0.XHCI", 0x0);
+if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Failed to disable XHCI!\n"));
+ ASSERT_EFI_ERROR (Status);
+ AcpiSdtProtocol->Close (TableHandle);
+ return Status;
+}
+
+AcpiSdtProtocol->Close (TableHandle);
+AcpiUpdateChecksum ((UINT8 *)Table, Table->Length);
+ }
+
+ return Status;
+}
+
+
EFI_STATUS
EFIAPI
InitializeSbsaQemuAcpiDxe (
@@ -738,5 +804,5 @@ InitializeSbsaQemuAcpiDxe (
DEBUG ((DEBUG_ERROR, "Failed to add GTDT table\n"));
}
- return EFI_SUCCESS;
+ return DisableXhciOnOlderPlatVer ();
}
diff --git a/Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl
b/Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl
index 543b5782580a..ba3eefc975a5 100644
--- a/Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl
+++ b/Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl
@@ -73,8 +73,9 @@ DefinitionBlock ("DsdtTable.aml", "DSDT",
Name (_HID, "PNP0D10") // _HID: Hardware ID
Name (_UID, 0x00)// _UID: Unique ID
Name (_CCA, 0x01)// _CCA: Cache Coherency Attribute
+Name (XHCI, 0xF)// will be set using AcpiLib
Method (_STA) {
- Return (0xF)
+ Return (XHCI)
}
Method (_CRS, 0x0, Serialized) {
Name (RBUF,
