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

Commit Message

Jan Beulich Aug. 15, 2018, 6:19 a.m. UTC

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 <[e-mail]>
Acked-by: Juergen Gross <[e-mail]>
---
Resend due to prior omission of file system related addressees.
---
 fs/compat_ioctl.c |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

Christoph Hellwig Aug. 15, 2018, 12:41 p.m. UTC | #1

Please wire up a compat_ioctl handler for the xen privcmd handler
instead of adding these to a global table.

Al Viro Aug. 15, 2018, 12:51 p.m. UTC | #2

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 <[e-mail]>
> Acked-by: Juergen Gross <[e-mail]>


Nacked-by: Al Viro <[e-mail]>

	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.

Jan Beulich Aug. 15, 2018, 1:16 p.m. UTC | #3

>>> On 15.08.18 at 14:51, <[e-mail]> 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 <[e-mail]>
>> Acked-by: Juergen Gross <[e-mail]>
> 
> 
> Nacked-by: Al Viro <[e-mail]>
> 
> 	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.

Jan

Al Viro Aug. 15, 2018, 1:27 p.m. UTC | #4

On Wed, Aug 15, 2018 at 07:16:43AM -0600, Jan Beulich wrote:
> >>> On 15.08.18 at 14:51, <[e-mail]> 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 <[e-mail]>
> >> Acked-by: Juergen Gross <[e-mail]>
> > 
> > 
> > Nacked-by: Al Viro <[e-mail]>
> > 
> > 	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...

Patch

--- 4.18/fs/compat_ioctl.c
+++ 4.18-xen-privcmd-compat-ioctls/fs/compat_ioctl.c
@@ -115,6 +115,12 @@ 
 #include <asm/fbio.h>
 #endif
 
+#ifdef CONFIG_XEN
+#include <xen/interface/xen.h>
+#include <xen/evtchn.h>
+#include <xen/privcmd.h>
+#endif
+
 #define convert_in_user(srcptr, dstptr)			\
 ({							\
 	typeof(*srcptr) val;				\
@@ -1300,6 +1306,17 @@  IGNORE_IOCTL(FBIOGETCMAP32)
 IGNORE_IOCTL(FBIOSCURSOR32)
 IGNORE_IOCTL(FBIOGCURSOR32)
 #endif
+
+#ifdef CONFIG_XEN
+COMPATIBLE_IOCTL(IOCTL_PRIVCMD_HYPERCALL)
+COMPATIBLE_IOCTL(IOCTL_PRIVCMD_RESTRICT)
+COMPATIBLE_IOCTL(IOCTL_EVTCHN_BIND_VIRQ)
+COMPATIBLE_IOCTL(IOCTL_EVTCHN_BIND_INTERDOMAIN)
+COMPATIBLE_IOCTL(IOCTL_EVTCHN_BIND_UNBOUND_PORT)
+COMPATIBLE_IOCTL(IOCTL_EVTCHN_UNBIND)
+COMPATIBLE_IOCTL(IOCTL_EVTCHN_NOTIFY)
+COMPATIBLE_IOCTL(IOCTL_EVTCHN_RESET)
+#endif
 };
 
 /*