public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH edk2-platforms 0/2] SynQuacer: enable MMIO translation
@ 2018-06-26 10:44 Ard Biesheuvel
  2018-06-26 10:44 ` [PATCH edk2-platforms 1/2] Silicon/SynQuacer: add preliminary support for PCIe MMIO32 translation Ard Biesheuvel
  2018-06-26 10:44 ` [PATCH edk2-platforms 2/2] Silicon/Socionext/SynQuacer: use single translated MMIO window for PCI1 Ard Biesheuvel
  0 siblings, 2 replies; 7+ messages in thread
From: Ard Biesheuvel @ 2018-06-26 10:44 UTC (permalink / raw)
  To: edk2-devel; +Cc: leif.lindholm, Ard Biesheuvel

Even if the UEFI spec permits it and accommodates it, many UEFI drivers
for PCI peripherals appear to misbehave in the presence of MMIO windows
that appear at different addresses in the CPU and PCI address spaces.
So let's add some plumbing to allow us to test this on SynQuacer.

Patch #1 adds some groundwork, and can be taken into edk2-platforms.

Patch #2 is included for reference, and may be applied locally to build
firmware images to be used in driver testing.

Ard Biesheuvel (2):
  Silicon/SynQuacer: add preliminary support for PCIe MMIO32 translation
  Silicon/Socionext/SynQuacer: use single translated MMIO window for
    PCI1

 .../SynQuacer/AcpiTables/AcpiSsdtRootPci.asl  | 19 ++++--------------
 .../SynQuacer/DeviceTree/SynQuacer.dtsi       | 11 +++++-----
 .../SynQuacer/Include/Platform/Pcie.h         | 20 ++++++++++---------
 .../SynQuacerPciHostBridgeLib.c               | 11 +++++-----
 .../SynQuacerPciHostBridgeLibConstructor.c    |  2 +-
 5 files changed, 27 insertions(+), 36 deletions(-)

-- 
2.17.1



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

* [PATCH edk2-platforms 1/2] Silicon/SynQuacer: add preliminary support for PCIe MMIO32 translation
  2018-06-26 10:44 [PATCH edk2-platforms 0/2] SynQuacer: enable MMIO translation Ard Biesheuvel
@ 2018-06-26 10:44 ` Ard Biesheuvel
  2018-06-26 14:24   ` Leif Lindholm
  2018-06-26 10:44 ` [PATCH edk2-platforms 2/2] Silicon/Socionext/SynQuacer: use single translated MMIO window for PCI1 Ard Biesheuvel
  1 sibling, 1 reply; 7+ messages in thread
From: Ard Biesheuvel @ 2018-06-26 10:44 UTC (permalink / raw)
  To: edk2-devel; +Cc: leif.lindholm, Ard Biesheuvel

Add the basic support for enabling PCIe MMIO32 translation on the
SynQuacer, without actually enabling it just yet. It would allow us
to increase the bus range to 255 MB [from 127 MB] and the MMIO32
range to 512 MB or more [from 128 MB], but it is more likely to
cause compatibility issues with code ported from the PC platform.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl                                           | 8 ++++----
 Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h                                                  | 2 ++
 Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c            | 6 ++++--
 Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c | 2 +-
 4 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl b/Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl
index 51e9d0b22c3d..77d4763d1a85 100644
--- a/Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl
+++ b/Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl
@@ -86,14 +86,14 @@ DefinitionBlock ("SsdtPci.aml", "SSDT", 1, "SNI", "SYNQUACR",
                     SYNQUACER_PCI_SEG0_BUSNUM_RANGE // RangeLength - # of Busses
                 )
 
-                DWordMemory ( // 32-bit BAR Windows
+                QWordMemory ( // 32-bit BAR Windows
                     ResourceProducer, PosDecode,
                     MinFixed, MaxFixed,
                     Cacheable, ReadWrite,
                     0x00000000,                         // Granularity
                     SYNQUACER_PCI_SEG0_MMIO32_MIN,      // Min Base Address
                     SYNQUACER_PCI_SEG0_MMIO32_MAX,      // Max Base Address
-                    0x00000000,                         // Translate
+                    SYNQUACER_PCI_SEG0_MMIO32_XLATE,    // Translate
                     SYNQUACER_PCI_SEG0_MMIO32_SIZE      // Length
                 )
 
@@ -224,14 +224,14 @@ DefinitionBlock ("SsdtPci.aml", "SSDT", 1, "SNI", "SYNQUACR",
                     SYNQUACER_PCI_SEG1_BUSNUM_RANGE // RangeLength - # of Busses
                 )
 
-                DWordMemory ( // 32-bit BAR Windows
+                QWordMemory ( // 32-bit BAR Windows
                     ResourceProducer, PosDecode,
                     MinFixed, MaxFixed,
                     Cacheable, ReadWrite,
                     0x00000000,                         // Granularity
                     SYNQUACER_PCI_SEG1_MMIO32_MIN,      // Min Base Address
                     SYNQUACER_PCI_SEG1_MMIO32_MAX,      // Max Base Address
-                    0x00000000,                         // Translate
+                    SYNQUACER_PCI_SEG1_MMIO32_XLATE,    // Translate
                     SYNQUACER_PCI_SEG1_MMIO32_SIZE      // Length
                 )
 
diff --git a/Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h b/Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h
index 950cece13e81..798f59db2a94 100644
--- a/Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h
+++ b/Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h
@@ -34,6 +34,7 @@
 #define SYNQUACER_PCI_SEG0_MMIO32_MIN       0x68000000
 #define SYNQUACER_PCI_SEG0_MMIO32_MAX       0x6fffffff
 #define SYNQUACER_PCI_SEG0_MMIO32_SIZE      0x08000000
+#define SYNQUACER_PCI_SEG0_MMIO32_XLATE     0x0
 
 #define SYNQUACER_PCI_SEG0_MMIO64_MIN       0x3e00000000
 #define SYNQUACER_PCI_SEG0_MMIO64_MAX       0x3effffffff
@@ -57,6 +58,7 @@
 #define SYNQUACER_PCI_SEG1_MMIO32_MIN       0x78000000
 #define SYNQUACER_PCI_SEG1_MMIO32_MAX       0x7fffffff
 #define SYNQUACER_PCI_SEG1_MMIO32_SIZE      0x08000000
+#define SYNQUACER_PCI_SEG1_MMIO32_XLATE     0x0
 
 #define SYNQUACER_PCI_SEG1_MMIO64_MIN       0x3f00000000
 #define SYNQUACER_PCI_SEG1_MMIO64_MAX       0x3fffffffff
diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c
index 341939876bd3..7c096f0801dd 100644
--- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c
+++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c
@@ -107,7 +107,8 @@ PCI_ROOT_BRIDGE mPciRootBridges[] = {
       SYNQUACER_PCI_SEG0_PORTIO_MAX,
       MAX_UINT64 - SYNQUACER_PCI_SEG0_PORTIO_OFFSET + 1 },   // Io
     { SYNQUACER_PCI_SEG0_MMIO32_MIN,
-      SYNQUACER_PCI_SEG0_MMIO32_MAX },      // Mem
+      SYNQUACER_PCI_SEG0_MMIO32_MAX,
+      MAX_UINT64 - SYNQUACER_PCI_SEG0_MMIO32_XLATE + 1 },    // Mem
     { SYNQUACER_PCI_SEG0_MMIO64_MIN,
       SYNQUACER_PCI_SEG0_MMIO64_MAX },      // MemAbove4G
     { MAX_UINT64, 0x0 },                    // PMem
@@ -127,7 +128,8 @@ PCI_ROOT_BRIDGE mPciRootBridges[] = {
       SYNQUACER_PCI_SEG1_PORTIO_MAX,
       MAX_UINT64 - SYNQUACER_PCI_SEG1_PORTIO_OFFSET + 1 },   // Io
     { SYNQUACER_PCI_SEG1_MMIO32_MIN,
-      SYNQUACER_PCI_SEG1_MMIO32_MAX },      // Mem
+      SYNQUACER_PCI_SEG1_MMIO32_MAX,
+      MAX_UINT64 - SYNQUACER_PCI_SEG1_MMIO32_XLATE + 1 },    // Mem
     { SYNQUACER_PCI_SEG1_MMIO64_MIN,
       SYNQUACER_PCI_SEG1_MMIO64_MAX },      // MemAbove4G
     { MAX_UINT64, 0x0 },                    // PMem
diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
index 227f9a725ce8..75a663e974e1 100644
--- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
+++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
@@ -322,7 +322,7 @@ PciInitControllerPost (
 
   // Region 0: MMIO32 range
   ConfigureWindow (DbiBase, 0,
-    RootBridge->Mem.Base,
+    RootBridge->Mem.Base - RootBridge->Mem.Translation,
     RootBridge->Mem.Base,
     RootBridge->Mem.Limit - RootBridge->Mem.Base + 1,
     IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TYPE_MEM |
-- 
2.17.1



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

* [PATCH edk2-platforms 2/2] Silicon/Socionext/SynQuacer: use single translated MMIO window for PCI1
  2018-06-26 10:44 [PATCH edk2-platforms 0/2] SynQuacer: enable MMIO translation Ard Biesheuvel
  2018-06-26 10:44 ` [PATCH edk2-platforms 1/2] Silicon/SynQuacer: add preliminary support for PCIe MMIO32 translation Ard Biesheuvel
