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

[Philippe Mathieu-Daudé]

(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."