[RFC] VGDrvCommonIoCtl: Add f32bit flag argument

[RFC] VGDrvCommonIoCtl: Add f32bit flag argument

[Hans de Goede]

Note to linux-kernel readers: This Cc-ed to linux-kernel because it is
relevant for the "[RFC 0/2] Add Virtual Box vboxguest and vboxsf guest
drivers to the mainline kernel" thread.

Hi Michael, Knut,

My first submission of the vboxguest driver for inclusion into
the Linux kernel has lead to some questions about the use of the
VBOXGUEST_IOCTL_FLAG to differentiate between 32 and 64 bit
ioctls. Under Linux this is not necessary, as the driver already
knows if it is serving a 32 bit compat or a regular ioctl.

So I've come up with this patch to make VBOXGUEST_IOCTL_FLAG
always 0 under Linux. I was hoping the f32bit flag could be
used under more platforms so that it would actually be a cleanup,
but it seems that Linux is the only platform with a compat_ioctl
callback in its file-operations struct.

Please let me know if you would be willing to merge this patch
into upstream VirtualBox.

Regards,

Hans

[PATCH] VGDrvCommonIoCtl: Add f32bit flag argument

[Hans de Goede]

On some operating-systems the driver can tell whether an ioctl is a
32bit compat callback or not without looking at the ioctl-nr, at a flag
argument to pass this info along and use it for Linux to not need
separate ioctl codes for 32 and 64 bit.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 include/VBox/VBoxGuest.h                           |  4 +--
 .../Additions/common/VBoxGuest/VBoxDev-haiku.c     |  2 +-
 .../common/VBoxGuest/VBoxGuest-darwin.cpp          |  2 +-
 .../Additions/common/VBoxGuest/VBoxGuest-freebsd.c |  2 +-
 .../common/VBoxGuest/VBoxGuest-haiku-stubs.c       |  4 +--
 .../Additions/common/VBoxGuest/VBoxGuest-haiku.h   |  2 +-
 .../Additions/common/VBoxGuest/VBoxGuest-linux.c   | 35 ++++++++++++++--------
 .../Additions/common/VBoxGuest/VBoxGuest-netbsd.c  |  2 +-
 .../Additions/common/VBoxGuest/VBoxGuest-os2.cpp   |  4 +--
 .../Additions/common/VBoxGuest/VBoxGuest-solaris.c |  2 +-
 .../Additions/common/VBoxGuest/VBoxGuest-win.cpp   |  2 +-
 src/VBox/Additions/common/VBoxGuest/VBoxGuest.cpp  | 16 +++++-----
 .../common/VBoxGuest/VBoxGuestIDC-unix.c.h         |  2 +-
 .../Additions/common/VBoxGuest/VBoxGuestInternal.h |  2 +-
 14 files changed, 47 insertions(+), 34 deletions(-)