@ 2018-06-26 10:44 ` Ard Biesheuvel
  1 sibling, 0 replies; 7+ messages in thread
From: Ard Biesheuvel @ 2018-06-26 10:44 UTC (permalink / raw)
  To: edk2-devel; +Cc: leif.lindholm, Ard Biesheuvel

For testing purposes, reconfigure the second PCI host bridge to only
expose a single MMIO window of 512 MB in size, and map it below 4 GB
on the PCI side (so 32-bit BARs can be allocated from it) and above
4 GB on the CPU side.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl                                | 11 -----------
 Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi                                     | 11 +++++------
 Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h                                       | 20 ++++++++++----------
 Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c |  5 ++---
 4 files changed, 17 insertions(+), 30 deletions(-)

diff --git a/Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl b/Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl
index 77d4763d1a85..e59d4cf98ebe 100644
--- a/Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl
+++ b/Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl
@@ -235,17 +235,6 @@ DefinitionBlock ("SsdtPci.aml", "SSDT", 1, "SNI", "SYNQUACR",
                     SYNQUACER_PCI_SEG1_MMIO32_SIZE      // Length
                 )
 
-                QWordMemory ( // 64-bit BAR Windows
-                    ResourceProducer, PosDecode,
-                    MinFixed, MaxFixed,
-                    Cacheable, ReadWrite,
-                    0x00000000,                         // Granularity
-                    SYNQUACER_PCI_SEG1_MMIO64_MIN,      // Min Base Address
-                    SYNQUACER_PCI_SEG1_MMIO64_MAX,      // Max Base Address
-                    0x00000000,                         // Translate
-                    SYNQUACER_PCI_SEG1_MMIO64_SIZE      // Length
-                )
-
                 DWordIo ( // IO window
                     ResourceProducer,
                     MinFixed,
diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
index 37d642e4b237..57d111c65bc2 100644
--- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
+++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
@@ -478,19 +478,18 @@
     pcie1: pcie@70000000 {
         compatible = "socionext,synquacer-pcie-ecam", "snps,dw-pcie-ecam";
         device_type = "pci";
-        reg = <0x0 0x70000000 0x0 0x7f00000>;
-        bus-range = <0x0 0x7e>;
+        reg = <0x0 0x70000000 0x0 0xff00000>;
+        bus-range = <0x0 0xfe>;
         #address-cells = <3>;
         #size-cells = <2>;
-        ranges = <0x1000000 0x00 0x00000000 0x00 0x77f00000 0x0 0x00010000>,
-                 <0x2000000 0x00 0x78000000 0x00 0x78000000 0x0 0x08000000>,
-                 <0x3000000 0x3f 0x00000000 0x3f 0x00000000 0x1 0x00000000>;
+        ranges = <0x1000000 0x00 0x00000000 0x00 0x7ff00000 0x0 0x00010000>,
+                 <0x2000000 0x3f 0xe0000000 0x00 0x20000000 0x0 0x20000000>;
 
         #interrupt-cells = <0x1>;
         interrupt-map-mask = <0x0 0x0 0x0 0x0>;
         interrupt-map = <0x0 0x0 0x0 0x0 &gic 0x0 0x0 GIC_SPI 182 IRQ_TYPE_LEVEL_HIGH>;
 
-        msi-map = <0x0 &its 0x10000 0x7f00>;
+        msi-map = <0x0 &its 0x10000 0xff00>;
         dma-coherent;
     };
 
diff --git a/Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h b/Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h
index 798f59db2a94..643c1a581d0a 100644
--- a/Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h
+++ b/Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h
@@ -41,28 +41,28 @@
 #define SYNQUACER_PCI_SEG0_MMIO64_SIZE      0x100000000
 
 #define SYNQUACER_PCI_SEG1_CONFIG_BASE      0x70000000
-#define SYNQUACER_PCI_SEG1_CONFIG_SIZE      0x07f00000
+#define SYNQUACER_PCI_SEG1_CONFIG_SIZE      0x0ff00000
 #define SYNQUACER_PCI_SEG1_DBI_BASE         0x583c0000
 #define SYNQUACER_PCI_SEG1_EXS_BASE         0x58380000
 
 #define SYNQUACER_PCI_SEG1_BUSNUM_MIN       0x0
-#define SYNQUACER_PCI_SEG1_BUSNUM_MAX       0x7e
-#define SYNQUACER_PCI_SEG1_BUSNUM_RANGE     0x7f
+#define SYNQUACER_PCI_SEG1_BUSNUM_MAX       0xfe
+#define SYNQUACER_PCI_SEG1_BUSNUM_RANGE     0xff
 
 #define SYNQUACER_PCI_SEG1_PORTIO_MIN       0x0
 #define SYNQUACER_PCI_SEG1_PORTIO_MAX       0xffff
-#define SYNQUACER_PCI_SEG1_PORTIO_MEMBASE   0x77f00000
+#define SYNQUACER_PCI_SEG1_PORTIO_MEMBASE   0x7ff00000
 #define SYNQUACER_PCI_SEG1_PORTIO_MEMSIZE   0x10000
 #define SYNQUACER_PCI_SEG1_PORTIO_OFFSET    0x10000
 
-#define SYNQUACER_PCI_SEG1_MMIO32_MIN       0x78000000
-#define SYNQUACER_PCI_SEG1_MMIO32_MAX       0x7fffffff
-#define SYNQUACER_PCI_SEG1_MMIO32_SIZE      0x08000000
-#define SYNQUACER_PCI_SEG1_MMIO32_XLATE     0x0
+#define SYNQUACER_PCI_SEG1_MMIO32_MIN       0x20000000
+#define SYNQUACER_PCI_SEG1_MMIO32_MAX       0x3fffffff
+#define SYNQUACER_PCI_SEG1_MMIO32_SIZE      0x20000000
+#define SYNQUACER_PCI_SEG1_MMIO32_XLATE     0x3fc0000000
 
 #define SYNQUACER_PCI_SEG1_MMIO64_MIN       0x3f00000000
-#define SYNQUACER_PCI_SEG1_MMIO64_MAX       0x3fffffffff
-#define SYNQUACER_PCI_SEG1_MMIO64_SIZE      0x100000000
+#define SYNQUACER_PCI_SEG1_MMIO64_MAX       0x3fdfffffff
+#define SYNQUACER_PCI_SEG1_MMIO64_SIZE      0xe0000000
 
 #define SYNQUACER_PCI_LOCATION(s,b,d)       (((s) << 16) | ((b) << 8) | (d))
 #define SYNQUACER_PCI_SLOT0_LOCATION        SYNQUACER_PCI_LOCATION(1, 0, 0)
diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c
index 7c096f0801dd..cf8ea6da2095 100644
--- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c
+++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c
@@ -121,7 +121,7 @@ PCI_ROOT_BRIDGE mPciRootBridges[] = {
     TRUE,                                   // DmaAbove4G
     FALSE,                                  // NoExtendedConfigSpace
     FALSE,                                  // ResourceAssigned
-    PCI_ALLOCATION_ATTRIBUTES,              // AllocationAttributes
+    EFI_PCI_HOST_BRIDGE_COMBINE_MEM_PMEM,   // AllocationAttributes
     { SYNQUACER_PCI_SEG1_BUSNUM_MIN,
       SYNQUACER_PCI_SEG1_BUSNUM_MAX },      // Bus
     { SYNQUACER_PCI_SEG1_PORTIO_MIN,
@@ -130,8 +130,7 @@ PCI_ROOT_BRIDGE mPciRootBridges[] = {
     { SYNQUACER_PCI_SEG1_MMIO32_MIN,
       SYNQUACER_PCI_SEG1_MMIO32_MAX,
       MAX_UINT64 - SYNQUACER_PCI_SEG1_MMIO32_XLATE + 1 },    // Mem
-    { SYNQUACER_PCI_SEG1_MMIO64_MIN,
-      SYNQUACER_PCI_SEG1_MMIO64_MAX },      // MemAbove4G
+    { MAX_UINT64, 0x0 },                    // MemAbove4G
     { MAX_UINT64, 0x0 },                    // PMem
     { MAX_UINT64, 0x0 },                    // PMemAbove4G
     (EFI_DEVICE_PATH_PROTOCOL *)&mEfiPciRootBridgeDevicePath[1]
-- 
2.17.1



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

* Re: [PATCH edk2-platforms 1/2] Silicon/SynQuacer: add preliminary support for PCIe MMIO32 translation
  2018-06-26 10:44 ` [PATCH edk2-platforms 1/2] Silicon/SynQuacer: add preliminary support for PCIe MMIO32 translation Ard Biesheuvel
