diff mbox series

[RFC,v2,3/5] i386/kvm: Support event with select & umask format in KVM PMU filter

Message ID 20250122090517.294083-4-zhao1.liu@intel.com (mailing list archive)
State New
Headers show
Series accel/kvm: Support KVM PMU filter | expand

Commit Message

Zhao Liu Jan. 22, 2025, 9:05 a.m. UTC
The select&umask is the common way for x86 to identify the PMU event,
so support this way as the "x86-default" format in kvm-pmu-filter
object.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
Changes since RFC v1:
 * Bump up the supported QAPI version to 10.0.
---
 accel/kvm/kvm-pmu.c      | 62 ++++++++++++++++++++++++++++++++++++++++
 include/system/kvm-pmu.h | 13 +++++++++
 qapi/kvm.json            | 46 +++++++++++++++++++++++++++--
 target/i386/kvm/kvm.c    |  5 ++++
 4 files changed, 123 insertions(+), 3 deletions(-)

Comments

Markus Armbruster Feb. 5, 2025, 10:07 a.m. UTC | #1
Zhao Liu <zhao1.liu@intel.com> writes:

> The select&umask is the common way for x86 to identify the PMU event,
> so support this way as the "x86-default" format in kvm-pmu-filter
> object.

So, format 'raw' lets you specify the PMU event code as a number, wheras
'x86-default' lets you specify it as select and umask, correct?

Why do we want both?

> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>

[...]

> diff --git a/qapi/kvm.json b/qapi/kvm.json
> index d51aeeba7cd8..93b869e3f90c 100644
> --- a/qapi/kvm.json
> +++ b/qapi/kvm.json
> @@ -27,11 +27,13 @@
>  #
>  # @raw: the encoded event code that KVM can directly consume.
>  #
> +# @x86-default: standard x86 encoding format with select and umask.

Why is this named -default?

> +#
>  # Since 10.0
>  ##
>  { 'enum': 'KVMPMUEventEncodeFmt',
>    'prefix': 'KVM_PMU_EVENT_FMT',
> -  'data': ['raw'] }
> +  'data': ['raw', 'x86-default'] }
>  
>  ##
>  # @KVMPMURawEvent:
> @@ -46,6 +48,25 @@
>  { 'struct': 'KVMPMURawEvent',
>    'data': { 'code': 'uint64' } }
>  
> +##
> +# @KVMPMUX86DefalutEvent:

Default, I suppose.

> +#
> +# x86 PMU event encoding with select and umask.
> +# raw_event = ((select & 0xf00UL) << 24) | \
> +#              (select) & 0xff) | \
> +#              ((umask) & 0xff) << 8)

Sphinx rejects this with "Unexpected indentation."

Is the formula needed here?

> +#
> +# @select: x86 PMU event select field, which is a 12-bit unsigned
> +#     number.
> +#
> +# @umask: x86 PMU event umask field.
> +#
> +# Since 10.0
> +##
> +{ 'struct': 'KVMPMUX86DefalutEvent',
> +  'data': { 'select': 'uint16',
> +            'umask': 'uint8' } }
> +
>  ##
>  # @KVMPMUFilterEvent:
>  #
> @@ -58,7 +79,8 @@
>  { 'union': 'KVMPMUFilterEvent',
>    'base': { 'format': 'KVMPMUEventEncodeFmt' },
>    'discriminator': 'format',
> -  'data': { 'raw': 'KVMPMURawEvent' } }
> +  'data': { 'raw': 'KVMPMURawEvent',
> +            'x86-default': 'KVMPMUX86DefalutEvent' } }
>  
>  ##
>  # @KVMPMUFilterProperty:
> @@ -86,6 +108,23 @@
>  { 'struct': 'KVMPMURawEventVariant',
>    'data': { 'code': 'str' } }
>  
> +##
> +# @KVMPMUX86DefalutEventVariant:
> +#
> +# The variant of KVMPMUX86DefalutEvent with the string, rather than
> +# the numeric value.
> +#
> +# @select: x86 PMU event select field.  This field is a 12-bit
> +#     unsigned number string.
> +#
> +# @umask: x86 PMU event umask field. This field is a uint8 string.

Why are these strings?  How are they parsed into numbers?

> +#
> +# Since 10.0
> +##
> +{ 'struct': 'KVMPMUX86DefalutEventVariant',
> +  'data': { 'select': 'str',
> +            'umask': 'str' } }
> +
>  ##
>  # @KVMPMUFilterEventVariant:
>  #
> @@ -98,7 +137,8 @@
>  { 'union': 'KVMPMUFilterEventVariant',
>    'base': { 'format': 'KVMPMUEventEncodeFmt' },
>    'discriminator': 'format',
> -  'data': { 'raw': 'KVMPMURawEventVariant' } }
> +  'data': { 'raw': 'KVMPMURawEventVariant',
> +            'x86-default': 'KVMPMUX86DefalutEventVariant' } }
>  
>  ##
>  # @KVMPMUFilterPropertyVariant:

[...]
Daniel P. Berrangé Feb. 6, 2025, 9:42 a.m. UTC | #2
On Thu, Feb 06, 2025 at 05:54:32PM +0800, Zhao Liu wrote:
> On Wed, Feb 05, 2025 at 11:07:10AM +0100, Markus Armbruster wrote:
> > Date: Wed, 05 Feb 2025 11:07:10 +0100
> > From: Markus Armbruster <armbru@redhat.com>
> > Subject: Re: [RFC v2 3/5] i386/kvm: Support event with select & umask
> >  format in KVM PMU filter
> > 
> > Zhao Liu <zhao1.liu@intel.com> writes:
> > 
> > > The select&umask is the common way for x86 to identify the PMU event,
> > > so support this way as the "x86-default" format in kvm-pmu-filter
> > > object.
> > 
> > So, format 'raw' lets you specify the PMU event code as a number, wheras
> > 'x86-default' lets you specify it as select and umask, correct?
> 
> Yes!
> 
> > Why do we want both?
> 
> This 2 formats are both wildly used in x86(for example, in perf tool).
> 
> x86 documents usually specify the umask and select fields.
> 
> But raw format could also be applied for ARM since ARM just uses a number
> to encode event.
> 
> > > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> > 
> > [...]
> > 
> > > diff --git a/qapi/kvm.json b/qapi/kvm.json
> > > index d51aeeba7cd8..93b869e3f90c 100644
> > > --- a/qapi/kvm.json
> > > +++ b/qapi/kvm.json
> > > @@ -27,11 +27,13 @@
> > >  #
> > >  # @raw: the encoded event code that KVM can directly consume.
> > >  #
> > > +# @x86-default: standard x86 encoding format with select and umask.
> > 
> > Why is this named -default?
> 
> Intel and AMD both use umask+select to encode events, but this format
> doesn't have a name... so I call it `default`, or what about
> "x86-umask-select"?
> 
> > > +#
> > >  # Since 10.0
> > >  ##
> > >  { 'enum': 'KVMPMUEventEncodeFmt',
> > >    'prefix': 'KVM_PMU_EVENT_FMT',
> > > -  'data': ['raw'] }
> > > +  'data': ['raw', 'x86-default'] }
> > >  
> > >  ##
> > >  # @KVMPMURawEvent:
> > > @@ -46,6 +48,25 @@
> > >  { 'struct': 'KVMPMURawEvent',
> > >    'data': { 'code': 'uint64' } }
> > >  
> > > +##
> > > +# @KVMPMUX86DefalutEvent:
> > 
> > Default, I suppose.
> 
> Thanks!
> 
> > > +#
> > > +# x86 PMU event encoding with select and umask.
> > > +# raw_event = ((select & 0xf00UL) << 24) | \
> > > +#              (select) & 0xff) | \
> > > +#              ((umask) & 0xff) << 8)
> > 
> > Sphinx rejects this with "Unexpected indentation."
> > 
> > Is the formula needed here?
> 
> I tried to explain the relationship between raw format and umask+select.
> 
> Emm, where do you think is the right place to put the document like
> this?
> 
> ...
> 
> > > +##
> > > +# @KVMPMUX86DefalutEventVariant:

Typo   s/Defalut/Default/ - repeated many times in this patch.

