1. Patchew
  2. QEMU
  3. View series

[PATCH] target/xtensa/cpu: Move initialization of memory region to realize function

Thomas Huth posted 1 patch 3 weeks, 5 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260311202503.107026-1-thuth@redhat.com
Maintainers: Max Filippov <jcmvbkbc@gmail.com>
target/xtensa/cpu.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
[PATCH] target/xtensa/cpu: Move initialization of memory region to realize function
Posted by Thomas Huth 3 weeks, 5 days ago 
From: Thomas Huth <thuth@redhat.com>

When introspecting the xtensa CPUs from the command line, QEMU currently
crashes:

 $ ./qemu-system-xtensa -device dc233c-xtensa-cpu,help
 qemu-system-xtensa: ../../devel/qemu/system/physmem.c:1401:
  register_multipage: Assertion `num_pages' failed.
 Aborted (core dumped)

Move the initialization of the memory regions to the realize function
to fix this problem.

Reported-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 target/xtensa/cpu.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c
index 86ec899a67c..eebf40559bc 100644
--- a/target/xtensa/cpu.c
+++ b/target/xtensa/cpu.c
@@ -244,6 +244,14 @@ static void xtensa_cpu_realizefn(DeviceState *dev, Error **errp)
     Error *local_err = NULL;
 
 #ifndef CONFIG_USER_ONLY
+    CPUXtensaState *env = &XTENSA_CPU(dev)->env;
+
+    env->address_space_er = g_malloc(sizeof(*env->address_space_er));
+    env->system_er = g_malloc(sizeof(*env->system_er));
+    memory_region_init_io(env->system_er, OBJECT(dev), NULL, env, "er",
+                          UINT64_C(0x100000000));
+    address_space_init(env->address_space_er, env->system_er, "ER");
+
     xtensa_irq_init(&XTENSA_CPU(dev)->env);
 #endif
 
@@ -269,12 +277,6 @@ static void xtensa_cpu_initfn(Object *obj)
     env->config = xcc->config;
 
 #ifndef CONFIG_USER_ONLY
-    env->address_space_er = g_malloc(sizeof(*env->address_space_er));
-    env->system_er = g_malloc(sizeof(*env->system_er));
-    memory_region_init_io(env->system_er, obj, NULL, env, "er",
-                          UINT64_C(0x100000000));
-    address_space_init(env->address_space_er, env->system_er, "ER");
-
     cpu->clock = qdev_init_clock_in(DEVICE(obj), "clk-in", NULL, cpu, 0);
     clock_set_hz(cpu->clock, env->config->clock_freq_khz * 1000);
 #endif
-- 
2.53.0
Re: [PATCH] target/xtensa/cpu: Move initialization of memory region to realize function
Posted by Markus Armbruster 3 weeks, 5 days ago 
Thomas Huth <thuth@redhat.com> writes:

> From: Thomas Huth <thuth@redhat.com>
>
> When introspecting the xtensa CPUs from the command line, QEMU currently
> crashes:
>
>  $ ./qemu-system-xtensa -device dc233c-xtensa-cpu,help
>  qemu-system-xtensa: ../../devel/qemu/system/physmem.c:1401:
>   register_multipage: Assertion `num_pages' failed.
>  Aborted (core dumped)
>
> Move the initialization of the memory regions to the realize function
> to fix this problem.
>
> Reported-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Looking a bit closer at the code leading to the assertion failure:
TARGET_PAGE_BITS (macro expanding to target_page.bits) is still zero.
Hmm.  Is this a regression?  If yes, could there be more invalid uses of
TARGET_PAGE_BITS we haven't discovered, yet?  I guess they'd also be
relatively safe, as TARGET_PAGE_BITS becomes valid before things get
spicy, i.e. before the guest runs.

