Re: [PATCH] perf: arm_pmuv3: Zero initialize hw_id branch stack field

On 07/08/26 2:44 PM, James Clark wrote:
> PERF_SAMPLE_BRANCH_HW_INDEX is supported by BRBE so hw_id is passed to
> userspace, but it's never set by the BRBE driver. Zero initialize it as
> it should be according to the docs:
> 
>    * For the architectures whose raw branch records are
>    * already stored in age order, the hw_idx should be 0.

The in code documentation while defining perf_branch_stack.
Probably a good idea to specify the same above.

 * For the architectures whose raw branch records are
 * already stored in age order, the hw_idx should be 0.
 */
struct perf_branch_stack {
	u64				nr;
	u64				hw_idx;
	struct perf_branch_entry	entries[];
};

> 
> It's probably too risky to remove PERF_SAMPLE_BRANCH_HW_INDEX from BRBE
> now in case anyone is setting it and reading the value, but zero
> initializing the whole struct also protects against the same issue with
> new fields that are added in the future.

Agreed. Because PERF_SAMPLE_BRANCH_HW_INDEX is supported in BRBE,
hw_idx pushed to the userspace should be zero if HW never updates.
This is definitely better than dropping PERF_SAMPLE_BRANCH_HW_INDEX
flag all together to avoid breaking current users (if any).
> 
> Fixes: 58074a0fce66 ("perf: arm_pmuv3: Add support for the Branch Record Buffer Extension (BRBE)")
> Signed-off-by: James Clark <[email protected]>

Reviewed-by: Anshuman Khandual <[email protected]>

> ---
> Very small fix spotted by Sashiko. It was probably always zero during
> testing or never looked at.
> ---
>  drivers/perf/arm_pmuv3.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
> index 8014ff766cff..b9a8592bf112 100644
> --- a/drivers/perf/arm_pmuv3.c
> +++ b/drivers/perf/arm_pmuv3.c
> @@ -1361,7 +1361,7 @@ static int branch_records_alloc(struct arm_pmu *armpmu)
>  		struct pmu_hw_events *events_cpu;
>  
>  		events_cpu = per_cpu_ptr(armpmu->hw_events, cpu);
> -		events_cpu->branch_stack = kmalloc(size, GFP_KERNEL);
> +		events_cpu->branch_stack = kzalloc(size, GFP_KERNEL);
>  		if (!events_cpu->branch_stack)
>  			return -ENOMEM;
>  	}
> 
> ---
> base-commit: f9a2394a23482bfd330911e9c8295b71724feacd
> change-id: 20260807-james-brbe-init-hw-idx-53dec54fe266
> 
> Best regards,
> --  
> James Clark <[email protected]>
>