> > > +#
> > > +# The variant of KVMPMUX86DefalutEvent with the string, rather than
> > > +# the numeric value.
> > > +#
> > > +# @select: x86 PMU event select field.  This field is a 12-bit
> > > +#     unsigned number string.
> > > +#
> > > +# @umask: x86 PMU event umask field. This field is a uint8 string.
> > 
> > Why are these strings?  How are they parsed into numbers?
> 
> In practice, the values associated with PMU events (code for arm, select&
> umask for x86) are often expressed in hexadecimal. Further, from linux
> perf related information (tools/perf/pmu-events/arch/*/*/*.json), x86/
> arm64/riscv/nds32/powerpc all prefer the hexadecimal numbers and only
> s390 uses decimal value.
> 
> Therefore, it is necessary to support hexadecimal in order to honor PMU
> conventions.

IMHO having a data format that matches an arbitrary external tool is not
a goal for QMP. It should be neutral and exclusively use the normal JSON
encoding, ie base-10 decimal. Yes, this means a user/client may have to
convert from hex to dec before sending data over QMP. This is true of
many areas of QMP/QEMU config though and thus normal/expected behaviour.

With regards,
Daniel
Zhao Liu Feb. 6, 2025, 9:54 a.m. UTC | #3
On Wed, Feb 05, 2025 at 11:07:10AM +0100, Markus Armbruster wrote:
> Date: Wed, 05 Feb 2025 11:07:10 +0100
> From: Markus Armbruster <armbru@redhat.com>
> Subject: Re: [RFC v2 3/5] i386/kvm: Support event with select & umask
>  format in KVM PMU filter
> 
> Zhao Liu <zhao1.liu@intel.com> writes:
> 
> > The select&umask is the common way for x86 to identify the PMU event,
> > so support this way as the "x86-default" format in kvm-pmu-filter
> > object.
> 
> So, format 'raw' lets you specify the PMU event code as a number, wheras
> 'x86-default' lets you specify it as select and umask, correct?

Yes!

> Why do we want both?

This 2 formats are both wildly used in x86(for example, in perf tool).

x86 documents usually specify the umask and select fields.

But raw format could also be applied for ARM since ARM just uses a number
to encode event.

> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> 
> [...]
> 
> > diff --git a/qapi/kvm.json b/qapi/kvm.json
> > index d51aeeba7cd8..93b869e3f90c 100644
> > --- a/qapi/kvm.json
> > +++ b/qapi/kvm.json
> > @@ -27,11 +27,13 @@
> >  #
> >  # @raw: the encoded event code that KVM can directly consume.
> >  #
> > +# @x86-default: standard x86 encoding format with select and umask.
> 
> Why is this named -default?

Intel and AMD both use umask+select to encode events, but this format
doesn't have a name... so I call it `default`, or what about
"x86-umask-select"?

> > +#
> >  # Since 10.0
> >  ##
> >  { 'enum': 'KVMPMUEventEncodeFmt',
> >    'prefix': 'KVM_PMU_EVENT_FMT',
> > -  'data': ['raw'] }
> > +  'data': ['raw', 'x86-default'] }
> >  
> >  ##
> >  # @KVMPMURawEvent:
> > @@ -46,6 +48,25 @@
> >  { 'struct': 'KVMPMURawEvent',
> >    'data': { 'code': 'uint64' } }
> >  
> > +##
> > +# @KVMPMUX86DefalutEvent:
> 
> Default, I suppose.

Thanks!

> > +#
> > +# x86 PMU event encoding with select and umask.
> > +# raw_event = ((select & 0xf00UL) << 24) | \
> > +#              (select) & 0xff) | \
> > +#              ((umask) & 0xff) << 8)
> 
> Sphinx rejects this with "Unexpected indentation."
> 
> Is the formula needed here?

I tried to explain the relationship between raw format and umask+select.

Emm, where do you think is the right place to put the document like
this?

...

> > +##
> > +# @KVMPMUX86DefalutEventVariant:
> > +#
> > +# The variant of KVMPMUX86DefalutEvent with the string, rather than
> > +# the numeric value.
> > +#
> > +# @select: x86 PMU event select field.  This field is a 12-bit
> > +#     unsigned number string.
> > +#
> > +# @umask: x86 PMU event umask field. This field is a uint8 string.
> 
> Why are these strings?  How are they parsed into numbers?

In practice, the values associated with PMU events (code for arm, select&
umask for x86) are often expressed in hexadecimal. Further, from linux
perf related information (tools/perf/pmu-events/arch/*/*/*.json), x86/
arm64/riscv/nds32/powerpc all prefer the hexadecimal numbers and only
s390 uses decimal value.

