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

["Daniel P. Berrangé" <berrange@redhat.com>]

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.

None the less, since you've already taken this pattern of
dual machine types for BE & LE on  MIPS/SH4/XTensa, I think
we should stick with your precedent. Consistent modelling
of endian handling across all machine types is most important
IMHO


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|