On Wed, Mar 03, 2021 at 05:08:36PM -0500, Michael S. Tsirkin wrote:
> On Wed, Mar 03, 2021 at 10:46:43PM +0100, Philippe Mathieu-Daudé wrote:
> > Introduce the cpu_virtio_is_big_endian() generic helper to avoid
> > calling CPUClass internal virtio_is_big_endian() one.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>
>
> Using virtio in the name here probably because virtio wants this?
> That doesn't sound like a good naming strategy, name should
> tell us what function does not how it's used.
On a more concrete proposal, how about using this change
to rename the virtio_is_big_endian field to guest_is_big_endian(),
and put the wrapper somewhere in a virtio header instead?
> > ---
> > include/hw/core/cpu.h | 9 +++++++++
> > hw/core/cpu.c | 8 ++++++--
> > hw/virtio/virtio.c | 4 +---
> > 3 files changed, 16 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> > index 2d43f78819f..b12028c3c03 100644
> > --- a/include/hw/core/cpu.h
> > +++ b/include/hw/core/cpu.h
> > @@ -602,6 +602,15 @@ hwaddr cpu_get_phys_page_debug(CPUState *cpu, vaddr
> > addr);
> > */
> > int cpu_asidx_from_attrs(CPUState *cpu, MemTxAttrs attrs);
> >
> > +/**
> > + * cpu_virtio_is_big_endian:
> > + * @cpu: CPU
> > +
> > + * Returns %true if a CPU which supports runtime configurable endianness
> > + * is currently big-endian.
> > + */
> > +bool cpu_virtio_is_big_endian(CPUState *cpu);
> > +
> > #endif /* CONFIG_USER_ONLY */
> >
> > /**
> > diff --git a/hw/core/cpu.c b/hw/core/cpu.c
> > index 4dce35f832f..daaff56a79e 100644
> > --- a/hw/core/cpu.c
> > +++ b/hw/core/cpu.c
> > @@ -218,8 +218,13 @@ static int cpu_common_gdb_write_register(CPUState
> > *cpu, uint8_t *buf, int reg)
> > return 0;
> > }
> >
> > -static bool cpu_common_virtio_is_big_endian(CPUState *cpu)
> > +bool cpu_virtio_is_big_endian(CPUState *cpu)
> > {
> > + CPUClass *cc = CPU_GET_CLASS(cpu);
> > +
> > + if (cc->virtio_is_big_endian) {
> > + return cc->virtio_is_big_endian(cpu);
> > + }
> > return target_words_bigendian();
> > }
> >
> > @@ -438,7 +443,6 @@ static void cpu_class_init(ObjectClass *klass, void
> > *data)
> > k->write_elf64_note = cpu_common_write_elf64_note;
> > k->gdb_read_register = cpu_common_gdb_read_register;
> > k->gdb_write_register = cpu_common_gdb_write_register;
> > - k->virtio_is_big_endian = cpu_common_virtio_is_big_endian;
> > set_bit(DEVICE_CATEGORY_CPU, dc->categories);
> > dc->realize = cpu_common_realizefn;
> > dc->unrealize = cpu_common_unrealizefn;
> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > index 1fd1917ca0f..fe6a4be99e4 100644
> > --- a/hw/virtio/virtio.c
> > +++ b/hw/virtio/virtio.c
> > @@ -1973,9 +1973,7 @@ static enum virtio_device_endian
> > virtio_default_endian(void)
> >
> > static enum virtio_device_endian virtio_current_cpu_endian(void)
> > {
> > - CPUClass *cc = CPU_GET_CLASS(current_cpu);
> > -
> > - if (cc->virtio_is_big_endian(current_cpu)) {
> > + if (cpu_virtio_is_big_endian(current_cpu)) {
> > return VIRTIO_DEVICE_ENDIAN_BIG;
> > } else {
> > return VIRTIO_DEVICE_ENDIAN_LITTLE;
> > --
> > 2.26.2