Therefore, it is necessary to support hexadecimal in order to honor PMU
conventions.

However, unfortunately, standard JSON (RFC 8259) does not support
hexadecimal numbers. So I can only consider using the numeric string in
the QAPI and then parsing it to a number.

I parse this string into number by qemu_strtou64().
Zhao Liu Feb. 6, 2025, 10:23 a.m. UTC | #4
> > > > +##
> > > > +# @KVMPMUX86DefalutEventVariant:
> 
> Typo   s/Defalut/Default/ - repeated many times in this patch.

My bad! Will fix!

> > > > +#
> > > > +# The variant of KVMPMUX86DefalutEvent with the string, rather than
> > > > +# the numeric value.
> > > > +#
> > > > +# @select: x86 PMU event select field.  This field is a 12-bit
> > > > +#     unsigned number string.
> > > > +#
> > > > +# @umask: x86 PMU event umask field. This field is a uint8 string.
> > > 
> > > Why are these strings?  How are they parsed into numbers?
> > 
> > In practice, the values associated with PMU events (code for arm, select&
> > umask for x86) are often expressed in hexadecimal. Further, from linux
> > perf related information (tools/perf/pmu-events/arch/*/*/*.json), x86/
> > arm64/riscv/nds32/powerpc all prefer the hexadecimal numbers and only
> > s390 uses decimal value.
> > 
> > Therefore, it is necessary to support hexadecimal in order to honor PMU
> > conventions.
> 
> IMHO having a data format that matches an arbitrary external tool is not
> a goal for QMP. It should be neutral and exclusively use the normal JSON
> encoding, ie base-10 decimal. Yes, this means a user/client may have to
> convert from hex to dec before sending data over QMP. This is true of
> many areas of QMP/QEMU config though and thus normal/expected behaviour.
>

Thanks! This will simplify the code a lot.
Markus Armbruster Feb. 6, 2025, 10:24 a.m. UTC | #5
Daniel P. Berrangé <berrange@redhat.com> writes:

> On Thu, Feb 06, 2025 at 05:54:32PM +0800, Zhao Liu wrote:
>> On Wed, Feb 05, 2025 at 11:07:10AM +0100, Markus Armbruster wrote:
>> > Date: Wed, 05 Feb 2025 11:07:10 +0100
>> > From: Markus Armbruster <armbru@redhat.com>
>> > Subject: Re: [RFC v2 3/5] i386/kvm: Support event with select & umask
>> >  format in KVM PMU filter
>> > 
>> > Zhao Liu <zhao1.liu@intel.com> writes:
>> > 
>> > > The select&umask is the common way for x86 to identify the PMU event,
>> > > so support this way as the "x86-default" format in kvm-pmu-filter
>> > > object.
>> > 
>> > So, format 'raw' lets you specify the PMU event code as a number, wheras
>> > 'x86-default' lets you specify it as select and umask, correct?
>> 
>> Yes!
>> 
>> > Why do we want both?
>> 
>> This 2 formats are both wildly used in x86(for example, in perf tool).
>> 
>> x86 documents usually specify the umask and select fields.
>> 
>> But raw format could also be applied for ARM since ARM just uses a number
>> to encode event.

Is it really too much to ask of the client to compute a raw value from
umask and select values?

>> > > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
>> > 
>> > [...]
>> > 
>> > > diff --git a/qapi/kvm.json b/qapi/kvm.json
>> > > index d51aeeba7cd8..93b869e3f90c 100644
>> > > --- a/qapi/kvm.json
>> > > +++ b/qapi/kvm.json
>> > > @@ -27,11 +27,13 @@
>> > >  #
>> > >  # @raw: the encoded event code that KVM can directly consume.
>> > >  #
>> > > +# @x86-default: standard x86 encoding format with select and umask.
>> > 
>> > Why is this named -default?
>> 
>> Intel and AMD both use umask+select to encode events, but this format
>> doesn't have a name... so I call it `default`, or what about
>> "x86-umask-select"?

Works for me.

