public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH edk2-platforms 0/3] JunoPkg: Fix AcpiSsdtRootPci.asl to use spaces and reserve ECAM area
@ 2022-03-05  4:19 Rebecca Cran
  2022-03-05  4:19 ` [PATCH edk2-platforms 1/3] Platform/ARM/JunoPkg: Convert AcpiSsdtRootPci.asl from tabs to spaces Rebecca Cran
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Rebecca Cran @ 2022-03-05  4:19 UTC (permalink / raw)
  To: devel, Ard Biesheuvel, Thomas Abraham, Sami Mujawar; +Cc: Rebecca Cran

I noticed Linux reports a firmware bug with the current Juno ACPI
tables. These patches fix it by reserving the ECAM area with a RES0
device, while also converting AcpiSsdtRootPci.asl from tabs to spaces
and using the standard Pcd from MdePkg for the ECAM base address.

Rebecca Cran (3):
  Platform/ARM/JunoPkg: Convert AcpiSsdtRootPci.asl from tabs to spaces
  Platform/ARM/JunoPkg: Use MdePkg PcdPciExpressBaseAddress for ECAM
    addr
  Platform/ARM/JunoPkg: Reserve the ECAM area in ACPI with RES0 device

 Platform/ARM/JunoPkg/ArmJuno.dec                                                              |   4 +-
 Platform/ARM/JunoPkg/AcpiTables/AcpiTables.inf                                                |   4 +
 Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf |   2 +-
 Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf                                        |   2 +-
 Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoLib.inf                                        |   2 +-
 Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/JunoPciHostBridgeLib.inf                    |   2 +-
 Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/XPressRich3.h                               |   2 +-
 Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c      |   2 +-
 Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/AcpiTables.c                                          |   2 +-
 Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c                                          |   4 +-
 Platform/ARM/JunoPkg/AcpiTables/AcpiSsdtRootPci.asl                                           | 301 ++++++++++----------
 11 files changed, 172 insertions(+), 155 deletions(-)

-- 
2.25.1


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

* [PATCH edk2-platforms 1/3] Platform/ARM/JunoPkg: Convert AcpiSsdtRootPci.asl from tabs to spaces
  2022-03-05  4:19 [PATCH edk2-platforms 0/3] JunoPkg: Fix AcpiSsdtRootPci.asl to use spaces and reserve ECAM area Rebecca Cran
@ 2022-03-05  4:19 ` Rebecca Cran
  2022-03-16 15:40   ` Sami Mujawar
  2022-03-05  4:19 ` [PATCH edk2-platforms 2/3] Platform/ARM/JunoPkg: Use MdePkg PcdPciExpressBaseAddress for ECAM addr Rebecca Cran
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Rebecca Cran @ 2022-03-05  4:19 UTC (permalink / raw)
  To: devel, Ard Biesheuvel, Thomas Abraham, Sami Mujawar; +Cc: Rebecca Cran

Other .asl files in Platform/ARM/JunoPkg/AcpiTables use spaces, while
AcpiSsdtRootPci.asl uses tabs. To be consistent, convert it to spaces.

Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
---
 Platform/ARM/JunoPkg/AcpiTables/AcpiSsdtRootPci.asl | 288 ++++++++++----------
 1 file changed, 144 insertions(+), 144 deletions(-)

diff --git a/Platform/ARM/JunoPkg/AcpiTables/AcpiSsdtRootPci.asl b/Platform/ARM/JunoPkg/AcpiTables/AcpiSsdtRootPci.asl
index ba41a9586555..317b621e013e 100644
--- a/Platform/ARM/JunoPkg/AcpiTables/AcpiSsdtRootPci.asl
+++ b/Platform/ARM/JunoPkg/AcpiTables/AcpiSsdtRootPci.asl
@@ -28,24 +28,24 @@
   interrupt type as PCI defaults (Level Triggered, Active Low) are not
   compatible with GICv2.
 */
-#define LNK_DEVICE(Unique_Id, Link_Name, irq)							\
-	Device(Link_Name) {									\
-	    Name(_HID, EISAID("PNP0C0F"))							\
-	    Name(_UID, Unique_Id)								\
-	    Name(_PRS, ResourceTemplate() {							\
-	        Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive) { irq }		\
-	    })											\
-	    Method (_CRS, 0) { Return (_PRS) }							\
-	    Method (_SRS, 1) { }								\
-	    Method (_DIS) { }									\
-	}
+#define LNK_DEVICE(Unique_Id, Link_Name, irq)                                  \
+  Device(Link_Name) {                                                          \
+      Name(_HID, EISAID("PNP0C0F"))                                            \
+      Name(_UID, Unique_Id)                                                    \
+      Name(_PRS, ResourceTemplate() {                                          \
+          Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive) { irq }    \
+      })                                                                       \
+      Method (_CRS, 0) { Return (_PRS) }                                       \
+      Method (_SRS, 1) { }                                                     \
+      Method (_DIS) { }                                                        \
+  }
 
