diff mbox series

[v5,05/44] x86/boot: introduce struct boot_module

Message ID 20241006214956.24339-6-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
This will introduce a new struct boot_module to provide a rich state
representation around modules provided by the boot loader. Support is for 64
boot modules, one held in reserve for Xen, and up to 63 can be provided by the
boot loader. The array of struct boot_modules will be accessible via a
reference held in struct boot_info.

A temporary `mod` parameter is included in struct boot_module to ease the
transition from using Multiboot v1 structures over to struct boot_module. Once
the transition is complete, the parameter will be dropped from the structure.

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

Comments

Jason Andryuk Oct. 7, 2024, 6:29 p.m. UTC | #1
On 2024-10-06 17:49, Daniel P. Smith wrote:
> This will introduce a new struct boot_module to provide a rich state
> representation around modules provided by the boot loader. Support is for 64
> boot modules, one held in reserve for Xen, and up to 63 can be provided by the
> boot loader. The array of struct boot_modules will be accessible via a
> reference held in struct boot_info.
> 
> A temporary `mod` parameter is included in struct boot_module to ease the
> transition from using Multiboot v1 structures over to struct boot_module. Once
> the transition is complete, the parameter will be dropped from the structure.
> 
> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> ---
>   xen/arch/x86/include/asm/bootinfo.h | 14 ++++++++++++--
>   xen/arch/x86/setup.c                |  9 +++++++++
>   2 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
> index 87d311ac1399..d19473d8941e 100644
> --- a/xen/arch/x86/include/asm/bootinfo.h
> +++ b/xen/arch/x86/include/asm/bootinfo.h
> @@ -8,20 +8,30 @@
>   #ifndef __XEN_X86_BOOTINFO_H__
>   #define __XEN_X86_BOOTINFO_H__
>   
> +#include <xen/multiboot.h>
>   #include <xen/types.h>
>   
> +/* Max number of boot modules a bootloader can provide in addition to Xen */
> +#define MAX_NR_BOOTMODS 63
> +
> +struct boot_module {
> +    /* Transitionary only */
> +    module_t *mod;
> +};
> +
>   /*
>    * Xen internal representation of information provided by the
>    * bootloader/environment, or derived from the information.
>    */
>   struct boot_info {
> -    unsigned int nr_modules;
> -

(You should probably re-work the other patches to insert ahead of this 
and avoid the movement.)

>       const char *loader;
>       const char *cmdline;
>   
>       paddr_t memmap_addr;
>       size_t memmap_length;
> +
> +    unsigned int nr_modules;
> +    struct boot_module mods[MAX_NR_BOOTMODS + 1];
>   };

Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Daniel P. Smith Oct. 9, 2024, 11:31 a.m. UTC | #2
On 10/7/24 14:29, Jason Andryuk wrote:
> On 2024-10-06 17:49, Daniel P. Smith wrote:
>> This will introduce a new struct boot_module to provide a rich state
>> representation around modules provided by the boot loader. Support is 
>> for 64
>> boot modules, one held in reserve for Xen, and up to 63 can be 
>> provided by the
>> boot loader. The array of struct boot_modules will be accessible via a
>> reference held in struct boot_info.
>>
>> A temporary `mod` parameter is included in struct boot_module to ease the
>> transition from using Multiboot v1 structures over to struct 
>> boot_module. Once
>> the transition is complete, the parameter will be dropped from the 
>> structure.
>>
>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
>> ---
>>   xen/arch/x86/include/asm/bootinfo.h | 14 ++++++++++++--
>>   xen/arch/x86/setup.c                |  9 +++++++++
>>   2 files changed, 21 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/arch/x86/include/asm/bootinfo.h 
>> b/xen/arch/x86/include/asm/bootinfo.h
>> index 87d311ac1399..d19473d8941e 100644
>> --- a/xen/arch/x86/include/asm/bootinfo.h
>> +++ b/xen/arch/x86/include/asm/bootinfo.h
>> @@ -8,20 +8,30 @@
>>   #ifndef __XEN_X86_BOOTINFO_H__
>>   #define __XEN_X86_BOOTINFO_H__
>> +#include <xen/multiboot.h>
>>   #include <xen/types.h>
>> +/* Max number of boot modules a bootloader can provide in addition to 
>> Xen */
>> +#define MAX_NR_BOOTMODS 63
>> +
>> +struct boot_module {
>> +    /* Transitionary only */
>> +    module_t *mod;
>> +};
>> +
>>   /*
>>    * Xen internal representation of information provided by the
>>    * bootloader/environment, or derived from the information.
>>    */
>>   struct boot_info {
>> -    unsigned int nr_modules;
>> -
> 
> (You should probably re-work the other patches to insert ahead of this 
> and avoid the movement.)

Ack.

>>       const char *loader;
>>       const char *cmdline;
>>       paddr_t memmap_addr;
>>       size_t memmap_length;
>> +
>> +    unsigned int nr_modules;
>> +    struct boot_module mods[MAX_NR_BOOTMODS + 1];
>>   };
> 
> Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>

