On Mon, 28 Apr 2025, Philippe Mathieu-Daudé wrote:
On 25/4/25 22:36, Pierrick Bouvier wrote:
On 4/25/25 13:29, BALATON Zoltan wrote:
On Fri, 25 Apr 2025, Pierrick Bouvier wrote:
On 4/25/25 02:43, BALATON Zoltan wrote:
On Thu, 24 Apr 2025, Pierrick Bouvier wrote:
On 4/24/25 17:16, BALATON Zoltan wrote:
On Fri, 25 Apr 2025, Philippe Mathieu-Daudé wrote:
A machine defined with the DEFINE_MACHINE_ARM_AARCH64() macro
will be available on qemu-system-arm and qemu-system-aarch64
binaries.
One defined with DEFINE_MACHINE_AARCH64() will only be available
in the qemu-system-aarch64 binary.
Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
---
include/hw/arm/machines-qom.h | 13 +++++++++++++
target/arm/machine.c | 12 ++++++++++++
2 files changed, 25 insertions(+)
diff --git a/include/hw/arm/machines-qom.h
b/include/hw/arm/machines-qom.h
index a17225f5f92..6277ee986d9 100644
--- a/include/hw/arm/machines-qom.h
+++ b/include/hw/arm/machines-qom.h
@@ -9,10 +9,23 @@
#ifndef HW_ARM_MACHINES_QOM_H
#define HW_ARM_MACHINES_QOM_H
+#include "hw/boards.h"
+
#define TYPE_TARGET_ARM_MACHINE \
"target-info-arm-machine"
#define TYPE_TARGET_AARCH64_MACHINE \
"target-info-aarch64-machine"
+extern InterfaceInfo arm_aarch64_machine_interfaces[];
+extern InterfaceInfo aarch64_machine_interfaces[];
+
+#define DEFINE_MACHINE_ARM_AARCH64(namestr, machine_initfn) \
+ DEFINE_MACHINE_WITH_INTERFACES(namestr, machine_initfn, \
+
arm_aarch64_machine_interfaces)
+
+#define DEFINE_MACHINE_AARCH64(namestr, machine_initfn) \
+ DEFINE_MACHINE_WITH_INTERFACES(namestr, machine_initfn, \
+ aarch64_machine_interfaces)
+
#endif
diff --git a/target/arm/machine.c b/target/arm/machine.c
index 978249fb71b..193c7a9cff0 100644
--- a/target/arm/machine.c
+++ b/target/arm/machine.c
@@ -8,6 +8,7 @@
#include "cpu-features.h"
#include "migration/cpu.h"
#include "target/arm/gtimer.h"
+#include "hw/arm/machines-qom.h"
static bool vfp_needed(void *opaque)
{
@@ -1111,3 +1112,14 @@ const VMStateDescription vmstate_arm_cpu = {
NULL
}
};
+
+InterfaceInfo arm_aarch64_machine_interfaces[] = {
+ { TYPE_TARGET_ARM_MACHINE },
+ { TYPE_TARGET_AARCH64_MACHINE },
+ { }
+};
+
+InterfaceInfo aarch64_machine_interfaces[] = {
+ { TYPE_TARGET_AARCH64_MACHINE },
+ { }
+};
Why do you need these? If you define DEFINE_MACHINE_WITH_INTERFACES as
OBJECT_DEFINE_TYPE_WITH_INTERFACES then you can write:
This was requested in v4 by Richard to remove anonymous array
duplication
in
.data.
DEFINE_MACHINE_WITH_INTERFACES(name, initfn, { TYPE_TARGET_ARM_MACHINE
},
{ TYPE_TARGET_AARCH64_MACHINE }, { })
and no more macros needed. Ideally those places that are now blown up
should use DEFINE_MACHINE too. Maybe they don't yet because the parent
type is hardcoded so we should really have
Not sure what you mean by "no more macros needed".
No other specialised macros needed for each machine type other than
DEFINE_MACHINE_WITH_INTERFACES or DEFINE_MACHINE_EXTENDED. So I
suggested
to keep DEFINE_MACHINE by making it more general so it can cover the new
uses instead of bringing back the boiler plate and losing the clarity
hinding these behind the macros.
This is exactly what we have in this series.
Patch 7 introduces DEFINE_MACHINE_WITH_INTERFACES.
I guess Philippe chose a new name to avoid modifying all existing
DEFINE_MACHINE, and I think it's understandable, as we want those changes
to
impact hw/arm only first. That said, it would be very easy to
refactor/modify
later, so it's not a big deal.
This patch introduces DEFINE_MACHINE_ARM_AARCH64 and
DEFINE_MACHINE_AARCH64.
Is the problem with those specialized DEFINE_MACHINE_{ARM, AARCH64}
definition?
If yes, and if you prefer an explicit DEFINE_MACHINE_WITH_INTERFACES(...,
arm_aarch64_machine_interfaces), I'm sure Philippe would be open to make
such
a change to satisfy reviews.
Let's just try to decide something, and move on.
arm_aarch64_machine_interfaces or aarch64_machine_interfaces are arrays
(defined only once), which are passed as a parameter to
DEFINE_MACHINE_WITH_INTERFACES, or manually set with ".interfaces =".
Look at how OBJECT_DEFINE_TYPE_WITH_INTERFACES is defined.
This macro is not used for any machine definition so far, and
DEFINE_MACHINE
is the "standard" macro used, at least the one most commonly used in the
codebase. So it makes sense to simply expand the latter.
I was referring to that as an example how a DEFINE_MACHINE_WITH_INTERFACES
should work not suggesting to use OBJECT_DEFINE_TYPE_WITH_INTERFACES.
DEFINE_MACHINE_EXTENDED(name, parent, initfn, interfaces...)
and remove more bolier plate that way?
Could you can share a concrete example of what you expect, with the new
macros to add, and how to use them for a given board?
I tried to do that in this message you replied to.
If you refer to "DEFINE_MACHINE_EXTENDED(name, parent, initfn,
interfaces...)", this is almost exactly what patch 7 is introducing with
DEFINE_MACHINE_WITH_INTERFACES(namestr, machine_initfn, ifaces).
The difference is that OBJECT_DEFINE_TYPE_WITH_INTERFACES takes a list of
interfaces and defines the array itself and you pass the array which is
limiting as you then need to define a lot of arrays to pass to your macro
instead of only passing the elements and let it define tha array.
I just want to see instead of
static const TypeInfo machine_types[] = {
...lots of boiler plate code here
};
something like
DEFINE_MACHINE_EXTENDED(machine1, TYPE_WHATEVER_MACHINE, {INTERFACE1},
{INTERFACE2}, {})
DEFINE_MACHINE_EXTENDED(machine2, TYPE_OTHER_MACHINE, {INTERFACE1},
{INTERFACE3}, {})
DEFINE_MACHINE_EXTENDED(machine3, TYPE_THIRD_MACHINE, {INTERFACE1}, {})
Ok, I understand better.
It was my point as well on v4, that introducing those symbols is less
readable and less scalable, for a negligible benefit in terms of code size,
which was the primary concern.
We can always reconsider this later, especially when adding another
architecture to single binary, it's not a problem and something set in
stone.
Would you be ok if we proceed with the current version, knowing those
limitations, for now?
If Zoltan disagrees, we need Richard to agree to go back on v4.
Keep in mind that what we are trying to achieve is quite more complex
than code style or .rodata savings, besides we eventually want to have
dynamic machines & DSL.
Since you are touching the lines using DEFINE_MACHINE it's a good
opportunity to change the macro to be more general to be able to keep
using it instead of replacing it with the boiler plate it's supposed to
hide. Adding one or two more parameters to the macro is not a big change
so I don't see why you don't want to do it. This could be addressed later
to revert to use the macro again but in practice it will not be addressed
because everybody will be busy doing other things and doing that now would
prevent some churn. I too, don't like doing unrelated clean up which is
not the main goal, but if it's not much more work then it's not
unreasonable to do it. I only oppose to that if it's a lot of work so I
would not ask such change but what I asked is not unrelated and quite
simple change.
That said, I can't stop you so if you still don't want to do it now then
you can move on. I don't care that much as long as you stay within hw/arm,
but will raise my concern again when you submit a similar patch that
touches parts I care more about. If others don't think it's a problem and
not bothered by the boiler plate code then it's not so important but
otherwise I think I have a valid point. I remember when I started to get
to know QEMU it was quite difficult to wade through all the QOM boiler
plate just to see what is related to the actual functionality. These
macros help to make code more readable and accessible for new people.
Regards,
BALATON Zoltan
