diff mbox series

[2/3] package/qemu: select FDT for custom targets too

Message ID 20221104235727.587650-3-unixmania@gmail.com
State Superseded, archived
Headers show
Series package/qemu: add option to enable guest agent | expand

Commit Message

Carlos Santos Nov. 4, 2022, 11:57 p.m. UTC
From: Carlos Santos <unixmania@gmail.com>

Custom targets selects system and Linux user-land emulation, which in
their turn require FDT. Move the BR2_PACKAGE_QEMU_FDT selection to the
BR2_PACKAGE_QEMU_HAS_EMULS hidden boolean, to cover all cases.

Signed-off-by: Carlos Santos <unixmania@gmail.com>
---
 package/qemu/Config.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Thomas Huth Nov. 5, 2022, 10:44 a.m. UTC | #1
Am Fri,  4 Nov 2022 20:57:26 -0300
schrieb unixmania@gmail.com:

> From: Carlos Santos <unixmania@gmail.com>
> 
> Custom targets selects system and Linux user-land emulation, which in
> their turn require FDT. Move the BR2_PACKAGE_QEMU_FDT selection to the
> BR2_PACKAGE_QEMU_HAS_EMULS hidden boolean, to cover all cases.
> 
> Signed-off-by: Carlos Santos <unixmania@gmail.com>
> ---
>  package/qemu/Config.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/qemu/Config.in b/package/qemu/Config.in
> index 8b8a999885..9e0cbd4acf 100644
> --- a/package/qemu/Config.in
> +++ b/package/qemu/Config.in
> @@ -95,7 +95,6 @@ comment "... or you can select emulator families to enable, below:"
>  config BR2_PACKAGE_QEMU_SYSTEM
>  	bool "Enable all systems emulation"
>  	depends on !BR2_STATIC_LIBS # dtc
> -	select BR2_PACKAGE_QEMU_FDT
>  	help
>  	  Say 'y' to build all system emulators/virtualisers that QEMU
>  	  supports.
> @@ -121,6 +120,7 @@ endif # BR2_PACKAGE_QEMU_CUSTOM_TARGETS == ""
>  config BR2_PACKAGE_QEMU_HAS_EMULS
>  	def_bool y
>  	depends on BR2_PACKAGE_QEMU_SYSTEM || BR2_PACKAGE_QEMU_LINUX_USER || BR2_PACKAGE_QEMU_CUSTOM_TARGETS != ""
> +	select BR2_PACKAGE_QEMU_FDT

Technically, I think you don't need FDT if you are only compiling the
linux-user targets ... so maybe it would be better to add this to the
BR2_PACKAGE_QEMU_CUSTOM_TARGETS switch instead?

 Thomas
Carlos Santos Nov. 5, 2022, 8:23 p.m. UTC | #2
On Sat, Nov 5, 2022 at 7:44 AM Thomas Huth <huth@tuxfamily.org> wrote:
>
> Am Fri,  4 Nov 2022 20:57:26 -0300
> schrieb unixmania@gmail.com:
>
> > From: Carlos Santos <unixmania@gmail.com>
> >
> > Custom targets selects system and Linux user-land emulation, which in
> > their turn require FDT. Move the BR2_PACKAGE_QEMU_FDT selection to the
> > BR2_PACKAGE_QEMU_HAS_EMULS hidden boolean, to cover all cases.
> >
> > Signed-off-by: Carlos Santos <unixmania@gmail.com>
> > ---
> >  package/qemu/Config.in | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/package/qemu/Config.in b/package/qemu/Config.in
> > index 8b8a999885..9e0cbd4acf 100644
> > --- a/package/qemu/Config.in
> > +++ b/package/qemu/Config.in
> > @@ -95,7 +95,6 @@ comment "... or you can select emulator families to enable, below:"
> >  config BR2_PACKAGE_QEMU_SYSTEM
> >       bool "Enable all systems emulation"
> >       depends on !BR2_STATIC_LIBS # dtc
> > -     select BR2_PACKAGE_QEMU_FDT
> >       help
> >         Say 'y' to build all system emulators/virtualisers that QEMU
> >         supports.
> > @@ -121,6 +120,7 @@ endif # BR2_PACKAGE_QEMU_CUSTOM_TARGETS == ""
> >  config BR2_PACKAGE_QEMU_HAS_EMULS
> >       def_bool y
> >       depends on BR2_PACKAGE_QEMU_SYSTEM || BR2_PACKAGE_QEMU_LINUX_USER || BR2_PACKAGE_QEMU_CUSTOM_TARGETS != ""
> > +     select BR2_PACKAGE_QEMU_FDT
>
> Technically, I think you don't need FDT if you are only compiling the
> linux-user targets ... so maybe it would be better to add this to the
> BR2_PACKAGE_QEMU_CUSTOM_TARGETS switch instead?
>
>  Thomas

