diff mbox series

[v3,1/5] fdt: add support for adding pmem nodes

Message ID 20250120105045.1281262-2-sughosh.ganu@linaro.org
State New
Headers show
Series Add pmem node for preserving distro ISO's | expand

Commit Message

Sughosh Ganu Jan. 20, 2025, 10:50 a.m. UTC 
From: Masahisa Kojima <kojima.masahisa@socionext.com>

One of the problems OS installers face, when running in EFI, is that
the mounted ISO after calling ExitBootServices goes away. For some
distros this is a problem since they rely on finding some core packages
before continuing the installation. Distros have works around this --
e.g Fedora has a special kernel command line parameter called
inst.stage2 [0].

ACPI has NFIT and NVDIMM support to provide ramdisks to the OS, but we
don't have anything in place for DTs. Linux and device trees have support
for persistent memory devices. So add a function that can inject a pmem
node in a DT, so we can use it when launhing OS installers with EFI.

[0]
https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/7/html/installation_guide/chap-anaconda-boot-options#sect-boot-options-installer

Signed-off-by: Masahisa Kojima <kojima.masahisa@socionext.com>
Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---
Changes since V2:
* Fix a checkpatch error by putting a blank line after a function

 boot/fdt_support.c    | 40 +++++++++++++++++++++++++++++++++++++++-
 include/fdt_support.h | 13 +++++++++++++
 2 files changed, 52 insertions(+), 1 deletion(-)

Comments

Heinrich Schuchardt Jan. 20, 2025, 12:31 p.m. UTC | #1 
On 20.01.25 11:50, Sughosh Ganu wrote:
> From: Masahisa Kojima <kojima.masahisa@socionext.com>
>
> One of the problems OS installers face, when running in EFI, is that
> the mounted ISO after calling ExitBootServices goes away. For some
> distros this is a problem since they rely on finding some core packages
> before continuing the installation. Distros have works around this --
> e.g Fedora has a special kernel command line parameter called
> inst.stage2 [0].
>
> ACPI has NFIT and NVDIMM support to provide ramdisks to the OS, but we
> don't have anything in place for DTs. Linux and device trees have support
> for persistent memory devices. So add a function that can inject a pmem
> node in a DT, so we can use it when launhing OS installers with EFI.

We also have ACPI support in U-Boot. In a future patch series I guess we
could add the generation of said ACPI tables.

>
> [0]
> https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/7/html/installation_guide/chap-anaconda-boot-options#sect-boot-options-installer
>
> Signed-off-by: Masahisa Kojima <kojima.masahisa@socionext.com>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
> Changes since V2:
> * Fix a checkpatch error by putting a blank line after a function
>
>   boot/fdt_support.c    | 40 +++++++++++++++++++++++++++++++++++++++-
>   include/fdt_support.h | 13 +++++++++++++
>   2 files changed, 52 insertions(+), 1 deletion(-)
>
> diff --git a/boot/fdt_support.c b/boot/fdt_support.c
> index 49efeec3681..613685b80eb 100644
> --- a/boot/fdt_support.c
> +++ b/boot/fdt_support.c
> @@ -18,6 +18,7 @@
>   #include <dm/ofnode.h>
>   #include <linux/ctype.h>
>   #include <linux/types.h>
> +#include <linux/sizes.h>
>   #include <asm/global_data.h>
>   #include <asm/unaligned.h>
>   #include <linux/libfdt.h>
> @@ -464,7 +465,6 @@ void do_fixup_by_compat_u32(void *fdt, const char *compat,
>   	do_fixup_by_compat(fdt, compat, prop, &tmp, 4, create);
>   }
>
> -#ifdef CONFIG_ARCH_FIXUP_FDT_MEMORY>   /*
>    * fdt_pack_reg - pack address and size array into the "reg"-suitable stream
>    */
> @@ -493,6 +493,7 @@ static int fdt_pack_reg(const void *fdt, void *buf, u64 *address, u64 *size,
>   	return p - (char *)buf;
>   }
>
> +#ifdef CONFIG_ARCH_FIXUP_FDT_MEMORY

Do we need this #ifdef at all? We tend to leave it to the linker to
eliminate functions.

