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: Mon, 2 Jun 2025 08:19:22 -0400

 On 6/1/2025 1:55 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: Sun, 1 Jun 2025 05:52:25 -0000 (UTC)
 > 
 >  frchuckz%gmail.com@localhost writes:
 >  
 >  >--- sys/arch/xen/xen/xen_machdep.c.orig 2023-10-18 12:53:03.000000000 -0400
 >  >+++ sys/arch/xen/xen/xen_machdep.c      2025-05-30 20:42:39.936253878 -0400
 >  >@@ -553,7 +553,10 @@
 >  >                        /* XXX check device_cfdata as in x86_autoconf.c? */
 >  >                        booted_partition = toupper(
 >  >                                xcp.xcp_bootdev[strlen(devname)]) - 'A';
 >  >+                       /* Check that the value of booted_partition is sane */
 >  >+                       if (booted_partition & 0xfffffff0)
 >  >+                               continue;
 >  >                        DPRINTF(("%s: booted_partition: %d\n", __func__, booted_partition));
 >  >                }
 >  >                booted_device = dv;
 >  
 >  
 >  The code should check that the partition is between 0 and MAXPARTITIONS-1
 >  instead of using some bitmask and it shouldn't touch booted_partition
 >  before the value is validated.
 >  
 >  It should also parse xcp_bootdev better (this was bad before and the
 >  x86 version of that code isn't really better).
 >  
 >  
 >  However, nothing of that really helps to use wedges ("dkXX") as
 >  wedge unit numbers are a bit volatile.
 >  
 >  
 >  The better alternative is to not set "bootdev" but to pass the
 >  "root" command line option. The value is a string and interpreted
 >  by the MI part of the kernel as a device name (with partition
 >  for a disk) or as NAME=wedgename (or for compatiblity wedge:wedgename).
 >  
 >  E.g.:
 >  
 >  menu=Boot Xen Dom0:load /netbsd_xen console=pc root=NAME=my-root;multiboot /xen.gz
 >  
 >  
 
 I can verify that using the bootdev=NAME=wedgename form (or the root=NAME=wedgename
 form) in boot.cfg instead of the bootdev=dkXX form (or the root=dkXX form) is a way
 to work around this problem by completely bypassing this problematic xen MD code
 and use the MI code that Michael refers to above instead.
 
 Still, we should fix this problematic xen MD code so the bootdev=dkXX form (or the
 root=dkXX form) in boot.cfg will not find the wrong root device.
 
 So I will propose a patch for -current (after testing it on my box) that
 uses MAXPARTITIONS to validate booted_partition and avoids touching
 booted_partition before its value is validated. I don't know if such a patch
 fully addresses the problems with this code, but at least it would prevent us
 from finding the wrong root device when using the bootdev=dkXX form
 (or the root=dkXX form) in boot.cfg and allow us to close this PR.
 
 I also think the patch should be pulled to netbsd-10 and netbsd-9 (if netbsd-9
 is still supported). In netbsd-9, I think this problematic code is in a different
 file under sys/arch/xen, I think it is sys/arch/xen/x86/autoconf.c IIRC.