>> > > +#
>> > >  # Since 10.0
>> > >  ##
>> > >  { 'enum': 'KVMPMUEventEncodeFmt',
>> > >    'prefix': 'KVM_PMU_EVENT_FMT',
>> > > -  'data': ['raw'] }
>> > > +  'data': ['raw', 'x86-default'] }
>> > >  
>> > >  ##
>> > >  # @KVMPMURawEvent:
>> > > @@ -46,6 +48,25 @@
>> > >  { 'struct': 'KVMPMURawEvent',
>> > >    'data': { 'code': 'uint64' } }
>> > >  
>> > > +##
>> > > +# @KVMPMUX86DefalutEvent:
>> > 
>> > Default, I suppose.
>> 
>> Thanks!
>> 
>> > > +#
>> > > +# x86 PMU event encoding with select and umask.
>> > > +# raw_event = ((select & 0xf00UL) << 24) | \
>> > > +#              (select) & 0xff) | \
>> > > +#              ((umask) & 0xff) << 8)
>> > 
>> > Sphinx rejects this with "Unexpected indentation."
>> > 
>> > Is the formula needed here?
>> 
>> I tried to explain the relationship between raw format and umask+select.
>> 
>> Emm, where do you think is the right place to put the document like
>> this?

Do users need to know how to compute the raw event value from @select
and @umask?

If yes, is C code the best way?

Here's another way:

    bits  0..7 : bits 0..7 of @select
    bits  8..15: @umask
    bits 24..27: bits 8..11 of @select
    all other bits: zero

>> ...
>> 
>> > > +##
>> > > +# @KVMPMUX86DefalutEventVariant:
>
> Typo   s/Defalut/Default/ - repeated many times in this patch.
>
>> > > +#
>> > > +# The variant of KVMPMUX86DefalutEvent with the string, rather than
>> > > +# the numeric value.
>> > > +#
>> > > +# @select: x86 PMU event select field.  This field is a 12-bit
>> > > +#     unsigned number string.
>> > > +#
>> > > +# @umask: x86 PMU event umask field. This field is a uint8 string.
>> > 
>> > Why are these strings?  How are they parsed into numbers?
>> 
>> In practice, the values associated with PMU events (code for arm, select&
>> umask for x86) are often expressed in hexadecimal. Further, from linux
>> perf related information (tools/perf/pmu-events/arch/*/*/*.json), x86/
>> arm64/riscv/nds32/powerpc all prefer the hexadecimal numbers and only
>> s390 uses decimal value.
>> 
>> Therefore, it is necessary to support hexadecimal in order to honor PMU
>> conventions.
>
> IMHO having a data format that matches an arbitrary external tool is not
> a goal for QMP. It should be neutral and exclusively use the normal JSON
> encoding, ie base-10 decimal. Yes, this means a user/client may have to
> convert from hex to dec before sending data over QMP. This is true of
> many areas of QMP/QEMU config though and thus normal/expected behaviour.

Concur.
Zhao Liu Feb. 6, 2025, 2:22 p.m. UTC | #6
> >> > > The select&umask is the common way for x86 to identify the PMU event,
> >> > > so support this way as the "x86-default" format in kvm-pmu-filter
> >> > > object.
> >> > 
> >> > So, format 'raw' lets you specify the PMU event code as a number, wheras
> >> > 'x86-default' lets you specify it as select and umask, correct?
> >> 
> >> Yes!
> >> 
> >> > Why do we want both?
> >> 
> >> This 2 formats are both wildly used in x86(for example, in perf tool).
> >> 
> >> x86 documents usually specify the umask and select fields.
> >> 
> >> But raw format could also be applied for ARM since ARM just uses a number
> >> to encode event.
> 
> Is it really too much to ask of the client to compute a raw value from
> umask and select values?

I understand you're wondering if we can omit the select+umask format...

In principle, having either one should work correctly... The additional
support for the umask+select format is more user-friendly and doesn't
introduce much complexity in the code.

My own observation is that Intel's doc rarely uses the raw format
directly, whereas AMD uses the raw format relatively more often.
Personally, when using the perf tool, I prefer the umask+select format.

Even if only the raw format is supported, users still need to be aware
of the umask+select format because there is the third format - "masked
entry" (which tries to mask a group of events with same select, patch 4).

So I would like to keep both raw and umask+select formats if possible.

...

