public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [Patch V2] MinPlatformPkg: Check if Acpi table is already installed.
@ 2021-12-06  6:16 kavya
  2021-12-07  5:56 ` [edk2-devel] " Ni, Ray
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: kavya @ 2021-12-06  6:16 UTC (permalink / raw)
  To: devel; +Cc: kavya, Chasel Chiu, Nate DeSimone, Liming Gao, Eric Dong

Check if Acpi table is already installed by locating the first ACPI table
in XSDT/RSDT based on Signature

Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Eric Dong <eric.dong@intel.com>
Signed-off-by: kavya <k.kavyax.sravanthi@intel.com>
Reviewed-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
 Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index 785cf4c2f9..dcbb8b7c7f 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -1048,12 +1048,21 @@ InstallMcfgFromScratch (
 {
   EFI_STATUS                                                                            Status;
   EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER                        *McfgTable;
+  EFI_ACPI_COMMON_HEADER                                                                *Mcfg;
   EFI_ACPI_MEMORY_MAPPED_ENHANCED_CONFIGURATION_SPACE_BASE_ADDRESS_ALLOCATION_STRUCTURE *Segment;
   UINTN                                                                                 Index;
   UINTN                                                                                 SegmentCount;
   PCI_SEGMENT_INFO                                                                      *PciSegmentInfo;
   UINTN                                                                                 TableHandle;
 
+  Mcfg = (EFI_ACPI_COMMON_HEADER *) EfiLocateFirstAcpiTable (
+                                      EFI_ACPI_3_0_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE
+                                      );
+  if (Mcfg != NULL) {
+    DEBUG ((DEBUG_INFO, "MCFG table already installed\n"));
+    return EFI_SUCCESS;
+  }
+
   PciSegmentInfo = GetPciSegmentInfo (&SegmentCount);
 
   McfgTable = AllocateZeroPool (
@@ -1365,6 +1374,7 @@ UpdateLocalTable (
 {
   EFI_STATUS                    Status;
   EFI_ACPI_COMMON_HEADER        *CurrentTable;
+  EFI_ACPI_COMMON_HEADER        *Table;
   EFI_ACPI_TABLE_VERSION        Version;
   UINTN                         TableHandle;
   UINTN                         Index;
@@ -1372,6 +1382,14 @@ UpdateLocalTable (
   for (Index = 0; Index < sizeof(mLocalTable)/sizeof(mLocalTable[0]); Index++) {
     CurrentTable = mLocalTable[Index];
 
+    Table = (EFI_ACPI_COMMON_HEADER *) EfiLocateFirstAcpiTable (CurrentTable->Signature);
+    if (Table != NULL) {
+      DEBUG ((DEBUG_INFO, "Acpi table with signature=%c%c%c%c already installed\n",
+            ((CHAR8*)&CurrentTable->Signature)[0], ((CHAR8*)&CurrentTable->Signature)[1],
+            ((CHAR8*)&CurrentTable->Signature)[2], ((CHAR8*)&CurrentTable->Signature)[3]));
+      continue;
+    }
+
     PlatformUpdateTables (CurrentTable, &Version);
 
     TableHandle = 0;
-- 
2.16.2.windows.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [edk2-devel] [Patch V2] MinPlatformPkg: Check if Acpi table is already installed.
  2021-12-06  6:16 [Patch V2] MinPlatformPkg: Check if Acpi table is already installed kavya
@ 2021-12-07  5:56 ` Ni, Ray
  2021-12-07  7:23 ` Dong, Eric
  2021-12-08  2:06 ` Nate DeSimone
  2 siblings, 0 replies; 6+ messages in thread
From: Ni, Ray @ 2021-12-07  5:56 UTC (permalink / raw)
  To: kavya, devel

