[PATCH] riscv: add BUILTIN_DTB support for MMU-enabled targets

[PATCH] riscv: add BUILTIN_DTB support for MMU-enabled targets

[Vitaly Wool]

Sometimes, especially in a production system we may not want to
use a "smart bootloader" like u-boot to load kernel, ramdisk and
device tree from a filesystem on eMMC, but rather load the kernel
from a NAND partition and just run it as soon as we can, and in
this case it is convenient to have device tree compiled into the
kernel binary. Since this case is not limited to MMU-less systems,
let's support it for these which have MMU enabled too.

Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.com>
---
 arch/riscv/Kconfig   |  1 -
 arch/riscv/mm/init.c | 12 ++++++++++--
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 2b41f6d8e458..9464b4e3a71a 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -419,7 +419,6 @@ endmenu
 
 config BUILTIN_DTB
 	def_bool n
-	depends on RISCV_M_MODE
 	depends on OF
 
 menu "Power management options"
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 87c305c566ac..5d1c7a3ec01c 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -194,12 +194,20 @@ void __init setup_bootmem(void)
 	setup_initrd();
 #endif /* CONFIG_BLK_DEV_INITRD */
 
+	/*
+	 * If DTB is built in, no need to reserve its memblock.
+	 * OTOH, initial_boot_params has to be set to properly copy DTB
+	 * before unflattening later on.
+	 */
+	if (IS_ENABLED(CONFIG_BUILTIN_DTB))
+		initial_boot_params = __va(dtb_early_pa);
+	else
+		memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
+
 	/*
 	 * Avoid using early_init_fdt_reserve_self() since __pa() does
 	 * not work for DTB pointers that are fixmap addresses
 	 */
-	memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
-
 	early_init_fdt_scan_reserved_mem();
 	dma_contiguous_reserve(dma32_phys_limit);
 	memblock_allow_resize();
-- 
2.29.2

[PATCH] riscv: add BUILTIN_DTB support for MMU-enabled targets

[Vitaly Wool]

Sometimes, especially in a production system we may not want to
use a "smart bootloader" like u-boot to load kernel, ramdisk and
device tree from a filesystem on eMMC, but rather load the kernel
from a NAND partition and just run it as soon as we can, and in
this case it is convenient to have device tree compiled into the
kernel binary. Since this case is not limited to MMU-less systems,
let's support it for these which have MMU enabled too.

Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.com>
---
 arch/riscv/Kconfig   |  1 -
 arch/riscv/mm/init.c | 12 ++++++++++--
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 2b41f6d8e458..9464b4e3a71a 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -419,7 +419,6 @@ endmenu
 
 config BUILTIN_DTB
 	def_bool n
-	depends on RISCV_M_MODE
 	depends on OF
 
 menu "Power management options"
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 87c305c566ac..5d1c7a3ec01c 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -194,12 +194,20 @@ void __init setup_bootmem(void)
 	setup_initrd();
 #endif /* CONFIG_BLK_DEV_INITRD */
 
+	/*
+	 * If DTB is built in, no need to reserve its memblock.
+	 * OTOH, initial_boot_params has to be set to properly copy DTB
+	 * before unflattening later on.
+	 */
+	if (IS_ENABLED(CONFIG_BUILTIN_DTB))
+		initial_boot_params = __va(dtb_early_pa);
+	else
+		memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
+
 	/*
 	 * Avoid using early_init_fdt_reserve_self() since __pa() does
 	 * not work for DTB pointers that are fixmap addresses
 	 */
-	memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
-
 	early_init_fdt_scan_reserved_mem();
 	dma_contiguous_reserve(dma32_phys_limit);
 	memblock_allow_resize();
-- 
2.29.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] riscv: add BUILTIN_DTB support for MMU-enabled targets

[Anup Patel]

On Sat, Dec 26, 2020 at 10:03 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
>
> Sometimes, especially in a production system we may not want to
> use a "smart bootloader" like u-boot to load kernel, ramdisk and
> device tree from a filesystem on eMMC, but rather load the kernel
> from a NAND partition and just run it as soon as we can, and in
> this case it is convenient to have device tree compiled into the
> kernel binary. Since this case is not limited to MMU-less systems,
> let's support it for these which have MMU enabled too.
>
> Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.com>
> ---
>  arch/riscv/Kconfig   |  1 -
>  arch/riscv/mm/init.c | 12 ++++++++++--
>  2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 2b41f6d8e458..9464b4e3a71a 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -419,7 +419,6 @@ endmenu
>
>  config BUILTIN_DTB
>         def_bool n
> -       depends on RISCV_M_MODE
>         depends on OF
>
>  menu "Power management options"
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 87c305c566ac..5d1c7a3ec01c 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -194,12 +194,20 @@ void __init setup_bootmem(void)
>         setup_initrd();
>  #endif /* CONFIG_BLK_DEV_INITRD */
>
> +       /*
> +        * If DTB is built in, no need to reserve its memblock.
> +        * OTOH, initial_boot_params has to be set to properly copy DTB
> +        * before unflattening later on.
> +        */
> +       if (IS_ENABLED(CONFIG_BUILTIN_DTB))
> +               initial_boot_params = __va(dtb_early_pa);

Don't assign initial_boot_params directly here because the
early_init_dt_scan() will do it.

The setup_vm() is supposed to setup dtb_early_va and dtb_early_pa
for MMU-enabled case so please add a "#ifdef" over there for the
built-in DTB case.

> +       else
> +               memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> +
>         /*
>          * Avoid using early_init_fdt_reserve_self() since __pa() does
>          * not work for DTB pointers that are fixmap addresses
>          */

This comment needs to be updated and moved along the memblock_reserve()
statement.

> -       memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> -
>         early_init_fdt_scan_reserved_mem();
>         dma_contiguous_reserve(dma32_phys_limit);
>         memblock_allow_resize();
> --
> 2.29.2
>

This patch should be based upon Damiens builtin DTB patch.
Refer, https://www.spinics.net/lists/linux-gpio/msg56616.html

Regards,
Anup

Re: [PATCH] riscv: add BUILTIN_DTB support for MMU-enabled targets

[Anup Patel]

On Sat, Dec 26, 2020 at 10:03 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
>
> Sometimes, especially in a production system we may not want to
> use a "smart bootloader" like u-boot to load kernel, ramdisk and
> device tree from a filesystem on eMMC, but rather load the kernel
> from a NAND partition and just run it as soon as we can, and in
> this case it is convenient to have device tree compiled into the
> kernel binary. Since this case is not limited to MMU-less systems,
> let's support it for these which have MMU enabled too.
>
> Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.com>
> ---
>  arch/riscv/Kconfig   |  1 -
>  arch/riscv/mm/init.c | 12 ++++++++++--
>  2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 2b41f6d8e458..9464b4e3a71a 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -419,7 +419,6 @@ endmenu
>
>  config BUILTIN_DTB
>         def_bool n
> -       depends on RISCV_M_MODE
>         depends on OF
>
>  menu "Power management options"
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 87c305c566ac..5d1c7a3ec01c 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -194,12 +194,20 @@ void __init setup_bootmem(void)
>         setup_initrd();
>  #endif /* CONFIG_BLK_DEV_INITRD */
>
> +       /*
> +        * If DTB is built in, no need to reserve its memblock.
> +        * OTOH, initial_boot_params has to be set to properly copy DTB
> +        * before unflattening later on.
> +        */
> +       if (IS_ENABLED(CONFIG_BUILTIN_DTB))
> +               initial_boot_params = __va(dtb_early_pa);

Don't assign initial_boot_params directly here because the
early_init_dt_scan() will do it.

The setup_vm() is supposed to setup dtb_early_va and dtb_early_pa
for MMU-enabled case so please add a "#ifdef" over there for the
built-in DTB case.

> +       else
> +               memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> +
>         /*
>          * Avoid using early_init_fdt_reserve_self() since __pa() does
>          * not work for DTB pointers that are fixmap addresses
>          */

This comment needs to be updated and moved along the memblock_reserve()
statement.

> -       memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> -
>         early_init_fdt_scan_reserved_mem();
>         dma_contiguous_reserve(dma32_phys_limit);
>         memblock_allow_resize();
> --
> 2.29.2
>

This patch should be based upon Damiens builtin DTB patch.
Refer, https://www.spinics.net/lists/linux-gpio/msg56616.html

Regards,
Anup

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] riscv: add BUILTIN_DTB support for MMU-enabled targets

[Vitaly Wool]