@ 2018-06-26 14:24   ` Leif Lindholm
  2018-06-26 14:25     ` Ard Biesheuvel
  0 siblings, 1 reply; 7+ messages in thread
From: Leif Lindholm @ 2018-06-26 14:24 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel

On Tue, Jun 26, 2018 at 12:44:23PM +0200, Ard Biesheuvel wrote:
> Add the basic support for enabling PCIe MMIO32 translation on the
> SynQuacer, without actually enabling it just yet. It would allow us
> to increase the bus range to 255 MB [from 127 MB] and the MMIO32
> range to 512 MB or more [from 128 MB], but it is more likely to
> cause compatibility issues with code ported from the PC platform.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl                                           | 8 ++++----
>  Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h                                                  | 2 ++
>  Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c            | 6 ++++--
>  Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c | 2 +-
>  4 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl b/Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl
> index 51e9d0b22c3d..77d4763d1a85 100644
> --- a/Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl
> +++ b/Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl
> @@ -86,14 +86,14 @@ DefinitionBlock ("SsdtPci.aml", "SSDT", 1, "SNI", "SYNQUACR",
>                      SYNQUACER_PCI_SEG0_BUSNUM_RANGE // RangeLength - # of Busses
>                  )
>  
> -                DWordMemory ( // 32-bit BAR Windows
> +                QWordMemory ( // 32-bit BAR Windows
>                      ResourceProducer, PosDecode,
>                      MinFixed, MaxFixed,
>                      Cacheable, ReadWrite,
>                      0x00000000,                         // Granularity
>                      SYNQUACER_PCI_SEG0_MMIO32_MIN,      // Min Base Address
>                      SYNQUACER_PCI_SEG0_MMIO32_MAX,      // Max Base Address
> -                    0x00000000,                         // Translate
> +                    SYNQUACER_PCI_SEG0_MMIO32_XLATE,    // Translate
>                      SYNQUACER_PCI_SEG0_MMIO32_SIZE      // Length
>                  )
>  
> @@ -224,14 +224,14 @@ DefinitionBlock ("SsdtPci.aml", "SSDT", 1, "SNI", "SYNQUACR",
>                      SYNQUACER_PCI_SEG1_BUSNUM_RANGE // RangeLength - # of Busses
>                  )
>  
> -                DWordMemory ( // 32-bit BAR Windows
> +                QWordMemory ( // 32-bit BAR Windows
>                      ResourceProducer, PosDecode,
>                      MinFixed, MaxFixed,
>                      Cacheable, ReadWrite,
>                      0x00000000,                         // Granularity
>                      SYNQUACER_PCI_SEG1_MMIO32_MIN,      // Min Base Address
>                      SYNQUACER_PCI_SEG1_MMIO32_MAX,      // Max Base Address
> -                    0x00000000,                         // Translate
> +                    SYNQUACER_PCI_SEG1_MMIO32_XLATE,    // Translate
>                      SYNQUACER_PCI_SEG1_MMIO32_SIZE      // Length
>                  )
>  
> diff --git a/Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h b/Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h
> index 950cece13e81..798f59db2a94 100644
> --- a/Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h
> +++ b/Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h
> @@ -34,6 +34,7 @@
>  #define SYNQUACER_PCI_SEG0_MMIO32_MIN       0x68000000
>  #define SYNQUACER_PCI_SEG0_MMIO32_MAX       0x6fffffff
>  #define SYNQUACER_PCI_SEG0_MMIO32_SIZE      0x08000000
> +#define SYNQUACER_PCI_SEG0_MMIO32_XLATE     0x0
>  
>  #define SYNQUACER_PCI_SEG0_MMIO64_MIN       0x3e00000000
>  #define SYNQUACER_PCI_SEG0_MMIO64_MAX       0x3effffffff
> @@ -57,6 +58,7 @@
>  #define SYNQUACER_PCI_SEG1_MMIO32_MIN       0x78000000
>  #define SYNQUACER_PCI_SEG1_MMIO32_MAX       0x7fffffff
>  #define SYNQUACER_PCI_SEG1_MMIO32_SIZE      0x08000000
> +#define SYNQUACER_PCI_SEG1_MMIO32_XLATE     0x0
>  
>  #define SYNQUACER_PCI_SEG1_MMIO64_MIN       0x3f00000000
>  #define SYNQUACER_PCI_SEG1_MMIO64_MAX       0x3fffffffff
> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c
> index 341939876bd3..7c096f0801dd 100644
> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c
> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c
> @@ -107,7 +107,8 @@ PCI_ROOT_BRIDGE mPciRootBridges[] = {
>        SYNQUACER_PCI_SEG0_PORTIO_MAX,
>        MAX_UINT64 - SYNQUACER_PCI_SEG0_PORTIO_OFFSET + 1 },   // Io
>      { SYNQUACER_PCI_SEG0_MMIO32_MIN,
> -      SYNQUACER_PCI_SEG0_MMIO32_MAX },      // Mem
> +      SYNQUACER_PCI_SEG0_MMIO32_MAX,
> +      MAX_UINT64 - SYNQUACER_PCI_SEG0_MMIO32_XLATE + 1 },    // Mem

So, this had me scratching my head for a second.
I may get pickier about requring explicitly initializing the
Translation field in future, but for this patch:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

>      { SYNQUACER_PCI_SEG0_MMIO64_MIN,
>        SYNQUACER_PCI_SEG0_MMIO64_MAX },      // MemAbove4G
>      { MAX_UINT64, 0x0 },                    // PMem
> @@ -127,7 +128,8 @@ PCI_ROOT_BRIDGE mPciRootBridges[] = {
>        SYNQUACER_PCI_SEG1_PORTIO_MAX,
>        MAX_UINT64 - SYNQUACER_PCI_SEG1_PORTIO_OFFSET + 1 },   // Io
>      { SYNQUACER_PCI_SEG1_MMIO32_MIN,
> -      SYNQUACER_PCI_SEG1_MMIO32_MAX },      // Mem
> +      SYNQUACER_PCI_SEG1_MMIO32_MAX,
> +      MAX_UINT64 - SYNQUACER_PCI_SEG1_MMIO32_XLATE + 1 },    // Mem
>      { SYNQUACER_PCI_SEG1_MMIO64_MIN,
>        SYNQUACER_PCI_SEG1_MMIO64_MAX },      // MemAbove4G
>      { MAX_UINT64, 0x0 },                    // PMem
> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
> index 227f9a725ce8..75a663e974e1 100644
> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
> @@ -322,7 +322,7 @@ PciInitControllerPost (
>  
>    // Region 0: MMIO32 range
>    ConfigureWindow (DbiBase, 0,
> -    RootBridge->Mem.Base,
> +    RootBridge->Mem.Base - RootBridge->Mem.Translation,
>      RootBridge->Mem.Base,
>      RootBridge->Mem.Limit - RootBridge->Mem.Base + 1,
>      IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TYPE_MEM |
> -- 
> 2.17.1
> 


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

* Re: [PATCH edk2-platforms 1/2] Silicon/SynQuacer: add preliminary support for PCIe MMIO32 translation
  2018-06-26 14:24   ` Leif Lindholm
@ 2018-06-26 14:25     ` Ard Biesheuvel
  2018-06-26 14:34       ` Leif Lindholm
  0 siblings, 1 reply; 7+ messages in thread
From: Ard Biesheuvel @ 2018-06-26 14:25 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: edk2-devel@lists.01.org

On 26 June 2018 at 16:24, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Tue, Jun 26, 2018 at 12:44:23PM +0200, Ard Biesheuvel wrote:
>> Add the basic support for enabling PCIe MMIO32 translation on the
>> SynQuacer, without actually enabling it just yet. It would allow us
>> to increase the bus range to 255 MB [from 127 MB] and the MMIO32
>> range to 512 MB or more [from 128 MB], but it is more likely to
>> cause compatibility issues with code ported from the PC platform.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl                                           | 8 ++++----
>>  Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h                                                  | 2 ++
>>  Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c            | 6 ++++--
>>  Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c | 2 +-
>>  4 files changed, 11 insertions(+), 7 deletions(-)
>>
>> diff --git a/Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl b/Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl
>> index 51e9d0b22c3d..77d4763d1a85 100644
>> --- a/Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl
>> +++ b/Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl
>> @@ -86,14 +86,14 @@ DefinitionBlock ("SsdtPci.aml", "SSDT", 1, "SNI", "SYNQUACR",
>>                      SYNQUACER_PCI_SEG0_BUSNUM_RANGE // RangeLength - # of Busses
>>                  )
>>
>> -                DWordMemory ( // 32-bit BAR Windows
>> +                QWordMemory ( // 32-bit BAR Windows
>>                      ResourceProducer, PosDecode,
>>                      MinFixed, MaxFixed,
>>                      Cacheable, ReadWrite,
>>                      0x00000000,                         // Granularity
>>                      SYNQUACER_PCI_SEG0_MMIO32_MIN,      // Min Base Address
>>                      SYNQUACER_PCI_SEG0_MMIO32_MAX,      // Max Base Address
>> -                    0x00000000,                         // Translate
>> +                    SYNQUACER_PCI_SEG0_MMIO32_XLATE,    // Translate
>>                      SYNQUACER_PCI_SEG0_MMIO32_SIZE      // Length
>>                  )
>>
>> @@ -224,14 +224,14 @@ DefinitionBlock ("SsdtPci.aml", "SSDT", 1, "SNI", "SYNQUACR",
>>                      SYNQUACER_PCI_SEG1_BUSNUM_RANGE // RangeLength - # of Busses
>>                  )
>>
>> -                DWordMemory ( // 32-bit BAR Windows
>> +                QWordMemory ( // 32-bit BAR Windows
>>                      ResourceProducer, PosDecode,
>>                      MinFixed, MaxFixed,
>>                      Cacheable, ReadWrite,
>>                      0x00000000,                         // Granularity
>>                      SYNQUACER_PCI_SEG1_MMIO32_MIN,      // Min Base Address
>>                      SYNQUACER_PCI_SEG1_MMIO32_MAX,      // Max Base Address
>> -                    0x00000000,                         // Translate
>> +                    SYNQUACER_PCI_SEG1_MMIO32_XLATE,    // Translate
>>                      SYNQUACER_PCI_SEG1_MMIO32_SIZE      // Length
>>                  )
>>
>> diff --git a/Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h b/Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h
>> index 950cece13e81..798f59db2a94 100644
>> --- a/Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h
>> +++ b/Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h
>> @@ -34,6 +34,7 @@
>>  #define SYNQUACER_PCI_SEG0_MMIO32_MIN       0x68000000
>>  #define SYNQUACER_PCI_SEG0_MMIO32_MAX       0x6fffffff
>>  #define SYNQUACER_PCI_SEG0_MMIO32_SIZE      0x08000000
>> +#define SYNQUACER_PCI_SEG0_MMIO32_XLATE     0x0
>>
>>  #define SYNQUACER_PCI_SEG0_MMIO64_MIN       0x3e00000000
>>  #define SYNQUACER_PCI_SEG0_MMIO64_MAX       0x3effffffff
>> @@ -57,6 +58,7 @@
>>  #define SYNQUACER_PCI_SEG1_MMIO32_MIN       0x78000000
>>  #define SYNQUACER_PCI_SEG1_MMIO32_MAX       0x7fffffff
>>  #define SYNQUACER_PCI_SEG1_MMIO32_SIZE      0x08000000
>> +#define SYNQUACER_PCI_SEG1_MMIO32_XLATE     0x0
>>
>>  #define SYNQUACER_PCI_SEG1_MMIO64_MIN       0x3f00000000
>>  #define SYNQUACER_PCI_SEG1_MMIO64_MAX       0x3fffffffff
>> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c
>> index 341939876bd3..7c096f0801dd 100644
>> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c
>> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c
>> @@ -107,7 +107,8 @@ PCI_ROOT_BRIDGE mPciRootBridges[] = {
>>        SYNQUACER_PCI_SEG0_PORTIO_MAX,
>>        MAX_UINT64 - SYNQUACER_PCI_SEG0_PORTIO_OFFSET + 1 },   // Io
>>      { SYNQUACER_PCI_SEG0_MMIO32_MIN,
>> -      SYNQUACER_PCI_SEG0_MMIO32_MAX },      // Mem
>> +      SYNQUACER_PCI_SEG0_MMIO32_MAX,
>> +      MAX_UINT64 - SYNQUACER_PCI_SEG0_MMIO32_XLATE + 1 },    // Mem
>
> So, this had me scratching my head for a second.
> I may get pickier about requring explicitly initializing the
> Translation field in future, but for this patch:
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>

That field did not exist yet when this code was merged. It was
introduced by Heyi's recent PCI patches.

>>      { SYNQUACER_PCI_SEG0_MMIO64_MIN,
>>        SYNQUACER_PCI_SEG0_MMIO64_MAX },      // MemAbove4G
>>      { MAX_UINT64, 0x0 },                    // PMem
>> @@ -127,7 +128,8 @@ PCI_ROOT_BRIDGE mPciRootBridges[] = {
>>        SYNQUACER_PCI_SEG1_PORTIO_MAX,
>>        MAX_UINT64 - SYNQUACER_PCI_SEG1_PORTIO_OFFSET + 1 },   // Io
>>      { SYNQUACER_PCI_SEG1_MMIO32_MIN,
>> -      SYNQUACER_PCI_SEG1_MMIO32_MAX },      // Mem
>> +      SYNQUACER_PCI_SEG1_MMIO32_MAX,
>> +      MAX_UINT64 - SYNQUACER_PCI_SEG1_MMIO32_XLATE + 1 },    // Mem
>>      { SYNQUACER_PCI_SEG1_MMIO64_MIN,
>>        SYNQUACER_PCI_SEG1_MMIO64_MAX },      // MemAbove4G
>>      { MAX_UINT64, 0x0 },                    // PMem
>> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
>> index 227f9a725ce8..75a663e974e1 100644
>> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
>> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
>> @@ -322,7 +322,7 @@ PciInitControllerPost (
>>
>>    // Region 0: MMIO32 range
>>    ConfigureWindow (DbiBase, 0,
>> -    RootBridge->Mem.Base,
>> +    RootBridge->Mem.Base - RootBridge->Mem.Translation,
>>      RootBridge->Mem.Base,
>>      RootBridge->Mem.Limit - RootBridge->Mem.Base + 1,
>>      IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TYPE_MEM |
>> --
>> 2.17.1
>>


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

* Re: [PATCH edk2-platforms 1/2] Silicon/SynQuacer: add preliminary support for PCIe MMIO32 translation
  2018-06-26 14:25     ` Ard Biesheuvel
@ 2018-06-26 14:34       ` Leif Lindholm
  2018-06-26 18:21         ` Ard Biesheuvel
  0 siblings, 1 reply; 7+ messages in thread
From: Leif Lindholm @ 2018-06-26 14:34 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel@lists.01.org

On Tue, Jun 26, 2018 at 04:25:46PM +0200, Ard Biesheuvel wrote:
> On 26 June 2018 at 16:24, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > On Tue, Jun 26, 2018 at 12:44:23PM +0200, Ard Biesheuvel wrote:
> >> Add the basic support for enabling PCIe MMIO32 translation on the
> >> SynQuacer, without actually enabling it just yet. It would allow us
> >> to increase the bus range to 255 MB [from 127 MB] and the MMIO32
> >> range to 512 MB or more [from 128 MB], but it is more likely to
> >> cause compatibility issues with code ported from the PC platform.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> ---
> >>  Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl                                           | 8 ++++----
> >>  Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h                                                  | 2 ++
> >>  Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c            | 6 ++++--
> >>  Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c | 2 +-
> >>  4 files changed, 11 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl b/Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl
> >> index 51e9d0b22c3d..77d4763d1a85 100644
> >> --- a/Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl
> >> +++ b/Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl
> >> @@ -86,14 +86,14 @@ DefinitionBlock ("SsdtPci.aml", "SSDT", 1, "SNI", "SYNQUACR",
> >>                      SYNQUACER_PCI_SEG0_BUSNUM_RANGE // RangeLength - # of Busses
> >>                  )
> >>
> >> -                DWordMemory ( // 32-bit BAR Windows
> >> +                QWordMemory ( // 32-bit BAR Windows
> >>                      ResourceProducer, PosDecode,
> >>                      MinFixed, MaxFixed,
> >>                      Cacheable, ReadWrite,
> >>                      0x00000000,                         // Granularity
> >>                      SYNQUACER_PCI_SEG0_MMIO32_MIN,      // Min Base Address
> >>                      SYNQUACER_PCI_SEG0_MMIO32_MAX,      // Max Base Address
> >> -                    0x00000000,                         // Translate
> >> +                    SYNQUACER_PCI_SEG0_MMIO32_XLATE,    // Translate
> >>                      SYNQUACER_PCI_SEG0_MMIO32_SIZE      // Length
> >>                  )
> >>
> >> @@ -224,14 +224,14 @@ DefinitionBlock ("SsdtPci.aml", "SSDT", 1, "SNI", "SYNQUACR",
> >>                      SYNQUACER_PCI_SEG1_BUSNUM_RANGE // RangeLength - # of Busses
> >>                  )
> >>
> >> -                DWordMemory ( // 32-bit BAR Windows
> >> +                QWordMemory ( // 32-bit BAR Windows
> >>                      ResourceProducer, PosDecode,
> >>                      MinFixed, MaxFixed,
> >>                      Cacheable, ReadWrite,
> >>                      0x00000000,                         // Granularity
> >>                      SYNQUACER_PCI_SEG1_MMIO32_MIN,      // Min Base Address
> >>                      SYNQUACER_PCI_SEG1_MMIO32_MAX,      // Max Base Address
> >> -                    0x00000000,                         // Translate
> >> +                    SYNQUACER_PCI_SEG1_MMIO32_XLATE,    // Translate
> >>                      SYNQUACER_PCI_SEG1_MMIO32_SIZE      // Length
> >>                  )
> >>
> >> diff --git a/Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h b/Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h
> >> index 950cece13e81..798f59db2a94 100644
> >> --- a/Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h
> >> +++ b/Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h
> >> @@ -34,6 +34,7 @@
> >>  #define SYNQUACER_PCI_SEG0_MMIO32_MIN       0x68000000
> >>  #define SYNQUACER_PCI_SEG0_MMIO32_MAX       0x6fffffff
> >>  #define SYNQUACER_PCI_SEG0_MMIO32_SIZE      0x08000000
> >> +#define SYNQUACER_PCI_SEG0_MMIO32_XLATE     0x0
> >>
> >>  #define SYNQUACER_PCI_SEG0_MMIO64_MIN       0x3e00000000
> >>  #define SYNQUACER_PCI_SEG0_MMIO64_MAX       0x3effffffff
> >> @@ -57,6 +58,7 @@
> >>  #define SYNQUACER_PCI_SEG1_MMIO32_MIN       0x78000000
> >>  #define SYNQUACER_PCI_SEG1_MMIO32_MAX       0x7fffffff
> >>  #define SYNQUACER_PCI_SEG1_MMIO32_SIZE      0x08000000
> >> +#define SYNQUACER_PCI_SEG1_MMIO32_XLATE     0x0
> >>
> >>  #define SYNQUACER_PCI_SEG1_MMIO64_MIN       0x3f00000000
> >>  #define SYNQUACER_PCI_SEG1_MMIO64_MAX       0x3fffffffff
> >> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c
> >> index 341939876bd3..7c096f0801dd 100644
> >> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c
> >> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c
> >> @@ -107,7 +107,8 @@ PCI_ROOT_BRIDGE mPciRootBridges[] = {
> >>        SYNQUACER_PCI_SEG0_PORTIO_MAX,
> >>        MAX_UINT64 - SYNQUACER_PCI_SEG0_PORTIO_OFFSET + 1 },   // Io
> >>      { SYNQUACER_PCI_SEG0_MMIO32_MIN,
> >> -      SYNQUACER_PCI_SEG0_MMIO32_MAX },      // Mem
> >> +      SYNQUACER_PCI_SEG0_MMIO32_MAX,
> >> +      MAX_UINT64 - SYNQUACER_PCI_SEG0_MMIO32_XLATE + 1 },    // Mem
> >
> > So, this had me scratching my head for a second.
> > I may get pickier about requring explicitly initializing the
> > Translation field in future, but for this patch:
> > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> >
> 
> That field did not exist yet when this code was merged. It was
> introduced by Heyi's recent PCI patches.

Ah, fair enough.
Slightly unfortunate C just silently lets that through though...

/
    Leif

> >>      { SYNQUACER_PCI_SEG0_MMIO64_MIN,
> >>        SYNQUACER_PCI_SEG0_MMIO64_MAX },      // MemAbove4G
> >>      { MAX_UINT64, 0x0 },                    // PMem
> >> @@ -127,7 +128,8 @@ PCI_ROOT_BRIDGE mPciRootBridges[] = {
> >>        SYNQUACER_PCI_SEG1_PORTIO_MAX,
> >>        MAX_UINT64 - SYNQUACER_PCI_SEG1_PORTIO_OFFSET + 1 },   // Io
> >>      { SYNQUACER_PCI_SEG1_MMIO32_MIN,
> >> -      SYNQUACER_PCI_SEG1_MMIO32_MAX },      // Mem
> >> +      SYNQUACER_PCI_SEG1_MMIO32_MAX,
> >> +      MAX_UINT64 - SYNQUACER_PCI_SEG1_MMIO32_XLATE + 1 },    // Mem
> >>      { SYNQUACER_PCI_SEG1_MMIO64_MIN,
> >>        SYNQUACER_PCI_SEG1_MMIO64_MAX },      // MemAbove4G
> >>      { MAX_UINT64, 0x0 },                    // PMem
> >> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
> >> index 227f9a725ce8..75a663e974e1 100644
> >> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
> >> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
> >> @@ -322,7 +322,7 @@ PciInitControllerPost (
> >>
> >>    // Region 0: MMIO32 range
> >>    ConfigureWindow (DbiBase, 0,
> >> -    RootBridge->Mem.Base,
> >> +    RootBridge->Mem.Base - RootBridge->Mem.Translation,
> >>      RootBridge->Mem.Base,
> >>      RootBridge->Mem.Limit - RootBridge->Mem.Base + 1,
> >>      IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TYPE_MEM |
> >> --
> >> 2.17.1
> >>


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

* Re: [PATCH edk2-platforms 1/2] Silicon/SynQuacer: add preliminary support for PCIe MMIO32 translation
  2018-06-26 14:34       ` Leif Lindholm
@ 2018-06-26 18:21         ` Ard Biesheuvel
  0 siblings, 0 replies; 7+ messages in thread
From: Ard Biesheuvel @ 2018-06-26 18:21 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: edk2-devel@lists.01.org

On 26 June 2018 at 16:34, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Tue, Jun 26, 2018 at 04:25:46PM +0200, Ard Biesheuvel wrote:
>> On 26 June 2018 at 16:24, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>> > On Tue, Jun 26, 2018 at 12:44:23PM +0200, Ard Biesheuvel wrote:
>> >> Add the basic support for enabling PCIe MMIO32 translation on the
>> >> SynQuacer, without actually enabling it just yet. It would allow us
>> >> to increase the bus range to 255 MB [from 127 MB] and the MMIO32
>> >> range to 512 MB or more [from 128 MB], but it is more likely to
>> >> cause compatibility issues with code ported from the PC platform.
>> >>
>> >> Contributed-under: TianoCore Contribution Agreement 1.1
>> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> >> ---
>> >>  Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl                                           | 8 ++++----
>> >>  Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h                                                  | 2 ++
>> >>  Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c            | 6 ++++--
>> >>  Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c | 2 +-
>> >>  4 files changed, 11 insertions(+), 7 deletions(-)
>> >>
>> >> diff --git a/Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl b/Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl
>> >> index 51e9d0b22c3d..77d4763d1a85 100644
>> >> --- a/Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl
>> >> +++ b/Silicon/Socionext/SynQuacer/AcpiTables/AcpiSsdtRootPci.asl
>> >> @@ -86,14 +86,14 @@ DefinitionBlock ("SsdtPci.aml", "SSDT", 1, "SNI", "SYNQUACR",
>> >>                      SYNQUACER_PCI_SEG0_BUSNUM_RANGE // RangeLength - # of Busses
>> >>                  )
>> >>
>> >> -                DWordMemory ( // 32-bit BAR Windows
>> >> +                QWordMemory ( // 32-bit BAR Windows
>> >>                      ResourceProducer, PosDecode,
>> >>                      MinFixed, MaxFixed,
>> >>                      Cacheable, ReadWrite,
>> >>                      0x00000000,                         // Granularity
>> >>                      SYNQUACER_PCI_SEG0_MMIO32_MIN,      // Min Base Address
>> >>                      SYNQUACER_PCI_SEG0_MMIO32_MAX,      // Max Base Address
>> >> -                    0x00000000,                         // Translate
>> >> +                    SYNQUACER_PCI_SEG0_MMIO32_XLATE,    // Translate
>> >>                      SYNQUACER_PCI_SEG0_MMIO32_SIZE      // Length
>> >>                  )
>> >>
>> >> @@ -224,14 +224,14 @@ DefinitionBlock ("SsdtPci.aml", "SSDT", 1, "SNI", "SYNQUACR",
>> >>                      SYNQUACER_PCI_SEG1_BUSNUM_RANGE // RangeLength - # of Busses
>> >>                  )
>> >>
>> >> -                DWordMemory ( // 32-bit BAR Windows
>> >> +                QWordMemory ( // 32-bit BAR Windows
>> >>                      ResourceProducer, PosDecode,
>> >>                      MinFixed, MaxFixed,
>> >>                      Cacheable, ReadWrite,
>> >>                      0x00000000,                         // Granularity
>> >>                      SYNQUACER_PCI_SEG1_MMIO32_MIN,      // Min Base Address
>> >>                      SYNQUACER_PCI_SEG1_MMIO32_MAX,      // Max Base Address
>> >> -                    0x00000000,                         // Translate
>> >> +                    SYNQUACER_PCI_SEG1_MMIO32_XLATE,    // Translate
>> >>                      SYNQUACER_PCI_SEG1_MMIO32_SIZE      // Length
>> >>                  )
>> >>
>> >> diff --git a/Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h b/Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h
>> >> index 950cece13e81..798f59db2a94 100644
>> >> --- a/Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h
>> >> +++ b/Silicon/Socionext/SynQuacer/Include/Platform/Pcie.h
>> >> @@ -34,6 +34,7 @@
>> >>  #define SYNQUACER_PCI_SEG0_MMIO32_MIN       0x68000000
>> >>  #define SYNQUACER_PCI_SEG0_MMIO32_MAX       0x6fffffff
>> >>  #define SYNQUACER_PCI_SEG0_MMIO32_SIZE      0x08000000
>> >> +#define SYNQUACER_PCI_SEG0_MMIO32_XLATE     0x0
>> >>
>> >>  #define SYNQUACER_PCI_SEG0_MMIO64_MIN       0x3e00000000
>> >>  #define SYNQUACER_PCI_SEG0_MMIO64_MAX       0x3effffffff
>> >> @@ -57,6 +58,7 @@
>> >>  #define SYNQUACER_PCI_SEG1_MMIO32_MIN       0x78000000
>> >>  #define SYNQUACER_PCI_SEG1_MMIO32_MAX       0x7fffffff
>> >>  #define SYNQUACER_PCI_SEG1_MMIO32_SIZE      0x08000000
>> >> +#define SYNQUACER_PCI_SEG1_MMIO32_XLATE     0x0
>> >>
>> >>  #define SYNQUACER_PCI_SEG1_MMIO64_MIN       0x3f00000000
>> >>  #define SYNQUACER_PCI_SEG1_MMIO64_MAX       0x3fffffffff
>> >> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c
>> >> index 341939876bd3..7c096f0801dd 100644
>> >> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c
>> >> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c
>> >> @@ -107,7 +107,8 @@ PCI_ROOT_BRIDGE mPciRootBridges[] = {
>> >>        SYNQUACER_PCI_SEG0_PORTIO_MAX,
>> >>        MAX_UINT64 - SYNQUACER_PCI_SEG0_PORTIO_OFFSET + 1 },   // Io
>> >>      { SYNQUACER_PCI_SEG0_MMIO32_MIN,
>> >> -      SYNQUACER_PCI_SEG0_MMIO32_MAX },      // Mem
>> >> +      SYNQUACER_PCI_SEG0_MMIO32_MAX,
>> >> +      MAX_UINT64 - SYNQUACER_PCI_SEG0_MMIO32_XLATE + 1 },    // Mem
>> >
>> > So, this had me scratching my head for a second.
>> > I may get pickier about requring explicitly initializing the
>> > Translation field in future, but for this patch:
>> > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>> >
>>
>> That field did not exist yet when this code was merged. It was
>> introduced by Heyi's recent PCI patches.
>
> Ah, fair enough.
> Slightly unfortunate C just silently lets that through though...
>
> /
>     Leif
>

Pushed as 3165d24e0c85..5ed298efba3b

Thanks

>> >>      { SYNQUACER_PCI_SEG0_MMIO64_MIN,
>> >>        SYNQUACER_PCI_SEG0_MMIO64_MAX },      // MemAbove4G
>> >>      { MAX_UINT64, 0x0 },                    // PMem
>> >> @@ -127,7 +128,8 @@ PCI_ROOT_BRIDGE mPciRootBridges[] = {
>> >>        SYNQUACER_PCI_SEG1_PORTIO_MAX,
>> >>        MAX_UINT64 - SYNQUACER_PCI_SEG1_PORTIO_OFFSET + 1 },   // Io
>> >>      { SYNQUACER_PCI_SEG1_MMIO32_MIN,
>> >> -      SYNQUACER_PCI_SEG1_MMIO32_MAX },      // Mem
>> >> +      SYNQUACER_PCI_SEG1_MMIO32_MAX,
>> >> +      MAX_UINT64 - SYNQUACER_PCI_SEG1_MMIO32_XLATE + 1 },    // Mem
>> >>      { SYNQUACER_PCI_SEG1_MMIO64_MIN,
>> >>        SYNQUACER_PCI_SEG1_MMIO64_MAX },      // MemAbove4G
>> >>      { MAX_UINT64, 0x0 },                    // PMem
>> >> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
>> >> index 227f9a725ce8..75a663e974e1 100644
>> >> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
>> >> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
>> >> @@ -322,7 +322,7 @@ PciInitControllerPost (
>> >>
>> >>    // Region 0: MMIO32 range
>> >>    ConfigureWindow (DbiBase, 0,
>> >> -    RootBridge->Mem.Base,
>> >> +    RootBridge->Mem.Base - RootBridge->Mem.Translation,
>> >>      RootBridge->Mem.Base,
>> >>      RootBridge->Mem.Limit - RootBridge->Mem.Base + 1,
>> >>      IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TYPE_MEM |
>> >> --
>> >> 2.17.1
>> >>


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

end of thread, other threads:[~2018-06-26 18:21 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-26 10:44 [PATCH edk2-platforms 0/2] SynQuacer: enable MMIO translation Ard Biesheuvel
2018-06-26 10:44 ` [PATCH edk2-platforms 1/2] Silicon/SynQuacer: add preliminary support for PCIe MMIO32 translation Ard Biesheuvel
2018-06-26 14:24   ` Leif Lindholm
2018-06-26 14:25     ` Ard Biesheuvel
2018-06-26 14:34       ` Leif Lindholm
2018-06-26 18:21         ` Ard Biesheuvel
2018-06-26 10:44 ` [PATCH edk2-platforms 2/2] Silicon/Socionext/SynQuacer: use single translated MMIO window for PCI1 Ard Biesheuvel

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