Re: port-xen/59451: XEN3_DOM0 kernel finds the wrong root device

["Michael van Elst via gnats" <gnats-admin%NetBSD.org@localhost>]

The following reply was made to PR port-xen/59451; it has been noted by GNATS.

From: mlelstv%serpens.de@localhost (Michael van Elst)
To: gnats-bugs%netbsd.org@localhost
Cc: 
Subject: Re: port-xen/59451: XEN3_DOM0 kernel finds the wrong root device
Date: Mon, 2 Jun 2025 12:18:41 -0000 (UTC)

 gnats-admin%NetBSD.org@localhost ("Chuck Zmudzinski via gnats") writes:
 
 > Still, the bootdev=dk12 form needs some fix/kernel patch for this case when
 > NetBSD/xen is installed on a wedge with a double digit index, so this PR should
 > remain open.
 
 Yes. I'm currently testing the following patch. The parsing is
 still not perfect, e.g. it accepts a 'partition' for devices that
 don't support partitions (like dk or dm).
 
 Index: xen_machdep.c
 ===================================================================
 RCS file: /cvsroot/src/sys/arch/xen/xen/xen_machdep.c,v
 retrieving revision 1.29
 diff -p -u -r1.29 xen_machdep.c
 --- xen_machdep.c       17 Oct 2023 10:24:11 -0000      1.29
 +++ xen_machdep.c       2 Jun 2025 11:56:22 -0000
 @@ -62,6 +62,7 @@ __KERNEL_RCSID(0, "$NetBSD: xen_machdep.
  #include <sys/boot_flag.h>
  #include <sys/conf.h>
  #include <sys/disk.h>
 +#include <sys/disklabel.h>
  #include <sys/device.h>
  #include <sys/mount.h>
  #include <sys/reboot.h>
 @@ -546,14 +547,28 @@ xen_bootconf(void)
                         break;
                 }
  
 -               if (strncmp(xcp.xcp_bootdev, devname, strlen(devname)))
 -                       continue;
 +               if (is_disk) {
 +                       size_t len;
 +                       int part;
 +
 +                       len = strlen(devname);
 +                       if  (strncmp(xcp.xcp_bootdev, devname, len) != 0)
 +                               continue;
 +
 +                       if (xcp.xcp_bootdev[len] != '\0') {
 +                               part = xcp.xcp_bootdev[len] - 'a';
 +                               if (part < 0 || part >= MAXPARTITIONS)
 +                                       continue;
 +
 +                               if (xcp.xcp_bootdev[len+1] != '\0')
 +                                       continue;
  
 -               if (is_disk && strlen(xcp.xcp_bootdev) > strlen(devname)) {
 -                       /* XXX check device_cfdata as in x86_autoconf.c? */
 -                       booted_partition = toupper(
 -                               xcp.xcp_bootdev[strlen(devname)]) - 'A';
 -                       DPRINTF(("%s: booted_partition: %d\n", __func__, booted_partition));
 +                               booted_partition = part;
 +                               DPRINTF(("%s: booted_partition: %d\n", __func__, booted_partition));
 +                       }
 +               } else {
 +                       if  (strcmp(xcp.xcp_bootdev, devname) != 0)
 +                               continue;
                 }
  
                 booted_device = dv;
 
 
 > >  Also, according to a message I received on netbsd-users from Manuel who AFAIK is
 > >  a port-xen maintainer, there is no difference in the arch/xen code between
 > >  bootdev= and rootdev=. What you say here puts that in doubt, though.
 > 
 > My testing verified that Manuel is correct: bootdev= and root= behave the same way,
 > but actually root= is better because it gives me three more characters for the
 > boot command line before it overflows and starts to truncate the end of the command
 > line.
 
 
 bootdev= and root= currently work the same way.
 
 The supplied string is checked against all disk and network interface
 device names.
 
 If there is a match, then the result is a pointer "booted_device"
 to the driver and an integer "booted_partition". The kernel will
 later try to access the particular driver and read data from the
 numbered partition. Obviously the partition is ignored for a
 network interface, and it should be 0 for something like a wedge
 (that doesn't have partitions).
 
 If there is no match, then the string is passed as is, similar to
 a hardcoded embedded "config root" in the kernel config file. The
 result is the string pointer "bootspec" that is evaluated later
 by the kernel.
 
 In either case, the supplied string can have 144 bytes (including
 the terminating NUL character), but the parser truncates the
 full command line to 255 characters first, not sure why.