diff mbox series

[v3,03/29] include/exec/cpu-all: move compile time check for CPUArchState to cpu-target.c

Message ID 20250325045915.994760-4-pierrick.bouvier@linaro.org
State New
Headers show
Series single-binary: start make hw/arm/ common | expand

Commit Message

Pierrick Bouvier March 25, 2025, 4:58 a.m. UTC 
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 include/exec/cpu-all.h | 4 ----
 cpu-target.c           | 4 ++++
 2 files changed, 4 insertions(+), 4 deletions(-)

Comments

Philippe Mathieu-Daudé April 2, 2025, 3:31 a.m. UTC | #1 
On 25/3/25 05:58, Pierrick Bouvier wrote:
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   include/exec/cpu-all.h | 4 ----
>   cpu-target.c           | 4 ++++
>   2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index 74017a5ce7c..b1067259e6b 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -34,8 +34,4 @@
>   
>   #include "cpu.h"

This include ^^^^^^ ...

>   
> -/* Validate correct placement of CPUArchState. */
> -QEMU_BUILD_BUG_ON(offsetof(ArchCPU, parent_obj) != 0);
> -QEMU_BUILD_BUG_ON(offsetof(ArchCPU, env) != sizeof(CPUState));
> -
>   #endif /* CPU_ALL_H */
> diff --git a/cpu-target.c b/cpu-target.c
> index 519b0f89005..587f24b34e5 100644
> --- a/cpu-target.c
> +++ b/cpu-target.c
> @@ -29,6 +29,10 @@
>   #include "accel/accel-cpu-target.h"
>   #include "trace/trace-root.h"

... is also needed here, otherwise we get:

../../cpu-target.c:30:19: error: offsetof of incomplete type 'ArchCPU' 
(aka 'struct ArchCPU')
    30 | QEMU_BUILD_BUG_ON(offsetof(ArchCPU, parent_obj) != 0);
       |                   ^

>   
 > +/* Validate correct placement of CPUArchState. */> 
+QEMU_BUILD_BUG_ON(offsetof(ArchCPU, parent_obj) != 0);
> +QEMU_BUILD_BUG_ON(offsetof(ArchCPU, env) != sizeof(CPUState));
> +
>   char *cpu_model_from_type(const char *typename)
>   {
>       const char *suffix = "-" CPU_RESOLVING_TYPE;

With "cpu.h" include:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Pierrick Bouvier April 2, 2025, 3:25 p.m. UTC | #2 
On 4/1/25 20:31, Philippe Mathieu-Daudé wrote:
> On 25/3/25 05:58, Pierrick Bouvier wrote:
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>    include/exec/cpu-all.h | 4 ----
>>    cpu-target.c           | 4 ++++
>>    2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
>> index 74017a5ce7c..b1067259e6b 100644
>> --- a/include/exec/cpu-all.h
>> +++ b/include/exec/cpu-all.h
>> @@ -34,8 +34,4 @@
>>    
>>    #include "cpu.h"
> 
> This include ^^^^^^ ...
> 
>>    
>> -/* Validate correct placement of CPUArchState. */
>> -QEMU_BUILD_BUG_ON(offsetof(ArchCPU, parent_obj) != 0);
>> -QEMU_BUILD_BUG_ON(offsetof(ArchCPU, env) != sizeof(CPUState));
>> -
>>    #endif /* CPU_ALL_H */
>> diff --git a/cpu-target.c b/cpu-target.c
>> index 519b0f89005..587f24b34e5 100644
>> --- a/cpu-target.c
>> +++ b/cpu-target.c
>> @@ -29,6 +29,10 @@
>>    #include "accel/accel-cpu-target.h"
>>    #include "trace/trace-root.h"
> 
> ... is also needed here, otherwise we get:
> 
> ../../cpu-target.c:30:19: error: offsetof of incomplete type 'ArchCPU'
> (aka 'struct ArchCPU')
>      30 | QEMU_BUILD_BUG_ON(offsetof(ArchCPU, parent_obj) != 0);
>         |                   ^
> 
>>    
>   > +/* Validate correct placement of CPUArchState. */>
> +QEMU_BUILD_BUG_ON(offsetof(ArchCPU, parent_obj) != 0);
>> +QEMU_BUILD_BUG_ON(offsetof(ArchCPU, env) != sizeof(CPUState));
>> +
>>    char *cpu_model_from_type(const char *typename)
>>    {
>>        const char *suffix = "-" CPU_RESOLVING_TYPE;
> 
> With "cpu.h" include:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 

I can't reproduce this error.
With this series, cpu.h is pulled transitively from 
"accel/accel-cpu-target.h". Ideally, it would be better to add it 
explicitely yes.

@Richard, could you please amend this commit on tcg-next and add a 
direct include to cpu.h?

Thanks,
Pierrick
Richard Henderson April 2, 2025, 8:06 p.m. UTC | #3 
On 4/2/25 08:25, Pierrick Bouvier wrote:
> On 4/1/25 20:31, Philippe Mathieu-Daudé wrote:
>> With "cpu.h" include:
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>
> 
> I can't reproduce this error.
> With this series, cpu.h is pulled transitively from "accel/accel-cpu-target.h". Ideally, 
> it would be better to add it explicitely yes.
> 
> @Richard, could you please amend this commit on tcg-next and add a direct include to cpu.h?

Done.


r~
Pierrick Bouvier April 3, 2025, 4:18 p.m. UTC | #4 
On 4/2/25 13:06, Richard Henderson wrote:
> On 4/2/25 08:25, Pierrick Bouvier wrote:
>> On 4/1/25 20:31, Philippe Mathieu-Daudé wrote:
>>> With "cpu.h" include:
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>
>>
>> I can't reproduce this error.
>> With this series, cpu.h is pulled transitively from "accel/accel-cpu-target.h". Ideally,
>> it would be better to add it explicitely yes.
>>
>> @Richard, could you please amend this commit on tcg-next and add a direct include to cpu.h?
> 
> Done.
> 

Thanks Richard!

> 
> r~
diff mbox series

Patch

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 74017a5ce7c..b1067259e6b 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -34,8 +34,4 @@ 
 
 #include "cpu.h"
 
-/* Validate correct placement of CPUArchState. */
-QEMU_BUILD_BUG_ON(offsetof(ArchCPU, parent_obj) != 0);
-QEMU_BUILD_BUG_ON(offsetof(ArchCPU, env) != sizeof(CPUState));
-
 #endif /* CPU_ALL_H */
diff --git a/cpu-target.c b/cpu-target.c
index 519b0f89005..587f24b34e5 100644
--- a/cpu-target.c
+++ b/cpu-target.c
@@ -29,6 +29,10 @@ 
 #include "accel/accel-cpu-target.h"
 #include "trace/trace-root.h"
 
+/* Validate correct placement of CPUArchState. */
+QEMU_BUILD_BUG_ON(offsetof(ArchCPU, parent_obj) != 0);
+QEMU_BUILD_BUG_ON(offsetof(ArchCPU, env) != sizeof(CPUState));
+
 char *cpu_model_from_type(const char *typename)
 {
     const char *suffix = "-" CPU_RESOLVING_TYPE;