All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: "Philippe Mathieu-Daudé" <philmd@linaro.org>
Cc: qemu-devel@nongnu.org, qemu-arm@nongnu.org,
	Anton Johansson <anjo@rev.ng>, Jason Wang <jasowang@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Alistair Francis <alistair@alistair23.me>,
	Thomas Huth <thuth@redhat.com>,
	Richard Henderson <richard.henderson@linaro.org>,
	Peter Maydell <peter.maydell@linaro.org>,
	"Edgar E. Iglesias" <edgar.iglesias@gmail.com>,
	Markus Armbruster <armbru@redhat.com>
Subject: Re: [PATCH v5 11/16] hw/microblaze: Support various endianness for s3adsp1800 machines
Date: Thu, 6 Feb 2025 14:31:17 +0000	[thread overview]
Message-ID: <Z6THtfjL6UVhBiW7@redhat.com> (raw)
In-Reply-To: <4624f149-76d0-4da5-8f13-8c015043c335@linaro.org>

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 ?

Picking an arbitrary endianess is compatible with our
test suite, it just has the implication that we would
only end up testing the machine in a single endianness
configuration.

If we wanted to test both endianness options, the test
would need amending to know to try both values of the
endian property on the machine.

> This approach is the same I took to merge MIPS*, SH4* and
> Xtensa* machines in endianness-agnostic binaries.

If we have prior art like this, then remaining consistentv is
desirable and thus my comments are too late.

> Also the same I'm using to merge 32/64-bit targets into the
> same binaries.
> Assuming we have a qemu-system-x86 binary able to run i386 and
> x86_64 machines, what do you expect when starting '-M pc'? How
> to not confuse users wanting to run FreeDOS in 32-bit mode?
> 
> Again, IMO having '-M pc,mode=32' is simpler, but that breaks
> the test suites assumptions than machines can start with no
> default values (see QOM introspection tests for example).

With x86 there's no need for mode=32. Whether the machine
supports 64-bit or not is a property of the CPU model
chosen. eg  "qemu -M pc -cpu Nehalem" would be 64-bit and
"qemu -M pc -cpu pentium" would be 32-bit.

The qemu-system-i386 binary is pretty much pointless as a
separate thing. Libvirt will happily use qemu-system-x86_64
to run 32-bit guests, by just specifying a 32-bit CPU
model


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 :|


  reply	other threads:[~2025-02-06 14:32 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-06 13:10 [PATCH v5 00/16] hw/microblaze: Allow running cross-endian vCPUs Philippe Mathieu-Daudé
2025-02-06 13:10 ` [PATCH v5 01/16] hw/intc/xilinx_intc: Make device endianness configurable Philippe Mathieu-Daudé
2025-02-06 13:10 ` [PATCH v5 02/16] hw/net/xilinx_ethlite: " Philippe Mathieu-Daudé
2025-02-06 17:14   ` Thomas Huth
2025-02-06 21:32   ` Richard Henderson
2025-02-06 13:10 ` [PATCH v5 03/16] hw/timer/xilinx_timer: " Philippe Mathieu-Daudé
2025-02-06 13:10 ` [PATCH v5 04/16] hw/char/xilinx_uartlite: " Philippe Mathieu-Daudé
2025-02-06 13:10 ` [PATCH v5 05/16] hw/ssi/xilinx_spi: " Philippe Mathieu-Daudé
2025-02-06 13:10 ` [PATCH v5 06/16] hw/arm/xlnx-zynqmp: Use &error_abort for programming errors Philippe Mathieu-Daudé
2025-02-06 13:10 ` [PATCH v5 07/16] target/microblaze: Explode MO_TExx -> MO_TE | MO_xx Philippe Mathieu-Daudé
2025-02-06 13:10 ` [PATCH v5 08/16] target/microblaze: Set MO_TE once in do_load() / do_store() Philippe Mathieu-Daudé
2025-02-06 13:10 ` [PATCH v5 09/16] target/microblaze: Introduce mo_endian() helper Philippe Mathieu-Daudé
2025-02-06 13:10 ` [PATCH v5 10/16] target/microblaze: Consider endianness while translating code Philippe Mathieu-Daudé
2025-02-06 13:10 ` [PATCH v5 11/16] hw/microblaze: Support various endianness for s3adsp1800 machines Philippe Mathieu-Daudé
2025-02-06 13:20   ` Daniel P. Berrangé
2025-02-06 13:53     ` Philippe Mathieu-Daudé
2025-02-06 14:31       ` Daniel P. Berrangé [this message]
2025-02-06 15:04         ` Philippe Mathieu-Daudé
2025-02-06 17:08           ` Thomas Huth
2025-02-06 17:12           ` Daniel P. Berrangé
2025-02-06 17:49             ` Philippe Mathieu-Daudé
2025-02-06 18:06               ` Daniel P. Berrangé
2025-02-06 18:24                 ` Philippe Mathieu-Daudé
2025-02-06 18:29                   ` Daniel P. Berrangé
2025-02-06 18:43                     ` Philippe Mathieu-Daudé
2025-02-06 18:37                   ` Philippe Mathieu-Daudé
2025-02-06 17:34           ` Max Filippov
2025-02-06 17:44             ` Philippe Mathieu-Daudé
2025-02-11  9:22     ` Philippe Mathieu-Daudé
2025-02-06 13:10 ` [PATCH v5 12/16] tests/functional: Explicit endianness of microblaze assets Philippe Mathieu-Daudé
2025-02-06 13:10 ` [PATCH v5 13/16] tests/functional: Allow microblaze tests to take a machine name argument Philippe Mathieu-Daudé
2025-02-06 13:10 ` [PATCH v5 14/16] tests/functional: Remove sleep() kludges from microblaze tests Philippe Mathieu-Daudé
2025-02-06 17:10   ` Thomas Huth
2025-02-06 21:40   ` Richard Henderson
2025-02-06 13:10 ` [PATCH v5 15/16] tests/functional: Have microblaze tests inherit common parent class Philippe Mathieu-Daudé
2025-02-06 13:10 ` [PATCH v5 16/16] tests/functional: Run cross-endian microblaze tests Philippe Mathieu-Daudé
2025-02-10 20:35 ` [PATCH v5 00/16] hw/microblaze: Allow running cross-endian vCPUs Philippe Mathieu-Daudé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z6THtfjL6UVhBiW7@redhat.com \
    --to=berrange@redhat.com \
    --cc=alistair@alistair23.me \
    --cc=anjo@rev.ng \
    --cc=armbru@redhat.com \
    --cc=edgar.iglesias@gmail.com \
    --cc=jasowang@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=philmd@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.