linux-kernel - Re: [V14 3/8] drivers: perf: arm_pmuv3: Enable branch stack sampling framework

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5bf73672-4094-490b-b0a9-b983d512c904@arm.com>
Date:   Wed, 15 Nov 2023 11:14:12 +0530
From:   Anshuman Khandual <anshuman.khandual@....com>
To:     James Clark <james.clark@....com>
Cc:     Mark Brown <broonie@...nel.org>, Rob Herring <robh@...nel.org>,
        Marc Zyngier <maz@...nel.org>,
        Suzuki Poulose <suzuki.poulose@....com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        linux-perf-users@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        will@...nel.org, catalin.marinas@....com, mark.rutland@....com
Subject: Re: [V14 3/8] drivers: perf: arm_pmuv3: Enable branch stack sampling
 framework

On 11/14/23 15:28, James Clark wrote:
> 
> 
> On 14/11/2023 05:13, Anshuman Khandual wrote:
>> Branch stack sampling support i.e capturing branch records during execution
>> in core perf, rides along with normal HW events being scheduled on the PMU.
>> This prepares ARMV8 PMU framework for branch stack support on relevant PMUs
>> with required HW implementation.
>>
> 
> [...]
> 
>> - All armv8pmu_branch_xxxx() stub definitions have been moved inside
>>   include/linux/perf/arm_pmuv3.h for easy access from both arm32 and
>>   arm64 platforms
>>
> 
> This causes lots of W=1 build errors because the prototypes are in
> arm_pmuv3.h, but arm_brbe.c doesn't include it.

I guess these are the W=1 warnings you mentioned above.

drivers/perf/arm_brbe.c:11:6: warning: no previous prototype for ‘armv8pmu_branch_reset’ [-Wmissing-prototypes]
   11 | void armv8pmu_branch_reset(void)                                                                                                                                                                   
      |      ^~~~~~~~~~~~~~~~~~~~~                                                                                                                                                                         
drivers/perf/arm_brbe.c:190:6: warning: no previous prototype for ‘armv8pmu_branch_save’ [-Wmissing-prototypes]      
  190 | void armv8pmu_branch_save(struct arm_pmu *arm_pmu, void *ctx)                      
      |      ^~~~~~~~~~~~~~~~~~~~                                                                                                                                                                          
drivers/perf/arm_brbe.c:236:6: warning: no previous prototype for ‘armv8pmu_branch_attr_valid’ [-Wmissing-prototypes]
  236 | bool armv8pmu_branch_attr_valid(struct perf_event *event)                                                                                                                                          
      |      ^~~~~~~~~~~~~~~~~~~~~~~~~~                                                                                                                                                                    
drivers/perf/arm_brbe.c:269:5: warning: no previous prototype for ‘armv8pmu_task_ctx_cache_alloc’ [-Wmissing-prototypes]
  269 | int armv8pmu_task_ctx_cache_alloc(struct arm_pmu *arm_pmu)                            
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~                                                            
drivers/perf/arm_brbe.c:279:6: warning: no previous prototype for ‘armv8pmu_task_ctx_cache_free’ [-Wmissing-prototypes]
  279 | void armv8pmu_task_ctx_cache_free(struct arm_pmu *arm_pmu)                       
      |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~                                                            
drivers/perf/arm_brbe.c:303:6: warning: no previous prototype for ‘armv8pmu_branch_probe’ [-Wmissing-prototypes]
  303 | void armv8pmu_branch_probe(struct arm_pmu *armpmu)                                
      |      ^~~~~~~~~~~~~~~~~~~~~                                                                   
drivers/perf/arm_brbe.c:449:6: warning: no previous prototype for ‘armv8pmu_branch_enable’ [-Wmissing-prototypes]
  449 | void armv8pmu_branch_enable(struct arm_pmu *arm_pmu)                             
      |      ^~~~~~~~~~~~~~~~~~~~~~                                                                  
drivers/perf/arm_brbe.c:474:6: warning: no previous prototype for ‘armv8pmu_branch_disable’ [-Wmissing-prototypes]
  474 | void armv8pmu_branch_disable(void)                                                           
      |      ^~~~~~~~~~~~~~~~~~~~~~~                                                                 
drivers/perf/arm_brbe.c:717:6: warning: no previous prototype for ‘armv8pmu_branch_read’ [-Wmissing-prototypes]
  717 | void armv8pmu_branch_read(struct pmu_hw_events *cpuc, struct perf_event *event)

Branch helpers are used in ARM PMU V3 driver i.e drivers/perf/arm_pmuv3.c.
Whether the actual BRBE helper definitions, or their fallback stubs (when
CONFIG_ARM64_BRBE is not enabled), need to be accessible from arm_pmuv3.c
driver not from brbe.c implementations itself.

> 
> It seems like the main reason you can't include arm_brbe.h in arm32 code
> is because there are a load of inline functions and references to
> registers in there. But these are only used in arm_brbe.c, so they don't

Right, arm32 should not be exposed to BRBE internals via arm_brbe.h header.

> need to be in the header anyway.

Right, these are only used in arm_brbe.c

> 
> If you removed the code from the header and moved it to the source file
> you could move the brbe prototypes to the brbe header and it would be a
> bit cleaner and more idiomatic.

Alight, how about the following changes - build tested on arm32 and arm64.

- Move BRBE helpers from arm_brbe.h into arm_brbe.c
- Move armv8_pmu_xxx() declaration inside arm_brbe.h for arm64 (CONFIG_ARM64_BRBE)
- Move armv8_pmu_xxx() stub definitions inside arm_pmuv3.c for arm32 (!CONFIG_ARM64_BRBE)
- Include arm_brbe.h header both in arm_pmuv3.c and arm_brbe.c