Re: [PATCH v2 03/15] x86/boot: add cmdline to struct boot_domain

[Jason Andryuk <jason.andryuk@amd.com>]

On 2025-01-15 12:22, Daniel P. Smith wrote:
> On 1/10/25 14:52, Jason Andryuk wrote:
>> On 2024-12-26 11:57, Daniel P. Smith wrote:
>>> 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@apertussolutions.com>
>>
>>
>>> @@ -1018,39 +1037,52 @@ static struct domain *__init 
>>> create_dom0(struct boot_info *bi)
>>>           panic("Error creating d%uv0\n", bd->domid);
>>>       /* Grab the DOM0 command line. */
>>> -    if ( bd->kernel->cmdline_pa || bi->kextra )
>>> +    if ( (bd->kernel->cmdline_pa &&
>>> +          ((char *)__va(bd->kernel->cmdline_pa))[0]) ||
>>> +         bi->kextra )
>>>       {
>>> -        if ( bd->kernel->cmdline_pa )
>>> -            safe_strcpy(cmdline,
>>> -                        cmdline_cook(__va(bd->kernel->cmdline_pa), 
>>> bi->loader));
>>> +        size_t cmdline_size = domain_cmdline_size(bi, bd);
>>> +
>>> +        if ( cmdline_size )
>>> +        {
>>> +            if ( !(cmdline = xzalloc_array(char, cmdline_size)) )
>>> +                panic("Error allocating cmdline buffer for %pd\n", d);
>>
>> I guess I wasn't clear last time.  Instead of two levels of indent, I 
>> was thinking at the top level:
>>
>>      /* Grab the DOM0 command line. */
>>      cmdline_size = domain_cmdline_size(bi, bd);
>>      if ( cmdline_size )
>>      {
>>
>> domain_cmdline_size() checks all the pointers, so this removes 
>> duplication and indent.
> 
> But it is possible for there to be no command line, thus there is a 
> legitimate case where cmdline_size will be 0. If it is 0, there is no 
> reason to go through all of this logic.

Sure, but domain_cmdline_size() already handles an empty command line 
and returns 0 for that.  So I think this logic is still skipped in that 
case.

Regards,
Jason