[-- Attachment #1: Type: text/plain, Size: 40 bytes --]

Reviewed-by: Ray Ni <ray.ni@intel.com>

[-- Attachment #2: Type: text/html, Size: 99 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch V2] MinPlatformPkg: Check if Acpi table is already installed.
  2021-12-06  6:16 [Patch V2] MinPlatformPkg: Check if Acpi table is already installed kavya
  2021-12-07  5:56 ` [edk2-devel] " Ni, Ray
@ 2021-12-07  7:23 ` Dong, Eric
  2021-12-08  2:06 ` Nate DeSimone
  2 siblings, 0 replies; 6+ messages in thread
From: Dong, Eric @ 2021-12-07  7:23 UTC (permalink / raw)
  To: Sravanthi, K KavyaX, devel@edk2.groups.io
  Cc: Chiu, Chasel, Desimone, Nathaniel L, Liming Gao

Reviewed-by: Eric Dong <eric.dong@intel.com>

-----Original Message-----
From: Sravanthi, K KavyaX <k.kavyax.sravanthi@intel.com> 
Sent: Monday, December 6, 2021 2:16 PM
To: devel@edk2.groups.io
Cc: Sravanthi, K KavyaX <k.kavyax.sravanthi@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>
Subject: [Patch V2] MinPlatformPkg: Check if Acpi table is already installed.

Check if Acpi table is already installed by locating the first ACPI table in XSDT/RSDT based on Signature

Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Eric Dong <eric.dong@intel.com>
Signed-off-by: kavya <k.kavyax.sravanthi@intel.com>
Reviewed-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
 Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index 785cf4c2f9..dcbb8b7c7f 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -1048,12 +1048,21 @@ InstallMcfgFromScratch (  {
   EFI_STATUS                                                                            Status;
   EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER                        *McfgTable;
+  EFI_ACPI_COMMON_HEADER                                                                *Mcfg;
   EFI_ACPI_MEMORY_MAPPED_ENHANCED_CONFIGURATION_SPACE_BASE_ADDRESS_ALLOCATION_STRUCTURE *Segment;
   UINTN                                                                                 Index;
   UINTN                                                                                 SegmentCount;
   PCI_SEGMENT_INFO                                                                      *PciSegmentInfo;
   UINTN                                                                                 TableHandle;
 
+  Mcfg = (EFI_ACPI_COMMON_HEADER *) EfiLocateFirstAcpiTable (
+                                      EFI_ACPI_3_0_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE
+                                      );  if (Mcfg != NULL) {
+    DEBUG ((DEBUG_INFO, "MCFG table already installed\n"));
+    return EFI_SUCCESS;
+  }
+
   PciSegmentInfo = GetPciSegmentInfo (&SegmentCount);
 
   McfgTable = AllocateZeroPool (
@@ -1365,6 +1374,7 @@ UpdateLocalTable (  {
   EFI_STATUS                    Status;
   EFI_ACPI_COMMON_HEADER        *CurrentTable;
+  EFI_ACPI_COMMON_HEADER        *Table;
   EFI_ACPI_TABLE_VERSION        Version;
   UINTN                         TableHandle;
   UINTN                         Index;
@@ -1372,6 +1382,14 @@ UpdateLocalTable (
   for (Index = 0; Index < sizeof(mLocalTable)/sizeof(mLocalTable[0]); Index++) {
     CurrentTable = mLocalTable[Index];
 
+    Table = (EFI_ACPI_COMMON_HEADER *) EfiLocateFirstAcpiTable (CurrentTable->Signature);
+    if (Table != NULL) {
+      DEBUG ((DEBUG_INFO, "Acpi table with signature=%c%c%c%c already installed\n",
+            ((CHAR8*)&CurrentTable->Signature)[0], ((CHAR8*)&CurrentTable->Signature)[1],
+            ((CHAR8*)&CurrentTable->Signature)[2], ((CHAR8*)&CurrentTable->Signature)[3]));
+      continue;
+    }
+
     PlatformUpdateTables (CurrentTable, &Version);
 
     TableHandle = 0;
--
2.16.2.windows.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Patch V2] MinPlatformPkg: Check if Acpi table is already installed.
  2021-12-06  6:16 [Patch V2] MinPlatformPkg: Check if Acpi table is already installed kavya
  2021-12-07  5:56 ` [edk2-devel] " Ni, Ray
  2021-12-07  7:23 ` Dong, Eric
@ 2021-12-08  2:06 ` Nate DeSimone
  2021-12-08  6:41   ` [edk2-devel] " Ni, Ray
  2 siblings, 1 reply; 6+ messages in thread
From: Nate DeSimone @ 2021-12-08  2:06 UTC (permalink / raw)
  To: Sravanthi, K KavyaX, devel@edk2.groups.io
  Cc: Chiu, Chasel, Liming Gao, Dong, Eric, Oram, Isaac W,
	Michael Kubacki

This seems like a hack to me. One of the major goals of the Minimum Platform Architecture is consistency. A board override for the MinPlatform provided installation of the MCFG table runs counter to that goal. Every field in the MCFG table produced by MinPlatform's implementation is fully customizable by board code via the PciSegmentInfoLib's GetPciSegmentInfo() function. If a multi-segment MCFG table is needed, then all the board needs to do is choose a different implementation of PciSegmentInfoLib than the default one (MinPlatformPkg\Pci\Library\PciSegmentInfoLibSimple\PciSegmentInfoLibSimple.inf)

I cannot conceive of any reason why a board override of this functionality is required. Why do you need this? 

-----Original Message-----
From: Sravanthi, K KavyaX <k.kavyax.sravanthi@intel.com> 
Sent: Sunday, December 5, 2021 10:16 PM
To: devel@edk2.groups.io
Cc: Sravanthi, K KavyaX <k.kavyax.sravanthi@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>
Subject: [Patch V2] MinPlatformPkg: Check if Acpi table is already installed.

Check if Acpi table is already installed by locating the first ACPI table in XSDT/RSDT based on Signature

Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Eric Dong <eric.dong@intel.com>
Signed-off-by: kavya <k.kavyax.sravanthi@intel.com>
Reviewed-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
 Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index 785cf4c2f9..dcbb8b7c7f 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -1048,12 +1048,21 @@ InstallMcfgFromScratch (  {
   EFI_STATUS                                                                            Status;
   EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER                        *McfgTable;
+  EFI_ACPI_COMMON_HEADER                                                                *Mcfg;
   EFI_ACPI_MEMORY_MAPPED_ENHANCED_CONFIGURATION_SPACE_BASE_ADDRESS_ALLOCATION_STRUCTURE *Segment;
   UINTN                                                                                 Index;
   UINTN                                                                                 SegmentCount;
   PCI_SEGMENT_INFO                                                                      *PciSegmentInfo;
   UINTN                                                                                 TableHandle;
 
+  Mcfg = (EFI_ACPI_COMMON_HEADER *) EfiLocateFirstAcpiTable (
+                                      EFI_ACPI_3_0_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE
+                                      );  if (Mcfg != NULL) {
+    DEBUG ((DEBUG_INFO, "MCFG table already installed\n"));
+    return EFI_SUCCESS;
+  }
+
   PciSegmentInfo = GetPciSegmentInfo (&SegmentCount);
 
   McfgTable = AllocateZeroPool (
@@ -1365,6 +1374,7 @@ UpdateLocalTable (  {
   EFI_STATUS                    Status;
   EFI_ACPI_COMMON_HEADER        *CurrentTable;
+  EFI_ACPI_COMMON_HEADER        *Table;
   EFI_ACPI_TABLE_VERSION        Version;
   UINTN                         TableHandle;
   UINTN                         Index;
@@ -1372,6 +1382,14 @@ UpdateLocalTable (
   for (Index = 0; Index < sizeof(mLocalTable)/sizeof(mLocalTable[0]); Index++) {
     CurrentTable = mLocalTable[Index];
 
+    Table = (EFI_ACPI_COMMON_HEADER *) EfiLocateFirstAcpiTable (CurrentTable->Signature);
+    if (Table != NULL) {
+      DEBUG ((DEBUG_INFO, "Acpi table with signature=%c%c%c%c already installed\n",
+            ((CHAR8*)&CurrentTable->Signature)[0], ((CHAR8*)&CurrentTable->Signature)[1],
+            ((CHAR8*)&CurrentTable->Signature)[2], ((CHAR8*)&CurrentTable->Signature)[3]));
+      continue;
+    }
+
     PlatformUpdateTables (CurrentTable, &Version);
 
     TableHandle = 0;
--
2.16.2.windows.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [edk2-devel] [Patch V2] MinPlatformPkg: Check if Acpi table is already installed.
  2021-12-08  2:06 ` Nate DeSimone
@ 2021-12-08  6:41   ` Ni, Ray
  2021-12-13 23:56     ` Nate DeSimone
  0 siblings, 1 reply; 6+ messages in thread
From: Ni, Ray @ 2021-12-08  6:41 UTC (permalink / raw)
  To: devel@edk2.groups.io, Desimone, Nathaniel L, Sravanthi, K KavyaX
  Cc: Chiu, Chasel, Liming Gao, Dong, Eric, Oram, Isaac W,
	Michael Kubacki

Nate,
I don't consider it as a hack. UefiPayloadPkg requires that bootloader produces all ACPI tables.
Now we are in a middle stage. So, only the MCFG and FADT are produced (as gUniversalPayloadAcpiTableGuid HOB) in PEI phase.

Thanks,
Ray


-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Nate DeSimone
Sent: Wednesday, December 8, 2021 10:06 AM
To: Sravanthi, K KavyaX <k.kavyax.sravanthi@intel.com>; devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>; Michael Kubacki <michael.kubacki@microsoft.com>
Subject: Re: [edk2-devel] [Patch V2] MinPlatformPkg: Check if Acpi table is already installed.

This seems like a hack to me. One of the major goals of the Minimum Platform Architecture is consistency. A board override for the MinPlatform provided installation of the MCFG table runs counter to that goal. Every field in the MCFG table produced by MinPlatform's implementation is fully customizable by board code via the PciSegmentInfoLib's GetPciSegmentInfo() function. If a multi-segment MCFG table is needed, then all the board needs to do is choose a different implementation of PciSegmentInfoLib than the default one (MinPlatformPkg\Pci\Library\PciSegmentInfoLibSimple\PciSegmentInfoLibSimple.inf)

I cannot conceive of any reason why a board override of this functionality is required. Why do you need this? 

-----Original Message-----
From: Sravanthi, K KavyaX <k.kavyax.sravanthi@intel.com> 
Sent: Sunday, December 5, 2021 10:16 PM
To: devel@edk2.groups.io
Cc: Sravanthi, K KavyaX <k.kavyax.sravanthi@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>
Subject: [Patch V2] MinPlatformPkg: Check if Acpi table is already installed.

Check if Acpi table is already installed by locating the first ACPI table in XSDT/RSDT based on Signature

Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Eric Dong <eric.dong@intel.com>
Signed-off-by: kavya <k.kavyax.sravanthi@intel.com>
Reviewed-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
 Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index 785cf4c2f9..dcbb8b7c7f 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -1048,12 +1048,21 @@ InstallMcfgFromScratch (  {
   EFI_STATUS                                                                            Status;
   EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER                        *McfgTable;
+  EFI_ACPI_COMMON_HEADER                                                                *Mcfg;
   EFI_ACPI_MEMORY_MAPPED_ENHANCED_CONFIGURATION_SPACE_BASE_ADDRESS_ALLOCATION_STRUCTURE *Segment;
   UINTN                                                                                 Index;
   UINTN                                                                                 SegmentCount;
   PCI_SEGMENT_INFO                                                                      *PciSegmentInfo;
   UINTN                                                                                 TableHandle;
 
+  Mcfg = (EFI_ACPI_COMMON_HEADER *) EfiLocateFirstAcpiTable (
+                                      EFI_ACPI_3_0_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE
+                                      );  if (Mcfg != NULL) {
+    DEBUG ((DEBUG_INFO, "MCFG table already installed\n"));
+    return EFI_SUCCESS;
+  }
+
   PciSegmentInfo = GetPciSegmentInfo (&SegmentCount);
 
   McfgTable = AllocateZeroPool (
@@ -1365,6 +1374,7 @@ UpdateLocalTable (  {
   EFI_STATUS                    Status;
   EFI_ACPI_COMMON_HEADER        *CurrentTable;
+  EFI_ACPI_COMMON_HEADER        *Table;
   EFI_ACPI_TABLE_VERSION        Version;
   UINTN                         TableHandle;
   UINTN                         Index;
@@ -1372,6 +1382,14 @@ UpdateLocalTable (
   for (Index = 0; Index < sizeof(mLocalTable)/sizeof(mLocalTable[0]); Index++) {
     CurrentTable = mLocalTable[Index];
 
+    Table = (EFI_ACPI_COMMON_HEADER *) EfiLocateFirstAcpiTable (CurrentTable->Signature);
+    if (Table != NULL) {
+      DEBUG ((DEBUG_INFO, "Acpi table with signature=%c%c%c%c already installed\n",
+            ((CHAR8*)&CurrentTable->Signature)[0], ((CHAR8*)&CurrentTable->Signature)[1],
+            ((CHAR8*)&CurrentTable->Signature)[2], ((CHAR8*)&CurrentTable->Signature)[3]));
+      continue;
+    }
+
     PlatformUpdateTables (CurrentTable, &Version);
 
     TableHandle = 0;
--
2.16.2.windows.1







^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [edk2-devel] [Patch V2] MinPlatformPkg: Check if Acpi table is already installed.
  2021-12-08  6:41   ` [edk2-devel] " Ni, Ray
@ 2021-12-13 23:56     ` Nate DeSimone
  0 siblings, 0 replies; 6+ messages in thread
From: Nate DeSimone @ 2021-12-13 23:56 UTC (permalink / raw)
  To: Ni, Ray, devel@edk2.groups.io, Sravanthi, K KavyaX
  Cc: Chiu, Chasel, Gao, Liming, Dong, Eric, Oram, Isaac W,
	Kubacki, Michael

Hi Ray,

Since UefiPayloadPkg is in edk2, and MinPlatformPkg is in edk2-platforms, UefiPayloadPkg should not have any dependencies on MinPlatformPkg. Therefore, I don't see how that is relevant.

Thanks,
Nate

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com> 
Sent: Tuesday, December 7, 2021 10:42 PM
To: devel@edk2.groups.io; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Sravanthi, K KavyaX <k.kavyax.sravanthi@intel.com>
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>; Michael Kubacki <michael.kubacki@microsoft.com>
Subject: RE: [edk2-devel] [Patch V2] MinPlatformPkg: Check if Acpi table is already installed.

Nate,
I don't consider it as a hack. UefiPayloadPkg requires that bootloader produces all ACPI tables.
Now we are in a middle stage. So, only the MCFG and FADT are produced (as gUniversalPayloadAcpiTableGuid HOB) in PEI phase.

Thanks,
Ray


-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Nate DeSimone
Sent: Wednesday, December 8, 2021 10:06 AM
To: Sravanthi, K KavyaX <k.kavyax.sravanthi@intel.com>; devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>; Michael Kubacki <michael.kubacki@microsoft.com>
Subject: Re: [edk2-devel] [Patch V2] MinPlatformPkg: Check if Acpi table is already installed.

This seems like a hack to me. One of the major goals of the Minimum Platform Architecture is consistency. A board override for the MinPlatform provided installation of the MCFG table runs counter to that goal. Every field in the MCFG table produced by MinPlatform's implementation is fully customizable by board code via the PciSegmentInfoLib's GetPciSegmentInfo() function. If a multi-segment MCFG table is needed, then all the board needs to do is choose a different implementation of PciSegmentInfoLib than the default one (MinPlatformPkg\Pci\Library\PciSegmentInfoLibSimple\PciSegmentInfoLibSimple.inf)

I cannot conceive of any reason why a board override of this functionality is required. Why do you need this? 

-----Original Message-----
From: Sravanthi, K KavyaX <k.kavyax.sravanthi@intel.com> 
Sent: Sunday, December 5, 2021 10:16 PM
To: devel@edk2.groups.io
Cc: Sravanthi, K KavyaX <k.kavyax.sravanthi@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>
Subject: [Patch V2] MinPlatformPkg: Check if Acpi table is already installed.

Check if Acpi table is already installed by locating the first ACPI table in XSDT/RSDT based on Signature

Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Eric Dong <eric.dong@intel.com>
Signed-off-by: kavya <k.kavyax.sravanthi@intel.com>
Reviewed-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
 Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index 785cf4c2f9..dcbb8b7c7f 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -1048,12 +1048,21 @@ InstallMcfgFromScratch (  {
   EFI_STATUS                                                                            Status;
   EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER                        *McfgTable;
+  EFI_ACPI_COMMON_HEADER                                                                *Mcfg;
   EFI_ACPI_MEMORY_MAPPED_ENHANCED_CONFIGURATION_SPACE_BASE_ADDRESS_ALLOCATION_STRUCTURE *Segment;
   UINTN                                                                                 Index;
   UINTN                                                                                 SegmentCount;
   PCI_SEGMENT_INFO                                                                      *PciSegmentInfo;
   UINTN                                                                                 TableHandle;
 
+  Mcfg = (EFI_ACPI_COMMON_HEADER *) EfiLocateFirstAcpiTable (
+                                      EFI_ACPI_3_0_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE
+                                      );  if (Mcfg != NULL) {
+    DEBUG ((DEBUG_INFO, "MCFG table already installed\n"));
+    return EFI_SUCCESS;
+  }
+
   PciSegmentInfo = GetPciSegmentInfo (&SegmentCount);
 
   McfgTable = AllocateZeroPool (
@@ -1365,6 +1374,7 @@ UpdateLocalTable (  {
   EFI_STATUS                    Status;
   EFI_ACPI_COMMON_HEADER        *CurrentTable;
+  EFI_ACPI_COMMON_HEADER        *Table;
   EFI_ACPI_TABLE_VERSION        Version;
   UINTN                         TableHandle;
   UINTN                         Index;
@@ -1372,6 +1382,14 @@ UpdateLocalTable (
   for (Index = 0; Index < sizeof(mLocalTable)/sizeof(mLocalTable[0]); Index++) {
     CurrentTable = mLocalTable[Index];
 
+    Table = (EFI_ACPI_COMMON_HEADER *) EfiLocateFirstAcpiTable (CurrentTable->Signature);
+    if (Table != NULL) {
+      DEBUG ((DEBUG_INFO, "Acpi table with signature=%c%c%c%c already installed\n",
+            ((CHAR8*)&CurrentTable->Signature)[0], ((CHAR8*)&CurrentTable->Signature)[1],
+            ((CHAR8*)&CurrentTable->Signature)[2], ((CHAR8*)&CurrentTable->Signature)[3]));
+      continue;
+    }
+
     PlatformUpdateTables (CurrentTable, &Version);
 
     TableHandle = 0;
--
2.16.2.windows.1







^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-12-13 23:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-12-06  6:16 [Patch V2] MinPlatformPkg: Check if Acpi table is already installed kavya
2021-12-07  5:56 ` [edk2-devel] " Ni, Ray
2021-12-07  7:23 ` Dong, Eric
2021-12-08  2:06 ` Nate DeSimone
2021-12-08  6:41   ` [edk2-devel] " Ni, Ray
2021-12-13 23:56     ` Nate DeSimone

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox