Kröning, Martin <[email protected]> writes:

>> On 22.01.26, 14:16, Martin Kröning wrote:
>> 
>> On 19.01.26, 10:39, Peter Maydell wrote:
>> 
>> > On (2):
>> > 
>> > > Alex was worried about expanding the use of `virtio_is_big_endian`:
>> > >
>> > > > Hmm looking at the description:
>> > > >
>> > > > /**
>> > > > * @virtio_is_big_endian: Callback to return %true if a CPU which 
>> > > > supports
>> > > > * runtime configurable endianness is currently big-endian.
>> > > > * Non-configurable CPUs can use the default implementation of this 
>> > > > method.
>> > > > * This method should not be used by any callers other than the pre-1.0
>> > > > * virtio devices.
>> > > > */
>> > > > bool (*virtio_is_big_endian)(CPUState *cpu);
>> > > >
>> > > > I'm not sure if we want to expand the usage of this hack. I think
>> > > > Philippe is hoping to get rid of these warts eventually. Of course we
>> > > > could rename the method and just provide a way to get the current
>> > > > systems endianess.
>> > >
>> > > While not being very familiar with QEMU's source code, something like 
>> > > this
>> > > seems necessary to me. I can rename `virtio_is_big_endian` to 
>> > > `is_big_endian`
>> > > and `cpu_virtio_is_big_endian` to `cpu_is_big_endian` if you prefer that.
>> > 
>> > The reason for naming these functions the way we have now is
>> > that we really didn't want them to be a "this looks like the
>> > right function to use for my purpose" general name like
>> > "is_big_endian". Almost nothing in QEMU outside the CPU itself
>> > should need to know about the data endianness of the CPU,
>> > because in real hardware the devices can't tell what the CPU
>> > is set up as, they just get/send data. The exceptions are
>> > legacy virtio (which is a mistake that was made because nobody
>> > was thinking about it in the original virtio device design)
>> > and some oddball interfaces like semihosting. So the name is
>> > intended to push people away from using it (and towards e.g.
>> > target_big_endian()).
>> 
>> That makes sense. How about something like `internal_is_big_endian` and 
>> `cpu_internal_is_big_endian`, then? I am just making things up, though.
>> Given my limited experience with QEMU internals, I am not really qualified
>> to choose any name.
>
> Is there anything I can do to help move this forward?
> Should I send a new series that also changes the names to
> `internal_is_big_endian`?

Sounds like a plan, also make it clear the very limited use cases for
using this helper.

> To me it sounded like the technical details are fine to all of you.
> If this needs further internal discussions, though, I am happy to wait, of 
> course.
>
> Cheers,
> Martin

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Reply via email to