On Wed Apr 9, 2025 at 10:05 PM BST, Denis Mukhin wrote:
> On Tuesday, April 8th, 2025 at 9:07 AM, Alejandro Vallejo <agarc...@amd.com>
> wrote:
>
>>
>>
>> From: "Daniel P. Smith" dpsm...@apertussolutions.com
>>
>>
>> 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 dpsm...@apertussolutions.com
>>
>> Signed-off-by: Jason Andryuk jason.andr...@amd.com
>>
>> Signed-off-by: Alejandro Vallejo agarc...@amd.com
>>
>> ---
>> No changes on ACPI cmdline handling on PVH, as it's orthogonal to the
>> purpose of this patch.
>>
>> v3:
>> * s/xfree/XFREE/ on failed construct_dom0() to avoid a dangling
>> cmdline ptr.
>> * Re-flow hvm_copy_to_guest_phys() into a multi-line call.
>> * s/bd->cmdline != NULL/b->cmdline/ (to homogenise with the previous
>>
>> cmdline pointer check)
>> ---
>> xen/arch/x86/hvm/dom0_build.c | 12 +++----
>> xen/arch/x86/include/asm/boot-domain.h | 1 +
>> xen/arch/x86/pv/dom0_build.c | 4 +--
>> xen/arch/x86/setup.c | 50 +++++++++++++++++++-------
>> 4 files changed, 47 insertions(+), 20 deletions(-)
>>
>> diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
>> index 2a094b3145..ebad5a49b8 100644
>> --- a/xen/arch/x86/hvm/dom0_build.c
>> +++ b/xen/arch/x86/hvm/dom0_build.c
>> @@ -653,7 +653,6 @@ static int __init pvh_load_kernel(
>> void *image_start = image_base + image->headroom;
>>
>> unsigned long image_len = image->size;
>>
>> unsigned long initrd_len = initrd ? initrd->size : 0;
>>
>> - const char *cmdline = image->cmdline_pa ? __va(image->cmdline_pa) : NULL;
>>
>> const char *initrd_cmdline = NULL;
>> struct elf_binary elf;
>> struct elf_dom_parms parms;
>> @@ -736,8 +735,8 @@ static int __init pvh_load_kernel(
>> initrd = NULL;
>> }
>>
>> - if ( cmdline )
>> - extra_space += elf_round_up(&elf, strlen(cmdline) + 1);
>> + if ( bd->cmdline )
>>
>> + extra_space += elf_round_up(&elf, strlen(bd->cmdline) + 1);
>
> I think it makes sense to use local variable to cache the results of
>
> strlen(bd->cmdline) + 1
>
> since the construct is used multiple times within pvh_load_kernel()
>
> E.g. cmdline_len similarly to {image,initrd}_len.
>
Seeing how you're the second person to propose the same thing, I'll bite
and just do it. I really dislike the wall of locals in
pvh_load_kernel(), but oh well...
Cheers,
Alejandro
