[edk2-devel] [PATCH edk2-platforms v6 4/4] SbsaQemu: disable XHCI in DSDT if not present

Marcin Juszkiewicz posted 4 patches 2 years, 3 months ago
[edk2-devel] [PATCH edk2-platforms v6 4/4] SbsaQemu: disable XHCI in DSDT if not present
Posted by Marcin Juszkiewicz 2 years, 3 months ago
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 <marcin.juszkiewicz@linaro.org>
---
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf      |  4 ++
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c        | 74 +++++++++++++++++++-
 Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl            |  3 +-
 3 files changed, 79 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..523d9035e0c1 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
@@ -10,6 +10,7 @@
 #include <IndustryStandard/AcpiAml.h>
 #include <IndustryStandard/IoRemappingTable.h>
 #include <IndustryStandard/SbsaQemuAcpi.h>
+#include <IndustryStandard/SbsaQemuPlatformVersion.h>
 #include <Library/AcpiLib.h>
 #include <Library/ArmLib.h>
 #include <Library/BaseMemoryLib.h>
@@ -682,6 +683,72 @@ 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;
+  UINTN                        TableKey;
+  UINTN                        TableIndex;
+  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 (
+                                  &gEfiAcpiSdtProtocolGuid,
+                                  NULL,
+                                  (VOID **)&AcpiSdtProtocol
+                                  );
+    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,
+                                         &TableIndex,
+                                         &Table,
+                                         &TableKey
+                                         );
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_ERROR, "ACPI DSDT table not found!\n"));
+      ASSERT_EFI_ERROR (Status);
+      return Status;
+    }
+
+    Status = AcpiSdtProtocol->OpenSdt (TableKey, &TableHandle);
+    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 +805,10 @@ InitializeSbsaQemuAcpiDxe (
     DEBUG ((DEBUG_ERROR, "Failed to add GTDT table\n"));
   }
 
-  return EFI_SUCCESS;
+  Status = DisableXhciOnOlderPlatVer ();
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Failed to handle XHCI enablement\n"));
+  }
+
+  return Status;
 }
diff --git a/Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl b/Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl
index 6661bc8195ee..b55ad6c5cc07 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 (#110137): https://edk2.groups.io/g/devel/message/110137
Mute This Topic: https://groups.io/mt/102205083/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH edk2-platforms v6 4/4] SbsaQemu: disable XHCI in DSDT if not present
Posted by Leif Lindholm 2 years, 3 months ago
On Thu, Oct 26, 2023 at 19:33:52 +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 <marcin.juszkiewicz@linaro.org>
> ---
>  .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf      |  4 ++
>  .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c        | 74 +++++++++++++++++++-
>  Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl            |  3 +-
>  3 files changed, 79 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..523d9035e0c1 100644
> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> @@ -10,6 +10,7 @@
>  #include <IndustryStandard/AcpiAml.h>
>  #include <IndustryStandard/IoRemappingTable.h>
>  #include <IndustryStandard/SbsaQemuAcpi.h>
> +#include <IndustryStandard/SbsaQemuPlatformVersion.h>
>  #include <Library/AcpiLib.h>
>  #include <Library/ArmLib.h>
>  #include <Library/BaseMemoryLib.h>
> @@ -682,6 +683,72 @@ 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;
> +  UINTN                        TableKey;
> +  UINTN                        TableIndex;
> +  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 (
> +                                  &gEfiAcpiSdtProtocolGuid,
> +                                  NULL,
> +                                  (VOID **)&AcpiSdtProtocol
> +                                  );
> +    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,
> +                                         &TableIndex,
> +                                         &Table,
> +                                         &TableKey
> +                                         );
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((DEBUG_ERROR, "ACPI DSDT table not found!\n"));
> +      ASSERT_EFI_ERROR (Status);
> +      return Status;
> +    }
> +
> +    Status = AcpiSdtProtocol->OpenSdt (TableKey, &TableHandle);
> +    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 +805,10 @@ InitializeSbsaQemuAcpiDxe (
>      DEBUG ((DEBUG_ERROR, "Failed to add GTDT table\n"));
>    }
>  
> -  return EFI_SUCCESS;
> +  Status = DisableXhciOnOlderPlatVer ();
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "Failed to handle XHCI enablement\n"));
> +  }
> +
> +  return Status;

Right, this isn't what I asked for though.
There is nothing valid about returning a bad status here - it is the
entry point of the module. If we wanted some sort of error handling,
we would need to plumb that in manually.

Anyway, no point in you spinning another version, I'll change this
back to the original return statement.

For the series:
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
Pushed as 74b9eacfd453..d61836283a4c.

Thanks!


>  }
> diff --git a/Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl b/Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl
> index 6661bc8195ee..b55ad6c5cc07 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 (#110138): https://edk2.groups.io/g/devel/message/110138
Mute This Topic: https://groups.io/mt/102205083/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-