Re: [PATCH v5 11/16] hw/microblaze: Support various endianness for s3adsp1800 machines

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

(sorry, posted too quick)

On 6/2/25 19:24, Philippe Mathieu-Daudé wrote:
> +Michal
> 
> On 6/2/25 19:06, Daniel P. Berrangé wrote:
>> On Thu, Feb 06, 2025 at 06:49:38PM +0100, Philippe Mathieu-Daudé wrote:
>>> On 6/2/25 18:12, Daniel P. Berrangé wrote:
>>>> On Thu, Feb 06, 2025 at 04:04:20PM +0100, Philippe Mathieu-Daudé wrote:
>>>>> On 6/2/25 15:31, Daniel P. Berrangé wrote:
>>>>>> On Thu, Feb 06, 2025 at 02:53:58PM +0100, Philippe Mathieu-Daudé 
>>>>>> wrote:
>>>>>>> Hi Daniel,
>>>>>>>
>>>>>>> On 6/2/25 14:20, Daniel P. Berrangé wrote:
>>>>>>>> On Thu, Feb 06, 2025 at 02:10:47PM +0100, Philippe Mathieu-Daudé 
>>>>>>>> wrote:
>>>>>>>>> Introduce an abstract machine parent class which defines
>>>>>>>>> the 'little_endian' property. Duplicate the current machine,
>>>>>>>>> which endian is tied to the binary endianness, to one big
>>>>>>>>> endian and a little endian machine; updating the machine
>>>>>>>>> description. Keep the current default machine for each binary.
>>>>>>>>>
>>>>>>>>> 'petalogix-s3adsp1800' machine is aliased as:
>>>>>>>>> - 'petalogix-s3adsp1800-be' on big-endian binary,
>>>>>>>>> - 'petalogix-s3adsp1800-le' on little-endian one.
>>>>>>>>
>>>>>>>> Does it makes sense to expose these as different machine types ?
>>>>>>>>
>>>>>>>> If all the HW is identical in both cases, it feels like the
>>>>>>>> endianness could just be a bool property of the machine type,
>>>>>>>> rather than a new machine type.
>>>>>>>
>>>>>>> Our test suites expect "qemu-system-foo -M bar" to work out of
>>>>>>> the box, we can not have non-default properties.
>>>>>>>
>>>>>>> (This is related to the raspberry pi discussion in
>>>>>>> https://lore.kernel.org/qemu-devel/20250204002240.97830-1- 
>>>>>>> philmd@linaro.org/).
>>>>>>>
>>>>>>> My plan is to deprecate 'petalogix-s3adsp1800', so once we
>>>>>>> remove it we can merge both qemu-system-microblaze and
>>>>>>> qemu-system-microblazeel into a single binary.
>>>>>>>
>>>>>>> If you don't want to add more machines, what should be the
>>>>>>> endianness of the 'petalogix-s3adsp1800' machine in a binary
>>>>>>> with no particular endianness? Either we add for explicit
>>>>>>> endianness (fixing test suites) or we add one machine for
>>>>>>> each endianness; I fail to see other options not too
>>>>>>> confusing for our users.
>>>>>>
>>>>>> We would pick an arbitrary endianness of our choosing
>>>>>> I guess. How does this work in physical machines ? Is
>>>>>> the choice of endianess a firmware setting, or a choice
>>>>>> by the vendor when manufacturing in some way ?
>>>>>
>>>>> Like MIPS*, SH4* and Xtensa*, it is a jumper on the board
>>>>> (wired to a CPU pin which is sampled once at cold reset).
>>>>
>>>> That makes me thing even more it is just a machine type property.
>>>
>>> I'm happy with a machine property, this was even my first approach
>>> using OnOffAuto until I ran the test-suite and have qom-introspection
>>> failing.
>>>
>>> What should be the default?
>>>
>>> Per the SH4 datasheet:
>>>
>>>    Bit 31—Endian Flag (ENDIAN): Samples the value of the endian
>>>    specification external pin (MD5) in a power-on reset by the
>>>    RESET pin. The endian mode of all spaces is determined by this
>>>    bit. ENDIAN is a read-only bit.
>>>
>>> There is no default per the spec, and I believe using one is
>>> a mistake.
>>
>> If it is left as an unspecified choice in the spec, then I would
>> presume HW vendors are picking an option based on what they
>> expect "most" common usage to be amongst their customers. IOW,
>> if we know of typically used guest OS prefer big or little, that
>> could influence our choice.
> 
> Please have a look at this thread:
> https://lore.kernel.org/qemu-devel/ 
> d3346a55-584b-427b-8c87-32f7411a9290@amd.com/

Per Michal:

   Truth is that I am not aware about anybody configuring MB to big
   endian and we  are on AXI for quite a long time. There is still
   code in Linux kernel for it but I can't see any reason to keep it
   around. I don't think that make sense to keep big endian
   configurations alive at all.

So with that in mind, using a default of little-endian=true for
MicroBlaze seems safe as of 2025.

For MIPS, big-endian was the default but as the industry shifted,
it stayed big-endian on low-end 32-bit cores but mostly became
little-endian on high-end 64-bit cores.

For Renesas RX, the "Endian" section in the "CPU" chapter of the
hardware manual mentions:

   Both big and little endian are supported for the treatment of data,
   and the endian is switched by changing the setting on a mode pin (MDE)
   at the time of a power-on reset.

SH-4 "is bi-endian, running in either big-endian or little-endian byte
ordering."