public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH edk2-platforms 0/3] More SynQuacer updates
@ 2018-05-30 18:19 Ard Biesheuvel
  2018-05-30 18:19 ` [PATCH edk2-platforms 1/3] Silicon/SynQuacerPciHostBridgeLib: add workaround for PCIe MMIO64 Ard Biesheuvel
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2018-05-30 18:19 UTC (permalink / raw)
  To: edk2-devel; +Cc: leif.lindholm, masahisa.kojima, Ard Biesheuvel

Some more patches for SynQuacer, for PCIe and SMMU support.

Patch #1 was contributed by Kojima-san, and enables another quirk in the
Synopsys IP to work around issues with Samsung 970 SSDs

Patch #2 adds a couple of level 3 tables to the static page table region
in the NOR flash so that ARM-TF can easily reprogram the north SMMU to
present the ECAM spaces in a sane manner. This patch only implements the
static page table entries, the code to program it needs to run in the secure
world, and will be added to ARM-TF.

Patch #3 enables the SMMU for the netsec and SDHCI controllers.

Ard Biesheuvel (2):
  Silicon/Socionext/SynQuacer/Stage2Tables: add north SMMU level 3 table
  Silicon/SynQuacer/AcpiTables: add NETSEC/eMMC SMMU to the IORT

Masahisa KOJIMA (1):
  Silicon/SynQuacerPciHostBridgeLib: add workaround for PCIe MMIO64

 .../Socionext/SynQuacer/AcpiTables/Iort.aslc  | 109 +++++++++++++++++-
 .../SynQuacerPciHostBridgeLibConstructor.c    |   5 +-
 .../SynQuacer/Stage2Tables/Stage2Tables.S     |  35 ++++--
 3 files changed, 136 insertions(+), 13 deletions(-)

-- 
2.17.0



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

* [PATCH edk2-platforms 1/3] Silicon/SynQuacerPciHostBridgeLib: add workaround for PCIe MMIO64
  2018-05-30 18:19 [PATCH edk2-platforms 0/3] More SynQuacer updates Ard Biesheuvel
@ 2018-05-30 18:19 ` Ard Biesheuvel
  2018-05-31  9:11   ` Leif Lindholm
  2018-05-30 18:19 ` [PATCH edk2-platforms 2/3] Silicon/Socionext/SynQuacer/Stage2Tables: add north SMMU level 3 table Ard Biesheuvel
  2018-05-30 18:19 ` [PATCH edk2-platforms 3/3] Silicon/SynQuacer/AcpiTables: add NETSEC/eMMC SMMU to the IORT Ard Biesheuvel
  2 siblings, 1 reply; 12+ messages in thread
From: Ard Biesheuvel @ 2018-05-30 18:19 UTC (permalink / raw)
  To: edk2-devel; +Cc: leif.lindholm, masahisa.kojima

From: Masahisa KOJIMA <masahisa.kojima@linaro.org>

The current revision of SC2A11 contains PCIe bus issue.
In MRd transaction, 1st/Last DW BE fields are not correctly set
by hardware.

As a workaround, set TH bit and specify MSG_CODE in iATU.
With this setup, the value specified as MSG_CODE is set to the
1st/Last DW BE fields and PCIe controller can emit the correct
MRd TLP header.
Same workaround was already included for MMIO32 region,
MMIO64 region also requires this workaround.
Some deivices, such as Samsong SSD 970 EVO, do not work
without this modification.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Masahisa KOJIMA <masahisa.kojima@linaro.org>
---
 Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
index e4679543cc66..227f9a725ce8 100644
--- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
+++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
@@ -359,8 +359,9 @@ PciInitControllerPost (
       RootBridge->MemAbove4G.Base,
       RootBridge->MemAbove4G.Base,
       RootBridge->MemAbove4G.Limit - RootBridge->MemAbove4G.Base + 1,
-      IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TYPE_MEM,
-      0);
+      IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TYPE_MEM |
+      IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TH,
+      IATU_REGION_CTRL_2_OFF_OUTBOUND_0_MSG_CODE_32BIT);
   }
 
   // enable link
-- 
2.17.0



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

* [PATCH edk2-platforms 2/3] Silicon/Socionext/SynQuacer/Stage2Tables: add north SMMU level 3 table
  2018-05-30 18:19 [PATCH edk2-platforms 0/3] More SynQuacer updates Ard Biesheuvel
  2018-05-30 18:19 ` [PATCH edk2-platforms 1/3] Silicon/SynQuacerPciHostBridgeLib: add workaround for PCIe MMIO64 Ard Biesheuvel
