Re: [PATCH v8 4/7] kvm: x86/pmu: Introduce masked events to the pmu event filter

[Like Xu <like.xu.linux@gmail.com>]

On 21/12/2022 12:12 am, Aaron Lewis wrote:
> When building a list of filter events, it can sometimes be a challenge
> to fit all the events needed to adequately restrict the guest into the
> limited space available in the pmu event filter.  This stems from the
> fact that the pmu event filter requires each event (i.e. event select +
> unit mask) be listed, when the intention might be to restrict the
> event select all together, regardless of it's unit mask.  Instead of

This memory/cpu saving requirement is quite reasonable. To clarify,

PMU events with different unit masks for the same event select doesn't
point to the same or pipeline-associated microarchitectural component.

In practice, there will be a lot of cases where the "exclusion bit" is required.
That's one of the reasons I was asking about real use cases in another thread.

> increasing the number of filter events in the pmu event filter, add a
> new encoding that is able to do a more generalized match on the unit mask.
> 
> Introduce masked events as another encoding the pmu event filter
> understands.  Masked events has the fields: mask, match, and exclude.
> When filtering based on these events, the mask is applied to the guest's
> unit mask to see if it matches the match value (i.e. umask & mask ==
> match).  The exclude bit can then be used to exclude events from that
> match.  E.g. for a given event select, if it's easier to say which unit
> mask values shouldn't be filtered, a masked event can be set up to match
> all possible unit mask values, then another masked event can be set up to
> match the unit mask values that shouldn't be filtered.
> 
> Userspace can query to see if this feature exists by looking for the
> capability, KVM_CAP_PMU_EVENT_MASKED_EVENTS.
> 
> This feature is enabled by setting the flags field in the pmu event
> filter to KVM_PMU_EVENT_FLAG_MASKED_EVENTS.
> 
> Events can be encoded by using KVM_PMU_ENCODE_MASKED_ENTRY().
> 
> It is an error to have a bit set outside the valid bits for a masked
> event, and calls to KVM_SET_PMU_EVENT_FILTER will return -EINVAL in
> such cases, including the high bits of the event select (35:32) if
> called on Intel.

Some users want to count Intel TSX events and their VM instances needs:

#define HSW_IN_TX					(1ULL << 32)
#define HSW_IN_TX_CHECKPOINTED				(1ULL << 33)

> 
> With these updates the filter matching code has been updated to match on
> a common event.  Masked events were flexible enough to handle both event
> types, so they were used as the common event.  This changes how guest
> events get filtered because regardless of the type of event used in the
> uAPI, they will be converted to masked events.  Because of this there
> could be a slight performance hit because instead of matching the filter

Bonus, if this performance loss is quantified, we could make a side-by-side
comparison of alternative solutions, considering wasting memory seems to
be a habit of many kernel developers.

> event with a lookup on event select + unit mask, it does a lookup on event
> select then walks the unit masks to find the match.  This shouldn't be a
> big problem because I would expect the set of common event selects to be

A quick rough statistic about Intel PMU based on 
https://github.com/intel/perfmon.git:
our filtering mechanism needs to consider 388 different EventCode and 183 different
UMask, prioritizing filtering event_select will instead bring more entries.

> small, and if they aren't the set can likely be reduced by using masked
> events to generalize the unit mask.  Using one type of event when
> filtering guest events allows for a common code path to be used.
> 
> Signed-off-by: Aaron Lewis <aaronlewis@google.com>
> ---
>   Documentation/virt/kvm/api.rst  |  77 +++++++++++--
>   arch/x86/include/asm/kvm_host.h |  14 ++-
>   arch/x86/include/uapi/asm/kvm.h |  29 +++++
>   arch/x86/kvm/pmu.c              | 197 +++++++++++++++++++++++++++-----
>   arch/x86/kvm/x86.c              |   1 +
>   include/uapi/linux/kvm.h        |   1 +
>   6 files changed, 281 insertions(+), 38 deletions(-)
> 

...

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 312aea1854ae..d2023076f363 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -4401,6 +4401,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>    	case KVM_CAP_SPLIT_IRQCHIP:
>   	case KVM_CAP_IMMEDIATE_EXIT:
>   	case KVM_CAP_PMU_EVENT_FILTER:
> +	case KVM_CAP_PMU_EVENT_MASKED_EVENTS:

How about reusing KVM_CAP_PMU_CAPABILITY to advertise this new cap ?

>   	case KVM_CAP_GET_MSR_FEATURES:
>   	case KVM_CAP_MSR_PLATFORM_INFO:
>   	case KVM_CAP_EXCEPTION_PAYLOAD:
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 20522d4ba1e0..0b16b9ed3b23 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1175,6 +1175,7 @@ struct kvm_ppc_resize_hpt {
>   #define KVM_CAP_DIRTY_LOG_RING_ACQ_REL 223
>   #define KVM_CAP_S390_PROTECTED_ASYNC_DISABLE 224
>   #define KVM_CAP_DIRTY_LOG_RING_WITH_BITMAP 225
> +#define KVM_CAP_PMU_EVENT_MASKED_EVENTS 226
>   
>   #ifdef KVM_CAP_IRQ_ROUTING
>