> >> > > +#
> >> > > +# x86 PMU event encoding with select and umask.
> >> > > +# raw_event = ((select & 0xf00UL) << 24) | \
> >> > > +#              (select) & 0xff) | \
> >> > > +#              ((umask) & 0xff) << 8)
> >> > 
> >> > Sphinx rejects this with "Unexpected indentation."
> >> > 
> >> > Is the formula needed here?
> >> 
> >> I tried to explain the relationship between raw format and umask+select.
> >> 
> >> Emm, where do you think is the right place to put the document like
> >> this?
> 
> Do users need to know how to compute the raw event value from @select
> and @umask?

Yes, because it's also a unified calculation. AMD and Intel have
differences in bits for supported select field, but this calculation
(which follows from the KVM code) makes both compatible.

> If yes, is C code the best way?
> 
> Here's another way:
> 
>     bits  0..7 : bits 0..7 of @select
>     bits  8..15: @umask
>     bits 24..27: bits 8..11 of @select
>     all other bits: zero
>

Thank you! This is what I want.
Zhao Liu Feb. 6, 2025, 2:54 p.m. UTC | #7
> > Do users need to know how to compute the raw event value from @select
> > and @umask?
> 
> Yes, because it's also a unified calculation. AMD and Intel have
> differences in bits for supported select field, but this calculation
> (which follows from the KVM code) makes both compatible.
> 
> > If yes, is C code the best way?

Sorry, I missed this line. In this patch, there's macro:

+#define X86_PMU_RAW_EVENT(eventsel, umask) (((eventsel & 0xf00UL) << 24) | \
+                                            ((eventsel) & 0xff) | \
+                                            ((umask) & 0xff) << 8)

So could I said something like the following?

+##
+# @KVMPMUX86SelectUmaskEvent:
+#
+# x86 PMU event encoding with select and umask.  Using the X86_PMU_RAW_EVENT
+# macro, the select and umask fields will be encoded into raw foramt and
+# delivered to KVM.
+#
+# @select: x86 PMU event select field, which is a 12-bit unsigned
+#     number.
+#
+# @umask: x86 PMU event umask field.
+#
+# Since 10.0
+##
+{ 'struct': 'KVMPMUX86DefalutEvent',
+  'data': { 'select': 'uint16',
+            'umask': 'uint8' } }
+

Thanks very much!

> > Here's another way:
> > 
> >     bits  0..7 : bits 0..7 of @select
> >     bits  8..15: @umask
> >     bits 24..27: bits 8..11 of @select
> >     all other bits: zero
> >
> 
> Thank you! This is what I want.
> 
> 
>
Markus Armbruster Feb. 7, 2025, 1:06 p.m. UTC | #8
Zhao Liu <zhao1.liu@intel.com> writes:

>> > Do users need to know how to compute the raw event value from @select
>> > and @umask?
>> 
>> Yes, because it's also a unified calculation. AMD and Intel have
>> differences in bits for supported select field, but this calculation
>> (which follows from the KVM code) makes both compatible.
>> 
>> > If yes, is C code the best way?
>
> Sorry, I missed this line. In this patch, there's macro:
>
> +#define X86_PMU_RAW_EVENT(eventsel, umask) (((eventsel & 0xf00UL) << 24) | \
> +                                            ((eventsel) & 0xff) | \
> +                                            ((umask) & 0xff) << 8)
>
> So could I said something like the following?
>
> +##
> +# @KVMPMUX86SelectUmaskEvent:
> +#
> +# x86 PMU event encoding with select and umask.  Using the X86_PMU_RAW_EVENT
> +# macro, the select and umask fields will be encoded into raw foramt and
> +# delivered to KVM.

Doc comments are for the QMP reference manual, i.e. for users of QMP.
Explaining the QMP interface in terms of its implementation in QEMU is
not nice.

> +#
> +# @select: x86 PMU event select field, which is a 12-bit unsigned
> +#     number.
> +#
> +# @umask: x86 PMU event umask field.
> +#
> +# Since 10.0
> +##
> +{ 'struct': 'KVMPMUX86DefalutEvent',
> +  'data': { 'select': 'uint16',
> +            'umask': 'uint8' } }
> +
>
> Thanks very much!
>
>> > Here's another way:
>> > 
>> >     bits  0..7 : bits 0..7 of @select
>> >     bits  8..15: @umask
>> >     bits 24..27: bits 8..11 of @select
>> >     all other bits: zero
>> >
>> 
>> Thank you! This is what I want.
>> 
>> 
>>
diff mbox series

Patch

diff --git a/accel/kvm/kvm-pmu.c b/accel/kvm/kvm-pmu.c
index 4d0d4e7a452b..cbd32e8e21f8 100644
--- a/accel/kvm/kvm-pmu.c
+++ b/accel/kvm/kvm-pmu.c
@@ -17,6 +17,8 @@ 
 #include "qom/object_interfaces.h"
 #include "system/kvm-pmu.h"
 
+#define UINT12_MAX (4095)
+
 static void kvm_pmu_filter_set_action(Object *obj, int value,
                                       Error **errp G_GNUC_UNUSED)
 {
@@ -54,6 +56,12 @@  static void kvm_pmu_filter_get_event(Object *obj, Visitor *v, const char *name,
             str_event->u.raw.code = g_strdup_printf("0x%lx",
                                                     event->u.raw.code);
             break;
+        case KVM_PMU_EVENT_FMT_X86_DEFAULT:
+            str_event->u.x86_default.select =
+                g_strdup_printf("0x%x", event->u.x86_default.select);
+            str_event->u.x86_default.umask =
+                g_strdup_printf("0x%x", event->u.x86_default.umask);
+            break;
         default:
             g_assert_not_reached();
         }
@@ -98,6 +106,60 @@  static void kvm_pmu_filter_set_event(Object *obj, Visitor *v, const char *name,
                 goto fail;
             }
             break;
+        case KVM_PMU_EVENT_FMT_X86_DEFAULT: {
+            uint64_t select, umask;
+
+            ret = qemu_strtou64(str_event->u.x86_default.select, NULL,
+                                0, &select);
+            if (ret < 0) {
+                error_setg(errp,
+                           "Invalid %s PMU event (select: %s): %s. "
+                           "The select must be a "
+                           "12-bit unsigned number string.",
+                           KVMPMUEventEncodeFmt_str(str_event->format),
+                           str_event->u.x86_default.select,
+                           strerror(-ret));
+                g_free(event);
+                goto fail;
+            }
+            if (select > UINT12_MAX) {
+                error_setg(errp,
+                           "Invalid %s PMU event (select: %s): "
+                           "Numerical result out of range. "
+                           "The select must be a "
+                           "12-bit unsigned number string.",
+                           KVMPMUEventEncodeFmt_str(str_event->format),
+                           str_event->u.x86_default.select);
+                g_free(event);
+                goto fail;
+            }
+            event->u.x86_default.select = select;
+
+            ret = qemu_strtou64(str_event->u.x86_default.umask, NULL,
+                                0, &umask);
+            if (ret < 0) {
+                error_setg(errp,
+                           "Invalid %s PMU event (umask: %s): %s. "
+                           "The umask must be a uint8 string.",
+                           KVMPMUEventEncodeFmt_str(str_event->format),
+                           str_event->u.x86_default.umask,
+                           strerror(-ret));
+                g_free(event);
+                goto fail;
+            }
+            if (umask > UINT8_MAX) {
+                error_setg(errp,
+                           "Invalid %s PMU event (umask: %s): "
+                           "Numerical result out of range. "
+                           "The umask must be a uint8 string.",
+                           KVMPMUEventEncodeFmt_str(str_event->format),
+                           str_event->u.x86_default.umask);
+                g_free(event);
+                goto fail;
+            }
+            event->u.x86_default.umask = umask;
+            break;
+        }
         default:
             g_assert_not_reached();
         }
diff --git a/include/system/kvm-pmu.h b/include/system/kvm-pmu.h
index b09f70d3a370..63402f75cfdc 100644
--- a/include/system/kvm-pmu.h
+++ b/include/system/kvm-pmu.h
@@ -27,4 +27,17 @@  struct KVMPMUFilter {
     KVMPMUFilterEventList *events;
 };
 
+/*
+ * Stolen from Linux kernel (RAW_EVENT at tools/testing/selftests/kvm/include/
+ * x86_64/pmu.h).
+ *
+ * Encode an eventsel+umask pair into event-select MSR format.  Note, this is
+ * technically AMD's format, as Intel's format only supports 8 bits for the
+ * event selector, i.e. doesn't use bits 24:16 for the selector.  But, OR-ing
+ * in '0' is a nop and won't clobber the CMASK.
+ */
+#define X86_PMU_RAW_EVENT(eventsel, umask) (((eventsel & 0xf00UL) << 24) | \
+                                            ((eventsel) & 0xff) | \
+                                            ((umask) & 0xff) << 8)
+
 #endif /* KVM_PMU_H */
