On 11/17/23 18:50, Chiu, Chasel wrote:
>
> Hi Dhaval,
>
> Just a small feedback,
> the only difference will be TableToInstall between XDsdt and Dsdt, could we optimize the code flow to reduce duplicate lines?

since a v3 is being requested, let me ask for even more:

- can we specify the precise ACPI spec location in the commit message?

- s/abscent/absent/

thanks!
Laszlo

>
> Thanks,
> Chasel
>
>
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Dhaval
>> Sharma
>> Sent: Friday, November 17, 2023 3:35 AM
>> To: devel@edk2.groups.io
>> Cc: Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang
>> <zhiguang.liu@intel.com>; Bi, Dandan <dandan.bi@intel.com>
>> Subject: [edk2-devel] [PATCH v2 1/1] MdeModulePkg: Fix issue with ACPI table
>> creation
>>
>> As per spec if xDSDT is avaialble, it should be used first.
>> Handle required flow when xDSDT is abscent or present.
>>
>> Test: Tested on RISCV64 Qemu platform with xDSDT and booted to linux kernel.
>>
>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
>> Cc: Dandan Bi <dandan.bi@intel.com>
>> Signed-off-by: Dhaval Sharma <dhaval@rivosinc.com>
>> ---
>>
>> Notes:
>>     v2:
>>     - Added proper indentation for else if
>>
>>  MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 13
>> ++++++++++++-
>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
>> b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
>> index e09bc9b704f5..11097ed4c3be 100644
>> --- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
>> +++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
>> @@ -1892,7 +1892,18 @@ InstallAcpiTableFromHob (
>>            }         } -        if (((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE
>> *)ChildTable)->Dsdt != 0) {+        //+        // First check if xDSDT is available that is
>> preferred+        //+        if (((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE
>> *)ChildTable)->XDsdt != 0) {+          TableToInstall = (VOID
>> *)(UINTN)((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)ChildTable)-
>>> XDsdt;+          Status         = AddTableToList (AcpiTableInstance, TableToInstall,
>> TRUE, Version, TRUE, &TableKey);+          if (EFI_ERROR (Status)) {+            DEBUG
>> ((DEBUG_ERROR, "InstallAcpiTableFromHob: Fail to add ACPI table xDSDT\n"));+
>> ASSERT_EFI_ERROR (Status);+            break;+          }+        } else if
>> (((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)ChildTable)->Dsdt != 0)
>> {           TableToInstall = (VOID
>> *)(UINTN)((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)ChildTable)-
>>> Dsdt;           Status         = AddTableToList (AcpiTableInstance, TableToInstall,
>> TRUE, Version, TRUE, &TableKey);           if (EFI_ERROR (Status)) {--
>> 2.39.2
>>
>>
>>
>> -=-=-=-=-=-=
>> Groups.io Links: You receive all messages sent to this group.
>> View/Reply Online (#111395): https://edk2.groups.io/g/devel/message/111395
>> Mute This Topic: https://groups.io/mt/102645488/1777047
>> Group Owner: devel+owner@edk2.groups.io
>> Unsubscribe: https://edk2.groups.io/g/devel/unsub [chasel.chiu@intel.com] -=-
>> =-=-=-=-=
>>
>
>
>
>
>
>