@ 2018-05-30 18:19 ` Ard Biesheuvel
  2018-05-31  9:14   ` Leif Lindholm
  2018-05-30 18:19 ` [PATCH edk2-platforms 3/3] Silicon/SynQuacer/AcpiTables: add NETSEC/eMMC SMMU to the IORT Ard Biesheuvel
  2 siblings, 1 reply; 12+ messages in thread
From: Ard Biesheuvel @ 2018-05-30 18:19 UTC (permalink / raw)
  To: edk2-devel; +Cc: leif.lindholm, masahisa.kojima, Ard Biesheuvel

Extend the static stage 2 page tables with a set of level 3 tables that
describe the ECAM space in a manner that allows the north SMMU to be used
to make the ECAM space appear sane to the CPUs.

It is up to the secure firmware to manipulate the north SMMU page tables
so that the level 2 block entries corresponding with busses #0 .. #1 in
the respective config spaces of PCI0 and PCI1 are replaced with table
entries pointing to the level 3 tables added by this patch.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Silicon/Socionext/SynQuacer/Stage2Tables/Stage2Tables.S | 35 +++++++++++++++-----
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/Silicon/Socionext/SynQuacer/Stage2Tables/Stage2Tables.S b/Silicon/Socionext/SynQuacer/Stage2Tables/Stage2Tables.S
index 313ef3c56abc..af55f27bca47 100644
--- a/Silicon/Socionext/SynQuacer/Stage2Tables/Stage2Tables.S
+++ b/Silicon/Socionext/SynQuacer/Stage2Tables/Stage2Tables.S
@@ -20,15 +20,17 @@
  *   the SoC.
  */
 
-#define TT_S2_CONT_SHIFT          52
-#define TT_S2_AF                  (0x1 << 10)
-#define TT_S2_SH_NON_SHAREABLE    (0x0 << 8)
-#define TT_S2_AP_RW               (0x3 << 6)
-#define TT_S2_MEMATTR_DEVICE_nGRE (0x2 << 2)
-#define TT_S2_MEMATTR_MEMORY_WB   (0xf << 2)
-#define TT_S2_TABLE               (0x3 << 0)
-#define TT_S2_L3_PAGE             (0x1 << 1)
-#define TT_S2_VALID               (0x1 << 0)
+#define TT_S2_CONT_SHIFT            52
+#define TT_S2_AF                    (0x1 << 10)
+#define TT_S2_SH_NON_SHAREABLE      (0x0 << 8)
+#define TT_S2_AP_RO                 (0x1 << 6)
+#define TT_S2_AP_RW                 (0x3 << 6)
+#define TT_S2_MEMATTR_DEVICE_nGRE   (0x2 << 2)
+#define TT_S2_MEMATTR_DEVICE_nGnRE  (0x1 << 2)
+#define TT_S2_MEMATTR_MEMORY_WB     (0xf << 2)
+#define TT_S2_TABLE                 (0x3 << 0)
+#define TT_S2_L3_PAGE               (0x1 << 1)
+#define TT_S2_VALID                 (0x1 << 0)
 
   .altmacro
   .macro    for, start, count, do, arg2, arg3, arg4
@@ -58,6 +60,12 @@
             TT_S2_L3_PAGE | TT_S2_VALID | (\cont << TT_S2_CONT_SHIFT)
   .endm
 
+  .macro    smmu_l3_entry, base, offset=0, ignore=0
+  .quad     ((\base << 12) + \offset) | TT_S2_AF | TT_S2_AP_RO | \
+            TT_S2_SH_NON_SHAREABLE | TT_S2_MEMATTR_DEVICE_nGnRE | \
+            TT_S2_L3_PAGE | TT_S2_VALID
+  .endm
+
   .section  ".rodata", "a", %progbits
   /* level 1 */
   s2_mem_entry  0      /* 0x0000_0000 - 0x3fff_ffff */
@@ -86,3 +94,12 @@
 3:for     0,   8, s2_l3_entry, 0x70000000
   for     0,   8, s2_l3_entry, 0x70010000  /* hide device #1 */
   for     0, 496, s2_l3_entry, 0x70010000, 1
+
+  /* level 3 for north SMMU */
+  .org    0x6000
+  for     0,   8, smmu_l3_entry, 0xc00060000000
+  for     0,   8, smmu_l3_entry, 0xc00060010000  /* hide device #1 */
+  for     0, 496, smmu_l3_entry, 0xc00060010000
+  for     0,   8, smmu_l3_entry, 0x800070000000
+  for     0,   8, smmu_l3_entry, 0x800070010000  /* hide device #1 */
+  for     0, 496, smmu_l3_entry, 0x800070010000
-- 
2.17.0



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

* [PATCH edk2-platforms 3/3] Silicon/SynQuacer/AcpiTables: add NETSEC/eMMC SMMU to the IORT
  2018-05-30 18:19 [PATCH edk2-platforms 0/3] More SynQuacer updates Ard Biesheuvel
  2018-05-30 18:19 ` [PATCH edk2-platforms 1/3] Silicon/SynQuacerPciHostBridgeLib: add workaround for PCIe MMIO64 Ard Biesheuvel
  2018-05-30 18:19 ` [PATCH edk2-platforms 2/3] Silicon/Socionext/SynQuacer/Stage2Tables: add north SMMU level 3 table Ard Biesheuvel
@ 2018-05-30 18:19 ` Ard Biesheuvel
  2018-05-31  9:16   ` Leif Lindholm
  2 siblings, 1 reply; 12+ messages in thread
From: Ard Biesheuvel @ 2018-05-30 18:19 UTC (permalink / raw)
  To: edk2-devel; +Cc: leif.lindholm, masahisa.kojima, Ard Biesheuvel

Add a description of the SMMU that sits in front of the NETSEC and
eMMC controllers to the IORT table so that ACPI based OSes can
utilize it.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Silicon/Socionext/SynQuacer/AcpiTables/Iort.aslc | 109 +++++++++++++++++++-
 1 file changed, 107 insertions(+), 2 deletions(-)

diff --git a/Silicon/Socionext/SynQuacer/AcpiTables/Iort.aslc b/Silicon/Socionext/SynQuacer/AcpiTables/Iort.aslc
index 92c485f8006f..3f2aaa3d8858 100644
--- a/Silicon/Socionext/SynQuacer/AcpiTables/Iort.aslc
+++ b/Silicon/Socionext/SynQuacer/AcpiTables/Iort.aslc
@@ -13,6 +13,7 @@
 **/
 
 #include <IndustryStandard/IoRemappingTable.h>
+#include <Platform/MemoryMap.h>
 
 #include "AcpiTables.h"
 
@@ -29,10 +30,23 @@ typedef struct {
   EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE        RcIdMapping;
 } SYNQUACER_RC_NODE;
 
+typedef struct {
+  EFI_ACPI_6_0_IO_REMAPPING_SMMU_NODE       Node;
+  EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT        Context[8];
+} SYNQUACER_SMMU_NODE;
+
+typedef struct {
+  EFI_ACPI_6_0_IO_REMAPPING_NAMED_COMP_NODE Node;
+  CONST CHAR8                               Name[11];
+  EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE        RcIdMapping;
+} SYNQUACER_NC_NODE;
+
 typedef struct {
   EFI_ACPI_6_0_IO_REMAPPING_TABLE           Iort;
   SYNQUACER_ITS_NODE                        ItsNode;
   SYNQUACER_RC_NODE                         RcNode[2];
+  SYNQUACER_SMMU_NODE                       SmmuNode;
+  SYNQUACER_NC_NODE                         NamedCompNode[2];
 } SYNQUACER_IO_REMAPPING_STRUCTURE;
 
 #define __SYNQUACER_ID_MAPPING(In, Num, Out, Ref, Flags)    \
@@ -49,7 +63,7 @@ STATIC SYNQUACER_IO_REMAPPING_STRUCTURE Iort = {
     __ACPI_HEADER(EFI_ACPI_6_0_IO_REMAPPING_TABLE_SIGNATURE,
                   SYNQUACER_IO_REMAPPING_STRUCTURE,
                   EFI_ACPI_IO_REMAPPING_TABLE_REVISION),
-    3,                                              // NumNodes
+    6,                                              // NumNodes
     sizeof(EFI_ACPI_6_0_IO_REMAPPING_TABLE),        // NodeOffset
     0                                               // Reserved
   }, {
@@ -94,7 +108,7 @@ STATIC SYNQUACER_IO_REMAPPING_STRUCTURE Iort = {
       //
       __SYNQUACER_ID_MAPPING(0x0, 0x0, 0x0, ItsNode,
                              EFI_ACPI_IORT_ID_MAPPING_FLAGS_SINGLE),
-  }, {
+    }, {
       // PciRcNode
       {
         {
@@ -121,6 +135,97 @@ STATIC SYNQUACER_IO_REMAPPING_STRUCTURE Iort = {
       __SYNQUACER_ID_MAPPING(0x0, 0x0, 0x0, ItsNode,
                              EFI_ACPI_IORT_ID_MAPPING_FLAGS_SINGLE),
     }
+  }, {
+    // NETSEC/eMMC SMMU node
+    {
+      {
+        EFI_ACPI_IORT_TYPE_SMMUv1v2,
+        sizeof(SYNQUACER_SMMU_NODE),
+        0x0,
+        0x0,
+        0x0,
+        0x0,
+      },
+      SYNQUACER_SCB_SMMU_BASE,
+      SYNQUACER_SCB_SMMU_SIZE,
+      EFI_ACPI_IORT_SMMUv1v2_MODEL_MMU500,
+      EFI_ACPI_IORT_SMMUv1v2_FLAG_COH_WALK,
+      FIELD_OFFSET(EFI_ACPI_6_0_IO_REMAPPING_SMMU_NODE,
+                   SMMU_NSgIrpt),
+      0x8,
+      sizeof(EFI_ACPI_6_0_IO_REMAPPING_SMMU_NODE),
+      0x0,
+      0x0,
+      228,
+      EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL,
+      0x0,
+      0x0,
+    }, {
+      { 228, EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL, },
+      { 228, EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL, },
+      { 228, EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL, },
+      { 228, EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL, },
+      { 228, EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL, },
+      { 228, EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL, },
+      { 228, EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL, },
+      { 228, EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL, },
+    },
+  }, {
+    {
+      // NETSEC named component node
+      {
+        {
+          EFI_ACPI_IORT_TYPE_NAMED_COMP,
+          sizeof(SYNQUACER_NC_NODE),
+          0x0,
+          0x0,
+          0x1,
+          FIELD_OFFSET(SYNQUACER_NC_NODE, RcIdMapping),
+        },
+        0x0,
+        EFI_ACPI_IORT_MEM_ACCESS_PROP_CCA,
+        0x0,
+        0x0,
+        EFI_ACPI_IORT_MEM_ACCESS_FLAGS_CPM |
+        EFI_ACPI_IORT_MEM_ACCESS_FLAGS_DACS,
+        40,
+      }, {
+        "\\_SB_.NET0"
+      }, {
+        0x0,
+        0x0,
+        0x0,
+        FIELD_OFFSET(SYNQUACER_IO_REMAPPING_STRUCTURE, SmmuNode),
+        EFI_ACPI_IORT_ID_MAPPING_FLAGS_SINGLE
+      }
+    }, {
+      // eMMC named component node
+      {
+        {
+          EFI_ACPI_IORT_TYPE_NAMED_COMP,
+          sizeof(SYNQUACER_NC_NODE),
+          0x0,
+          0x0,
+          0x1,
+          FIELD_OFFSET(SYNQUACER_NC_NODE, RcIdMapping),
+        },
+        0x0,
+        EFI_ACPI_IORT_MEM_ACCESS_PROP_CCA,
+        0x0,
+        0x0,
+        EFI_ACPI_IORT_MEM_ACCESS_FLAGS_CPM |
+        EFI_ACPI_IORT_MEM_ACCESS_FLAGS_DACS,
+        40,
+      }, {
+        "\\_SB_.MMC0"
+      }, {
+        0x0,
+        0x0,
+        0x0,
+        FIELD_OFFSET(SYNQUACER_IO_REMAPPING_STRUCTURE, SmmuNode),
+        EFI_ACPI_IORT_ID_MAPPING_FLAGS_SINGLE
+      }
+    }
   }
 };
 
-- 
2.17.0



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

* Re: [PATCH edk2-platforms 1/3] Silicon/SynQuacerPciHostBridgeLib: add workaround for PCIe MMIO64
  2018-05-30 18:19 ` [PATCH edk2-platforms 1/3] Silicon/SynQuacerPciHostBridgeLib: add workaround for PCIe MMIO64 Ard Biesheuvel