-#define PRT_ENTRY(Address, Pin, Link)								  \
+#define PRT_ENTRY(Address, Pin, Link)                                                             \
         Package (4) {                                                                             \
             Address,    /* uses the same format as _ADR */                                        \
             Pin,        /* The PCI pin number of the device (0-INTA, 1-INTB, 2-INTC, 3-INTD). */  \
-            Link,       /* Interrupt allocated via Link device. */   	     	     	      	  \
-            Zero        /* global system interrupt number (no used) */				  \
+            Link,       /* Interrupt allocated via Link device. */                                \
+            Zero        /* global system interrupt number (no used) */                            \
           }
 
 /*
@@ -59,155 +59,155 @@
 
 DefinitionBlock("SsdtPci.aml", "SSDT", 1, "ARMLTD", "ARM-JUNO", EFI_ACPI_ARM_OEM_REVISION) {
   Scope(_SB) {
-	//
-	// PCI Root Complex
-	//
-	LNK_DEVICE(1, LNKA, 168)
-	LNK_DEVICE(2, LNKB, 169)
-	LNK_DEVICE(3, LNKC, 170)
-	LNK_DEVICE(4, LNKD, 171)
+    //
+    // PCI Root Complex
+    //
+    LNK_DEVICE(1, LNKA, 168)
+    LNK_DEVICE(2, LNKB, 169)
+    LNK_DEVICE(3, LNKC, 170)
+    LNK_DEVICE(4, LNKD, 171)
 
-	Device(PCI0)
+    Device(PCI0)
     {
-  		Name(_HID, EISAID("PNP0A08")) // PCI Express Root Bridge
-  		Name(_CID, EISAID("PNP0A03")) // Compatible PCI Root Bridge
-  		Name(_SEG, Zero) // PCI Segment Group number
-  		Name(_BBN, Zero) // PCI Base Bus Number
-  		Name(_CCA, 1)    // Initially mark the PCI coherent (for JunoR1)
+      Name(_HID, EISAID("PNP0A08")) // PCI Express Root Bridge
+      Name(_CID, EISAID("PNP0A03")) // Compatible PCI Root Bridge
+      Name(_SEG, Zero) // PCI Segment Group number
+      Name(_BBN, Zero) // PCI Base Bus Number
+      Name(_CCA, 1)    // Initially mark the PCI coherent (for JunoR1)
 
-        // Root Complex 0
-        Device (RP0) {
-            Name(_ADR, 0xF0000000)    // Dev 0, Func 0
-        }
+      // Root Complex 0
+      Device (RP0) {
+        Name(_ADR, 0xF0000000)    // Dev 0, Func 0
+      }
 
-		// PCI Routing Table
-		Name(_PRT, Package() {
-			ROOT_PRT_ENTRY(0, LNKA),   // INTA
-			ROOT_PRT_ENTRY(1, LNKB),   // INTB
-			ROOT_PRT_ENTRY(2, LNKC),   // INTC
-			ROOT_PRT_ENTRY(3, LNKD),   // INTD
-		})
-        // Root complex resources
-		Method (_CRS, 0, Serialized) {
-			Name (RBUF, ResourceTemplate () {
-				WordBusNumber ( // Bus numbers assigned to this root
-					ResourceProducer,
-					MinFixed, MaxFixed, PosDecode,
-					0,   // AddressGranularity
-					0,   // AddressMinimum - Minimum Bus Number
-					255, // AddressMaximum - Maximum Bus Number
-					0,   // AddressTranslation - Set to 0
-					256  // RangeLength - Number of Busses
-				)
+      // PCI Routing Table
+      Name(_PRT, Package() {
+        ROOT_PRT_ENTRY(0, LNKA),   // INTA
+        ROOT_PRT_ENTRY(1, LNKB),   // INTB
+        ROOT_PRT_ENTRY(2, LNKC),   // INTC
+        ROOT_PRT_ENTRY(3, LNKD),   // INTD
+      })
+      // Root complex resources
+      Method (_CRS, 0, Serialized) {
+        Name (RBUF, ResourceTemplate () {
+          WordBusNumber ( // Bus numbers assigned to this root
+            ResourceProducer,
+            MinFixed, MaxFixed, PosDecode,
+            0,   // AddressGranularity
+            0,   // AddressMinimum - Minimum Bus Number
+            255, // AddressMaximum - Maximum Bus Number
+            0,   // AddressTranslation - Set to 0
+            256  // RangeLength - Number of Busses
+          )
 
-				DWordMemory ( // 32-bit BAR Windows
-					ResourceProducer, PosDecode,
-					MinFixed, MaxFixed,
-					Cacheable, ReadWrite,
-					0x00000000, 							// Granularity
-					0x50000000, 							// Min Base Address
-					0x57FFFFFF, 							// Max Base Address
-					0x00000000, 							// Translate
-					0x08000000								// Length
-				)
+          DWordMemory ( // 32-bit BAR Windows
+            ResourceProducer, PosDecode,
+            MinFixed, MaxFixed,
+            Cacheable, ReadWrite,
+            0x00000000,               // Granularity
+            0x50000000,               // Min Base Address
+            0x57FFFFFF,               // Max Base Address
+            0x00000000,               // Translate
+            0x08000000                // Length
+          )
 
-				QWordMemory ( // 64-bit BAR Windows
-					ResourceProducer, PosDecode,
-					MinFixed, MaxFixed,
-					Cacheable, ReadWrite,
-					0x00000000, 							// Granularity
-					0x4000000000, 							// Min Base Address
-					0x40FFFFFFFF, 							// Max Base Address
-					0x00000000, 							// Translate
-					0x100000000								// Length
-				)
+          QWordMemory ( // 64-bit BAR Windows
+            ResourceProducer, PosDecode,
+            MinFixed, MaxFixed,
+            Cacheable, ReadWrite,
+            0x00000000,               // Granularity
+            0x4000000000,             // Min Base Address
+            0x40FFFFFFFF,             // Max Base Address
+            0x00000000,               // Translate
+            0x100000000               // Length
+          )
 
-				DWordIo ( // IO window
-					ResourceProducer,
-					MinFixed,
-					MaxFixed,
-					PosDecode,
-					EntireRange,
-					0x00000000, 							// Granularity
-					0x00000000, 							// Min Base Address
-					0x007fffff, 							// Max Base Address
-					0x5f800000, 							// Translate
-					0x00800000,  							// Length
-					,,,TypeTranslation
-				)
-			}) // Name(RBUF)
+          DWordIo ( // IO window
+            ResourceProducer,
+            MinFixed,
+            MaxFixed,
+            PosDecode,
+            EntireRange,
+            0x00000000,               // Granularity
+            0x00000000,               // Min Base Address
+            0x007fffff,               // Max Base Address
+            0x5f800000,               // Translate
+            0x00800000,               // Length
+            ,,,TypeTranslation
+          )
+        }) // Name(RBUF)
 
-			Return (RBUF)
-		} // Method(_CRS)
+        Return (RBUF)
+      } // Method(_CRS)
 
-		//
-		// OS Control Handoff
-		//
-		Name(SUPP, Zero) // PCI _OSC Support Field value
-		Name(CTRL, Zero) // PCI _OSC Control Field value
+      //
+      // OS Control Handoff
+      //
+      Name(SUPP, Zero) // PCI _OSC Support Field value
+      Name(CTRL, Zero) // PCI _OSC Control Field value
 
-		/*
-	  See [1] 6.2.10, [2] 4.5
-		*/
-		Method(_OSC,4) {
-			// Check for proper UUID
-			If(LEqual(Arg0,ToUUID("33DB4D5B-1FF7-401C-9657-7441C03DD766"))) {
-				// Create DWord-adressable fields from the Capabilities Buffer
-				CreateDWordField(Arg3,0,CDW1)
-				CreateDWordField(Arg3,4,CDW2)
-				CreateDWordField(Arg3,8,CDW3)
+      /*
+      See [1] 6.2.10, [2] 4.5
+      */
+      Method(_OSC,4) {
+        // Check for proper UUID
+        If(LEqual(Arg0,ToUUID("33DB4D5B-1FF7-401C-9657-7441C03DD766"))) {
+          // Create DWord-adressable fields from the Capabilities Buffer
+          CreateDWordField(Arg3,0,CDW1)
+          CreateDWordField(Arg3,4,CDW2)
+          CreateDWordField(Arg3,8,CDW3)
 
-				// Save Capabilities DWord2 & 3
-				Store(CDW2,SUPP)
-				Store(CDW3,CTRL)
+          // Save Capabilities DWord2 & 3
+          Store(CDW2,SUPP)
+          Store(CDW3,CTRL)
 
-				// Only allow native hot plug control if OS supports:
-				// * ASPM
-				// * Clock PM
-				// * MSI/MSI-X
-				If(LNotEqual(And(SUPP, 0x16), 0x16)) {
-					And(CTRL,0x1E,CTRL) // Mask bit 0 (and undefined bits)
-				}
+          // Only allow native hot plug control if OS supports:
+          // * ASPM
+          // * Clock PM
+          // * MSI/MSI-X
+          If(LNotEqual(And(SUPP, 0x16), 0x16)) {
+            And(CTRL,0x1E,CTRL) // Mask bit 0 (and undefined bits)
+          }
 
-				// Always allow native PME, AER (no dependencies)
+          // Always allow native PME, AER (no dependencies)
 
-				// Never allow SHPC (no SHPC controller in this system)
-				And(CTRL,0x1D,CTRL)
+          // Never allow SHPC (no SHPC controller in this system)
+          And(CTRL,0x1D,CTRL)
 
 #if 0
-				If(LNot(And(CDW1,1))) {		// Query flag clear?
-					// Disable GPEs for features granted native control.
-					If(And(CTRL,0x01)) {	// Hot plug control granted?
-						Store(0,HPCE)		// clear the hot plug SCI enable bit
-						Store(1,HPCS)		// clear the hot plug SCI status bit
-					}
-					If(And(CTRL,0x04)) {	// PME control granted?
-						Store(0,PMCE)		// clear the PME SCI enable bit
-						Store(1,PMCS)		// clear the PME SCI status bit
-					}
-					If(And(CTRL,0x10)) {	// OS restoring PCIe cap structure?
-						// Set status to not restore PCIe cap structure
-						// upon resume from S3
-						Store(1,S3CR)
-					}
-				}
+          If(LNot(And(CDW1,1))) {    // Query flag clear?
+            // Disable GPEs for features granted native control.
+            If(And(CTRL,0x01)) {     // Hot plug control granted?
+              Store(0,HPCE)          // clear the hot plug SCI enable bit
+              Store(1,HPCS)          // clear the hot plug SCI status bit
+            }
+            If(And(CTRL,0x04)) {     // PME control granted?
+              Store(0,PMCE)          // clear the PME SCI enable bit
+              Store(1,PMCS)          // clear the PME SCI status bit
+            }
+            If(And(CTRL,0x10)) {     // OS restoring PCIe cap structure?
+              // Set status to not restore PCIe cap structure
+              // upon resume from S3
+              Store(1,S3CR)
+            }
+          }
 #endif
 
-				If(LNotEqual(Arg1,One)) {	// Unknown revision
-					Or(CDW1,0x08,CDW1)
-				}
+          If(LNotEqual(Arg1,One)) {  // Unknown revision
+            Or(CDW1,0x08,CDW1)
+          }
 
-				If(LNotEqual(CDW3,CTRL)) {	// Capabilities bits were masked
-					Or(CDW1,0x10,CDW1)
-				}
-				// Update DWORD3 in the buffer
-				Store(CTRL,CDW3)
-				Return(Arg3)
-			} Else {
-				Or(CDW1,4,CDW1) // Unrecognized UUID
-				Return(Arg3)
-			}
-		} // End _OSC
+          If(LNotEqual(CDW3,CTRL)) {  // Capabilities bits were masked
+            Or(CDW1,0x10,CDW1)
+          }
+          // Update DWORD3 in the buffer
+          Store(CTRL,CDW3)
+          Return(Arg3)
+        } Else {
+          Or(CDW1,4,CDW1) // Unrecognized UUID
+          Return(Arg3)
+        }
+      } // End _OSC
     } // PCI0
   }
 }
