diff mbox series

[v2] of: of_reserved_mem: clean-up reserved memory with no-map

Message ID 20240501132359.488616-1-skseofh@gmail.com
State Needs Review / ACK
Headers show
Series [v2] of: of_reserved_mem: clean-up reserved memory with no-map | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied fail build log

Commit Message

DaeRo Lee May 1, 2024, 1:23 p.m. UTC
From: Daero Lee <daero_le.lee@samsung.com>

In early_init_dt_reserve_memory we only add memory w/o no-map flag to
memblock.reserved. But we need to add memory w/ no-map flag to
memblock.reserved, because NOMAP and memblock.reserved are semantically
different.

Signed-off-by: Daero Lee <daero_le.lee@samsung.com>
---
 drivers/of/of_reserved_mem.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Rob Herring (Arm) May 22, 2024, 2:31 p.m. UTC | #1
On Wed, May 01, 2024 at 10:23:59PM +0900, skseofh@gmail.com wrote:
> From: Daero Lee <daero_le.lee@samsung.com>
> 
> In early_init_dt_reserve_memory we only add memory w/o no-map flag to
> memblock.reserved. But we need to add memory w/ no-map flag to
> memblock.reserved, because NOMAP and memblock.reserved are semantically
> different.
> 
> Signed-off-by: Daero Lee <daero_le.lee@samsung.com>
> ---
>  drivers/of/of_reserved_mem.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 8236ecae2953..d00a17a9cebc 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -81,6 +81,7 @@ static void __init fdt_reserved_mem_save_node(unsigned long node, const char *un
>  static int __init early_init_dt_reserve_memory(phys_addr_t base,
>  					       phys_addr_t size, bool nomap)
>  {
> +	int err = 0;
>  	if (nomap) {
>  		/*
>  		 * If the memory is already reserved (by another region), we
> @@ -91,7 +92,10 @@ static int __init early_init_dt_reserve_memory(phys_addr_t base,
>  		    memblock_is_region_reserved(base, size))
>  			return -EBUSY;
>  
> -		return memblock_mark_nomap(base, size);
> +
> +		err = memblock_mark_nomap(base, size);

The last time this was touched, it was to make the handling aligned with 
EFI memory map handling. Is that still going to be the case with this 
change? Or does EFI memory map handling have the same issue?

> +		if (err)
> +			return err;
>  	}
>  	return memblock_reserve(base, size);
>  }
> -- 
> 2.25.1
>
DaeRo Lee May 24, 2024, 9:32 a.m. UTC | #2
2024년 5월 22일 (수) 오후 11:31, Rob Herring <robh@kernel.org>님이 작성:
>
> On Wed, May 01, 2024 at 10:23:59PM +0900, skseofh@gmail.com wrote:
> > From: Daero Lee <daero_le.lee@samsung.com>
> >
> > In early_init_dt_reserve_memory we only add memory w/o no-map flag to
> > memblock.reserved. But we need to add memory w/ no-map flag to
> > memblock.reserved, because NOMAP and memblock.reserved are semantically
> > different.
> >
> > Signed-off-by: Daero Lee <daero_le.lee@samsung.com>
> > ---
> >  drivers/of/of_reserved_mem.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> > index 8236ecae2953..d00a17a9cebc 100644
> > --- a/drivers/of/of_reserved_mem.c
> > +++ b/drivers/of/of_reserved_mem.c
> > @@ -81,6 +81,7 @@ static void __init fdt_reserved_mem_save_node(unsigned long node, const char *un
> >  static int __init early_init_dt_reserve_memory(phys_addr_t base,
> >                                              phys_addr_t size, bool nomap)
> >  {
> > +     int err = 0;
> >       if (nomap) {
> >               /*
> >                * If the memory is already reserved (by another region), we
> > @@ -91,7 +92,10 @@ static int __init early_init_dt_reserve_memory(phys_addr_t base,
> >                   memblock_is_region_reserved(base, size))
> >                       return -EBUSY;
> >
> > -             return memblock_mark_nomap(base, size);
> > +
> > +             err = memblock_mark_nomap(base, size);
>
> The last time this was touched, it was to make the handling aligned with
> EFI memory map handling. Is that still going to be the case with this
> change? Or does EFI memory map handling have the same issue?
Can I get more information about EFI memory map handling that you're saying?

1) Are you talking about uefi_mem in the reserved-memory node like below?
 ex) arm64/boot/dts/qcom/qcs404.dtsi
                uefi_mem: memory@9f800000 {
                        reg = <0 0x9f800000 0 0x800000>;
                        no-map;
                };

2) Or, about handling EFI memory map function efi_init() -> reserve_regions()?

>
> > +             if (err)
> > +                     return err;
> >       }
> >       return memblock_reserve(base, size);
> >  }
> > --
> > 2.25.1
> >
diff mbox series

Patch

diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index 8236ecae2953..d00a17a9cebc 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -81,6 +81,7 @@  static void __init fdt_reserved_mem_save_node(unsigned long node, const char *un
 static int __init early_init_dt_reserve_memory(phys_addr_t base,
 					       phys_addr_t size, bool nomap)
 {
+	int err = 0;
 	if (nomap) {
 		/*
 		 * If the memory is already reserved (by another region), we
@@ -91,7 +92,10 @@  static int __init early_init_dt_reserve_memory(phys_addr_t base,
 		    memblock_is_region_reserved(base, size))
 			return -EBUSY;
 
-		return memblock_mark_nomap(base, size);
+
+		err = memblock_mark_nomap(base, size);
+		if (err)
+			return err;
 	}
 	return memblock_reserve(base, size);
 }