@ 2018-05-31  9:11   ` Leif Lindholm
  2018-05-31  9:17     ` Ard Biesheuvel
  0 siblings, 1 reply; 12+ messages in thread
From: Leif Lindholm @ 2018-05-31  9:11 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel, masahisa.kojima

On Wed, May 30, 2018 at 08:19:27PM +0200, Ard Biesheuvel wrote:
> From: Masahisa KOJIMA <masahisa.kojima@linaro.org>
> 
> The current revision of SC2A11 contains PCIe bus issue.
> In MRd transaction, 1st/Last DW BE fields are not correctly set
> by hardware.
> 
> As a workaround, set TH bit and specify MSG_CODE in iATU.
> With this setup, the value specified as MSG_CODE is set to the
> 1st/Last DW BE fields and PCIe controller can emit the correct
> MRd TLP header.
> Same workaround was already included for MMIO32 region,
> MMIO64 region also requires this workaround.
> Some deivices, such as Samsong SSD 970 EVO, do not work
> without this modification.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Masahisa KOJIMA <masahisa.kojima@linaro.org>

Please add own S-o-b.

> ---
>  Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
> index e4679543cc66..227f9a725ce8 100644
> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
> @@ -359,8 +359,9 @@ PciInitControllerPost (
>        RootBridge->MemAbove4G.Base,
>        RootBridge->MemAbove4G.Base,
>        RootBridge->MemAbove4G.Limit - RootBridge->MemAbove4G.Base + 1,
> -      IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TYPE_MEM,
> -      0);
> +      IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TYPE_MEM |
> +      IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TH,
> +      IATU_REGION_CTRL_2_OFF_OUTBOUND_0_MSG_CODE_32BIT);

Hmm ...
This fix clearly needs to go in. But since this is working around a
bug in first-revision silicon, should we not have something
conditional here?

/
    Leif

>    }
>  
>    // enable link
> -- 
> 2.17.0
> 


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