-- 
2.25.1


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

* [PATCH edk2-platforms 2/3] Platform/ARM/JunoPkg: Use MdePkg PcdPciExpressBaseAddress for ECAM addr
  2022-03-05  4:19 [PATCH edk2-platforms 0/3] JunoPkg: Fix AcpiSsdtRootPci.asl to use spaces and reserve ECAM area Rebecca Cran
  2022-03-05  4:19 ` [PATCH edk2-platforms 1/3] Platform/ARM/JunoPkg: Convert AcpiSsdtRootPci.asl from tabs to spaces Rebecca Cran
@ 2022-03-05  4:19 ` Rebecca Cran
  2022-03-17  9:54   ` Sami Mujawar
  2022-03-05  4:19 ` [PATCH edk2-platforms 3/3] Platform/ARM/JunoPkg: Reserve the ECAM area in ACPI with RES0 device Rebecca Cran
  2022-03-15  2:44 ` [PATCH edk2-platforms 0/3] JunoPkg: Fix AcpiSsdtRootPci.asl to use spaces and reserve ECAM area Rebecca Cran
  3 siblings, 1 reply; 10+ messages in thread
From: Rebecca Cran @ 2022-03-05  4:19 UTC (permalink / raw)
  To: devel, Ard Biesheuvel, Thomas Abraham, Sami Mujawar; +Cc: Rebecca Cran

Instead of using a custom Pcd for the ECAM address
(gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceBaseAddress),
use the Pcd from MdePkg.

Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
---
 Platform/ARM/JunoPkg/ArmJuno.dec                                                              | 4 ++--
 Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf | 2 +-
 Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf                                        | 2 +-
 Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoLib.inf                                        | 2 +-
 Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/JunoPciHostBridgeLib.inf                    | 2 +-
 Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/XPressRich3.h                               | 2 +-
 Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c      | 2 +-
 Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/AcpiTables.c                                          | 2 +-
 Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c                                          | 4 ++--
 9 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/Platform/ARM/JunoPkg/ArmJuno.dec b/Platform/ARM/JunoPkg/ArmJuno.dec
index 37ea6857366f..b6437d6fe98c 100644
--- a/Platform/ARM/JunoPkg/ArmJuno.dec
+++ b/Platform/ARM/JunoPkg/ArmJuno.dec
@@ -34,8 +34,8 @@ [PcdsFeatureFlag.common]
 [PcdsFixedAtBuild.common]
   gArmJunoTokenSpaceGuid.PcdPcieControlBaseAddress|0x7FF20000|UINT64|0x0000000B
   gArmJunoTokenSpaceGuid.PcdPcieRootPortBaseAddress|0x7FF30000|UINT64|0x0000000C
-  gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceBaseAddress|0x40000000|UINT64|0x00000011
-  gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceSize|0x10000000|UINT64|0x00000012
+  gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceSize|0x10000000|UINT64|0x00000011
+  gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceLimit|0x4FFFFFFF|UINT64|0x00000012
 
   gArmJunoTokenSpaceGuid.PcdSynopsysUsbOhciBaseAddress|0x7FFB0000|UINT32|0x00000004
   gArmJunoTokenSpaceGuid.PcdSynopsysUsbEhciBaseAddress|0x7FFC0000|UINT32|0x00000005
diff --git a/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf b/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
index 00be2c435bd6..7ca134d6674b 100644
--- a/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
+++ b/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
@@ -46,7 +46,7 @@ [Protocols]
 
 [FixedPcd]
   # PCI Root complex specific PCDs
-  gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceBaseAddress
+  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
   gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceSize
 
   ## PL011 Serial Debug UART
diff --git a/Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf b/Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf
index d016967c3c37..c35984c172e1 100644
--- a/Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf
+++ b/Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf
@@ -67,7 +67,7 @@ [FixedPcd]
   gArmJunoTokenSpaceGuid.PcdJunoFdtDevicePath
 
   # PCI Root complex specific PCDs
-  gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceBaseAddress
+  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
   gArmTokenSpaceGuid.PcdPciBusMin
   gArmTokenSpaceGuid.PcdPciBusMax
 
diff --git a/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoLib.inf b/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoLib.inf
index 145663c2fa28..fb80f10a9409 100644
--- a/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoLib.inf
+++ b/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoLib.inf
@@ -45,7 +45,7 @@ [FixedPcd]
   gArmTokenSpaceGuid.PcdArmPrimaryCoreMask
 
   gArmJunoTokenSpaceGuid.PcdPcieControlBaseAddress
-  gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceBaseAddress
+  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
   gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceSize
 
   # Framebuffer Memory
diff --git a/Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/JunoPciHostBridgeLib.inf b/Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/JunoPciHostBridgeLib.inf
index f448803fda7d..784618ffa013 100644
--- a/Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/JunoPciHostBridgeLib.inf
+++ b/Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/JunoPciHostBridgeLib.inf
@@ -62,7 +62,7 @@ [FixedPcd]
 
   gArmJunoTokenSpaceGuid.PcdPcieControlBaseAddress
   gArmJunoTokenSpaceGuid.PcdPcieRootPortBaseAddress
-  gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceBaseAddress
+  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
   gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceSize
 
 [Protocols]
diff --git a/Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/XPressRich3.h b/Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/XPressRich3.h
index 420bdda7534b..78889c1b1196 100644
--- a/Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/XPressRich3.h
+++ b/Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/XPressRich3.h
@@ -13,7 +13,7 @@
 #include <Protocol/CpuIo2.h>
 #include <Library/PcdLib.h>
 
-#define PCI_ECAM_BASE       FixedPcdGet64 (PcdPciConfigurationSpaceBaseAddress)
+#define PCI_ECAM_BASE       FixedPcdGet64 (PcdPciExpressBaseAddress)
 #define PCI_ECAM_SIZE       FixedPcdGet64 (PcdPciConfigurationSpaceSize)
 #define PCI_IO_BASE         FixedPcdGet64 (PcdPciIoTranslation)
 #define PCI_IO_SIZE         FixedPcdGet64 (PcdPciIoSize)
diff --git a/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c b/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
index 80a98a10d869..3420b6eba66a 100644
--- a/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
+++ b/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
@@ -230,7 +230,7 @@ EDKII_PLATFORM_REPOSITORY_INFO ArmJunoPlatformRepositoryInfo = {
   // PCI Configuration Space Info
   {
     // The physical base address for the PCI segment
-    FixedPcdGet64 (PcdPciConfigurationSpaceBaseAddress),
+    FixedPcdGet64 (gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress),
     // The PCI segment group number
     0,
     // The start bus number
diff --git a/Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/AcpiTables.c b/Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/AcpiTables.c
index aaa493a9284b..870dfa5f7ef3 100644
--- a/Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/AcpiTables.c
+++ b/Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/AcpiTables.c
@@ -30,7 +30,7 @@ MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ACCESS_TABLE mAcpiMcfgTable = {
         ),
         0, // Reserved
     }, {
-        FixedPcdGet32 (PcdPciConfigurationSpaceBaseAddress),
+        FixedPcdGet32 (PcdPciExpressBaseAddress),
         0, // PciSegmentGroupNumber
         FixedPcdGet32 (PcdPciBusMin),
         FixedPcdGet32 (PcdPciBusMax),
diff --git a/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c b/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c
index 990a1664e496..8ac2e3273a4a 100644
--- a/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c
+++ b/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c
@@ -118,8 +118,8 @@ ArmPlatformGetVirtualMemoryMap (
   //
   // PCI Configuration Space
   //
-  VirtualMemoryTable[++Index].PhysicalBase  = PcdGet64 (PcdPciConfigurationSpaceBaseAddress);
-  VirtualMemoryTable[Index].VirtualBase     = PcdGet64 (PcdPciConfigurationSpaceBaseAddress);
+  VirtualMemoryTable[++Index].PhysicalBase  = PcdGet64 (PcdPciExpressBaseAddress);
+  VirtualMemoryTable[Index].VirtualBase     = PcdGet64 (PcdPciExpressBaseAddress);
   VirtualMemoryTable[Index].Length          = PcdGet64 (PcdPciConfigurationSpaceSize);
   VirtualMemoryTable[Index].Attributes      = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
 
-- 
2.25.1


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

* [PATCH edk2-platforms 3/3] Platform/ARM/JunoPkg: Reserve the ECAM area in ACPI with RES0 device
  2022-03-05  4:19 [PATCH edk2-platforms 0/3] JunoPkg: Fix AcpiSsdtRootPci.asl to use spaces and reserve ECAM area Rebecca Cran
  2022-03-05  4:19 ` [PATCH edk2-platforms 1/3] Platform/ARM/JunoPkg: Convert AcpiSsdtRootPci.asl from tabs to spaces Rebecca Cran
  2022-03-05  4:19 ` [PATCH edk2-platforms 2/3] Platform/ARM/JunoPkg: Use MdePkg PcdPciExpressBaseAddress for ECAM addr Rebecca Cran
@ 2022-03-05  4:19 ` Rebecca Cran
  2022-03-17  9:55   ` Sami Mujawar
  2022-03-15  2:44 ` [PATCH edk2-platforms 0/3] JunoPkg: Fix AcpiSsdtRootPci.asl to use spaces and reserve ECAM area Rebecca Cran
  3 siblings, 1 reply; 10+ messages in thread
