[PATCH] system: initialize target_page_bits as soon as possible

[PATCH] system: initialize target_page_bits as soon as possible

[Pierrick Bouvier]

Allow device init functions to use it, which can be convenient in some
cases (like hw/hyperv/hyperv.c).

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 system/physmem.c | 1 -
 system/vl.c      | 3 +++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/system/physmem.c b/system/physmem.c
index 8df9f30a0bb..c5fb784a9e1 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -3281,7 +3281,6 @@ void cpu_exec_init_all(void)
      * do this much later, rather than requiring board models to state
      * up front what their requirements are.
      */
-    finalize_target_page_bits();
     io_mem_init();
     memory_map_init();
 }
diff --git a/system/vl.c b/system/vl.c
index ec93988a03a..c64f8c8e808 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -2848,6 +2848,9 @@ void qemu_init(int argc, char **argv)
     bool userconfig = true;
     FILE *vmstate_dump_file = NULL;
 
+    /* Set target page info before creating machine and associated devices */
+    finalize_target_page_bits();
+
     qemu_add_opts(&qemu_drive_opts);
     qemu_add_drive_opts(&qemu_legacy_drive_opts);
     qemu_add_drive_opts(&qemu_common_drive_opts);
-- 
2.39.5

Re: [PATCH] system: initialize target_page_bits as soon as possible

[Pierrick Bouvier]

On 3/9/25 12:37, Pierrick Bouvier wrote:
> Allow device init functions to use it, which can be convenient in some
> cases (like hw/hyperv/hyperv.c).
> 
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   system/physmem.c | 1 -
>   system/vl.c      | 3 +++
>   2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/system/physmem.c b/system/physmem.c
> index 8df9f30a0bb..c5fb784a9e1 100644
> --- a/system/physmem.c
> +++ b/system/physmem.c
> @@ -3281,7 +3281,6 @@ void cpu_exec_init_all(void)
>        * do this much later, rather than requiring board models to state
>        * up front what their requirements are.
>        */
> -    finalize_target_page_bits();
>       io_mem_init();
>       memory_map_init();
>   }
> diff --git a/system/vl.c b/system/vl.c
> index ec93988a03a..c64f8c8e808 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -2848,6 +2848,9 @@ void qemu_init(int argc, char **argv)
>       bool userconfig = true;
>       FILE *vmstate_dump_file = NULL;
>   
> +    /* Set target page info before creating machine and associated devices */
> +    finalize_target_page_bits();
> +
>       qemu_add_opts(&qemu_drive_opts);
>       qemu_add_drive_opts(&qemu_legacy_drive_opts);
>       qemu_add_drive_opts(&qemu_common_drive_opts);

This is related to a very recent change merged.
58d0053: include/exec: Move TARGET_PAGE_{SIZE,MASK,BITS} to target_page.h

Regards,
Pierrick

Re: [PATCH] system: initialize target_page_bits as soon as possible

[Richard Henderson]

On 3/9/25 12:37, Pierrick Bouvier wrote:
> Allow device init functions to use it, which can be convenient in some
> cases (like hw/hyperv/hyperv.c).
> 
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   system/physmem.c | 1 -
>   system/vl.c      | 3 +++
>   2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/system/physmem.c b/system/physmem.c
> index 8df9f30a0bb..c5fb784a9e1 100644
> --- a/system/physmem.c
> +++ b/system/physmem.c
> @@ -3281,7 +3281,6 @@ void cpu_exec_init_all(void)
>        * do this much later, rather than requiring board models to state
>        * up front what their requirements are.
>        */
> -    finalize_target_page_bits();
>       io_mem_init();
>       memory_map_init();
>   }
> diff --git a/system/vl.c b/system/vl.c
> index ec93988a03a..c64f8c8e808 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -2848,6 +2848,9 @@ void qemu_init(int argc, char **argv)
>       bool userconfig = true;
>       FILE *vmstate_dump_file = NULL;
>   
> +    /* Set target page info before creating machine and associated devices */
> +    finalize_target_page_bits();

This is far too early, and therefore incorrect.

You have to wait until after all possible calls to set_preferred_target_page_bits(). The 
only relevant call is in arm_cpu_realizefn, invoked from the MachineClass.init, invoked 
from machine_run_board_init().


r~

Re: [PATCH] system: initialize target_page_bits as soon as possible

[Pierrick Bouvier]

On 3/9/25 16:12, Richard Henderson wrote:
> On 3/9/25 12:37, Pierrick Bouvier wrote:
>> Allow device init functions to use it, which can be convenient in some
>> cases (like hw/hyperv/hyperv.c).
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>    system/physmem.c | 1 -
>>    system/vl.c      | 3 +++
>>    2 files changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/system/physmem.c b/system/physmem.c
>> index 8df9f30a0bb..c5fb784a9e1 100644
>> --- a/system/physmem.c
>> +++ b/system/physmem.c
>> @@ -3281,7 +3281,6 @@ void cpu_exec_init_all(void)
>>         * do this much later, rather than requiring board models to state
>>         * up front what their requirements are.
>>         */
>> -    finalize_target_page_bits();
>>        io_mem_init();
>>        memory_map_init();
>>    }
>> diff --git a/system/vl.c b/system/vl.c
>> index ec93988a03a..c64f8c8e808 100644
>> --- a/system/vl.c
>> +++ b/system/vl.c
>> @@ -2848,6 +2848,9 @@ void qemu_init(int argc, char **argv)
>>        bool userconfig = true;
>>        FILE *vmstate_dump_file = NULL;
>>    
>> +    /* Set target page info before creating machine and associated devices */
>> +    finalize_target_page_bits();
> 
> This is far too early, and therefore incorrect.
> 
> You have to wait until after all possible calls to set_preferred_target_page_bits(). The
> only relevant call is in arm_cpu_realizefn, invoked from the MachineClass.init, invoked
> from machine_run_board_init().
>

Thanks.
Moving it later is not possible in the case I was solving (assert in 
hv_syndbg_class_init() from hw/hyperv/syndbg.c).
I'll simply move this assert later, but if a device needs to query 
target_page_size in its init function, we'll be in trouble. But maybe a 
device should not have to use this information anyway.

I'll drop this patch.

> 
> r~