* Re: [PATCH edk2-platforms 2/3] Silicon/Socionext/SynQuacer/Stage2Tables: add north SMMU level 3 table
  2018-05-30 18:19 ` [PATCH edk2-platforms 2/3] Silicon/Socionext/SynQuacer/Stage2Tables: add north SMMU level 3 table Ard Biesheuvel
@ 2018-05-31  9:14   ` Leif Lindholm
  2018-06-01 10:18     ` Ard Biesheuvel
  0 siblings, 1 reply; 12+ messages in thread
From: Leif Lindholm @ 2018-05-31  9:14 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel, masahisa.kojima

On Wed, May 30, 2018 at 08:19:28PM +0200, Ard Biesheuvel wrote:
> Extend the static stage 2 page tables with a set of level 3 tables that
> describe the ECAM space in a manner that allows the north SMMU to be used
> to make the ECAM space appear sane to the CPUs.
> 
> It is up to the secure firmware to manipulate the north SMMU page tables

s/secure/EL3/ or s/secure/Secure/?

With either of those:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>


> so that the level 2 block entries corresponding with busses #0 .. #1 in
> the respective config spaces of PCI0 and PCI1 are replaced with table
> entries pointing to the level 3 tables added by this patch.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  Silicon/Socionext/SynQuacer/Stage2Tables/Stage2Tables.S | 35 +++++++++++++++-----
>  1 file changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/Silicon/Socionext/SynQuacer/Stage2Tables/Stage2Tables.S b/Silicon/Socionext/SynQuacer/Stage2Tables/Stage2Tables.S
> index 313ef3c56abc..af55f27bca47 100644
> --- a/Silicon/Socionext/SynQuacer/Stage2Tables/Stage2Tables.S
> +++ b/Silicon/Socionext/SynQuacer/Stage2Tables/Stage2Tables.S
> @@ -20,15 +20,17 @@
>   *   the SoC.
>   */
>  
> -#define TT_S2_CONT_SHIFT          52
> -#define TT_S2_AF                  (0x1 << 10)
> -#define TT_S2_SH_NON_SHAREABLE    (0x0 << 8)
> -#define TT_S2_AP_RW               (0x3 << 6)
> -#define TT_S2_MEMATTR_DEVICE_nGRE (0x2 << 2)
> -#define TT_S2_MEMATTR_MEMORY_WB   (0xf << 2)
> -#define TT_S2_TABLE               (0x3 << 0)
> -#define TT_S2_L3_PAGE             (0x1 << 1)
> -#define TT_S2_VALID               (0x1 << 0)
> +#define TT_S2_CONT_SHIFT            52
> +#define TT_S2_AF                    (0x1 << 10)
> +#define TT_S2_SH_NON_SHAREABLE      (0x0 << 8)
> +#define TT_S2_AP_RO                 (0x1 << 6)
> +#define TT_S2_AP_RW                 (0x3 << 6)
> +#define TT_S2_MEMATTR_DEVICE_nGRE   (0x2 << 2)
> +#define TT_S2_MEMATTR_DEVICE_nGnRE  (0x1 << 2)
> +#define TT_S2_MEMATTR_MEMORY_WB     (0xf << 2)
> +#define TT_S2_TABLE                 (0x3 << 0)
> +#define TT_S2_L3_PAGE               (0x1 << 1)
> +#define TT_S2_VALID                 (0x1 << 0)
>  
>    .altmacro
>    .macro    for, start, count, do, arg2, arg3, arg4
> @@ -58,6 +60,12 @@
>              TT_S2_L3_PAGE | TT_S2_VALID | (\cont << TT_S2_CONT_SHIFT)
>    .endm
>  
> +  .macro    smmu_l3_entry, base, offset=0, ignore=0
> +  .quad     ((\base << 12) + \offset) | TT_S2_AF | TT_S2_AP_RO | \
> +            TT_S2_SH_NON_SHAREABLE | TT_S2_MEMATTR_DEVICE_nGnRE | \
> +            TT_S2_L3_PAGE | TT_S2_VALID
> +  .endm
> +
>    .section  ".rodata", "a", %progbits
>    /* level 1 */
>    s2_mem_entry  0      /* 0x0000_0000 - 0x3fff_ffff */
> @@ -86,3 +94,12 @@
>  3:for     0,   8, s2_l3_entry, 0x70000000
>    for     0,   8, s2_l3_entry, 0x70010000  /* hide device #1 */
>    for     0, 496, s2_l3_entry, 0x70010000, 1
> +
> +  /* level 3 for north SMMU */
> +  .org    0x6000
> +  for     0,   8, smmu_l3_entry, 0xc00060000000
> +  for     0,   8, smmu_l3_entry, 0xc00060010000  /* hide device #1 */
> +  for     0, 496, smmu_l3_entry, 0xc00060010000
> +  for     0,   8, smmu_l3_entry, 0x800070000000
> +  for     0,   8, smmu_l3_entry, 0x800070010000  /* hide device #1 */
> +  for     0, 496, smmu_l3_entry, 0x800070010000
> -- 
> 2.17.0
> 


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

* Re: [PATCH edk2-platforms 3/3] Silicon/SynQuacer/AcpiTables: add NETSEC/eMMC SMMU to the IORT
  2018-05-30 18:19 ` [PATCH edk2-platforms 3/3] Silicon/SynQuacer/AcpiTables: add NETSEC/eMMC SMMU to the IORT Ard Biesheuvel
@ 2018-05-31  9:16   ` Leif Lindholm
  2018-10-12 10:03     ` Ard Biesheuvel
  0 siblings, 1 reply; 12+ messages in thread
From: Leif Lindholm @ 2018-05-31  9:16 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel, masahisa.kojima