From: Rebecca Cran @ 2022-03-05  4:19 UTC (permalink / raw)
  To: devel, Ard Biesheuvel, Thomas Abraham, Sami Mujawar; +Cc: Rebecca Cran

Add a RES0 device to the SSDT to reserve the PCI ECAM area.

This fixes the warning that Linux prints:

acpi PNP0A08:00: [Firmware Bug]: ECAM area [mem 0x40000000-0x4fffffff]
not reserved in ACPI namespace

Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
---
 Platform/ARM/JunoPkg/AcpiTables/AcpiTables.inf      |  4 ++++
 Platform/ARM/JunoPkg/AcpiTables/AcpiSsdtRootPci.asl | 13 +++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/Platform/ARM/JunoPkg/AcpiTables/AcpiTables.inf b/Platform/ARM/JunoPkg/AcpiTables/AcpiTables.inf
index f140febc4ad4..9a76475765f0 100644
--- a/Platform/ARM/JunoPkg/AcpiTables/AcpiTables.inf
+++ b/Platform/ARM/JunoPkg/AcpiTables/AcpiTables.inf
@@ -45,6 +45,10 @@ [FixedPcd]
   gArmTokenSpaceGuid.PcdGenericWatchdogControlBase
   gArmTokenSpaceGuid.PcdGenericWatchdogRefreshBase
 
+  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
+  gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceSize
+  gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceLimit
+
   #
   # PL011 UART Settings for Serial Port Console Redirection
   #
diff --git a/Platform/ARM/JunoPkg/AcpiTables/AcpiSsdtRootPci.asl b/Platform/ARM/JunoPkg/AcpiTables/AcpiSsdtRootPci.asl
index 317b621e013e..e60fc42a3340 100644
--- a/Platform/ARM/JunoPkg/AcpiTables/AcpiSsdtRootPci.asl
+++ b/Platform/ARM/JunoPkg/AcpiTables/AcpiSsdtRootPci.asl
@@ -140,6 +140,19 @@ DefinitionBlock("SsdtPci.aml", "SSDT", 1, "ARMLTD", "ARM-JUNO", EFI_ACPI_ARM_OEM
         Return (RBUF)
       } // Method(_CRS)
 
+      Device (RES0) {
+        Name (_HID, "PNP0C02" /* PNP Motherboard Resources */)  // _HID: Hardware ID
+        Name (_CRS, ResourceTemplate () {                       // _CRS: Current Resource Settings
+           QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, NonCacheable, ReadWrite,
+           0x0000000000000000,                                  // Granularity
+           FixedPcdGet64 (PcdPciExpressBaseAddress),            // Range Minimum
+           FixedPcdGet64 (PcdPciConfigurationSpaceLimit),       // Range Maximum
+           0x0000000000000000,                                  // Translation Offset
+           FixedPcdGet64 (PcdPciConfigurationSpaceSize),        // Length
+           ,, , AddressRangeMemory, TypeStatic)
+        })
+      }
+
       //
       // OS Control Handoff
       //
-- 
2.25.1


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

* Re: [PATCH edk2-platforms 0/3] JunoPkg: Fix AcpiSsdtRootPci.asl to use spaces and reserve ECAM area
  2022-03-05  4:19 [PATCH edk2-platforms 0/3] JunoPkg: Fix AcpiSsdtRootPci.asl to use spaces and reserve ECAM area Rebecca Cran
                   ` (2 preceding siblings ...)
  2022-03-05  4:19 ` [PATCH edk2-platforms 3/3] Platform/ARM/JunoPkg: Reserve the ECAM area in ACPI with RES0 device Rebecca Cran
@ 2022-03-15  2:44 ` Rebecca Cran
  3 siblings, 0 replies; 10+ messages in thread
From: Rebecca Cran @ 2022-03-15  2:44 UTC (permalink / raw)
  To: devel, Ard Biesheuvel, Thomas Abraham, Sami Mujawar

Could someone review this please?


Thanks.

Rebecca Cran


On 3/4/22 21:19, Rebecca Cran wrote:
> I noticed Linux reports a firmware bug with the current Juno ACPI
> tables. These patches fix it by reserving the ECAM area with a RES0
> device, while also converting AcpiSsdtRootPci.asl from tabs to spaces
> and using the standard Pcd from MdePkg for the ECAM base address.
>
> Rebecca Cran (3):
>    Platform/ARM/JunoPkg: Convert AcpiSsdtRootPci.asl from tabs to spaces
>    Platform/ARM/JunoPkg: Use MdePkg PcdPciExpressBaseAddress for ECAM
>      addr
>    Platform/ARM/JunoPkg: Reserve the ECAM area in ACPI with RES0 device
>
>   Platform/ARM/JunoPkg/ArmJuno.dec                                                              |   4 +-
>   Platform/ARM/JunoPkg/AcpiTables/AcpiTables.inf                                                |   4 +
>   Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf |   2 +-
>   Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf                                        |   2 +-
>   Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoLib.inf                                        |   2 +-
>   Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/JunoPciHostBridgeLib.inf                    |   2 +-
>   Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/XPressRich3.h                               |   2 +-
>   Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c      |   2 +-
>   Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/AcpiTables.c                                          |   2 +-
>   Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c                                          |   4 +-
>   Platform/ARM/JunoPkg/AcpiTables/AcpiSsdtRootPci.asl                                           | 301 ++++++++++----------
>   11 files changed, 172 insertions(+), 155 deletions(-)
>

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

* Re: [PATCH edk2-platforms 1/3] Platform/ARM/JunoPkg: Convert AcpiSsdtRootPci.asl from tabs to spaces
  2022-03-05  4:19 ` [PATCH edk2-platforms 1/3] Platform/ARM/JunoPkg: Convert AcpiSsdtRootPci.asl from tabs to spaces Rebecca Cran
