Re: vio9p vs. GENERIC.local vs. XEN3_DOM[0U]

[Christoph Badura <bad%bsd.de@localhost>]

On Sun, Aug 11, 2024 at 02:08:32PM -0400, Greg Troxel wrote:
> Christoph Badura <bad%bsd.de@localhost> writes:
> 
> > Currently the vio9p driver is commented out in {i386,amd64}/conf/GENERIC:
> > #vio9p*	at virtio?			# Virtio 9P device
> >
> > The obvious way to enable that is by adding a line to GENERIC.local:
> > vio9p*	at virtio?			
> >
> >
> > But doing so breaks the builds of the XEN3_DOM? kernels like so
> >     sys/arch/amd64/conf/GENERIC.local:1: `vio9p* at virtio?' is orphaned (nothing matching `virtio?' found)
> > because
> > $ grep cinclude {i386,amd64}/conf/*XEN3*
> > i386/conf/XEN3PAE_DOM0:cinclude "arch/i386/conf/GENERIC.local"
> > i386/conf/XEN3PAE_DOM0:cinclude "arch/i386/conf/XEN3_DOM0.local"
> > i386/conf/XEN3PAE_DOMU:cinclude "arch/i386/conf/GENERIC.local"
> > i386/conf/XEN3PAE_DOMU:cinclude "arch/i386/conf/XEN3_DOMU.local"
> > amd64/conf/XEN3_DOM0:cinclude   "arch/amd64/conf/GENERIC.local"
> > amd64/conf/XEN3_DOMU:cinclude   "arch/amd64/conf/GENERIC.local"
> > amd64/conf/XEN3_DOMU:cinclude   "arch/amd64/conf/XEN3_DOMU.local"
> >
> > This is extremely annoying, as that breaks "build.sh release" because that
> > builds the XEN3 kernels.  And it prevents us from enabling vio9p on x86
> > kernels by default.

> You have merely found something is problematic because it assumes there
> is virtio.

No, not at all.  You can find any number of examples in GENERIC both in
commented out and not-commented out form that, when added to GENERIC.local
make the XEN3* kernels not configure.

E.g.:

i915drm*     at drm?
qat*   at pci? dev ? function ?
audio* at audiobus?

The technical reason why this doesn't work is that the XEN3* kernels
declare a device tree that is different and not compatible with the one
the the non-XEN3 kernels declare.

Anyway, the situation can, of course, be worked around on the config(1)
level.

mlelstv found the xpci attribute that exists on XEN3* kernels only.
So one could write:
  ifndef xpci
  vio9p* at virtio?
  endif
and jump through that hoop to accomodate XEN3 kernel ideosyncrasies.

Similarly, with uwe's "ifinstance" one
could write:
  ifinstance
  vio9p* at virtio?
  endif
and jump through a different hoop to accomodate XEN3 kernel ideosyncrasies.

Note that these workarounds are so that config(1) doesn't complain about
invalid *declarations* that can't be fitted into the device tree.

One could also try to abuse "pseudo-root":
  pseudo-root virtio*
  vio9p* at virtio?
and jump through yet another hoop to accomodate XEN3 kernel ideosyncrasies.

This at least stops the complaints from config(1) and a quick reading of
ioconf.c for the affected kernels suggest that it should give the desired
results at run time for the XEN3 kernels because none of its busses tries
to attach virtio.

Of course, what looks simple for this one particular examply quickly gets
unwieldy if your GENERIC.local is bigger.  And it requires running
config(1) on all kernels that include GENERIC.local even if they are of no
interest locally.

But these are all besides the point that I care about, which is:

As a NetBSD user with no immediate interest in XEN kernels,
I am made to Jump Through Hoops(tm) to accommodate the ideosyncrasies of
the XEN kernels!

And the root cause of that is that including GENERIC.local to the XEN
kernels was done apparently without thinking through the ramifications.

> > The obvious and simplest fix is to make the XEN3 kernels stop including
> > GENERIC.local.  (And make amd64 XEN3_DOM0 cinclude XEN3_DOM0.local as on
> > i386.)
> 
> I don't think that's reasonable at all.  GENERIC.local is for things you
> want in your kernels, and if it's something else, then it certainly
> belongs in XEN kernels.

That sounds like you think that GENERIC.local is something well thought
out or there is even a policy about it.  Not so!  GENERIC.local
came into existance without public discussion as a, not very well
thought-out, workaround of having to deal with the hassle of uncommited
changes in GENERIC by developers.  And there hasn't been any public
discussion about it that I can find except for jmmv proposing to
remove GENERIC.local from the repository and switching to config(1)'s
"cinclude" directive some 8 years after that directive was add.
GENERIC.local wasn't even a thing on most arches until abs took it upon
himself to add it to mosts arches' GENERIC in February 2023.  (BTW.
thank you for doing that, David!)

> > It seems to me that the best way to remedy the situation is to make the
> > XEN3 kernels not include GENERIC.local.
> 
> That will break a lot of other things, and many will find it very
> surprising.

Then please name at least 3 things that will break.  Or else I'll consider
this a strawman.

Of course, people who include stuff in their XEN kernel will have to make
a trivial one time change.  We even have a standard procedure for handling
these types of changes:  we send a heads-up mail to the relevant lists.
We can document the change in doc/CHANGES too. 

> > If people really want to include GENERIC.local they can do so in their
> > XEN3_DOM?.local files or create a XEN3.local (or XEN3.common.local or
> > whatever) that is included from them.
> 
> If you really want this, you can just add it to GENERIC.  That seems
> better than asking the rest of the world to change.

No.  GENERIC.local is for local changes to GENERIC.  In particular so that
GENERIC doesn't have uncommitted changes that have to be dealt with when
the source is updated.  And that includes bus specific things.

> But seriously, I don't see why making XEN not include GENERIC.local is
> better than people that want bus-specific things putting it someplace
> else.

Frankly, if the Xen people want to insist that the XEN* kernels should
include GENERIC.local (and so far that is only you) then the onus is on
them to make sure that everything that is valid in GENERIC.local when
configuring a GENERIC kernel stays valid when configuring a XEN kernel.

--chris