Re: [PATCH v2 1/2] target/i386: add "-cpu, lbr-fmt=*" support to enable guest LBR

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

Hi Paolo & Eduardo,

Do we have any comment for the QEMU LBR enabling patches?

https://lore.kernel.org/qemu-devel/20210201045453.240258-1-like.xu@linux.intel.com/

On 2021/2/1 12:54, Like Xu wrote:
> The last branch recording (LBR) is a performance monitor unit (PMU)
> feature on Intel processors that records a running trace of the most
> recent branches taken by the processor in the LBR stack. The QEMU
> could configure whether it's enabled or not for each guest via CLI.
> 
> The LBR feature would be enabled on the guest if:
> - the KVM is enabled and the PMU is enabled and,
> - the msr-based-feature IA32_PERF_CAPABILITIES is supporterd on KVM and,
> - the supported returned value for lbr_fmt from this msr is not zero and,
> - the requested guest vcpu model does support FEAT_1_ECX.CPUID_EXT_PDCM,
> - the configured lbr-fmt value is the same as the host lbr_fmt value
>    or use the QEMU option "-cpu host,migratable=no".
> 
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Like Xu <like.xu@linux.intel.com>
> ---
>   target/i386/cpu.c     | 16 ++++++++++++++++
>   target/i386/cpu.h     | 10 ++++++++++
>   target/i386/kvm/kvm.c |  5 +++--
>   3 files changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index ae89024d36..80a5d3f0c2 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -6504,6 +6504,13 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool verbose)
>               x86_cpu_get_supported_feature_word(w, false);
>           uint64_t requested_features = env->features[w];
>           uint64_t unavailable_features = requested_features & ~host_feat;
> +        if (kvm_enabled() && w == FEAT_PERF_CAPABILITIES &&
> +            (requested_features & PERF_CAP_LBR_FMT)) {
> +            if ((host_feat & PERF_CAP_LBR_FMT) !=
> +                (requested_features & PERF_CAP_LBR_FMT)) {
> +                unavailable_features |= PERF_CAP_LBR_FMT;
> +            }
> +        }
>           mark_unavailable_features(cpu, w, unavailable_features, prefix);
>       }
>   
> @@ -6611,6 +6618,14 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>           }
>       }
>   
> +    if (cpu->lbr_fmt) {
> +        if (!cpu->enable_pmu) {
> +            error_setg(errp, "LBR is unsupported since guest PMU is disabled.");
> +            return;
> +        }
> +        env->features[FEAT_PERF_CAPABILITIES] |= cpu->lbr_fmt;
> +    }
> +
>       /* mwait extended info: needed for Core compatibility */
>       /* We always wake on interrupt even if host does not have the capability */
>       cpu->mwait.ecx |= CPUID_MWAIT_EMX | CPUID_MWAIT_IBE;
> @@ -7184,6 +7199,7 @@ static Property x86_cpu_properties[] = {
>   #endif
>       DEFINE_PROP_INT32("node-id", X86CPU, node_id, CPU_UNSET_NUMA_NODE_ID),
>       DEFINE_PROP_BOOL("pmu", X86CPU, enable_pmu, false),
> +    DEFINE_PROP_UINT8("lbr-fmt", X86CPU, lbr_fmt, 0),
>   
>       DEFINE_PROP_UINT32("hv-spinlocks", X86CPU, hyperv_spinlock_attempts,
>                          HYPERV_SPINLOCK_NEVER_NOTIFY),
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index d23a5b340a..64320bced2 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -354,6 +354,7 @@ typedef enum X86Seg {
>   #define ARCH_CAP_TSX_CTRL_MSR		(1<<7)
>   
>   #define MSR_IA32_PERF_CAPABILITIES      0x345
> +#define PERF_CAP_LBR_FMT      0x3f
>   
>   #define MSR_IA32_TSX_CTRL		0x122
>   #define MSR_IA32_TSCDEADLINE            0x6e0
> @@ -1709,6 +1710,15 @@ struct X86CPU {
>        */
>       bool enable_pmu;
>   
> +    /*
> +     * Configure LBR_FMT bits on IA32_PERF_CAPABILITIES MSR.
> +     * This can't be enabled by default yet because it doesn't have
> +     * ABI stability guarantees, as it is only allowed to pass all
> +     * LBR_FMT bits returned by kvm_arch_get_supported_msr_feature()
> +     * (that depends on host CPU and kernel capabilities) to the guest.
> +     */
> +    uint8_t lbr_fmt;
> +
>       /* LMCE support can be enabled/disabled via cpu option 'lmce=on/off'. It is
>        * disabled by default to avoid breaking migration between QEMU with
>        * different LMCE configurations.
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index 6dc1ee052d..49745efb78 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -2705,8 +2705,9 @@ static void kvm_msr_entry_add_perf(X86CPU *cpu, FeatureWordArray f)
>                                              MSR_IA32_PERF_CAPABILITIES);
>   
>       if (kvm_perf_cap) {
> -        kvm_msr_entry_add(cpu, MSR_IA32_PERF_CAPABILITIES,
> -                        kvm_perf_cap & f[FEAT_PERF_CAPABILITIES]);
> +        kvm_perf_cap = cpu->migratable ?
> +            (kvm_perf_cap & f[FEAT_PERF_CAPABILITIES]) : kvm_perf_cap;
> +        kvm_msr_entry_add(cpu, MSR_IA32_PERF_CAPABILITIES, kvm_perf_cap);
>       }
>   }
>   
>