On 7/19/2024 4:40 PM, Zhao Liu wrote:
> 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@xxxxxxxxxxxxxxx/>
> 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 raw event format, I suppose user should know the event layout and
would directly input the raw event code, qemu may not concern this.
>
> 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)
It's always good if Intel and AMD can share same event layout, the extended
umask field occupies bits [40:47]. So I think we'd better follow this for
the RAW_EVENT as well.
>
> Thanks,
> Zhao
>