Thank you.

v/r,
dps
Jan Beulich Oct. 9, 2024, 3:17 p.m. UTC | #3
On 06.10.2024 23:49, Daniel P. Smith wrote:
> @@ -302,6 +304,13 @@ static struct boot_info __init *multiboot_fill_boot_info(unsigned long mbi_p)
>          bi->memmap_length = mbi->mmap_length;
>      }
>  
> +    /*
> +     * This will iterate over all modules to include an extra mb module, which
> +     * should have been reserved to hold an entry for Xen.
> +     */
> +    for ( i = 0; i <= bi->nr_modules; i++ )
> +        bi->mods[i].mod = &mods[i];

I find the comment difficult to follow / match with code here and elsewhere.
How about "Iterate over all modules, including the extra one which should
have been reserved for Xen itself"?

Jan
Daniel P. Smith Oct. 10, 2024, 1:01 a.m. UTC | #4
On 10/9/24 11:17, Jan Beulich wrote:
> On 06.10.2024 23:49, Daniel P. Smith wrote:
>> @@ -302,6 +304,13 @@ static struct boot_info __init *multiboot_fill_boot_info(unsigned long mbi_p)
>>           bi->memmap_length = mbi->mmap_length;
>>       }
>>   
>> +    /*
>> +     * This will iterate over all modules to include an extra mb module, which
>> +     * should have been reserved to hold an entry for Xen.
>> +     */
>> +    for ( i = 0; i <= bi->nr_modules; i++ )
>> +        bi->mods[i].mod = &mods[i];
> 
> I find the comment difficult to follow / match with code here and elsewhere.
> How about "Iterate over all modules, including the extra one which should
> have been reserved for Xen itself"?

Okay.

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 87d311ac1399..d19473d8941e 100644
--- a/xen/arch/x86/include/asm/bootinfo.h
+++ b/xen/arch/x86/include/asm/bootinfo.h
@@ -8,20 +8,30 @@ 
 #ifndef __XEN_X86_BOOTINFO_H__
 #define __XEN_X86_BOOTINFO_H__
 
+#include <xen/multiboot.h>
 #include <xen/types.h>
 
+/* Max number of boot modules a bootloader can provide in addition to Xen */
+#define MAX_NR_BOOTMODS 63
+
+struct boot_module {
+    /* Transitionary only */
+    module_t *mod;
+};
+
 /*
  * Xen internal representation of information provided by the
  * bootloader/environment, or derived from the information.
  */
 struct boot_info {
-    unsigned int nr_modules;
-
     const char *loader;
     const char *cmdline;
 
     paddr_t memmap_addr;
     size_t memmap_length;
+
+    unsigned int nr_modules;
+    struct boot_module mods[MAX_NR_BOOTMODS + 1];
 };
 
 #endif /* __XEN_X86_BOOTINFO_H__ */
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index f0482ca8cc55..68eb4c848ae8 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -284,6 +284,8 @@  static struct boot_info __init *multiboot_fill_boot_info(unsigned long mbi_p)
 {
     struct boot_info *bi = &xen_boot_info;
     const multiboot_info_t *mbi = __va(mbi_p);
+    module_t *mods = __va(mbi->mods_addr);
+    unsigned int i;
 
     bi->nr_modules = (mbi->flags & MBI_MODULES) ? mbi->mods_count : 0;
 
@@ -302,6 +304,13 @@  static struct boot_info __init *multiboot_fill_boot_info(unsigned long mbi_p)
         bi->memmap_length = mbi->mmap_length;
     }
 
+    /*
+     * This will iterate over all modules to include an extra mb module, which
+     * should have been reserved to hold an entry for Xen.
+     */
+    for ( i = 0; i <= bi->nr_modules; i++ )
+        bi->mods[i].mod = &mods[i];
+
     return bi;
 }