All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com>
To: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: "Philippe Mathieu-Daudé" <philmd@oss.qualcomm.com>,
	qemu-devel@nongnu.org, "Nicholas Piggin" <npiggin@gmail.com>,
	"Daniel Henrique Barboza" <daniel.barboza@oss.qualcomm.com>,
	"Bernhard Beschow" <shentey@gmail.com>,
	"Anton Johansson" <anjo@rev.ng>,
	"Alistair Francis" <alistair@alistair23.me>,
	"Peter Maydell" <peter.maydell@linaro.org>,
	"Alistair Francis" <alistair.francis@wdc.com>
Subject: Re: [PATCH 01/27] include/qemu/target-info-qom.h: declare TYPE_TARGET_SPECIFIC interface
Date: Mon, 10 Aug 2026 15:26:26 -0700	[thread overview]
Message-ID: <102a3de8-ac17-429a-9294-e3b159aa642a@oss.qualcomm.com> (raw)
In-Reply-To: <anTPOqFKWFKAX_3C@redhat.com>

On 8/6/2026 11:15 AM, Daniel P. Berrangé wrote:
> On Thu, Aug 06, 2026 at 09:38:43AM -0700, Pierrick Bouvier wrote:
>> On 8/6/2026 7:52 AM, Philippe Mathieu-Daudé wrote:
>>> On 6/8/26 12:21, Daniel P. Berrangé wrote:
>>>> On Wed, Aug 05, 2026 at 12:10:27PM -0700, Pierrick Bouvier wrote:
>>>>> On 8/5/2026 9:53 AM, Daniel P. Berrangé wrote:
>>>>>> On Wed, Aug 05, 2026 at 09:16:20AM -0700, Pierrick Bouvier wrote:
>>>>>>> On 8/5/2026 7:05 AM, Daniel P. Berrangé wrote:
>>>>>>>> On Fri, Jul 24, 2026 at 12:09:21AM +0000, Pierrick Bouvier wrote:
>>>>>>>>> In the next commits, We'll replace the logic to filter QOM types per
>>>>>>>>> target from a static one (based on INTERFACES) to a runtime one,
>>>>>>>>> based
>>>>>>>>> on is_available() function, that can be overriden per class.
>>>>>>>>>
>>>>>>>>> Introduce the new interface we'll use for that.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com>
>>>>>>>>> ---
>>>>>>>>>   include/qemu/target-info-qom.h | 15 +++++++++++++++
>>>>>>>>>   target-info-qom.c              |  5 +++++
>>>>>>>>>   2 files changed, 20 insertions(+)
>>>>>>>>>
>>>>>>>>> diff --git a/include/qemu/target-info-qom.h b/include/qemu/
>>>>>>>>> target-info-qom.h
>>>>>>>>> index 91be415ed33..83eb537333b 100644
>>>>>>>>> --- a/include/qemu/target-info-qom.h
>>>>>>>>> +++ b/include/qemu/target-info-qom.h
>>>>>>>>> @@ -14,6 +14,21 @@
>>>>>>>>>     #define TYPE_TARGET_INFO "target-info"
>>>>>>>>>   +#define TYPE_TARGET_SPECIFIC "target-specific"
>>>>>>>>> +
>>>>>>>>> +typedef struct TargetSpecific TargetSpecific;
>>>>>>>>> +
>>>>>>>>> +typedef struct TargetSpecificClass {
>>>>>>>>> +    InterfaceClass parent_class;
>>>>>>>>> +
>>>>>>>>> +    bool (*is_available)(void);
>>>>>>>>> +} TargetSpecificClass;
>>>>>>>>> +
>>>>>>>>> +#define TARGET_SPECIFIC(obj) \
>>>>>>>>> +    INTERFACE_CHECK(TargetSpecific, (obj), TYPE_TARGET_SPECIFIC)
>>>>>>>>> +DECLARE_CLASS_CHECKERS(TargetSpecificClass, TARGET_SPECIFIC,
>>>>>>>>> +                       TYPE_TARGET_SPECIFIC)
>>>>>>>>
>>>>>>>> Looking through the series,I don't really see the point
>>>>>>>> in this interface.   Why is this not possible to do by
>>>>>>>> adding 'is_available' to MachineClass.  It would make
>>>>>>>> the rest of the series simpler and especially avoid the
>>>>>>>> need to introduced yet more series of macros for defining
>>>>>>>> machine classes.
>>>>>>>>
>>>>>>>
>>>>>>> We'll need the exact same interface for cpus, and devices also.
>>>>>>> IMHO, it
>>>>>>> makes sense to have this in an external interface, instead of
>>>>>>> forcing it
>>>>>>> to be present in all cpus/devices/machines. I also considered
>>>>>>> adding it
>>>>>>> directly in Object class directly (would be the simplest), but I
>>>>>>> felt it
>>>>>>> would be hard to motivate it.
>>>>>>
>>>>>> I don't see a need for the common interface across cpus/devices/etc as
>>>>>> as code that's filtering only cares about the specific types. It also
>>>>>> definitely doesn't beloong in Object class, but the Object class could
>>>>>> be changed to make it simpler.
>>>>>>
>>>>>
>>>>> We agree on this.
>>>>>
>>>>>> The object_class_get_list() method could get a 'bool
>>>>>> filter(ObjectClass *cl)'
>>>>>> callback which could be invoked on each class to filter it.
>>>>>>
>>>>>> That said I find it pretty undesirable as an approach that we're
>>>>>> registering classes that can't then be used in a given situation.
>>>>>> This has a ripple effect where every bit of code that iterates over
>>>>>> classes needs changing to add filtering after the fact. It is also
>>>>>> not great for scalability, as it means every QEMU process will have
>>>>>> the union of all classes for all targets registered, most of which
>>>>>> have to be discarded / ignored at runtime.
>>>>>>
>>>>>
>>>>> This is inherent to the nature of having a single binary.
>>>>> We need to cover those two requirements:
>>>>> 1. having a filter mechanism (per target)
>>>>> 2. have all the classes accessible for the heterogeneous machines that
>>>>> will be coming in the future
>>>>>
>>>>> 1. could be covered by what you describe, however, it breaks 2. For
>>>>> this, you need to be able to register all types by design.
>>>>
>>>>
>>>> Even the heterogeneous machines aren't going to need all the
>>>> classes from all 30+ targets that QEMU supports.
>>>>
>>>> IIUC, the current approach relies on '--target <blah>' to select
>>>> which target we need. Would the heterogeneous machines not just
>>>> change that to allow "--target <this> --target <that>". It still
>>>> looks like we should be able to significantly limit  what we
>>>> register for heterogeneous machines.
>>>
>>> Heterogeneous binary won't filter anything at runtime (if we want
>>> to filter components we already have Kconfig at compile time).
>>>
>>> "--target <foo>" is only needed to have a single binary backward
>>> compatible. If you use it, you fall back to single architecture
>>> (our current binaries). If you want anything heterogeneous, you
>>> can not use it. This will be by design.
>>
>> I've been thinking about it yesterday, and we could apply the paradigm
>> "register only classes that will be used". For heterogeneous machine,
>> either we'll provide a "none" target, which enables all target, or use
>> --target aarch64,riscv64, like Daniel proposed.
>>
>> I will implement what Daniel asked for v2.
>> However, please be aware it will be probably be more invasive and
>> verbose, since we'll need to add macros and stuff to have conditional
>> type_register_static.
>>
>> I would like to avoid doing a change you asked, and have no further
>> comment after that or something like "it was better before".
>> Do we agree on this Daniel?
> 
> That is an unreasonable request - I can't approve of a patch series
> that doesn't exist yet.
> 
> I can only say that I don't think the current series is a desirable
> approach and make suggestions of what I think is (hopefully) a
> better way.
> 
> 
> Something else that I didn't consider previously is possible interaction
> with modules. Much of our use of modules is to make backends loadable,
> but we've got a few loadable devices too. It seems likely that with the
> single binary model we'll have greater motivation to make more device &
> machine code into loadable modules, as people have repeatedly pushed us
> to have a lower mandatory footprint for QEMU.
> 
> The loadable modules relies on some metadata defined in the source
> code alongside the type info. For example
> 
>   static const TypeInfo emulated_card_info = {
>     .name          = TYPE_EMULATED_CCID,
>     .parent        = TYPE_CCID_CARD,
>     .instance_size = sizeof(EmulatedState),
>     .class_init    = emulated_class_initfn,
>   };
>   module_obj(TYPE_EMULATED_CCID);
>   module_kconfig(USB);
> 
> In particular it feels like the module_kconfig() and module_arch()
> metadata have overlap with what you're trying todo in this series.
> 
> So I wonder if we could generalize the module metadata so that it
> serves a broader set of use cases. Instead of having to manually
> write conditional logic in XXX_register_type() methods, perhaps we
> can do the right thing automatically in type_register() using the
> metadata we collect from the code ?
>

