Re: [PATCH] x86: events: Do not return bogus capabilities if PMU is broken

From: Peter Zijlstra
Date: Wed Jun 01 2022 - 07:08:17 EST


On Wed, Jun 01, 2022 at 05:42:56AM -0400, Paolo Bonzini wrote:
> From: Like Xu <likexu@xxxxxxxxxxx>
>
> If the PMU is broken due to firmware issues, check_hw_exists() will return
> false but perf_get_x86_pmu_capability() will still return data from x86_pmu.
> Likewise if some of the hotplug callbacks cannot be installed the contents
> of x86_pmu will not be reverted.
>
> Handle the failure in both cases by clearing x86_pmu if init_hw_perf_events()
> or reverts to software events only.
>
> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>

No SoB from Like,

> ---
> arch/x86/events/core.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
> index 99cf67d63cf3..d23f3821daf5 100644
> --- a/arch/x86/events/core.c
> +++ b/arch/x86/events/core.c
> @@ -2095,14 +2095,15 @@ static int __init init_hw_perf_events(void)
> }
> if (err != 0) {
> pr_cont("no PMU driver, software events only.\n");
> - return 0;
> + err = 0;
> + goto out_bad_pmu;
> }
>
> pmu_check_apic();
>
> /* sanity check that the hardware exists or is emulated */
> if (!check_hw_exists(&pmu, x86_pmu.num_counters, x86_pmu.num_counters_fixed))
> - return 0;
> + goto out_bad_pmu;
>
> pr_cont("%s PMU driver.\n", x86_pmu.name);
>
> @@ -2211,6 +2212,8 @@ static int __init init_hw_perf_events(void)
> cpuhp_remove_state(CPUHP_AP_PERF_X86_STARTING);
> out:
> cpuhp_remove_state(CPUHP_PERF_X86_PREPARE);
> +out_bad_pmu:
> + memset(&x86_pmu, 0, sizeof(x86_pmu));
> return err;
> }
> early_initcall(init_hw_perf_events);
> @@ -2982,6 +2985,11 @@ unsigned long perf_misc_flags(struct pt_regs *regs)
>
> void perf_get_x86_pmu_capability(struct x86_pmu_capability *cap)
> {
> + if (!x86_pmu.name) {

We have x86_pmu_initialized(), the implementation is a bit daft, but
might as well use it here too, no?

> + memset(cap, 0, sizeof(*cap));
> + return;
> + }
> +
> cap->version = x86_pmu.version;
> /*
> * KVM doesn't support the hybrid PMU yet.

Otherwise seems fine I suppose.