diff mbox series

[PULL,8/9] meson: Disallow 64-bit on 32-bit emulation

Message ID 20250208205725.568631-9-richard.henderson@linaro.org
State Accepted
Commit acce728cbc6c154b215dfc8d05c12d8fcc2483d5
Headers show
Series [PULL,1/9] meson: Drop tcg as a module | expand

Commit Message

Richard Henderson Feb. 8, 2025, 8:57 p.m. UTC
For system mode, we can rarely support the amount of RAM that
the guest requires. TCG emulation is restricted to round-robin
mode, which solves many of the atomicity issues, but not those
associated with virtio.  In any case, round-robin does nothing
to help the speed of emulation.

For user mode, most emulation does not succeed at all.  Most
of the time we cannot even load 64-bit non-PIE binaries due
to lack of a 64-bit address space.  Threads are run in
parallel, not round-robin, which means that atomicity
is not handled.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 meson.build | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

Comments

Daniel P. Berrangé April 11, 2025, 1:42 p.m. UTC | #1
On Sat, Feb 08, 2025 at 12:57:23PM -0800, Richard Henderson wrote:
> For system mode, we can rarely support the amount of RAM that
> the guest requires. TCG emulation is restricted to round-robin
> mode, which solves many of the atomicity issues, but not those
> associated with virtio.  In any case, round-robin does nothing
> to help the speed of emulation.
> 
> For user mode, most emulation does not succeed at all.  Most
> of the time we cannot even load 64-bit non-PIE binaries due
> to lack of a 64-bit address space.  Threads are run in
> parallel, not round-robin, which means that atomicity
> is not handled.
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  meson.build | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)

Shouldn't  this patch and the earlier ones in this series have
added something to removed-features.rst, as this is a significant
feature removal which is impacting downstream users, and distros
in particular.

> 
> diff --git a/meson.build b/meson.build
> index 85317cd63f..ec51827f40 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -3185,6 +3185,9 @@ if host_os == 'windows'
>    endif
>  endif
>  
> +# Detect host pointer size for the target configuration loop.
> +host_long_bits = cc.sizeof('void *') * 8
> +
>  ########################
>  # Target configuration #
>  ########################
> @@ -3277,8 +3280,14 @@ foreach target : target_dirs
>      }
>    endif
>  
> +  config_target += keyval.load('configs/targets' / target + '.mak')
> +
>    target_kconfig = []
>    foreach sym: accelerators
> +    # Disallow 64-bit on 32-bit emulation and virtualization
> +    if host_long_bits < config_target['TARGET_LONG_BITS'].to_int()
> +      continue
> +    endif
>      if sym == 'CONFIG_TCG' or target in accelerator_targets.get(sym, [])
>        config_target += { sym: 'y' }
>        config_all_accel += { sym: 'y' }
> @@ -3292,9 +3301,6 @@ foreach target : target_dirs
>      error('No accelerator available for target @0@'.format(target))
>    endif
>  
> -  config_target += keyval.load('configs/targets' / target + '.mak')
> -  config_target += { 'TARGET_' + config_target['TARGET_ARCH'].to_upper(): 'y' }
> -
>    if 'TARGET_NEED_FDT' in config_target and not fdt.found()
>      if default_targets
>        warning('Disabling ' + target + ' due to missing libfdt')
> @@ -3307,6 +3313,7 @@ foreach target : target_dirs
>    actual_target_dirs += target
>  
>    # Add default keys
> +  config_target += { 'TARGET_' + config_target['TARGET_ARCH'].to_upper(): 'y' }
>    if 'TARGET_BASE_ARCH' not in config_target
>      config_target += {'TARGET_BASE_ARCH': config_target['TARGET_ARCH']}
>    endif
> -- 
> 2.43.0
> 
> 

With regards,
Daniel
Philippe Mathieu-Daudé April 11, 2025, 3:05 p.m. UTC | #2
On 11/4/25 15:42, Daniel P. Berrangé wrote:
> On Sat, Feb 08, 2025 at 12:57:23PM -0800, Richard Henderson wrote:
>> For system mode, we can rarely support the amount of RAM that
>> the guest requires. TCG emulation is restricted to round-robin
>> mode, which solves many of the atomicity issues, but not those
>> associated with virtio.  In any case, round-robin does nothing
>> to help the speed of emulation.
>>
>> For user mode, most emulation does not succeed at all.  Most
>> of the time we cannot even load 64-bit non-PIE binaries due
>> to lack of a 64-bit address space.  Threads are run in
>> parallel, not round-robin, which means that atomicity
>> is not handled.
>>
>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   meson.build | 13 ++++++++++---
>>   1 file changed, 10 insertions(+), 3 deletions(-)
> 
> Shouldn't  this patch and the earlier ones in this series have
> added something to removed-features.rst, as this is a significant
> feature removal which is impacting downstream users, and distros
> in particular.

IIRC the rationale for not deprecating was the feature is largely
broken already. Skipping the deprecation process we indeed forgot
to document in removed-features.rst.