diff --git a/include/VBox/VBoxGuest.h b/include/VBox/VBoxGuest.h
index 743fbb3c..5327bb1c 100644
--- a/include/VBox/VBoxGuest.h
+++ b/include/VBox/VBoxGuest.h
@@ -123,9 +123,9 @@
  *          used.
  * @{
  */
-#if defined(RT_ARCH_AMD64) || defined(RT_ARCH_SPARC64)
+#if (defined(RT_ARCH_AMD64) || defined(RT_ARCH_SPARC64)) && !defined(RT_OS_LINUX)
 # define VBOXGUEST_IOCTL_FLAG     128
-#elif defined(RT_ARCH_X86) || defined(RT_ARCH_SPARC)
+#elif defined(RT_ARCH_X86) || defined(RT_ARCH_SPARC) || defined(RT_OS_LINUX)
 # define VBOXGUEST_IOCTL_FLAG     0
 #else
 # error "dunno which arch this is!"
diff --git a/src/VBox/Additions/common/VBoxGuest/VBoxDev-haiku.c b/src/VBox/Additions/common/VBoxGuest/VBoxDev-haiku.c
index 8a664f12..6a240eb6 100644
--- a/src/VBox/Additions/common/VBoxGuest/VBoxDev-haiku.c
+++ b/src/VBox/Additions/common/VBoxGuest/VBoxDev-haiku.c
@@ -236,7 +236,7 @@ static status_t vgdrvHaikuIOCtl(void *cookie, uint32 op, void *data, size_t len)
      * Process the IOCtl.
      */
     size_t cbDataReturned;
-    rc = VGDrvCommonIoCtl(op, &g_DevExt, pSession, pvBuf, len, &cbDataReturned);
+    rc = VGDrvCommonIoCtl(op, &g_DevExt, pSession, pvBuf, len, &cbDataReturned, false);
     if (RT_SUCCESS(rc))
     {
         rc = 0;
diff --git a/src/VBox/Additions/common/VBoxGuest/VBoxGuest-darwin.cpp b/src/VBox/Additions/common/VBoxGuest/VBoxGuest-darwin.cpp
index 794f7793..0cf91423 100644
--- a/src/VBox/Additions/common/VBoxGuest/VBoxGuest-darwin.cpp
+++ b/src/VBox/Additions/common/VBoxGuest/VBoxGuest-darwin.cpp
@@ -543,7 +543,7 @@ static int vgdrvDarwinIOCtlSlow(PVBOXGUESTSESSION pSession, u_long iCmd, caddr_t
      * Process the IOCtl.
      */
     size_t cbReqRet = 0;
-    int rc = VGDrvCommonIoCtl(iCmd, &g_DevExt, pSession, pvReqData, cbReq, &cbReqRet);
+    int rc = VGDrvCommonIoCtl(iCmd, &g_DevExt, pSession, pvReqData, cbReq, &cbReqRet, false);
     if (RT_SUCCESS(rc))
     {
         /*
diff --git a/src/VBox/Additions/common/VBoxGuest/VBoxGuest-freebsd.c b/src/VBox/Additions/common/VBoxGuest/VBoxGuest-freebsd.c
index 75b51a67..d7cc5aee 100644
--- a/src/VBox/Additions/common/VBoxGuest/VBoxGuest-freebsd.c
+++ b/src/VBox/Additions/common/VBoxGuest/VBoxGuest-freebsd.c
@@ -345,7 +345,7 @@ static int vgdrvFreeBSDIOCtl(struct cdev *pDev, u_long ulCmd, caddr_t pvData, in
      * Process the IOCtl.
      */
     size_t cbDataReturned;
-    rc = VGDrvCommonIoCtl(ulCmd, &g_DevExt, pSession, pvBuf, ReqWrap->cbData, &cbDataReturned);
+    rc = VGDrvCommonIoCtl(ulCmd, &g_DevExt, pSession, pvBuf, ReqWrap->cbData, &cbDataReturned, false);
     if (RT_SUCCESS(rc))
     {
         rc = 0;
diff --git a/src/VBox/Additions/common/VBoxGuest/VBoxGuest-haiku-stubs.c b/src/VBox/Additions/common/VBoxGuest/VBoxGuest-haiku-stubs.c
index 1526c10e..d264f3bb 100644
--- a/src/VBox/Additions/common/VBoxGuest/VBoxGuest-haiku-stubs.c
+++ b/src/VBox/Additions/common/VBoxGuest/VBoxGuest-haiku-stubs.c
@@ -346,9 +346,9 @@ RTDECL(int) RTErrConvertToErrno(int iErr)
 {
     return g_VBoxGuest->_RTErrConvertToErrno(iErr);
 }
-int VGDrvCommonIoCtl(unsigned iFunction, PVBOXGUESTDEVEXT pDevExt, PVBOXGUESTSESSION pSession, void *pvData, size_t cbData, size_t *pcbDataReturned)
+int VGDrvCommonIoCtl(unsigned iFunction, PVBOXGUESTDEVEXT pDevExt, PVBOXGUESTSESSION pSession, void *pvData, size_t cbData, size_t *pcbDataReturned, bool f32bit)
 {
-    return g_VBoxGuest->_VGDrvCommonIoCtl(iFunction, pDevExt, pSession, pvData, cbData, pcbDataReturned);
+    return g_VBoxGuest->_VGDrvCommonIoCtl(iFunction, pDevExt, pSession, pvData, cbData, pcbDataReturned, f32bit);
 }
 int VGDrvCommonCreateUserSession(PVBOXGUESTDEVEXT pDevExt, PVBOXGUESTSESSION *ppSession)
 {
diff --git a/src/VBox/Additions/common/VBoxGuest/VBoxGuest-haiku.h b/src/VBox/Additions/common/VBoxGuest/VBoxGuest-haiku.h
index e53b5112..907de537 100644
--- a/src/VBox/Additions/common/VBoxGuest/VBoxGuest-haiku.h
+++ b/src/VBox/Additions/common/VBoxGuest/VBoxGuest-haiku.h
@@ -184,7 +184,7 @@ struct vboxguest_module_info
     PRTLOGGER(*_RTLogRelGetDefaultInstanceEx)(uint32_t fFlagsAndGroup);
     int (*_RTErrConvertToErrno)(int iErr);
     int (*_VGDrvCommonIoCtl)(unsigned iFunction, PVBOXGUESTDEVEXT pDevExt, PVBOXGUESTSESSION pSession,
-                             void *pvData, size_t cbData, size_t *pcbDataReturned);
+                             void *pvData, size_t cbData, size_t *pcbDataReturned, bool f32bit);
     int (*_VGDrvCommonCreateUserSession)(PVBOXGUESTDEVEXT pDevExt, PVBOXGUESTSESSION *ppSession);
     void (*_VGDrvCommonCloseSession)(PVBOXGUESTDEVEXT pDevExt, PVBOXGUESTSESSION pSession);
     void* (*_VBoxGuestIDCOpen)(uint32_t *pu32Version);
diff --git a/src/VBox/Additions/common/VBoxGuest/VBoxGuest-linux.c b/src/VBox/Additions/common/VBoxGuest/VBoxGuest-linux.c
index 1ba3ba64..73717ebb 100644
--- a/src/VBox/Additions/common/VBoxGuest/VBoxGuest-linux.c
+++ b/src/VBox/Additions/common/VBoxGuest/VBoxGuest-linux.c
@@ -78,11 +78,7 @@ static int  vgdrvLinuxModInit(void);
 static void vgdrvLinuxModExit(void);
 static int  vgdrvLinuxOpen(struct inode *pInode, struct file *pFilp);
 static int  vgdrvLinuxRelease(struct inode *pInode, struct file *pFilp);
-#ifdef HAVE_UNLOCKED_IOCTL
-static long vgdrvLinuxIOCtl(struct file *pFilp, unsigned int uCmd, unsigned long ulArg);
-#else
-static int  vgdrvLinuxIOCtl(struct inode *pInode, struct file *pFilp, unsigned int uCmd, unsigned long ulArg);
-#endif
+static long vgdrvLinuxIOCtlImpl(struct file *pFilp, unsigned int uCmd, unsigned long ulArg, bool f23bit);
 static int  vgdrvLinuxFAsync(int fd, struct file *pFile, int fOn);
 static unsigned int vgdrvLinuxPoll(struct file *pFile, poll_table *pPt);
 static ssize_t vgdrvLinuxRead(struct file *pFile, char *pbBuf, size_t cbRead, loff_t *poff);
@@ -138,6 +134,22 @@ static char                     g_szDbgLogDst[128];
 static struct input_dev        *g_pInputDevice = NULL;
 #endif
 
+#ifdef HAVE_UNLOCKED_IOCTL
+static long vgdrvLinuxIOCtl(struct file *pFilp, unsigned int uCmd, unsigned long ulArg)
+#else
+static int vgdrvLinuxIOCtl(struct inode *pInode, struct file *pFilp, unsigned int uCmd, unsigned long ulArg)
+#endif
+{
+	return vgdrvLinuxIOCtlImpl(pFilp, uCmd, ulArg, false);
+}
+
+#ifdef CONFIG_COMPAT
+static long vgdrvLinuxIOCtlCompat(struct file *pFilp, unsigned int uCmd, unsigned long ulArg)
+{
+	return vgdrvLinuxIOCtlImpl(pFilp, uCmd, ulArg, true);
+}
+#endif
+
 /** The file_operations structure. */
 static struct file_operations   g_FileOps =
 {
@@ -149,6 +161,9 @@ static struct file_operations   g_FileOps =
 #else
     ioctl:          vgdrvLinuxIOCtl,
 #endif
+#ifdef CONFIG_COMPAT
+    compat_ioctl:   vgdrvLinuxIOCtlCompat,
+#endif
     fasync:         vgdrvLinuxFAsync,
     read:           vgdrvLinuxRead,
     poll:           vgdrvLinuxPoll,
@@ -395,7 +410,7 @@ static void vgdrvLinuxTermISR(void)
  */
 static int vgdrvLinuxSetMouseStatus(uint32_t fStatus)
 {
-    return VGDrvCommonIoCtl(VBOXGUEST_IOCTL_SET_MOUSE_STATUS, &g_DevExt, g_pKernelSession, &fStatus, sizeof(fStatus), NULL);
+    return VGDrvCommonIoCtl(VBOXGUEST_IOCTL_SET_MOUSE_STATUS, &g_DevExt, g_pKernelSession, &fStatus, sizeof(fStatus), NULL, false);
 }
 
 
@@ -754,11 +769,7 @@ static int vgdrvLinuxRelease(struct inode *pInode, struct file *pFilp)
  * @param   uCmd        The function specified to ioctl().
  * @param   ulArg       The argument specified to ioctl().
  */
-#ifdef HAVE_UNLOCKED_IOCTL
-static long vgdrvLinuxIOCtl(struct file *pFilp, unsigned int uCmd, unsigned long ulArg)
-#else
-static int vgdrvLinuxIOCtl(struct inode *pInode, struct file *pFilp, unsigned int uCmd, unsigned long ulArg)
-#endif
+static long vgdrvLinuxIOCtlImpl(struct file *pFilp, unsigned int uCmd, unsigned long ulArg, bool f32bit)
 {
     PVBOXGUESTSESSION   pSession = (PVBOXGUESTSESSION)pFilp->private_data;
     uint32_t            cbData   = _IOC_SIZE(uCmd);
@@ -792,7 +803,7 @@ static int vgdrvLinuxIOCtl(struct inode *pInode, struct file *pFilp, unsigned in
          * Process the IOCtl.
          */
         size_t cbDataReturned;
-        rc = VGDrvCommonIoCtl(uCmd, &g_DevExt, pSession, pvBuf, cbData, &cbDataReturned);
+        rc = VGDrvCommonIoCtl(uCmd, &g_DevExt, pSession, pvBuf, cbData, &cbDataReturned, f32bit);
 
         /*
          * Copy ioctl data and output buffer back to user space.
diff --git a/src/VBox/Additions/common/VBoxGuest/VBoxGuest-netbsd.c b/src/VBox/Additions/common/VBoxGuest/VBoxGuest-netbsd.c
index 42f48dd9..c2a9cb08 100644
--- a/src/VBox/Additions/common/VBoxGuest/VBoxGuest-netbsd.c
+++ b/src/VBox/Additions/common/VBoxGuest/VBoxGuest-netbsd.c
@@ -317,7 +317,7 @@ static int VBoxGuestNetBSDIOCtl(struct file *fp, u_long command, void *data)
      * Process the IOCtl.
      */
     size_t cbDataReturned;
-    rc = VGDrvCommonIoCtl(command, &g_DevExt, session->session, pvBuf, ReqWrap->cbData, &cbDataReturned);
+    rc = VGDrvCommonIoCtl(command, &g_DevExt, session->session, pvBuf, ReqWrap->cbData, &cbDataReturned, false);
     if (RT_SUCCESS(rc))
     {
         rc = 0;
diff --git a/src/VBox/Additions/common/VBoxGuest/VBoxGuest-os2.cpp b/src/VBox/Additions/common/VBoxGuest/VBoxGuest-os2.cpp
index 991ffdf6..ecbefbef 100644
--- a/src/VBox/Additions/common/VBoxGuest/VBoxGuest-os2.cpp
+++ b/src/VBox/Additions/common/VBoxGuest/VBoxGuest-os2.cpp
@@ -486,7 +486,7 @@ DECLASM(int) VGDrvOS2IDCService(uint32_t u32Session, unsigned iFunction, void *p
     switch (iFunction)
     {
         default:
-            rc = VGDrvCommonIoCtl(iFunction, &g_DevExt, pSession, pvData, cbData, pcbDataReturned);
+            rc = VGDrvCommonIoCtl(iFunction, &g_DevExt, pSession, pvData, cbData, pcbDataReturned, false);
             break;
 
         case VBOXGUEST_IOCTL_OS2_IDC_DISCONNECT:
@@ -590,7 +590,7 @@ DECLASM(int) vgdrvOS2IOCtl(uint16_t sfn, uint8_t iCat, uint8_t iFunction, void *
          * Process the IOCtl.
          */
         size_t cbDataReturned;
-        rc = VGDrvCommonIoCtl(iFunction, &g_DevExt, pSession, pvParm, *pcbParm, &cbDataReturned);
+        rc = VGDrvCommonIoCtl(iFunction, &g_DevExt, pSession, pvParm, *pcbParm, &cbDataReturned, false);
 
         /*
          * Unlock the buffers.
diff --git a/src/VBox/Additions/common/VBoxGuest/VBoxGuest-solaris.c b/src/VBox/Additions/common/VBoxGuest/VBoxGuest-solaris.c
index 5052af68..ca2e83fc 100644
--- a/src/VBox/Additions/common/VBoxGuest/VBoxGuest-solaris.c
+++ b/src/VBox/Additions/common/VBoxGuest/VBoxGuest-solaris.c
@@ -735,7 +735,7 @@ static int vgdrvSolarisIOCtl(dev_t Dev, int Cmd, intptr_t pArg, int Mode, cred_t
      * Process the IOCtl.
      */
     size_t cbDataReturned = 0;
-    rc = VGDrvCommonIoCtl(Cmd, &g_DevExt, pSession, pvBuf, ReqWrap.cbData, &cbDataReturned);
+    rc = VGDrvCommonIoCtl(Cmd, &g_DevExt, pSession, pvBuf, ReqWrap.cbData, &cbDataReturned, false);
     if (RT_SUCCESS(rc))
     {
         rc = 0;
diff --git a/src/VBox/Additions/common/VBoxGuest/VBoxGuest-win.cpp b/src/VBox/Additions/common/VBoxGuest/VBoxGuest-win.cpp
index 2f2401aa..0fb3e00d 100644
--- a/src/VBox/Additions/common/VBoxGuest/VBoxGuest-win.cpp
+++ b/src/VBox/Additions/common/VBoxGuest/VBoxGuest-win.cpp
@@ -788,7 +788,7 @@ static NTSTATUS vgdrvNtIOCtl(PDEVICE_OBJECT pDevObj, PIRP pIrp)
          * Process the common IOCtls.
          */
         size_t cbDataReturned;
-        int vrc = VGDrvCommonIoCtl(uCmd, &pDevExt->Core, pSession, pBuf, cbData, &cbDataReturned);
+        int vrc = VGDrvCommonIoCtl(uCmd, &pDevExt->Core, pSession, pBuf, cbData, &cbDataReturned, false);
 
         LogFlowFunc(("rc=%Rrc, pBuf=0x%p, cbData=%u, cbDataReturned=%u\n",
                      vrc, pBuf, cbData, cbDataReturned));
diff --git a/src/VBox/Additions/common/VBoxGuest/VBoxGuest.cpp b/src/VBox/Additions/common/VBoxGuest/VBoxGuest.cpp
index a13874a2..56aac366 100644
--- a/src/VBox/Additions/common/VBoxGuest/VBoxGuest.cpp
+++ b/src/VBox/Additions/common/VBoxGuest/VBoxGuest.cpp
@@ -3374,9 +3374,11 @@ static int vgdrvIoCtl_SetCapabilities(PVBOXGUESTDEVEXT pDevExt, PVBOXGUESTSESSIO
  * @param   pvData              The input/output data buffer. Can be NULL depending on the function.
  * @param   cbData              The max size of the data buffer.
  * @param   pcbDataReturned     Where to store the amount of returned data. Can be NULL.
+ * @param   f32bit              Is this a 32 bit compat call? Note when false the iFunction
+ *                              itself may still indicate this is a 32 bit compat call.
  */
 int VGDrvCommonIoCtl(unsigned iFunction, PVBOXGUESTDEVEXT pDevExt, PVBOXGUESTSESSION pSession,
-                     void *pvData, size_t cbData, size_t *pcbDataReturned)
+                     void *pvData, size_t cbData, size_t *pcbDataReturned, bool f32bit)
 {
     int rc;
     LogFlow(("VGDrvCommonIoCtl: iFunction=%#x pDevExt=%p pSession=%p pvData=%p cbData=%zu\n",
@@ -3448,7 +3450,7 @@ int VGDrvCommonIoCtl(unsigned iFunction, PVBOXGUESTDEVEXT pDevExt, PVBOXGUESTSES
         bool fInterruptible = pSession->R0Process != NIL_RTR0PROCESS;
         CHECKRET_MIN_SIZE("HGCM_CALL", sizeof(VBoxGuestHGCMCallInfo));
         rc = vgdrvIoCtl_HGCMCall(pDevExt, pSession, (VBoxGuestHGCMCallInfo *)pvData, RT_INDEFINITE_WAIT,
-                                 fInterruptible, false /*f32bit*/, false /* fUserData */,
+                                 fInterruptible, f32bit, false /* fUserData */,
                                  0, cbData, pcbDataReturned);
     }
     else if (VBOXGUEST_IOCTL_STRIP_SIZE(iFunction) == VBOXGUEST_IOCTL_STRIP_SIZE(VBOXGUEST_IOCTL_HGCM_CALL_TIMED(0)))
@@ -3457,7 +3459,7 @@ int VGDrvCommonIoCtl(unsigned iFunction, PVBOXGUESTDEVEXT pDevExt, PVBOXGUESTSES
         CHECKRET_MIN_SIZE("HGCM_CALL_TIMED", sizeof(VBoxGuestHGCMCallInfoTimed));
         rc = vgdrvIoCtl_HGCMCall(pDevExt, pSession, &pInfo->info, pInfo->u32Timeout,
                                  !!pInfo->fInterruptible || pSession->R0Process != NIL_RTR0PROCESS,
-                                 false /*f32bit*/, false /* fUserData */,
+                                 f32bit, false /* fUserData */,
                                  RT_OFFSETOF(VBoxGuestHGCMCallInfoTimed, info), cbData, pcbDataReturned);
     }
     else if (VBOXGUEST_IOCTL_STRIP_SIZE(iFunction) == VBOXGUEST_IOCTL_STRIP_SIZE(VBOXGUEST_IOCTL_HGCM_CALL_USERDATA(0)))
@@ -3465,10 +3467,10 @@ int VGDrvCommonIoCtl(unsigned iFunction, PVBOXGUESTDEVEXT pDevExt, PVBOXGUESTSES
         bool fInterruptible = true;
         CHECKRET_MIN_SIZE("HGCM_CALL", sizeof(VBoxGuestHGCMCallInfo));
         rc = vgdrvIoCtl_HGCMCall(pDevExt, pSession, (VBoxGuestHGCMCallInfo *)pvData, RT_INDEFINITE_WAIT,
-                                 fInterruptible, false /*f32bit*/, true /* fUserData */,
+                                 fInterruptible, f32bit, true /* fUserData */,
                                  0, cbData, pcbDataReturned);
     }
-# ifdef RT_ARCH_AMD64
+# if defined RT_ARCH_AMD64 && !defined(RT_OS_LINUX)
     else if (VBOXGUEST_IOCTL_STRIP_SIZE(iFunction) == VBOXGUEST_IOCTL_STRIP_SIZE(VBOXGUEST_IOCTL_HGCM_CALL_32(0)))
     {
         bool fInterruptible = pSession->R0Process != NIL_RTR0PROCESS;
@@ -3529,7 +3531,7 @@ int VGDrvCommonIoCtl(unsigned iFunction, PVBOXGUESTDEVEXT pDevExt, PVBOXGUESTSES
 
 #ifdef VBOX_WITH_HGCM
             case VBOXGUEST_IOCTL_HGCM_CONNECT:
-# ifdef RT_ARCH_AMD64
+# if defined RT_ARCH_AMD64 && !defined(RT_OS_LINUX)
             case VBOXGUEST_IOCTL_HGCM_CONNECT_32:
 # endif
                 CHECKRET_MIN_SIZE("HGCM_CONNECT", sizeof(VBoxGuestHGCMConnectInfo));
@@ -3537,7 +3539,7 @@ int VGDrvCommonIoCtl(unsigned iFunction, PVBOXGUESTDEVEXT pDevExt, PVBOXGUESTSES
                 break;
 
             case VBOXGUEST_IOCTL_HGCM_DISCONNECT:
-# ifdef RT_ARCH_AMD64
+# if defined RT_ARCH_AMD64 && !defined(RT_OS_LINUX)
             case VBOXGUEST_IOCTL_HGCM_DISCONNECT_32:
 # endif
                 CHECKRET_MIN_SIZE("HGCM_DISCONNECT", sizeof(VBoxGuestHGCMDisconnectInfo));
diff --git a/src/VBox/Additions/common/VBoxGuest/VBoxGuestIDC-unix.c.h b/src/VBox/Additions/common/VBoxGuest/VBoxGuestIDC-unix.c.h
index 7ce136f3..030f7554 100644
--- a/src/VBox/Additions/common/VBoxGuest/VBoxGuestIDC-unix.c.h
+++ b/src/VBox/Additions/common/VBoxGuest/VBoxGuestIDC-unix.c.h
@@ -142,6 +142,6 @@ DECLEXPORT(int) VBOXCALL VBoxGuestIDCCall(void *pvSession, unsigned iCmd, void *
     AssertPtrReturn(pSession, VERR_INVALID_POINTER);
     AssertMsgReturn(pSession->pDevExt == &g_DevExt, ("SC: %p != %p\n", pSession->pDevExt, &g_DevExt), VERR_INVALID_HANDLE);
 
-    return VGDrvCommonIoCtl(iCmd, &g_DevExt, pSession, pvData, cbData, pcbDataReturned);
+    return VGDrvCommonIoCtl(iCmd, &g_DevExt, pSession, pvData, cbData, pcbDataReturned, false);
 }
 
diff --git a/src/VBox/Additions/common/VBoxGuest/VBoxGuestInternal.h b/src/VBox/Additions/common/VBoxGuest/VBoxGuestInternal.h
index 319f6c96..763691ee 100644
--- a/src/VBox/Additions/common/VBoxGuest/VBoxGuestInternal.h
+++ b/src/VBox/Additions/common/VBoxGuest/VBoxGuestInternal.h
@@ -336,7 +336,7 @@ void VGDrvCommonCloseSession(PVBOXGUESTDEVEXT pDevExt, PVBOXGUESTSESSION pSessio
 
 int  VGDrvCommonIoCtlFast(unsigned iFunction, PVBOXGUESTDEVEXT pDevExt, PVBOXGUESTSESSION pSession);
 int  VGDrvCommonIoCtl(unsigned iFunction, PVBOXGUESTDEVEXT pDevExt, PVBOXGUESTSESSION pSession,
-                      void *pvData, size_t cbData, size_t *pcbDataReturned);
+                      void *pvData, size_t cbData, size_t *pcbDataReturned, bool f32bit);
 
 /**
  * ISR callback for notifying threads polling for mouse events.
-- 
2.13.4

Re: [vbox-dev] [RFC] VGDrvCommonIoCtl: Add f32bit flag argument

[Hans de Goede]

Hi,

On 14-08-17 13:43, Knut St. Osmundsen wrote:
> Hi Hans,
> 
> the other platforms also have KPIs or similar constructs for figuring
> out whether the client process issuing the I/O controls is a 32-bit or
> 64-bit one.  However, using the VBOXGUEST_IOCTL_FLAG set to 0 or 0x80 if
> 32-bit or 64-bit was a less complex (+faster(+safer)) way of deal with
> this.  The host driver does this as well.  I would like the structure of
> the two to be as similar as possible.
> 
> I'm not very keen to adding linux specific clutter (f32Bit + #ifndef
> RT_OS_LINUX) to the common code just because you can do it differently
> only Linux.  Sorry.  Want minimal platform specific cruft in common
> code.  Hope you understand.

OK and yes I understand.

> PS. I noticed in the Linux kernel RFC email thread that we've agreed to
> freeze the I/O control ABI.  We cannot guarantee that's it's 100% frozen
> at this point, since the generic status code fix (getting rid of that
> ioctl non-zero return value) hasn't been done yet.  I will see if I can
> squeeze it in later this week.

I had already decided to just live with the positive return for vbox
host status codes, but if you want this changed for other reasons, then yes
now would be the time to do that. But you don't have to do it just on
my account.

Regards,

Hans



> 
> 
> On 2017-08-14 9:30 AM, Hans de Goede wrote:
>> On 14-08-17 09:27, Hans de Goede wrote:
>>> Note to linux-kernel readers: This Cc-ed to linux-kernel because it is
>>> relevant for the "[RFC 0/2] Add Virtual Box vboxguest and vboxsf guest
>>> drivers to the mainline kernel" thread.
>>>
>>> Hi Michael, Knut,
>>>
>>> My first submission of the vboxguest driver for inclusion into
>>> the Linux kernel has lead to some questions about the use of the
>>> VBOXGUEST_IOCTL_FLAG to differentiate between 32 and 64 bit
>>> ioctls. Under Linux this is not necessary, as the driver already
>>> knows if it is serving a 32 bit compat or a regular ioctl.
>>>
>>> So I've come up with this patch to make VBOXGUEST_IOCTL_FLAG
>>> always 0 under Linux. I was hoping the f32bit flag could be
>>> used under more platforms so that it would actually be a cleanup,
>>> but it seems that Linux is the only platform with a compat_ioctl
>>> callback in its file-operations struct.
>>>
>>> Please let me know if you would be willing to merge this patch
>>> into upstream VirtualBox.
>>
>> p.s.
>>
>> I've only tested this patch with Linux!
>> _______________________________________________
>> vbox-dev mailing list
>> vbox-dev@virtualbox.org
>> https://www.virtualbox.org/mailman/listinfo/vbox-dev
> 
> 
> _______________________________________________
> vbox-dev mailing list
> vbox-dev@virtualbox.org
> https://www.virtualbox.org/mailman/listinfo/vbox-dev
> 

Modifying the vboxguest ioctl API (Was: Re: [vbox-dev] [RFC] VGDrvCommonIoCtl: Add f32bit flag argument)

[Hans de Goede]

Hi,

On 14-08-17 14:19, Hans de Goede wrote:
> Hi,
> 
> On 14-08-17 13:43, Knut St. Osmundsen wrote:
>> Hi Hans,
>>
>> the other platforms also have KPIs or similar constructs for figuring
>> out whether the client process issuing the I/O controls is a 32-bit or
>> 64-bit one.  However, using the VBOXGUEST_IOCTL_FLAG set to 0 or 0x80 if
>> 32-bit or 64-bit was a less complex (+faster(+safer)) way of deal with
>> this.  The host driver does this as well.  I would like the structure of
>> the two to be as similar as possible.
>>
>> I'm not very keen to adding linux specific clutter (f32Bit + #ifndef
>> RT_OS_LINUX) to the common code just because you can do it differently
>> only Linux.  Sorry.  Want minimal platform specific cruft in common
>> code.  Hope you understand.
> 
> OK and yes I understand.
> 
>> PS. I noticed in the Linux kernel RFC email thread that we've agreed to
>> freeze the I/O control ABI.  We cannot guarantee that's it's 100% frozen
>> at this point, since the generic status code fix (getting rid of that
>> ioctl non-zero return value) hasn't been done yet.  I will see if I can
>> squeeze it in later this week.
> 
> I had already decided to just live with the positive return for vbox
> host status codes, but if you want this changed for other reasons, then yes
> now would be the time to do that. But you don't have to do it just on
> my account.

I plan to post a v2 of the vboxguest driver for upstream tomorrow. Given
the above I will mark it as a RFC for now. Anychance you can wrap this up
soonish so that I can post a non RFC version with the final ABI upstream ?

Regards,

Hans