Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <aa2ef58a-f02b-8c9d-2b62-8587f3b091bc@amd.com>
Date:   Wed, 28 Jun 2023 10:19:49 +0530
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [PATCH v2] perf evsel amd: Fix IBS error message
To:     Namhyung Kim <namhyung@kernel.org>
Cc:     acme@kernel.org, peterz@infradead.org, irogers@google.com,
        jolsa@kernel.org, adrian.hunter@intel.com,
        linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
        sandipan.das@amd.com, ananth.narayan@amd.com,
        santosh.shukla@amd.com, Ravi Bangoria <ravi.bangoria@amd.com>
References: <20230626103958.834-1-ravi.bangoria@amd.com>
 <CAM9d7cgL2+jHtyy6k19NbmW1cD+RGRkwUANDk8xH3oGqxB0qqQ@mail.gmail.com>
Content-Language: en-US
From:   Ravi Bangoria <ravi.bangoria@amd.com>
In-Reply-To: <CAM9d7cgL2+jHtyy6k19NbmW1cD+RGRkwUANDk8xH3oGqxB0qqQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?OTBvZEpJcWpRQURyd0VUdDlZbUF4WkFBZmlrMUVCczZnWWZlSGFUN3BzM3lC?=
 =?utf-8?B?S0l1RU9pSW1JYUVqZStnZjlUMGFBaE04U3FlWlRHWWdFZTJsN0NMTUZUZ1p4?=
 =?utf-8?B?RlowSDRlVFQwZE1GdkcxczQ4Q0tnQlZiWkJVNW94KzVDYW5uYi9abWNOM3cw?=
 =?utf-8?B?aUxrREZKSE1HWVVvUWlhSmhER1BLVG1tY2xVZlBoVFZBU01DZEtWOXQ2MXlm?=
 =?utf-8?B?N3VVUnNnRGlGeWNzVmYzWUtWYWFYeVFoSlpMYm83Nks2eHBMQUZ3RWhBTU9B?=
 =?utf-8?B?aU9LbWxpQ2ZzSDJLbnI2UENyaVFLbkZMOThGWDl4R1JmaUliTmFnelJuM2hj?=
 =?utf-8?B?d25PQzl2YlJsZE9KT1ZwdTVvenJ2WmpHejZyOUJZcXR3SzNtbUp0aTlnUkJP?=
 =?utf-8?B?UkxOYlZoei9aNHRSNXBxek53UXRnNFpLaEtLK2JML0I2WjdEU25UdERoVlBj?=
 =?utf-8?B?RHZDQ2FXcHMzZEtIeXNwMldNY2VlYU55bi85bEZMTHFsS1ZEU1lOb0ZnSy9V?=
 =?utf-8?B?TU40eUFRTUpEZnY1K1FNLzZra1YrWW50Tk5oMGNIU2NsZmVvbVgxdGRCbFdi?=
 =?utf-8?B?UzFqdndoNzBGME5jb0Y3UTFTUFVTdmYxbVZaK0tSR3J3QTRiYTl3RG9TYmYz?=
 =?utf-8?B?alBzWjUrM1EyTTY4ZVdNQjM3K2t0dlI4NDMrVEREUjVVZWpycUlFRk4rM0U4?=
 =?utf-8?B?bHZBQ1pXQzMvU28xaWZtSkI1Sm1mZC83STdSUDF0bVl3L2Y5M05Xd09pOEdx?=
 =?utf-8?B?YllJUjZHYjlnRm01SEIxSExYQUZseE5WbjJOaVFtYmhSeThTeklkKzhJRFpX?=
 =?utf-8?B?WXJkaTBUVU9oeS8wVks2WmxoQnBmT0M5V0tYQUhWUmkxT095V21jRWZPMXdm?=
 =?utf-8?B?ajZIRUtxNjNYSjM0TnNmMDd1MmNNMjh6ejljN2JOOTF1NlJobTNYdFVkTzVS?=
 =?utf-8?B?WnVmc3VJaHdTeGZQVVVEQUwraDBoTklITzhEMjFBUTVkS29tY29RNVNmQ05I?=
 =?utf-8?B?RXVLelhoNTNQWTAra1p3WXRJNyt3bmgxZy9ua3N3QVI5bWpjcVFoSURkMHhv?=
 =?utf-8?B?QkxwQzB3SjFveEtPZXRscEpqOHk2TTAyVTlFMFIvWWZ5a1RsM0QxeDhZOCtQ?=
 =?utf-8?B?aFFPTkhZcUNuT0xHeHZ4eUVtTHJjdUlxbzNBaWp4SVJxdXNwQWNmck1GbjU1?=
 =?utf-8?B?M0tPZnVkTFQyeDVJd1hVbmlWTlcwZDFNUUdtYTNmTmZEdDR6TGlkN1pIQmRo?=
 =?utf-8?B?clFzY1VYeGQ5d2pxWjE4ajUzTzFxc0xIbjU1ck1ibWw2YmJzU21CMm1JU2Vm?=
 =?utf-8?B?TzRoK01ZSEFkcmpwcWFLZXhabXVvMkUwS09aRWdRQUVpa3ZqS21YS1kybkZ0?=
 =?utf-8?B?Qldsd2VpVzJTWndYVnRoQnRCMWFBa2NjMDI3SEVYeUpxay91QXcrbjFJRlFB?=
 =?utf-8?B?UEVNaDFzN0dCS0FuYjVITElYUzl5elludlpSYVk2SjlTbEhmM0prYTBvSUhO?=
 =?utf-8?B?OEw2aTFzVytkSW8wV0VTOUhXMUV2a1F0SUcxTFBMZFgzckltS1FjaXZvMVQw?=
 =?utf-8?B?RzJFUnl0NXc3NTZiM1ZsNEFtNEwrRkQyK1pQMkpnTjBST3R2cG5YcHAyQ0Rm?=
 =?utf-8?B?ekZnZ0M4Q3NSM2JmNmd5T21KYm5CL1FndG1FQzBHRHlacTFqSDlPMFltTHY1?=
 =?utf-8?B?cFRUYWltNnAvNkNna3QwZnhRT1ErYy81eTc0TkpCeVorTlFWYndJa0prWHdE?=
 =?utf-8?B?K2ZBbmR2THZ3R0VMSkZtZWVUaytlRFI0d1huT2JFRURYbGdGRmErRkxxOFFt?=
 =?utf-8?B?ajVybVgwbDBtNkFGMXlEeE1IeWhEdlB5VDI1cTBPVEwrbU00dFdGS05ZNzlT?=
 =?utf-8?B?WDhxYVNVRXQrbFMyckRvVlZPNmtHYUpEa3d4WGluTHllZ3FuakRxNFhDclZi?=
 =?utf-8?B?eWc2VEJWQlVGZDN3K2dvaEs5ZFNJeUVveDhHYWxGUEJ4WEhWdUgzRmY5Q29W?=
 =?utf-8?B?UmNoM2FKT3pheE1wVnVMbmM5Q0pONEtoK1NieThNSEFmc0JjRm9yNThMZHJI?=
 =?utf-8?B?bTZoYVg2ZVBFL29qZmVzZDh0SzNkbGFWTjErK2swc3VWc01aaFczSFpuNDJJ?=
 =?utf-8?B?SzE1YStiY05Yc1kwNVd5c2FxNDN2bHBVVkZBcmpReFhwVXVnOTQyblI1Y0ZE?=
 =?utf-8?Q?YrlKyJUu3eIf2Ggaj4nBAic1vnrryeqcuDEIbeZDaCPR?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 50b49651-d83f-4190-f49e-08db77933654
