Re: [PATCH 1/5] qapi/qom: Introduce kvm-pmu-filter object

[Markus Armbruster <armbru@redhat.com>]

Philippe, there's a question for you on target-specific QAPI schema.

Zhao Liu <zhao1.liu@intel.com> writes:

> Introduce the kvm-pmu-filter object and support the PMU event with raw
> format.
>
> The raw format, as a native PMU event code representation, can be used
> for several architectures.
>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> Tested-by: Yi Lai <yi1.lai@intel.com>

[...]

> diff --git a/accel/kvm/kvm-pmu.c b/accel/kvm/kvm-pmu.c
> new file mode 100644
> index 000000000000..22f749bf9183
> --- /dev/null
> +++ b/accel/kvm/kvm-pmu.c

[...]

> +static const TypeInfo kvm_pmu_filter_info = {
> +    .parent = TYPE_OBJECT,
> +    .name = TYPE_KVM_PMU_FILTER,
> +    .class_init = kvm_pmu_filter_class_init,
> +    .instance_size = sizeof(KVMPMUFilter),
> +    .instance_init = kvm_pmu_filter_instance_init,
> +    .interfaces = (InterfaceInfo[]) {
> +        { TYPE_USER_CREATABLE },
> +        { }
> +    }
> +};
> +
> +static void kvm_pmu_event_register_type(void)
> +{
> +    type_register_static(&kvm_pmu_filter_info);
> +}
> +
> +type_init(kvm_pmu_event_register_type);
> diff --git a/accel/kvm/meson.build b/accel/kvm/meson.build
> index 397a1fe1fd1e..dfab2854f3a8 100644
> --- a/accel/kvm/meson.build
> +++ b/accel/kvm/meson.build
> @@ -2,6 +2,7 @@ kvm_ss = ss.source_set()
>  kvm_ss.add(files(
>    'kvm-all.c',
>    'kvm-accel-ops.c',
> +  'kvm-pmu.c',
>  ))
>  
>  specific_ss.add_all(when: 'CONFIG_KVM', if_true: kvm_ss)

The new file is compiled into the binary when CONFIG_KVM.  Therefore,
object kvm-pmu-filter is available exactly then.  Makes sense.
However, ...

[...]

> diff --git a/qapi/kvm.json b/qapi/kvm.json
> new file mode 100644
> index 000000000000..1861d86a9726
> --- /dev/null
> +++ b/qapi/kvm.json
> @@ -0,0 +1,84 @@
> +# -*- Mode: Python -*-
> +# vim: filetype=python
> +#
> +# Copyright (C) 2025 Intel Corporation.
> +#
> +# Author: Zhao Liu <zhao1.liu@intel.com>
> +#
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +##
> +# == KVM
> +##
> +
> +##
> +# === PMU stuff (KVM related)
> +##
> +
> +##
> +# @KvmPmuFilterAction:
> +#
> +# Actions that KVM PMU filter supports.
> +#
> +# @deny: disable the PMU event/counter in KVM PMU filter.
> +#
> +# @allow: enable the PMU event/counter in KVM PMU filter.
> +#
> +# Since 10.1
> +##
> +{ 'enum': 'KvmPmuFilterAction',
> +  'data': ['allow', 'deny'] }
> +
> +##
> +# @KvmPmuEventFormat:
> +#
> +# Encoding formats of PMU event that QEMU/KVM supports.
> +#
> +# @raw: the encoded event code that KVM can directly consume.
> +#
> +# Since 10.1
> +##
> +{ 'enum': 'KvmPmuEventFormat',
> +  'data': ['raw'] }
> +
> +##
> +# @KvmPmuRawEvent:
> +#
> +# Raw PMU event code.
> +#
> +# @code: the raw value that has been encoded, and QEMU could deliver
> +#     to KVM directly.
> +#
> +# Since 10.1
> +##
> +{ 'struct': 'KvmPmuRawEvent',
> +  'data': { 'code': 'uint64' } }
> +
> +##
> +# @KvmPmuFilterEvent:
> +#
> +# PMU event filtered by KVM.
> +#
> +# @format: PMU event format.
> +#
> +# Since 10.1
> +##
> +{ 'union': 'KvmPmuFilterEvent',
> +  'base': { 'format': 'KvmPmuEventFormat' },
> +  'discriminator': 'format',
> +  'data': { 'raw': 'KvmPmuRawEvent' } }
> +
> +##
> +# @KvmPmuFilterProperties:
> +#
> +# Properties of KVM PMU Filter.
> +#
> +# @action: action that KVM PMU filter will take for selected PMU events.
> +#
> +# @events: list of selected PMU events.
> +#
> +# Since 10.1
> +##
> +{ 'struct': 'KvmPmuFilterProperties',
> +  'data': { 'action': 'KvmPmuFilterAction',
> +            '*events': ['KvmPmuFilterEvent'] } }

... the QAPI schema doesn't reflect that.

To make it reflect, we'd have to add 'if': 'CONFIG_KVM'.  Since
CONFIG_KVM can only be used in target-specific code, we'd have to put
the definitions in a target-specific schema module kvm-target.json.

This makes the headers generated for the module target-specific, which
can be inconvenient.  Whether it's inconvenient here, I can't say.

I understand target-specific QAPI modules are problematic for the single
binary / heterogeneous machine work.  Philippe, thoughts on this one?

[...]