Re: [PATCH RESEND] compat-ioctl/Xen: support at least some IOCTLs of evtchn and privcmd

[Al Viro <viro@ZenIV.linux.org.uk>]

On Wed, Aug 15, 2018 at 07:16:43AM -0600, Jan Beulich wrote:
> >>> On 15.08.18 at 14:51, <viro@ZenIV.linux.org.uk> wrote:
> > On Wed, Aug 15, 2018 at 12:19:00AM -0600, Jan Beulich wrote:
> >> While this is only a start (IOCTL_PRIVCMD_MMAP* and IOCTL_PRIVCMD_DM_OP
> >> require more work), it at least allows some simple operations (like
> >> "xl dmesg") which have always been available on XenoLinux to work again
> >> with a 64-bit kernel underneath a 32-bit distro.
> >> 
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >> Acked-by: Juergen Gross <jgross@suse.com>
> > 
> > 
> > Nacked-by: Al Viro <viro@zeniv.linux.org.uk>
> > 
> > 	This belongs in drivers/xen/privcmd.c, not in fs/compat_ioctl.c
> > *ANY* single-driver ioctl definitely should be handled by ->compat_ioctl()
> > method and any patches dumping such stuff into fs/compat_ioctl.c will be
> > rejected.
> 
> Okay, I'll see about doing this; I have to admit I was unaware of
> ->compat_ioctl()'s existence. It would be rather helpful if the header
> of compat_ioctl.c indicated what you say. Without that, I've simply
> gone by the observation that there are things in the file which
> according to what you say shouldn't be there, and I've then wrongly
> implied adding the Xen stuff here would be fine.

Originally fs/compat_ioctl.c was a result of merger between several such
beasts in arch/*; it's *old* - well before ->compat_ioctl() introduction.
If you check the history of that thing, you'll see crap getting removed
over the years.  Sometimes the things sneak in, but generally it keeps
shrinking...

The only stuff that might belong there is widely-implemented ioctls.
Anything that is natively handled in a couple of ->ioctl() instances
should be getting ->compat_ioctl() instance to go with that one.

And yes, big fat warning along the lines of "don't dump here" is
probably a good idea...