On Mon, Dec 28, 2020 at 12:59 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Sat, Dec 26, 2020 at 10:03 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
> >
> > Sometimes, especially in a production system we may not want to
> > use a "smart bootloader" like u-boot to load kernel, ramdisk and
> > device tree from a filesystem on eMMC, but rather load the kernel
> > from a NAND partition and just run it as soon as we can, and in
> > this case it is convenient to have device tree compiled into the
> > kernel binary. Since this case is not limited to MMU-less systems,
> > let's support it for these which have MMU enabled too.
> >
> > Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.com>
> > ---
> >  arch/riscv/Kconfig   |  1 -
> >  arch/riscv/mm/init.c | 12 ++++++++++--
> >  2 files changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 2b41f6d8e458..9464b4e3a71a 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -419,7 +419,6 @@ endmenu
> >
> >  config BUILTIN_DTB
> >         def_bool n
> > -       depends on RISCV_M_MODE
> >         depends on OF
> >
> >  menu "Power management options"
> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > index 87c305c566ac..5d1c7a3ec01c 100644
> > --- a/arch/riscv/mm/init.c
> > +++ b/arch/riscv/mm/init.c
> > @@ -194,12 +194,20 @@ void __init setup_bootmem(void)
> >         setup_initrd();
> >  #endif /* CONFIG_BLK_DEV_INITRD */
> >
> > +       /*
> > +        * If DTB is built in, no need to reserve its memblock.
> > +        * OTOH, initial_boot_params has to be set to properly copy DTB
> > +        * before unflattening later on.
> > +        */
> > +       if (IS_ENABLED(CONFIG_BUILTIN_DTB))
> > +               initial_boot_params = __va(dtb_early_pa);
>
> Don't assign initial_boot_params directly here because the
> early_init_dt_scan() will do it.

early_init_dt_scan will set initial_boot_params to dtb_early_va from
the early mapping which will be gone by the time
unflatten_and_copy_device_tree() is called.

> The setup_vm() is supposed to setup dtb_early_va and dtb_early_pa
> for MMU-enabled case so please add a "#ifdef" over there for the
> built-in DTB case.
>
> > +       else
> > +               memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> > +
> >         /*
> >          * Avoid using early_init_fdt_reserve_self() since __pa() does
> >          * not work for DTB pointers that are fixmap addresses
> >          */
>
> This comment needs to be updated and moved along the memblock_reserve()
> statement.
>
> > -       memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> > -
> >         early_init_fdt_scan_reserved_mem();
> >         dma_contiguous_reserve(dma32_phys_limit);
> >         memblock_allow_resize();
> > --
> > 2.29.2
> >
>
> This patch should be based upon Damiens builtin DTB patch.
> Refer, https://www.spinics.net/lists/linux-gpio/msg56616.html

Thanks for the pointer, however I don't think our patches have
intersections. Besides, Damien is dealing with the MMU-less case
there.

Best regards,
   Vitaly

Re: [PATCH] riscv: add BUILTIN_DTB support for MMU-enabled targets

[Vitaly Wool]

On Mon, Dec 28, 2020 at 12:59 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Sat, Dec 26, 2020 at 10:03 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
> >
> > Sometimes, especially in a production system we may not want to
> > use a "smart bootloader" like u-boot to load kernel, ramdisk and
> > device tree from a filesystem on eMMC, but rather load the kernel
> > from a NAND partition and just run it as soon as we can, and in
> > this case it is convenient to have device tree compiled into the
> > kernel binary. Since this case is not limited to MMU-less systems,
> > let's support it for these which have MMU enabled too.
> >
> > Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.com>
> > ---
> >  arch/riscv/Kconfig   |  1 -
> >  arch/riscv/mm/init.c | 12 ++++++++++--
> >  2 files changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 2b41f6d8e458..9464b4e3a71a 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -419,7 +419,6 @@ endmenu
> >
> >  config BUILTIN_DTB
> >         def_bool n
> > -       depends on RISCV_M_MODE
> >         depends on OF
> >
> >  menu "Power management options"
> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > index 87c305c566ac..5d1c7a3ec01c 100644
> > --- a/arch/riscv/mm/init.c
> > +++ b/arch/riscv/mm/init.c
> > @@ -194,12 +194,20 @@ void __init setup_bootmem(void)
> >         setup_initrd();
> >  #endif /* CONFIG_BLK_DEV_INITRD */
> >
> > +       /*
> > +        * If DTB is built in, no need to reserve its memblock.
> > +        * OTOH, initial_boot_params has to be set to properly copy DTB
> > +        * before unflattening later on.
> > +        */
> > +       if (IS_ENABLED(CONFIG_BUILTIN_DTB))
> > +               initial_boot_params = __va(dtb_early_pa);
>
> Don't assign initial_boot_params directly here because the
> early_init_dt_scan() will do it.

early_init_dt_scan will set initial_boot_params to dtb_early_va from
the early mapping which will be gone by the time
unflatten_and_copy_device_tree() is called.

> The setup_vm() is supposed to setup dtb_early_va and dtb_early_pa
> for MMU-enabled case so please add a "#ifdef" over there for the
> built-in DTB case.
>
> > +       else
> > +               memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> > +
> >         /*
> >          * Avoid using early_init_fdt_reserve_self() since __pa() does
> >          * not work for DTB pointers that are fixmap addresses
> >          */
>
> This comment needs to be updated and moved along the memblock_reserve()
> statement.
>
> > -       memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> > -
> >         early_init_fdt_scan_reserved_mem();
> >         dma_contiguous_reserve(dma32_phys_limit);
> >         memblock_allow_resize();
> > --
> > 2.29.2
> >
>
> This patch should be based upon Damiens builtin DTB patch.
> Refer, https://www.spinics.net/lists/linux-gpio/msg56616.html

Thanks for the pointer, however I don't think our patches have
intersections. Besides, Damien is dealing with the MMU-less case
there.

Best regards,
   Vitaly

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] riscv: add BUILTIN_DTB support for MMU-enabled targets

[Anup Patel]

On Mon, Dec 28, 2020 at 7:05 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
>
> On Mon, Dec 28, 2020 at 12:59 PM Anup Patel <anup@brainfault.org> wrote:
> >
> > On Sat, Dec 26, 2020 at 10:03 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
> > >
> > > Sometimes, especially in a production system we may not want to
> > > use a "smart bootloader" like u-boot to load kernel, ramdisk and
> > > device tree from a filesystem on eMMC, but rather load the kernel
> > > from a NAND partition and just run it as soon as we can, and in
> > > this case it is convenient to have device tree compiled into the
> > > kernel binary. Since this case is not limited to MMU-less systems,
> > > let's support it for these which have MMU enabled too.
> > >
> > > Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.com>
> > > ---
> > >  arch/riscv/Kconfig   |  1 -
> > >  arch/riscv/mm/init.c | 12 ++++++++++--
> > >  2 files changed, 10 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > index 2b41f6d8e458..9464b4e3a71a 100644
> > > --- a/arch/riscv/Kconfig
> > > +++ b/arch/riscv/Kconfig
> > > @@ -419,7 +419,6 @@ endmenu
> > >
> > >  config BUILTIN_DTB
> > >         def_bool n
> > > -       depends on RISCV_M_MODE
> > >         depends on OF
> > >
> > >  menu "Power management options"
> > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > > index 87c305c566ac..5d1c7a3ec01c 100644
> > > --- a/arch/riscv/mm/init.c
> > > +++ b/arch/riscv/mm/init.c
> > > @@ -194,12 +194,20 @@ void __init setup_bootmem(void)
> > >         setup_initrd();
> > >  #endif /* CONFIG_BLK_DEV_INITRD */
> > >
> > > +       /*
> > > +        * If DTB is built in, no need to reserve its memblock.
> > > +        * OTOH, initial_boot_params has to be set to properly copy DTB
> > > +        * before unflattening later on.
> > > +        */
> > > +       if (IS_ENABLED(CONFIG_BUILTIN_DTB))
> > > +               initial_boot_params = __va(dtb_early_pa);
> >
> > Don't assign initial_boot_params directly here because the
> > early_init_dt_scan() will do it.
>
> early_init_dt_scan will set initial_boot_params to dtb_early_va from
> the early mapping which will be gone by the time
> unflatten_and_copy_device_tree() is called.

That's why we are doing early_init_dt_verify() again for the MMU-enabled
case which already takes care of your concern.

We use early_init_dt_verify() like most architectures to set the initial DTB.

>
> > The setup_vm() is supposed to setup dtb_early_va and dtb_early_pa
> > for MMU-enabled case so please add a "#ifdef" over there for the
> > built-in DTB case.
> >
> > > +       else
> > > +               memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> > > +
> > >         /*
> > >          * Avoid using early_init_fdt_reserve_self() since __pa() does
> > >          * not work for DTB pointers that are fixmap addresses
> > >          */
> >
> > This comment needs to be updated and moved along the memblock_reserve()
> > statement.
> >
> > > -       memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> > > -
> > >         early_init_fdt_scan_reserved_mem();
> > >         dma_contiguous_reserve(dma32_phys_limit);
> > >         memblock_allow_resize();
> > > --
> > > 2.29.2
> > >
> >
> > This patch should be based upon Damiens builtin DTB patch.
> > Refer, https://www.spinics.net/lists/linux-gpio/msg56616.html
>
> Thanks for the pointer, however I don't think our patches have
> intersections. Besides, Damien is dealing with the MMU-less case
> there.

Damien's patch is also trying to move to use generic BUILTIN_DTB
support for the MMU-less case so it is similar work hence the chance
of patch conflict.

Regards,
Anup

Re: [PATCH] riscv: add BUILTIN_DTB support for MMU-enabled targets

