回复: [edk2-devel] [PATCH v4 1/1] MdeModulePkg/AcpiTableDxe: Prefer xDSDT over DSDT when installing tables

["gaoliming via groups.io" <gaoliming=byosoft.com.cn@groups.io>]

Mike:
  Seemly, I miss this patch. 

Dhaval:
  I add one comment below. 
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Michael D
> Kinney
> 发送时间: 2024年1月24日 10:24
> 收件人: devel@edk2.groups.io; dhaval@rivosinc.com
> 抄送: Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang
> <zhiguang.liu@intel.com>; Bi, Dandan <dandan.bi@intel.com>; Pedro Falcato
> <pedro.falcato@gmail.com>; Chiu, Chasel <chasel.chiu@intel.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>
> 主题: Re: [edk2-devel] [PATCH v4 1/1] MdeModulePkg/AcpiTableDxe: Prefer
> xDSDT over DSDT when installing tables
> 
> Hi Liming,
> 
> I do not see any reviews of this patch.
> 
> What is the status?
> 
> Mike
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Dhaval
> > Sharma
> > Sent: Monday, January 8, 2024 7:00 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>; Pedro
> > Falcato <pedro.falcato@gmail.com>; Chiu, Chasel <chasel.chiu@intel.com>
> > Subject: [edk2-devel] [PATCH v4 1/1] MdeModulePkg/AcpiTableDxe: Prefer
> > xDSDT over DSDT when installing tables
> >
> > As per ACPI Spec 6.5+ Table 5-9 if xDSDT is available,
> > it should be used first. Handle required flow when xDSDT
> > is absent 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>
> > Cc: Pedro Falcato <pedro.falcato@gmail.com>
> > Cc: devel@edk2.groups.io
> > Signed-off-by: Dhaval Sharma <dhaval@rivosinc.com>
> > Acked-by: Chasel Chiu <chasel.chiu@...>
> > ---
> >
> > Notes:
> >     v4:
> >     - Fix typos and commit message adding more clarity to patch subject
> >     v3:
> >     - Added description of ACPI spec clarification based on which this
> > patch is created
> >     - Optimizing if-else flow
> >     v2:
> >     - Added proper indentation for else if
> >
> >  MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 23
> > ++++++++++++++------
> >  1 file changed, 16 insertions(+), 7 deletions(-)
> >
> > diff --git
> > a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> > b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> > index e09bc9b704f5..61af6047a2a7 100644
> > --- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> > +++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> > @@ -1892,14 +1892,23 @@ InstallAcpiTableFromHob (
> >            }
> >
> >          }
> >
> >
> >
> > -        if (((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE
> *)ChildTable)-
> > >Dsdt != 0) {
> >
> > +        //
> >
> > +        // First check if xDSDT is available, as that is preferred as
> > per
> >
> > +        // ACPI Spec 6.5+ Table 5-9 X_DSDT definition
> >
> > +        //
> >
> > +        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;
> >
> > +        } 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)) {
> >
> > -            DEBUG ((DEBUG_ERROR, "InstallAcpiTableFromHob: Fail to
> add
> > ACPI table DSDT\n"));
> >
> > -            ASSERT_EFI_ERROR (Status);
> >
> > -            break;
> >
> > -          }
> >
> > +        } else {
> >
> > +          break;

[Liming] If XDsdt and Dsdt are both zero, here is break. The previous logic
will continue to look when Dsdt is zero. 
Please check whether this change is by purpose. If this change is by design,
I still think the return Status should be set to EFI_NOT_FOUND.

Thanks
Liming
> >
> > +        }
> >
> > +
> >
> > +        Status = AddTableToList (AcpiTableInstance, TableToInstall,
> > TRUE, Version, TRUE, &TableKey);
> >
> > +        if (EFI_ERROR (Status)) {
> >
> > +          DEBUG ((DEBUG_ERROR, "InstallAcpiTableFromHob: Fail to
> add
> > ACPI table DSDT\n"));
> >
> > +          ASSERT_EFI_ERROR (Status);
> >
> > +          break;
> >
> >          }
> >
> >        }
> >
> >      }
> >
> > --
> > 2.39.2
> >
> >
> >
> > -=-=-=-=-=-=
> > Groups.io Links: You receive all messages sent to this group.
> > View/Reply Online (#113401):
> > https://edk2.groups.io/g/devel/message/113401
> > Mute This Topic: https://groups.io/mt/103598583/1643496
> > Group Owner: devel+owner@edk2.groups.io
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub
> > [michael.d.kinney@intel.com]
> > -=-=-=-=-=-=
> >
> 
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114306): https://edk2.groups.io/g/devel/message/114306
Mute This Topic: https://groups.io/mt/103932913/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-