linux-kernel - Re: [PATCH] vbox: arm64 support for vboxguest

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <a969298e-0986-470c-9711-703af784d672@redhat.com>
Date: Mon, 6 Jan 2025 15:39:05 +0100
From: Hans de Goede <hdegoede@...hat.com>
To: Christian Kujau <lists@...dbynature.de>, Arnd Bergmann <arnd@...db.de>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
 linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH] vbox: arm64 support for vboxguest

Hi Christian,

Thank you for your patch.

On 15-Dec-24 8:52 PM, Christian Kujau wrote:
> My last email[0] still had "RFC" in the subject line, but no comments were 
> received. So, in an attempt of stupi^W boldness, this is the same patch 
> again, with RFC removed and ready for mainline, maybe?
> 
> Hello,
> 
> now that VirtualBox able to run as a host on arm64 (e.g. the Apple M3 
> processors) I was wondering if there are any plans to port the vboxguest 
> driver to that platform? I added ARM64 to the Kconfig files (see below) on 
> vboxguest and vboxsf, and also for vboxvideo, and it compiled just like 
> that and at least vboxsf appears to work just fine.
> 
> I don't know how to test vboxvideo yet (the module loads just fine), but 
> if we at least enable to option in the Kconfig file at least people would 
> be able to test it :-)
> 
> Thanks,
> Christian.

This part (the body) of the commit msg really should just describe
the what and why of the patch, the info you are providing above would
be something to put into a cover-letter not into the actual commit
message itself.

Also no Hello + thanks/regards in the commit message please
(that too is more something for the cover-letter).

At the moment this patch does 2 things in 1 patch, that
is it enables ARM64 support for both the vboxvideo
driver under drivers/gpu/drm/vboxvideo/Kconfig and
for the other vbox guest drivers under
drivers/virt/vboxguest/Kconfig + fs/vboxsf/Kconfig

The latter 2 go hand in hand and by testing vboxsf you
have also tested the vboxguest driver since that is how
vboxsf talks to the host. So enabling ARM64 support for
those in a single commit is fine.

But please split out the untested vboxvideo changes into
a separate commit. 

As for testing those, if you start the virtualbox uI, then
select the VM you are testing with (and do not start it,
or if started stop it first),

Then click on "Settings" then under "Expert" and then "Display"
you should be able to change the "Graphics Controller" setting
from the default "VMSVGA" to "VBoxSVGA". Save this as new
setting (press Ok) and then start the VM, now lspci in the
VM should show:

00:02.0 VGA compatible controller: InnoTek Systemberatung GmbH VirtualBox Graphics Adapter

and:

lspci -d "80ee:beef" -v

should show the driver being used is vboxvideo, e.g. :

user@...x:~$ lspci -d "80ee:beef" -v
00:02.0 VGA compatible controller: InnoTek Systemberatung GmbH VirtualBox Graphics Adapter (prog-if 00 [VGA controller])
	Subsystem: VMware Device 0405
	Flags: fast devsel, IRQ 18
	Memory at e0000000 (32-bit, prefetchable) [size=128M]
	I/O ports at d010 [size=16]
	Memory at f0000000 (32-bit, non-prefetchable) [size=2M]
	Expansion ROM at 000c0000 [virtual] [disabled] [size=128K]
	Kernel driver in use: vboxvideo
	Kernel modules: vboxvideo

Note virtualbox upstream consideres the vboxsvga adapter deprecated, so they
may have disabled it for ARM VMs. So if you cannot select it, then it likely
simply is disabled for ARM VMs and we should not enable the vboxvideo driver
for ARM64.

Regards,

Hans




> 
> [0] https://lore.kernel.org/lkml/f088e1da-8fae-2acb-6f7a-e414708d8e67@nerdbynature.de/
> 
> Signed-off-by: Christian Kujau <lists@...dbynature.de>
> 
>     vbox: Enable VBOXGUEST on ARM64
> 
> diff --git a/drivers/gpu/drm/vboxvideo/Kconfig b/drivers/gpu/drm/vboxvideo/Kconfig
> index 180e30b82ab9..dfe92bf23bde 100644
> --- a/drivers/gpu/drm/vboxvideo/Kconfig
> +++ b/drivers/gpu/drm/vboxvideo/Kconfig
> @@ -1,7 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0
>  config DRM_VBOXVIDEO
>  	tristate "Virtual Box Graphics Card"
> -	depends on DRM && X86 && PCI
> +	depends on DRM && (ARM64 || X86) && PCI
>  	select DRM_CLIENT_SELECTION
>  	select DRM_KMS_HELPER
>  	select DRM_VRAM_HELPER
> diff --git a/drivers/virt/vboxguest/Kconfig b/drivers/virt/vboxguest/Kconfig
> index cc329887bfae..11b153e7454e 100644
> --- a/drivers/virt/vboxguest/Kconfig
> +++ b/drivers/virt/vboxguest/Kconfig
> @@ -1,7 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  config VBOXGUEST
>  	tristate "Virtual Box Guest integration support"
> -	depends on X86 && PCI && INPUT
> +	depends on (ARM64 || X86) && PCI && INPUT
>  	help
>  	  This is a driver for the Virtual Box Guest PCI device used in
>  	  Virtual Box virtual machines. Enabling this driver will add
> diff --git a/fs/vboxsf/Kconfig b/fs/vboxsf/Kconfig
> index b84586ae08b3..d4694026db8b 100644
> --- a/fs/vboxsf/Kconfig
> +++ b/fs/vboxsf/Kconfig
> @@ -1,6 +1,6 @@
>  config VBOXSF_FS
>  	tristate "VirtualBox guest shared folder (vboxsf) support"
> -	depends on X86 && VBOXGUEST
> +	depends on (ARM64 || X86) && VBOXGUEST
>  	select NLS
>  	help
>  	  VirtualBox hosts can share folders with guests, this driver
>