[Anup Patel]

On Mon, Dec 28, 2020 at 7:05 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
>
> On Mon, Dec 28, 2020 at 12:59 PM Anup Patel <anup@brainfault.org> wrote:
> >
> > On Sat, Dec 26, 2020 at 10:03 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
> > >
> > > Sometimes, especially in a production system we may not want to
> > > use a "smart bootloader" like u-boot to load kernel, ramdisk and
> > > device tree from a filesystem on eMMC, but rather load the kernel
> > > from a NAND partition and just run it as soon as we can, and in
> > > this case it is convenient to have device tree compiled into the
> > > kernel binary. Since this case is not limited to MMU-less systems,
> > > let's support it for these which have MMU enabled too.
> > >
> > > Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.com>
> > > ---
> > >  arch/riscv/Kconfig   |  1 -
> > >  arch/riscv/mm/init.c | 12 ++++++++++--
> > >  2 files changed, 10 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > index 2b41f6d8e458..9464b4e3a71a 100644
> > > --- a/arch/riscv/Kconfig
> > > +++ b/arch/riscv/Kconfig
> > > @@ -419,7 +419,6 @@ endmenu
> > >
> > >  config BUILTIN_DTB
> > >         def_bool n
> > > -       depends on RISCV_M_MODE
> > >         depends on OF
> > >
> > >  menu "Power management options"
> > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > > index 87c305c566ac..5d1c7a3ec01c 100644
> > > --- a/arch/riscv/mm/init.c
> > > +++ b/arch/riscv/mm/init.c
> > > @@ -194,12 +194,20 @@ void __init setup_bootmem(void)
> > >         setup_initrd();
> > >  #endif /* CONFIG_BLK_DEV_INITRD */
> > >
> > > +       /*
> > > +        * If DTB is built in, no need to reserve its memblock.
> > > +        * OTOH, initial_boot_params has to be set to properly copy DTB
> > > +        * before unflattening later on.
> > > +        */
> > > +       if (IS_ENABLED(CONFIG_BUILTIN_DTB))
> > > +               initial_boot_params = __va(dtb_early_pa);
> >
> > Don't assign initial_boot_params directly here because the
> > early_init_dt_scan() will do it.
>
> early_init_dt_scan will set initial_boot_params to dtb_early_va from
> the early mapping which will be gone by the time
> unflatten_and_copy_device_tree() is called.

That's why we are doing early_init_dt_verify() again for the MMU-enabled
case which already takes care of your concern.

We use early_init_dt_verify() like most architectures to set the initial DTB.

>
> > The setup_vm() is supposed to setup dtb_early_va and dtb_early_pa
> > for MMU-enabled case so please add a "#ifdef" over there for the
> > built-in DTB case.
> >
> > > +       else
> > > +               memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> > > +
> > >         /*
> > >          * Avoid using early_init_fdt_reserve_self() since __pa() does
> > >          * not work for DTB pointers that are fixmap addresses
> > >          */
> >
> > This comment needs to be updated and moved along the memblock_reserve()
> > statement.
> >
> > > -       memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> > > -
> > >         early_init_fdt_scan_reserved_mem();
> > >         dma_contiguous_reserve(dma32_phys_limit);
> > >         memblock_allow_resize();
> > > --
> > > 2.29.2
> > >
> >
> > This patch should be based upon Damiens builtin DTB patch.
> > Refer, https://www.spinics.net/lists/linux-gpio/msg56616.html
>
> Thanks for the pointer, however I don't think our patches have
> intersections. Besides, Damien is dealing with the MMU-less case
> there.

Damien's patch is also trying to move to use generic BUILTIN_DTB
support for the MMU-less case so it is similar work hence the chance
of patch conflict.

Regards,
Anup

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] riscv: add BUILTIN_DTB support for MMU-enabled targets

[Vitaly Wool]

On Mon, Dec 28, 2020 at 3:10 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Mon, Dec 28, 2020 at 7:05 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
> >
> > On Mon, Dec 28, 2020 at 12:59 PM Anup Patel <anup@brainfault.org> wrote:
> > >
> > > On Sat, Dec 26, 2020 at 10:03 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
> > > >
> > > > Sometimes, especially in a production system we may not want to
> > > > use a "smart bootloader" like u-boot to load kernel, ramdisk and
> > > > device tree from a filesystem on eMMC, but rather load the kernel
> > > > from a NAND partition and just run it as soon as we can, and in
> > > > this case it is convenient to have device tree compiled into the
> > > > kernel binary. Since this case is not limited to MMU-less systems,
> > > > let's support it for these which have MMU enabled too.
> > > >
> > > > Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.com>
> > > > ---
> > > >  arch/riscv/Kconfig   |  1 -
> > > >  arch/riscv/mm/init.c | 12 ++++++++++--
> > > >  2 files changed, 10 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > > index 2b41f6d8e458..9464b4e3a71a 100644
> > > > --- a/arch/riscv/Kconfig
> > > > +++ b/arch/riscv/Kconfig
> > > > @@ -419,7 +419,6 @@ endmenu
> > > >
> > > >  config BUILTIN_DTB
> > > >         def_bool n
> > > > -       depends on RISCV_M_MODE
> > > >         depends on OF
> > > >
> > > >  menu "Power management options"
> > > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > > > index 87c305c566ac..5d1c7a3ec01c 100644
> > > > --- a/arch/riscv/mm/init.c
> > > > +++ b/arch/riscv/mm/init.c
> > > > @@ -194,12 +194,20 @@ void __init setup_bootmem(void)
> > > >         setup_initrd();
> > > >  #endif /* CONFIG_BLK_DEV_INITRD */
> > > >
> > > > +       /*
> > > > +        * If DTB is built in, no need to reserve its memblock.
> > > > +        * OTOH, initial_boot_params has to be set to properly copy DTB
> > > > +        * before unflattening later on.
> > > > +        */
> > > > +       if (IS_ENABLED(CONFIG_BUILTIN_DTB))
> > > > +               initial_boot_params = __va(dtb_early_pa);
> > >
> > > Don't assign initial_boot_params directly here because the
> > > early_init_dt_scan() will do it.
> >
> > early_init_dt_scan will set initial_boot_params to dtb_early_va from
> > the early mapping which will be gone by the time
> > unflatten_and_copy_device_tree() is called.
>
> That's why we are doing early_init_dt_verify() again for the MMU-enabled
> case which already takes care of your concern.

I might be out in the woods here but... Do you mean the call to
early_init_dt_verify() in setup_arch() which is compiled out
completely in the CONFIG_BUILTIN_DTB case?
Or is there any other call that I'm overlooking?

Best regards,
   Vitaly

> We use early_init_dt_verify() like most architectures to set the initial DTB.
>
> >
> > > The setup_vm() is supposed to setup dtb_early_va and dtb_early_pa
> > > for MMU-enabled case so please add a "#ifdef" over there for the
> > > built-in DTB case.
> > >
> > > > +       else
> > > > +               memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> > > > +
> > > >         /*
> > > >          * Avoid using early_init_fdt_reserve_self() since __pa() does
> > > >          * not work for DTB pointers that are fixmap addresses
> > > >          */
> > >
> > > This comment needs to be updated and moved along the memblock_reserve()
> > > statement.
> > >
> > > > -       memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> > > > -
> > > >         early_init_fdt_scan_reserved_mem();
> > > >         dma_contiguous_reserve(dma32_phys_limit);
> > > >         memblock_allow_resize();
> > > > --
> > > > 2.29.2
> > > >
> > >
> > > This patch should be based upon Damiens builtin DTB patch.
> > > Refer, https://www.spinics.net/lists/linux-gpio/msg56616.html
> >
> > Thanks for the pointer, however I don't think our patches have
> > intersections. Besides, Damien is dealing with the MMU-less case
> > there.
>
> Damien's patch is also trying to move to use generic BUILTIN_DTB
> support for the MMU-less case so it is similar work hence the chance
> of patch conflict.
>
> Regards,
> Anup

Re: [PATCH] riscv: add BUILTIN_DTB support for MMU-enabled targets

[Vitaly Wool]