@ 2022-03-16 15:40   ` Sami Mujawar
  0 siblings, 0 replies; 10+ messages in thread
From: Sami Mujawar @ 2022-03-16 15:40 UTC (permalink / raw)
  To: Rebecca Cran, devel, Ard Biesheuvel, Thomas Abraham, nd

Hi Rebecca,

Thank you for this patch. These change look good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 05/03/2022 04:19 AM, Rebecca Cran wrote:
> Other .asl files in Platform/ARM/JunoPkg/AcpiTables use spaces, while
> AcpiSsdtRootPci.asl uses tabs. To be consistent, convert it to spaces.
>
> Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
> ---
>   Platform/ARM/JunoPkg/AcpiTables/AcpiSsdtRootPci.asl | 288 ++++++++++----------
>   1 file changed, 144 insertions(+), 144 deletions(-)
>
> diff --git a/Platform/ARM/JunoPkg/AcpiTables/AcpiSsdtRootPci.asl b/Platform/ARM/JunoPkg/AcpiTables/AcpiSsdtRootPci.asl
> index ba41a9586555..317b621e013e 100644
> --- a/Platform/ARM/JunoPkg/AcpiTables/AcpiSsdtRootPci.asl
> +++ b/Platform/ARM/JunoPkg/AcpiTables/AcpiSsdtRootPci.asl
> @@ -28,24 +28,24 @@
>     interrupt type as PCI defaults (Level Triggered, Active Low) are not
>     compatible with GICv2.
>   */
> -#define LNK_DEVICE(Unique_Id, Link_Name, irq)                                                        \
> -     Device(Link_Name) {                                                                     \
> -         Name(_HID, EISAID("PNP0C0F"))                                                       \
> -         Name(_UID, Unique_Id)                                                               \
> -         Name(_PRS, ResourceTemplate() {                                                     \
> -             Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive) { irq }               \
> -         })                                                                                  \
> -         Method (_CRS, 0) { Return (_PRS) }                                                  \
> -         Method (_SRS, 1) { }                                                                \
> -         Method (_DIS) { }                                                                   \
> -     }
> +#define LNK_DEVICE(Unique_Id, Link_Name, irq)                                  \
> +  Device(Link_Name) {                                                          \
> +      Name(_HID, EISAID("PNP0C0F"))                                            \
> +      Name(_UID, Unique_Id)                                                    \
> +      Name(_PRS, ResourceTemplate() {                                          \
> +          Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive) { irq }    \
> +      })                                                                       \
> +      Method (_CRS, 0) { Return (_PRS) }                                       \
> +      Method (_SRS, 1) { }                                                     \
> +      Method (_DIS) { }                                                        \
> +  }
>
> -#define PRT_ENTRY(Address, Pin, Link)                                                                  \
> +#define PRT_ENTRY(Address, Pin, Link)                                                             \
>           Package (4) {                                                                             \
>               Address,    /* uses the same format as _ADR */                                        \
>               Pin,        /* The PCI pin number of the device (0-INTA, 1-INTB, 2-INTC, 3-INTD). */  \
> -            Link,       /* Interrupt allocated via Link device. */                                     \
> -            Zero        /* global system interrupt number (no used) */                                 \
> +            Link,       /* Interrupt allocated via Link device. */                                \
> +            Zero        /* global system interrupt number (no used) */                            \
>             }
>
>   /*
> @@ -59,155 +59,155 @@
>
>   DefinitionBlock("SsdtPci.aml", "SSDT", 1, "ARMLTD", "ARM-JUNO", EFI_ACPI_ARM_OEM_REVISION) {
>     Scope(_SB) {
> -     //
> -     // PCI Root Complex
> -     //
> -     LNK_DEVICE(1, LNKA, 168)
> -     LNK_DEVICE(2, LNKB, 169)
> -     LNK_DEVICE(3, LNKC, 170)
> -     LNK_DEVICE(4, LNKD, 171)
> +    //
> +    // PCI Root Complex
> +    //
> +    LNK_DEVICE(1, LNKA, 168)
> +    LNK_DEVICE(2, LNKB, 169)
> +    LNK_DEVICE(3, LNKC, 170)
> +    LNK_DEVICE(4, LNKD, 171)
>
> -     Device(PCI0)
> +    Device(PCI0)
>       {
> -             Name(_HID, EISAID("PNP0A08")) // PCI Express Root Bridge
> -             Name(_CID, EISAID("PNP0A03")) // Compatible PCI Root Bridge
> -             Name(_SEG, Zero) // PCI Segment Group number
> -             Name(_BBN, Zero) // PCI Base Bus Number
> -             Name(_CCA, 1)    // Initially mark the PCI coherent (for JunoR1)
> +      Name(_HID, EISAID("PNP0A08")) // PCI Express Root Bridge
> +      Name(_CID, EISAID("PNP0A03")) // Compatible PCI Root Bridge
> +      Name(_SEG, Zero) // PCI Segment Group number
> +      Name(_BBN, Zero) // PCI Base Bus Number
> +      Name(_CCA, 1)    // Initially mark the PCI coherent (for JunoR1)
>
> -        // Root Complex 0
> -        Device (RP0) {
> -            Name(_ADR, 0xF0000000)    // Dev 0, Func 0
> -        }
> +      // Root Complex 0
> +      Device (RP0) {
> +        Name(_ADR, 0xF0000000)    // Dev 0, Func 0
> +      }
>
> -             // PCI Routing Table
> -             Name(_PRT, Package() {
> -                     ROOT_PRT_ENTRY(0, LNKA),   // INTA
> -                     ROOT_PRT_ENTRY(1, LNKB),   // INTB
> -                     ROOT_PRT_ENTRY(2, LNKC),   // INTC
> -                     ROOT_PRT_ENTRY(3, LNKD),   // INTD
> -             })
> -        // Root complex resources
> -             Method (_CRS, 0, Serialized) {
> -                     Name (RBUF, ResourceTemplate () {
> -                             WordBusNumber ( // Bus numbers assigned to this root
> -                                     ResourceProducer,
> -                                     MinFixed, MaxFixed, PosDecode,
> -                                     0,   // AddressGranularity
> -                                     0,   // AddressMinimum - Minimum Bus Number
> -                                     255, // AddressMaximum - Maximum Bus Number
> -                                     0,   // AddressTranslation - Set to 0
> -                                     256  // RangeLength - Number of Busses
> -                             )
> +      // PCI Routing Table
> +      Name(_PRT, Package() {
> +        ROOT_PRT_ENTRY(0, LNKA),   // INTA
> +        ROOT_PRT_ENTRY(1, LNKB),   // INTB
> +        ROOT_PRT_ENTRY(2, LNKC),   // INTC
> +        ROOT_PRT_ENTRY(3, LNKD),   // INTD
> +      })
> +      // Root complex resources
> +      Method (_CRS, 0, Serialized) {
> +        Name (RBUF, ResourceTemplate () {
> +          WordBusNumber ( // Bus numbers assigned to this root
> +            ResourceProducer,
> +            MinFixed, MaxFixed, PosDecode,
> +            0,   // AddressGranularity
> +            0,   // AddressMinimum - Minimum Bus Number
> +            255, // AddressMaximum - Maximum Bus Number
> +            0,   // AddressTranslation - Set to 0
> +            256  // RangeLength - Number of Busses
> +          )
>
> -                             DWordMemory ( // 32-bit BAR Windows
> -                                     ResourceProducer, PosDecode,
> -                                     MinFixed, MaxFixed,
> -                                     Cacheable, ReadWrite,
> -                                     0x00000000,                                                     // Granularity
> -                                     0x50000000,                                                     // Min Base Address
> -                                     0x57FFFFFF,                                                     // Max Base Address
> -                                     0x00000000,                                                     // Translate
> -                                     0x08000000                                                              // Length
> -                             )
> +          DWordMemory ( // 32-bit BAR Windows
> +            ResourceProducer, PosDecode,
> +            MinFixed, MaxFixed,
> +            Cacheable, ReadWrite,
> +            0x00000000,               // Granularity
> +            0x50000000,               // Min Base Address
> +            0x57FFFFFF,               // Max Base Address
> +            0x00000000,               // Translate
> +            0x08000000                // Length
> +          )
>
> -                             QWordMemory ( // 64-bit BAR Windows
> -                                     ResourceProducer, PosDecode,
> -                                     MinFixed, MaxFixed,
> -                                     Cacheable, ReadWrite,
> -                                     0x00000000,                                                     // Granularity
> -                                     0x4000000000,                                                   // Min Base Address
> -                                     0x40FFFFFFFF,                                                   // Max Base Address
> -                                     0x00000000,                                                     // Translate
> -                                     0x100000000                                                             // Length
> -                             )
> +          QWordMemory ( // 64-bit BAR Windows
> +            ResourceProducer, PosDecode,
> +            MinFixed, MaxFixed,
> +            Cacheable, ReadWrite,
> +            0x00000000,               // Granularity
> +            0x4000000000,             // Min Base Address
> +            0x40FFFFFFFF,             // Max Base Address
> +            0x00000000,               // Translate
> +            0x100000000               // Length
> +          )
>
> -                             DWordIo ( // IO window
> -                                     ResourceProducer,
> -                                     MinFixed,
> -                                     MaxFixed,
> -                                     PosDecode,
> -                                     EntireRange,
> -                                     0x00000000,                                                     // Granularity
> -                                     0x00000000,                                                     // Min Base Address
> -                                     0x007fffff,                                                     // Max Base Address
> -                                     0x5f800000,                                                     // Translate
> -                                     0x00800000,                                                     // Length
> -                                     ,,,TypeTranslation
> -                             )
> -                     }) // Name(RBUF)
> +          DWordIo ( // IO window
> +            ResourceProducer,
> +            MinFixed,
> +            MaxFixed,
> +            PosDecode,
> +            EntireRange,
> +            0x00000000,               // Granularity
> +            0x00000000,               // Min Base Address
> +            0x007fffff,               // Max Base Address
> +            0x5f800000,               // Translate
> +            0x00800000,               // Length
> +            ,,,TypeTranslation
> +          )
> +        }) // Name(RBUF)
>
> -                     Return (RBUF)
> -             } // Method(_CRS)
> +        Return (RBUF)
> +      } // Method(_CRS)
>
> -             //
> -             // OS Control Handoff
> -             //
> -             Name(SUPP, Zero) // PCI _OSC Support Field value
> -             Name(CTRL, Zero) // PCI _OSC Control Field value
> +      //
> +      // OS Control Handoff
> +      //
> +      Name(SUPP, Zero) // PCI _OSC Support Field value
> +      Name(CTRL, Zero) // PCI _OSC Control Field value
>
> -             /*
> -       See [1] 6.2.10, [2] 4.5
> -             */
> -             Method(_OSC,4) {
> -                     // Check for proper UUID
> -                     If(LEqual(Arg0,ToUUID("33DB4D5B-1FF7-401C-9657-7441C03DD766"))) {
> -                             // Create DWord-adressable fields from the Capabilities Buffer
> -                             CreateDWordField(Arg3,0,CDW1)
> -                             CreateDWordField(Arg3,4,CDW2)
> -                             CreateDWordField(Arg3,8,CDW3)
> +      /*
> +      See [1] 6.2.10, [2] 4.5
> +      */
> +      Method(_OSC,4) {
> +        // Check for proper UUID
> +        If(LEqual(Arg0,ToUUID("33DB4D5B-1FF7-401C-9657-7441C03DD766"))) {
> +          // Create DWord-adressable fields from the Capabilities Buffer
> +          CreateDWordField(Arg3,0,CDW1)
> +          CreateDWordField(Arg3,4,CDW2)
> +          CreateDWordField(Arg3,8,CDW3)
>
> -                             // Save Capabilities DWord2 & 3
> -                             Store(CDW2,SUPP)
> -                             Store(CDW3,CTRL)
> +          // Save Capabilities DWord2 & 3
> +          Store(CDW2,SUPP)
> +          Store(CDW3,CTRL)
>
> -                             // Only allow native hot plug control if OS supports:
> -                             // * ASPM
> -                             // * Clock PM
> -                             // * MSI/MSI-X
> -                             If(LNotEqual(And(SUPP, 0x16), 0x16)) {
> -                                     And(CTRL,0x1E,CTRL) // Mask bit 0 (and undefined bits)
> -                             }
> +          // Only allow native hot plug control if OS supports:
> +          // * ASPM
> +          // * Clock PM
> +          // * MSI/MSI-X
> +          If(LNotEqual(And(SUPP, 0x16), 0x16)) {
> +            And(CTRL,0x1E,CTRL) // Mask bit 0 (and undefined bits)
> +          }
>
> -                             // Always allow native PME, AER (no dependencies)
> +          // Always allow native PME, AER (no dependencies)
>
> -                             // Never allow SHPC (no SHPC controller in this system)
> -                             And(CTRL,0x1D,CTRL)
> +          // Never allow SHPC (no SHPC controller in this system)
> +          And(CTRL,0x1D,CTRL)
>
>   #if 0
> -                             If(LNot(And(CDW1,1))) {         // Query flag clear?
> -                                     // Disable GPEs for features granted native control.
> -                                     If(And(CTRL,0x01)) {    // Hot plug control granted?
> -                                             Store(0,HPCE)           // clear the hot plug SCI enable bit
> -                                             Store(1,HPCS)           // clear the hot plug SCI status bit
> -                                     }
> -                                     If(And(CTRL,0x04)) {    // PME control granted?
> -                                             Store(0,PMCE)           // clear the PME SCI enable bit
> -                                             Store(1,PMCS)           // clear the PME SCI status bit
> -                                     }
> -                                     If(And(CTRL,0x10)) {    // OS restoring PCIe cap structure?
> -                                             // Set status to not restore PCIe cap structure
> -                                             // upon resume from S3
> -                                             Store(1,S3CR)
> -                                     }
> -                             }
> +          If(LNot(And(CDW1,1))) {    // Query flag clear?
> +            // Disable GPEs for features granted native control.
> +            If(And(CTRL,0x01)) {     // Hot plug control granted?
> +              Store(0,HPCE)          // clear the hot plug SCI enable bit
> +              Store(1,HPCS)          // clear the hot plug SCI status bit
> +            }
> +            If(And(CTRL,0x04)) {     // PME control granted?
> +              Store(0,PMCE)          // clear the PME SCI enable bit
> +              Store(1,PMCS)          // clear the PME SCI status bit
> +            }
> +            If(And(CTRL,0x10)) {     // OS restoring PCIe cap structure?
> +              // Set status to not restore PCIe cap structure
> +              // upon resume from S3
> +              Store(1,S3CR)
> +            }
> +          }
>   #endif
>
> -                             If(LNotEqual(Arg1,One)) {       // Unknown revision
> -                                     Or(CDW1,0x08,CDW1)
> -                             }
> +          If(LNotEqual(Arg1,One)) {  // Unknown revision
> +            Or(CDW1,0x08,CDW1)
> +          }
>
> -                             If(LNotEqual(CDW3,CTRL)) {      // Capabilities bits were masked
> -                                     Or(CDW1,0x10,CDW1)
> -                             }
> -                             // Update DWORD3 in the buffer
> -                             Store(CTRL,CDW3)
> -                             Return(Arg3)
> -                     } Else {
> -                             Or(CDW1,4,CDW1) // Unrecognized UUID
> -                             Return(Arg3)
> -                     }
> -             } // End _OSC
> +          If(LNotEqual(CDW3,CTRL)) {  // Capabilities bits were masked
> +            Or(CDW1,0x10,CDW1)
> +          }
> +          // Update DWORD3 in the buffer
> +          Store(CTRL,CDW3)
> +          Return(Arg3)
> +        } Else {
> +          Or(CDW1,4,CDW1) // Unrecognized UUID
> +          Return(Arg3)
> +        }
> +      } // End _OSC
>       } // PCI0
>     }
>   }

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

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

* Re: [PATCH edk2-platforms 2/3] Platform/ARM/JunoPkg: Use MdePkg PcdPciExpressBaseAddress for ECAM addr
  2022-03-05  4:19 ` [PATCH edk2-platforms 2/3] Platform/ARM/JunoPkg: Use MdePkg PcdPciExpressBaseAddress for ECAM addr Rebecca Cran