On Wed, May 30, 2018 at 08:19:29PM +0200, Ard Biesheuvel wrote:
> Add a description of the SMMU that sits in front of the NETSEC and
> eMMC controllers to the IORT table so that ACPI based OSes can
> utilize it.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Looks reasonable to me.
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  Silicon/Socionext/SynQuacer/AcpiTables/Iort.aslc | 109 +++++++++++++++++++-
>  1 file changed, 107 insertions(+), 2 deletions(-)
> 
> diff --git a/Silicon/Socionext/SynQuacer/AcpiTables/Iort.aslc b/Silicon/Socionext/SynQuacer/AcpiTables/Iort.aslc
> index 92c485f8006f..3f2aaa3d8858 100644
> --- a/Silicon/Socionext/SynQuacer/AcpiTables/Iort.aslc
> +++ b/Silicon/Socionext/SynQuacer/AcpiTables/Iort.aslc
> @@ -13,6 +13,7 @@
>  **/
>  
>  #include <IndustryStandard/IoRemappingTable.h>
> +#include <Platform/MemoryMap.h>
>  
>  #include "AcpiTables.h"
>  
> @@ -29,10 +30,23 @@ typedef struct {
>    EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE        RcIdMapping;
>  } SYNQUACER_RC_NODE;
>  
> +typedef struct {
> +  EFI_ACPI_6_0_IO_REMAPPING_SMMU_NODE       Node;
> +  EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT        Context[8];
> +} SYNQUACER_SMMU_NODE;
> +
> +typedef struct {
> +  EFI_ACPI_6_0_IO_REMAPPING_NAMED_COMP_NODE Node;
> +  CONST CHAR8                               Name[11];
> +  EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE        RcIdMapping;
> +} SYNQUACER_NC_NODE;
> +
>  typedef struct {
>    EFI_ACPI_6_0_IO_REMAPPING_TABLE           Iort;
>    SYNQUACER_ITS_NODE                        ItsNode;
>    SYNQUACER_RC_NODE                         RcNode[2];
> +  SYNQUACER_SMMU_NODE                       SmmuNode;
> +  SYNQUACER_NC_NODE                         NamedCompNode[2];
>  } SYNQUACER_IO_REMAPPING_STRUCTURE;
>  
>  #define __SYNQUACER_ID_MAPPING(In, Num, Out, Ref, Flags)    \
> @@ -49,7 +63,7 @@ STATIC SYNQUACER_IO_REMAPPING_STRUCTURE Iort = {
>      __ACPI_HEADER(EFI_ACPI_6_0_IO_REMAPPING_TABLE_SIGNATURE,
>                    SYNQUACER_IO_REMAPPING_STRUCTURE,
>                    EFI_ACPI_IO_REMAPPING_TABLE_REVISION),
> -    3,                                              // NumNodes
> +    6,                                              // NumNodes
>      sizeof(EFI_ACPI_6_0_IO_REMAPPING_TABLE),        // NodeOffset
>      0                                               // Reserved
>    }, {
> @@ -94,7 +108,7 @@ STATIC SYNQUACER_IO_REMAPPING_STRUCTURE Iort = {
>        //
>        __SYNQUACER_ID_MAPPING(0x0, 0x0, 0x0, ItsNode,
>                               EFI_ACPI_IORT_ID_MAPPING_FLAGS_SINGLE),
> -  }, {
> +    }, {
>        // PciRcNode
>        {
>          {
> @@ -121,6 +135,97 @@ STATIC SYNQUACER_IO_REMAPPING_STRUCTURE Iort = {
>        __SYNQUACER_ID_MAPPING(0x0, 0x0, 0x0, ItsNode,
>                               EFI_ACPI_IORT_ID_MAPPING_FLAGS_SINGLE),
>      }
> +  }, {
> +    // NETSEC/eMMC SMMU node
> +    {
> +      {
> +        EFI_ACPI_IORT_TYPE_SMMUv1v2,
> +        sizeof(SYNQUACER_SMMU_NODE),
> +        0x0,
> +        0x0,
> +        0x0,
> +        0x0,
> +      },
> +      SYNQUACER_SCB_SMMU_BASE,
> +      SYNQUACER_SCB_SMMU_SIZE,
> +      EFI_ACPI_IORT_SMMUv1v2_MODEL_MMU500,
> +      EFI_ACPI_IORT_SMMUv1v2_FLAG_COH_WALK,
> +      FIELD_OFFSET(EFI_ACPI_6_0_IO_REMAPPING_SMMU_NODE,
> +                   SMMU_NSgIrpt),
> +      0x8,
> +      sizeof(EFI_ACPI_6_0_IO_REMAPPING_SMMU_NODE),
> +      0x0,
> +      0x0,
> +      228,
> +      EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL,
> +      0x0,
> +      0x0,
> +    }, {
> +      { 228, EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL, },
> +      { 228, EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL, },
> +      { 228, EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL, },
> +      { 228, EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL, },
> +      { 228, EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL, },
> +      { 228, EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL, },
> +      { 228, EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL, },
> +      { 228, EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL, },
> +    },
> +  }, {
> +    {
> +      // NETSEC named component node
> +      {
> +        {
> +          EFI_ACPI_IORT_TYPE_NAMED_COMP,
> +          sizeof(SYNQUACER_NC_NODE),
> +          0x0,
> +          0x0,
> +          0x1,
> +          FIELD_OFFSET(SYNQUACER_NC_NODE, RcIdMapping),
> +        },
> +        0x0,
> +        EFI_ACPI_IORT_MEM_ACCESS_PROP_CCA,
> +        0x0,
> +        0x0,
> +        EFI_ACPI_IORT_MEM_ACCESS_FLAGS_CPM |
> +        EFI_ACPI_IORT_MEM_ACCESS_FLAGS_DACS,
> +        40,
> +      }, {
> +        "\\_SB_.NET0"
> +      }, {
> +        0x0,
> +        0x0,
> +        0x0,
> +        FIELD_OFFSET(SYNQUACER_IO_REMAPPING_STRUCTURE, SmmuNode),
> +        EFI_ACPI_IORT_ID_MAPPING_FLAGS_SINGLE
> +      }
> +    }, {
> +      // eMMC named component node
> +      {
> +        {
> +          EFI_ACPI_IORT_TYPE_NAMED_COMP,
> +          sizeof(SYNQUACER_NC_NODE),
> +          0x0,
> +          0x0,
> +          0x1,
> +          FIELD_OFFSET(SYNQUACER_NC_NODE, RcIdMapping),
> +        },
> +        0x0,
> +        EFI_ACPI_IORT_MEM_ACCESS_PROP_CCA,
> +        0x0,
> +        0x0,
> +        EFI_ACPI_IORT_MEM_ACCESS_FLAGS_CPM |
> +        EFI_ACPI_IORT_MEM_ACCESS_FLAGS_DACS,
> +        40,
> +      }, {
> +        "\\_SB_.MMC0"
> +      }, {
> +        0x0,
> +        0x0,
> +        0x0,
> +        FIELD_OFFSET(SYNQUACER_IO_REMAPPING_STRUCTURE, SmmuNode),
> +        EFI_ACPI_IORT_ID_MAPPING_FLAGS_SINGLE
> +      }
> +    }
>    }
>  };
>  
> -- 
> 2.17.0
> 


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

* Re: [PATCH edk2-platforms 1/3] Silicon/SynQuacerPciHostBridgeLib: add workaround for PCIe MMIO64
  2018-05-31  9:11   ` Leif Lindholm
@ 2018-05-31  9:17     ` Ard Biesheuvel
  2018-05-31  9:46       ` Leif Lindholm
  0 siblings, 1 reply; 12+ messages in thread
From: Ard Biesheuvel @ 2018-05-31  9:17 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: edk2-devel@lists.01.org, Masahisa Kojima

On 31 May 2018 at 11:11, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Wed, May 30, 2018 at 08:19:27PM +0200, Ard Biesheuvel wrote:
>> From: Masahisa KOJIMA <masahisa.kojima@linaro.org>
>>
>> The current revision of SC2A11 contains PCIe bus issue.
>> In MRd transaction, 1st/Last DW BE fields are not correctly set
>> by hardware.
>>
>> As a workaround, set TH bit and specify MSG_CODE in iATU.
>> With this setup, the value specified as MSG_CODE is set to the
>> 1st/Last DW BE fields and PCIe controller can emit the correct
>> MRd TLP header.
>> Same workaround was already included for MMIO32 region,
>> MMIO64 region also requires this workaround.
>> Some deivices, such as Samsong SSD 970 EVO, do not work
>> without this modification.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Masahisa KOJIMA <masahisa.kojima@linaro.org>
>
> Please add own S-o-b.
>
>> ---
>>  Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
>> index e4679543cc66..227f9a725ce8 100644
>> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
>> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
>> @@ -359,8 +359,9 @@ PciInitControllerPost (
>>        RootBridge->MemAbove4G.Base,
>>        RootBridge->MemAbove4G.Base,
>>        RootBridge->MemAbove4G.Limit - RootBridge->MemAbove4G.Base + 1,
>> -      IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TYPE_MEM,
>> -      0);
>> +      IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TYPE_MEM |
>> +      IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TH,
>> +      IATU_REGION_CTRL_2_OFF_OUTBOUND_0_MSG_CODE_32BIT);
>
> Hmm ...
> This fix clearly needs to go in. But since this is working around a
> bug in first-revision silicon, should we not have something
> conditional here?
>

In theory, yes. In practice, we have no idea yet whether a fixed
revision will ever materialize (the limited respin for the next
revision does not address this issue afaik), nor do we have any idea
how to distinguish them at runtime. Also, it is unlikely that we will
need to run older firmware builds on these new chips. So for the time
being, I think it is reasonable to apply this unconditionally (like we
do for the MMIO32 region already)


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

* Re: [PATCH edk2-platforms 1/3] Silicon/SynQuacerPciHostBridgeLib: add workaround for PCIe MMIO64
  2018-05-31  9:17     ` Ard Biesheuvel
@ 2018-05-31  9:46       ` Leif Lindholm
  2018-05-31 10:49         ` Ard Biesheuvel
  0 siblings, 1 reply; 12+ messages in thread
From: Leif Lindholm @ 2018-05-31  9:46 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel@lists.01.org, Masahisa Kojima

On Thu, May 31, 2018 at 11:17:47AM +0200, Ard Biesheuvel wrote:
> On 31 May 2018 at 11:11, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > On Wed, May 30, 2018 at 08:19:27PM +0200, Ard Biesheuvel wrote:
> >> From: Masahisa KOJIMA <masahisa.kojima@linaro.org>
> >>
> >> The current revision of SC2A11 contains PCIe bus issue.
> >> In MRd transaction, 1st/Last DW BE fields are not correctly set
> >> by hardware.
> >>
> >> As a workaround, set TH bit and specify MSG_CODE in iATU.
> >> With this setup, the value specified as MSG_CODE is set to the
> >> 1st/Last DW BE fields and PCIe controller can emit the correct
> >> MRd TLP header.
> >> Same workaround was already included for MMIO32 region,
> >> MMIO64 region also requires this workaround.
> >> Some deivices, such as Samsong SSD 970 EVO, do not work
> >> without this modification.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Masahisa KOJIMA <masahisa.kojima@linaro.org>
> >
> > Please add own S-o-b.
> >
> >> ---
> >>  Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c | 5 +++--
> >>  1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
> >> index e4679543cc66..227f9a725ce8 100644
> >> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
> >> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
> >> @@ -359,8 +359,9 @@ PciInitControllerPost (
> >>        RootBridge->MemAbove4G.Base,
> >>        RootBridge->MemAbove4G.Base,
> >>        RootBridge->MemAbove4G.Limit - RootBridge->MemAbove4G.Base + 1,
> >> -      IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TYPE_MEM,
> >> -      0);
> >> +      IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TYPE_MEM |
> >> +      IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TH,
> >> +      IATU_REGION_CTRL_2_OFF_OUTBOUND_0_MSG_CODE_32BIT);
> >
> > Hmm ...
> > This fix clearly needs to go in. But since this is working around a
> > bug in first-revision silicon, should we not have something
> > conditional here?
> 
> In theory, yes. In practice, we have no idea yet whether a fixed
> revision will ever materialize (the limited respin for the next
> revision does not address this issue afaik), nor do we have any idea
> how to distinguish them at runtime.

This is clearly a problem, but even if we can't distinguish them, that
could be made a menu setting (since getting it wrong won't prevent you
from going into the menu and changing it back).

> Also, it is unlikely that we will
> need to run older firmware builds on these new chips. So for the time
> being, I think it is reasonable to apply this unconditionally (like we
> do for the MMIO32 region already)

My opinion stands. But if we already haven't conditionalised the
MMIO32 workaround, then I guess we don't need to tear up the world at
this point.

But I'll just file this forward request that workarounds for hardware
bugs are always conditionalised in future, so that
1) it's crystal clear what bits are workarounds
2) they can more easily be disabled when functional hardware appears.