>   #if CONFIG_NR_DRAM_BANKS > 4
>   #define MEMORY_BANKS_MAX CONFIG_NR_DRAM_BANKS
>   #else
> @@ -2222,3 +2223,40 @@ int fdt_valid(struct fdt_header **blobp)
>   	}
>   	return 1;
>   }
> +
> +int fdt_fixup_pmem_region(void *blob, ulong addr, u32 size)
> +{
> +	u64 pmem_start[2] = { 0 };
> +	u64 pmem_size[2] = { 0 };
> +	char pmem_node[32] = {0};
> +	int nodeoffset, len;
> +	int err;
> +	u8 tmp[4 * 16]; /* Up to 64-bit address + 64-bit size */
> +
> +	if (!IS_ALIGNED(addr, SZ_2M) || !IS_ALIGNED(addr + size, SZ_2M)) {
> +		printf("Start and end address needs at 2MB alignment\n");

Nits:

%s/needs at 2MB alignment/must be 2 MiB aligned/

> +		return -1;
> +	}
> +	snprintf(pmem_node, sizeof(pmem_node), "pmem@%lx", addr);
> +	nodeoffset = fdt_find_or_add_subnode(blob, 0, pmem_node);
> +	if (nodeoffset < 0)
> +		return nodeoffset;
> +
> +	err = fdt_setprop_string(blob, nodeoffset, "compatible", "pmem-region");
> +	if (err)
> +		return err;
> +	err = fdt_setprop_empty(blob, nodeoffset, "volatile");
> +	if (err)
> +		return err;
> +	pmem_start[0] = addr;
> +	pmem_size[0] = size;
> +	len = fdt_pack_reg(blob, tmp, pmem_start, pmem_size, 1);
> +	err = fdt_setprop(blob, nodeoffset, "reg", tmp, len);
> +	if (err < 0) {
> +		printf("WARNING: could not set pmem %s %s.\n", "reg",
> +		       fdt_strerror(err));
> +		return err;
> +	}
> +
> +	return 0;
> +}
> diff --git a/include/fdt_support.h b/include/fdt_support.h
> index f0ad2e6b365..aea24df828f 100644
> --- a/include/fdt_support.h
> +++ b/include/fdt_support.h
> @@ -507,4 +507,17 @@ void fdt_fixup_pstore(void *blob);
>    */
>   int fdt_kaslrseed(void *blob, bool overwrite);
>
> +/**
> + * fdt_fixup_pmem_region() - add a pmem node on the device tree
> + *
> + * This functions injects a pmem node to the device tree. Usually
> + * used with EFI installers to preserve installer images

If I understand the code correctly, the function adds or updates a pmem
device-tree node for the given start address.

Otherwise looks good to me.

Best regards

Heinrich

> + *
> + * @blob:	device tree provided by caller
> + * @addr:	start address of the pmem node
> + * @size:	size of the memory of the pmem node
> + * Return:	0 on success or < 0 on failure
> + */
> +int fdt_fixup_pmem_region(void *blob, ulong addr, u32 size);
> +
>   #endif /* ifndef __FDT_SUPPORT_H */
Sughosh Ganu Jan. 20, 2025, 2:02 p.m. UTC | #2 
On Mon, 20 Jan 2025 at 18:01, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 20.01.25 11:50, Sughosh Ganu wrote:
> > From: Masahisa Kojima <kojima.masahisa@socionext.com>
> >
> > One of the problems OS installers face, when running in EFI, is that
> > the mounted ISO after calling ExitBootServices goes away. For some
> > distros this is a problem since they rely on finding some core packages
> > before continuing the installation. Distros have works around this --
> > e.g Fedora has a special kernel command line parameter called
> > inst.stage2 [0].
> >
> > ACPI has NFIT and NVDIMM support to provide ramdisks to the OS, but we
> > don't have anything in place for DTs. Linux and device trees have support
> > for persistent memory devices. So add a function that can inject a pmem
> > node in a DT, so we can use it when launhing OS installers with EFI.
>
> We also have ACPI support in U-Boot. In a future patch series I guess we
> could add the generation of said ACPI tables.
>
> >
> > [0]
> > https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/7/html/installation_guide/chap-anaconda-boot-options#sect-boot-options-installer
> >
> > Signed-off-by: Masahisa Kojima <kojima.masahisa@socionext.com>
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > ---
> > Changes since V2:
> > * Fix a checkpatch error by putting a blank line after a function
> >
> >   boot/fdt_support.c    | 40 +++++++++++++++++++++++++++++++++++++++-
> >   include/fdt_support.h | 13 +++++++++++++
> >   2 files changed, 52 insertions(+), 1 deletion(-)
> >
> > diff --git a/boot/fdt_support.c b/boot/fdt_support.c
> > index 49efeec3681..613685b80eb 100644
> > --- a/boot/fdt_support.c
> > +++ b/boot/fdt_support.c
> > @@ -18,6 +18,7 @@
> >   #include <dm/ofnode.h>
> >   #include <linux/ctype.h>
> >   #include <linux/types.h>
> > +#include <linux/sizes.h>
> >   #include <asm/global_data.h>
> >   #include <asm/unaligned.h>
> >   #include <linux/libfdt.h>
> > @@ -464,7 +465,6 @@ void do_fixup_by_compat_u32(void *fdt, const char *compat,
> >       do_fixup_by_compat(fdt, compat, prop, &tmp, 4, create);
> >   }
> >
> > -#ifdef CONFIG_ARCH_FIXUP_FDT_MEMORY>   /*
> >    * fdt_pack_reg - pack address and size array into the "reg"-suitable stream
> >    */
> > @@ -493,6 +493,7 @@ static int fdt_pack_reg(const void *fdt, void *buf, u64 *address, u64 *size,
> >       return p - (char *)buf;
> >   }
> >
> > +#ifdef CONFIG_ARCH_FIXUP_FDT_MEMORY
>
> Do we need this #ifdef at all? We tend to leave it to the linker to
> eliminate functions.

