Re: [PATCH v6 01/11] hw/qdev-properties-system: Introduce EndianMode QAPI enum

[Thomas Huth <thuth@redhat.com>]

On 13/02/2025 14.59, BALATON Zoltan wrote:
> On Thu, 13 Feb 2025, Thomas Huth wrote:
>> On 12/02/2025 23.34, BALATON Zoltan wrote:
[...]
>>> So then can the behaviour of NATIVE_ENDIAN be changed to look at the 
>>> machine endianness instead of replacing it with a constant?
>>
>> No, that does not work. First, the machine knows about its devices, but a 
>> device should not know about the wiring of the global machine (just like 
>> in real life).
> 
> That means all devices should be either big or little endian and there 
> should be no native endian ones. Why do we have those then?

Some device can indeed be either big or little endian - think of devices 
that are synthesized in an FPGA for example. But in most cases, it rather 
depends on the bus wiring. Anyway, we need a config knob to allow either the 
one or the other endianness for certain devices.

> That's why this 
> endianness property should either be removed from ops and only attached to 
> it when added to a machine if needed or kept to show which machines it can 
> be attached to: only big, little or both endian which is what it seems to be 
> doing now.

Again, devices should not know about machines, not the other way round. So 
the device should offer a config switch (property) and the machine should 
set it to the value that it needs.

>> Second, imagine a board with e.g. a big endian main CPU and a little 
>> endian service processor - how should a device know the right endianness 
>> here?
> 
> How would that work with this series? So the proposed solution is to double 
> the devices now marked as NATIVE_ENDIAN to have a big and a little endian 
> variant for them so the board can choose?

This is not doubling the devices. It just introduces a config property to 
let the machine switch the endianness.

> That does not exist in real as you 
> wrote, there's only one device so then this is probably not the right way to 
> model it.

Some devices can exist in both, big and little endian variants. We could of 
course create two devices for this, but that's nonsense if it can simply be 
handled by a property instead.

>>> Or would that be too much overhead? If always looking up the endianness 
>>> is not wanted could the ops declaration keep NATIVE_ENDIAN 
>>
>> IMHO we should get rid of NATIVE_ENDIAN completely since there is no 
>> "native" endian in multi-CPU boards.
> 
> If we say NATIVE_ENDIAN means that the device can be attached to either big 
> or little endian machine then we can keep this constant but when adding the 
> ops to a memory region the board has to then decide which endianness it is 
> and replace it with either big or little. Then we don't need two versions of 
> the same device and NATIVE_ENDIAN means that the device can be used in both 
> machines.

Well, it's currently the devices that are calling memory_region_init_io().
And since memory_region_init_io() does not copy the MemoryRegionOps struct,
we need two implementations right now for this, one for big and one for 
little endian. So I think Philippe's series here is fine. But feel free to 
suggest clean up patches on top if you think that the 
memory_region_init_io() needs to be handled differently in QEMU everywhere.

  Thomas