I have been experimenting today with static filtering of types, as you
suggested, and came up with two possibilities. Before implementing this
for all types, I would like to get some feedback on which way is the
best for you.

1. Add a new callback is_available to TypeInfo, similar to existing
interface.
Pros: local information is present in type definition, making obvious
      why a type is available or not. Very easy to implement
      since only type_register_static has to be modified to check it.
Cons: Add a new field to each existing type. Not the end of the world
      in terms of memory consumption, but worth mentioning.
2. Add conditional around each type_register_static, or add an
alternative type_register_static_cond function.
Pros: I don't see any
Cons: Decouples type information from type definition, makes it harder
      to follow why a type is available or not. Much more verbose since
      we need to modify all type_register_static buried in macros.

Based on this, I'm much more in favor or 1. The fact availability
information is in the same location than type definition is the biggest
advantage for me.

On example given above, this would give something like this:
static const TypeInfo emulated_card_info = {
     .name          = TYPE_EMULATED_CCID,
     .parent        = TYPE_CCID_CARD,
     .instance_size = sizeof(EmulatedState),
     .class_init    = emulated_class_initfn,
     .is_available  = target_config_X,
};

Would that work for you?
Do you have a 3rd way to offer to solve the problem we have?

> With regards,
> Daniel

Thanks,
Pierrick


  parent reply	other threads:[~2026-08-10 22:27 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-24  0:09 [PATCH 00/27] single-binary: implement dynamic filtering for machine types Pierrick Bouvier
2026-07-24  0:09 ` [PATCH 01/27] include/qemu/target-info-qom.h: declare TYPE_TARGET_SPECIFIC interface Pierrick Bouvier
2026-07-24  6:21   ` Philippe Mathieu-Daudé
2026-07-24 16:54     ` Pierrick Bouvier
2026-08-05 14:05   ` Daniel P. Berrangé
2026-08-05 16:16     ` Pierrick Bouvier
2026-08-05 16:53       ` Daniel P. Berrangé
2026-08-05 19:10         ` Pierrick Bouvier
2026-08-06 10:21           ` Daniel P. Berrangé
2026-08-06 14:52             ` Philippe Mathieu-Daudé
2026-08-06 16:38               ` Pierrick Bouvier
2026-08-06 18:15                 ` Daniel P. Berrangé
2026-08-06 21:04                   ` Pierrick Bouvier
2026-08-10 22:26                   ` Pierrick Bouvier [this message]
2026-08-06 16:52               ` Daniel P. Berrangé
2026-08-06 20:21                 ` Pierrick Bouvier
2026-08-10 15:56                   ` Daniel P. Berrangé
2026-08-10 16:11                     ` Pierrick Bouvier
2026-07-24  0:09 ` [PATCH 02/27] hw/arm: implement TYPE_TARGET_SPECIFIC Pierrick Bouvier
2026-07-24  0:09 ` [PATCH 03/27] target-info: add target_riscv32 and target_base_riscv Pierrick Bouvier
2026-07-24  6:09   ` Philippe Mathieu-Daudé
2026-07-24  0:09 ` [PATCH 04/27] hw/riscv: implement TYPE_TARGET_SPECIFIC Pierrick Bouvier
2026-07-24  0:09 ` [PATCH 05/27] target-info: add target_config_multiprocess Pierrick Bouvier
2026-07-24  0:09 ` [PATCH 06/27] hw/remote/machine: remove unsupported arm target Pierrick Bouvier
2026-07-24  0:09 ` [PATCH 07/27] hw/remote/machine: implement TYPE_TARGET_SPECIFIC Pierrick Bouvier
2026-07-24  0:09 ` [PATCH 08/27] target-info: add target_config_xen Pierrick Bouvier
2026-07-24  0:09 ` [PATCH 09/27] hw/arm/xen-pvh: implement TYPE_TARGET_SPECIFIC Pierrick Bouvier
2026-07-24  0:09 ` [PATCH 10/27] hw/xenpv/xen_machine_pv: " Pierrick Bouvier
2026-07-24  0:09 ` [PATCH 11/27] target-info: add target_config_nitro Pierrick Bouvier
2026-07-24  0:09 ` [PATCH 12/27] hw/nitro/machine: implement TYPE_TARGET_SPECIFIC Pierrick Bouvier
2026-07-24  0:09 ` [PATCH 13/27] target-info-qom: implement new machine filtering per target Pierrick Bouvier
2026-07-24  6:12   ` Philippe Mathieu-Daudé
2026-07-24  0:09 ` [PATCH 14/27] target-info-qom: use TYPE_MACHINE instead of target_machine_typename Pierrick Bouvier
2026-07-24  6:12   ` Philippe Mathieu-Daudé
2026-07-24  0:09 ` [PATCH 15/27] target-info: remove target_machine_typename Pierrick Bouvier
2026-07-24  0:09 ` [PATCH 16/27] target-info-qom: add type_target_specific Pierrick Bouvier
2026-07-24  0:09 ` [PATCH 17/27] hw/arm: remove TYPE_TARGET_{AARCH64,ARM}_MACHINE Pierrick Bouvier
2026-07-24  0:09 ` [PATCH 18/27] hw/arm: remove {arm, arm_aarch64, aarch64}_machine_interfaces Pierrick Bouvier via qemu development
2026-07-24  0:09 ` [PATCH 19/27] include/hw/core/boards.h: add DEFINE_MACHINE_TARGET_SPECIFIC Pierrick Bouvier
2026-07-24  0:09 ` [PATCH 20/27] hw/arm: remove DEFINE_MACHINE_{AARCH64,ARM} Pierrick Bouvier
2026-07-24  0:09 ` [PATCH 21/27] hw/arm: remove machines-qom.h Pierrick Bouvier
2026-07-24  0:09 ` [PATCH 22/27] hw/riscv: remove TYPE_TARGET_{RISCV32,RISCV64}_MACHINE Pierrick Bouvier
2026-07-24  0:09 ` [PATCH 23/27] hw/riscv: remove {riscv32, riscv32_64, riscv64}_machine_interfaces Pierrick Bouvier via qemu development
2026-07-24  0:09 ` [PATCH 24/27] hw/riscv: remove DEFINE_MACHINE_{RISCV32,RISCV64} Pierrick Bouvier
2026-07-24  0:09 ` [PATCH 25/27] hw/riscv: remove machines-qom.h Pierrick Bouvier
2026-07-24  0:09 ` [PATCH 26/27] configs/targets: remove target info definitions Pierrick Bouvier
2026-07-24  0:09 ` [PATCH 27/27] target-info: rename target-info-stub.c in target-info-def.c Pierrick Bouvier
2026-07-30 17:33 ` [PATCH 00/27] single-binary: implement dynamic filtering for machine types Pierrick Bouvier
2026-08-05 13:30   ` Yonggang Luo
2026-08-05 16:20     ` Pierrick Bouvier
2026-08-06 20:29       ` Daniel Henrique Barboza
2026-08-10 15:46 ` Peter Maydell
2026-08-10 15:55   ` Pierrick Bouvier
2026-08-10 16:00     ` Peter Maydell
2026-08-10 16:14       ` Pierrick Bouvier
2026-08-10 16:42         ` Daniel P. Berrangé

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=102a3de8-ac17-429a-9294-e3b159aa642a@oss.qualcomm.com \
    --to=pierrick.bouvier@oss.qualcomm.com \
    --cc=alistair.francis@wdc.com \
    --cc=alistair@alistair23.me \
    --cc=anjo@rev.ng \
    --cc=berrange@redhat.com \
    --cc=daniel.barboza@oss.qualcomm.com \
    --cc=npiggin@gmail.com \
    --cc=peter.maydell@linaro.org \
    --cc=philmd@oss.qualcomm.com \
    --cc=qemu-devel@nongnu.org \
    --cc=shentey@gmail.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.