On Mon, Dec 28, 2020 at 3:10 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Mon, Dec 28, 2020 at 7:05 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
> >
> > On Mon, Dec 28, 2020 at 12:59 PM Anup Patel <anup@brainfault.org> wrote:
> > >
> > > On Sat, Dec 26, 2020 at 10:03 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
> > > >
> > > > Sometimes, especially in a production system we may not want to
> > > > use a "smart bootloader" like u-boot to load kernel, ramdisk and
> > > > device tree from a filesystem on eMMC, but rather load the kernel
> > > > from a NAND partition and just run it as soon as we can, and in
> > > > this case it is convenient to have device tree compiled into the
> > > > kernel binary. Since this case is not limited to MMU-less systems,
> > > > let's support it for these which have MMU enabled too.
> > > >
> > > > Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.com>
> > > > ---
> > > >  arch/riscv/Kconfig   |  1 -
> > > >  arch/riscv/mm/init.c | 12 ++++++++++--
> > > >  2 files changed, 10 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > > index 2b41f6d8e458..9464b4e3a71a 100644
> > > > --- a/arch/riscv/Kconfig
> > > > +++ b/arch/riscv/Kconfig
> > > > @@ -419,7 +419,6 @@ endmenu
> > > >
> > > >  config BUILTIN_DTB
> > > >         def_bool n
> > > > -       depends on RISCV_M_MODE
> > > >         depends on OF
> > > >
> > > >  menu "Power management options"
> > > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > > > index 87c305c566ac..5d1c7a3ec01c 100644
> > > > --- a/arch/riscv/mm/init.c
> > > > +++ b/arch/riscv/mm/init.c
> > > > @@ -194,12 +194,20 @@ void __init setup_bootmem(void)
> > > >         setup_initrd();
> > > >  #endif /* CONFIG_BLK_DEV_INITRD */
> > > >
> > > > +       /*
> > > > +        * If DTB is built in, no need to reserve its memblock.
> > > > +        * OTOH, initial_boot_params has to be set to properly copy DTB
> > > > +        * before unflattening later on.
> > > > +        */
> > > > +       if (IS_ENABLED(CONFIG_BUILTIN_DTB))
> > > > +               initial_boot_params = __va(dtb_early_pa);
> > >
> > > Don't assign initial_boot_params directly here because the
> > > early_init_dt_scan() will do it.
> >
> > early_init_dt_scan will set initial_boot_params to dtb_early_va from
> > the early mapping which will be gone by the time
> > unflatten_and_copy_device_tree() is called.
>
> That's why we are doing early_init_dt_verify() again for the MMU-enabled
> case which already takes care of your concern.

I might be out in the woods here but... Do you mean the call to
early_init_dt_verify() in setup_arch() which is compiled out
completely in the CONFIG_BUILTIN_DTB case?
Or is there any other call that I'm overlooking?

Best regards,
   Vitaly

> We use early_init_dt_verify() like most architectures to set the initial DTB.
>
> >
> > > The setup_vm() is supposed to setup dtb_early_va and dtb_early_pa
> > > for MMU-enabled case so please add a "#ifdef" over there for the
> > > built-in DTB case.
> > >
> > > > +       else
> > > > +               memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> > > > +
> > > >         /*
> > > >          * Avoid using early_init_fdt_reserve_self() since __pa() does
> > > >          * not work for DTB pointers that are fixmap addresses
> > > >          */
> > >
> > > This comment needs to be updated and moved along the memblock_reserve()
> > > statement.
> > >
> > > > -       memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> > > > -
> > > >         early_init_fdt_scan_reserved_mem();
> > > >         dma_contiguous_reserve(dma32_phys_limit);
> > > >         memblock_allow_resize();
> > > > --
> > > > 2.29.2
> > > >
> > >
> > > This patch should be based upon Damiens builtin DTB patch.
> > > Refer, https://www.spinics.net/lists/linux-gpio/msg56616.html
> >
> > Thanks for the pointer, however I don't think our patches have
> > intersections. Besides, Damien is dealing with the MMU-less case
> > there.
>
> Damien's patch is also trying to move to use generic BUILTIN_DTB
> support for the MMU-less case so it is similar work hence the chance
> of patch conflict.
>
> Regards,
> Anup

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] riscv: add BUILTIN_DTB support for MMU-enabled targets

[Anup Patel]

On Mon, Dec 28, 2020 at 10:08 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
>
> On Mon, Dec 28, 2020 at 3:10 PM Anup Patel <anup@brainfault.org> wrote:
> >
> > On Mon, Dec 28, 2020 at 7:05 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
> > >
> > > On Mon, Dec 28, 2020 at 12:59 PM Anup Patel <anup@brainfault.org> wrote:
> > > >
> > > > On Sat, Dec 26, 2020 at 10:03 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
> > > > >
> > > > > Sometimes, especially in a production system we may not want to
> > > > > use a "smart bootloader" like u-boot to load kernel, ramdisk and
> > > > > device tree from a filesystem on eMMC, but rather load the kernel
> > > > > from a NAND partition and just run it as soon as we can, and in
> > > > > this case it is convenient to have device tree compiled into the
> > > > > kernel binary. Since this case is not limited to MMU-less systems,
> > > > > let's support it for these which have MMU enabled too.
> > > > >
> > > > > Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.com>
> > > > > ---
> > > > >  arch/riscv/Kconfig   |  1 -
> > > > >  arch/riscv/mm/init.c | 12 ++++++++++--
> > > > >  2 files changed, 10 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > > > index 2b41f6d8e458..9464b4e3a71a 100644
> > > > > --- a/arch/riscv/Kconfig
> > > > > +++ b/arch/riscv/Kconfig
> > > > > @@ -419,7 +419,6 @@ endmenu
> > > > >
> > > > >  config BUILTIN_DTB
> > > > >         def_bool n
> > > > > -       depends on RISCV_M_MODE
> > > > >         depends on OF
> > > > >
> > > > >  menu "Power management options"
> > > > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > > > > index 87c305c566ac..5d1c7a3ec01c 100644
> > > > > --- a/arch/riscv/mm/init.c
> > > > > +++ b/arch/riscv/mm/init.c
> > > > > @@ -194,12 +194,20 @@ void __init setup_bootmem(void)
> > > > >         setup_initrd();
> > > > >  #endif /* CONFIG_BLK_DEV_INITRD */
> > > > >
> > > > > +       /*
> > > > > +        * If DTB is built in, no need to reserve its memblock.
> > > > > +        * OTOH, initial_boot_params has to be set to properly copy DTB
> > > > > +        * before unflattening later on.
> > > > > +        */
> > > > > +       if (IS_ENABLED(CONFIG_BUILTIN_DTB))
> > > > > +               initial_boot_params = __va(dtb_early_pa);
> > > >
> > > > Don't assign initial_boot_params directly here because the
> > > > early_init_dt_scan() will do it.
> > >
> > > early_init_dt_scan will set initial_boot_params to dtb_early_va from
> > > the early mapping which will be gone by the time
> > > unflatten_and_copy_device_tree() is called.
> >
> > That's why we are doing early_init_dt_verify() again for the MMU-enabled
> > case which already takes care of your concern.
>
> I might be out in the woods here but... Do you mean the call to
> early_init_dt_verify() in setup_arch() which is compiled out
> completely in the CONFIG_BUILTIN_DTB case?
> Or is there any other call that I'm overlooking?

Sorry for the confusion, what I meant was that we are calling
early_init_dt_verify() from setup_arch() for the MMU-enabled
with built-in DTB disabled case to update "initial_boot_params"
after the boot CPU has switched from early_pg_dir to swapper_pg_dir.

For MMU-enabled with built-in DTB case, if setup_vm() sets the
dtb_early_va and dtb_early_pa correctly then early_init_dt_scan()
called from setup_arch() will automatically set correct value for
"initial_boot_params".

It is strange that early_init_dt_verify() is being compiled-out for you
because the early_init_dt_scan() called from setup_arch() also uses
early_init_dt_verify(). I quickly compiled the NoMMU kernel for K210
which also uses built-in DTB and I see that early_init_dt_verify()
is not being compiled-out when built-in DTB is enabled.

Regards,
Anup

>
> Best regards,
>    Vitaly
>
> > We use early_init_dt_verify() like most architectures to set the initial DTB.
> >
> > >
> > > > The setup_vm() is supposed to setup dtb_early_va and dtb_early_pa
> > > > for MMU-enabled case so please add a "#ifdef" over there for the
> > > > built-in DTB case.
> > > >
> > > > > +       else
> > > > > +               memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> > > > > +
> > > > >         /*
> > > > >          * Avoid using early_init_fdt_reserve_self() since __pa() does
> > > > >          * not work for DTB pointers that are fixmap addresses
> > > > >          */
> > > >
> > > > This comment needs to be updated and moved along the memblock_reserve()
> > > > statement.
> > > >
> > > > > -       memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> > > > > -
> > > > >         early_init_fdt_scan_reserved_mem();
> > > > >         dma_contiguous_reserve(dma32_phys_limit);
> > > > >         memblock_allow_resize();
> > > > > --
> > > > > 2.29.2
> > > > >
> > > >
> > > > This patch should be based upon Damiens builtin DTB patch.
> > > > Refer, https://www.spinics.net/lists/linux-gpio/msg56616.html
> > >
> > > Thanks for the pointer, however I don't think our patches have
> > > intersections. Besides, Damien is dealing with the MMU-less case
> > > there.
> >
> > Damien's patch is also trying to move to use generic BUILTIN_DTB
> > support for the MMU-less case so it is similar work hence the chance
> > of patch conflict.
> >
> > Regards,
> > Anup

Re: [PATCH] riscv: add BUILTIN_DTB support for MMU-enabled targets

[Anup Patel]