Anyway
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

/
    Leif


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

* Re: [PATCH edk2-platforms 1/3] Silicon/SynQuacerPciHostBridgeLib: add workaround for PCIe MMIO64
  2018-05-31  9:46       ` Leif Lindholm
@ 2018-05-31 10:49         ` Ard Biesheuvel
  0 siblings, 0 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2018-05-31 10:49 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: edk2-devel@lists.01.org, Masahisa Kojima

On 31 May 2018 at 11:46, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Thu, May 31, 2018 at 11:17:47AM +0200, Ard Biesheuvel wrote:
>> On 31 May 2018 at 11:11, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>> > On Wed, May 30, 2018 at 08:19:27PM +0200, Ard Biesheuvel wrote:
>> >> From: Masahisa KOJIMA <masahisa.kojima@linaro.org>
>> >>
>> >> The current revision of SC2A11 contains PCIe bus issue.
>> >> In MRd transaction, 1st/Last DW BE fields are not correctly set
>> >> by hardware.
>> >>
>> >> As a workaround, set TH bit and specify MSG_CODE in iATU.
>> >> With this setup, the value specified as MSG_CODE is set to the
>> >> 1st/Last DW BE fields and PCIe controller can emit the correct
>> >> MRd TLP header.
>> >> Same workaround was already included for MMIO32 region,
>> >> MMIO64 region also requires this workaround.
>> >> Some deivices, such as Samsong SSD 970 EVO, do not work
>> >> without this modification.
>> >>
>> >> Contributed-under: TianoCore Contribution Agreement 1.1
>> >> Signed-off-by: Masahisa KOJIMA <masahisa.kojima@linaro.org>
>> >
>> > Please add own S-o-b.
>> >
>> >> ---
>> >>  Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c | 5 +++--
>> >>  1 file changed, 3 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
>> >> index e4679543cc66..227f9a725ce8 100644
>> >> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
>> >> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
>> >> @@ -359,8 +359,9 @@ PciInitControllerPost (
>> >>        RootBridge->MemAbove4G.Base,
>> >>        RootBridge->MemAbove4G.Base,
>> >>        RootBridge->MemAbove4G.Limit - RootBridge->MemAbove4G.Base + 1,
>> >> -      IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TYPE_MEM,
>> >> -      0);
>> >> +      IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TYPE_MEM |
>> >> +      IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TH,
>> >> +      IATU_REGION_CTRL_2_OFF_OUTBOUND_0_MSG_CODE_32BIT);
>> >
>> > Hmm ...
>> > This fix clearly needs to go in. But since this is working around a
>> > bug in first-revision silicon, should we not have something
>> > conditional here?
>>
>> In theory, yes. In practice, we have no idea yet whether a fixed
>> revision will ever materialize (the limited respin for the next
>> revision does not address this issue afaik), nor do we have any idea
>> how to distinguish them at runtime.
>
> This is clearly a problem, but even if we can't distinguish them, that
> could be made a menu setting (since getting it wrong won't prevent you
> from going into the menu and changing it back).
>
>> Also, it is unlikely that we will
>> need to run older firmware builds on these new chips. So for the time
>> being, I think it is reasonable to apply this unconditionally (like we
>> do for the MMIO32 region already)
>
> My opinion stands. But if we already haven't conditionalised the
> MMIO32 workaround, then I guess we don't need to tear up the world at
> this point.
>
> But I'll just file this forward request that workarounds for hardware
> bugs are always conditionalised in future, so that
> 1) it's crystal clear what bits are workarounds
> 2) they can more easily be disabled when functional hardware appears.
>
> Anyway
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>

Thanks

Pushed as c9be7b11ea10 (with my S-o-b added)


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

* Re: [PATCH edk2-platforms 2/3] Silicon/Socionext/SynQuacer/Stage2Tables: add north SMMU level 3 table
  2018-05-31  9:14   ` Leif Lindholm
@ 2018-06-01 10:18     ` Ard Biesheuvel
  0 siblings, 0 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2018-06-01 10:18 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: edk2-devel@lists.01.org, Masahisa Kojima

On 31 May 2018 at 11:14, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Wed, May 30, 2018 at 08:19:28PM +0200, Ard Biesheuvel wrote:
>> Extend the static stage 2 page tables with a set of level 3 tables that
>> describe the ECAM space in a manner that allows the north SMMU to be used
>> to make the ECAM space appear sane to the CPUs.
>>
>> It is up to the secure firmware to manipulate the north SMMU page tables
>
> s/secure/EL3/ or s/secure/Secure/?
>
> With either of those:
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>