X-MS-Exchange-CrossTenant-AuthSource: PH7PR12MB6588.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Jun 2023 04:50:36.1843
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: qzytF5NnSiR5QvFaPdzzEFtqv6koL2J1bN1816naSxLiu3ralCj6BQ+4GhkD9OAshQtrG2dF8nrqpaqwMP9JXQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR12MB6682
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 27-Jun-23 4:34 AM, Namhyung Kim wrote:
> Hi Ravi,
> 
> On Mon, Jun 26, 2023 at 3:40 AM Ravi Bangoria <ravi.bangoria@amd.com> wrote:
>>
>> AMD IBS can do per-process profiling[1] and is no longer restricted to
>> per-cpu or systemwide only. Remove stale error message. Also, checking
>> just exclude_kernel is not sufficient since IBS does not support any
>> privilege filters. So include all exclude_* checks. And finally, move
>> these checks under tools/perf/arch/x86/ from generic code.
>>
>> Before:
>>   $ sudo ./perf record -e ibs_op//k -C 0
>>   Error:
>>   AMD IBS may only be available in system-wide/per-cpu mode.  Try
>>   using -a, or -C and workload affinity
>>
>> After:
>>   $ sudo ./perf record -e ibs_op//k -C 0
>>   Error:
>>   AMD IBS doesn't support privilege filtering. Try again with
>>   exclude_{kernel|user|hv|idle|host|guest}=0.
> 
> Can we have more user-friendly messages like below?
> 
>  "Try again without the privilege modifiers like 'k' at the end."
Sure.