On Mon, Dec 28, 2020 at 10:08 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
>
> On Mon, Dec 28, 2020 at 3:10 PM Anup Patel <anup@brainfault.org> wrote:
> >
> > On Mon, Dec 28, 2020 at 7:05 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
> > >
> > > On Mon, Dec 28, 2020 at 12:59 PM Anup Patel <anup@brainfault.org> wrote:
> > > >
> > > > On Sat, Dec 26, 2020 at 10:03 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
> > > > >
> > > > > Sometimes, especially in a production system we may not want to
> > > > > use a "smart bootloader" like u-boot to load kernel, ramdisk and
> > > > > device tree from a filesystem on eMMC, but rather load the kernel
> > > > > from a NAND partition and just run it as soon as we can, and in
> > > > > this case it is convenient to have device tree compiled into the
> > > > > kernel binary. Since this case is not limited to MMU-less systems,
> > > > > let's support it for these which have MMU enabled too.
> > > > >
> > > > > Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.com>
> > > > > ---
> > > > >  arch/riscv/Kconfig   |  1 -
> > > > >  arch/riscv/mm/init.c | 12 ++++++++++--
> > > > >  2 files changed, 10 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > > > index 2b41f6d8e458..9464b4e3a71a 100644
> > > > > --- a/arch/riscv/Kconfig
> > > > > +++ b/arch/riscv/Kconfig
> > > > > @@ -419,7 +419,6 @@ endmenu
> > > > >
> > > > >  config BUILTIN_DTB
> > > > >         def_bool n
> > > > > -       depends on RISCV_M_MODE
> > > > >         depends on OF
> > > > >
> > > > >  menu "Power management options"
> > > > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > > > > index 87c305c566ac..5d1c7a3ec01c 100644
> > > > > --- a/arch/riscv/mm/init.c
> > > > > +++ b/arch/riscv/mm/init.c
> > > > > @@ -194,12 +194,20 @@ void __init setup_bootmem(void)
> > > > >         setup_initrd();
> > > > >  #endif /* CONFIG_BLK_DEV_INITRD */
> > > > >
> > > > > +       /*
> > > > > +        * If DTB is built in, no need to reserve its memblock.
> > > > > +        * OTOH, initial_boot_params has to be set to properly copy DTB
> > > > > +        * before unflattening later on.
> > > > > +        */
> > > > > +       if (IS_ENABLED(CONFIG_BUILTIN_DTB))
> > > > > +               initial_boot_params = __va(dtb_early_pa);
> > > >
> > > > Don't assign initial_boot_params directly here because the
> > > > early_init_dt_scan() will do it.
> > >
> > > early_init_dt_scan will set initial_boot_params to dtb_early_va from
> > > the early mapping which will be gone by the time
> > > unflatten_and_copy_device_tree() is called.
> >
> > That's why we are doing early_init_dt_verify() again for the MMU-enabled
> > case which already takes care of your concern.
>
> I might be out in the woods here but... Do you mean the call to
> early_init_dt_verify() in setup_arch() which is compiled out
> completely in the CONFIG_BUILTIN_DTB case?
> Or is there any other call that I'm overlooking?

Sorry for the confusion, what I meant was that we are calling
early_init_dt_verify() from setup_arch() for the MMU-enabled
with built-in DTB disabled case to update "initial_boot_params"
after the boot CPU has switched from early_pg_dir to swapper_pg_dir.

For MMU-enabled with built-in DTB case, if setup_vm() sets the
dtb_early_va and dtb_early_pa correctly then early_init_dt_scan()
called from setup_arch() will automatically set correct value for
"initial_boot_params".

It is strange that early_init_dt_verify() is being compiled-out for you
because the early_init_dt_scan() called from setup_arch() also uses
early_init_dt_verify(). I quickly compiled the NoMMU kernel for K210
which also uses built-in DTB and I see that early_init_dt_verify()
is not being compiled-out when built-in DTB is enabled.

Regards,
Anup

>
> Best regards,
>    Vitaly
>
> > We use early_init_dt_verify() like most architectures to set the initial DTB.
> >
> > >
> > > > The setup_vm() is supposed to setup dtb_early_va and dtb_early_pa
> > > > for MMU-enabled case so please add a "#ifdef" over there for the
> > > > built-in DTB case.
> > > >
> > > > > +       else
> > > > > +               memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> > > > > +
> > > > >         /*
> > > > >          * Avoid using early_init_fdt_reserve_self() since __pa() does
> > > > >          * not work for DTB pointers that are fixmap addresses
> > > > >          */
> > > >
> > > > This comment needs to be updated and moved along the memblock_reserve()
> > > > statement.
> > > >
> > > > > -       memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> > > > > -
> > > > >         early_init_fdt_scan_reserved_mem();
> > > > >         dma_contiguous_reserve(dma32_phys_limit);
> > > > >         memblock_allow_resize();
> > > > > --
> > > > > 2.29.2
> > > > >
> > > >
> > > > This patch should be based upon Damiens builtin DTB patch.
> > > > Refer, https://www.spinics.net/lists/linux-gpio/msg56616.html
> > >
> > > Thanks for the pointer, however I don't think our patches have
> > > intersections. Besides, Damien is dealing with the MMU-less case
> > > there.
> >
> > Damien's patch is also trying to move to use generic BUILTIN_DTB
> > support for the MMU-less case so it is similar work hence the chance
> > of patch conflict.
> >
> > Regards,
> > Anup

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] riscv: add BUILTIN_DTB support for MMU-enabled targets

[Vitaly Wool]

On Tue, Dec 29, 2020 at 6:05 AM Anup Patel <anup@brainfault.org> wrote:
>
> On Mon, Dec 28, 2020 at 10:08 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
> >
> > On Mon, Dec 28, 2020 at 3:10 PM Anup Patel <anup@brainfault.org> wrote:
> > >
> > > On Mon, Dec 28, 2020 at 7:05 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
> > > >
> > > > On Mon, Dec 28, 2020 at 12:59 PM Anup Patel <anup@brainfault.org> wrote:
> > > > >
> > > > > On Sat, Dec 26, 2020 at 10:03 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
> > > > > >
> > > > > > Sometimes, especially in a production system we may not want to
> > > > > > use a "smart bootloader" like u-boot to load kernel, ramdisk and
> > > > > > device tree from a filesystem on eMMC, but rather load the kernel
> > > > > > from a NAND partition and just run it as soon as we can, and in
> > > > > > this case it is convenient to have device tree compiled into the
> > > > > > kernel binary. Since this case is not limited to MMU-less systems,
> > > > > > let's support it for these which have MMU enabled too.
> > > > > >
> > > > > > Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.com>
> > > > > > ---
> > > > > >  arch/riscv/Kconfig   |  1 -
> > > > > >  arch/riscv/mm/init.c | 12 ++++++++++--
> > > > > >  2 files changed, 10 insertions(+), 3 deletions(-)
> > > > > >
> > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > > > > index 2b41f6d8e458..9464b4e3a71a 100644
> > > > > > --- a/arch/riscv/Kconfig
> > > > > > +++ b/arch/riscv/Kconfig
> > > > > > @@ -419,7 +419,6 @@ endmenu
> > > > > >
> > > > > >  config BUILTIN_DTB
> > > > > >         def_bool n
> > > > > > -       depends on RISCV_M_MODE
> > > > > >         depends on OF
> > > > > >
> > > > > >  menu "Power management options"
> > > > > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > > > > > index 87c305c566ac..5d1c7a3ec01c 100644
> > > > > > --- a/arch/riscv/mm/init.c
> > > > > > +++ b/arch/riscv/mm/init.c
> > > > > > @@ -194,12 +194,20 @@ void __init setup_bootmem(void)
> > > > > >         setup_initrd();
> > > > > >  #endif /* CONFIG_BLK_DEV_INITRD */
> > > > > >
> > > > > > +       /*
> > > > > > +        * If DTB is built in, no need to reserve its memblock.
> > > > > > +        * OTOH, initial_boot_params has to be set to properly copy DTB
> > > > > > +        * before unflattening later on.
> > > > > > +        */
> > > > > > +       if (IS_ENABLED(CONFIG_BUILTIN_DTB))
> > > > > > +               initial_boot_params = __va(dtb_early_pa);
> > > > >
> > > > > Don't assign initial_boot_params directly here because the
> > > > > early_init_dt_scan() will do it.
> > > >
> > > > early_init_dt_scan will set initial_boot_params to dtb_early_va from
> > > > the early mapping which will be gone by the time
> > > > unflatten_and_copy_device_tree() is called.
> > >
> > > That's why we are doing early_init_dt_verify() again for the MMU-enabled
> > > case which already takes care of your concern.
> >
> > I might be out in the woods here but... Do you mean the call to
> > early_init_dt_verify() in setup_arch() which is compiled out
> > completely in the CONFIG_BUILTIN_DTB case?
> > Or is there any other call that I'm overlooking?
>
> Sorry for the confusion, what I meant was that we are calling
> early_init_dt_verify() from setup_arch() for the MMU-enabled
> with built-in DTB disabled case to update "initial_boot_params"
> after the boot CPU has switched from early_pg_dir to swapper_pg_dir.
>
> For MMU-enabled with built-in DTB case, if setup_vm() sets the
> dtb_early_va and dtb_early_pa correctly then early_init_dt_scan()
> called from setup_arch() will automatically set correct value for
> "initial_boot_params".

