diff mbox series

[v5,28/44] x86/boot: add cmdline to struct boot_module

Message ID 20241006214956.24339-29-dpsmith@apertussolutions.com (mailing list archive)
State Superseded
Headers show
Series Boot modules for Hyperlaunch | expand

Commit Message

Daniel P. Smith Oct. 6, 2024, 9:49 p.m. UTC
Add a char pointer field, cmdline, to struct boot_module to hold the address
pointed to by the string field of struct mod. This removes the need to use the
early_mod field to get to the dom0 kernel command line.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/include/asm/bootinfo.h | 2 ++
 xen/arch/x86/setup.c                | 9 ++++++---
 2 files changed, 8 insertions(+), 3 deletions(-)

Comments

Jason Andryuk Oct. 8, 2024, 5:08 p.m. UTC | #1
On 2024-10-06 17:49, Daniel P. Smith wrote:
> Add a char pointer field, cmdline, to struct boot_module to hold the address
> pointed to by the string field of struct mod. This removes the need to use the
> early_mod field to get to the dom0 kernel command line.
> 
> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> ---
>   xen/arch/x86/include/asm/bootinfo.h | 2 ++
>   xen/arch/x86/setup.c                | 9 ++++++---
>   2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
> index 9ed260629012..3b6bfbe88770 100644
> --- a/xen/arch/x86/include/asm/bootinfo.h
> +++ b/xen/arch/x86/include/asm/bootinfo.h
> @@ -38,6 +38,8 @@ struct boot_module {
>   #define BOOTMOD_FLAG_X86_RELOCATED     (1U << 0)
>   #define BOOTMOD_FLAG_X86_CONSUMED      (1U << 1)
>   
> +    const char *cmdline;
> +
>       paddr_t start;
>       size_t size;
>   };
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index aed0837902c4..d5916e85f68e 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -312,6 +312,8 @@ static struct boot_info __init *multiboot_fill_boot_info(unsigned long mbi_p)
>       {
>           bi->mods[i].mod = &mods[i];
>   
> +        bi->mods[i].cmdline = (char *)(paddr_t)mods[i].string;
> +
>           bi->mods[i].start = (paddr_t)mods[i].mod_start;
>           bi->mods[i].size = mods[i].mod_end - mods[i].mod_start;
>       }
> @@ -1000,10 +1002,11 @@ static struct domain *__init create_dom0(struct boot_info *bi)
>           panic("Error creating d%uv0\n", domid);
>   
>       /* Grab the DOM0 command line. */
> -    if ( image->mod->string || bi->kextra )
> +    if ( image->cmdline || bi->kextra )
>       {
> -        if ( image->mod->string )
> -            safe_strcpy(cmdline, cmdline_cook(__va(image->mod->string),
> +        if ( image->cmdline )
> +            safe_strcpy(cmdline,
> +                        cmdline_cook(__va((unsigned long)image->cmdline),

char * seems inappropriate if cmdline isn't usable as a string.  Maybe 
have cmdline as a paddr_t, or can __va() be used at assignment time?

Regards,
Jason

>                           bi->loader));
>   
>           if ( bi->kextra )
Daniel P. Smith Oct. 9, 2024, 11:09 p.m. UTC | #2
On 10/8/24 13:08, Jason Andryuk wrote:
> On 2024-10-06 17:49, Daniel P. Smith wrote:
>> Add a char pointer field, cmdline, to struct boot_module to hold the 
>> address
>> pointed to by the string field of struct mod. This removes the need to 
>> use the
>> early_mod field to get to the dom0 kernel command line.
>>
>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
>> ---
>>   xen/arch/x86/include/asm/bootinfo.h | 2 ++
>>   xen/arch/x86/setup.c                | 9 ++++++---
>>   2 files changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/arch/x86/include/asm/bootinfo.h 
>> b/xen/arch/x86/include/asm/bootinfo.h
>> index 9ed260629012..3b6bfbe88770 100644
>> --- a/xen/arch/x86/include/asm/bootinfo.h
>> +++ b/xen/arch/x86/include/asm/bootinfo.h
>> @@ -38,6 +38,8 @@ struct boot_module {
>>   #define BOOTMOD_FLAG_X86_RELOCATED     (1U << 0)
>>   #define BOOTMOD_FLAG_X86_CONSUMED      (1U << 1)
>> +    const char *cmdline;
>> +
>>       paddr_t start;
>>       size_t size;
>>   };
>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
>> index aed0837902c4..d5916e85f68e 100644
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -312,6 +312,8 @@ static struct boot_info __init 
>> *multiboot_fill_boot_info(unsigned long mbi_p)
>>       {
>>           bi->mods[i].mod = &mods[i];
>> +        bi->mods[i].cmdline = (char *)(paddr_t)mods[i].string;
>> +
>>           bi->mods[i].start = (paddr_t)mods[i].mod_start;
>>           bi->mods[i].size = mods[i].mod_end - mods[i].mod_start;
>>       }
>> @@ -1000,10 +1002,11 @@ static struct domain *__init 
>> create_dom0(struct boot_info *bi)
>>           panic("Error creating d%uv0\n", domid);
>>       /* Grab the DOM0 command line. */
>> -    if ( image->mod->string || bi->kextra )
>> +    if ( image->cmdline || bi->kextra )
>>       {
>> -        if ( image->mod->string )
>> -            safe_strcpy(cmdline, cmdline_cook(__va(image->mod->string),
>> +        if ( image->cmdline )
>> +            safe_strcpy(cmdline,
>> +                        cmdline_cook(__va((unsigned 
>> long)image->cmdline),
> 
> char * seems inappropriate if cmdline isn't usable as a string.  Maybe 
> have cmdline as a paddr_t, or can __va() be used at assignment time?
> 

Yah, char * was not the correct choice, paddr_t is more appropriate.

v/r,
dps
diff mbox series

Patch

diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
index 9ed260629012..3b6bfbe88770 100644
--- a/xen/arch/x86/include/asm/bootinfo.h
+++ b/xen/arch/x86/include/asm/bootinfo.h
@@ -38,6 +38,8 @@  struct boot_module {
 #define BOOTMOD_FLAG_X86_RELOCATED     (1U << 0)
 #define BOOTMOD_FLAG_X86_CONSUMED      (1U << 1)
 
+    const char *cmdline;
+
     paddr_t start;
     size_t size;
 };
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index aed0837902c4..d5916e85f68e 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -312,6 +312,8 @@  static struct boot_info __init *multiboot_fill_boot_info(unsigned long mbi_p)
     {
         bi->mods[i].mod = &mods[i];
 
+        bi->mods[i].cmdline = (char *)(paddr_t)mods[i].string;
+
         bi->mods[i].start = (paddr_t)mods[i].mod_start;
         bi->mods[i].size = mods[i].mod_end - mods[i].mod_start;
     }
@@ -1000,10 +1002,11 @@  static struct domain *__init create_dom0(struct boot_info *bi)
         panic("Error creating d%uv0\n", domid);
 
     /* Grab the DOM0 command line. */
-    if ( image->mod->string || bi->kextra )
+    if ( image->cmdline || bi->kextra )
     {
-        if ( image->mod->string )
-            safe_strcpy(cmdline, cmdline_cook(__va(image->mod->string),
+        if ( image->cmdline )
+            safe_strcpy(cmdline,
+                        cmdline_cook(__va((unsigned long)image->cmdline),
                         bi->loader));
 
         if ( bi->kextra )