[vbox-dev] Opportunity to significantly shrink the vboxsf Linux driver

[Hans de Goede]

Hi Larry,

On 10-07-17 21:37, Larry Finger wrote:
> On 07/10/2017 10:12 AM, Michael Thayer wrote:
>> Hello Hans,
>>
>> 10.07.2017 12:29, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 10-07-17 12:08, Hans de Goede wrote:
>>>> Hi all,
>>>>
>>>> During my vboxguest Linux cleanup work I made a small
>>>> detour to the vboxsf driver.
>>>>
>>>> I noticed that although it depends on vboxguest to make
>>>> hgcm calls, it still comes with its own copy of
>>>> GenericRequest, Physheap, Init and VMMDev.c
>>>>
>>>> It seems the only reason it uses this is to get
>>>> g_vbgldata.hostVersion so that it can check for
>>>> page-list support.
>>>>
>>>> IMHO it would be better if the vboxguest driver were
>>>> to store the hostVersion in an exported global
>>>> variable, say vbg_host_version. And then
>>>> vboxfs would use that. Combine that with moving:
>>>>
>>>> RTSEMFASTMUTEX mutexHGCMHandle;
>>>> struct VBGLHGCMHANDLEDATA aHGCMHandleData[64];
>>>>
>>>>   From g_vbgldata to static globals in HGCM.c
>>>> and then g_vbgldata can be dropped from
>>>> VBGLInternal.h together with dropping
>>>> GenericRequest, Physheap, Init and VMMDev.c
>>>> completely.
>>>>
>>>> I've attached a patch which does this from my
>>>> own local vboxsf version.
>>>
>>> Ok, that version was no good, I did not notice that
>>> some of the removed code was responsible for
>>> calling vbglR0HGCMInit(), here is a new version
>>> replacing the VbglR0SfInit call with vbglR0HGCMInit.
>> HGCM.cpp
>> I have just started reviewing the patch, as the person most familiar
>> with the code is away just now.  I am not terribly familiar with the
>> code, but I have had to go through it on occasion in the past, and my
>> first thought is that this might cause problems on other platforms,
>> since the common code is shared by the Windows and Solaris drivers.  I
>> will give you an update when I have read more code.
> 
> Hans,
> 
> I tried your patch. The first problem was that the first file to be patched is HGCM.cpp, not HGCM.c. That was easily fixed.
> 
> The second problem is that when I try to load vboxsf.ko in a guest, I get a message that vbg_host_version is an unknown symbol.

Correct, my patch was intended to give an idea how the vboxsf driver
could be made smaller, not as a ready to use patch.

Just like I did with the vboxvideo driver I'm currently working on
cleaning up the vboxguest driver for upstream submission. Since
this requires some major surgery I'm not feeding back those changes
to VirtualBox svn. This specific change looks like it can be adapted
to upstream with a little effort which is why I send it to the list.

If you're curious about my vboxguest efforts you can find them
here (very much WIP):

https://github.com/jwrdegoede/vboxguest

I've already replaced a bunch of VirtualBox custom malloc functions
with regular kernel ones and the same for spinlocks and wait_queues.

I still need to replace the VirtualBox custom malloc functions
for the memory baloon and for vgdrvInitFixateGuestMappings() once
that is done the code will be more or less free of relying on
a lot of the VirtualBox portability / runtime code and I should
be able to remove most of the code, leaving just 3 .c files and
a bunch of headers.

Regards,

Hans