> 
>>
>> Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
>> ---
>> v1: https://lore.kernel.org/r/20230621062359.201-1-ravi.bangoria@amd.com
>> v1->v2:
>>   - Check all exclude_* flags not just exclude_kernel
>>   - Move AMD specific checks under tools/perf/arch/x86/
>>
>>  tools/perf/arch/x86/util/evsel.c | 25 +++++++++++++++++++++++++
>>  tools/perf/util/evsel.c          | 30 +++++++++---------------------
>>  tools/perf/util/evsel.h          |  1 +
>>  3 files changed, 35 insertions(+), 21 deletions(-)
>>
>> diff --git a/tools/perf/arch/x86/util/evsel.c b/tools/perf/arch/x86/util/evsel.c
>> index 512c2d885d24..9a7141c5a4ea 100644
>> --- a/tools/perf/arch/x86/util/evsel.c
>> +++ b/tools/perf/arch/x86/util/evsel.c
>> @@ -102,3 +102,28 @@ void arch__post_evsel_config(struct evsel *evsel, struct perf_event_attr *attr)
>>                 }
>>         }
>>  }
>> +
>> +int arch_evsel__open_strerror(struct evsel *evsel, char *msg, size_t size)
>> +{
>> +       if (!x86__is_amd_cpu())
>> +               return 0;
>> +
>> +       if (!evsel->core.attr.precise_ip &&
>> +           !(evsel->pmu_name && !strncmp(evsel->pmu_name, "ibs", 3)))
>> +               return 0;
>> +
>> +       /* More verbose IBS errors. */
>> +       if (evsel->core.attr.exclude_kernel || evsel->core.attr.exclude_user ||
>> +           evsel->core.attr.exclude_hv || evsel->core.attr.exclude_idle ||
>> +           evsel->core.attr.exclude_host || evsel->core.attr.exclude_guest) {
>> +               return scnprintf(msg, size, "AMD IBS doesn't support privilege filtering. Try "
>> +                                "again with exclude_{kernel|user|hv|idle|host|guest}=0.");
>> +       }
>> +
>> +       if (!evsel->core.attr.sample_period) {
>> +               return scnprintf(msg, size, "AMD IBS doesn't support counting mode. Try "
>> +                                "again with sample_{period|freq} with non-zero value.");
> 
> Is this for perf stat?   We don't allow zero period for perf record.
> 
>   $ perf record -F 0 true
>   frequency and count are zero, aborting
> 
> Then maybe it can say: "IBS doesn't support perf stat, Use perf record."

Ok. Let me skip this since generic code is already covering it.

Thanks,
Ravi