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

[Thomas Huth <thuth@redhat.com>]

On 06/02/2025 16.04, 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).

If it's a jumper on the board instead of two separate boards, then I think 
this is a good indication that this should only be a machine property, and 
not two separate boards?

 > My worry is about how to not break what distros currently ship.

So once the two binaries got unified, maybe we could still add a hook 
somewhere that checks argv[0] and sets the endianess property according to 
the name of the binary, so users can still use a symlink to force the 
opposite behavior?

Anyway, you likely don't have to solve this problem right in this series 
here already, we can think of this later when one of the binaries gets 
marked as deprecated. So for this series here, I'd suggest to go ahead 
without adding the -le and -be machine types and only use a property first, 
then tackle the remaining questions later...?

  Thomas