Re: [PATCH 09/15] x86/hyperlaunch: obtain cmdline from device tree

["Daniel P. Smith" <dpsmith@apertussolutions.com>]

On 11/25/24 18:12, Jason Andryuk wrote:
> On 2024-11-23 13:20, Daniel P. Smith wrote:
>> If a command line is not provided through the bootloader's mechanism, 
>> e.g.
>> muiltboot module string field, then use one from the device tree if 
>> present.
>> The device tree command line is located in the bootargs property of the
>> `multiboot,kernel` node.
>>
>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
>> ---
>>   xen/arch/x86/domain_builder/core.c       | 28 +++++++++++++++++++
>>   xen/arch/x86/domain_builder/fdt.c        | 34 ++++++++++++++++++++++++
>>   xen/arch/x86/domain_builder/fdt.h        | 24 +++++++++++++++++
>>   xen/arch/x86/include/asm/bootinfo.h      |  6 +++--
>>   xen/arch/x86/include/asm/domainbuilder.h |  4 +++
>>   xen/arch/x86/setup.c                     | 10 +++++--
>>   6 files changed, 102 insertions(+), 4 deletions(-)
>>
>> diff --git a/xen/arch/x86/domain_builder/core.c b/xen/arch/x86/ 
>> domain_builder/core.c
>> index 9335f3a9ebef..95cab06e6159 100644
>> --- a/xen/arch/x86/domain_builder/core.c
>> +++ b/xen/arch/x86/domain_builder/core.c
>> @@ -8,9 +8,37 @@
>>   #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)
>> +{
>> +#ifdef CONFIG_DOMAIN_BUILDER
> 
> I wasnted to suggest:
> 
> if ( !IS_ENABLED(CONFIG_DOMAIN_BUILDER) )
>      return 0;
> 
> but that fails to compile for a missing fdt_cmdline_prop_size().

Yes, I wanted to do the same as well.

>> +    const void *fdt = bootstrap_map_bm(&bi- 
>> >mods[HYPERLAUNCH_MODULE_IDX]);
>> +    int size = fdt_cmdline_prop_size(fdt, offset);
>> +
>> +    bootstrap_unmap();
>> +    return size < 0 ? 0 : (size_t) size;
>> +#else
>> +    return 0;
>> +#endif
>> +}
>> +
>> +int __init builder_get_cmdline(
>> +    struct boot_info *bi, int offset, char *cmdline, size_t size)
>> +{
>> +#ifdef CONFIG_DOMAIN_BUILDER
> 
> and here fdt_cmdline_prop_copy().  I'm not sure the addition of more 
> stubs offsets these ifdefs, so:
> 
> Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>

Thanks!

v/r,
dps