This patch was authored by Kojima-san. I will have to check if we need
to keep the ifdef.

>
> >   #if CONFIG_NR_DRAM_BANKS > 4
> >   #define MEMORY_BANKS_MAX CONFIG_NR_DRAM_BANKS
> >   #else
> > @@ -2222,3 +2223,40 @@ int fdt_valid(struct fdt_header **blobp)
> >       }
> >       return 1;
> >   }
> > +
> > +int fdt_fixup_pmem_region(void *blob, ulong addr, u32 size)
> > +{
> > +     u64 pmem_start[2] = { 0 };
> > +     u64 pmem_size[2] = { 0 };
> > +     char pmem_node[32] = {0};
> > +     int nodeoffset, len;
> > +     int err;
> > +     u8 tmp[4 * 16]; /* Up to 64-bit address + 64-bit size */
> > +
> > +     if (!IS_ALIGNED(addr, SZ_2M) || !IS_ALIGNED(addr + size, SZ_2M)) {
> > +             printf("Start and end address needs at 2MB alignment\n");
>
> Nits:
>
> %s/needs at 2MB alignment/must be 2 MiB aligned/

Will change

>
> > +             return -1;
> > +     }
> > +     snprintf(pmem_node, sizeof(pmem_node), "pmem@%lx", addr);
> > +     nodeoffset = fdt_find_or_add_subnode(blob, 0, pmem_node);
> > +     if (nodeoffset < 0)
> > +             return nodeoffset;
> > +
> > +     err = fdt_setprop_string(blob, nodeoffset, "compatible", "pmem-region");
> > +     if (err)
> > +             return err;
> > +     err = fdt_setprop_empty(blob, nodeoffset, "volatile");
> > +     if (err)
> > +             return err;
> > +     pmem_start[0] = addr;
> > +     pmem_size[0] = size;
> > +     len = fdt_pack_reg(blob, tmp, pmem_start, pmem_size, 1);
> > +     err = fdt_setprop(blob, nodeoffset, "reg", tmp, len);
> > +     if (err < 0) {
> > +             printf("WARNING: could not set pmem %s %s.\n", "reg",
> > +                    fdt_strerror(err));
> > +             return err;
> > +     }
> > +
> > +     return 0;
> > +}
> > diff --git a/include/fdt_support.h b/include/fdt_support.h
> > index f0ad2e6b365..aea24df828f 100644
> > --- a/include/fdt_support.h
> > +++ b/include/fdt_support.h
> > @@ -507,4 +507,17 @@ void fdt_fixup_pstore(void *blob);
> >    */
> >   int fdt_kaslrseed(void *blob, bool overwrite);
> >
> > +/**
> > + * fdt_fixup_pmem_region() - add a pmem node on the device tree
> > + *
> > + * This functions injects a pmem node to the device tree. Usually
> > + * used with EFI installers to preserve installer images
>
> If I understand the code correctly, the function adds or updates a pmem
> device-tree node for the given start address.

Will reword the comment.

-sughosh

>
> Otherwise looks good to me.
>
> Best regards
>
> Heinrich
>
> > + *
> > + * @blob:    device tree provided by caller
> > + * @addr:    start address of the pmem node
> > + * @size:    size of the memory of the pmem node
> > + * Return:   0 on success or < 0 on failure
> > + */
> > +int fdt_fixup_pmem_region(void *blob, ulong addr, u32 size);
> > +
> >   #endif /* ifndef __FDT_SUPPORT_H */
>
diff mbox series

