From: "Daniel P. Smith" <dpsmith@apertussolutions.com>
Add support to read the command line from the hyperlauunch device tree.
The device tree command line is located in the "bootargs" property of the
"multiboot,kernel" node.
A boot loader command line, e.g. a grub module string field, takes
precendence ove the device tree one since it is easier to modify.
Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
---
v4:
* Removed __init from header declarations.
* Removed ifdef guards, as DCE ensures the missing definitions don't
matter.
* Removed ifdef guards from new helpers in domain-builder/fdt.c
* Rely on DCE instead.
* Restored checks for cmdline_pa being zero.
* swapped offset and poffset varnames in libfdt-xen.h helper.
* swapped name and pname varnames in libfdt-xen.h helper.
* Turned foo == 0 into !foo in libfdt-xen.h helper.
---
xen/arch/x86/include/asm/bootinfo.h | 6 ++++--
xen/arch/x86/setup.c | 10 +++++++--
xen/common/domain-builder/core.c | 32 +++++++++++++++++++++++++++++
xen/common/domain-builder/fdt.c | 6 ++++++
xen/common/domain-builder/fdt.h | 24 ++++++++++++++++++++++
xen/include/xen/domain-builder.h | 4 ++++
xen/include/xen/libfdt/libfdt-xen.h | 23 +++++++++++++++++++++
7 files changed, 101 insertions(+), 4 deletions(-)
diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
index 1e3d582e45..5b2c93b1ef 100644
--- a/xen/arch/x86/include/asm/bootinfo.h
+++ b/xen/arch/x86/include/asm/bootinfo.h
@@ -35,11 +35,13 @@ struct boot_module {
/*
* Module State Flags:
- * relocated: indicates module has been relocated in memory.
- * released: indicates module's pages have been freed.
+ * relocated: indicates module has been relocated in memory.
+ * released: indicates module's pages have been freed.
+ * fdt_cmdline: indicates module's cmdline is in the FDT.
*/
bool relocated:1;
bool released:1;
+ bool fdt_cmdline:1;
/*
* A boot module may need decompressing by Xen. Headroom is an estimate of
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 4f669f3c60..4cae13163b 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -984,7 +984,10 @@ static size_t __init domain_cmdline_size(const struct boot_info *bi,
{
size_t s = bi->kextra ? strlen(bi->kextra) : 0;
- s += bd->kernel->cmdline_pa ? strlen(__va(bd->kernel->cmdline_pa)) : 0;
+ if ( bd->kernel->fdt_cmdline )
+ s += builder_get_cmdline_size(bi, bd->kernel->cmdline_pa);
+ else if ( bd->kernel->cmdline_pa )
+ s += strlen(__va(bd->kernel->cmdline_pa));
if ( s == 0 )
return s;
@@ -1047,7 +1050,10 @@ static struct domain *__init create_dom0(struct boot_info *bi)
if ( !(cmdline = xzalloc_array(char, cmdline_size)) )
panic("Error allocating cmdline buffer for %pd\n", d);
- if ( bd->kernel->cmdline_pa )
+ if ( bd->kernel->fdt_cmdline )
+ builder_get_cmdline(
+ bi, bd->kernel->cmdline_pa, cmdline, cmdline_size);
+ else if ( bd->kernel->cmdline_pa )
strlcpy(cmdline,
cmdline_cook(__va(bd->kernel->cmdline_pa), bi->loader),
cmdline_size);
diff --git a/xen/common/domain-builder/core.c b/xen/common/domain-builder/core.c
index 924cb495a3..4b4230f2ff 100644
--- a/xen/common/domain-builder/core.c
+++ b/xen/common/domain-builder/core.c
@@ -8,9 +8,41 @@
#include <xen/lib.h>
#include <asm/bootinfo.h>
+#include <asm/setup.h>
#include "fdt.h"
+size_t __init builder_get_cmdline_size(struct boot_info *bi, int offset)
+{
+ const void *fdt;
+ int size;
+
+ if ( !IS_ENABLED(CONFIG_DOMAIN_BUILDER) )
+ return 0;
+
+ fdt = bootstrap_map_bm(&bi->mods[HYPERLAUNCH_MODULE_IDX]);
+ size = fdt_cmdline_prop_size(fdt, offset);
+ bootstrap_unmap();
+
+ return max(0, size);
+}
+
+int __init builder_get_cmdline(
+ struct boot_info *bi, int offset, char *cmdline, size_t size)
+{
+ const void *fdt;
+ int ret;
+
+ if ( !IS_ENABLED(CONFIG_DOMAIN_BUILDER) )
+ return 0;
+
+ fdt = bootstrap_map_bm(&bi->mods[HYPERLAUNCH_MODULE_IDX]);
+ ret = fdt_cmdline_prop_copy(fdt, offset, cmdline, size);
+ bootstrap_unmap();
+
+ return ret;
+}
+
void __init builder_init(struct boot_info *bi)
{
bi->hyperlaunch_enabled = false;
diff --git a/xen/common/domain-builder/fdt.c b/xen/common/domain-builder/fdt.c
index 1fae6add3b..50fde2d007 100644
--- a/xen/common/domain-builder/fdt.c
+++ b/xen/common/domain-builder/fdt.c
@@ -209,6 +209,12 @@ static int __init process_domain_node(
printk(XENLOG_INFO " kernel: multiboot-index=%d\n", idx);
bi->mods[idx].type = BOOTMOD_KERNEL;
bd->kernel = &bi->mods[idx];
+
+ /* If bootloader didn't set cmdline, see if FDT provides one. */
+ if ( bd->kernel->cmdline_pa &&
+ !((char *)__va(bd->kernel->cmdline_pa))[0] )
+ bd->kernel->fdt_cmdline = fdt_get_prop_offset(
+ fdt, node, "bootargs", &bd->kernel->cmdline_pa);
}
}
diff --git a/xen/common/domain-builder/fdt.h b/xen/common/domain-builder/fdt.h
index 8c98a256eb..d2ae7d5c36 100644
--- a/xen/common/domain-builder/fdt.h
+++ b/xen/common/domain-builder/fdt.h
@@ -9,6 +9,30 @@ struct boot_info;
/* hyperlaunch fdt is required to be module 0 */
#define HYPERLAUNCH_MODULE_IDX 0
+static inline int __init fdt_cmdline_prop_size(const void *fdt, int offset)
+{
+ int ret;
+
+ fdt_get_property_by_offset(fdt, offset, &ret);
+
+ return ret;
+}
+
+static inline int __init fdt_cmdline_prop_copy(
+ const void *fdt, int offset, char *cmdline, size_t size)
+{
+ int ret;
+ const struct fdt_property *prop =
+ fdt_get_property_by_offset(fdt, offset, &ret);
+
+ if ( ret < 0 )
+ return ret;
+
+ ASSERT(size > ret);
+
+ return strlcpy(cmdline, prop->data, ret);
+}
+
int has_hyperlaunch_fdt(const struct boot_info *bi);
int walk_hyperlaunch_fdt(struct boot_info *bi);
diff --git a/xen/include/xen/domain-builder.h b/xen/include/xen/domain-builder.h
index ac2b84775d..ac0fd8f60b 100644
--- a/xen/include/xen/domain-builder.h
+++ b/xen/include/xen/domain-builder.h
@@ -4,6 +4,10 @@
struct boot_info;
+size_t builder_get_cmdline_size(const struct boot_info *bi, int offset);
+int builder_get_cmdline(const struct boot_info *bi, int offset,
+ char *cmdline, size_t size);
+
void builder_init(struct boot_info *bi);
#endif /* __XEN_DOMAIN_BUILDER_H__ */
diff --git a/xen/include/xen/libfdt/libfdt-xen.h b/xen/include/xen/libfdt/libfdt-xen.h
index deafb25d98..afc76e7750 100644
--- a/xen/include/xen/libfdt/libfdt-xen.h
+++ b/xen/include/xen/libfdt/libfdt-xen.h
@@ -24,6 +24,29 @@ static inline int __init fdt_prop_as_u32(
return 0;
}
+static inline bool __init fdt_get_prop_offset(
+ const void *fdt, int node, const char *pname, unsigned long *poffset)
+{
+ int ret, offset;
+ const char *name;
+
+ fdt_for_each_property_offset(offset, fdt, node)
+ {
+ fdt_getprop_by_offset(fdt, offset, &name, &ret);
+
+ if ( ret < 0 )
+ continue;
+
+ if ( !strcmp(name, pname) )
+ {
+ *poffset = offset;
+ return true;
+ }
+ }
+
+ return false;
+}
+
static inline int fdt_get_mem_rsv_paddr(const void *fdt, int n,
paddr_t *address,
paddr_t *size)
--
2.43.0On Thu, Apr 17, 2025 at 01:48:29PM +0100, Alejandro Vallejo wrote:
> From: "Daniel P. Smith" <dpsmith@apertussolutions.com>
>
> Add support to read the command line from the hyperlauunch device tree.
> The device tree command line is located in the "bootargs" property of the
> "multiboot,kernel" node.
>
> A boot loader command line, e.g. a grub module string field, takes
> precendence ove the device tree one since it is easier to modify.
^^ over
Also, I would explain the command line parsing rules in the code commentary for
domain_cmdline_size() below.
>
> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
> Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
> ---
> v4:
> * Removed __init from header declarations.
> * Removed ifdef guards, as DCE ensures the missing definitions don't
> matter.
> * Removed ifdef guards from new helpers in domain-builder/fdt.c
> * Rely on DCE instead.
> * Restored checks for cmdline_pa being zero.
> * swapped offset and poffset varnames in libfdt-xen.h helper.
> * swapped name and pname varnames in libfdt-xen.h helper.
> * Turned foo == 0 into !foo in libfdt-xen.h helper.
> ---
> xen/arch/x86/include/asm/bootinfo.h | 6 ++++--
> xen/arch/x86/setup.c | 10 +++++++--
> xen/common/domain-builder/core.c | 32 +++++++++++++++++++++++++++++
> xen/common/domain-builder/fdt.c | 6 ++++++
> xen/common/domain-builder/fdt.h | 24 ++++++++++++++++++++++
> xen/include/xen/domain-builder.h | 4 ++++
> xen/include/xen/libfdt/libfdt-xen.h | 23 +++++++++++++++++++++
> 7 files changed, 101 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
> index 1e3d582e45..5b2c93b1ef 100644
> --- a/xen/arch/x86/include/asm/bootinfo.h
> +++ b/xen/arch/x86/include/asm/bootinfo.h
> @@ -35,11 +35,13 @@ struct boot_module {
>
> /*
> * Module State Flags:
> - * relocated: indicates module has been relocated in memory.
> - * released: indicates module's pages have been freed.
> + * relocated: indicates module has been relocated in memory.
> + * released: indicates module's pages have been freed.
> + * fdt_cmdline: indicates module's cmdline is in the FDT.
> */
> bool relocated:1;
> bool released:1;
> + bool fdt_cmdline:1;
>
> /*
> * A boot module may need decompressing by Xen. Headroom is an estimate of
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 4f669f3c60..4cae13163b 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -984,7 +984,10 @@ static size_t __init domain_cmdline_size(const struct boot_info *bi,
> {
> size_t s = bi->kextra ? strlen(bi->kextra) : 0;
>
> - s += bd->kernel->cmdline_pa ? strlen(__va(bd->kernel->cmdline_pa)) : 0;
> + if ( bd->kernel->fdt_cmdline )
> + s += builder_get_cmdline_size(bi, bd->kernel->cmdline_pa);
> + else if ( bd->kernel->cmdline_pa )
> + s += strlen(__va(bd->kernel->cmdline_pa));
>
> if ( s == 0 )
> return s;
> @@ -1047,7 +1050,10 @@ static struct domain *__init create_dom0(struct boot_info *bi)
> if ( !(cmdline = xzalloc_array(char, cmdline_size)) )
> panic("Error allocating cmdline buffer for %pd\n", d);
>
> - if ( bd->kernel->cmdline_pa )
> + if ( bd->kernel->fdt_cmdline )
> + builder_get_cmdline(
> + bi, bd->kernel->cmdline_pa, cmdline, cmdline_size);
> + else if ( bd->kernel->cmdline_pa )
> strlcpy(cmdline,
> cmdline_cook(__va(bd->kernel->cmdline_pa), bi->loader),
> cmdline_size);
> diff --git a/xen/common/domain-builder/core.c b/xen/common/domain-builder/core.c
> index 924cb495a3..4b4230f2ff 100644
> --- a/xen/common/domain-builder/core.c
> +++ b/xen/common/domain-builder/core.c
> @@ -8,9 +8,41 @@
> #include <xen/lib.h>
>
> #include <asm/bootinfo.h>
> +#include <asm/setup.h>
>
> #include "fdt.h"
>
> +size_t __init builder_get_cmdline_size(struct boot_info *bi, int offset)
> +{
> + const void *fdt;
> + int size;
> +
> + if ( !IS_ENABLED(CONFIG_DOMAIN_BUILDER) )
> + return 0;
> +
> + fdt = bootstrap_map_bm(&bi->mods[HYPERLAUNCH_MODULE_IDX]);
> + size = fdt_cmdline_prop_size(fdt, offset);
> + bootstrap_unmap();
> +
> + return max(0, size);
> +}
> +
> +int __init builder_get_cmdline(
> + struct boot_info *bi, int offset, char *cmdline, size_t size)
> +{
> + const void *fdt;
> + int ret;
> +
> + if ( !IS_ENABLED(CONFIG_DOMAIN_BUILDER) )
> + return 0;
> +
> + fdt = bootstrap_map_bm(&bi->mods[HYPERLAUNCH_MODULE_IDX]);
> + ret = fdt_cmdline_prop_copy(fdt, offset, cmdline, size);
> + bootstrap_unmap();
> +
> + return ret;
> +}
> +
> void __init builder_init(struct boot_info *bi)
> {
> bi->hyperlaunch_enabled = false;
> diff --git a/xen/common/domain-builder/fdt.c b/xen/common/domain-builder/fdt.c
> index 1fae6add3b..50fde2d007 100644
> --- a/xen/common/domain-builder/fdt.c
> +++ b/xen/common/domain-builder/fdt.c
> @@ -209,6 +209,12 @@ static int __init process_domain_node(
> printk(XENLOG_INFO " kernel: multiboot-index=%d\n", idx);
> bi->mods[idx].type = BOOTMOD_KERNEL;
> bd->kernel = &bi->mods[idx];
> +
> + /* If bootloader didn't set cmdline, see if FDT provides one. */
> + if ( bd->kernel->cmdline_pa &&
> + !((char *)__va(bd->kernel->cmdline_pa))[0] )
> + bd->kernel->fdt_cmdline = fdt_get_prop_offset(
> + fdt, node, "bootargs", &bd->kernel->cmdline_pa);
> }
> }
>
> diff --git a/xen/common/domain-builder/fdt.h b/xen/common/domain-builder/fdt.h
> index 8c98a256eb..d2ae7d5c36 100644
> --- a/xen/common/domain-builder/fdt.h
> +++ b/xen/common/domain-builder/fdt.h
> @@ -9,6 +9,30 @@ struct boot_info;
> /* hyperlaunch fdt is required to be module 0 */
> #define HYPERLAUNCH_MODULE_IDX 0
>
> +static inline int __init fdt_cmdline_prop_size(const void *fdt, int offset)
> +{
> + int ret;
> +
> + fdt_get_property_by_offset(fdt, offset, &ret);
Perhaps something like
(void)fdt_get_property_by_offset...
since there's no need to check the return value?
> +
> + return ret;
> +}
> +
> +static inline int __init fdt_cmdline_prop_copy(
> + const void *fdt, int offset, char *cmdline, size_t size)
> +{
> + int ret;
> + const struct fdt_property *prop =
> + fdt_get_property_by_offset(fdt, offset, &ret);
> +
> + if ( ret < 0 )
> + return ret;
> +
> + ASSERT(size > ret);
> +
> + return strlcpy(cmdline, prop->data, ret);
> +}
> +
> int has_hyperlaunch_fdt(const struct boot_info *bi);
> int walk_hyperlaunch_fdt(struct boot_info *bi);
>
> diff --git a/xen/include/xen/domain-builder.h b/xen/include/xen/domain-builder.h
> index ac2b84775d..ac0fd8f60b 100644
> --- a/xen/include/xen/domain-builder.h
> +++ b/xen/include/xen/domain-builder.h
> @@ -4,6 +4,10 @@
>
> struct boot_info;
>
> +size_t builder_get_cmdline_size(const struct boot_info *bi, int offset);
> +int builder_get_cmdline(const struct boot_info *bi, int offset,
> + char *cmdline, size_t size);
> +
> void builder_init(struct boot_info *bi);
>
> #endif /* __XEN_DOMAIN_BUILDER_H__ */
> diff --git a/xen/include/xen/libfdt/libfdt-xen.h b/xen/include/xen/libfdt/libfdt-xen.h
> index deafb25d98..afc76e7750 100644
> --- a/xen/include/xen/libfdt/libfdt-xen.h
> +++ b/xen/include/xen/libfdt/libfdt-xen.h
> @@ -24,6 +24,29 @@ static inline int __init fdt_prop_as_u32(
> return 0;
> }
>
> +static inline bool __init fdt_get_prop_offset(
> + const void *fdt, int node, const char *pname, unsigned long *poffset)
> +{
> + int ret, offset;
> + const char *name;
> +
> + fdt_for_each_property_offset(offset, fdt, node)
> + {
> + fdt_getprop_by_offset(fdt, offset, &name, &ret);
Add
(void)fdt_get_property_by_offset...
here as well?
> +
> + if ( ret < 0 )
> + continue;
> +
> + if ( !strcmp(name, pname) )
> + {
> + *poffset = offset;
> + return true;
> + }
> + }
> +
> + return false;
> +}
> +
> static inline int fdt_get_mem_rsv_paddr(const void *fdt, int n,
> paddr_t *address,
> paddr_t *size)
> --
> 2.43.0
>
>
On Fri Apr 18, 2025 at 11:53 PM BST, dmkhn wrote:
> On Thu, Apr 17, 2025 at 01:48:29PM +0100, Alejandro Vallejo wrote:
>> From: "Daniel P. Smith" <dpsmith@apertussolutions.com>
>>
>> Add support to read the command line from the hyperlauunch device tree.
>> The device tree command line is located in the "bootargs" property of the
>> "multiboot,kernel" node.
>>
>> A boot loader command line, e.g. a grub module string field, takes
>> precendence ove the device tree one since it is easier to modify.
>
> ^^ over
Oops. yes.
>
> Also, I would explain the command line parsing rules in the code commentary for
> domain_cmdline_size() below.
As in, also state the contents of the commit message in the function
header?
>
>>
>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
>> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
>> Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
>> ---
>> v4:
>> * Removed __init from header declarations.
>> * Removed ifdef guards, as DCE ensures the missing definitions don't
>> matter.
>> * Removed ifdef guards from new helpers in domain-builder/fdt.c
>> * Rely on DCE instead.
>> * Restored checks for cmdline_pa being zero.
>> * swapped offset and poffset varnames in libfdt-xen.h helper.
>> * swapped name and pname varnames in libfdt-xen.h helper.
>> * Turned foo == 0 into !foo in libfdt-xen.h helper.
>> ---
>> xen/arch/x86/include/asm/bootinfo.h | 6 ++++--
>> xen/arch/x86/setup.c | 10 +++++++--
>> xen/common/domain-builder/core.c | 32 +++++++++++++++++++++++++++++
>> xen/common/domain-builder/fdt.c | 6 ++++++
>> xen/common/domain-builder/fdt.h | 24 ++++++++++++++++++++++
>> xen/include/xen/domain-builder.h | 4 ++++
>> xen/include/xen/libfdt/libfdt-xen.h | 23 +++++++++++++++++++++
>> 7 files changed, 101 insertions(+), 4 deletions(-)
>>
>> diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
>> index 1e3d582e45..5b2c93b1ef 100644
>> --- a/xen/arch/x86/include/asm/bootinfo.h
>> +++ b/xen/arch/x86/include/asm/bootinfo.h
>> @@ -35,11 +35,13 @@ struct boot_module {
>>
>> /*
>> * Module State Flags:
>> - * relocated: indicates module has been relocated in memory.
>> - * released: indicates module's pages have been freed.
>> + * relocated: indicates module has been relocated in memory.
>> + * released: indicates module's pages have been freed.
>> + * fdt_cmdline: indicates module's cmdline is in the FDT.
>> */
>> bool relocated:1;
>> bool released:1;
>> + bool fdt_cmdline:1;
>>
>> /*
>> * A boot module may need decompressing by Xen. Headroom is an estimate of
>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
>> index 4f669f3c60..4cae13163b 100644
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -984,7 +984,10 @@ static size_t __init domain_cmdline_size(const struct boot_info *bi,
>> {
>> size_t s = bi->kextra ? strlen(bi->kextra) : 0;
>>
>> - s += bd->kernel->cmdline_pa ? strlen(__va(bd->kernel->cmdline_pa)) : 0;
>> + if ( bd->kernel->fdt_cmdline )
>> + s += builder_get_cmdline_size(bi, bd->kernel->cmdline_pa);
>> + else if ( bd->kernel->cmdline_pa )
>> + s += strlen(__va(bd->kernel->cmdline_pa));
>>
>> if ( s == 0 )
>> return s;
>> @@ -1047,7 +1050,10 @@ static struct domain *__init create_dom0(struct boot_info *bi)
>> if ( !(cmdline = xzalloc_array(char, cmdline_size)) )
>> panic("Error allocating cmdline buffer for %pd\n", d);
>>
>> - if ( bd->kernel->cmdline_pa )
>> + if ( bd->kernel->fdt_cmdline )
>> + builder_get_cmdline(
>> + bi, bd->kernel->cmdline_pa, cmdline, cmdline_size);
>> + else if ( bd->kernel->cmdline_pa )
>> strlcpy(cmdline,
>> cmdline_cook(__va(bd->kernel->cmdline_pa), bi->loader),
>> cmdline_size);
>> diff --git a/xen/common/domain-builder/core.c b/xen/common/domain-builder/core.c
>> index 924cb495a3..4b4230f2ff 100644
>> --- a/xen/common/domain-builder/core.c
>> +++ b/xen/common/domain-builder/core.c
>> @@ -8,9 +8,41 @@
>> #include <xen/lib.h>
>>
>> #include <asm/bootinfo.h>
>> +#include <asm/setup.h>
>>
>> #include "fdt.h"
>>
>> +size_t __init builder_get_cmdline_size(struct boot_info *bi, int offset)
>> +{
>> + const void *fdt;
>> + int size;
>> +
>> + if ( !IS_ENABLED(CONFIG_DOMAIN_BUILDER) )
>> + return 0;
>> +
>> + fdt = bootstrap_map_bm(&bi->mods[HYPERLAUNCH_MODULE_IDX]);
>> + size = fdt_cmdline_prop_size(fdt, offset);
>> + bootstrap_unmap();
>> +
>> + return max(0, size);
>> +}
>> +
>> +int __init builder_get_cmdline(
>> + struct boot_info *bi, int offset, char *cmdline, size_t size)
>> +{
>> + const void *fdt;
>> + int ret;
>> +
>> + if ( !IS_ENABLED(CONFIG_DOMAIN_BUILDER) )
>> + return 0;
>> +
>> + fdt = bootstrap_map_bm(&bi->mods[HYPERLAUNCH_MODULE_IDX]);
>> + ret = fdt_cmdline_prop_copy(fdt, offset, cmdline, size);
>> + bootstrap_unmap();
>> +
>> + return ret;
>> +}
>> +
>> void __init builder_init(struct boot_info *bi)
>> {
>> bi->hyperlaunch_enabled = false;
>> diff --git a/xen/common/domain-builder/fdt.c b/xen/common/domain-builder/fdt.c
>> index 1fae6add3b..50fde2d007 100644
>> --- a/xen/common/domain-builder/fdt.c
>> +++ b/xen/common/domain-builder/fdt.c
>> @@ -209,6 +209,12 @@ static int __init process_domain_node(
>> printk(XENLOG_INFO " kernel: multiboot-index=%d\n", idx);
>> bi->mods[idx].type = BOOTMOD_KERNEL;
>> bd->kernel = &bi->mods[idx];
>> +
>> + /* If bootloader didn't set cmdline, see if FDT provides one. */
>> + if ( bd->kernel->cmdline_pa &&
>> + !((char *)__va(bd->kernel->cmdline_pa))[0] )
>> + bd->kernel->fdt_cmdline = fdt_get_prop_offset(
>> + fdt, node, "bootargs", &bd->kernel->cmdline_pa);
>> }
>> }
>>
>> diff --git a/xen/common/domain-builder/fdt.h b/xen/common/domain-builder/fdt.h
>> index 8c98a256eb..d2ae7d5c36 100644
>> --- a/xen/common/domain-builder/fdt.h
>> +++ b/xen/common/domain-builder/fdt.h
>> @@ -9,6 +9,30 @@ struct boot_info;
>> /* hyperlaunch fdt is required to be module 0 */
>> #define HYPERLAUNCH_MODULE_IDX 0
>>
>> +static inline int __init fdt_cmdline_prop_size(const void *fdt, int offset)
>> +{
>> + int ret;
>> +
>> + fdt_get_property_by_offset(fdt, offset, &ret);
>
> Perhaps something like
>
> (void)fdt_get_property_by_offset...
>
> since there's no need to check the return value?
I vaguely seem to remember doing something like that a few years ago
(because it does show intent and many linters require it) and being told
not to. But maybe I misremember. I'm definitely happy to use that
convention here and later unless someone pushes back for some reason.
Cheers,
Alejandro
On 23.04.2025 15:01, Alejandro Vallejo wrote:
> On Fri Apr 18, 2025 at 11:53 PM BST, dmkhn wrote:
>> On Thu, Apr 17, 2025 at 01:48:29PM +0100, Alejandro Vallejo wrote:
>>> --- a/xen/common/domain-builder/fdt.h
>>> +++ b/xen/common/domain-builder/fdt.h
>>> @@ -9,6 +9,30 @@ struct boot_info;
>>> /* hyperlaunch fdt is required to be module 0 */
>>> #define HYPERLAUNCH_MODULE_IDX 0
>>>
>>> +static inline int __init fdt_cmdline_prop_size(const void *fdt, int offset)
>>> +{
>>> + int ret;
>>> +
>>> + fdt_get_property_by_offset(fdt, offset, &ret);
>>
>> Perhaps something like
>>
>> (void)fdt_get_property_by_offset...
>>
>> since there's no need to check the return value?
>
> I vaguely seem to remember doing something like that a few years ago
> (because it does show intent and many linters require it) and being told
> not to. But maybe I misremember. I'm definitely happy to use that
> convention here and later unless someone pushes back for some reason.
Unless we settle on the need for such for Misra's sake, I'd like to ask
to avoid them. We generally try to avoid casts as much as possible. We
then also shouldn't add ones like suggested here.
Jan
On Wed, Apr 23, 2025 at 03:08:18PM +0200, Jan Beulich wrote:
> On 23.04.2025 15:01, Alejandro Vallejo wrote:
> > On Fri Apr 18, 2025 at 11:53 PM BST, dmkhn wrote:
> >> On Thu, Apr 17, 2025 at 01:48:29PM +0100, Alejandro Vallejo wrote:
> >>> --- a/xen/common/domain-builder/fdt.h
> >>> +++ b/xen/common/domain-builder/fdt.h
> >>> @@ -9,6 +9,30 @@ struct boot_info;
> >>> /* hyperlaunch fdt is required to be module 0 */
> >>> #define HYPERLAUNCH_MODULE_IDX 0
> >>>
> >>> +static inline int __init fdt_cmdline_prop_size(const void *fdt, int offset)
> >>> +{
> >>> + int ret;
> >>> +
> >>> + fdt_get_property_by_offset(fdt, offset, &ret);
> >>
> >> Perhaps something like
> >>
> >> (void)fdt_get_property_by_offset...
> >>
> >> since there's no need to check the return value?
> >
> > I vaguely seem to remember doing something like that a few years ago
> > (because it does show intent and many linters require it) and being told
> > not to. But maybe I misremember. I'm definitely happy to use that
> > convention here and later unless someone pushes back for some reason.
>
> Unless we settle on the need for such for Misra's sake, I'd like to ask
> to avoid them. We generally try to avoid casts as much as possible. We
> then also shouldn't add ones like suggested here.
Ack.
>
> Jan
© 2016 - 2025 Red Hat, Inc.