Thanks

Pushed as ba32985a0631

>
>> so that the level 2 block entries corresponding with busses #0 .. #1 in
>> the respective config spaces of PCI0 and PCI1 are replaced with table
>> entries pointing to the level 3 tables added by this patch.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  Silicon/Socionext/SynQuacer/Stage2Tables/Stage2Tables.S | 35 +++++++++++++++-----
>>  1 file changed, 26 insertions(+), 9 deletions(-)
>>
>> diff --git a/Silicon/Socionext/SynQuacer/Stage2Tables/Stage2Tables.S b/Silicon/Socionext/SynQuacer/Stage2Tables/Stage2Tables.S
>> index 313ef3c56abc..af55f27bca47 100644
>> --- a/Silicon/Socionext/SynQuacer/Stage2Tables/Stage2Tables.S
>> +++ b/Silicon/Socionext/SynQuacer/Stage2Tables/Stage2Tables.S
>> @@ -20,15 +20,17 @@
>>   *   the SoC.
>>   */
>>
>> -#define TT_S2_CONT_SHIFT          52
>> -#define TT_S2_AF                  (0x1 << 10)
>> -#define TT_S2_SH_NON_SHAREABLE    (0x0 << 8)
>> -#define TT_S2_AP_RW               (0x3 << 6)
>> -#define TT_S2_MEMATTR_DEVICE_nGRE (0x2 << 2)
>> -#define TT_S2_MEMATTR_MEMORY_WB   (0xf << 2)
>> -#define TT_S2_TABLE               (0x3 << 0)
>> -#define TT_S2_L3_PAGE             (0x1 << 1)
>> -#define TT_S2_VALID               (0x1 << 0)
>> +#define TT_S2_CONT_SHIFT            52
>> +#define TT_S2_AF                    (0x1 << 10)
>> +#define TT_S2_SH_NON_SHAREABLE      (0x0 << 8)
>> +#define TT_S2_AP_RO                 (0x1 << 6)
>> +#define TT_S2_AP_RW                 (0x3 << 6)
>> +#define TT_S2_MEMATTR_DEVICE_nGRE   (0x2 << 2)
>> +#define TT_S2_MEMATTR_DEVICE_nGnRE  (0x1 << 2)
>> +#define TT_S2_MEMATTR_MEMORY_WB     (0xf << 2)
>> +#define TT_S2_TABLE                 (0x3 << 0)
>> +#define TT_S2_L3_PAGE               (0x1 << 1)
>> +#define TT_S2_VALID                 (0x1 << 0)
>>
>>    .altmacro
>>    .macro    for, start, count, do, arg2, arg3, arg4
>> @@ -58,6 +60,12 @@
>>              TT_S2_L3_PAGE | TT_S2_VALID | (\cont << TT_S2_CONT_SHIFT)
>>    .endm
>>
>> +  .macro    smmu_l3_entry, base, offset=0, ignore=0
>> +  .quad     ((\base << 12) + \offset) | TT_S2_AF | TT_S2_AP_RO | \
>> +            TT_S2_SH_NON_SHAREABLE | TT_S2_MEMATTR_DEVICE_nGnRE | \
>> +            TT_S2_L3_PAGE | TT_S2_VALID
>> +  .endm
>> +
>>    .section  ".rodata", "a", %progbits
>>    /* level 1 */
>>    s2_mem_entry  0      /* 0x0000_0000 - 0x3fff_ffff */
>> @@ -86,3 +94,12 @@
>>  3:for     0,   8, s2_l3_entry, 0x70000000
>>    for     0,   8, s2_l3_entry, 0x70010000  /* hide device #1 */
>>    for     0, 496, s2_l3_entry, 0x70010000, 1
>> +
>> +  /* level 3 for north SMMU */
>> +  .org    0x6000
>> +  for     0,   8, smmu_l3_entry, 0xc00060000000
>> +  for     0,   8, smmu_l3_entry, 0xc00060010000  /* hide device #1 */
>> +  for     0, 496, smmu_l3_entry, 0xc00060010000
>> +  for     0,   8, smmu_l3_entry, 0x800070000000
>> +  for     0,   8, smmu_l3_entry, 0x800070010000  /* hide device #1 */
>> +  for     0, 496, smmu_l3_entry, 0x800070010000
>> --
>> 2.17.0
>>


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

* Re: [PATCH edk2-platforms 3/3] Silicon/SynQuacer/AcpiTables: add NETSEC/eMMC SMMU to the IORT
  2018-05-31  9:16   ` Leif Lindholm
@ 2018-10-12 10:03     ` Ard Biesheuvel
  0 siblings, 0 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2018-10-12 10:03 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: edk2-devel@lists.01.org, Masahisa Kojima

On 31 May 2018 at 11:16, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Wed, May 30, 2018 at 08:19:29PM +0200, Ard Biesheuvel wrote:
>> Add a description of the SMMU that sits in front of the NETSEC and
>> eMMC controllers to the IORT table so that ACPI based OSes can
>> utilize it.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> Looks reasonable to me.
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>

As it turns out, we don't actually need a SCP firmware update for
this, and the other remaining issue (only 40 address bits wired up
while the IP can driver 48) was fixed both in the Linux driver (and
backported) and in the DMA layer, so let's enable this piece of
functionality now.

Note that DT boot will not enable the SMMU.

Pushed as 8c3914c90ecd..a82113852ae1


