On 12/2/25 17:23, BALATON Zoltan wrote:
On Wed, 12 Feb 2025, Philippe Mathieu-Daudé wrote:
On 12/2/25 14:53, Philippe Mathieu-Daudé wrote:
On 12/2/25 13:56, BALATON Zoltan wrote:
On Wed, 12 Feb 2025, Philippe Mathieu-Daudé wrote:
On 12/2/25 12:37, Thomas Huth wrote:
On 12/02/2025 12.24, Philippe Mathieu-Daudé wrote:
Introduce the EndianMode type and the DEFINE_PROP_ENDIAN() macros.
Endianness can be BIG, LITTLE or unspecified (default).
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
qapi/common.json | 16 ++++++++++++++++
include/hw/qdev-properties-system.h | 7 +++++++
hw/core/qdev-properties-system.c | 11 +++++++++++
3 files changed, 34 insertions(+)
+{ 'enum': 'EndianMode',
+ 'data': [ 'little', 'big', 'unspecified' ] }
Should 'unspecified' come first? ... so that it gets the value 0,
i.e. when someone forgets to properly initialize a related variable,
the chances are higher that it ends up as "unspecified" than as
"little" ?
Hmm but then in this series the dual-endianness regions are defined as:
+static const MemoryRegionOps pic_ops[2] = {
+ [0 ... 1] = {
This is already confusing as you'd have to know that 0 and 1 here
means ENDIAN_MODE_LITTLE and ENDIAN_MODE_BIG (using those constants
here as well might be clearer). It's easy to miss what this does so
At this point 0 / 1 only mean "from the index #0 included to the index
#1 included", 0 being the first one and 1 the last one.
maybe repeating the definitions for each case would be longer but less
confusing and then it does not matter what the values are.
This is what I tried to do with:
+ [ENDIAN_MODE_BIG].endianness = DEVICE_BIG_ENDIAN,
+ [ENDIAN_MODE_LITTLE].endianness = DEVICE_LITTLE_ENDIAN,
};
but in v7 we are back of picking an arbitrary value.
Or what uses the ops.endianness now should look at the property
introduced by this patch to avoid having to propagate it like below
and drop the ops.endianness? Or it should move to the memory region
and set when the ops are assigned?
I'm not understanding well what you ask, but maybe the answer is in v7 :)
I'm not sure I understand it well either. I think what I was asking about
is the same as what Thomas asked if this could be avoided to make it
necessary to allocate two separate ops for this. Looks like from now on
this ops struct should really loose the endianness value and this should
be assigned when the ops is added to the io region because that's where
it decides which endianness is it based on the property added in this
series. But I don't know if that could be done or would need deeper
changes as what later uses this ops struct might not have access to the
property and if we have a single ops struct it may need to be copied to
set different endianness intstead of just referencing it. So I'm not sure
there's a better way but I think this change makes an already cryptic
boiler plate even more confusing for people less knowledgeable about QEMU
and C programming so it makes even harder to write devices. But as long
as it's just a few devices that need to work with different endianness
then it might be OK. But wasn't that what NATIVE_ENDIAN was meant for?
What can't that be kept then?
Moving toward a single binary able to run heterogeneous machines, we
can't rely on a particular target endianness, so we need to remove
DEVICE_NATIVE_ENDIAN. The endianness is a property a device / machine,
not of the binary.