On 12.06.2025 10:20, Alejandro Vallejo wrote: > On Wed Jun 11, 2025 at 7:35 AM CEST, Jan Beulich wrote: >> On 10.06.2025 19:39, Jason Andryuk wrote: >>> >>> >>> On 2025-06-10 02:56, Jan Beulich wrote: >>>> On 09.06.2025 19:07, Jason Andryuk wrote: >>>>> On 2025-04-29 08:36, Alejandro Vallejo wrote: >>>>>> From: "Daniel P. Smith" <dpsm...@apertussolutions.com> >>>>>> >>>>>> Add support to read the command line from the hyperlaunch device tree. >>>>>> The device tree command line is located in the "bootargs" property of the >>>>>> "multiboot,kernel" node. >>>>>> >>>>>> A boot loader command line, e.g. a grub module string field, takes >>>>>> precendence over the device tree one since it is easier to modify. >>>>>> >>>>>> 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> >>>>>> Reviewed-by: Denis Mukhin <dmuk...@ford.com> >>>>>> --- >>>>> >>>>>> diff --git a/xen/common/domain-builder/fdt.c >>>>>> b/xen/common/domain-builder/fdt.c >>>>>> index cbb0ed30a2..dabe201b04 100644 >>>>>> --- a/xen/common/domain-builder/fdt.c >>>>>> +++ b/xen/common/domain-builder/fdt.c >>>>>> @@ -219,6 +219,12 @@ static int __init fdt_process_domain_node( >>>>>> printk(XENLOG_INFO " kernel: multiboot-index=%d\n", idx); >>>>>> bi->mods[idx].type = BOOTMOD_KERNEL; >>>>>> bd->kernel = &bi->mods[idx]; >>>>>> + >>>>>> + /* If bootloader didn't set cmdline, see if FDT provides >>>>>> one. */ >>>>>> + if ( bd->kernel->cmdline_pa && >>>>>> + !((char *)__va(bd->kernel->cmdline_pa))[0] ) >>>>> >>>>> The logic is incorrect - it should be: >>>>> >>>>> if ( !bd->kernel->cmdline_pa || >>>>> !((char *)__va(bd->kernel->cmdline_pa))[0] ) >>>>> >>>>> If there is no cmdline_pa (which happens with the "reg" property) or the >>>>> if there is a 0-length string, then check the DT for bootargs. >>>> >>>> Even that sounds bogus to me: There's a difference between "no command >>>> line" >>>> and "empty command line". >>> >>> Yes, you have a point. The difficulty is grub always provides a NUL >>> terminated string, so Xen can't differentiate the two. >> >> Which may call for either special-casing GrUB, or at least calling out that >> behavior in the comment. (Ideally we'd still have a way to distinguish >> between both cases, but likely we'll need to resort to documenting that some >> dummy option will need adding to tell "none" from [intended to be] empty.) > > We can add suitable comments where required, sure. > > About the dummy option, note that even if we have an option for Xen, that does > nothing for the kernel cmdlines. If there's such dummy option there I don't > know > of it.
Indeed I meant we may need to introduce something. Jan