Oh I think I get it now. You are suggesting to skip the temporary
mapping for DT altogether since it is anyway in the kernel mapping
range, aren't you?
That does make sense indeed, thanks :)

> It is strange that early_init_dt_verify() is being compiled-out for you
> because the early_init_dt_scan() called from setup_arch() also uses
> early_init_dt_verify(). I quickly compiled the NoMMU kernel for K210
> which also uses built-in DTB and I see that early_init_dt_verify()
> is not being compiled-out when built-in DTB is enabled.

I did not say that early_init_dt_verify() is compiled out, it's the
call to it in setup_arch() that is compiled out if BUILTIN_DTB is
selected. However, if I understand you correctly now, it should not
matter if we don't set dtb_early_va to point to the temporary mapping.

Best regards,
   Vitaly
>
> Regards,
> Anup
>
> >
> > Best regards,
> >    Vitaly
> >
> > > We use early_init_dt_verify() like most architectures to set the initial DTB.
> > >
> > > >
> > > > > The setup_vm() is supposed to setup dtb_early_va and dtb_early_pa
> > > > > for MMU-enabled case so please add a "#ifdef" over there for the
> > > > > built-in DTB case.
> > > > >
> > > > > > +       else
> > > > > > +               memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> > > > > > +
> > > > > >         /*
> > > > > >          * Avoid using early_init_fdt_reserve_self() since __pa() does
> > > > > >          * not work for DTB pointers that are fixmap addresses
> > > > > >          */
> > > > >
> > > > > This comment needs to be updated and moved along the memblock_reserve()
> > > > > statement.
> > > > >
> > > > > > -       memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> > > > > > -
> > > > > >         early_init_fdt_scan_reserved_mem();
> > > > > >         dma_contiguous_reserve(dma32_phys_limit);
> > > > > >         memblock_allow_resize();
> > > > > > --
> > > > > > 2.29.2
> > > > > >
> > > > >
> > > > > This patch should be based upon Damiens builtin DTB patch.
> > > > > Refer, https://www.spinics.net/lists/linux-gpio/msg56616.html
> > > >
> > > > Thanks for the pointer, however I don't think our patches have
> > > > intersections. Besides, Damien is dealing with the MMU-less case
> > > > there.
> > >
> > > Damien's patch is also trying to move to use generic BUILTIN_DTB
> > > support for the MMU-less case so it is similar work hence the chance
> > > of patch conflict.
> > >
> > > Regards,
> > > Anup

Re: [PATCH] riscv: add BUILTIN_DTB support for MMU-enabled targets

[Vitaly Wool]

On Tue, Dec 29, 2020 at 6:05 AM Anup Patel <anup@brainfault.org> wrote:
>
> On Mon, Dec 28, 2020 at 10:08 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
> >
> > On Mon, Dec 28, 2020 at 3:10 PM Anup Patel <anup@brainfault.org> wrote:
> > >
> > > On Mon, Dec 28, 2020 at 7:05 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
> > > >
> > > > On Mon, Dec 28, 2020 at 12:59 PM Anup Patel <anup@brainfault.org> wrote:
> > > > >
> > > > > On Sat, Dec 26, 2020 at 10:03 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
> > > > > >
> > > > > > Sometimes, especially in a production system we may not want to
> > > > > > use a "smart bootloader" like u-boot to load kernel, ramdisk and
> > > > > > device tree from a filesystem on eMMC, but rather load the kernel
> > > > > > from a NAND partition and just run it as soon as we can, and in
> > > > > > this case it is convenient to have device tree compiled into the
> > > > > > kernel binary. Since this case is not limited to MMU-less systems,
> > > > > > let's support it for these which have MMU enabled too.
> > > > > >
> > > > > > Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.com>
> > > > > > ---
> > > > > >  arch/riscv/Kconfig   |  1 -
> > > > > >  arch/riscv/mm/init.c | 12 ++++++++++--
> > > > > >  2 files changed, 10 insertions(+), 3 deletions(-)
> > > > > >
> > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > > > > index 2b41f6d8e458..9464b4e3a71a 100644
> > > > > > --- a/arch/riscv/Kconfig
> > > > > > +++ b/arch/riscv/Kconfig
> > > > > > @@ -419,7 +419,6 @@ endmenu
> > > > > >
> > > > > >  config BUILTIN_DTB
> > > > > >         def_bool n
> > > > > > -       depends on RISCV_M_MODE
> > > > > >         depends on OF
> > > > > >
> > > > > >  menu "Power management options"
> > > > > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > > > > > index 87c305c566ac..5d1c7a3ec01c 100644
> > > > > > --- a/arch/riscv/mm/init.c
> > > > > > +++ b/arch/riscv/mm/init.c
> > > > > > @@ -194,12 +194,20 @@ void __init setup_bootmem(void)
> > > > > >         setup_initrd();
> > > > > >  #endif /* CONFIG_BLK_DEV_INITRD */
> > > > > >
> > > > > > +       /*
> > > > > > +        * If DTB is built in, no need to reserve its memblock.
> > > > > > +        * OTOH, initial_boot_params has to be set to properly copy DTB
> > > > > > +        * before unflattening later on.
> > > > > > +        */
> > > > > > +       if (IS_ENABLED(CONFIG_BUILTIN_DTB))
> > > > > > +               initial_boot_params = __va(dtb_early_pa);
> > > > >
> > > > > Don't assign initial_boot_params directly here because the
> > > > > early_init_dt_scan() will do it.
> > > >
> > > > early_init_dt_scan will set initial_boot_params to dtb_early_va from
> > > > the early mapping which will be gone by the time
> > > > unflatten_and_copy_device_tree() is called.
> > >
> > > That's why we are doing early_init_dt_verify() again for the MMU-enabled
> > > case which already takes care of your concern.
> >
> > I might be out in the woods here but... Do you mean the call to
> > early_init_dt_verify() in setup_arch() which is compiled out
> > completely in the CONFIG_BUILTIN_DTB case?
> > Or is there any other call that I'm overlooking?
>
> Sorry for the confusion, what I meant was that we are calling
> early_init_dt_verify() from setup_arch() for the MMU-enabled
> with built-in DTB disabled case to update "initial_boot_params"
> after the boot CPU has switched from early_pg_dir to swapper_pg_dir.
>
> For MMU-enabled with built-in DTB case, if setup_vm() sets the
> dtb_early_va and dtb_early_pa correctly then early_init_dt_scan()
> called from setup_arch() will automatically set correct value for
> "initial_boot_params".

Oh I think I get it now. You are suggesting to skip the temporary
mapping for DT altogether since it is anyway in the kernel mapping
range, aren't you?
That does make sense indeed, thanks :)

> It is strange that early_init_dt_verify() is being compiled-out for you
> because the early_init_dt_scan() called from setup_arch() also uses
> early_init_dt_verify(). I quickly compiled the NoMMU kernel for K210
> which also uses built-in DTB and I see that early_init_dt_verify()
> is not being compiled-out when built-in DTB is enabled.

I did not say that early_init_dt_verify() is compiled out, it's the
call to it in setup_arch() that is compiled out if BUILTIN_DTB is
selected. However, if I understand you correctly now, it should not
matter if we don't set dtb_early_va to point to the temporary mapping.

Best regards,
   Vitaly
>
> Regards,
> Anup
>
> >
> > Best regards,
> >    Vitaly
> >
> > > We use early_init_dt_verify() like most architectures to set the initial DTB.
> > >
> > > >
> > > > > The setup_vm() is supposed to setup dtb_early_va and dtb_early_pa
> > > > > for MMU-enabled case so please add a "#ifdef" over there for the
> > > > > built-in DTB case.
> > > > >
> > > > > > +       else
> > > > > > +               memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> > > > > > +
> > > > > >         /*
> > > > > >          * Avoid using early_init_fdt_reserve_self() since __pa() does
> > > > > >          * not work for DTB pointers that are fixmap addresses
> > > > > >          */
> > > > >
> > > > > This comment needs to be updated and moved along the memblock_reserve()
> > > > > statement.
> > > > >
> > > > > > -       memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> > > > > > -
> > > > > >         early_init_fdt_scan_reserved_mem();
> > > > > >         dma_contiguous_reserve(dma32_phys_limit);
> > > > > >         memblock_allow_resize();
> > > > > > --
> > > > > > 2.29.2
> > > > > >
> > > > >
> > > > > This patch should be based upon Damiens builtin DTB patch.
> > > > > Refer, https://www.spinics.net/lists/linux-gpio/msg56616.html
> > > >
> > > > Thanks for the pointer, however I don't think our patches have
> > > > intersections. Besides, Damien is dealing with the MMU-less case
> > > > there.
> > >
> > > Damien's patch is also trying to move to use generic BUILTIN_DTB
> > > support for the MMU-less case so it is similar work hence the chance
> > > of patch conflict.
> > >
> > > Regards,
> > > Anup

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] riscv: add BUILTIN_DTB support for MMU-enabled targets

[Anup Patel]

