| Message ID | 20221216215519.5522-2-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/cpu: System/User cleanups around hwaddr/vaddr | expand |
On 12/16/22 13:55, Philippe Mathieu-Daudé wrote: > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > dump/dump.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/dump/dump.c b/dump/dump.c > index 279b07f09b..c62dc94213 100644 > --- a/dump/dump.c > +++ b/dump/dump.c > @@ -29,6 +29,7 @@ > #include "qemu/main-loop.h" > #include "hw/misc/vmcoreinfo.h" > #include "migration/blocker.h" > +#include "cpu.h" Does it work to include "exec/cpu-all.h" instead? r~
On 12/16/22 18:55, Philippe Mathieu-Daudé wrote: > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> > dump/dump.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/dump/dump.c b/dump/dump.c > index 279b07f09b..c62dc94213 100644 > --- a/dump/dump.c > +++ b/dump/dump.c > @@ -29,6 +29,7 @@ > #include "qemu/main-loop.h" > #include "hw/misc/vmcoreinfo.h" > #include "migration/blocker.h" > +#include "cpu.h" > > #ifdef TARGET_X86_64 > #include "win_dump.h"
On 17/12/22 00:58, Richard Henderson wrote: > On 12/16/22 13:55, Philippe Mathieu-Daudé wrote: >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> --- >> dump/dump.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/dump/dump.c b/dump/dump.c >> index 279b07f09b..c62dc94213 100644 >> --- a/dump/dump.c >> +++ b/dump/dump.c >> @@ -29,6 +29,7 @@ >> #include "qemu/main-loop.h" >> #include "hw/misc/vmcoreinfo.h" >> #include "migration/blocker.h" >> +#include "cpu.h" > > Does it work to include "exec/cpu-all.h" instead? We get: include/exec/cpu-all.h:110:5: warning: 'TARGET_LONG_SIZE' is not defined, evaluates to 0 [-Wundef] #if TARGET_LONG_SIZE == 4 ^ TARGET_LONG_SIZE is defined in "exec/cpu-defs.h" which is target specific. If I add "exec/cpu-defs.h" to "exec/cpu-all.h" I get: In file included from ../../dump/dump.c:18: include/exec/cpu-all.h:439:8: error: incomplete definition of type 'struct ArchCPU' cpu->parent_obj.env_ptr = &cpu->env; ~~~^ Is it worth extracting the few tswapX() declarations to "exec/tswap.h"?
On 2/23/23 00:09, Philippe Mathieu-Daudé wrote: >>> +#include "cpu.h" >> >> Does it work to include "exec/cpu-all.h" instead? > > We get: > > include/exec/cpu-all.h:110:5: warning: 'TARGET_LONG_SIZE' is not defined, evaluates to 0 > [-Wundef] > #if TARGET_LONG_SIZE == 4 > ^ > > TARGET_LONG_SIZE is defined in "exec/cpu-defs.h" which is > target specific. If I add "exec/cpu-defs.h" to "exec/cpu-all.h" > I get: > > In file included from ../../dump/dump.c:18: > include/exec/cpu-all.h:439:8: error: incomplete definition of type 'struct ArchCPU' > cpu->parent_obj.env_ptr = &cpu->env; > ~~~^ > > Is it worth extracting the few tswapX() declarations to "exec/tswap.h"? That's probably worthwhile, using cpu-param.h directly, perhaps, rather than pulling in the rest of cpu stuff? r~
On 23/2/23 19:01, Richard Henderson wrote: > On 2/23/23 00:09, Philippe Mathieu-Daudé wrote: >>>> +#include "cpu.h" >>> >>> Does it work to include "exec/cpu-all.h" instead? >> >> We get: >> >> include/exec/cpu-all.h:110:5: warning: 'TARGET_LONG_SIZE' is not >> defined, evaluates to 0 [-Wundef] >> #if TARGET_LONG_SIZE == 4 >> ^ >> >> TARGET_LONG_SIZE is defined in "exec/cpu-defs.h" which is >> target specific. If I add "exec/cpu-defs.h" to "exec/cpu-all.h" >> I get: >> >> In file included from ../../dump/dump.c:18: >> include/exec/cpu-all.h:439:8: error: incomplete definition of type >> 'struct ArchCPU' >> cpu->parent_obj.env_ptr = &cpu->env; >> ~~~^ >> >> Is it worth extracting the few tswapX() declarations to "exec/tswap.h"? > > That's probably worthwhile, using cpu-param.h directly, perhaps, rather > than pulling in the rest of cpu stuff? TARGET_BIG_ENDIAN is only defined for target-specific objects, so this as is it will never work (dump.o must be compiled for each target). For heterogeneous emulation we need to pass a CPU[Arch]State* to get the endianness of the CPU. As of today the endianness isn't runtime, so this field doesn't exist in the common CPUState (some arch might have some equivalent field). This file uses tswap() 4 times in the same function: get_note_sizes(), so I could extract it to a dump-target.c unit. I have no clue what that file is for, but this particularity is odd. Looking further, we have in "hw/core/cpu.h": int (SysemuCPUOps::write_elf32/64_note)(WriteCoreDumpFunction, ... but no ReadCoreDumpFunction equivalent.
On 2/23/23 11:19, Philippe Mathieu-Daudé wrote: > This file uses tswap() 4 times in the same function: get_note_sizes(), > so I could extract it to a dump-target.c unit. > I have no clue what that file is for, but this particularity is odd. All uses of tswap in that file are wrong, and should be using cpu_to_dumpN, which correctly tests the endianness of the output. r~
On 23/2/23 22:43, Richard Henderson wrote: > On 2/23/23 11:19, Philippe Mathieu-Daudé wrote: >> This file uses tswap() 4 times in the same function: get_note_sizes(), >> so I could extract it to a dump-target.c unit. >> I have no clue what that file is for, but this particularity is odd. > > All uses of tswap in that file are wrong, and should be using > cpu_to_dumpN, which correctly tests the endianness of the output. Yes! Thank you :)
diff --git a/dump/dump.c b/dump/dump.c index 279b07f09b..c62dc94213 100644 --- a/dump/dump.c +++ b/dump/dump.c @@ -29,6 +29,7 @@ #include "qemu/main-loop.h" #include "hw/misc/vmcoreinfo.h" #include "migration/blocker.h" +#include "cpu.h" #ifdef TARGET_X86_64 #include "win_dump.h"
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- dump/dump.c | 1 + 1 file changed, 1 insertion(+)