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é <phi...@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(+)

diff --git a/qapi/common.json b/qapi/common.json
index 6ffc7a37890..217feaaf683 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -212,3 +212,19 @@
  ##
  { 'struct': 'HumanReadableText',
    'data': { 'human-readable-text': 'str' } }
+
+##
+# @EndianMode:
+#
+# An enumeration of three options: little, big, and unspecified
+#
+# @little: Little endianness
+#
+# @big: Big endianness
+#
+# @unspecified: Endianness not specified
+#
+# Since: 10.0
+##
+{ '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 :)


Reply via email to