On Thu, Dec 31, 2020 at 3:23 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
>
> On Tue, Dec 29, 2020 at 6:05 AM Anup Patel <anup@brainfault.org> wrote:
> >
> > On Mon, Dec 28, 2020 at 10:08 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
> > >
> > > On Mon, Dec 28, 2020 at 3:10 PM Anup Patel <anup@brainfault.org> wrote:
> > > >
> > > > On Mon, Dec 28, 2020 at 7:05 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
> > > > >
> > > > > On Mon, Dec 28, 2020 at 12:59 PM Anup Patel <anup@brainfault.org> wrote:
> > > > > >
> > > > > > On Sat, Dec 26, 2020 at 10:03 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
> > > > > > >
> > > > > > > Sometimes, especially in a production system we may not want to
> > > > > > > use a "smart bootloader" like u-boot to load kernel, ramdisk and
> > > > > > > device tree from a filesystem on eMMC, but rather load the kernel
> > > > > > > from a NAND partition and just run it as soon as we can, and in
> > > > > > > this case it is convenient to have device tree compiled into the
> > > > > > > kernel binary. Since this case is not limited to MMU-less systems,
> > > > > > > let's support it for these which have MMU enabled too.
> > > > > > >
> > > > > > > Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.com>
> > > > > > > ---
> > > > > > >  arch/riscv/Kconfig   |  1 -
> > > > > > >  arch/riscv/mm/init.c | 12 ++++++++++--
> > > > > > >  2 files changed, 10 insertions(+), 3 deletions(-)
> > > > > > >
> > > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > > > > > index 2b41f6d8e458..9464b4e3a71a 100644
> > > > > > > --- a/arch/riscv/Kconfig
> > > > > > > +++ b/arch/riscv/Kconfig
> > > > > > > @@ -419,7 +419,6 @@ endmenu
> > > > > > >
> > > > > > >  config BUILTIN_DTB
> > > > > > >         def_bool n
> > > > > > > -       depends on RISCV_M_MODE
> > > > > > >         depends on OF
> > > > > > >
> > > > > > >  menu "Power management options"
> > > > > > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > > > > > > index 87c305c566ac..5d1c7a3ec01c 100644
> > > > > > > --- a/arch/riscv/mm/init.c
> > > > > > > +++ b/arch/riscv/mm/init.c
> > > > > > > @@ -194,12 +194,20 @@ void __init setup_bootmem(void)
> > > > > > >         setup_initrd();
> > > > > > >  #endif /* CONFIG_BLK_DEV_INITRD */
> > > > > > >
> > > > > > > +       /*
> > > > > > > +        * If DTB is built in, no need to reserve its memblock.
> > > > > > > +        * OTOH, initial_boot_params has to be set to properly copy DTB
> > > > > > > +        * before unflattening later on.
> > > > > > > +        */
> > > > > > > +       if (IS_ENABLED(CONFIG_BUILTIN_DTB))
> > > > > > > +               initial_boot_params = __va(dtb_early_pa);
> > > > > >
> > > > > > Don't assign initial_boot_params directly here because the
> > > > > > early_init_dt_scan() will do it.
> > > > >
> > > > > early_init_dt_scan will set initial_boot_params to dtb_early_va from
> > > > > the early mapping which will be gone by the time
> > > > > unflatten_and_copy_device_tree() is called.
> > > >
> > > > That's why we are doing early_init_dt_verify() again for the MMU-enabled
> > > > case which already takes care of your concern.
> > >
> > > I might be out in the woods here but... Do you mean the call to
> > > early_init_dt_verify() in setup_arch() which is compiled out
> > > completely in the CONFIG_BUILTIN_DTB case?
> > > Or is there any other call that I'm overlooking?
> >
> > Sorry for the confusion, what I meant was that we are calling
> > early_init_dt_verify() from setup_arch() for the MMU-enabled
> > with built-in DTB disabled case to update "initial_boot_params"
> > after the boot CPU has switched from early_pg_dir to swapper_pg_dir.
> >
> > For MMU-enabled with built-in DTB case, if setup_vm() sets the
> > dtb_early_va and dtb_early_pa correctly then early_init_dt_scan()
> > called from setup_arch() will automatically set correct value for
> > "initial_boot_params".
>
> Oh I think I get it now. You are suggesting to skip the temporary
> mapping for DT altogether since it is anyway in the kernel mapping
> range, aren't you?
> That does make sense indeed, thanks :)

Yes, that's what I am suggesting.

>
> > It is strange that early_init_dt_verify() is being compiled-out for you
> > because the early_init_dt_scan() called from setup_arch() also uses
> > early_init_dt_verify(). I quickly compiled the NoMMU kernel for K210
> > which also uses built-in DTB and I see that early_init_dt_verify()
> > is not being compiled-out when built-in DTB is enabled.
>
> I did not say that early_init_dt_verify() is compiled out, it's the
> call to it in setup_arch() that is compiled out if BUILTIN_DTB is
> selected. However, if I understand you correctly now, it should not
> matter if we don't set dtb_early_va to point to the temporary mapping.

Yes, dtb_early_va can point to some other location such as
built-in DTB.

For MMU-enabled with built-in DTB case, the setup_vm() should
skip the temporary mapping to DT and instead set dtb_early_va
and dtb_early_pa to point to the built-in DTB.

Regards,
Anup

Re: [PATCH] riscv: add BUILTIN_DTB support for MMU-enabled targets

[Anup Patel]

On Thu, Dec 31, 2020 at 3:23 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
>
> On Tue, Dec 29, 2020 at 6:05 AM Anup Patel <anup@brainfault.org> wrote:
> >
> > On Mon, Dec 28, 2020 at 10:08 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
> > >
> > > On Mon, Dec 28, 2020 at 3:10 PM Anup Patel <anup@brainfault.org> wrote:
> > > >
> > > > On Mon, Dec 28, 2020 at 7:05 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
> > > > >
> > > > > On Mon, Dec 28, 2020 at 12:59 PM Anup Patel <anup@brainfault.org> wrote:
> > > > > >
> > > > > > On Sat, Dec 26, 2020 at 10:03 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
> > > > > > >
> > > > > > > Sometimes, especially in a production system we may not want to
> > > > > > > use a "smart bootloader" like u-boot to load kernel, ramdisk and
> > > > > > > device tree from a filesystem on eMMC, but rather load the kernel
> > > > > > > from a NAND partition and just run it as soon as we can, and in
> > > > > > > this case it is convenient to have device tree compiled into the
> > > > > > > kernel binary. Since this case is not limited to MMU-less systems,
> > > > > > > let's support it for these which have MMU enabled too.
> > > > > > >
> > > > > > > Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.com>
> > > > > > > ---
> > > > > > >  arch/riscv/Kconfig   |  1 -
> > > > > > >  arch/riscv/mm/init.c | 12 ++++++++++--
> > > > > > >  2 files changed, 10 insertions(+), 3 deletions(-)
> > > > > > >
> > > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > > > > > index 2b41f6d8e458..9464b4e3a71a 100644
> > > > > > > --- a/arch/riscv/Kconfig
> > > > > > > +++ b/arch/riscv/Kconfig
> > > > > > > @@ -419,7 +419,6 @@ endmenu
> > > > > > >
> > > > > > >  config BUILTIN_DTB
> > > > > > >         def_bool n
> > > > > > > -       depends on RISCV_M_MODE
> > > > > > >         depends on OF
> > > > > > >
> > > > > > >  menu "Power management options"
> > > > > > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > > > > > > index 87c305c566ac..5d1c7a3ec01c 100644
> > > > > > > --- a/arch/riscv/mm/init.c
> > > > > > > +++ b/arch/riscv/mm/init.c
> > > > > > > @@ -194,12 +194,20 @@ void __init setup_bootmem(void)
> > > > > > >         setup_initrd();
> > > > > > >  #endif /* CONFIG_BLK_DEV_INITRD */
> > > > > > >
> > > > > > > +       /*
> > > > > > > +        * If DTB is built in, no need to reserve its memblock.
> > > > > > > +        * OTOH, initial_boot_params has to be set to properly copy DTB
> > > > > > > +        * before unflattening later on.
> > > > > > > +        */
> > > > > > > +       if (IS_ENABLED(CONFIG_BUILTIN_DTB))
> > > > > > > +               initial_boot_params = __va(dtb_early_pa);
> > > > > >
> > > > > > Don't assign initial_boot_params directly here because the
> > > > > > early_init_dt_scan() will do it.
> > > > >
> > > > > early_init_dt_scan will set initial_boot_params to dtb_early_va from
> > > > > the early mapping which will be gone by the time
> > > > > unflatten_and_copy_device_tree() is called.
> > > >
> > > > That's why we are doing early_init_dt_verify() again for the MMU-enabled
> > > > case which already takes care of your concern.
> > >
> > > I might be out in the woods here but... Do you mean the call to
> > > early_init_dt_verify() in setup_arch() which is compiled out
> > > completely in the CONFIG_BUILTIN_DTB case?
> > > Or is there any other call that I'm overlooking?
> >
> > Sorry for the confusion, what I meant was that we are calling
> > early_init_dt_verify() from setup_arch() for the MMU-enabled
> > with built-in DTB disabled case to update "initial_boot_params"
> > after the boot CPU has switched from early_pg_dir to swapper_pg_dir.
> >
> > For MMU-enabled with built-in DTB case, if setup_vm() sets the
> > dtb_early_va and dtb_early_pa correctly then early_init_dt_scan()
> > called from setup_arch() will automatically set correct value for
> > "initial_boot_params".
>
> Oh I think I get it now. You are suggesting to skip the temporary
> mapping for DT altogether since it is anyway in the kernel mapping
> range, aren't you?
> That does make sense indeed, thanks :)