diff --git a/qapi/kvm.json b/qapi/kvm.json
index d51aeeba7cd8..93b869e3f90c 100644
--- a/qapi/kvm.json
+++ b/qapi/kvm.json
@@ -27,11 +27,13 @@ 
 #
 # @raw: the encoded event code that KVM can directly consume.
 #
+# @x86-default: standard x86 encoding format with select and umask.
+#
 # Since 10.0
 ##
 { 'enum': 'KVMPMUEventEncodeFmt',
   'prefix': 'KVM_PMU_EVENT_FMT',
-  'data': ['raw'] }
+  'data': ['raw', 'x86-default'] }
 
 ##
 # @KVMPMURawEvent:
@@ -46,6 +48,25 @@ 
 { 'struct': 'KVMPMURawEvent',
   'data': { 'code': 'uint64' } }
 
+##
+# @KVMPMUX86DefalutEvent:
+#
+# x86 PMU event encoding with select and umask.
+# raw_event = ((select & 0xf00UL) << 24) | \
+#              (select) & 0xff) | \
+#              ((umask) & 0xff) << 8)
+#
+# @select: x86 PMU event select field, which is a 12-bit unsigned
+#     number.
+#
+# @umask: x86 PMU event umask field.
+#
+# Since 10.0
+##
+{ 'struct': 'KVMPMUX86DefalutEvent',
+  'data': { 'select': 'uint16',
+            'umask': 'uint8' } }
+
 ##
 # @KVMPMUFilterEvent:
 #
@@ -58,7 +79,8 @@ 
 { 'union': 'KVMPMUFilterEvent',
   'base': { 'format': 'KVMPMUEventEncodeFmt' },
   'discriminator': 'format',
-  'data': { 'raw': 'KVMPMURawEvent' } }
+  'data': { 'raw': 'KVMPMURawEvent',
+            'x86-default': 'KVMPMUX86DefalutEvent' } }
 
 ##
 # @KVMPMUFilterProperty:
@@ -86,6 +108,23 @@ 
 { 'struct': 'KVMPMURawEventVariant',
   'data': { 'code': 'str' } }
 
+##
+# @KVMPMUX86DefalutEventVariant:
+#
+# The variant of KVMPMUX86DefalutEvent with the string, rather than
+# the numeric value.
+#
+# @select: x86 PMU event select field.  This field is a 12-bit
+#     unsigned number string.
+#
+# @umask: x86 PMU event umask field. This field is a uint8 string.
+#
+# Since 10.0
+##
+{ 'struct': 'KVMPMUX86DefalutEventVariant',
+  'data': { 'select': 'str',
+            'umask': 'str' } }
+
 ##
 # @KVMPMUFilterEventVariant:
 #
@@ -98,7 +137,8 @@ 
 { 'union': 'KVMPMUFilterEventVariant',
   'base': { 'format': 'KVMPMUEventEncodeFmt' },
   'discriminator': 'format',
-  'data': { 'raw': 'KVMPMURawEventVariant' } }
+  'data': { 'raw': 'KVMPMURawEventVariant',
+            'x86-default': 'KVMPMUX86DefalutEventVariant' } }
 
 ##
 # @KVMPMUFilterPropertyVariant:
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index b82adbed50f4..bab58761417a 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -5970,6 +5970,10 @@  static bool kvm_config_pmu_event(KVMPMUFilter *filter,
         case KVM_PMU_EVENT_FMT_RAW:
             code = event->u.raw.code;
             break;
+        case KVM_PMU_EVENT_FMT_X86_DEFAULT:
+            code = X86_PMU_RAW_EVENT(event->u.x86_default.select,
+                                     event->u.x86_default.umask);
+            break;
         default:
             g_assert_not_reached();
         }
@@ -6640,6 +6644,7 @@  static void kvm_arch_check_pmu_filter(const Object *obj, const char *name,
 
         switch (event->format) {
         case KVM_PMU_EVENT_FMT_RAW:
+        case KVM_PMU_EVENT_FMT_X86_DEFAULT:
             break;
         default:
             error_setg(errp,