Re: [RFC v2 1/5] qapi/qom: Introduce kvm-pmu-filter object
Zhao Liu writes:
> Hi Mrkus,
>
> I'm really sorry I completely missed your reply (and your patient
> advice). It wasn't until I looked back at the lore archives that I
> realized my mistake. Thinking it over again, I see that your reply,
> which I missed, really helped clear up my confusion:
I'm glad I was able to help some!
> On Fri, Feb 07, 2025 at 02:02:44PM +0100, Markus Armbruster wrote:
>> Date: Fri, 07 Feb 2025 14:02:44 +0100
>> From: Markus Armbruster
>> Subject: Re: [RFC v2 1/5] qapi/qom: Introduce kvm-pmu-filter object
>>
>> Zhao Liu writes:
>>
>> >> Let's ignore how to place it for now, and focus on where we would *like*
>> >> to place it.
>> >>
>> >> Is it related to anything other than ObjectType / ObjectOptions in the
>> >> QMP reference manual?
>> >
>> > Yes!
>>
>> Now I'm confused :)
>>
>> It is related to ObjectType / ObjectType.
>>
>> Is it related to anything else in the QMP reference manual, and if yes,
>> to what exactly is it related?
>
> I misunderstood your point. The PMU stuff and the QAPI definitions for
> ObjectType/ObjectOptions are not related. They should belong to separate
> categories or sections.
>
>> >> I guess qapi/kvm.json is for KVM-specific stuff in general, not just the
>> >> KVM PMU filter. Should we have a section for accelerator-specific
>> >> stuff, with subsections for the various accelerators?
>> >>
>> >> [...]
>> >
>> > If we consider the accelerator from a top-down perspective, I understand
>> > that we need to add accelerator.json, kvm.json, and kvm-pmu-filter.json.
>> >
>> > The first two files are just to include subsections without any additional
>> > content. Is this overkill? Could we just add a single kvm-pmu-filter.json
>> > (I also considered this name, thinking that kvm might need to add more
>> > things in the future)?
>> >
>> > Of course, I lack experience with the file organization here. If you think
>> > the three-level sections (accelerator.json, kvm.json, and
>> > kvm-pmu-filter.json)
>> > is necessary, I am happy to try this way. :-)
>>
>> We don't have to create files just to get a desired section structure.
>>
>> I'll show you how in a jiffie, but before I do that, let me stress: we
>> should figure out what we want *first*, and only then how to get it.
>> So, what section structure would make the most sense for the QMP
>> reference manual?
>
> Thank you for your patience. I have revisited and carefully considered
> the "QEMU QMP Reference Manual," especially from a reader's perspective.
> Indeed, I agree that, as you mentioned, a three-level directory
> (accelerator - kvm - kvm stuff) is more readable and easier to maintain.
Sounds good to me.
> For this question "what we want *first*, and only then how to get it", I
> think my thought is:
>
> First, the structure should be considered, and then the specific content
> can be added. Once the structure is clearly defined, categorizing items
> into their appropriate places becomes a natural process...
>
> Then for this question "what section structure would make the most sense
> for the QMP reference manual?", I understand that a top-down, clearly
> defined hierarchical directory makes the most sense, allowing readers to
> follow the structure to find what they want. Directly adding
> kvm-pmu-filter.json or kvm.json would disrupt the entire structure, because
> KVM is just one of the accelerators supported by QEMU. Using "accelerator"
> as the entry point for the documentation, similar to the "accel" directory
> in QEMU's source code, would make indexing more convenient.
I think so, too.
>> A few hints on how...
>>
>> Consider how qapi/block.json includes qapi/block-core.json:
>>
>> ##
>> # = Block devices
>> ##
>>
>> { 'include': 'block-core.json' }
>>
>> ##
>> # == Additional block stuff (VM related)
>> ##
>>
>> block-core.json starts with
>>
>> ##
>> # == Block core (VM unrelated)
>> ##
>>
>> Together, this produces this section structure
>>
>> = Block devices
>> ==
>> ##
>>
>> Together, this produces this section structure
>>
>> = Block devices
>> == Block core (VM unrelated)
>> == Additional block stuff (VM related)
>>
>&
Re: [RFC v2 1/5] qapi/qom: Introduce kvm-pmu-filter object
Hi Mrkus,
I'm really sorry I completely missed your reply (and your patient
advice). It wasn't until I looked back at the lore archives that I
realized my mistake. Thinking it over again, I see that your reply,
which I missed, really helped clear up my confusion:
On Fri, Feb 07, 2025 at 02:02:44PM +0100, Markus Armbruster wrote:
> Date: Fri, 07 Feb 2025 14:02:44 +0100
> From: Markus Armbruster
> Subject: Re: [RFC v2 1/5] qapi/qom: Introduce kvm-pmu-filter object
>
> Zhao Liu writes:
>
> >> Let's ignore how to place it for now, and focus on where we would *like*
> >> to place it.
> >>
> >> Is it related to anything other than ObjectType / ObjectOptions in the
> >> QMP reference manual?
> >
> > Yes!
>
> Now I'm confused :)
>
> It is related to ObjectType / ObjectType.
>
> Is it related to anything else in the QMP reference manual, and if yes,
> to what exactly is it related?
I misunderstood your point. The PMU stuff and the QAPI definitions for
ObjectType/ObjectOptions are not related. They should belong to separate
categories or sections.
> >> I guess qapi/kvm.json is for KVM-specific stuff in general, not just the
> >> KVM PMU filter. Should we have a section for accelerator-specific
> >> stuff, with subsections for the various accelerators?
> >>
> >> [...]
> >
> > If we consider the accelerator from a top-down perspective, I understand
> > that we need to add accelerator.json, kvm.json, and kvm-pmu-filter.json.
> >
> > The first two files are just to include subsections without any additional
> > content. Is this overkill? Could we just add a single kvm-pmu-filter.json
> > (I also considered this name, thinking that kvm might need to add more
> > things in the future)?
> >
> > Of course, I lack experience with the file organization here. If you think
> > the three-level sections (accelerator.json, kvm.json, and
> > kvm-pmu-filter.json)
> > is necessary, I am happy to try this way. :-)
>
> We don't have to create files just to get a desired section structure.
>
> I'll show you how in a jiffie, but before I do that, let me stress: we
> should figure out what we want *first*, and only then how to get it.
> So, what section structure would make the most sense for the QMP
> reference manual?
Thank you for your patience. I have revisited and carefully considered
the "QEMU QMP Reference Manual," especially from a reader's perspective.
Indeed, I agree that, as you mentioned, a three-level directory
(accelerator - kvm - kvm stuff) is more readable and easier to maintain.
For this question "what we want *first*, and only then how to get it", I
think my thought is:
First, the structure should be considered, and then the specific content
can be added. Once the structure is clearly defined, categorizing items
into their appropriate places becomes a natural process...
Then for this question "what section structure would make the most sense
for the QMP reference manual?", I understand that a top-down, clearly
defined hierarchical directory makes the most sense, allowing readers to
follow the structure to find what they want. Directly adding
kvm-pmu-filter.json or kvm.json would disrupt the entire structure, because
KVM is just one of the accelerators supported by QEMU. Using "accelerator"
as the entry point for the documentation, similar to the "accel" directory
in QEMU's source code, would make indexing more convenient.
> A few hints on how...
>
> Consider how qapi/block.json includes qapi/block-core.json:
>
> ##
> # = Block devices
> ##
>
> { 'include': 'block-core.json' }
>
> ##
> # == Additional block stuff (VM related)
> ##
>
> block-core.json starts with
>
> ##
> # == Block core (VM unrelated)
> ##
>
> Together, this produces this section structure
>
> = Block devices
> ==
> ##
>
> Together, this produces this section structure
>
> = Block devices
> == Block core (VM unrelated)
> == Additional block stuff (VM related)
>
> Note that qapi/block-core.json isn't included anywhere else.
> qapi/qapi-schema.json advises:
>
> # Documentation generated with qapi-gen.py is in source order, with
> # included sub-schemas inserted at the first include directive
> # (subsequent include directives have no effect). To get a sane and
> # stable order, it's best to include each sub-schema just once, or
> # include it first right here.
Thank you very much!!
Based on your inspiration, I think the ideal section structure for my
issue could be:
= accelerator
== KVM
=== PMU
Firstly, I should have a new accelerator.json () to include KVM stuff:
##
# = Accelerator
##
{ 'include': 'kvm.json' }
Next, in kvm.json, I could organize stuffs like:
##
# == KVM
##
##
# === PMU stuff
##
... (the below are my current QPAI definitions.)
Is such a structure reasonable?
Thank you again for your guidance!
Regards,
Zhao
Re: [RFC v2 1/5] qapi/qom: Introduce kvm-pmu-filter object
Zhao Liu writes:
>> Let's ignore how to place it for now, and focus on where we would *like*
>> to place it.
>>
>> Is it related to anything other than ObjectType / ObjectOptions in the
>> QMP reference manual?
>
> Yes!
Now I'm confused :)
It is related to ObjectType / ObjectType.
Is it related to anything else in the QMP reference manual, and if yes,
to what exactly is it related?
>> I guess qapi/kvm.json is for KVM-specific stuff in general, not just the
>> KVM PMU filter. Should we have a section for accelerator-specific
>> stuff, with subsections for the various accelerators?
>>
>> [...]
>
> If we consider the accelerator from a top-down perspective, I understand
> that we need to add accelerator.json, kvm.json, and kvm-pmu-filter.json.
>
> The first two files are just to include subsections without any additional
> content. Is this overkill? Could we just add a single kvm-pmu-filter.json
> (I also considered this name, thinking that kvm might need to add more
> things in the future)?
>
> Of course, I lack experience with the file organization here. If you think
> the three-level sections (accelerator.json, kvm.json, and kvm-pmu-filter.json)
> is necessary, I am happy to try this way. :-)
We don't have to create files just to get a desired section structure.
I'll show you how in a jiffie, but before I do that, let me stress: we
should figure out what we want *first*, and only then how to get it.
So, what section structure would make the most sense for the QMP
reference manual?
A few hints on how...
Consider how qapi/block.json includes qapi/block-core.json:
##
# = Block devices
##
{ 'include': 'block-core.json' }
##
# == Additional block stuff (VM related)
##
block-core.json starts with
##
# == Block core (VM unrelated)
##
Together, this produces this section structure
= Block devices
==
##
Together, this produces this section structure
= Block devices
== Block core (VM unrelated)
== Additional block stuff (VM related)
Note that qapi/block-core.json isn't included anywhere else.
qapi/qapi-schema.json advises:
# Documentation generated with qapi-gen.py is in source order, with
# included sub-schemas inserted at the first include directive
# (subsequent include directives have no effect). To get a sane and
# stable order, it's best to include each sub-schema just once, or
# include it first right here.
Re: [RFC v2 1/5] qapi/qom: Introduce kvm-pmu-filter object
> Let's ignore how to place it for now, and focus on where we would *like* > to place it. > > Is it related to anything other than ObjectType / ObjectOptions in the > QMP reference manual? Yes! > I guess qapi/kvm.json is for KVM-specific stuff in general, not just the > KVM PMU filter. Should we have a section for accelerator-specific > stuff, with subsections for the various accelerators? > > [...] If we consider the accelerator from a top-down perspective, I understand that we need to add accelerator.json, kvm.json, and kvm-pmu-filter.json. The first two files are just to include subsections without any additional content. Is this overkill? Could we just add a single kvm-pmu-filter.json (I also considered this name, thinking that kvm might need to add more things in the future)? Of course, I lack experience with the file organization here. If you think the three-level sections (accelerator.json, kvm.json, and kvm-pmu-filter.json) is necessary, I am happy to try this way. :-)
Re: [RFC v2 1/5] qapi/qom: Introduce kvm-pmu-filter object
Zhao Liu writes:
> On Wed, Feb 05, 2025 at 11:03:51AM +0100, Markus Armbruster wrote:
>> Date: Wed, 05 Feb 2025 11:03:51 +0100
>> From: Markus Armbruster
>> Subject: Re: [RFC v2 1/5] qapi/qom: Introduce kvm-pmu-filter object
>>
>> Quick & superficial review for now.
>
> Thanks!
>
>> > diff --git a/qapi/kvm.json b/qapi/kvm.json
>> > new file mode 100644
>> > index ..d51aeeba7cd8
>> > --- /dev/null
>> > +++ b/qapi/kvm.json
>> > @@ -0,0 +1,116 @@
>> > +# -*- Mode: Python -*-
>> > +# vim: filetype=python
>> > +
>> > +##
>> > +# = KVM based feature API
>>
>> This is a top-level section. It ends up between sections "QMP
>> introspection" and "QEMU Object Model (QOM)". Is this what we want? Or
>> should it be a sub-section of something? Or next to something else?
>
> Do you mean it's not in the right place in the qapi-schema.json?
>
> diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
> index b1581988e4eb..742818d16e45 100644
> --- a/qapi/qapi-schema.json
> +++ b/qapi/qapi-schema.json
> @@ -64,6 +64,7 @@
> { 'include': 'compat.json' }
> { 'include': 'control.json' }
> { 'include': 'introspect.json' }
> +{ 'include': 'kvm.json' }
> { 'include': 'qom.json' }
> { 'include': 'qdev.json' }
> { 'include': 'machine-common.json' }
>
> Because qom.json includes kvm.json, so I have to place it before
> qom.json.
>
> It doesn't have any dependencies itself, so placing it in the previous
> position should be fine, where do you prefer?
Let's ignore how to place it for now, and focus on where we would *like*
to place it.
Is it related to anything other than ObjectType / ObjectOptions in the
QMP reference manual?
I guess qapi/kvm.json is for KVM-specific stuff in general, not just the
KVM PMU filter. Should we have a section for accelerator-specific
stuff, with subsections for the various accelerators?
[...]
Re: [RFC v2 1/5] qapi/qom: Introduce kvm-pmu-filter object
> > > @@ -1183,6 +1185,7 @@ > > >'if': 'CONFIG_LINUX' }, > > >'iommufd':'IOMMUFDProperties', > > >'iothread': 'IothreadProperties', > > > + 'kvm-pmu-filter': 'KVMPMUFilterPropertyVariant', > > > > The others are like > > > > 'mumble': 'MumbleProperties' > > > > Let's stick to that, and also avoid running together multiple > > capitalized acronyms: KvmPmuFilterProperties. > > IIUC, then I should use the name "KvmPmuFilterProperties" (string version > for QAPI), and the name "KvmPmuFilterPropertiesVariant" (numeric version > in codes), do you agree? > Thanks to Daniel's feedback, pmu filter doesn't need the string version anymore. So there's only 1 "KvmPmuFilterProperties" structure in QAPI.
Re: [RFC v2 1/5] qapi/qom: Introduce kvm-pmu-filter object
On Wed, Feb 05, 2025 at 11:03:51AM +0100, Markus Armbruster wrote:
> Date: Wed, 05 Feb 2025 11:03:51 +0100
> From: Markus Armbruster
> Subject: Re: [RFC v2 1/5] qapi/qom: Introduce kvm-pmu-filter object
>
> Quick & superficial review for now.
Thanks!
> > diff --git a/qapi/kvm.json b/qapi/kvm.json
> > new file mode 100644
> > index ..d51aeeba7cd8
> > --- /dev/null
> > +++ b/qapi/kvm.json
> > @@ -0,0 +1,116 @@
> > +# -*- Mode: Python -*-
> > +# vim: filetype=python
> > +
> > +##
> > +# = KVM based feature API
>
> This is a top-level section. It ends up between sections "QMP
> introspection" and "QEMU Object Model (QOM)". Is this what we want? Or
> should it be a sub-section of something? Or next to something else?
Do you mean it's not in the right place in the qapi-schema.json?
diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
index b1581988e4eb..742818d16e45 100644
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -64,6 +64,7 @@
{ 'include': 'compat.json' }
{ 'include': 'control.json' }
{ 'include': 'introspect.json' }
+{ 'include': 'kvm.json' }
{ 'include': 'qom.json' }
{ 'include': 'qdev.json' }
{ 'include': 'machine-common.json' }
Because qom.json includes kvm.json, so I have to place it before
qom.json.
It doesn't have any dependencies itself, so placing it in the previous
position should be fine, where do you prefer?
> > +##
> > +
> > +##
> > +# @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.0
> > +##
> > +{ 'enum': 'KVMPMUFilterAction',
> > + 'prefix': 'KVM_PMU_FILTER_ACTION',
> > + 'data': ['allow', 'deny'] }
> > +
> > +##
> > +# @KVMPMUEventEncodeFmt:
>
> Please don't abbreviate Format to Fmt. We use Format elsewhere, and
> consistency is desirable.
OK, will fix.
> > ##
> > # = QEMU Object Model (QOM)
> > @@ -1108,6 +1109,7 @@
> >'if': 'CONFIG_LINUX' },
> > 'iommufd',
> > 'iothread',
> > +'kvm-pmu-filter',
> > 'main-loop',
> > { 'name': 'memory-backend-epc',
> >'if': 'CONFIG_LINUX' },
> > @@ -1183,6 +1185,7 @@
> >'if': 'CONFIG_LINUX' },
> >'iommufd':'IOMMUFDProperties',
> >'iothread': 'IothreadProperties',
> > + 'kvm-pmu-filter': 'KVMPMUFilterPropertyVariant',
>
> The others are like
>
> 'mumble': 'MumbleProperties'
>
> Let's stick to that, and also avoid running together multiple
> capitalized acronyms: KvmPmuFilterProperties.
IIUC, then I should use the name "KvmPmuFilterProperties" (string version
for QAPI), and the name "KvmPmuFilterPropertiesVariant" (numeric version
in codes), do you agree?
> >'main-loop': 'MainLoopProperties',
> >'memory-backend-epc': { 'type': 'MemoryBackendEpcProperties',
> >'if': 'CONFIG_LINUX' },
>
Re: [RFC v2 1/5] qapi/qom: Introduce kvm-pmu-filter object
Quick & superficial review for now.
Zhao Liu 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.
>
> Considering that PMU event related fields are commonly used in
> hexadecimal, define KVMPMURawEventVariant, KVMPMUFilterEventVariant, and
> KVMPMUFilterPropertyVariant in kvm.json to support hexadecimal number
> strings in JSON.
>
> Additionally, define the corresponding numeric versions of
> KVMPMURawEvent, KVMPMUFilterEvent, and KVMPMUFilterProperty in kvm.json.
> This allows to handle numeric values more effectively and take advantage
> of the qapi helpers.
>
> Signed-off-by: Zhao Liu
[...]
> diff --git a/qapi/kvm.json b/qapi/kvm.json
> new file mode 100644
> index ..d51aeeba7cd8
> --- /dev/null
> +++ b/qapi/kvm.json
> @@ -0,0 +1,116 @@
> +# -*- Mode: Python -*-
> +# vim: filetype=python
> +
> +##
> +# = KVM based feature API
This is a top-level section. It ends up between sections "QMP
introspection" and "QEMU Object Model (QOM)". Is this what we want? Or
should it be a sub-section of something? Or next to something else?
> +##
> +
> +##
> +# @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.0
> +##
> +{ 'enum': 'KVMPMUFilterAction',
> + 'prefix': 'KVM_PMU_FILTER_ACTION',
> + 'data': ['allow', 'deny'] }
> +
> +##
> +# @KVMPMUEventEncodeFmt:
Please don't abbreviate Format to Fmt. We use Format elsewhere, and
consistency is desirable.
> +#
> +# Encoding formats of PMU event that QEMU/KVM supports.
> +#
> +# @raw: the encoded event code that KVM can directly consume.
> +#
> +# Since 10.0
> +##
> +{ 'enum': 'KVMPMUEventEncodeFmt',
> + 'prefix': 'KVM_PMU_EVENT_FMT',
> + 'data': ['raw'] }
> +
> +##
> +# @KVMPMURawEvent:
> +#
> +# Raw PMU event code.
> +#
> +# @code: the raw value that has been encoded, and QEMU could deliver
> +# to KVM directly.
> +#
> +# Since 10.0
> +##
> +{ 'struct': 'KVMPMURawEvent',
> + 'data': { 'code': 'uint64' } }
> +
> +##
> +# @KVMPMUFilterEvent:
> +#
> +# PMU event filtered by KVM.
> +#
> +# @format: PMU event format.
> +#
> +# Since 10.0
> +##
> +{ 'union': 'KVMPMUFilterEvent',
> + 'base': { 'format': 'KVMPMUEventEncodeFmt' },
> + 'discriminator': 'format',
> + 'data': { 'raw': 'KVMPMURawEvent' } }
> +
> +##
> +# @KVMPMUFilterProperty:
> +#
> +# Property of KVM PMU Filter.
> +#
> +# @events: the KVMPMUFilterEvent list.
> +#
> +# Since 10.0
> +##
> +{ 'struct': 'KVMPMUFilterProperty',
> + 'data': { '*events': ['KVMPMUFilterEvent'] } }
> +
> +##
> +# @KVMPMURawEventVariant:
> +#
> +# The variant of KVMPMURawEvent with the string, rather than the
> +# numeric value.
> +#
> +# @code: the raw value that has been encoded, and QEMU could deliver
> +# to KVM directly. This field is a uint64 string.
> +#
> +# Since 10.0
> +##
> +{ 'struct': 'KVMPMURawEventVariant',
> + 'data': { 'code': 'str' } }
> +
> +##
> +# @KVMPMUFilterEventVariant:
> +#
> +# The variant of KVMPMUFilterEvent.
> +#
> +# @format: PMU event format.
> +#
> +# Since 10.0
> +##
> +{ 'union': 'KVMPMUFilterEventVariant',
> + 'base': { 'format': 'KVMPMUEventEncodeFmt' },
> + 'discriminator': 'format',
> + 'data': { 'raw': 'KVMPMURawEventVariant' } }
> +
> +##
> +# @KVMPMUFilterPropertyVariant:
> +#
> +# The variant of KVMPMUFilterProperty.
> +#
> +# @action: action that KVM PMU filter will take.
> +#
> +# @events: the KVMPMUFilterEventVariant list.
> +#
> +# Since 10.0
> +##
> +{ 'struct': 'KVMPMUFilterPropertyVariant',
> + 'data': { 'action': 'KVMPMUFilterAction',
> +'*events': ['KVMPMUFilterEventVariant'] } }
> diff --git a/qapi/meson.build b/qapi/meson.build
> index e7bc54e5d047..856439c76b67 100644
> --- a/qapi/meson.build
> +++ b/qapi/meson.build
> @@ -37,6 +37,7 @@ qapi_all_modules = [
>'error',
>'introspect',
>'job',
> + 'kvm',
>'machine-common',
>'machine',
>'machine-target',
> diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
> index b1581988e4eb..742818d16e45 100644
> --- a/qapi/qapi-schema.json
> +++ b/qapi/qapi-schema.json
> @@ -64,6 +64,7 @@
> { 'include': 'compat.json' }
> { 'include': 'control.json' }
> { 'include': 'introspect.json' }
> +{ 'include': 'kvm.json' }
> { 'include': 'qom.json' }
> { 'include': 'qdev.json' }
> { 'include': 'machine-common.json' }
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 28ce24cd8d08..c75ec4b21e95 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -8,6 +8,7 @@
> { 'include': 'block-core.json' }
> { 'include': 'common.json' }
> { 'include': 'crypto.json' }
> +{ 'include': 'kvm.json' }
>
> ##
> # = QEMU Object Model (QOM)
> @@ -1108,6 +1109,7 @@
>'if': 'CONFIG_LINUX' },
> 'iommufd',
> 'iothread',
> +'kvm-pmu-filter',
> 'main-loo