I tested v10.2.0, and it also crashes, but the assertion differs:

    qemu-system-xtensa: ../system/physmem.c:1337: phys_section_add: Assertion `map->sections_nb < TARGET_PAGE_SIZE' failed.

So, if it's a regression, it's not a recent one.

Another interesting detail is how TARGET_PAGE_BITS is defined:

    #ifdef TARGET_PAGE_BITS_VARY
    [...]
    # ifdef CONFIG_DEBUG_TCG
    #  define TARGET_PAGE_BITS   ({ assert(target_page.decided); \
                                    target_page.bits; })

Here, we assert it's valid.

    [...]
    # else
    #  define TARGET_PAGE_BITS   target_page.bits

Here, we just trust it.  I guess because the assertion above is
too expensive.

    [...]
    #else

And here it seems to be pulled from elsewhere.  The possible elsewheres
seem to be target/$target/cpu-param.h, which #define it as an integer
literal.

    [...]
    #endif

Cc'ing Richard, who has worked in the TARGET_PAGE_BITS area.  Just fyi,
Richard, I'm not asking you for further input.

> ---
>  target/xtensa/cpu.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c
> index 86ec899a67c..eebf40559bc 100644
> --- a/target/xtensa/cpu.c
> +++ b/target/xtensa/cpu.c
> @@ -244,6 +244,14 @@ static void xtensa_cpu_realizefn(DeviceState *dev, Error **errp)
>      Error *local_err = NULL;
>  
>  #ifndef CONFIG_USER_ONLY
> +    CPUXtensaState *env = &XTENSA_CPU(dev)->env;
> +
> +    env->address_space_er = g_malloc(sizeof(*env->address_space_er));
> +    env->system_er = g_malloc(sizeof(*env->system_er));
> +    memory_region_init_io(env->system_er, OBJECT(dev), NULL, env, "er",
> +                          UINT64_C(0x100000000));
> +    address_space_init(env->address_space_er, env->system_er, "ER");
> +
>      xtensa_irq_init(&XTENSA_CPU(dev)->env);
>  #endif
>  
> @@ -269,12 +277,6 @@ static void xtensa_cpu_initfn(Object *obj)
>      env->config = xcc->config;
>  
>  #ifndef CONFIG_USER_ONLY
> -    env->address_space_er = g_malloc(sizeof(*env->address_space_er));
> -    env->system_er = g_malloc(sizeof(*env->system_er));
> -    memory_region_init_io(env->system_er, obj, NULL, env, "er",
> -                          UINT64_C(0x100000000));
> -    address_space_init(env->address_space_er, env->system_er, "ER");
> -
>      cpu->clock = qdev_init_clock_in(DEVICE(obj), "clk-in", NULL, cpu, 0);
>      clock_set_hz(cpu->clock, env->config->clock_freq_khz * 1000);
>  #endif

Fixes the crash for all xtensa CPUs.

Tested-by: Markus Armbruster <armbru@redhat.com>
Re: [PATCH] target/xtensa/cpu: Move initialization of memory region to realize function
Posted by Philippe Mathieu-Daudé 3 weeks, 5 days ago 
On 12/3/26 08:51, Markus Armbruster wrote:
> Thomas Huth <thuth@redhat.com> writes:
> 
>> From: Thomas Huth <thuth@redhat.com>
>>
>> When introspecting the xtensa CPUs from the command line, QEMU currently
>> crashes:
>>
>>   $ ./qemu-system-xtensa -device dc233c-xtensa-cpu,help
>>   qemu-system-xtensa: ../../devel/qemu/system/physmem.c:1401:
>>    register_multipage: Assertion `num_pages' failed.
>>   Aborted (core dumped)
>>
>> Move the initialization of the memory regions to the realize function
>> to fix this problem.
>>
>> Reported-by: Markus Armbruster <armbru@redhat.com>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
> 
> Looking a bit closer at the code leading to the assertion failure:
> TARGET_PAGE_BITS (macro expanding to target_page.bits) is still zero.
> Hmm.  Is this a regression?  If yes, could there be more invalid uses of
> TARGET_PAGE_BITS we haven't discovered, yet?  I guess they'd also be
> relatively safe, as TARGET_PAGE_BITS becomes valid before things get
> spicy, i.e. before the guest runs.
> 
> I tested v10.2.0, and it also crashes, but the assertion differs:
> 
>      qemu-system-xtensa: ../system/physmem.c:1337: phys_section_add: Assertion `map->sections_nb < TARGET_PAGE_SIZE' failed.
> 
> So, if it's a regression, it's not a recent one.
> 
> Another interesting detail is how TARGET_PAGE_BITS is defined:
> 
>      #ifdef TARGET_PAGE_BITS_VARY
>      [...]
>      # ifdef CONFIG_DEBUG_TCG
>      #  define TARGET_PAGE_BITS   ({ assert(target_page.decided); \
>                                      target_page.bits; })
> 
> Here, we assert it's valid.
> 
>      [...]
>      # else
>      #  define TARGET_PAGE_BITS   target_page.bits
> 
> Here, we just trust it.  I guess because the assertion above is
> too expensive.
> 
>      [...]
>      #else
> 
> And here it seems to be pulled from elsewhere.  The possible elsewheres
> seem to be target/$target/cpu-param.h, which #define it as an integer
> literal.
> 
>      [...]
>      #endif
> 
> Cc'ing Richard, who has worked in the TARGET_PAGE_BITS area.  Just fyi,
> Richard, I'm not asking you for further input.

Correct analysis, I'll have a look.

> 
>> ---
>>   target/xtensa/cpu.c | 14 ++++++++------
>>   1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c
>> index 86ec899a67c..eebf40559bc 100644
>> --- a/target/xtensa/cpu.c
>> +++ b/target/xtensa/cpu.c
>> @@ -244,6 +244,14 @@ static void xtensa_cpu_realizefn(DeviceState *dev, Error **errp)
>>       Error *local_err = NULL;
>>   
>>   #ifndef CONFIG_USER_ONLY
>> +    CPUXtensaState *env = &XTENSA_CPU(dev)->env;
>> +
>> +    env->address_space_er = g_malloc(sizeof(*env->address_space_er));
>> +    env->system_er = g_malloc(sizeof(*env->system_er));
>> +    memory_region_init_io(env->system_er, OBJECT(dev), NULL, env, "er",
>> +                          UINT64_C(0x100000000));
>> +    address_space_init(env->address_space_er, env->system_er, "ER");
>> +
>>       xtensa_irq_init(&XTENSA_CPU(dev)->env);
>>   #endif
>>   
>> @@ -269,12 +277,6 @@ static void xtensa_cpu_initfn(Object *obj)
>>       env->config = xcc->config;
>>   
>>   #ifndef CONFIG_USER_ONLY
>> -    env->address_space_er = g_malloc(sizeof(*env->address_space_er));
>> -    env->system_er = g_malloc(sizeof(*env->system_er));
>> -    memory_region_init_io(env->system_er, obj, NULL, env, "er",
>> -                          UINT64_C(0x100000000));
>> -    address_space_init(env->address_space_er, env->system_er, "ER");
>> -
>>       cpu->clock = qdev_init_clock_in(DEVICE(obj), "clk-in", NULL, cpu, 0);
>>       clock_set_hz(cpu->clock, env->config->clock_freq_khz * 1000);
>>   #endif
> 
> Fixes the crash for all xtensa CPUs.
> 
> Tested-by: Markus Armbruster <armbru@redhat.com>
> 
>

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.