BR2_PACKAGE_QEMU_CUSTOM_TARGETS is a string, so adding a "select
BR2_PACKAGE_QEMU_FDT" does not have any effect.

A better solution for the case in which only linux-user targets are
selected requires a more complex approach. I will submit a separate
patch for this purpose. Meanwhile, let's put this series on hold.
Thomas Petazzoni Nov. 5, 2022, 9:18 p.m. UTC | #3
On Sat, 5 Nov 2022 17:23:53 -0300
Carlos Santos <unixmania@gmail.com> wrote:

> BR2_PACKAGE_QEMU_CUSTOM_TARGETS is a string, so adding a "select
> BR2_PACKAGE_QEMU_FDT" does not have any effect.
> 
> A better solution for the case in which only linux-user targets are
> selected requires a more complex approach. I will submit a separate
> patch for this purpose. Meanwhile, let's put this series on hold.

Hm, I think I start to understand the issue, and with the current
organization of the Config.in options in package/qemu/Config.in, it's
going to be difficult to fix in a correct way.

For example, it looks like the linux-user mode emulation doesn't work
with the musl C library:

config BR2_PACKAGE_QEMU_LINUX_USER
        bool "Enable all Linux user-land emulation"
        # Incompatible "struct sigevent" definition on musl
        depends on !BR2_TOOLCHAIN_USES_MUSL

So, it means that BR2_PACKAGE_QEMU_CUSTOM_TARGETS="i386-linux-user"
will in fact fail to build with a musl toolchain...

I only sane way to address this I believe would be to remove
BR2_PACKAGE_QEMU_CUSTOM_TARGETS entirely, and instead have
BR2_PACKAGE_QEMU_SYSTEM_TARGETS and
BR2_PACKAGE_QEMU_LINUX_USER_TARGETS, which would only be accessible
when BR2_PACKAGE_QEMU_SYSTEM or BR2_PACKAGE_QEMU_LINUX_USER
respectively are enabled. When BR2_PACKAGE_QEMU_SYSTEM_TARGETS is
empty, all system emulation targets are built, otherwise only the
specified ones are built. Ditto for the user emulation targets.

Obviously as usual, the main drawback is that is breaks backward
compatibility with existing configurations...

The other approach, which you took, is to assume for the "worst", and
assume that when BR2_PACKAGE_QEMU_CUSTOM_TARGETS != "", we might build
system or user emulation targets, and therefore this option needs to
have the combination of the dependencies of the system and user
emulation options.... which would mean disabling this option with musl
toolchains, for example.

Thomas
diff mbox series

Patch

diff --git a/package/qemu/Config.in b/package/qemu/Config.in
index 8b8a999885..9e0cbd4acf 100644
--- a/package/qemu/Config.in
+++ b/package/qemu/Config.in
@@ -95,7 +95,6 @@  comment "... or you can select emulator families to enable, below:"
 config BR2_PACKAGE_QEMU_SYSTEM
 	bool "Enable all systems emulation"
 	depends on !BR2_STATIC_LIBS # dtc
-	select BR2_PACKAGE_QEMU_FDT
 	help
 	  Say 'y' to build all system emulators/virtualisers that QEMU
 	  supports.
@@ -121,6 +120,7 @@  endif # BR2_PACKAGE_QEMU_CUSTOM_TARGETS == ""
 config BR2_PACKAGE_QEMU_HAS_EMULS
 	def_bool y
 	depends on BR2_PACKAGE_QEMU_SYSTEM || BR2_PACKAGE_QEMU_LINUX_USER || BR2_PACKAGE_QEMU_CUSTOM_TARGETS != ""
+	select BR2_PACKAGE_QEMU_FDT
 
 if BR2_PACKAGE_QEMU_HAS_EMULS