13.07.2017 16:42, Michael Thayer wrote: > 10.07.2017 12:29, Hans de Goede wrote: > [Patch to shared folder code proposed by Hans to reduce the line count > of the Linux driver.] > > I reviewed the patch and made a few changes. Not yet tested on any > platform, but I am posting my adjusted version so that other people can > test, including on FreeBSD and Haiku which I cannot do easily. Note > that this applies to the normal VirtualBox tree, not to the packaged > Linux driver. I redid the patch again, after realising that the non-physical-page path was for supporting VirtualBox 3.0 and older on the host, which we have not tested or supported for a long time. Therefore VbglR0CanUsePhysPageList() could be removed altogether. Hopefully this code will not affect other platforms at all, though testing is better than hoping (that is addressed particularly to community port maintainers).
Hans, I kept your signed-off-by despite my changes, I hope that is fine. Thanks again. The changes I committed are also attached to this message. Regards Michael -- Michael Thayer | VirtualBox engineer ORACLE Deutschland B.V. & Co. KG | Werkstr. 24 | D-71384 Weinstadt ORACLE Deutschland B.V. & Co. KG Hauptverwaltung: Riesstraße 25, D-80992 München Registergericht: Amtsgericht München, HRA 95603 Komplementärin: ORACLE Deutschland Verwaltung B.V. Hertogswetering 163/167, 3543 AS Utrecht, Niederlande Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697 Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher
>From 21bd2fc1249c1278cae08c4013f2d8e6748306f6 Mon Sep 17 00:00:00 2001 From: Michael Thayer <[email protected]> Date: Mon, 24 Jul 2017 20:26:03 +0200 Subject: [PATCH 1/5] Shared folders: stop supporting legacy host code on Linux, add Windows todo. bugref:8524: Additions/linux: play nicely with distribution-installed Additions Using physical page lists in HGCM was added in VirtualBox 3.1, and the Linux and Windows shared folder Additions still have code paths for supporting older versions. Remove this on Linux and add a todo to Windows to remove it. This removes all shared folder driver dependencies on the guest library other than HGCM. Signed-off-by: Hans de Goede <[email protected]> Reworked by Oracle. --- src/VBox/Additions/WINNT/SharedFolders/driver/file.c | 3 +++ src/VBox/Additions/linux/sharedfolders/regops.c | 12 ++---------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/VBox/Additions/WINNT/SharedFolders/driver/file.c b/src/VBox/Additions/WINNT/SharedFolders/driver/file.c index f42c0ca14f..c3ca43ec55 100644 --- a/src/VBox/Additions/WINNT/SharedFolders/driver/file.c +++ b/src/VBox/Additions/WINNT/SharedFolders/driver/file.c @@ -91,6 +91,9 @@ static int vbsfTransferCommon(VBSFTRANSFERCTX *pCtx) uint32_t cbToTransfer; uint32_t cbIO; + /** @todo Remove the test and the fall-back path. VbglR0CanUsePhysPageList() + * returns true for any host version after 3.0, i.e. further back than + * we support. */ if (VbglR0CanUsePhysPageList()) { ULONG offFirstPage = MmGetMdlByteOffset(pCtx->pMdl); diff --git a/src/VBox/Additions/linux/sharedfolders/regops.c b/src/VBox/Additions/linux/sharedfolders/regops.c index d411228a9d..3aec9f2cbd 100644 --- a/src/VBox/Additions/linux/sharedfolders/regops.c +++ b/src/VBox/Additions/linux/sharedfolders/regops.c @@ -232,16 +232,8 @@ static ssize_t sf_reg_write(struct file *file, const char *buf, size_t size, lof goto fail; } -#if 1 - if (VbglR0CanUsePhysPageList()) - { - err = VbglR0SfWritePhysCont(&client_handle, &sf_g->map, sf_r->handle, - pos, &nwritten, tmp_phys); - err = RT_FAILURE(err) ? -EPROTO : 0; - } - else -#endif - err = sf_reg_write_aux(__func__, sf_g, sf_r, tmp, &nwritten, pos); + err = VbglR0SfWritePhysCont(&client_handle, &sf_g->map, sf_r->handle, + pos, &nwritten, tmp_phys); if (err) goto fail; -- 2.11.0
>From c49fa98e0eadf77962587dee89dd4301edae4e95 Mon Sep 17 00:00:00 2001 From: Michael Thayer <[email protected]> Date: Mon, 24 Jul 2017 21:03:54 +0200 Subject: [PATCH 2/5] R0 guest library: move HGCM globals out of global library structure. bugref:8524: Additions/linux: play nicely with distribution-installed Additions As part of elimitating dependencies of the HGCM R0 guest library code on the rest of the library, move its global variables (handles and mutex) out of the global library structure and into HGCM.cpp. The aim is to reduce the number of library files included in the Linux shared folder driver. Signed-off-by: Hans de Goede <[email protected]> Reworked by Oracle. --- src/VBox/Additions/common/VBoxGuestLib/HGCM.cpp | 28 +++++++++++++++------- .../Additions/common/VBoxGuestLib/VBGLInternal.h | 11 --------- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/VBox/Additions/common/VBoxGuestLib/HGCM.cpp b/src/VBox/Additions/common/VBoxGuestLib/HGCM.cpp index 6490abfa49..e5c8d03055 100644 --- a/src/VBox/Additions/common/VBoxGuestLib/HGCM.cpp +++ b/src/VBox/Additions/common/VBoxGuestLib/HGCM.cpp @@ -39,13 +39,25 @@ #define VBGL_HGCM_ASSERT_MSG AssertReleaseMsg /** + * Fast heap for HGCM handles data. + * @{ + */ + +static RTSEMFASTMUTEX mutexHGCMHandle; + +static struct VBGLHGCMHANDLEDATA aHGCMHandleData[64]; + +/** @} */ + +/** * Initializes the HGCM VBGL bits. * * @return VBox status code. */ int vbglR0HGCMInit(void) { - return RTSemFastMutexCreate(&g_vbgldata.mutexHGCMHandle); + AssertReturn(mutexHGCMHandle == NIL_RTSEMFASTMUTEX, VINF_ALREADY_INITIALIZED); + return RTSemFastMutexCreate(&mutexHGCMHandle); } /** @@ -55,15 +67,15 @@ int vbglR0HGCMInit(void) */ int vbglR0HGCMTerminate(void) { - RTSemFastMutexDestroy(g_vbgldata.mutexHGCMHandle); - g_vbgldata.mutexHGCMHandle = NIL_RTSEMFASTMUTEX; + RTSemFastMutexDestroy(mutexHGCMHandle); + mutexHGCMHandle = NIL_RTSEMFASTMUTEX; return VINF_SUCCESS; } DECLINLINE(int) vbglHandleHeapEnter(void) { - int rc = RTSemFastMutexRequest(g_vbgldata.mutexHGCMHandle); + int rc = RTSemFastMutexRequest(mutexHGCMHandle); VBGL_HGCM_ASSERT_MSG(RT_SUCCESS(rc), ("Failed to request handle heap mutex, rc = %Rrc\n", rc)); @@ -72,7 +84,7 @@ DECLINLINE(int) vbglHandleHeapEnter(void) DECLINLINE(void) vbglHandleHeapLeave(void) { - RTSemFastMutexRelease(g_vbgldata.mutexHGCMHandle); + RTSemFastMutexRelease(mutexHGCMHandle); } struct VBGLHGCMHANDLEDATA *vbglHGCMHandleAlloc(void) @@ -85,11 +97,11 @@ struct VBGLHGCMHANDLEDATA *vbglHGCMHandleAlloc(void) /* Simple linear search in array. This will be called not so often, only connect/disconnect. */ /** @todo bitmap for faster search and other obvious optimizations. */ - for (i = 0; i < RT_ELEMENTS(g_vbgldata.aHGCMHandleData); i++) + for (i = 0; i < RT_ELEMENTS(aHGCMHandleData); i++) { - if (!g_vbgldata.aHGCMHandleData[i].fAllocated) + if (!aHGCMHandleData[i].fAllocated) { - p = &g_vbgldata.aHGCMHandleData[i]; + p = &aHGCMHandleData[i]; p->fAllocated = 1; break; } diff --git a/src/VBox/Additions/common/VBoxGuestLib/VBGLInternal.h b/src/VBox/Additions/common/VBoxGuestLib/VBGLInternal.h index df78a1c70b..8c6323cbd5 100644 --- a/src/VBox/Additions/common/VBoxGuestLib/VBGLInternal.h +++ b/src/VBox/Additions/common/VBoxGuestLib/VBGLInternal.h @@ -107,17 +107,6 @@ typedef struct VBGLDATA VBGLDRIVER driver; /** @} */ - - /** - * Fast heap for HGCM handles data. - * @{ - */ - - RTSEMFASTMUTEX mutexHGCMHandle; - - struct VBGLHGCMHANDLEDATA aHGCMHandleData[64]; - - /** @} */ #endif } VBGLDATA; -- 2.11.0
>From cd6b0497fbb259fdf449a582248db9863017edac Mon Sep 17 00:00:00 2001 From: Michael Thayer <[email protected]> Date: Mon, 24 Jul 2017 21:12:55 +0200 Subject: [PATCH 3/5] R0 guest library: make vbglR0HGCMInit and Terminate externally visible. bugref:8524: Additions/linux: play nicely with distribution-installed Additions So that the Linux shared folder code can call vbglR0HGCMInit and Terminate directly in future without neediing to call the guest library initialisation, make these symbols externally visible and make the first letter of the names capital to reflect this. Signed-off-by: Hans de Goede <[email protected]> Reworked by Oracle. --- include/VBox/VBoxGuestLib.h | 15 +++++++++++++++ src/VBox/Additions/common/VBoxGuestLib/HGCM.cpp | 4 ++-- src/VBox/Additions/common/VBoxGuestLib/Init.cpp | 4 ++-- src/VBox/Additions/common/VBoxGuestLib/VBGLInternal.h | 4 ---- 4 files changed, 19 insertions(+), 8 deletions(-) diff --git a/include/VBox/VBoxGuestLib.h b/include/VBox/VBoxGuestLib.h index 8106d66dc0..85fade6cd7 100644 --- a/include/VBox/VBoxGuestLib.h +++ b/include/VBox/VBoxGuestLib.h @@ -287,6 +287,21 @@ typedef struct VBGLHGCMHANDLEDATA *VBGLHGCMHANDLE; */ /** + * Initializes HGCM in the R0 guest library. Must be called before any HGCM + * connections are made. Is called by VbglInitClient(). + * + * @return VBox status code. + */ +DECLVBGL(int) VbglR0HGCMInit(void); + +/** + * Terminates HGCM in the R0 guest library. Is called by VbglTerminate(). + * + * @return VBox status code. + */ +DECLVBGL(int) VbglR0HGCMTerminate(void); + +/** * Connect to a service. * * @param pHandle Pointer to variable that will hold a handle to be used diff --git a/src/VBox/Additions/common/VBoxGuestLib/HGCM.cpp b/src/VBox/Additions/common/VBoxGuestLib/HGCM.cpp index e5c8d03055..593f5a9fa5 100644 --- a/src/VBox/Additions/common/VBoxGuestLib/HGCM.cpp +++ b/src/VBox/Additions/common/VBoxGuestLib/HGCM.cpp @@ -54,7 +54,7 @@ static struct VBGLHGCMHANDLEDATA aHGCMHandleData[64]; * * @return VBox status code. */ -int vbglR0HGCMInit(void) +DECLVBGL(int) VbglR0HGCMInit(void) { AssertReturn(mutexHGCMHandle == NIL_RTSEMFASTMUTEX, VINF_ALREADY_INITIALIZED); return RTSemFastMutexCreate(&mutexHGCMHandle); @@ -65,7 +65,7 @@ int vbglR0HGCMInit(void) * * @return VBox status code. */ -int vbglR0HGCMTerminate(void) +DECLVBGL(int) VbglR0HGCMTerminate(void) { RTSemFastMutexDestroy(mutexHGCMHandle); mutexHGCMHandle = NIL_RTSEMFASTMUTEX; diff --git a/src/VBox/Additions/common/VBoxGuestLib/Init.cpp b/src/VBox/Additions/common/VBoxGuestLib/Init.cpp index 55877cda1f..fbd6b81f8b 100644 --- a/src/VBox/Additions/common/VBoxGuestLib/Init.cpp +++ b/src/VBox/Additions/common/VBoxGuestLib/Init.cpp @@ -254,7 +254,7 @@ DECLVBGL(int) VbglInitClient(void) vbglQueryDriverInfo (); # ifdef VBOX_WITH_HGCM - rc = vbglR0HGCMInit (); + rc = VbglR0HGCMInit (); # endif /* VBOX_WITH_HGCM */ if (RT_FAILURE(rc)) @@ -277,7 +277,7 @@ DECLVBGL(int) VbglInitClient(void) DECLVBGL(void) VbglTerminate (void) { # ifdef VBOX_WITH_HGCM - vbglR0HGCMTerminate (); + VbglR0HGCMTerminate (); # endif /* driver open could fail, which does not prevent VbglInit from succeeding, diff --git a/src/VBox/Additions/common/VBoxGuestLib/VBGLInternal.h b/src/VBox/Additions/common/VBoxGuestLib/VBGLInternal.h index 8c6323cbd5..5fa38abd33 100644 --- a/src/VBox/Additions/common/VBoxGuestLib/VBGLInternal.h +++ b/src/VBox/Additions/common/VBoxGuestLib/VBGLInternal.h @@ -138,10 +138,6 @@ extern VBGLDATA g_vbgldata; int vbglR0Enter (void); #ifdef VBOX_WITH_HGCM -# ifndef VBGL_VBOXGUEST -int vbglR0HGCMInit(void); -int vbglR0HGCMTerminate(void); -# endif struct VBGLHGCMHANDLEDATA *vbglHGCMHandleAlloc(void); void vbglHGCMHandleFree(struct VBGLHGCMHANDLEDATA *pHandle); #endif /* VBOX_WITH_HGCM */ -- 2.11.0
>From 2a3a9e161b7bbf79d4b1a4cfd4bb48ee0cc1b82b Mon Sep 17 00:00:00 2001 From: Michael Thayer <[email protected]> Date: Mon, 24 Jul 2017 21:27:06 +0200 Subject: [PATCH 4/5] Linux shared folder driver: stop using VbglR0SfInit and Term. bugref:8524: Additions/linux: play nicely with distribution-installed Additions Stop using the VbglR0SfInit and Term symbols in the Linux shared folders guest driver and use VbglR0HGCMInit and Terminate instead. Remove the former symbols from the guest library altogether on Linux. After this, the Linux shared folder driver only depends on the HGCM code in the guest library. Signed-off-by: Hans de Goede <[email protected]> Reworked by Oracle. --- .../Additions/common/VBoxGuestLib/VBoxGuestR0LibSharedFolders.c | 5 +++++ src/VBox/Additions/linux/sharedfolders/vfsmod.c | 9 +++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/VBox/Additions/common/VBoxGuestLib/VBoxGuestR0LibSharedFolders.c b/src/VBox/Additions/common/VBoxGuestLib/VBoxGuestR0LibSharedFolders.c index 9207c35877..befd58a7c2 100644 --- a/src/VBox/Additions/common/VBoxGuestLib/VBoxGuestR0LibSharedFolders.c +++ b/src/VBox/Additions/common/VBoxGuestLib/VBoxGuestR0LibSharedFolders.c @@ -55,6 +55,10 @@ +/** @todo We only need HGCM, not physical memory, so other guests should also + * switch to calling vbglR0HGCMInit() and vbglR0HGCMTerminate() instead + * of VbglR0SfInit() and VbglR0SfTerm(). */ +#ifndef RT_OS_LINUX DECLVBGL(int) VbglR0SfInit(void) { return VbglInitClient(); @@ -64,6 +68,7 @@ DECLVBGL(void) VbglR0SfTerm(void) { VbglTerminate(); } +#endif DECLVBGL(int) VbglR0SfConnect(PVBGLSFCLIENT pClient) { diff --git a/src/VBox/Additions/linux/sharedfolders/vfsmod.c b/src/VBox/Additions/linux/sharedfolders/vfsmod.c index 3d43f9b072..a54cb849a7 100644 --- a/src/VBox/Additions/linux/sharedfolders/vfsmod.c +++ b/src/VBox/Additions/linux/sharedfolders/vfsmod.c @@ -32,6 +32,7 @@ #include "version-generated.h" #include "revision-generated.h" #include "product-generated.h" +#include "VBGLInternal.h" MODULE_DESCRIPTION(VBOX_PRODUCT " VFS Module for Host File System Access"); MODULE_AUTHOR(VBOX_VENDOR); @@ -601,10 +602,10 @@ static int __init init(void) return err; } - rcVBox = VbglR0SfInit(); + rcVBox = VbglR0HGCMInit(); if (RT_FAILURE(rcVBox)) { - LogRelFunc(("VbglR0SfInit failed, rc=%d\n", rcVBox)); + LogRelFunc(("VbglR0HGCMInit failed, rc=%d\n", rcVBox)); rcRet = -EPROTO; goto fail0; } @@ -648,7 +649,7 @@ fail2: VbglR0SfDisconnect(&client_handle); fail1: - VbglR0SfTerm(); + VbglR0HGCMTerminate(); fail0: unregister_filesystem(&vboxsf_fs_type); @@ -660,7 +661,7 @@ static void __exit fini(void) TRACE(); VbglR0SfDisconnect(&client_handle); - VbglR0SfTerm(); + VbglR0HGCMTerminate(); unregister_filesystem(&vboxsf_fs_type); } -- 2.11.0
>From 14427c15b382e0f772d34b42b94d8d24992ee9e0 Mon Sep 17 00:00:00 2001 From: Michael Thayer <[email protected]> Date: Mon, 24 Jul 2017 21:33:48 +0200 Subject: [PATCH 5/5] Linux shared folder driver: remove files which are no longer used. bugref:8524: Additions/linux: play nicely with distribution-installed Additions The preceding patches removed the dependencies of the Linux guest shared folder driver on various R0 guest library files. Remove them from the driver. Signed-off-by: Hans de Goede <[email protected]> Reworked by Oracle. --- src/VBox/Additions/linux/sharedfolders/Makefile.module | 7 +------ src/VBox/Additions/linux/sharedfolders/files_vboxsf | 5 ----- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/src/VBox/Additions/linux/sharedfolders/Makefile.module b/src/VBox/Additions/linux/sharedfolders/Makefile.module index fddff28900..85c1919597 100644 --- a/src/VBox/Additions/linux/sharedfolders/Makefile.module +++ b/src/VBox/Additions/linux/sharedfolders/Makefile.module @@ -25,14 +25,9 @@ MOD_OBJS = \ lnkops.o \ regops.o \ utils.o \ - GenericRequest.o \ SysHlp.o \ - PhysHeap.o \ - Init.o \ - VMMDev.o \ HGCM.o \ - VBoxGuestR0LibSharedFolders.o \ - VbglR0CanUsePhysPageList.o + VBoxGuestR0LibSharedFolders.o ifeq ($(BUILD_TARGET_ARCH),x86) MOD_OBJS += \ divdi3.o \ diff --git a/src/VBox/Additions/linux/sharedfolders/files_vboxsf b/src/VBox/Additions/linux/sharedfolders/files_vboxsf index 7755bd4ef0..4f1851e316 100644 --- a/src/VBox/Additions/linux/sharedfolders/files_vboxsf +++ b/src/VBox/Additions/linux/sharedfolders/files_vboxsf @@ -56,17 +56,12 @@ FILES_VBOXSF_NOBIN=" \ ${PATH_ROOT}/include/VBox/VBoxGuestMangling.h=>include/VBox/VBoxGuestMangling.h \ ${PATH_ROOT}/include/VBox/VMMDev.h=>include/VBox/VMMDev.h \ ${PATH_ROOT}/include/VBox/VMMDev2.h=>include/VBox/VMMDev2.h \ - ${PATH_ROOT}/src/VBox/Additions/common/VBoxGuestLib/GenericRequest.cpp=>GenericRequest.c \ ${PATH_ROOT}/src/VBox/Additions/common/VBoxGuestLib/HGCM.cpp=>HGCM.c \ - ${PATH_ROOT}/src/VBox/Additions/common/VBoxGuestLib/Init.cpp=>Init.c \ - ${PATH_ROOT}/src/VBox/Additions/common/VBoxGuestLib/PhysHeap.cpp=>PhysHeap.c \ ${PATH_ROOT}/src/VBox/Additions/common/VBoxGuestLib/SysHlp.cpp=>SysHlp.c \ ${PATH_ROOT}/src/VBox/Additions/common/VBoxGuestLib/SysHlp.h=>SysHlp.h \ ${PATH_ROOT}/src/VBox/Additions/common/VBoxGuestLib/VBGLInternal.h=>VBGLInternal.h \ ${PATH_ROOT}/src/VBox/Additions/common/VBoxGuestLib/VBoxGuestR0LibSharedFolders.c=>VBoxGuestR0LibSharedFolders.c \ ${PATH_ROOT}/src/VBox/Additions/common/VBoxGuestLib/VBoxGuestLog.h=>VBoxGuestLog.h \ - ${PATH_ROOT}/src/VBox/Additions/common/VBoxGuestLib/VMMDev.cpp=>VMMDev.c \ - ${PATH_ROOT}/src/VBox/Additions/common/VBoxGuestLib/VbglR0CanUsePhysPageList.cpp=>VbglR0CanUsePhysPageList.c \ ${PATH_ROOT}/src/VBox/Installer/linux/Makefile.include.header=>Makefile.include.header \ ${PATH_ROOT}/src/VBox/Installer/linux/Makefile.include.footer=>Makefile.include.footer \ ${PATH_ROOT}/src/VBox/Runtime/common/math/gcc/divdi3.c=>divdi3.c \ -- 2.11.0
_______________________________________________ vbox-dev mailing list [email protected] https://www.virtualbox.org/mailman/listinfo/vbox-dev