Yes, that's what I am suggesting.

>
> > It is strange that early_init_dt_verify() is being compiled-out for you
> > because the early_init_dt_scan() called from setup_arch() also uses
> > early_init_dt_verify(). I quickly compiled the NoMMU kernel for K210
> > which also uses built-in DTB and I see that early_init_dt_verify()
> > is not being compiled-out when built-in DTB is enabled.
>
> I did not say that early_init_dt_verify() is compiled out, it's the
> call to it in setup_arch() that is compiled out if BUILTIN_DTB is
> selected. However, if I understand you correctly now, it should not
> matter if we don't set dtb_early_va to point to the temporary mapping.

Yes, dtb_early_va can point to some other location such as
built-in DTB.

For MMU-enabled with built-in DTB case, the setup_vm() should
skip the temporary mapping to DT and instead set dtb_early_va
and dtb_early_pa to point to the built-in DTB.

Regards,
Anup

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] riscv: add BUILTIN_DTB support for MMU-enabled targets

[Anup Patel]

On Mon, Dec 28, 2020 at 7:05 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
>
> On Mon, Dec 28, 2020 at 12:59 PM Anup Patel <anup@brainfault.org> wrote:
> >
> > On Sat, Dec 26, 2020 at 10:03 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
> > >
> > > Sometimes, especially in a production system we may not want to
> > > use a "smart bootloader" like u-boot to load kernel, ramdisk and
> > > device tree from a filesystem on eMMC, but rather load the kernel
> > > from a NAND partition and just run it as soon as we can, and in
> > > this case it is convenient to have device tree compiled into the
> > > kernel binary. Since this case is not limited to MMU-less systems,
> > > let's support it for these which have MMU enabled too.
> > >
> > > Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.com>
> > > ---
> > >  arch/riscv/Kconfig   |  1 -
> > >  arch/riscv/mm/init.c | 12 ++++++++++--
> > >  2 files changed, 10 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > index 2b41f6d8e458..9464b4e3a71a 100644
> > > --- a/arch/riscv/Kconfig
> > > +++ b/arch/riscv/Kconfig
> > > @@ -419,7 +419,6 @@ endmenu
> > >
> > >  config BUILTIN_DTB
> > >         def_bool n
> > > -       depends on RISCV_M_MODE
> > >         depends on OF
> > >
> > >  menu "Power management options"
> > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > > index 87c305c566ac..5d1c7a3ec01c 100644
> > > --- a/arch/riscv/mm/init.c
> > > +++ b/arch/riscv/mm/init.c
> > > @@ -194,12 +194,20 @@ void __init setup_bootmem(void)
> > >         setup_initrd();
> > >  #endif /* CONFIG_BLK_DEV_INITRD */
> > >
> > > +       /*
> > > +        * If DTB is built in, no need to reserve its memblock.
> > > +        * OTOH, initial_boot_params has to be set to properly copy DTB
> > > +        * before unflattening later on.
> > > +        */
> > > +       if (IS_ENABLED(CONFIG_BUILTIN_DTB))
> > > +               initial_boot_params = __va(dtb_early_pa);
> >
> > Don't assign initial_boot_params directly here because the
> > early_init_dt_scan() will do it.
>
> early_init_dt_scan will set initial_boot_params to dtb_early_va from
> the early mapping which will be gone by the time
> unflatten_and_copy_device_tree() is called.

The throw-away early DTB mapping in early_pg_dir is anyway
redundant when we have built-in DTB so this patch needs to
add an "#ifdef" around it for the MMU-enabled case. Also, please
update dtb_early_va and dtb_early_pa separately for MMU-enabled
case in the setup_vm()

>
> > The setup_vm() is supposed to setup dtb_early_va and dtb_early_pa
> > for MMU-enabled case so please add a "#ifdef" over there for the
> > built-in DTB case.
> >
> > > +       else
> > > +               memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> > > +
> > >         /*
> > >          * Avoid using early_init_fdt_reserve_self() since __pa() does
> > >          * not work for DTB pointers that are fixmap addresses
> > >          */
> >
> > This comment needs to be updated and moved along the memblock_reserve()
> > statement.
> >
> > > -       memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> > > -
> > >         early_init_fdt_scan_reserved_mem();
> > >         dma_contiguous_reserve(dma32_phys_limit);
> > >         memblock_allow_resize();
> > > --
> > > 2.29.2
> > >
> >
> > This patch should be based upon Damiens builtin DTB patch.
> > Refer, https://www.spinics.net/lists/linux-gpio/msg56616.html
>
> Thanks for the pointer, however I don't think our patches have
> intersections. Besides, Damien is dealing with the MMU-less case
> there.
>
> Best regards,
>    Vitaly

Regards,
Anup

Re: [PATCH] riscv: add BUILTIN_DTB support for MMU-enabled targets

[Anup Patel]

On Mon, Dec 28, 2020 at 7:05 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
>
> On Mon, Dec 28, 2020 at 12:59 PM Anup Patel <anup@brainfault.org> wrote:
> >
> > On Sat, Dec 26, 2020 at 10:03 PM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
> > >
> > > Sometimes, especially in a production system we may not want to
> > > use a "smart bootloader" like u-boot to load kernel, ramdisk and
> > > device tree from a filesystem on eMMC, but rather load the kernel
> > > from a NAND partition and just run it as soon as we can, and in
> > > this case it is convenient to have device tree compiled into the
> > > kernel binary. Since this case is not limited to MMU-less systems,
> > > let's support it for these which have MMU enabled too.
> > >
> > > Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.com>
> > > ---
> > >  arch/riscv/Kconfig   |  1 -
> > >  arch/riscv/mm/init.c | 12 ++++++++++--
> > >  2 files changed, 10 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > index 2b41f6d8e458..9464b4e3a71a 100644
> > > --- a/arch/riscv/Kconfig
> > > +++ b/arch/riscv/Kconfig
> > > @@ -419,7 +419,6 @@ endmenu
> > >
> > >  config BUILTIN_DTB
> > >         def_bool n
> > > -       depends on RISCV_M_MODE
> > >         depends on OF
> > >
> > >  menu "Power management options"
> > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > > index 87c305c566ac..5d1c7a3ec01c 100644
> > > --- a/arch/riscv/mm/init.c
> > > +++ b/arch/riscv/mm/init.c
> > > @@ -194,12 +194,20 @@ void __init setup_bootmem(void)
> > >         setup_initrd();
> > >  #endif /* CONFIG_BLK_DEV_INITRD */
> > >
> > > +       /*
> > > +        * If DTB is built in, no need to reserve its memblock.
> > > +        * OTOH, initial_boot_params has to be set to properly copy DTB
> > > +        * before unflattening later on.
> > > +        */
> > > +       if (IS_ENABLED(CONFIG_BUILTIN_DTB))
> > > +               initial_boot_params = __va(dtb_early_pa);
> >
> > Don't assign initial_boot_params directly here because the
> > early_init_dt_scan() will do it.
>
> early_init_dt_scan will set initial_boot_params to dtb_early_va from
> the early mapping which will be gone by the time
> unflatten_and_copy_device_tree() is called.

The throw-away early DTB mapping in early_pg_dir is anyway
redundant when we have built-in DTB so this patch needs to
add an "#ifdef" around it for the MMU-enabled case. Also, please
update dtb_early_va and dtb_early_pa separately for MMU-enabled
case in the setup_vm()

>
> > The setup_vm() is supposed to setup dtb_early_va and dtb_early_pa
> > for MMU-enabled case so please add a "#ifdef" over there for the
> > built-in DTB case.
> >
> > > +       else
> > > +               memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> > > +
> > >         /*
> > >          * Avoid using early_init_fdt_reserve_self() since __pa() does
> > >          * not work for DTB pointers that are fixmap addresses
> > >          */
> >
> > This comment needs to be updated and moved along the memblock_reserve()
> > statement.
> >
> > > -       memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> > > -
> > >         early_init_fdt_scan_reserved_mem();
> > >         dma_contiguous_reserve(dma32_phys_limit);
> > >         memblock_allow_resize();
> > > --
> > > 2.29.2
> > >
> >
> > This patch should be based upon Damiens builtin DTB patch.
> > Refer, https://www.spinics.net/lists/linux-gpio/msg56616.html
>
> Thanks for the pointer, however I don't think our patches have
> intersections. Besides, Damien is dealing with the MMU-less case
> there.
>
> Best regards,
>    Vitaly

Regards,
Anup

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv