Re: [PATCH] xen: Fix event channel callback via INTX/GSI

[David Woodhouse <dwmw2@xxxxxxxxxxxxx>]

On Sun, 2020-12-20 at 13:01 -0500, boris.ostrovsky@xxxxxxxxxx wrote:
> On 12/19/20 3:05 AM, David Woodhouse wrote:
> > On Fri, 2020-12-18 at 17:20 -0500, boris.ostrovsky@xxxxxxxxxx wrote:
> > > Are there other cases where this would be useful? If it's just to
> > > test a hypervisor under development I would think that people who are
> > > doing that kind of work are capable of building their own kernel. My
> > > concern is mostly about having yet another boot option that is of
> > > interest to very few people who can easily work around not having it.
> > 
> > For hypervisor testing we can just set the Xen major version number in
> > CPUID to 3, and that stops xs_reset_watches() from doing anything.
> > 
> > cf. https://lkml.org/lkml/2017/4/10/266
> > 
> > Karim ripped out all this INTX code in 2017 because it was broken, and
> > subsequently put it back because it *was* working for older versions of
> > Xen, due to that "coincidence". The conclusion back then was that if it
> > was put back it should at least *work* consistently, and he was going
> > to send a patch "shortly". This is a that patch :)
> 
> 
> Right, I am not arguing about usefulness of the fix, only of the new boot 
> option.

The boot option also makes it easier for Linux developers to actually
test this.

It's all very well for someone who already has the hypervisor code open
in an emacs buffer, who can turn off vector support then run the canned
test case which spins up that hypervisor and runs Linux nested inside
to. But most people don't live like that, and a command line option
that Linux can use and just run in a normal Xen environment to test
these code paths is kind of useful.


> > > Can we delay xs_init() for !XS_HVM as well? In other words wait until
> > > after PCI platform device has been probed (on HVM) and then call
> > > xs_init() for everyone.
> > 
> > We're half-way there already, because xenbus_probe() *does* happen
> > later as a device_initcall, and I've just made it call xs_init().
> > 
> > We could make it avoid calling xs_init() from xenbus_init() in the
> > XS_HVM *and* XS_PV cases fairly easily, and let xenbus_probe() do it.
> 
> 
> Yes, that's along the lines of what I was thinking.
> 
> 
> > 
> > But right now xenbus_probe() doesn't run for the other cases, so
> > there'd have to be a mode where it *only* calls xs_init() and doesn't
> > do the notifier chain. That seems like more churn that was needed so I
> > didn't do it.
> 
> 
> You think so? Yes, there would be a couple more places where you'd
> need to call xenbus_probe() but then you won't have to explain (with
> comments) why you call xs_init() here and not there and vice versa.
> It just looks to me a bit more complicated the way you do this but I
> suppose it's a matter of personal preference.

I suspect it probably just moves things around and leaves *different*
things to explain (with comments!).

I'll split it out into separate patches (and also fix up the fact that
we're registering the IPI and spinlock event channels even though we're
not going to use them).

Attachment: smime.p7s
Description: S/MIME cryptographic signature