On Tue, Nov 26, 2024 at 10:32:23AM -0700, Joel Knight wrote:
> On Mon, Nov 25, 2024 at 9:17 PM Jonathan Gray <jsg@jsg.id.au> wrote:
> >
> > On Wed, Nov 20, 2024 at 07:11:11AM -0700, Joel Knight wrote:
> > > On Tue, Nov 19, 2024 at 10:18 PM Jonathan Gray <jsg@jsg.id.au> wrote:
> > >
> > > On this version of Xen, there are just the 3 devices:
> > >
> > > % hostctl device
> > > 9pfs
> > > vbd
> > > vif
> > >
> > > dmesg with attached diff:
> > >
> > > bios0: vendor Xen version "4.17" date 10/02/2024
> > > bios0: Xen HVM domU
> > > pvbus0 at mainbus0: Xen 4.17
> > > xen0 at pvbus0: features 0x112705, 64 grant table frames, event channel 3
> > > xen0: failed to attach "device/9pfs/"
> > > xbf0 at xen0 backend 0 channel 8: cdrom
> > > xbf1 at xen0 backend 0 channel 8: disk
> > > sd0 at scsibus1 targ 0 lun 0: <Xen, phy xvda 768, 0000>
> > > xbf2 at xen0 backend 0 channel 9: disk
> > > sd1 at scsibus2 targ 0 lun 0: <Xen, phy xvdb 832, 0000>
> > > xnf0 at xen0 backend 0 channel 10: address 4e:4b:b7:f4:f4:94
> > > xspd0 at pci0 dev 3 function 0 "XenSource Platform Device" rev 0x02
> > > xen0: failed to attach "device/9pfs/"
> >
> > Ideally it would be something like
> > "9pfs" at xen0 backend 0 channel 8 not configured
> >
> > config_found()/config_found_sm() should do that.  I don't follow why
> > that doesn't happen.  Could you try and figure it out?  Is it because
> > 9pfs is not a different device, but rather a property of /device/vbd/ ?
> > Should xbf_match() not claim devices with the 9pfs property?
> 
> xen_attach_device() is kicking out early after xs_getprop() for
> "backend". With XEN_DEBUG:
> 
> xspd0 at pci0 dev 3 function 0 "XenSource Platform Device" rev 0x02
> xen0: attaching "9pfs/"
> xen0: failed to identify "backend" for "device/9pfs/"
> 
> There are no units under the 9pfs device. This appears to be how
> XCP-ng 8.3/Xen 4.17 presents this device since I see the same on a
> FreeBSD VM.
> 
> The diff below causes xen_attach_devices() to continue after
> xs_getprop()/getnum() so that config_found() is called. I'm not sure
> what side effects this might have, but I see no issues on this test
> VM.

I've zero'd the attach args, which may help there.

> 
> xen0 at pvbus0: features 112705, idtvec 112, 64 grant table frames,
> event channel 3
> "9pfs" at xen0: device/9pfs/ not configured
> xbf0 at xen0 backend 0 channel 8: cdrom
> [...]

Great, thanks for the patch, committed.

Your patch lacked tabs, so didn't apply.  What was committed, with tabs:

Index: sys/dev/pv/xen.c
===================================================================
RCS file: /cvs/src/sys/dev/pv/xen.c,v
diff -u -p -r1.98 -r1.100
--- sys/dev/pv/xen.c	24 May 2024 10:05:55 -0000	1.98
+++ sys/dev/pv/xen.c	27 Nov 2024 02:38:35 -0000	1.100
@@ -1,4 +1,4 @@
-/*	$OpenBSD: xen.c,v 1.98 2024/05/24 10:05:55 jsg Exp $	*/
+/*	$OpenBSD: xen.c,v 1.100 2024/11/27 02:38:35 jsg Exp $	*/
 
 /*
  * Copyright (c) 2015, 2016, 2017 Mike Belopuhov
@@ -1355,6 +1355,7 @@ xen_attach_device(struct xen_softc *sc, 
 	struct xen_device *xdv;
 	unsigned long long res;
 
+	memset(&xa, 0, sizeof(xa));
 	xa.xa_dmat = &xen_bus_dma_tag;
 
 	strlcpy(xa.xa_name, name, sizeof(xa.xa_name));
@@ -1364,15 +1365,14 @@ xen_attach_device(struct xen_softc *sc, 
 	    sizeof(xa.xa_backend))) {
 		DPRINTF("%s: failed to identify \"backend\" for "
 		    "\"%s\"\n", sc->sc_dev.dv_xname, xa.xa_node);
-		return (EIO);
 	}
 
 	if (xs_getnum(sc, xa.xa_node, "backend-id", &res) || res > UINT16_MAX) {
 		DPRINTF("%s: invalid \"backend-id\" for \"%s\"\n",
 		    sc->sc_dev.dv_xname, xa.xa_node);
-		return (EIO);
 	}
-	xa.xa_domid = (uint16_t)res;
+	if (res <= UINT16_MAX)
+		xa.xa_domid = (uint16_t)res;
 
 	xdv = malloc(sizeof(struct xen_device), M_DEVBUF, M_ZERO | M_NOWAIT);
 	if (xdv == NULL)
@@ -1426,7 +1426,7 @@ xen_probe_devices(struct xen_softc *sc)
 				printf("%s: failed to attach \"%s/%s\"\n",
 				    sc->sc_dev.dv_xname, path,
 				    (const char *)iovp2[j].iov_base);
-				goto out;
+				continue;
 			}
 		}
 		/* Setup a watch for every device subtree */