Re: [PATCH v4 01/13] x86/boot: add cmdline to struct boot_domain

[Alejandro Vallejo <agarciav@xxxxxxx>]

On Fri Apr 18, 2025 at 10:16 PM BST, dmkhn wrote:
> On Thu, Apr 17, 2025 at 01:48:23PM +0100, Alejandro Vallejo wrote:
>> From: "Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>
>> 
>> Add a container for the "cooked" command line for a domain. This
>> provides for the backing memory to be directly associated with the
>> domain being constructed.  This is done in anticipation that the domain
>> construction path may need to be invoked multiple times, thus ensuring
>> each instance had a distinct memory allocation.
>> 
>> Signed-off-by: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
>> Signed-off-by: Jason Andryuk <jason.andryuk@xxxxxxx>
>> Signed-off-by: Alejandro Vallejo <agarciav@xxxxxxx>
>
>
> Reviewed-by: Denis Mukhin <dmukhin@xxxxxxxx>

Thanks

>> diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
>> index 2a094b3145..49832f921c 100644
>> --- a/xen/arch/x86/hvm/dom0_build.c
>> +++ b/xen/arch/x86/hvm/dom0_build.c
>> @@ -778,21 +777,21 @@ static int __init pvh_load_kernel(
>>      /* Free temporary buffers. */
>>      free_boot_modules();
>> 
>> -    if ( cmdline != NULL )
>> +    rc = hvm_copy_to_guest_phys(last_addr, bd->cmdline, cmdline_len, v);
>> +    if ( rc )
>>      {
>> -        rc = hvm_copy_to_guest_phys(last_addr, cmdline, strlen(cmdline) + 
>> 1, v);
>> -        if ( rc )
>> -        {
>> -            printk("Unable to copy guest command line\n");
>> -            return rc;
>> -        }
>> -        start_info.cmdline_paddr = last_addr;
>> -        /*
>> -         * Round up to 32/64 bits (depending on the guest kernel bitness) so
>> -         * the modlist/start_info is aligned.
>> -         */
>> -        last_addr += elf_round_up(&elf, strlen(cmdline) + 1);
>> +        printk("Unable to copy guest command line\n");
>
> Side note: I think it makes sense to add domain ID to all printouts in
> pvh_load_kernel(). E.g. block under elf_xen_parse() logs domain ID.

Sounds sensible and easy enough. Sure.

Cheers,
Alejandro