[PATCH 1/5] armv8: fsl-layerscape: make some functions static

[PATCH 1/5] armv8: fsl-layerscape: make some functions static

[Laurentiu Tudor]

Some functions are not used outside this file, so make them static.

Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
---
 arch/arm/cpu/armv8/fsl-layerscape/icid.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/cpu/armv8/fsl-layerscape/icid.c b/arch/arm/cpu/armv8/fsl-layerscape/icid.c
index ad20d71717b3..c22e73253c3c 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/icid.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/icid.c
@@ -23,8 +23,8 @@ static void set_icid(struct icid_id_table *tbl, int size)
 			out_be32((u32 *)(tbl[i].reg_addr), tbl[i].reg);
 }
 
-#ifdef CONFIG_SYS_DPAA_FMAN
-void set_fman_icids(struct fman_icid_id_table *tbl, int size)
+#if defined(CONFIG_SYS_DPAA_FMAN) && !defined(CONFIG_SPL_BUILD)
+static void set_fman_icids(struct fman_icid_id_table *tbl, int size)
 {
 	int i;
 	ccsr_fman_t *fm = (void *)CFG_SYS_FSL_FM1_ADDR;
@@ -71,7 +71,7 @@ int fdt_set_iommu_prop(void *blob, int off, int smmu_ph, u32 *ids, int num_ids)
 	return 0;
 }
 
-int fdt_fixup_icid_tbl(void *blob, int smmu_ph,
+static int fdt_fixup_icid_tbl(void *blob, int smmu_ph,
 		       struct icid_id_table *tbl, int size)
 {
 	int i, err, off;
@@ -98,7 +98,7 @@ int fdt_fixup_icid_tbl(void *blob, int smmu_ph,
 }
 
 #ifdef CONFIG_SYS_DPAA_FMAN
-int get_fman_port_icid(int port_id, struct fman_icid_id_table *tbl,
+static int get_fman_port_icid(int port_id, struct fman_icid_id_table *tbl,
 		       const int size)
 {
 	int i;
@@ -111,7 +111,7 @@ int get_fman_port_icid(int port_id, struct fman_icid_id_table *tbl,
 	return -1;
 }
 
-void fdt_fixup_fman_port_icid_by_compat(void *blob, int smmu_ph,
+static void fdt_fixup_fman_port_icid_by_compat(void *blob, int smmu_ph,
 					const char *compat)
 {
 	int noff, len, icid;
@@ -140,7 +140,7 @@ void fdt_fixup_fman_port_icid_by_compat(void *blob, int smmu_ph,
 	}
 }
 
-void fdt_fixup_fman_icids(void *blob, int smmu_ph)
+static void fdt_fixup_fman_icids(void *blob, int smmu_ph)
 {
 	static const char * const compats[] = {
 		"fsl,fman-v3-port-oh",
-- 
2.17.1

[PATCH 2/5] drivers: net: fsl-mc: add support for MC reserved memory

[Laurentiu Tudor]

Add support for declaring in device tree the reserved memory ranges
required for MC. Since the MC firmware acts as any DMA master present
in the SoC, the reserved memory ranges need also be identity mapped
in the SMMU, so create the required 'iommu-addresses' property in
the reserved memory nodes.
For now this support is used only on LX2160A SoCs.

Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
---
 board/freescale/lx2160a/lx2160a.c |   1 +
 drivers/net/fsl-mc/mc.c           | 110 ++++++++++++++++++++++++++++++
 include/fsl-mc/fsl_mc.h           |   1 +
 3 files changed, 112 insertions(+)

diff --git a/board/freescale/lx2160a/lx2160a.c b/board/freescale/lx2160a/lx2160a.c
index d631a11ff667..688d81f04f64 100644
--- a/board/freescale/lx2160a/lx2160a.c
+++ b/board/freescale/lx2160a/lx2160a.c
@@ -834,6 +834,7 @@ int ft_board_setup(void *blob, struct bd_info *bd)
 #ifdef CONFIG_FSL_MC_ENET
 	fdt_fsl_mc_fixup_iommu_map_entry(blob);
 	fdt_fixup_board_enet(blob);
+	fdt_reserve_mc_mem(blob, 0x4000);
 #endif
 	fdt_fixup_icid(blob);
 
diff --git a/drivers/net/fsl-mc/mc.c b/drivers/net/fsl-mc/mc.c
index 984616fb65c0..f5c5057bec10 100644
--- a/drivers/net/fsl-mc/mc.c
+++ b/drivers/net/fsl-mc/mc.c
@@ -30,6 +30,8 @@
 #include <fsl-mc/fsl_qbman_portal.h>
 #include <fsl-mc/ldpaa_wriop.h>
 #include <net/ldpaa_eth.h>
+#include <asm/arch/cpu.h>
+#include <asm/arch-fsl-layerscape/fsl_icid.h>
 
 #define MC_RAM_BASE_ADDR_ALIGNMENT  (512UL * 1024 * 1024)
 #define MC_RAM_BASE_ADDR_ALIGNMENT_MASK	(~(MC_RAM_BASE_ADDR_ALIGNMENT - 1))
@@ -929,6 +931,114 @@ unsigned long mc_get_dram_block_size(void)
 	return dram_block_size;
 }
 
+/**
+ * Populate the device tree with MC reserved memory ranges.
+ */
+void fdt_reserve_mc_mem(void *blob, u32 mc_icid)
+{
+	u32 phandle, mc_ph;
+	int noff, ret, i;
+	char mem_name[16];
+	struct fdt_memory mc_mem_ranges[] = {
+		{
+			.start = 0,
+			.end = 0
+		},
+		{
+			.start = CFG_SYS_FSL_MC_BASE,
+			.end = CFG_SYS_FSL_MC_BASE + CFG_SYS_FSL_MC_SIZE - 1
+		},
+		{
+			.start = CFG_SYS_FSL_NI_BASE,
+			.end = CFG_SYS_FSL_NI_BASE + CFG_SYS_FSL_NI_SIZE - 1
+		},
+		{
+			.start = CFG_SYS_FSL_QBMAN_BASE,
+			.end = CFG_SYS_FSL_QBMAN_BASE +
+					CFG_SYS_FSL_QBMAN_SIZE - 1
+		},
+		{
+			.start = CFG_SYS_FSL_PEBUF_BASE,
+			.end = CFG_SYS_FSL_PEBUF_BASE +
+					CFG_SYS_FSL_PEBUF_SIZE - 1
+		},
+		{
+			.start = CFG_SYS_FSL_CCSR_BASE,
+			.end = CFG_SYS_FSL_CCSR_BASE + CFG_SYS_FSL_CCSR_SIZE - 1
+		}
+	};
+
+	mc_mem_ranges[0].start = gd->arch.resv_ram;
+	mc_mem_ranges[0].end = mc_mem_ranges[0].start +
+				mc_get_dram_block_size() - 1;
+
+	for (i = 0; i < ARRAY_SIZE(mc_mem_ranges); i++) {
+		noff = fdt_node_offset_by_compatible(blob, -1, "fsl,qoriq-mc");
+		if (noff < 0) {
+			printf("WARN: failed to get MC node: %d\n", noff);
+			return;
+		}
+		mc_ph = fdt_get_phandle(blob, noff);
+		if (!mc_ph) {
+			mc_ph = fdt_create_phandle(blob, noff);
+			if (!mc_ph) {
+				printf("WARN: failed to get MC node phandle\n");
+				return;
+			}
+		}
+
+		sprintf(mem_name, "mc-mem%d", i);
+		ret = fdtdec_add_reserved_memory(blob, mem_name,
+						 &mc_mem_ranges[i], NULL, 0,
+						 &phandle, 0);
+		if (ret < 0) {
+			printf("ERROR: failed to reserve MC memory: %d\n", ret);
+			return;
+		}
+
+		noff = fdt_node_offset_by_phandle(blob, phandle);
+		if (noff < 0) {
+			printf("ERROR: failed get resvmem node offset: %d\n",
+			       noff);
+			return;
+		}
+		ret = fdt_setprop_u32(blob, noff, "iommu-addresses", mc_ph);
+		if (ret < 0) {
+			printf("ERROR: failed to set 'iommu-addresses': %d\n",
+			       ret);
+			return;
+		}
+		ret = fdt_appendprop_u64(blob, noff, "iommu-addresses",
+					 mc_mem_ranges[i].start);
+		if (ret < 0) {
+			printf("ERROR: failed to set 'iommu-addresses': %d\n",
+			       ret);
+			return;
+		}
+		ret = fdt_appendprop_u64(blob, noff, "iommu-addresses",
+					 mc_mem_ranges[i].end -
+						mc_mem_ranges[i].start + 1);
+		if (ret < 0) {
+			printf("ERROR: failed to set 'iommu-addresses': %d\n",
+			       ret);
+			return;
+		}
+
+		noff = fdt_node_offset_by_phandle(blob, mc_ph);
+		if (noff < 0) {
+			printf("ERROR: failed get MC node offset: %d\n", noff);
+			return;
+		}
+		ret = fdt_appendprop_u32(blob, noff, "memory-region", phandle);
+		if (ret < 0) {
+			printf("ERROR: failed to set 'memory-region': %d\n",
+			       ret);
+		}
+	}
+
+	fdt_set_iommu_prop(blob, noff, fdt_get_smmu_phandle(blob), &mc_icid, 1);
+}
+
 int fsl_mc_ldpaa_init(struct bd_info *bis)
 {
 	int i;
diff --git a/include/fsl-mc/fsl_mc.h b/include/fsl-mc/fsl_mc.h
index c701dc1084b9..258738dfc8c2 100644
--- a/include/fsl-mc/fsl_mc.h
+++ b/include/fsl-mc/fsl_mc.h
@@ -66,6 +66,7 @@ int get_mc_boot_status(void);
 int get_dpl_apply_status(void);
 int is_lazy_dpl_addr_valid(void);
 void fdt_fixup_mc_ddr(u64 *base, u64 *size);
+void fdt_reserve_mc_mem(void *blob, u32 mc_icid);
 #ifdef CFG_SYS_LS_MC_DRAM_AIOP_IMG_OFFSET
 int get_aiop_apply_status(void);
 #endif
-- 
2.17.1

[PATCH 3/5] armv8: fsl-layerscape: create bypass smmu mapping for MC

[Laurentiu Tudor]

MC being a plain DMA master as any other device in the SoC and
being live at OS boot time, as soon as the SMMU is probed it
will immediately start triggering faults because there is no
mapping in the SMMU for the MC. Pre-create such a mapping in
the SMMU, being the OS's responsibility to preserve it.

Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
---
 arch/arm/cpu/armv8/fsl-layerscape/soc.c       | 26 ++++++++++++++++---
 .../asm/arch-fsl-layerscape/immap_lsch3.h     |  9 +++++++
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
index 3bfdc3f77431..870b99838ab5 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
@@ -376,6 +376,18 @@ void bypass_smmu(void)
 	val = (in_le32(SMMU_NSCR0) | SCR0_CLIENTPD_MASK) & ~(SCR0_USFCFG_MASK);
 	out_le32(SMMU_NSCR0, val);
 }
+
+void setup_smmu_mc_bypass(int icid, int mask)
+{
+	u32 val;
+
+	val = SMMU_SMR_VALID_MASK | (icid << SMMU_SMR_ID_SHIFT) |
+		(mask << SMMU_SMR_MASK_SHIFT);
+	out_le32(SMMU_REG_SMR(0), val);
+	val = SMMU_S2CR_EXIDVALID_VALID_MASK | SMMU_S2CR_TYPE_BYPASS_MASK;
+	out_le32(SMMU_REG_S2CR(0), val);
+}
+
 void fsl_lsch3_early_init_f(void)
 {
 	erratum_rcw_src();
@@ -402,10 +414,18 @@ void fsl_lsch3_early_init_f(void)
 		bypass_smmu();
 #endif
 
-#if defined(CONFIG_ARCH_LS1088A) || defined(CONFIG_ARCH_LS1028A) || \
-	defined(CONFIG_ARCH_LS2080A) || defined(CONFIG_ARCH_LX2160A) || \
-	defined(CONFIG_ARCH_LX2162A)
+#ifdef CONFIG_ARCH_LS1028A
+	set_icids();
+#endif
+
+#if defined(CONFIG_ARCH_LS1088A) || defined(CONFIG_ARCH_LS2080A)
+	set_icids();
+	setup_smmu_mc_bypass(0x300, 0);
+#endif
+
+#if defined(CONFIG_ARCH_LX2160A) || defined(CONFIG_ARCH_LX2162A)
 	set_icids();
+	setup_smmu_mc_bypass(0x4000, 0);
 #endif
 }
 
diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
index ca5e33379ba9..bec5355adaed 100644
--- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
+++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
@@ -190,6 +190,15 @@
 #define SCR0_CLIENTPD_MASK		0x00000001
 #define SCR0_USFCFG_MASK		0x00000400
 
+#define SMMU_REG_SMR(n)			(SMMU_BASE + 0x800 + ((n) << 2))
+#define SMMU_REG_S2CR(n)		(SMMU_BASE + 0xc00 + ((n) << 2))
+#define SMMU_SMR_VALID_MASK		0x80000000
+#define SMMU_SMR_MASK_MASK		0xffff0000
+#define SMMU_SMR_MASK_SHIFT		16
+#define SMMU_SMR_ID_MASK		0x0000ffff
+#define SMMU_SMR_ID_SHIFT		0
+#define SMMU_S2CR_EXIDVALID_VALID_MASK	0x00000400
+#define SMMU_S2CR_TYPE_BYPASS_MASK	0x00010000
 
 /* PCIe */
 #define CFG_SYS_PCIE1_ADDR			(CONFIG_SYS_IMMR + 0x2400000)
-- 
2.17.1

[PATCH 4/5] board: freescale: ls2080a: declare MC reserved regions

[Laurentiu Tudor]

Populate the device tree with the MC reserved memory regions.

Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
---
 board/freescale/ls2080aqds/ls2080aqds.c | 1 +
 board/freescale/ls2080ardb/ls2080ardb.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/board/freescale/ls2080aqds/ls2080aqds.c b/board/freescale/ls2080aqds/ls2080aqds.c
index ba25e9b0b8fc..5c94c83121b5 100644
--- a/board/freescale/ls2080aqds/ls2080aqds.c
+++ b/board/freescale/ls2080aqds/ls2080aqds.c
@@ -325,6 +325,7 @@ int ft_board_setup(void *blob, struct bd_info *bd)
 
 #if defined(CONFIG_FSL_MC_ENET) && !defined(CONFIG_SPL_BUILD)
 	fdt_fixup_board_enet(blob);
+	fdt_reserve_mc_mem(blob, 0x300);
 #endif
 
 	fdt_fixup_icid(blob);
diff --git a/board/freescale/ls2080ardb/ls2080ardb.c b/board/freescale/ls2080ardb/ls2080ardb.c
index 8d340f17a256..5c30de83d841 100644
--- a/board/freescale/ls2080ardb/ls2080ardb.c
+++ b/board/freescale/ls2080ardb/ls2080ardb.c
@@ -522,6 +522,7 @@ int ft_board_setup(void *blob, struct bd_info *bd)
 
 #ifdef CONFIG_FSL_MC_ENET
 	fdt_fixup_board_enet(blob);
+	fdt_reserve_mc_mem(blob, 0x300);
 #endif
 
 	fdt_fixup_icid(blob);
-- 
2.17.1

[PATCH 5/5] board: freescale: ls1088a: declare MC reserved regions

[Laurentiu Tudor]

Populate the device tree with the MC reserved memory regions.

Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
---
 board/freescale/ls1088a/ls1088a.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/board/freescale/ls1088a/ls1088a.c b/board/freescale/ls1088a/ls1088a.c
index 65593f10a3f5..7674e31a268a 100644
--- a/board/freescale/ls1088a/ls1088a.c
+++ b/board/freescale/ls1088a/ls1088a.c
@@ -983,6 +983,7 @@ int ft_board_setup(void *blob, struct bd_info *bd)
 
 #ifdef CONFIG_FSL_MC_ENET
 	fdt_fixup_board_enet(blob);
+	fdt_reserve_mc_mem(blob, 0x300);
 #endif
 
 	fdt_fixup_icid(blob);
-- 
2.17.1

Re: [PATCH 3/5] armv8: fsl-layerscape: create bypass smmu mapping for MC

[Robin Murphy]

On 2023-09-06 17:01, Laurentiu Tudor wrote:
> MC being a plain DMA master as any other device in the SoC and
> being live at OS boot time, as soon as the SMMU is probed it
> will immediately start triggering faults because there is no
> mapping in the SMMU for the MC. Pre-create such a mapping in
> the SMMU, being the OS's responsibility to preserve it.

Does U-Boot enable the SMMU? AFAICS the only thing it knows how to do is 
explicitly turn it *off*, therefore programming other registers appears 
to be a complete waste of time.

All that should matter to the OS, and that it is responsible for 
upholding, is the reserved memory regions from patch #2. For instance, 
if the OS is Linux, literally the first thing arm_smmu_device_reset() 
does is rewrite all the S2CRs and SMRs without so much as looking.

Thanks,
Robin.

> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> ---
>   arch/arm/cpu/armv8/fsl-layerscape/soc.c       | 26 ++++++++++++++++---
>   .../asm/arch-fsl-layerscape/immap_lsch3.h     |  9 +++++++
>   2 files changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> index 3bfdc3f77431..870b99838ab5 100644
> --- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> +++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
> @@ -376,6 +376,18 @@ void bypass_smmu(void)
>   	val = (in_le32(SMMU_NSCR0) | SCR0_CLIENTPD_MASK) & ~(SCR0_USFCFG_MASK);
>   	out_le32(SMMU_NSCR0, val);
>   }
> +
> +void setup_smmu_mc_bypass(int icid, int mask)
> +{
> +	u32 val;
> +
> +	val = SMMU_SMR_VALID_MASK | (icid << SMMU_SMR_ID_SHIFT) |
> +		(mask << SMMU_SMR_MASK_SHIFT);
> +	out_le32(SMMU_REG_SMR(0), val);
> +	val = SMMU_S2CR_EXIDVALID_VALID_MASK | SMMU_S2CR_TYPE_BYPASS_MASK;
> +	out_le32(SMMU_REG_S2CR(0), val);
> +}
> +
>   void fsl_lsch3_early_init_f(void)
>   {
>   	erratum_rcw_src();
> @@ -402,10 +414,18 @@ void fsl_lsch3_early_init_f(void)
>   		bypass_smmu();
>   #endif
>   
> -#if defined(CONFIG_ARCH_LS1088A) || defined(CONFIG_ARCH_LS1028A) || \
> -	defined(CONFIG_ARCH_LS2080A) || defined(CONFIG_ARCH_LX2160A) || \
> -	defined(CONFIG_ARCH_LX2162A)
> +#ifdef CONFIG_ARCH_LS1028A
> +	set_icids();
> +#endif
> +
> +#if defined(CONFIG_ARCH_LS1088A) || defined(CONFIG_ARCH_LS2080A)
> +	set_icids();
> +	setup_smmu_mc_bypass(0x300, 0);
> +#endif
> +
> +#if defined(CONFIG_ARCH_LX2160A) || defined(CONFIG_ARCH_LX2162A)
>   	set_icids();
> +	setup_smmu_mc_bypass(0x4000, 0);
>   #endif
>   }
>   
> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> index ca5e33379ba9..bec5355adaed 100644
> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> @@ -190,6 +190,15 @@
>   #define SCR0_CLIENTPD_MASK		0x00000001
>   #define SCR0_USFCFG_MASK		0x00000400
>   
> +#define SMMU_REG_SMR(n)			(SMMU_BASE + 0x800 + ((n) << 2))
> +#define SMMU_REG_S2CR(n)		(SMMU_BASE + 0xc00 + ((n) << 2))
> +#define SMMU_SMR_VALID_MASK		0x80000000
> +#define SMMU_SMR_MASK_MASK		0xffff0000
> +#define SMMU_SMR_MASK_SHIFT		16
> +#define SMMU_SMR_ID_MASK		0x0000ffff
> +#define SMMU_SMR_ID_SHIFT		0
> +#define SMMU_S2CR_EXIDVALID_VALID_MASK	0x00000400
> +#define SMMU_S2CR_TYPE_BYPASS_MASK	0x00010000
>   
>   /* PCIe */
>   #define CFG_SYS_PCIE1_ADDR			(CONFIG_SYS_IMMR + 0x2400000)

Re: [PATCH 3/5] armv8: fsl-layerscape: create bypass smmu mapping for MC

[Laurentiu Tudor]

On 9/6/2023 8:21 PM, Robin Murphy wrote:
> On 2023-09-06 17:01, Laurentiu Tudor wrote:
>> MC being a plain DMA master as any other device in the SoC and
>> being live at OS boot time, as soon as the SMMU is probed it
>> will immediately start triggering faults because there is no
>> mapping in the SMMU for the MC. Pre-create such a mapping in
>> the SMMU, being the OS's responsibility to preserve it.
> 
> Does U-Boot enable the SMMU? AFAICS the only thing it knows how to do is 
> explicitly turn it *off*, therefore programming other registers appears 
> to be a complete waste of time.

No, it doesn't enable SMMU but it does mark a SMR as valid for MC FW. 
And the ARM SMMU driver subtly preserves it, see [1] (it's late and I 
might be wrong, but I'll double check tomorrow). :-)

> All that should matter to the OS, and that it is responsible for 
> upholding, is the reserved memory regions from patch #2. For instance, 
> if the OS is Linux, literally the first thing arm_smmu_device_reset() 
> does is rewrite all the S2CRs and SMRs without so much as looking.

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/iommu/arm/arm-smmu/arm-smmu.c#n894

---
Best Regards, Laurentiu

> 
>> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
>> ---
>>   arch/arm/cpu/armv8/fsl-layerscape/soc.c       | 26 ++++++++++++++++---
>>   .../asm/arch-fsl-layerscape/immap_lsch3.h     |  9 +++++++
>>   2 files changed, 32 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c 
>> b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
>> index 3bfdc3f77431..870b99838ab5 100644
>> --- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
>> @@ -376,6 +376,18 @@ void bypass_smmu(void)
>>       val = (in_le32(SMMU_NSCR0) | SCR0_CLIENTPD_MASK) & 
>> ~(SCR0_USFCFG_MASK);
>>       out_le32(SMMU_NSCR0, val);
>>   }
>> +
>> +void setup_smmu_mc_bypass(int icid, int mask)
>> +{
>> +    u32 val;
>> +
>> +    val = SMMU_SMR_VALID_MASK | (icid << SMMU_SMR_ID_SHIFT) |
>> +        (mask << SMMU_SMR_MASK_SHIFT);
>> +    out_le32(SMMU_REG_SMR(0), val);
>> +    val = SMMU_S2CR_EXIDVALID_VALID_MASK | SMMU_S2CR_TYPE_BYPASS_MASK;
>> +    out_le32(SMMU_REG_S2CR(0), val);
>> +}
>> +
>>   void fsl_lsch3_early_init_f(void)
>>   {
>>       erratum_rcw_src();
>> @@ -402,10 +414,18 @@ void fsl_lsch3_early_init_f(void)
>>           bypass_smmu();
>>   #endif
>> -#if defined(CONFIG_ARCH_LS1088A) || defined(CONFIG_ARCH_LS1028A) || \
>> -    defined(CONFIG_ARCH_LS2080A) || defined(CONFIG_ARCH_LX2160A) || \
>> -    defined(CONFIG_ARCH_LX2162A)
>> +#ifdef CONFIG_ARCH_LS1028A
>> +    set_icids();
>> +#endif
>> +
>> +#if defined(CONFIG_ARCH_LS1088A) || defined(CONFIG_ARCH_LS2080A)
>> +    set_icids();
>> +    setup_smmu_mc_bypass(0x300, 0);
>> +#endif
>> +
>> +#if defined(CONFIG_ARCH_LX2160A) || defined(CONFIG_ARCH_LX2162A)
>>       set_icids();
>> +    setup_smmu_mc_bypass(0x4000, 0);
>>   #endif
>>   }
>> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h 
>> b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>> index ca5e33379ba9..bec5355adaed 100644
>> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>> @@ -190,6 +190,15 @@
>>   #define SCR0_CLIENTPD_MASK        0x00000001
>>   #define SCR0_USFCFG_MASK        0x00000400
>> +#define SMMU_REG_SMR(n)            (SMMU_BASE + 0x800 + ((n) << 2))
>> +#define SMMU_REG_S2CR(n)        (SMMU_BASE + 0xc00 + ((n) << 2))
>> +#define SMMU_SMR_VALID_MASK        0x80000000
>> +#define SMMU_SMR_MASK_MASK        0xffff0000
>> +#define SMMU_SMR_MASK_SHIFT        16
>> +#define SMMU_SMR_ID_MASK        0x0000ffff
>> +#define SMMU_SMR_ID_SHIFT        0
>> +#define SMMU_S2CR_EXIDVALID_VALID_MASK    0x00000400
>> +#define SMMU_S2CR_TYPE_BYPASS_MASK    0x00010000
>>   /* PCIe */
>>   #define CFG_SYS_PCIE1_ADDR            (CONFIG_SYS_IMMR + 0x2400000)

Re: [PATCH 3/5] armv8: fsl-layerscape: create bypass smmu mapping for MC

[Robin Murphy]

On 2023-09-06 19:10, Laurentiu Tudor wrote:
> 
> 
> On 9/6/2023 8:21 PM, Robin Murphy wrote:
>> On 2023-09-06 17:01, Laurentiu Tudor wrote:
>>> MC being a plain DMA master as any other device in the SoC and
>>> being live at OS boot time, as soon as the SMMU is probed it
>>> will immediately start triggering faults because there is no
>>> mapping in the SMMU for the MC. Pre-create such a mapping in
>>> the SMMU, being the OS's responsibility to preserve it.
>>
>> Does U-Boot enable the SMMU? AFAICS the only thing it knows how to do 
>> is explicitly turn it *off*, therefore programming other registers 
>> appears to be a complete waste of time.
> 
> No, it doesn't enable SMMU but it does mark a SMR as valid for MC FW. 
> And the ARM SMMU driver subtly preserves it, see [1] (it's late and I 
> might be wrong, but I'll double check tomorrow). :-)

No, that sets the SMR valid bit *if* the corresponding entry is 
allocated and marked as valid in the software state in smmu->smrs, which 
at probe time it isn't, because that's only just been allocated and is 
still zero-initialised. Unless, that is, 
arm_smmu_rmr_install_bypass_smr() found a reserved region and 
preallocated an entry to honour it. But even those entries are still 
constructed from scratch; we can't do anything with the existing 
SMR/S2CR register contents in general since they may be uninitialised 
random reset values, so we don't even look.

Pay no attention to the qcom_smmu_cfg_probe() hack either - that only 
exists on the promise that the relevant platforms couldn't have their 
firmware updated to use proper RMRs.

You're already doing the right thing in patch #2, so there's no need to 
waste code on doing a pointless wrong thing as well.

Thanks,
Robin.

>> All that should matter to the OS, and that it is responsible for 
>> upholding, is the reserved memory regions from patch #2. For instance, 
>> if the OS is Linux, literally the first thing arm_smmu_device_reset() 
>> does is rewrite all the S2CRs and SMRs without so much as looking.
> 
> [1] 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/iommu/arm/arm-smmu/arm-smmu.c#n894
> 
> ---
> Best Regards, Laurentiu
> 
>>
>>> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
>>> ---
>>>   arch/arm/cpu/armv8/fsl-layerscape/soc.c       | 26 ++++++++++++++++---
>>>   .../asm/arch-fsl-layerscape/immap_lsch3.h     |  9 +++++++
>>>   2 files changed, 32 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c 
>>> b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
>>> index 3bfdc3f77431..870b99838ab5 100644
>>> --- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
>>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
>>> @@ -376,6 +376,18 @@ void bypass_smmu(void)
>>>       val = (in_le32(SMMU_NSCR0) | SCR0_CLIENTPD_MASK) & 
>>> ~(SCR0_USFCFG_MASK);
>>>       out_le32(SMMU_NSCR0, val);
>>>   }
>>> +
>>> +void setup_smmu_mc_bypass(int icid, int mask)
>>> +{
>>> +    u32 val;
>>> +
>>> +    val = SMMU_SMR_VALID_MASK | (icid << SMMU_SMR_ID_SHIFT) |
>>> +        (mask << SMMU_SMR_MASK_SHIFT);
>>> +    out_le32(SMMU_REG_SMR(0), val);
>>> +    val = SMMU_S2CR_EXIDVALID_VALID_MASK | SMMU_S2CR_TYPE_BYPASS_MASK;
>>> +    out_le32(SMMU_REG_S2CR(0), val);
>>> +}
>>> +
>>>   void fsl_lsch3_early_init_f(void)
>>>   {
>>>       erratum_rcw_src();
>>> @@ -402,10 +414,18 @@ void fsl_lsch3_early_init_f(void)
>>>           bypass_smmu();
>>>   #endif
>>> -#if defined(CONFIG_ARCH_LS1088A) || defined(CONFIG_ARCH_LS1028A) || \
>>> -    defined(CONFIG_ARCH_LS2080A) || defined(CONFIG_ARCH_LX2160A) || \
>>> -    defined(CONFIG_ARCH_LX2162A)
>>> +#ifdef CONFIG_ARCH_LS1028A
>>> +    set_icids();
>>> +#endif
>>> +
>>> +#if defined(CONFIG_ARCH_LS1088A) || defined(CONFIG_ARCH_LS2080A)
>>> +    set_icids();
>>> +    setup_smmu_mc_bypass(0x300, 0);
>>> +#endif
>>> +
>>> +#if defined(CONFIG_ARCH_LX2160A) || defined(CONFIG_ARCH_LX2162A)
>>>       set_icids();
>>> +    setup_smmu_mc_bypass(0x4000, 0);
>>>   #endif
>>>   }
>>> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h 
>>> b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>>> index ca5e33379ba9..bec5355adaed 100644
>>> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>>> @@ -190,6 +190,15 @@
>>>   #define SCR0_CLIENTPD_MASK        0x00000001
>>>   #define SCR0_USFCFG_MASK        0x00000400
>>> +#define SMMU_REG_SMR(n)            (SMMU_BASE + 0x800 + ((n) << 2))
>>> +#define SMMU_REG_S2CR(n)        (SMMU_BASE + 0xc00 + ((n) << 2))
>>> +#define SMMU_SMR_VALID_MASK        0x80000000
>>> +#define SMMU_SMR_MASK_MASK        0xffff0000
>>> +#define SMMU_SMR_MASK_SHIFT        16
>>> +#define SMMU_SMR_ID_MASK        0x0000ffff
>>> +#define SMMU_SMR_ID_SHIFT        0
>>> +#define SMMU_S2CR_EXIDVALID_VALID_MASK    0x00000400
>>> +#define SMMU_S2CR_TYPE_BYPASS_MASK    0x00010000
>>>   /* PCIe */
>>>   #define CFG_SYS_PCIE1_ADDR            (CONFIG_SYS_IMMR + 0x2400000)

Re: [PATCH 3/5] armv8: fsl-layerscape: create bypass smmu mapping for MC

[Laurentiu Tudor]

On 9/6/2023 11:09 PM, Robin Murphy wrote:
> On 2023-09-06 19:10, Laurentiu Tudor wrote:
>>
>>
>> On 9/6/2023 8:21 PM, Robin Murphy wrote:
>>> On 2023-09-06 17:01, Laurentiu Tudor wrote:
>>>> MC being a plain DMA master as any other device in the SoC and
>>>> being live at OS boot time, as soon as the SMMU is probed it
>>>> will immediately start triggering faults because there is no
>>>> mapping in the SMMU for the MC. Pre-create such a mapping in
>>>> the SMMU, being the OS's responsibility to preserve it.
>>>
>>> Does U-Boot enable the SMMU? AFAICS the only thing it knows how to do
>>> is explicitly turn it *off*, therefore programming other registers
>>> appears to be a complete waste of time.
>>
>> No, it doesn't enable SMMU but it does mark a SMR as valid for MC FW.
>> And the ARM SMMU driver subtly preserves it, see [1] (it's late and I
>> might be wrong, but I'll double check tomorrow). :-)
>
> No, that sets the SMR valid bit *if* the corresponding entry is
> allocated and marked as valid in the software state in smmu->smrs, which
> at probe time it isn't, because that's only just been allocated and is
> still zero-initialised. Unless, that is,
> arm_smmu_rmr_install_bypass_smr() found a reserved region and
> preallocated an entry to honour it. But even those entries are still
> constructed from scratch; we can't do anything with the existing
> SMR/S2CR register contents in general since they may be uninitialised
> random reset values, so we don't even look.
>
> Pay no attention to the qcom_smmu_cfg_probe() hack either - that only
> exists on the promise that the relevant platforms couldn't have their
> firmware updated to use proper RMRs.
>
> You're already doing the right thing in patch #2, so there's no need to
> waste code on doing a pointless wrong thing as well.
>

Spent quite a while (sorry!) trying to remember why this was needed but
don't have a clear answer. I suspect that the hack was used with early
iort rmr implementations.
Anyway, I retested without it both DT and ACPI and looks good. Will drop
the patch and resubmit the series.

---
Thanks & Best Regards, Laurentiu

>
>>> All that should matter to the OS, and that it is responsible for
>>> upholding, is the reserved memory regions from patch #2. For
>>> instance, if the OS is Linux, literally the first thing
>>> arm_smmu_device_reset() does is rewrite all the S2CRs and SMRs
>>> without so much as looking.
>>
>> [1]
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/iommu/arm/arm-smmu/arm-smmu.c#n894
>>
>>
>> ---
>> Best Regards, Laurentiu
>>
>>>
>>>> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
>>>> ---
>>>>   arch/arm/cpu/armv8/fsl-layerscape/soc.c       | 26
>>>> ++++++++++++++++---
>>>>   .../asm/arch-fsl-layerscape/immap_lsch3.h     |  9 +++++++
>>>>   2 files changed, 32 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
>>>> b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
>>>> index 3bfdc3f77431..870b99838ab5 100644
>>>> --- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
>>>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
>>>> @@ -376,6 +376,18 @@ void bypass_smmu(void)
>>>>       val = (in_le32(SMMU_NSCR0) | SCR0_CLIENTPD_MASK) &
>>>> ~(SCR0_USFCFG_MASK);
>>>>       out_le32(SMMU_NSCR0, val);
>>>>   }
>>>> +
>>>> +void setup_smmu_mc_bypass(int icid, int mask)
>>>> +{
>>>> +    u32 val;
>>>> +
>>>> +    val = SMMU_SMR_VALID_MASK | (icid << SMMU_SMR_ID_SHIFT) |
>>>> +        (mask << SMMU_SMR_MASK_SHIFT);
>>>> +    out_le32(SMMU_REG_SMR(0), val);
>>>> +    val = SMMU_S2CR_EXIDVALID_VALID_MASK | SMMU_S2CR_TYPE_BYPASS_MASK;
>>>> +    out_le32(SMMU_REG_S2CR(0), val);
>>>> +}
>>>> +
>>>>   void fsl_lsch3_early_init_f(void)
>>>>   {
>>>>       erratum_rcw_src();
>>>> @@ -402,10 +414,18 @@ void fsl_lsch3_early_init_f(void)
>>>>           bypass_smmu();
>>>>   #endif
>>>> -#if defined(CONFIG_ARCH_LS1088A) || defined(CONFIG_ARCH_LS1028A) || \
>>>> -    defined(CONFIG_ARCH_LS2080A) || defined(CONFIG_ARCH_LX2160A) || \
>>>> -    defined(CONFIG_ARCH_LX2162A)
>>>> +#ifdef CONFIG_ARCH_LS1028A
>>>> +    set_icids();
>>>> +#endif
>>>> +
>>>> +#if defined(CONFIG_ARCH_LS1088A) || defined(CONFIG_ARCH_LS2080A)
>>>> +    set_icids();
>>>> +    setup_smmu_mc_bypass(0x300, 0);
>>>> +#endif
>>>> +
>>>> +#if defined(CONFIG_ARCH_LX2160A) || defined(CONFIG_ARCH_LX2162A)
>>>>       set_icids();
>>>> +    setup_smmu_mc_bypass(0x4000, 0);
>>>>   #endif
>>>>   }
>>>> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>>>> b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>>>> index ca5e33379ba9..bec5355adaed 100644
>>>> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>>>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>>>> @@ -190,6 +190,15 @@
>>>>   #define SCR0_CLIENTPD_MASK        0x00000001
>>>>   #define SCR0_USFCFG_MASK        0x00000400
>>>> +#define SMMU_REG_SMR(n)            (SMMU_BASE + 0x800 + ((n) << 2))
>>>> +#define SMMU_REG_S2CR(n)        (SMMU_BASE + 0xc00 + ((n) << 2))
>>>> +#define SMMU_SMR_VALID_MASK        0x80000000
>>>> +#define SMMU_SMR_MASK_MASK        0xffff0000
>>>> +#define SMMU_SMR_MASK_SHIFT        16
>>>> +#define SMMU_SMR_ID_MASK        0x0000ffff
>>>> +#define SMMU_SMR_ID_SHIFT        0
>>>> +#define SMMU_S2CR_EXIDVALID_VALID_MASK    0x00000400
>>>> +#define SMMU_S2CR_TYPE_BYPASS_MASK    0x00010000
>>>>   /* PCIe */
>>>>   #define CFG_SYS_PCIE1_ADDR            (CONFIG_SYS_IMMR + 0x2400000)