Patch

diff --git a/boot/fdt_support.c b/boot/fdt_support.c
index 49efeec3681..613685b80eb 100644
--- a/boot/fdt_support.c
+++ b/boot/fdt_support.c
@@ -18,6 +18,7 @@ 
 #include <dm/ofnode.h>
 #include <linux/ctype.h>
 #include <linux/types.h>
+#include <linux/sizes.h>
 #include <asm/global_data.h>
 #include <asm/unaligned.h>
 #include <linux/libfdt.h>
@@ -464,7 +465,6 @@  void do_fixup_by_compat_u32(void *fdt, const char *compat,
 	do_fixup_by_compat(fdt, compat, prop, &tmp, 4, create);
 }
 
-#ifdef CONFIG_ARCH_FIXUP_FDT_MEMORY
 /*
  * fdt_pack_reg - pack address and size array into the "reg"-suitable stream
  */
@@ -493,6 +493,7 @@  static int fdt_pack_reg(const void *fdt, void *buf, u64 *address, u64 *size,
 	return p - (char *)buf;
 }
 
+#ifdef CONFIG_ARCH_FIXUP_FDT_MEMORY
 #if CONFIG_NR_DRAM_BANKS > 4
 #define MEMORY_BANKS_MAX CONFIG_NR_DRAM_BANKS
 #else
@@ -2222,3 +2223,40 @@  int fdt_valid(struct fdt_header **blobp)
 	}
 	return 1;
 }
+
+int fdt_fixup_pmem_region(void *blob, ulong addr, u32 size)
+{
+	u64 pmem_start[2] = { 0 };
+	u64 pmem_size[2] = { 0 };
+	char pmem_node[32] = {0};
+	int nodeoffset, len;
+	int err;
+	u8 tmp[4 * 16]; /* Up to 64-bit address + 64-bit size */
+
+	if (!IS_ALIGNED(addr, SZ_2M) || !IS_ALIGNED(addr + size, SZ_2M)) {
+		printf("Start and end address needs at 2MB alignment\n");
+		return -1;
+	}
+	snprintf(pmem_node, sizeof(pmem_node), "pmem@%lx", addr);
+	nodeoffset = fdt_find_or_add_subnode(blob, 0, pmem_node);
+	if (nodeoffset < 0)
+		return nodeoffset;
+
+	err = fdt_setprop_string(blob, nodeoffset, "compatible", "pmem-region");
+	if (err)
+		return err;
+	err = fdt_setprop_empty(blob, nodeoffset, "volatile");
+	if (err)
+		return err;
+	pmem_start[0] = addr;
+	pmem_size[0] = size;
+	len = fdt_pack_reg(blob, tmp, pmem_start, pmem_size, 1);
+	err = fdt_setprop(blob, nodeoffset, "reg", tmp, len);
+	if (err < 0) {
+		printf("WARNING: could not set pmem %s %s.\n", "reg",
+		       fdt_strerror(err));
+		return err;
+	}
+
+	return 0;
+}
diff --git a/include/fdt_support.h b/include/fdt_support.h
index f0ad2e6b365..aea24df828f 100644
--- a/include/fdt_support.h
+++ b/include/fdt_support.h
@@ -507,4 +507,17 @@  void fdt_fixup_pstore(void *blob);
  */
 int fdt_kaslrseed(void *blob, bool overwrite);
 
+/**
+ * fdt_fixup_pmem_region() - add a pmem node on the device tree
+ *
+ * This functions injects a pmem node to the device tree. Usually
+ * used with EFI installers to preserve installer images
+ *
+ * @blob:	device tree provided by caller
+ * @addr:	start address of the pmem node
+ * @size:	size of the memory of the pmem node
+ * Return:	0 on success or < 0 on failure
+ */
+int fdt_fixup_pmem_region(void *blob, ulong addr, u32 size);
+
 #endif /* ifndef __FDT_SUPPORT_H */