Re: [edk2-devel] [PATCH edk2-platforms v3 4/4] SbsaQemu: disable XHCI in DSDT if not present
W dniu 18.10.2023 o 10:47, Ard Biesheuvel pisze: Nit: EDK2 Coding Style says that you need a space before (. Ah, right. forgot to crucify the source. Is it necessary to handle the result of Status? EDK2 is full of handling Status on touching ACPI tables. So I followed. Can you just do 'return DisableXhciOnOlderPlatVer();' instead? Done. Sent v4. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#109724): https://edk2.groups.io/g/devel/message/109724 Mute This Topic: https://groups.io/mt/102017316/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH edk2-platforms v3 4/4] SbsaQemu: disable XHCI in DSDT if not present
On Wed, 18 Oct 2023 at 07:56, Marcin Juszkiewicz wrote: > > W dniu 18.10.2023 o 05:28, Nhi Pham pisze: > > Hi Marcin, > > > > There is a nitpicking below. > > > > Other than, it looks good to me. > > > > Acked-by: Nhi Pham > > > >> a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c > >> b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c > >> index fd849ca1594b..cf6e534ca3a0 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,63 @@ AddGtdtTable ( > >> return Status; > >> } > >> +EFI_STATUS > >> +DisableXhciOnOlderPlatVer ( > >> + VOID > >> + ) > >> +{ > >> + EFI_STATUSStatus; > >> + 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; > >> +} > >> + > >> +TableIndex = 0; > >> +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; > >> +} > >> + > >> +AcpiAmlObjectUpdateInteger (AcpiSdtProtocol, TableHandle, > >> "\\_SB.USB0.XHCI", 0x0); > >> + > >> +AcpiSdtProtocol->Close (TableHandle); > >> +AcpiUpdateChecksum ((UINT8 *)Table, Table->Length); > >> + } > >> + > >> + return Status; > >> +} > >> + > >> + > >> EFI_STATUS > >> EFIAPI > >> InitializeSbsaQemuAcpiDxe ( > >> @@ -738,5 +796,7 @@ InitializeSbsaQemuAcpiDxe ( > >> DEBUG ((DEBUG_ERROR, "Failed to add GTDT table\n")); > >> } > >> + Status = DisableXhciOnOlderPlatVer(); > > > > Nit: EDK2 Coding Style says that you need a space before (. > > Ah, right. forgot to crucify the source. > > > Is it necessary to handle the result of Status? > > EDK2 is full of handling Status on touching ACPI tables. So I followed. > Can you just do 'return DisableXhciOnOlderPlatVer();' instead? -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#109715): https://edk2.groups.io/g/devel/message/109715 Mute This Topic: https://groups.io/mt/102017316/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH edk2-platforms v3 4/4] SbsaQemu: disable XHCI in DSDT if not present
W dniu 18.10.2023 o 05:28, Nhi Pham pisze: Hi Marcin, There is a nitpicking below. Other than, it looks good to me. Acked-by: Nhi Pham a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c index fd849ca1594b..cf6e534ca3a0 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,63 @@ AddGtdtTable ( return Status; } +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; + } + + TableIndex = 0; + 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; + } + + AcpiAmlObjectUpdateInteger (AcpiSdtProtocol, TableHandle, "\\_SB.USB0.XHCI", 0x0); + + AcpiSdtProtocol->Close (TableHandle); + AcpiUpdateChecksum ((UINT8 *)Table, Table->Length); + } + + return Status; +} + + EFI_STATUS EFIAPI InitializeSbsaQemuAcpiDxe ( @@ -738,5 +796,7 @@ InitializeSbsaQemuAcpiDxe ( DEBUG ((DEBUG_ERROR, "Failed to add GTDT table\n")); } + Status = DisableXhciOnOlderPlatVer(); Nit: EDK2 Coding Style says that you need a space before (. Ah, right. forgot to crucify the source. Is it necessary to handle the result of Status? EDK2 is full of handling Status on touching ACPI tables. So I followed. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#109711): https://edk2.groups.io/g/devel/message/109711 Mute This Topic: https://groups.io/mt/102017316/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH edk2-platforms v3 4/4] SbsaQemu: disable XHCI in DSDT if not present
Hi Marcin, There is a nitpicking below. Other than, it looks good to me. Acked-by: Nhi Pham Regards, Nhi On 10/17/2023 8:23 PM, 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| 60 Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl| 3 +- 3 files changed, 66 insertions(+), 1 deletion(-) diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf index 7c7e08e0fd3a..d5ded892d6ea 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 @@ -50,6 +51,8 @@ [Pcd] gArmTokenSpaceGuid.PcdGicRedistributorsBase gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdGicItsBase gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdSmmuBase + gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformVersionMajor + gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformVersionMinor [Depex] gEfiAcpiTableProtocolGuid ## CONSUMES @@ -59,6 +62,7 @@ [Guids] [Protocols] gEfiAcpiTableProtocolGuid ## CONSUMES + gEfiAcpiSdtProtocolGuid [FixedPcd] gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorId diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c index fd849ca1594b..cf6e534ca3a0 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,63 @@ AddGtdtTable ( return Status; } +EFI_STATUS +DisableXhciOnOlderPlatVer ( + VOID + ) +{ + EFI_STATUSStatus; + 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; +} + +TableIndex = 0; +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; +} + +AcpiAmlObjectUpdateInteger (AcpiSdtProtocol, TableHandle, "\\_SB.USB0.XHCI", 0x0); + +AcpiSdtProtocol->Close (TableHandle); +AcpiUpdateChecksum ((UINT8 *)Table, Table->Length); + } + + return Status; +} + + EFI_STATUS EFIAPI InitializeSbsaQemuAcpiDxe ( @@ -738,5 +796,7 @@ InitializeSbsaQemuAcpiDxe ( DEBUG ((DEBUG_ERROR, "Failed to add GTDT table\n")); } + Status = DisableXhciOnOlderPlatVer(); Nit: EDK2 Coding Style says that you need a space before (. Is it necessary to handle the result of Status? + return EFI_SUCCESS; } 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
[edk2-devel] [PATCH edk2-platforms v3 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| 60 Silicon/Qemu/SbsaQemu/AcpiTables/Dsdt.asl| 3 +- 3 files changed, 66 insertions(+), 1 deletion(-) diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf index 7c7e08e0fd3a..d5ded892d6ea 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 @@ -50,6 +51,8 @@ [Pcd] gArmTokenSpaceGuid.PcdGicRedistributorsBase gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdGicItsBase gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdSmmuBase + gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformVersionMajor + gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformVersionMinor [Depex] gEfiAcpiTableProtocolGuid ## CONSUMES @@ -59,6 +62,7 @@ [Guids] [Protocols] gEfiAcpiTableProtocolGuid ## CONSUMES + gEfiAcpiSdtProtocolGuid [FixedPcd] gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorId diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c index fd849ca1594b..cf6e534ca3a0 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,63 @@ AddGtdtTable ( return Status; } +EFI_STATUS +DisableXhciOnOlderPlatVer ( + VOID + ) +{ + EFI_STATUSStatus; + 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; +} + +TableIndex = 0; +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; +} + +AcpiAmlObjectUpdateInteger (AcpiSdtProtocol, TableHandle, "\\_SB.USB0.XHCI", 0x0); + +AcpiSdtProtocol->Close (TableHandle); +AcpiUpdateChecksum ((UINT8 *)Table, Table->Length); + } + + return Status; +} + + EFI_STATUS EFIAPI InitializeSbsaQemuAcpiDxe ( @@ -738,5 +796,7 @@ InitializeSbsaQemuAcpiDxe ( DEBUG ((DEBUG_ERROR, "Failed to add GTDT table\n")); } + Status = DisableXhciOnOlderPlatVer(); + return EFI_SUCCESS; } 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 (#109685): https://edk2.groups.io/g/devel/message/109685 Mute This Topic: https://groups.io/mt/102017316/21656 Group Owner: devel+ow...@edk2.grou