The early exit from domain_cmdline_size() is buggy.  Even if there's no
bootloader cmdline and no kextra, there still might be Xen parameters to
forward, and therefore a nonzero cmdline.

Explain what the function is doing, and rewrite it to be both more legible and
more extendible.

Fixes: 142f0a43a15a ("x86/boot: add cmdline to struct boot_domain")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>

And just to demonstrate how bad ternary operators are when they're not used
over trivial scalars, this "no effective change in logic" still saves most of
a cacheline of logic:

  add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-56 (-56)
  Function                                     old     new   delta
  __start_xen                                 8946    8890     -56

The "if ( cmdline_size )" in create_dom0() could be dropped.  It's impossible
to be 0 now, which is correct behaviour.  But, that should be deferred to a
later change.
---
 xen/arch/x86/setup.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 1f5cb67bd0ee..d47b99f1b2c9 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -980,15 +980,22 @@ static unsigned int __init copy_bios_e820(struct e820entry *map, unsigned int li
     return n;
 }
 
+/*
+ * Calculate the maximum possible size of the dom0 cmdline.  Pieces of the
+ * dom0 cmdline optionally come from the bootloader directly, from Xen's
+ * cmdline (following " -- "), and individual Xen parameters are forwarded
+ * too.
+ */
 static size_t __init domain_cmdline_size(const struct boot_info *bi,
                                          const struct boot_domain *bd)
 {
-    size_t s = bi->kextra ? strlen(bi->kextra) : 0;
+    size_t s = 0;
 
-    s += bd->kernel->cmdline_pa ? strlen(__va(bd->kernel->cmdline_pa)) : 0;
+    if ( bd->kernel->cmdline_pa )
+        s += strlen(__va(bd->kernel->cmdline_pa));
 
-    if ( s == 0 )
-        return s;
+    if ( bi->kextra )
+        s += strlen(bi->kextra);
 
     /*
      * Certain parameters from the Xen command line may be added to the dom0

base-commit: 2c4a3d688943b2034756859844b8337a5a97ce07
prerequisite-patch-id: 32a8746877e6b92075be2f022dca25c6bfa6f31e
-- 
2.39.5

On 04.06.2025 01:56, Andrew Cooper wrote:
> The early exit from domain_cmdline_size() is buggy.  Even if there's no
> bootloader cmdline and no kextra, there still might be Xen parameters to
> forward, and therefore a nonzero cmdline.
> 
> Explain what the function is doing, and rewrite it to be both more legible and
> more extendible.
> 
> Fixes: 142f0a43a15a ("x86/boot: add cmdline to struct boot_domain")

I don't think this is correct. Iirc I even commented on this apparent anomaly,
being (validly) told that it preserved original behavior, where all the code
was inside "if ( bd->kernel->cmdline_pa || bi->kextra )". Since I had asked to
fix this already back then, ...

> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
with the Fixes: tag corrected (or dropped, for simplicity).

Jan