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

["Chuck Zmudzinski via gnats" <gnats-admin%NetBSD.org@localhost>]

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

From: Chuck Zmudzinski <frchuckz%gmail.com@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: 
Subject: Re: port-xen/59451: XEN3_DOM0 kernel finds the wrong root device
Date: Tue, 3 Jun 2025 08:22:27 -0400

 On 6/2/2025 8:20 AM, Michael van Elst via gnats wrote:
 > 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;
 >  
 
 So taking into account everything I have learned, the only thing I would
 add to this patch is a check that we do not have a "dk" wedge device
 before we try to find a partition and set booted_partition. Probably replacing
 the first insertion of the second hunk:
 
  +               if (is_disk) {
 
 with
 
  +               if (is_disk && !device_is_a(dv, "dk")) {
 
 But I have not tested this yet.
 
 This patch that attempts to fix the parsing problems here is
 probably only suitable for -current. I still plan to propose a
 conservative patch more suitable for the stable branches that
 will be less likely to introduce unintended consequences for
 stable users.