@ 2022-03-17  9:54   ` Sami Mujawar
  2022-03-19 19:56     ` Rebecca Cran
  0 siblings, 1 reply; 10+ messages in thread
From: Sami Mujawar @ 2022-03-17  9:54 UTC (permalink / raw)
  To: Rebecca Cran, devel, Ard Biesheuvel, Thomas Abraham, nd

Hi Rebecca,

I have one minor suggestion marked inline as [SAMI]. Otherwise these
changes look good to me.

With that changed,

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar


On 05/03/2022 04:19 AM, Rebecca Cran wrote:
> Instead of using a custom Pcd for the ECAM address
> (gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceBaseAddress),
> use the Pcd from MdePkg.
>
> Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
> ---
>   Platform/ARM/JunoPkg/ArmJuno.dec                                                              | 4 ++--
>   Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf | 2 +-
>   Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf                                        | 2 +-
>   Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoLib.inf                                        | 2 +-
>   Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/JunoPciHostBridgeLib.inf                    | 2 +-
>   Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/XPressRich3.h                               | 2 +-
>   Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c      | 2 +-
>   Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/AcpiTables.c                                          | 2 +-
>   Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c                                          | 4 ++--
>   9 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/Platform/ARM/JunoPkg/ArmJuno.dec b/Platform/ARM/JunoPkg/ArmJuno.dec
> index 37ea6857366f..b6437d6fe98c 100644
> --- a/Platform/ARM/JunoPkg/ArmJuno.dec
> +++ b/Platform/ARM/JunoPkg/ArmJuno.dec
> @@ -34,8 +34,8 @@ [PcdsFeatureFlag.common]
>   [PcdsFixedAtBuild.common]
>     gArmJunoTokenSpaceGuid.PcdPcieControlBaseAddress|0x7FF20000|UINT64|0x0000000B
>     gArmJunoTokenSpaceGuid.PcdPcieRootPortBaseAddress|0x7FF30000|UINT64|0x0000000C
> -  gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceBaseAddress|0x40000000|UINT64|0x00000011
> -  gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceSize|0x10000000|UINT64|0x00000012
> +  gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceSize|0x10000000|UINT64|0x00000011
> +  gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceLimit|0x4FFFFFFF|UINT64|0x00000012
>
>     gArmJunoTokenSpaceGuid.PcdSynopsysUsbOhciBaseAddress|0x7FFB0000|UINT32|0x00000004
>     gArmJunoTokenSpaceGuid.PcdSynopsysUsbEhciBaseAddress|0x7FFC0000|UINT32|0x00000005
> diff --git a/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf b/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
> index 00be2c435bd6..7ca134d6674b 100644
> --- a/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
> +++ b/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
> @@ -46,7 +46,7 @@ [Protocols]
>
>   [FixedPcd]
>     # PCI Root complex specific PCDs
> -  gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceBaseAddress
> +  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
>     gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceSize
>
>     ## PL011 Serial Debug UART
> diff --git a/Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf b/Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf
> index d016967c3c37..c35984c172e1 100644
> --- a/Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf
> +++ b/Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf
> @@ -67,7 +67,7 @@ [FixedPcd]
>     gArmJunoTokenSpaceGuid.PcdJunoFdtDevicePath
>
>     # PCI Root complex specific PCDs
> -  gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceBaseAddress
> +  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
>     gArmTokenSpaceGuid.PcdPciBusMin
>     gArmTokenSpaceGuid.PcdPciBusMax
>
> diff --git a/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoLib.inf b/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoLib.inf
> index 145663c2fa28..fb80f10a9409 100644
> --- a/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoLib.inf
> +++ b/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoLib.inf
> @@ -45,7 +45,7 @@ [FixedPcd]
>     gArmTokenSpaceGuid.PcdArmPrimaryCoreMask
>
>     gArmJunoTokenSpaceGuid.PcdPcieControlBaseAddress
> -  gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceBaseAddress
> +  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
>     gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceSize
>
>     # Framebuffer Memory
> diff --git a/Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/JunoPciHostBridgeLib.inf b/Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/JunoPciHostBridgeLib.inf
> index f448803fda7d..784618ffa013 100644
> --- a/Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/JunoPciHostBridgeLib.inf
> +++ b/Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/JunoPciHostBridgeLib.inf
> @@ -62,7 +62,7 @@ [FixedPcd]
>
>     gArmJunoTokenSpaceGuid.PcdPcieControlBaseAddress
>     gArmJunoTokenSpaceGuid.PcdPcieRootPortBaseAddress
> -  gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceBaseAddress
> +  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
>     gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceSize
>
>   [Protocols]
> diff --git a/Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/XPressRich3.h b/Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/XPressRich3.h
> index 420bdda7534b..78889c1b1196 100644
> --- a/Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/XPressRich3.h
> +++ b/Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/XPressRich3.h
> @@ -13,7 +13,7 @@
>   #include <Protocol/CpuIo2.h>
>   #include <Library/PcdLib.h>
>
> -#define PCI_ECAM_BASE       FixedPcdGet64 (PcdPciConfigurationSpaceBaseAddress)
> +#define PCI_ECAM_BASE       FixedPcdGet64 (PcdPciExpressBaseAddress)
>   #define PCI_ECAM_SIZE       FixedPcdGet64 (PcdPciConfigurationSpaceSize)
>   #define PCI_IO_BASE         FixedPcdGet64 (PcdPciIoTranslation)
>   #define PCI_IO_SIZE         FixedPcdGet64 (PcdPciIoSize)
> diff --git a/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c b/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
> index 80a98a10d869..3420b6eba66a 100644
> --- a/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
> +++ b/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
> @@ -230,7 +230,7 @@ EDKII_PLATFORM_REPOSITORY_INFO ArmJunoPlatformRepositoryInfo = {
>     // PCI Configuration Space Info
>     {
>       // The physical base address for the PCI segment
> -    FixedPcdGet64 (PcdPciConfigurationSpaceBaseAddress),
> +    FixedPcdGet64 (gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress),
[SAMI] I think this should just be FixedPcdGet64
(PcdPciExpressBaseAddress).
>       // The PCI segment group number
>       0,
>       // The start bus number
> diff --git a/Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/AcpiTables.c b/Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/AcpiTables.c
> index aaa493a9284b..870dfa5f7ef3 100644
> --- a/Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/AcpiTables.c
> +++ b/Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/AcpiTables.c
> @@ -30,7 +30,7 @@ MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ACCESS_TABLE mAcpiMcfgTable = {
>           ),
>           0, // Reserved
>       }, {
> -        FixedPcdGet32 (PcdPciConfigurationSpaceBaseAddress),
> +        FixedPcdGet32 (PcdPciExpressBaseAddress),
>           0, // PciSegmentGroupNumber
>           FixedPcdGet32 (PcdPciBusMin),
>           FixedPcdGet32 (PcdPciBusMax),
> diff --git a/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c b/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c
> index 990a1664e496..8ac2e3273a4a 100644
> --- a/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c
> +++ b/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c
> @@ -118,8 +118,8 @@ ArmPlatformGetVirtualMemoryMap (
>     //
>     // PCI Configuration Space
>     //
> -  VirtualMemoryTable[++Index].PhysicalBase  = PcdGet64 (PcdPciConfigurationSpaceBaseAddress);
> -  VirtualMemoryTable[Index].VirtualBase     = PcdGet64 (PcdPciConfigurationSpaceBaseAddress);
> +  VirtualMemoryTable[++Index].PhysicalBase  = PcdGet64 (PcdPciExpressBaseAddress);
> +  VirtualMemoryTable[Index].VirtualBase     = PcdGet64 (PcdPciExpressBaseAddress);
>     VirtualMemoryTable[Index].Length          = PcdGet64 (PcdPciConfigurationSpaceSize);
>     VirtualMemoryTable[Index].Attributes      = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
>

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

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

* Re: [PATCH edk2-platforms 3/3] Platform/ARM/JunoPkg: Reserve the ECAM area in ACPI with RES0 device
  2022-03-05  4:19 ` [PATCH edk2-platforms 3/3] Platform/ARM/JunoPkg: Reserve the ECAM area in ACPI with RES0 device Rebecca Cran
@ 2022-03-17  9:55   ` Sami Mujawar
  2022-03-19 19:56     ` Rebecca Cran
  0 siblings, 1 reply; 10+ messages in thread
From: Sami Mujawar @ 2022-03-17  9:55 UTC (permalink / raw)
  To: Rebecca Cran, devel, Ard Biesheuvel, Thomas Abraham, nd

Hi Rebecca,

Thank you for this patch. This change looks good to me.

I have a minor suggestion marked inline as [SAMI].

With that upated,

Tested-by: Sami Mujawar <sami.mujawar@arm.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar
On 05/03/2022 04:19 AM, Rebecca Cran wrote:
> Add a RES0 device to the SSDT to reserve the PCI ECAM area.
>
> This fixes the warning that Linux prints:
>
> acpi PNP0A08:00: [Firmware Bug]: ECAM area [mem 0x40000000-0x4fffffff]
> not reserved in ACPI namespace
[SAMI] I noticed that the "Firmware Bug" message is no longer seen, but
instead the following message is now printed
"system 00:00: [mem 0x40000000-0x4fffffff window] could not be reserved"

It appears this is a harmless message and the relevant discussion can be
seen at: https://lore.kernel.org/all/20210603141641.GA17284@lpieralisi/#t

I think it may be better to update the commit message to reference this
discussion and clarify that this is an expected behavior.
[/SAMI]

> Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
> ---
>   Platform/ARM/JunoPkg/AcpiTables/AcpiTables.inf      |  4 ++++
>   Platform/ARM/JunoPkg/AcpiTables/AcpiSsdtRootPci.asl | 13 +++++++++++++
>   2 files changed, 17 insertions(+)
>
> diff --git a/Platform/ARM/JunoPkg/AcpiTables/AcpiTables.inf b/Platform/ARM/JunoPkg/AcpiTables/AcpiTables.inf
> index f140febc4ad4..9a76475765f0 100644
> --- a/Platform/ARM/JunoPkg/AcpiTables/AcpiTables.inf
> +++ b/Platform/ARM/JunoPkg/AcpiTables/AcpiTables.inf
> @@ -45,6 +45,10 @@ [FixedPcd]
>     gArmTokenSpaceGuid.PcdGenericWatchdogControlBase
>     gArmTokenSpaceGuid.PcdGenericWatchdogRefreshBase
>
> +  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
> +  gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceSize
> +  gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceLimit
> +
>     #
>     # PL011 UART Settings for Serial Port Console Redirection
>     #
> diff --git a/Platform/ARM/JunoPkg/AcpiTables/AcpiSsdtRootPci.asl b/Platform/ARM/JunoPkg/AcpiTables/AcpiSsdtRootPci.asl
> index 317b621e013e..e60fc42a3340 100644
> --- a/Platform/ARM/JunoPkg/AcpiTables/AcpiSsdtRootPci.asl
> +++ b/Platform/ARM/JunoPkg/AcpiTables/AcpiSsdtRootPci.asl
> @@ -140,6 +140,19 @@ DefinitionBlock("SsdtPci.aml", "SSDT", 1, "ARMLTD", "ARM-JUNO", EFI_ACPI_ARM_OEM
>           Return (RBUF)
>         } // Method(_CRS)
>
> +      Device (RES0) {
> +        Name (_HID, "PNP0C02" /* PNP Motherboard Resources */)  // _HID: Hardware ID
> +        Name (_CRS, ResourceTemplate () {                       // _CRS: Current Resource Settings
> +           QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, NonCacheable, ReadWrite,
> +           0x0000000000000000,                                  // Granularity
> +           FixedPcdGet64 (PcdPciExpressBaseAddress),            // Range Minimum
> +           FixedPcdGet64 (PcdPciConfigurationSpaceLimit),       // Range Maximum
> +           0x0000000000000000,                                  // Translation Offset
> +           FixedPcdGet64 (PcdPciConfigurationSpaceSize),        // Length
> +           ,, , AddressRangeMemory, TypeStatic)
> +        })
> +      }
> +
>         //
>         // OS Control Handoff
>         //

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

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

* Re: [PATCH edk2-platforms 2/3] Platform/ARM/JunoPkg: Use MdePkg PcdPciExpressBaseAddress for ECAM addr
  2022-03-17  9:54   ` Sami Mujawar
@ 2022-03-19 19:56     ` Rebecca Cran
  0 siblings, 0 replies; 10+ messages in thread
From: Rebecca Cran @ 2022-03-19 19:56 UTC (permalink / raw)
  To: Sami Mujawar, devel, Ard Biesheuvel, Thomas Abraham, nd

On 3/17/22 03:54, Sami Mujawar wrote:
>>       // The physical base address for the PCI segment
>> -    FixedPcdGet64 (PcdPciConfigurationSpaceBaseAddress),
>> +    FixedPcdGet64 (gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress),
> [SAMI] I think this should just be FixedPcdGet64
> (PcdPciExpressBaseAddress).

Oops, yes. I've fixed it in v2.


-- 
Rebecca Cran


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

* Re: [PATCH edk2-platforms 3/3] Platform/ARM/JunoPkg: Reserve the ECAM area in ACPI with RES0 device
  2022-03-17  9:55   ` Sami Mujawar
@ 2022-03-19 19:56     ` Rebecca Cran
  0 siblings, 0 replies; 10+ messages in thread
From: Rebecca Cran @ 2022-03-19 19:56 UTC (permalink / raw)
  To: Sami Mujawar, devel, Ard Biesheuvel, Thomas Abraham, nd

On 3/17/22 03:55, Sami Mujawar wrote:
> On 05/03/2022 04:19 AM, Rebecca Cran wrote:
>> Add a RES0 device to the SSDT to reserve the PCI ECAM area.
>>
>> This fixes the warning that Linux prints:
>>
>> acpi PNP0A08:00: [Firmware Bug]: ECAM area [mem 0x40000000-0x4fffffff]
>> not reserved in ACPI namespace
> [SAMI] I noticed that the "Firmware Bug" message is no longer seen, but
> instead the following message is now printed
> "system 00:00: [mem 0x40000000-0x4fffffff window] could not be reserved"
>
> It appears this is a harmless message and the relevant discussion can be
> seen at: https://lore.kernel.org/all/20210603141641.GA17284@lpieralisi/#t
>
> I think it may be better to update the commit message to reference this
> discussion and clarify that this is an expected behavior.
> [/SAMI]

Thanks, I've updated the commit message in v2.


-- 
Rebecca Cran


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

end of thread, other threads:[~2022-03-19 19:56 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-03-05  4:19 [PATCH edk2-platforms 0/3] JunoPkg: Fix AcpiSsdtRootPci.asl to use spaces and reserve ECAM area Rebecca Cran
2022-03-05  4:19 ` [PATCH edk2-platforms 1/3] Platform/ARM/JunoPkg: Convert AcpiSsdtRootPci.asl from tabs to spaces Rebecca Cran
2022-03-16 15:40   ` Sami Mujawar
2022-03-05  4:19 ` [PATCH edk2-platforms 2/3] Platform/ARM/JunoPkg: Use MdePkg PcdPciExpressBaseAddress for ECAM addr Rebecca Cran
2022-03-17  9:54   ` Sami Mujawar
2022-03-19 19:56     ` Rebecca Cran
2022-03-05  4:19 ` [PATCH edk2-platforms 3/3] Platform/ARM/JunoPkg: Reserve the ECAM area in ACPI with RES0 device Rebecca Cran
2022-03-17  9:55   ` Sami Mujawar
2022-03-19 19:56     ` Rebecca Cran
2022-03-15  2:44 ` [PATCH edk2-platforms 0/3] JunoPkg: Fix AcpiSsdtRootPci.asl to use spaces and reserve ECAM area Rebecca Cran

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