Do you expect the doc update for the 10.0 release?
Daniel P. Berrangé April 11, 2025, 3:08 p.m. UTC | #3
On Fri, Apr 11, 2025 at 05:05:20PM +0200, Philippe Mathieu-Daudé wrote:
> On 11/4/25 15:42, Daniel P. Berrangé wrote:
> > On Sat, Feb 08, 2025 at 12:57:23PM -0800, Richard Henderson wrote:
> > > For system mode, we can rarely support the amount of RAM that
> > > the guest requires. TCG emulation is restricted to round-robin
> > > mode, which solves many of the atomicity issues, but not those
> > > associated with virtio.  In any case, round-robin does nothing
> > > to help the speed of emulation.
> > > 
> > > For user mode, most emulation does not succeed at all.  Most
> > > of the time we cannot even load 64-bit non-PIE binaries due
> > > to lack of a 64-bit address space.  Threads are run in
> > > parallel, not round-robin, which means that atomicity
> > > is not handled.
> > > 
> > > Reviewed-by: Thomas Huth <thuth@redhat.com>
> > > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > > ---
> > >   meson.build | 13 ++++++++++---
> > >   1 file changed, 10 insertions(+), 3 deletions(-)
> > 
> > Shouldn't  this patch and the earlier ones in this series have
> > added something to removed-features.rst, as this is a significant
> > feature removal which is impacting downstream users, and distros
> > in particular.
> 
> IIRC the rationale for not deprecating was the feature is largely
> broken already. Skipping the deprecation process we indeed forgot
> to document in removed-features.rst.
> 
> Do you expect the doc update for the 10.0 release?

The issue is that

  https://wiki.qemu.org/ChangeLog/10.0

delegates to removed-features.rst, so if we don't have time to
update the rst file, then we at least need to edit ChangeLog/10.0
to mention this removal of binaries as a fallback


With regards,
Daniel
Stefan Hajnoczi April 11, 2025, 6:18 p.m. UTC | #4
On Fri, Apr 11, 2025 at 11:09 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Fri, Apr 11, 2025 at 05:05:20PM +0200, Philippe Mathieu-Daudé wrote:
> > On 11/4/25 15:42, Daniel P. Berrangé wrote:
> > > On Sat, Feb 08, 2025 at 12:57:23PM -0800, Richard Henderson wrote:
> > > > For system mode, we can rarely support the amount of RAM that
> > > > the guest requires. TCG emulation is restricted to round-robin
> > > > mode, which solves many of the atomicity issues, but not those
> > > > associated with virtio.  In any case, round-robin does nothing
> > > > to help the speed of emulation.
> > > >
> > > > For user mode, most emulation does not succeed at all.  Most
> > > > of the time we cannot even load 64-bit non-PIE binaries due
> > > > to lack of a 64-bit address space.  Threads are run in
> > > > parallel, not round-robin, which means that atomicity
> > > > is not handled.
> > > >
> > > > Reviewed-by: Thomas Huth <thuth@redhat.com>
> > > > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > > > ---
> > > >   meson.build | 13 ++++++++++---
> > > >   1 file changed, 10 insertions(+), 3 deletions(-)
> > >
> > > Shouldn't  this patch and the earlier ones in this series have
> > > added something to removed-features.rst, as this is a significant
> > > feature removal which is impacting downstream users, and distros
> > > in particular.
> >
> > IIRC the rationale for not deprecating was the feature is largely
> > broken already. Skipping the deprecation process we indeed forgot
> > to document in removed-features.rst.
> >
> > Do you expect the doc update for the 10.0 release?
>
> The issue is that
>
>   https://wiki.qemu.org/ChangeLog/10.0
>
> delegates to removed-features.rst, so if we don't have time to
> update the rst file, then we at least need to edit ChangeLog/10.0
> to mention this removal of binaries as a fallback

-rc4 will be tagged on Tuesday. There is still time to add something
to removed-features.rst.

Please make sure to CC me and put "for-10.0" in the patch prefix if
it's not part of a pull request.

Stefan

>
>
> With regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
>
>
diff mbox series

Patch

diff --git a/meson.build b/meson.build
index 85317cd63f..ec51827f40 100644
--- a/meson.build
+++ b/meson.build
@@ -3185,6 +3185,9 @@  if host_os == 'windows'
   endif
 endif
 
+# Detect host pointer size for the target configuration loop.
+host_long_bits = cc.sizeof('void *') * 8
+
 ########################
 # Target configuration #
 ########################
@@ -3277,8 +3280,14 @@  foreach target : target_dirs
     }
   endif
 
+  config_target += keyval.load('configs/targets' / target + '.mak')
+
   target_kconfig = []
   foreach sym: accelerators
+    # Disallow 64-bit on 32-bit emulation and virtualization
+    if host_long_bits < config_target['TARGET_LONG_BITS'].to_int()
+      continue
+    endif
     if sym == 'CONFIG_TCG' or target in accelerator_targets.get(sym, [])
       config_target += { sym: 'y' }
       config_all_accel += { sym: 'y' }
@@ -3292,9 +3301,6 @@  foreach target : target_dirs
     error('No accelerator available for target @0@'.format(target))
   endif
 
-  config_target += keyval.load('configs/targets' / target + '.mak')
-  config_target += { 'TARGET_' + config_target['TARGET_ARCH'].to_upper(): 'y' }
-
   if 'TARGET_NEED_FDT' in config_target and not fdt.found()
     if default_targets
       warning('Disabling ' + target + ' due to missing libfdt')
@@ -3307,6 +3313,7 @@  foreach target : target_dirs
   actual_target_dirs += target
 
   # Add default keys
+  config_target += { 'TARGET_' + config_target['TARGET_ARCH'].to_upper(): 'y' }
   if 'TARGET_BASE_ARCH' not in config_target
     config_target += {'TARGET_BASE_ARCH': config_target['TARGET_ARCH']}
   endif