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

[Zhao Liu]

Hi Dapeng,

On Thu, Jul 18, 2024 at 01:28:25PM +0800, Mi, Dapeng wrote:

[snip]

> > +        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) {
> 
> umask is extended to 16 bits from Perfmon v6+. Please notice we need to
> upgrade this to 16 bits in the future. More details can be found here.
> [PATCH V3 00/13] Support Lunar Lake and Arrow Lake core PMU - kan.liang
> (kernel.org)
> <https://lore.kernel.org/all/20240626143545.480761-1-kan.liang@linux.intel.com/>

It's tricky...now I referred the RAW_EVENT format in tools/testing/
selftests/kvm/include/x86_64/pmu.h, which is used in KVM PMU and is
compatible with AMD and Intel.

The current KVM PMU filter for raw code doesn't define the layout in
the standard API like masked entries (KVM_PMU_ENCODE_MASKED_ENTRY), but
actually uses the RAW_EVENT format. So I even plan to move RAW_EVENT
macro into arch/x86/include/uapi/asm/kvm.h...

For the changes you mentioned, I think it would be better for the raw
code layout design not to break RAW_EVENT, so that AMD and Intel can
equally use the same macro to encode. Is it possible for a unified
layout macro?

What about extending RAW_EVENT as the following example? I understand
the umask2 is at bit 40-47.

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

Thanks,
Zhao