Re: [PATCH v3 5/7] perf: arm_pmuv3: Add pmuv3_event_implemented()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] perf: arm_pmuv3: circular header inclusion in pmuv3_event_implemented()
--

commit fe846ad9d18510169af8401fb7c59990c15ea903
Author: Amir Ayupov <aaupov@fb.com>

perf: arm_pmuv3: Add pmuv3_event_implemented()

This commit adds pmuv3_event_implemented() to include/linux/perf/arm_pmuv3.h
to allow drivers outside the PMU driver to check whether the local CPU's PMU
implements a common event via PMCEID0/1, and moves
ARMV8_PMUV3_EXT_COMMON_EVENT_BASE into include/linux/perf/arm_pmuv3.h.

> diff --git a/include/linux/perf/arm_pmuv3.h b/include/linux/perf/arm_pmuv3.h
> index d698efba28a27..965c40b28dd77 100644
> --- a/include/linux/perf/arm_pmuv3.h
> +++ b/include/linux/perf/arm_pmuv3.h

[ ... ]

> @@ -315,4 +318,23 @@
>  
>  #include <asm/arm_pmuv3.h>
>  
> +/*
> + * Whether this CPU's PMU implements a common event, as reported by
> + * PMCEID{0,1}: events 0x0000-0x003f in bits [31:0], and from PMUv3p1 events
> + * 0x4000-0x403f in bits [63:32], which read as zero before PMUv3p1.
> + * Must run on the CPU whose PMU is queried.
> + */
> +static inline bool pmuv3_event_implemented(u16 event)
> +{
> +	bool ext = event >= ARMV8_PMUV3_EXT_COMMON_EVENT_BASE;
> +	u16 id = ext ? event - ARMV8_PMUV3_EXT_COMMON_EVENT_BASE : event;
> +	u64 pmceid;
> +
> +	if (id >= 64 || !pmuv3_implemented(read_pmuver()))
> +		return false;
> +
> +	pmceid = id < 32 ? read_pmceid0() : read_pmceid1();
> +	return pmceid & BIT_ULL(id % 32 + (ext ? 32 : 0));
> +}

[Severity: Low]
Does defining pmuv3_event_implemented() here break arm64 builds due to a
circular header dependency when a translation unit includes
<asm/arm_pmuv3.h> before <asm/kvm_host.h> or <linux/perf/arm_pmuv3.h>?

For example, arch/arm64/kernel/cpufeature.c includes <asm/arm_pmuv3.h>
first:

    #include <asm/arm_pmuv3.h>
    #include <asm/cpu.h>
    #include <asm/cpufeature.h>
    #include <asm/cpu_ops.h>
    #include <asm/fpsimd.h>
    #include <asm/hwcap.h>
    #include <asm/insn.h>
    #include <asm/kvm_host.h>

In arch/arm64/include/asm/arm_pmuv3.h, the header guard __ASM_PMUV3_H is
defined and <asm/kvm_host.h> is included at the top before read_pmuver(),
read_pmceid0(), read_pmceid1(), and pmuv3_implemented() are defined:

    #ifndef __ASM_PMUV3_H
    #define __ASM_PMUV3_H

    #include <asm/kvm_host.h>

That triggers the inclusion chain:

    arch/arm64/include/asm/arm_pmuv3.h
      -> arch/arm64/include/asm/kvm_host.h
        -> include/kvm/arm_pmu.h
          -> include/linux/perf/arm_pmuv3.h

When include/linux/perf/arm_pmuv3.h reaches #include <asm/arm_pmuv3.h>, it
is skipped because __ASM_PMUV3_H is already defined, so
pmuv3_event_implemented() is compiled before pmuv3_implemented(),
read_pmuver(), read_pmceid0(), and read_pmceid1() have been declared.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1792907332.git.aaupov@fb.com?part=5