Re: [PATCH v2 15/23] exec: Declare target_words_bigendian() in 'exec/tswap.h'

[Philippe Mathieu-Daudé]

On 13/12/23 12:20, Anton Johansson wrote:
> On 12/12/23, Philippe Mathieu-Daudé wrote:
> > We usually check target endianess before swapping values,
> > so target_words_bigendian() declaration makes sense in
> > "exec/tswap.h" with the target swapping helpers.
> >
> > Remove "hw/core/cpu.h" when it was only included to get
> > the target_words_bigendian() declaration.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >   include/exec/tswap.h     | 12 +++++++++++-
> >   include/hw/core/cpu.h    | 11 -----------
> >   cpu-target.c             |  1 +
> >   disas/disas.c            |  1 +
> >   hw/audio/virtio-snd.c    |  2 +-
> >   hw/core/cpu-sysemu.c     |  2 +-
> >   hw/core/generic-loader.c |  2 +-
> >   hw/display/vga.c         |  2 +-
> >   hw/virtio/virtio.c       |  1 +
> >   9 files changed, 18 insertions(+), 16 deletions(-)
> >
> > diff --git a/include/exec/tswap.h b/include/exec/tswap.h
> > index 68944a880b..77954bbc2b 100644
> > --- a/include/exec/tswap.h
> > +++ b/include/exec/tswap.h
> > @@ -8,9 +8,19 @@
> >   #ifndef TSWAP_H
> >   #define TSWAP_H
> >   
> > -#include "hw/core/cpu.h"
> >   #include "qemu/bswap.h"
>
> This breaks system/qtest.c which gets first_cpu from hw/core/cpu.h
> through the inclusion of tswap.h...
>
> ../system/qtest.c:548:33: error: ‘first_cpu’ undeclared (first use in this 
> function);
>    548 |             address_space_write(first_cpu->as, addr, 
> MEMTXATTRS_UNSPECIFIED,
>        |                                 ^~~~~~~~~

Doh sorry, in this series I dropped some already reviewed patches,
and extracted some not-that-related series. I was lazy to add the
many 'Based-on:' tags on the cover.

What you mentioned here is fixed here:
https://lore.kernel.org/qemu-devel/20231212113016.29808-1-philmd@linaro.org/

> Adding hw/core/cpu.h to system/qtest.c seems sufficient.
>
> Otherwise,
> Reviewed-by: Anton Johansson <anjo@rev.ng>

Thanks!