Re: [PATCH RFC] misc/pvpanic: add support for normal shutdowns

[Greg Kroah-Hartman]

On Sat, Nov 04, 2023 at 02:16:53PM +0100, Thomas Weißschuh wrote:
> On 2023-11-04 14:05:02+0100, Greg Kroah-Hartman wrote:
> > On Sat, Nov 04, 2023 at 12:29:30PM +0100, Thomas Weißschuh wrote:
> > > Shutdown requests are normally hardware dependent.
> > > By extending pvpanic to also handle shutdown requests, guests can
> > > submit such requests with an easily implementable and cross-platform
> > > mechanism.
> > > 
> > > Signed-off-by: Thomas Weißschuh <linux@xxxxxxxxxxxxxx>
> > > ---
> > > The corresponding patch to qemu has also been submitted[0].
> > > General discussions about the feature should happen on the other thread.
> > > 
> > > [0] https://lore.kernel.org/qemu-devel/20231104-pvpanic-shutdown-v1-0-02353157891b@xxxxxxxx/
> > > ---
> > >  drivers/misc/pvpanic/pvpanic.c | 19 +++++++++++++++++--
> > >  include/uapi/misc/pvpanic.h    |  1 +
> > >  2 files changed, 18 insertions(+), 2 deletions(-)
> 
> [..]
> 
> > > diff --git a/include/uapi/misc/pvpanic.h b/include/uapi/misc/pvpanic.h
> > > index 54b7485390d3..82fc618bfbcf 100644
> > > --- a/include/uapi/misc/pvpanic.h
> > > +++ b/include/uapi/misc/pvpanic.h
> > > @@ -5,5 +5,6 @@
> > >  
> > >  #define PVPANIC_PANICKED	(1 << 0)
> > >  #define PVPANIC_CRASH_LOADED	(1 << 1)
> > > +#define PVPANIC_SHUTDOWN	(1 << 2)
> > 
> > Why are these in a uapi file?
> 
> They are ABI between qemu and its guest.

But there's no interaction between Linux and userspace for these values,
so I would just drop them from here.

> The specification for these values is part of qemu but for some reason
> the header is part of Linux which is then imported back into qemu.
> 
> I guess this has historical reasons, maybe because qemu doesn't really
> ship ABI headers and for Linux it's natural.

That feels odd, are there other in-kernel examples of the Linux uapi
files being abused like this?

> The real reason probably doesn't matter today as the header propably
> can't be dropped from Linux anyways for compatibility reasons.
> 
> > And if they need to be here, why not use the proper BIT() macro for it?
> 
> This was for uniformity with the existing code.
> I can send a (standalone?) patch to fix it up.

If we keep it, sure, that would be nice.  But let's try to drop it if
possible :)

thanks,

greg k-h