[bug report] perf/x86/amd/uncore: Refactor uncore management

[bug report] perf/x86/amd/uncore: Refactor uncore management

[Dan Carpenter]

Hello Sandipan Das,

Commit d6389d3ccc13 ("perf/x86/amd/uncore: Refactor uncore
management") from Oct 5, 2023 (linux-next), leads to the following
Smatch static checker warning:

	arch/x86/events/amd/uncore.c:660 amd_uncore_df_event_init()
	warn: set error code if 'pmu_version < 2'

arch/x86/events/amd/uncore.c
    654 static int amd_uncore_df_event_init(struct perf_event *event)
    655 {
    656         struct hw_perf_event *hwc = &event->hw;
    657         int ret = amd_uncore_event_init(event);
    658 
    659         if (ret || pmu_version < 2)
--> 660                 return ret;

If pmu_version is < 2, do we really want to just return success?

    661 
    662         hwc->config = event->attr.config &
    663                       (pmu_version >= 2 ? AMD64_PERFMON_V2_RAW_EVENT_MASK_NB :
    664                                           AMD64_RAW_EVENT_MASK_NB);

Or do we want to set:

	hwc->config = event->attr.config & AMD64_RAW_EVENT_MASK_NB;

And then return success?

    665 
    666         return 0;
    667 }

regards,
dan carpenter

Re: [bug report] perf/x86/amd/uncore: Refactor uncore management

[Sandipan Das]

Hi Dan,

Thanks for reporting this.

On 12/8/2025 1:44 PM, Dan Carpenter wrote:
> Hello Sandipan Das,
> 
> Commit d6389d3ccc13 ("perf/x86/amd/uncore: Refactor uncore
> management") from Oct 5, 2023 (linux-next), leads to the following
> Smatch static checker warning:
> 
> 	arch/x86/events/amd/uncore.c:660 amd_uncore_df_event_init()
> 	warn: set error code if 'pmu_version < 2'
> 
> arch/x86/events/amd/uncore.c
>     654 static int amd_uncore_df_event_init(struct perf_event *event)
>     655 {
>     656         struct hw_perf_event *hwc = &event->hw;
>     657         int ret = amd_uncore_event_init(event);
>     658 
>     659         if (ret || pmu_version < 2)
> --> 660                 return ret;
> 
> If pmu_version is < 2, do we really want to just return success?
> 
>     661 
>     662         hwc->config = event->attr.config &
>     663                       (pmu_version >= 2 ? AMD64_PERFMON_V2_RAW_EVENT_MASK_NB :
>     664                                           AMD64_RAW_EVENT_MASK_NB);
> 
> Or do we want to set:
> 
> 	hwc->config = event->attr.config & AMD64_RAW_EVENT_MASK_NB;
> 
> And then return success?
> 

Right. If amd_uncore_event_init() returns an error, this should bail out
irrespective of pmu_version. I think it should be okay to set hwc->config
even if amd_uncore_event_init() errors out so that this can be simplified
further.

@@ -656,14 +656,11 @@ static int amd_uncore_df_event_init(struct perf_event *event)
        struct hw_perf_event *hwc = &event->hw;
        int ret = amd_uncore_event_init(event);

-       if (ret || pmu_version < 2)
-               return ret;
-
        hwc->config = event->attr.config &
                      (pmu_version >= 2 ? AMD64_PERFMON_V2_RAW_EVENT_MASK_NB :
                                          AMD64_RAW_EVENT_MASK_NB);

-       return 0;
+       return ret;
 }

 static int amd_uncore_df_add(struct perf_event *event, int flags

>     665 
>     666         return 0;
>     667 }
>