diff mbox series

[v5,15/44] x86/boot: introduce boot module interator

Message ID 20241006214956.24339-16-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
Provide an iterator to go through boot module array searching based on type.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/include/asm/bootinfo.h | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

Comments

Jason Andryuk Oct. 7, 2024, 8:59 p.m. UTC | #1
On 2024-10-06 17:49, Daniel P. Smith wrote:
> Provide an iterator to go through boot module array searching based on type.
> 
> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>

Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Jan Beulich Oct. 9, 2024, 3:53 p.m. UTC | #2
On 06.10.2024 23:49, Daniel P. Smith wrote:
> --- a/xen/arch/x86/include/asm/bootinfo.h
> +++ b/xen/arch/x86/include/asm/bootinfo.h
> @@ -54,8 +54,24 @@ struct boot_info {
>      struct boot_module mods[MAX_NR_BOOTMODS + 1];
>  };
>  
> -#endif /* __XEN_X86_BOOTINFO_H__ */
> +static inline int __init next_boot_module_index(
> +    const struct boot_info *bi, enum bootmod_type t, int offset)

Instead of "offset" maybe better "start" or "from"? Further, plain int
(as also used ...

> +{
> +    int i;

... here) isn't really liked for ...

> +    for ( i = offset; i < bi->nr_modules; i++ )
> +    {
> +        if ( bi->mods[i].type == t )

... array indexing. Perhaps the function itself would better have
unsigned int return type as well, ...

> +            return i;
> +    }
> +
> +    return -1;

... using UINT_MAX or some other suitable constant here instead?

Jan
Daniel P. Smith Oct. 10, 2024, 1:45 a.m. UTC | #3
On 10/9/24 11:53, Jan Beulich wrote:
> On 06.10.2024 23:49, Daniel P. Smith wrote:
>> --- a/xen/arch/x86/include/asm/bootinfo.h
>> +++ b/xen/arch/x86/include/asm/bootinfo.h
>> @@ -54,8 +54,24 @@ struct boot_info {
>>       struct boot_module mods[MAX_NR_BOOTMODS + 1];
>>   };
>>   
>> -#endif /* __XEN_X86_BOOTINFO_H__ */
>> +static inline int __init next_boot_module_index(
>> +    const struct boot_info *bi, enum bootmod_type t, int offset)
> 
> Instead of "offset" maybe better "start" or "from"? Further, plain int
> (as also used ...

Will change to start.

>> +{
>> +    int i;
> 
> ... here) isn't really liked for ...
> 
>> +    for ( i = offset; i < bi->nr_modules; i++ )
>> +    {
>> +        if ( bi->mods[i].type == t )
> 
> ... array indexing. Perhaps the function itself would better have
> unsigned int return type as well, ...
> 
>> +            return i;
>> +    }
>> +
>> +    return -1;
> 
> ... using UINT_MAX or some other suitable constant here instead?

I was initially going to disagree as returning a value less than zero is 
much more natural/reasonable than UINIT_MAX. But then thinking about it,
another natural value to reflect an error/not found is a value larger 
than MAX_NR_BOOTMODS.

Will switch to unsigned and add code comment that larger than 
MAX_NR_BOOTMODS is error/not found condition.

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 2ee0d5ad6d72..c79678840d31 100644
--- a/xen/arch/x86/include/asm/bootinfo.h
+++ b/xen/arch/x86/include/asm/bootinfo.h
@@ -54,8 +54,24 @@  struct boot_info {
     struct boot_module mods[MAX_NR_BOOTMODS + 1];
 };
 
-#endif /* __XEN_X86_BOOTINFO_H__ */
+static inline int __init next_boot_module_index(
+    const struct boot_info *bi, enum bootmod_type t, int offset)
+{
+    int i;
+
+    for ( i = offset; i < bi->nr_modules; i++ )
+    {
+        if ( bi->mods[i].type == t )
+            return i;
+    }
+
+    return -1;
+}
 
+#define first_boot_module_index(bi, t)              \
+    next_boot_module_index(bi, t, 0)
+
+#endif /* __XEN_X86_BOOTINFO_H__ */
 /*
  * Local variables:
  * mode: C