>> ---
>>  Silicon/Socionext/SynQuacer/AcpiTables/Iort.aslc | 109 +++++++++++++++++++-
>>  1 file changed, 107 insertions(+), 2 deletions(-)
>>
>> diff --git a/Silicon/Socionext/SynQuacer/AcpiTables/Iort.aslc b/Silicon/Socionext/SynQuacer/AcpiTables/Iort.aslc
>> index 92c485f8006f..3f2aaa3d8858 100644
>> --- a/Silicon/Socionext/SynQuacer/AcpiTables/Iort.aslc
>> +++ b/Silicon/Socionext/SynQuacer/AcpiTables/Iort.aslc
>> @@ -13,6 +13,7 @@
>>  **/
>>
>>  #include <IndustryStandard/IoRemappingTable.h>
>> +#include <Platform/MemoryMap.h>
>>
>>  #include "AcpiTables.h"
>>
>> @@ -29,10 +30,23 @@ typedef struct {
>>    EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE        RcIdMapping;
>>  } SYNQUACER_RC_NODE;
>>
>> +typedef struct {
>> +  EFI_ACPI_6_0_IO_REMAPPING_SMMU_NODE       Node;
>> +  EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT        Context[8];
>> +} SYNQUACER_SMMU_NODE;
>> +
>> +typedef struct {
>> +  EFI_ACPI_6_0_IO_REMAPPING_NAMED_COMP_NODE Node;
>> +  CONST CHAR8                               Name[11];
>> +  EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE        RcIdMapping;
>> +} SYNQUACER_NC_NODE;
>> +
>>  typedef struct {
>>    EFI_ACPI_6_0_IO_REMAPPING_TABLE           Iort;
>>    SYNQUACER_ITS_NODE                        ItsNode;
>>    SYNQUACER_RC_NODE                         RcNode[2];
>> +  SYNQUACER_SMMU_NODE                       SmmuNode;
>> +  SYNQUACER_NC_NODE                         NamedCompNode[2];
>>  } SYNQUACER_IO_REMAPPING_STRUCTURE;
>>
>>  #define __SYNQUACER_ID_MAPPING(In, Num, Out, Ref, Flags)    \
>> @@ -49,7 +63,7 @@ STATIC SYNQUACER_IO_REMAPPING_STRUCTURE Iort = {
>>      __ACPI_HEADER(EFI_ACPI_6_0_IO_REMAPPING_TABLE_SIGNATURE,
>>                    SYNQUACER_IO_REMAPPING_STRUCTURE,
>>                    EFI_ACPI_IO_REMAPPING_TABLE_REVISION),
>> -    3,                                              // NumNodes
>> +    6,                                              // NumNodes
>>      sizeof(EFI_ACPI_6_0_IO_REMAPPING_TABLE),        // NodeOffset
>>      0                                               // Reserved
>>    }, {
>> @@ -94,7 +108,7 @@ STATIC SYNQUACER_IO_REMAPPING_STRUCTURE Iort = {
>>        //
>>        __SYNQUACER_ID_MAPPING(0x0, 0x0, 0x0, ItsNode,
>>                               EFI_ACPI_IORT_ID_MAPPING_FLAGS_SINGLE),
>> -  }, {
>> +    }, {
>>        // PciRcNode
>>        {
>>          {
>> @@ -121,6 +135,97 @@ STATIC SYNQUACER_IO_REMAPPING_STRUCTURE Iort = {
>>        __SYNQUACER_ID_MAPPING(0x0, 0x0, 0x0, ItsNode,
>>                               EFI_ACPI_IORT_ID_MAPPING_FLAGS_SINGLE),
>>      }
>> +  }, {
>> +    // NETSEC/eMMC SMMU node
>> +    {
>> +      {
>> +        EFI_ACPI_IORT_TYPE_SMMUv1v2,
>> +        sizeof(SYNQUACER_SMMU_NODE),
>> +        0x0,
>> +        0x0,
>> +        0x0,
>> +        0x0,
>> +      },
>> +      SYNQUACER_SCB_SMMU_BASE,
>> +      SYNQUACER_SCB_SMMU_SIZE,
>> +      EFI_ACPI_IORT_SMMUv1v2_MODEL_MMU500,
>> +      EFI_ACPI_IORT_SMMUv1v2_FLAG_COH_WALK,
>> +      FIELD_OFFSET(EFI_ACPI_6_0_IO_REMAPPING_SMMU_NODE,
>> +                   SMMU_NSgIrpt),
>> +      0x8,
>> +      sizeof(EFI_ACPI_6_0_IO_REMAPPING_SMMU_NODE),
>> +      0x0,
>> +      0x0,
>> +      228,
>> +      EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL,
>> +      0x0,
>> +      0x0,
>> +    }, {
>> +      { 228, EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL, },
>> +      { 228, EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL, },
>> +      { 228, EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL, },
>> +      { 228, EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL, },
>> +      { 228, EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL, },
>> +      { 228, EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL, },
>> +      { 228, EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL, },
>> +      { 228, EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL, },
>> +    },
>> +  }, {
>> +    {
>> +      // NETSEC named component node
>> +      {
>> +        {
>> +          EFI_ACPI_IORT_TYPE_NAMED_COMP,
>> +          sizeof(SYNQUACER_NC_NODE),
>> +          0x0,
>> +          0x0,
>> +          0x1,
>> +          FIELD_OFFSET(SYNQUACER_NC_NODE, RcIdMapping),
>> +        },
>> +        0x0,
>> +        EFI_ACPI_IORT_MEM_ACCESS_PROP_CCA,
>> +        0x0,
>> +        0x0,
>> +        EFI_ACPI_IORT_MEM_ACCESS_FLAGS_CPM |
>> +        EFI_ACPI_IORT_MEM_ACCESS_FLAGS_DACS,
>> +        40,
>> +      }, {
>> +        "\\_SB_.NET0"
>> +      }, {
>> +        0x0,
>> +        0x0,
>> +        0x0,
>> +        FIELD_OFFSET(SYNQUACER_IO_REMAPPING_STRUCTURE, SmmuNode),
>> +        EFI_ACPI_IORT_ID_MAPPING_FLAGS_SINGLE
>> +      }
>> +    }, {
>> +      // eMMC named component node
>> +      {
>> +        {
>> +          EFI_ACPI_IORT_TYPE_NAMED_COMP,
>> +          sizeof(SYNQUACER_NC_NODE),
>> +          0x0,
>> +          0x0,
>> +          0x1,
>> +          FIELD_OFFSET(SYNQUACER_NC_NODE, RcIdMapping),
>> +        },
>> +        0x0,
>> +        EFI_ACPI_IORT_MEM_ACCESS_PROP_CCA,
>> +        0x0,
>> +        0x0,
>> +        EFI_ACPI_IORT_MEM_ACCESS_FLAGS_CPM |
>> +        EFI_ACPI_IORT_MEM_ACCESS_FLAGS_DACS,
>> +        40,
>> +      }, {
>> +        "\\_SB_.MMC0"
>> +      }, {
>> +        0x0,
>> +        0x0,
>> +        0x0,
>> +        FIELD_OFFSET(SYNQUACER_IO_REMAPPING_STRUCTURE, SmmuNode),
>> +        EFI_ACPI_IORT_ID_MAPPING_FLAGS_SINGLE
>> +      }
>> +    }
>>    }
>>  };
>>
>> --
>> 2.17.0
>>


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

end of thread, other threads:[~2018-10-12 10:03 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-05-30 18:19 [PATCH edk2-platforms 0/3] More SynQuacer updates Ard Biesheuvel
2018-05-30 18:19 ` [PATCH edk2-platforms 1/3] Silicon/SynQuacerPciHostBridgeLib: add workaround for PCIe MMIO64 Ard Biesheuvel
2018-05-31  9:11   ` Leif Lindholm
2018-05-31  9:17     ` Ard Biesheuvel
2018-05-31  9:46       ` Leif Lindholm
2018-05-31 10:49         ` Ard Biesheuvel
2018-05-30 18:19 ` [PATCH edk2-platforms 2/3] Silicon/Socionext/SynQuacer/Stage2Tables: add north SMMU level 3 table Ard Biesheuvel
2018-05-31  9:14   ` Leif Lindholm
2018-06-01 10:18     ` Ard Biesheuvel
2018-05-30 18:19 ` [PATCH edk2-platforms 3/3] Silicon/SynQuacer/AcpiTables: add NETSEC/eMMC SMMU to the IORT Ard Biesheuvel
2018-05-31  9:16   ` Leif Lindholm
2018-10-12 10:03     ` Ard Biesheuvel

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