Re: [PATCH 0/2] MdeModulePkg/AcpiTableDxe: improve FADT.{DSDT, X_DSDT} mutual exclusion

["Yao, Jiewen" <jiewen.yao@intel.com>]

Hi Laszlo
I agree with you that the strange order issue and spec compliance issue need to be fixed.

I am OK on the final result in RequireDsdtXDsdtExclusion().

However, the logic below is a little hard to read for me, because I have to switch my mind between "return FALSE" and "return TRUE".
=========================================
+  if (Fadt->Header.Revision < 5) {
+    return FALSE;
+  }
+  if (Fadt->Header.Revision > 5) {
+    return TRUE;
+  }
+  //
+  // For FADT Major Version 5, check the FADT Minor Version as well (offset 131
+  // decimal).
+  //
+  if (Fadt->Reserved2[2] < 1) {
+    return FALSE;
+  }
+  return TRUE;
=========================================

How about we use a simpler logic? Such as below:
=========================================
+  // version <= 5.0
+  if ((Fadt->Header.Revision < 5) ||
+     ((Fadt->Header.Revision == 5) && (((EFI_ACPI_5_1_FIXED_ACPI_DESCRIPTION_TABLE *)Fadt)->MinorVersion == 0))) {
+    return FALSE;
+  }
+  // version >= 5.1
+  return TRUE;
=========================================

Thank you
Yao Jiewen




> -----Original Message-----
> From: Zeng, Star
> Sent: Tuesday, February 7, 2017 6:20 PM
> To: Laszlo Ersek <lersek@redhat.com>; edk2-devel-01 <edk2-devel@lists.01.org>
> Cc: Tian, Feng <feng.tian@intel.com>; Michael Tsirkin <mtsirkin@redhat.com>;
> Ard Biesheuvel <ard.biesheuvel@linaro.org>; Phil Dennis-Jordan
> <phil@philjordan.eu>; Leo Duran <leo.duran@amd.com>; Al Stone
> <ahs3@redhat.com>; Zeng, Star <star.zeng@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>
> Subject: RE: [edk2] [PATCH 0/2] MdeModulePkg/AcpiTableDxe: improve
> FADT.{DSDT, X_DSDT} mutual exclusion
> 
> Laszlo,
> 
> I will give feedback tomorrow (PRC time) to this series, sorry for late.
> 
> Also Cc Jiewen.
> 
> Thanks,
> Star
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo
> Ersek
> Sent: Thursday, February 2, 2017 1:57 AM
> To: edk2-devel-01 <edk2-devel@lists.01.org>
> Cc: Tian, Feng <feng.tian@intel.com>; Michael Tsirkin <mtsirkin@redhat.com>;
> Ard Biesheuvel <ard.biesheuvel@linaro.org>; Phil Dennis-Jordan
> <phil@philjordan.eu>; Leo Duran <leo.duran@amd.com>; Al Stone
> <ahs3@redhat.com>; Zeng, Star <star.zeng@intel.com>
> Subject: [edk2] [PATCH 0/2] MdeModulePkg/AcpiTableDxe: improve FADT.{DSDT,
> X_DSDT} mutual exclusion
> 
> Patch #2 explains it all.
> 
> Repo:   https://github.com/lersek/edk2/
> Branch: fadt_dsdt
> 
> NOTE for people on the CC list:
> 
> If you are not presently subscribed to edk2-devel and wish to comment on this
> series publicly, you need to subscribe first, and wait for the subscription request
> to *complete* (see your inbox), *before* sending your followup. This is not
> ideal, but edk2-devel requires subscription before reflecting messages from
> someone.
> 
> Subscribe at <https://lists.01.org/mailman/listinfo/edk2-devel>. Thanks.
> 
> Cc: Al Stone <ahs3@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Feng Tian <feng.tian@intel.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Leo Duran <leo.duran@amd.com>
> Cc: Michael Tsirkin <mtsirkin@redhat.com>
> Cc: Phil Dennis-Jordan <phil@philjordan.eu>
> Cc: Star Zeng <star.zeng@intel.com>
> 
> Laszlo Ersek (2):
>   MdeModulePkg/AcpiTableDxe: condense whitespace around
>     FADT.{DSDT,X_DSDT}
>   MdeModulePkg/AcpiTableDxe: improve FADT.{DSDT,X_DSDT} mutual exclusion
> 
>  MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 82
> ++++++++++++++++----
>  1 file changed, 67 insertions(+), 15 deletions(-)
> 
> --
> 2.9.3
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel