[PATCH 0/4] of: request reserved memory regions so other code can't

[PATCH 0/4] of: request reserved memory regions so other code can't

[Ahmad Fatoum]

This series pulls out some parts from Rouven's eXecute Never series[1]
to improve reserved memory support.

Changes are described beneath each patch. This series was prompted by
Raspberry Pi 64-bit rework: barebox placed the kernel at address 0,
which is reserved for the spin table thereby breaking multicore boot.

Now that reserved memory regions are requested, they are skipped over
when determining a load address.

This may induce breakage if reserved memory regions are being used
by existing code that expects exclusive access. nvmem-rmem is one
such user that's fixed here. If you know of more, please tell.

[1]: https://lore.barebox.org/barebox/20210803094418.475609-1-r.czerwinski@pengutronix.de/

Ahmad Fatoum (1):
  nvmem: rmem: get, don't request, memory region

Rouven Czerwinski (3):
  of: reserve: mark runtime firmware code regions specially
  of: add of_get_reserve_map stub for !CONFIG_OFTREE
  of: request reserved memory regions so other code can't

 arch/arm/cpu/sm.c              |  3 ++-
 arch/arm/cpu/start.c           |  3 ++-
 arch/arm/mach-layerscape/ppa.c |  2 +-
 common/bootm.c                 |  3 ++-
 common/memory.c                | 21 +++++++++++++++++++--
 drivers/nvmem/rmem.c           |  2 +-
 drivers/of/Makefile            |  1 +
 drivers/of/fdt.c               | 18 +++++++++++++-----
 drivers/video/fb.c             |  3 ++-
 drivers/video/simplefb-fixup.c |  2 +-
 fs/pstore/ram.c                |  3 ++-
 include/of.h                   | 15 +++++++++++++--
 12 files changed, 59 insertions(+), 17 deletions(-)

-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/4] nvmem: rmem: get, don't request, memory region

[Ahmad Fatoum]

nvmem-rmem is a compatible for reserved memory entries, so the driver
can't expect exclusive access to this region.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/nvmem/rmem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvmem/rmem.c b/drivers/nvmem/rmem.c
index cd735e5ef39d..b9331f48ffac 100644
--- a/drivers/nvmem/rmem.c
+++ b/drivers/nvmem/rmem.c
@@ -31,7 +31,7 @@ static int rmem_probe(struct device_d *dev)
 	struct resource *mem;
 	struct rmem *priv;
 
-	mem = dev_request_mem_resource(dev, 0);
+	mem = dev_get_resource(dev, IORESOURCE_MEM, 0);
 	if (IS_ERR(mem))
 		return PTR_ERR(mem);
 
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/4] of: reserve: mark runtime firmware code regions specially

[Ahmad Fatoum]

From: Rouven Czerwinski <r.czerwinski@pengutronix.de>

We already map memory regions outside the memory banks with eXecute
Never to avoid speculative execution, but fail to do so currently for
firmware running from within a memory bank, but at a higher privilege
level.

In preparation for changing that, mark memory used by such elevated
firmware by a newly introduced OF_RESERVE_ENTRY_FLAG_RUNTIME_FW flag.

This introduces no functional change, but suitable future action could
be mapping eXecute Never or not mapping at all.

Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
Compared with Rouven's v2, rename FLAG_XN to FLASG_RUNTIME_FW and
apply it where applicable. Also change runtime_fw (previously xn)
member to be a bitfield to get an error should number of reserved
regions be changed inadequately in future.
---
 arch/arm/cpu/sm.c              | 3 ++-
 arch/arm/cpu/start.c           | 3 ++-
 arch/arm/mach-layerscape/ppa.c | 2 +-
 common/bootm.c                 | 3 ++-
 drivers/of/fdt.c               | 6 +++++-
 drivers/video/fb.c             | 3 ++-
 drivers/video/simplefb-fixup.c | 2 +-
 fs/pstore/ram.c                | 3 ++-
 include/of.h                   | 6 +++++-
 9 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/arch/arm/cpu/sm.c b/arch/arm/cpu/sm.c
index f5a1edbd4fe9..a404f843ecb2 100644
--- a/arch/arm/cpu/sm.c
+++ b/arch/arm/cpu/sm.c
@@ -203,7 +203,8 @@ static int of_secure_monitor_fixup(struct device_node *root, void *unused)
 	res_start = (unsigned long)_stext;
 	res_end = (unsigned long)__bss_stop;
 
-	of_add_reserve_entry(res_start, res_end);
+	/* MMU is already set up by now, so this only affects kernel DT */
+	of_add_reserve_entry(res_start, res_end, OF_RESERVE_ENTRY_FLAG_RUNTIME_FW);
 
 	pr_debug("Reserved memory range from 0x%08lx to 0x%08lx\n", res_start, res_end);
 
diff --git a/arch/arm/cpu/start.c b/arch/arm/cpu/start.c
index c61db668658b..9935bc8d53e3 100644
--- a/arch/arm/cpu/start.c
+++ b/arch/arm/cpu/start.c
@@ -227,7 +227,8 @@ __noreturn __no_sanitize_address void barebox_non_pbl_start(unsigned long membas
 	mem_malloc_init((void *)malloc_start, (void *)malloc_end - 1);
 
 	if (IS_ENABLED(CONFIG_BOOTM_OPTEE))
-		of_add_reserve_entry(endmem - OPTEE_SIZE, endmem - 1);
+		of_add_reserve_entry(endmem - OPTEE_SIZE, endmem - 1,
+				     OF_RESERVE_ENTRY_FLAG_RUNTIME_FW);
 
 	pr_debug("starting barebox...\n");
 
diff --git a/arch/arm/mach-layerscape/ppa.c b/arch/arm/mach-layerscape/ppa.c
index d962fba75105..a862ece69c6d 100644
--- a/arch/arm/mach-layerscape/ppa.c
+++ b/arch/arm/mach-layerscape/ppa.c
@@ -130,7 +130,7 @@ int ls1046a_ppa_init(resource_size_t ppa_start, resource_size_t ppa_size)
 	if (ret)
 		return ret;
 
-	of_add_reserve_entry(ppa_start, ppa_end);
+	of_add_reserve_entry(ppa_start, ppa_end, OF_RESERVE_ENTRY_FLAG_RUNTIME_FW);
 
 	return 0;
 }
diff --git a/common/bootm.c b/common/bootm.c
index 3c80e8bf949b..463fd3721c81 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -411,7 +411,8 @@ void *bootm_get_devicetree(struct image_data *data)
 	if (data->initrd_res) {
 		of_add_initrd(data->of_root_node, data->initrd_res->start,
 				data->initrd_res->end);
-		of_add_reserve_entry(data->initrd_res->start, data->initrd_res->end);
+		of_add_reserve_entry(data->initrd_res->start,
+				     data->initrd_res->end, 0);
 	}
 
 	oftree = of_get_fixed_tree(data->of_root_node);
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 5ccbd1bb69f8..4a4eeba24bc3 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -536,7 +536,8 @@ out_free:
 
 static struct of_reserve_map of_reserve_map;
 
-int of_add_reserve_entry(resource_size_t start, resource_size_t end)
+int of_add_reserve_entry(resource_size_t start, resource_size_t end,
+			 int flags)
 {
 	int e = of_reserve_map.num_entries;
 
@@ -547,6 +548,9 @@ int of_add_reserve_entry(resource_size_t start, resource_size_t end)
 	of_reserve_map.end[e] = end;
 	of_reserve_map.num_entries++;
 
+	if (flags & OF_RESERVE_ENTRY_FLAG_RUNTIME_FW)
+		of_reserve_map.runtime_fw |= BIT(e);
+
 	return 0;
 }
 
diff --git a/drivers/video/fb.c b/drivers/video/fb.c
index 1c93dafbc989..e0a73dd415a4 100644
--- a/drivers/video/fb.c
+++ b/drivers/video/fb.c
@@ -203,7 +203,8 @@ static int fb_of_reserve_fixup(struct device_node *root, void *context)
 		return 0;
 
 	of_add_reserve_entry((unsigned long)info->screen_base,
-			(unsigned long)info->screen_base + info->screen_size);
+			(unsigned long)info->screen_base + info->screen_size,
+			0);
 
 	return 0;
 }
diff --git a/drivers/video/simplefb-fixup.c b/drivers/video/simplefb-fixup.c
index a2c59de364a5..af7554d10ffd 100644
--- a/drivers/video/simplefb-fixup.c
+++ b/drivers/video/simplefb-fixup.c
@@ -131,7 +131,7 @@ static int simplefb_create_node(struct device_node *root,
 		return ret;
 
 	of_add_reserve_entry((u32)fbi->screen_base,
-			(u32)fbi->screen_base + fbi->screen_size);
+			(u32)fbi->screen_base + fbi->screen_size, 0);
 
 	return of_property_write_string(node, "status", "okay");
 }
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 0d8bb8f418f4..d0ebc7a37901 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -639,7 +639,8 @@ static int ramoops_probe(struct device_d *dev)
 			  ramoops_ecc);
 		globalvar_add_simple("linux.bootargs.ramoops", kernelargs);
 	} else {
-		of_add_reserve_entry(cxt->phys_addr, cxt->phys_addr + mem_size);
+		of_add_reserve_entry(cxt->phys_addr, cxt->phys_addr + mem_size,
+				0);
 		of_register_fixup(ramoops_of_fixup, pdata);
 	}
 
diff --git a/include/of.h b/include/of.h
index 46b96277d581..0d125d8256d6 100644
--- a/include/of.h
+++ b/include/of.h
@@ -55,9 +55,13 @@ struct of_reserve_map {
 	uint64_t start[OF_MAX_RESERVE_MAP];
 	uint64_t end[OF_MAX_RESERVE_MAP];
 	int num_entries;
+	u16 runtime_fw : OF_MAX_RESERVE_MAP;
 };
 
-int of_add_reserve_entry(resource_size_t start, resource_size_t end);
+#define OF_RESERVE_ENTRY_FLAG_RUNTIME_FW	BIT(0)
+
+int of_add_reserve_entry(resource_size_t start, resource_size_t end,
+		 int flags);
 struct of_reserve_map *of_get_reserve_map(void);
 void of_clean_reserve_map(void);
 void fdt_add_reserve_map(void *fdt);
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/4] of: reserve: mark runtime firmware code regions specially

[Sascha Hauer]

On Thu, Jun 09, 2022 at 07:43:40AM +0200, Ahmad Fatoum wrote:
> From: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> 
> We already map memory regions outside the memory banks with eXecute
> Never to avoid speculative execution, but fail to do so currently for
> firmware running from within a memory bank, but at a higher privilege
> level.
> 
> In preparation for changing that, mark memory used by such elevated
> firmware by a newly introduced OF_RESERVE_ENTRY_FLAG_RUNTIME_FW flag.
> 
> This introduces no functional change, but suitable future action could
> be mapping eXecute Never or not mapping at all.
> 
> Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> Compared with Rouven's v2, rename FLAG_XN to FLASG_RUNTIME_FW and
> apply it where applicable. Also change runtime_fw (previously xn)
> member to be a bitfield to get an error should number of reserved
> regions be changed inadequately in future.
> ---
>  arch/arm/cpu/sm.c              | 3 ++-
>  arch/arm/cpu/start.c           | 3 ++-
>  arch/arm/mach-layerscape/ppa.c | 2 +-
>  common/bootm.c                 | 3 ++-
>  drivers/of/fdt.c               | 6 +++++-
>  drivers/video/fb.c             | 3 ++-
>  drivers/video/simplefb-fixup.c | 2 +-
>  fs/pstore/ram.c                | 3 ++-
>  include/of.h                   | 6 +++++-
>  9 files changed, 22 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/cpu/sm.c b/arch/arm/cpu/sm.c
> index f5a1edbd4fe9..a404f843ecb2 100644
> --- a/arch/arm/cpu/sm.c
> +++ b/arch/arm/cpu/sm.c
> @@ -203,7 +203,8 @@ static int of_secure_monitor_fixup(struct device_node *root, void *unused)
>  	res_start = (unsigned long)_stext;
>  	res_end = (unsigned long)__bss_stop;
>  
> -	of_add_reserve_entry(res_start, res_end);
> +	/* MMU is already set up by now, so this only affects kernel DT */
> +	of_add_reserve_entry(res_start, res_end, OF_RESERVE_ENTRY_FLAG_RUNTIME_FW);
>  
>  	pr_debug("Reserved memory range from 0x%08lx to 0x%08lx\n", res_start, res_end);
>  
> diff --git a/arch/arm/cpu/start.c b/arch/arm/cpu/start.c
> index c61db668658b..9935bc8d53e3 100644
> --- a/arch/arm/cpu/start.c
> +++ b/arch/arm/cpu/start.c
> @@ -227,7 +227,8 @@ __noreturn __no_sanitize_address void barebox_non_pbl_start(unsigned long membas
>  	mem_malloc_init((void *)malloc_start, (void *)malloc_end - 1);
>  
>  	if (IS_ENABLED(CONFIG_BOOTM_OPTEE))
> -		of_add_reserve_entry(endmem - OPTEE_SIZE, endmem - 1);
> +		of_add_reserve_entry(endmem - OPTEE_SIZE, endmem - 1,
> +				     OF_RESERVE_ENTRY_FLAG_RUNTIME_FW);
>  
>  	pr_debug("starting barebox...\n");
>  
> diff --git a/arch/arm/mach-layerscape/ppa.c b/arch/arm/mach-layerscape/ppa.c
> index d962fba75105..a862ece69c6d 100644
> --- a/arch/arm/mach-layerscape/ppa.c
> +++ b/arch/arm/mach-layerscape/ppa.c
> @@ -130,7 +130,7 @@ int ls1046a_ppa_init(resource_size_t ppa_start, resource_size_t ppa_size)
>  	if (ret)
>  		return ret;
>  
> -	of_add_reserve_entry(ppa_start, ppa_end);
> +	of_add_reserve_entry(ppa_start, ppa_end, OF_RESERVE_ENTRY_FLAG_RUNTIME_FW);
>  
>  	return 0;
>  }
> diff --git a/common/bootm.c b/common/bootm.c
> index 3c80e8bf949b..463fd3721c81 100644
> --- a/common/bootm.c
> +++ b/common/bootm.c
> @@ -411,7 +411,8 @@ void *bootm_get_devicetree(struct image_data *data)
>  	if (data->initrd_res) {
>  		of_add_initrd(data->of_root_node, data->initrd_res->start,
>  				data->initrd_res->end);
> -		of_add_reserve_entry(data->initrd_res->start, data->initrd_res->end);
> +		of_add_reserve_entry(data->initrd_res->start,
> +				     data->initrd_res->end, 0);
>  	}
>  
>  	oftree = of_get_fixed_tree(data->of_root_node);
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 5ccbd1bb69f8..4a4eeba24bc3 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -536,7 +536,8 @@ out_free:
>  
>  static struct of_reserve_map of_reserve_map;
>  
> -int of_add_reserve_entry(resource_size_t start, resource_size_t end)
> +int of_add_reserve_entry(resource_size_t start, resource_size_t end,
> +			 int flags)
>  {
>  	int e = of_reserve_map.num_entries;
>  
> @@ -547,6 +548,9 @@ int of_add_reserve_entry(resource_size_t start, resource_size_t end)
>  	of_reserve_map.end[e] = end;
>  	of_reserve_map.num_entries++;
>  
> +	if (flags & OF_RESERVE_ENTRY_FLAG_RUNTIME_FW)
> +		of_reserve_map.runtime_fw |= BIT(e);
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/video/fb.c b/drivers/video/fb.c
> index 1c93dafbc989..e0a73dd415a4 100644
> --- a/drivers/video/fb.c
> +++ b/drivers/video/fb.c
> @@ -203,7 +203,8 @@ static int fb_of_reserve_fixup(struct device_node *root, void *context)
>  		return 0;
>  
>  	of_add_reserve_entry((unsigned long)info->screen_base,
> -			(unsigned long)info->screen_base + info->screen_size);
> +			(unsigned long)info->screen_base + info->screen_size,
> +			0);
>  
>  	return 0;
>  }
> diff --git a/drivers/video/simplefb-fixup.c b/drivers/video/simplefb-fixup.c
> index a2c59de364a5..af7554d10ffd 100644
> --- a/drivers/video/simplefb-fixup.c
> +++ b/drivers/video/simplefb-fixup.c
> @@ -131,7 +131,7 @@ static int simplefb_create_node(struct device_node *root,
>  		return ret;
>  
>  	of_add_reserve_entry((u32)fbi->screen_base,
> -			(u32)fbi->screen_base + fbi->screen_size);
> +			(u32)fbi->screen_base + fbi->screen_size, 0);
>  
>  	return of_property_write_string(node, "status", "okay");
>  }
> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> index 0d8bb8f418f4..d0ebc7a37901 100644
> --- a/fs/pstore/ram.c
> +++ b/fs/pstore/ram.c
> @@ -639,7 +639,8 @@ static int ramoops_probe(struct device_d *dev)
>  			  ramoops_ecc);
>  		globalvar_add_simple("linux.bootargs.ramoops", kernelargs);
>  	} else {
> -		of_add_reserve_entry(cxt->phys_addr, cxt->phys_addr + mem_size);
> +		of_add_reserve_entry(cxt->phys_addr, cxt->phys_addr + mem_size,
> +				0);
>  		of_register_fixup(ramoops_of_fixup, pdata);
>  	}
>  
> diff --git a/include/of.h b/include/of.h
> index 46b96277d581..0d125d8256d6 100644
> --- a/include/of.h
> +++ b/include/of.h
> @@ -55,9 +55,13 @@ struct of_reserve_map {
>  	uint64_t start[OF_MAX_RESERVE_MAP];
>  	uint64_t end[OF_MAX_RESERVE_MAP];
>  	int num_entries;
> +	u16 runtime_fw : OF_MAX_RESERVE_MAP;

Please change to:

struct of_reserve_map_entry {
	uint64_t start;
	uint64_t end;
	u32 flags;
};

struct of_reserve_map {
	struct of_reserve_map_entry[OF_MAX_RESERVE_MAP];
	int num_entries;
};

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/4] of: reserve: mark runtime firmware code regions specially

[Ahmad Fatoum]

Hello Sascha,

On 09.06.22 10:05, Sascha Hauer wrote:
>> diff --git a/include/of.h b/include/of.h
>> index 46b96277d581..0d125d8256d6 100644
>> --- a/include/of.h
>> +++ b/include/of.h
>> @@ -55,9 +55,13 @@ struct of_reserve_map {
>>  	uint64_t start[OF_MAX_RESERVE_MAP];
>>  	uint64_t end[OF_MAX_RESERVE_MAP];
>>  	int num_entries;
>> +	u16 runtime_fw : OF_MAX_RESERVE_MAP;
> 
> Please change to:
> 
> struct of_reserve_map_entry {
> 	uint64_t start;
> 	uint64_t end;
> 	u32 flags;
> };
> 
> struct of_reserve_map {
> 	struct of_reserve_map_entry[OF_MAX_RESERVE_MAP];
> 	int num_entries;
> };

Rouven will reuse/replicate this for PBL, so I'd prefer
to go the size conscious route for now.

Cheers,
Ahmad

> 
> Sascha
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/4] of: add of_get_reserve_map stub for !CONFIG_OFTREE

[Ahmad Fatoum]

From: Rouven Czerwinski <r.czerwinski@pengutronix.de>

This allows us to unconditionally include of.h in files which did not
require CONFIG_OFTREE, required for the MMU code in later patches.

Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
Compared with Rouven's v2, give extern declaration an extern
in front like functions around it. Move static inline stub
to top of #else for symmetry.
---
 include/of.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/of.h b/include/of.h
index 0d125d8256d6..d55016f1e372 100644
--- a/include/of.h
+++ b/include/of.h
@@ -62,7 +62,6 @@ struct of_reserve_map {
 
 int of_add_reserve_entry(resource_size_t start, resource_size_t end,
 		 int flags);
-struct of_reserve_map *of_get_reserve_map(void);
 void of_clean_reserve_map(void);
 void fdt_add_reserve_map(void *fdt);
 
@@ -120,6 +119,7 @@ struct device_node *of_unflatten_dtb_const(const void *infdt, int size);
 struct cdev;
 
 #ifdef CONFIG_OFTREE
+extern struct of_reserve_map *of_get_reserve_map(void);
 extern int of_bus_n_addr_cells(struct device_node *np);
 extern int of_n_addr_cells(struct device_node *np);
 extern int of_bus_n_size_cells(struct device_node *np);
@@ -322,6 +322,11 @@ int of_autoenable_device_by_path(char *path);
 int of_autoenable_i2c_by_component(char *path);
 int of_prepend_machine_compatible(struct device_node *root, const char *compat);
 #else
+static inline struct of_reserve_map *of_get_reserve_map(void)
+{
+	return NULL;
+}
+
 static inline bool of_node_name_eq(const struct device_node *np, const char *name)
 {
 	return false;
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 3/4] of: add of_get_reserve_map stub for !CONFIG_OFTREE

[Sascha Hauer]

On Thu, Jun 09, 2022 at 07:43:41AM +0200, Ahmad Fatoum wrote:
> From: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> 
> This allows us to unconditionally include of.h in files which did not
> require CONFIG_OFTREE, required for the MMU code in later patches.
> 
> Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> Compared with Rouven's v2, give extern declaration an extern
> in front like functions around it. Move static inline stub
> to top of #else for symmetry.
> ---
>  include/of.h | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

Applied this patch only.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 4/4] of: request reserved memory regions so other code can't

[Ahmad Fatoum]

From: Rouven Czerwinski <r.czerwinski@pengutronix.de>

Add a reserved_mem_read initcall which parses the reserved-memory
entries and adds barebox of reserve entries. This avoids e.g. bootm
trying to place the kernel into a reserved region.

Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
Compared with Rouven's v2, rename OF_RESERVE_ENTRY_FLAG_NO_RESERVE
to NO_FIXUP and read both /reserved-memory and /memreserve
to request memory regions.
---
 common/memory.c     | 21 +++++++++++++++++++--
 drivers/of/Makefile |  1 +
 drivers/of/fdt.c    | 12 ++++++++----
 include/of.h        |  2 ++
 4 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/common/memory.c b/common/memory.c
index 95995bb6e310..b40c74bfe97f 100644
--- a/common/memory.c
+++ b/common/memory.c
@@ -12,6 +12,7 @@
 #include <asm-generic/memory_layout.h>
 #include <asm/sections.h>
 #include <malloc.h>
+#include <of.h>
 
 /*
  * Begin and End of memory area for malloc(), and current "brk"
@@ -53,9 +54,12 @@ void mem_malloc_init(void *start, void *end)
 	mem_malloc_initialized = 1;
 }
 
-#if !defined __SANDBOX__
 static int mem_malloc_resource(void)
 {
+	struct of_reserve_map *map;
+	int i;
+
+#if !defined __SANDBOX__
 	/*
 	 * Normally it's a bug when one of these fails,
 	 * but we have some setups where some of these
@@ -80,10 +84,23 @@ static int mem_malloc_resource(void)
 #ifdef STACK_BASE
 	request_sdram_region("stack", STACK_BASE, STACK_SIZE);
 #endif
+#endif
+
+	map = of_get_reserve_map();
+	if (!map)
+		return 0;
+
+	for (i = 0; i < map->num_entries; i++) {
+		const char *name;
+
+		name = map->runtime_fw & BIT(i) ? "protected code" : "protected data";
+		request_sdram_region(name, map->start[i],
+				     map->end[i] - map->start[i] + 1);
+	}
+
 	return 0;
 }
 coredevice_initcall(mem_malloc_resource);
-#endif
 
 static void *sbrk_no_zero(ptrdiff_t increment)
 {
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index ca8da71cb4f0..99b610cba85e 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_OF_GPIO) += of_gpio.o
 obj-$(CONFIG_OF_PCI) += of_pci.o
 obj-y += partition.o
 obj-y += of_net.o
+obj-$(CONFIG_OFDEVICE) += reserved-mem.o
 obj-$(CONFIG_MTD) += of_mtd.o
 obj-$(CONFIG_OF_BAREBOX_DRIVERS) += barebox.o
 obj-$(CONFIG_OF_OVERLAY) += overlay.o resolver.o of_firmware.o
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 4a4eeba24bc3..ee875f5787b1 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -551,6 +551,9 @@ int of_add_reserve_entry(resource_size_t start, resource_size_t end,
 	if (flags & OF_RESERVE_ENTRY_FLAG_RUNTIME_FW)
 		of_reserve_map.runtime_fw |= BIT(e);
 
+	if (flags & OF_RESERVE_ENTRY_FLAG_NO_FIXUP)
+		of_reserve_map.no_fixup |= BIT(e);
+
 	return 0;
 }
 
@@ -592,11 +595,12 @@ void fdt_add_reserve_map(void *__fdt)
 
 	fdt_res += n;
 
-	for (i = 0; i < res->num_entries; i++) {
+	for (i = 0; i < res->num_entries; i++, fdt_res++) {
+		if (BIT(i) & res->no_fixup)
+			continue;
+
 		of_write_number(&fdt_res->address, res->start[i], 2);
-		of_write_number(&fdt_res->size, res->end[i] - res->start[i] + 1,
-				2);
-		fdt_res++;
+		of_write_number(&fdt_res->size, res->end[i] - res->start[i] + 1, 2);
 	}
 
 	of_write_number(&fdt_res->address, (unsigned long)__fdt, 2);
diff --git a/include/of.h b/include/of.h
index d55016f1e372..c2a0cdae8dc2 100644
--- a/include/of.h
+++ b/include/of.h
@@ -56,9 +56,11 @@ struct of_reserve_map {
 	uint64_t end[OF_MAX_RESERVE_MAP];
 	int num_entries;
 	u16 runtime_fw : OF_MAX_RESERVE_MAP;
+	u16 no_fixup : OF_MAX_RESERVE_MAP;
 };
 
 #define OF_RESERVE_ENTRY_FLAG_RUNTIME_FW	BIT(0)
+#define OF_RESERVE_ENTRY_FLAG_NO_FIXUP		BIT(1)
 
 int of_add_reserve_entry(resource_size_t start, resource_size_t end,
 		 int flags);
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH] fixup! of: request reserved memory regions so other code can't

[Ahmad Fatoum]

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
can squash for v2
---
 drivers/of/reserved-mem.c | 65 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)
 create mode 100644 drivers/of/reserved-mem.c

diff --git a/drivers/of/reserved-mem.c b/drivers/of/reserved-mem.c
new file mode 100644
index 000000000000..efa96fe65cd9
--- /dev/null
+++ b/drivers/of/reserved-mem.c
@@ -0,0 +1,65 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// SPDX-FileCopyrightText: 2021 Rouven Czerwinski <r.czerwinski@pengutronix.de>, Pengutronix
+
+#define pr_fmt(fmt) "reserved-memory: " fmt
+
+#include <common.h>
+#include <init.h>
+#include <of.h>
+#include <of_address.h>
+
+static void reserve(const char *name, u64 start, u64 end, bool xn)
+{
+	int flags = OF_RESERVE_ENTRY_FLAG_NO_FIXUP;
+
+	if (xn)
+		flags |= OF_RESERVE_ENTRY_FLAG_RUNTIME_FW;
+
+	pr_debug("region %s 0x%08llx-0x%08llx\n", name, start, end);
+
+	of_add_reserve_entry(start, end, flags);
+}
+
+static int reserved_mem_read(void)
+{
+	struct device_node *node, *child;
+	struct resource resource;
+	const __be32 *reg;
+
+	node = of_find_node_by_path("/reserved-memory");
+	if (node) {
+		for_each_available_child_of_node(node, child) {
+			/* skip e.g. linux,cma */
+			if (!of_get_property(child, "reg", NULL))
+				continue;
+
+			of_address_to_resource(child, 0, &resource);
+
+			/*
+			 * Just keep on mapping 1:1, but avoid speculative
+			 * execution into such a region
+			 */
+			reserve(child->name,
+				resource.start, resource.end,
+				of_find_property(child, "no-map", 0));
+		}
+	}
+
+	node = of_find_node_by_path("/memreserve");
+	reg = of_get_property(node, "reg", NULL);
+	if (reg) {
+		u32 na = 2, ns = 2;
+		u64 addr, size;
+
+		of_property_read_u32(node, "#address-cells", &na);
+		of_property_read_u32(node, "#size-cells", &ns);
+
+		addr = of_read_number(reg, na);
+		size = of_read_number(reg + na, ns);
+
+		reserve("/memreserve", addr, addr + size - 1, false);
+	}
+
+	return 0;
+}
+postconsole_initcall(reserved_mem_read);
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 4/4] of: request reserved memory regions so other code can't

[Sascha Hauer]

On Thu, Jun 09, 2022 at 07:43:42AM +0200, Ahmad Fatoum wrote:
> From: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> 
> Add a reserved_mem_read initcall which parses the reserved-memory
> entries and adds barebox of reserve entries. This avoids e.g. bootm
> trying to place the kernel into a reserved region.
> 
> Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> Compared with Rouven's v2, rename OF_RESERVE_ENTRY_FLAG_NO_RESERVE
> to NO_FIXUP and read both /reserved-memory and /memreserve
> to request memory regions.
> ---
>  common/memory.c     | 21 +++++++++++++++++++--
>  drivers/of/Makefile |  1 +
>  drivers/of/fdt.c    | 12 ++++++++----
>  include/of.h        |  2 ++
>  4 files changed, 30 insertions(+), 6 deletions(-)
> 
> diff --git a/common/memory.c b/common/memory.c
> index 95995bb6e310..b40c74bfe97f 100644
> --- a/common/memory.c
> +++ b/common/memory.c
> @@ -12,6 +12,7 @@
>  #include <asm-generic/memory_layout.h>
>  #include <asm/sections.h>
>  #include <malloc.h>
> +#include <of.h>
>  
>  /*
>   * Begin and End of memory area for malloc(), and current "brk"
> @@ -53,9 +54,12 @@ void mem_malloc_init(void *start, void *end)
>  	mem_malloc_initialized = 1;
>  }
>  
> -#if !defined __SANDBOX__
>  static int mem_malloc_resource(void)
>  {
> +	struct of_reserve_map *map;
> +	int i;
> +
> +#if !defined __SANDBOX__
>  	/*
>  	 * Normally it's a bug when one of these fails,
>  	 * but we have some setups where some of these
> @@ -80,10 +84,23 @@ static int mem_malloc_resource(void)
>  #ifdef STACK_BASE
>  	request_sdram_region("stack", STACK_BASE, STACK_SIZE);
>  #endif
> +#endif
> +
> +	map = of_get_reserve_map();
> +	if (!map)
> +		return 0;
> +
> +	for (i = 0; i < map->num_entries; i++) {
> +		const char *name;
> +
> +		name = map->runtime_fw & BIT(i) ? "protected code" : "protected data";
> +		request_sdram_region(name, map->start[i],
> +				     map->end[i] - map->start[i] + 1);
> +	}

Regions for entries that are present up to this point are always requested
whereas regions for entries that are added later are never requested.
This only works for you because all regions you are interested in
(OPTEE, ppa) happen to be registered before this point while all others
that you can't do a request_sdram_region() on happen to be added after
this point. That looks quite fragile.

If you want to protect OPTEE resources then call request_sdram_region()
from the code instantiating OPTEE. In case of OPTEE this happens too
early when the resource system is not yet ready, so pick it up in a
later initcall.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 4/4] of: request reserved memory regions so other code can't

[Ahmad Fatoum]

On 09.06.22 10:31, Sascha Hauer wrote:
> On Thu, Jun 09, 2022 at 07:43:42AM +0200, Ahmad Fatoum wrote:
>> From: Rouven Czerwinski <r.czerwinski@pengutronix.de>
>>
>> Add a reserved_mem_read initcall which parses the reserved-memory
>> entries and adds barebox of reserve entries. This avoids e.g. bootm
>> trying to place the kernel into a reserved region.
>>
>> Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>> Compared with Rouven's v2, rename OF_RESERVE_ENTRY_FLAG_NO_RESERVE
>> to NO_FIXUP and read both /reserved-memory and /memreserve
>> to request memory regions.
>> ---
>>  common/memory.c     | 21 +++++++++++++++++++--
>>  drivers/of/Makefile |  1 +
>>  drivers/of/fdt.c    | 12 ++++++++----
>>  include/of.h        |  2 ++
>>  4 files changed, 30 insertions(+), 6 deletions(-)
>>
>> diff --git a/common/memory.c b/common/memory.c
>> index 95995bb6e310..b40c74bfe97f 100644
>> --- a/common/memory.c
>> +++ b/common/memory.c
>> @@ -12,6 +12,7 @@
>>  #include <asm-generic/memory_layout.h>
>>  #include <asm/sections.h>
>>  #include <malloc.h>
>> +#include <of.h>
>>  
>>  /*
>>   * Begin and End of memory area for malloc(), and current "brk"
>> @@ -53,9 +54,12 @@ void mem_malloc_init(void *start, void *end)
>>  	mem_malloc_initialized = 1;
>>  }
>>  
>> -#if !defined __SANDBOX__
>>  static int mem_malloc_resource(void)
>>  {
>> +	struct of_reserve_map *map;
>> +	int i;
>> +
>> +#if !defined __SANDBOX__
>>  	/*
>>  	 * Normally it's a bug when one of these fails,
>>  	 * but we have some setups where some of these
>> @@ -80,10 +84,23 @@ static int mem_malloc_resource(void)
>>  #ifdef STACK_BASE
>>  	request_sdram_region("stack", STACK_BASE, STACK_SIZE);
>>  #endif
>> +#endif
>> +
>> +	map = of_get_reserve_map();
>> +	if (!map)
>> +		return 0;
>> +
>> +	for (i = 0; i < map->num_entries; i++) {
>> +		const char *name;
>> +
>> +		name = map->runtime_fw & BIT(i) ? "protected code" : "protected data";
>> +		request_sdram_region(name, map->start[i],
>> +				     map->end[i] - map->start[i] + 1);
>> +	}
> 
> Regions for entries that are present up to this point are always requested
> whereas regions for entries that are added later are never requested.
> This only works for you because all regions you are interested in
> (OPTEE, ppa) happen to be registered before this point while all others
> that you can't do a request_sdram_region() on happen to be added after
> this point. That looks quite fragile.

I can't do request_sdram_region before the SDRAM is registered. 

> If you want to protect OPTEE resources then call request_sdram_region()
> from the code instantiating OPTEE. In case of OPTEE this happens too
> early when the resource system is not yet ready, so pick it up in a
> later initcall.

This here is the later initcall doing the requesting?

> 
> Sascha
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox