On 4/17/25 11:38, Philippe Mathieu-Daudé wrote:
On 17/4/25 20:28, Pierrick Bouvier wrote:
Maybe it would be preferable to focus on providing a minimal but
*complete* TargetInfo before upstreaming any of this, as it's really
blocking the rest of the work for single binary.

I suppose I misunderstood you asking to post these reviewed patches as
separate of the TargetInfo series which need more work:

"I just feel the last 3 commits, and this one, are a bit disconnected
from the series."


You're right, it meant that the 4 commits (accel: *) are extra cleanups and not really related to the series title "Introduce TargetInfo API (for single binary)", which is fine to upstream by itself. However, introducing target_info.h partially just to do this cleanup first sounds a bit weird to me.

My complete thought was "This cleanup is ok, but please postpone it once we have TargetInfo API upstream". It's a remark I'll keep doing for TargetInfo, as the goal for v1 is to have a minimal API, introducing the concept of Machine and CPU types, and apply it to hw/arm, which was our initial need and why we talked about this in the first place.

Once it's there upstream, we can all enhance it in parallel, with the various needs we'll have for cleaning up other parts of the codebase.

My rationale is that it's easy to rebase our conflicting code against a common file, but it's hard to do it if it doesn't exist, thus my insistence on getting a minimal API first.

For instance, once it's upstream, we can easily add in different series (and different people):
- target_current() -> enum Target
- target_aarch64() -> bool
and progress without having to first be blocked by the existence of TargetInfo itself. We can as well cherry pick our mutual patches touching TargetInfo without getting blocked by the upstream process itself, and the first series to be pulled will make it available for everyone.

https://lore.kernel.org/qemu-devel/0b4376ee-504b-4096-a590-8a509ec78...@linaro.org/


Minimal requirements to have a complete series would be:
- Rename QMP TargetInfo struct to use that name
- Be able to query target cpu type (what this series does)
- Be able to query machine cpu type
- Modify generic functions listing machines/cpus to take this into account
- Tag labeled boards/cpu in hw/arm to prove this is working (without
doing any other cleanup to those files and *not* make them common)
- No other additional target information for the v1, let's keep that for
later.

Note: target_cpu_type will not be TYPE_ARM_CPU, as it wrongly wraps
arm32 and aarch64 cpus, while it should correctly identify one or the
other. I suggested TYPE_TARGET_CPU_ARM, TYPE_TARGET_CPU_AARCH64, and
same for machines: TYPE_TARGET_MACHINE_ARM, TYPE_TARGET_MACHINE_AARCH64.
So we can reuse this naming convention with any other target we'll reuse
in the future.

Got it.


Pierrick

Reply via email to