linux-kernel - Re: [PATCH v2 2/4] platform/mellanox: mlxbf-pmc: Add support for 64-bit counters and cycle count

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:
 <DM4PR12MB513695D2BE98AA46A95B4C60C0FF2@DM4PR12MB5136.namprd12.prod.outlook.com>
Date: Mon, 3 Jun 2024 10:29:50 +0000
From: Shravan Ramani <shravankr@...dia.com>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
CC: Hans de Goede <hdegoede@...hat.com>, Vadim Pasternak <vadimp@...dia.com>,
	David Thompson <davthompson@...dia.com>,
	"platform-driver-x86@...r.kernel.org" <platform-driver-x86@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/4] platform/mellanox: mlxbf-pmc: Add support for
 64-bit counters and cycle count

> > Both these features are supported by BlueField-3 PMC hardware, hence
> > the required bit-fields are exposed by the driver via sysfs to allow
> > the user to configure as needed.
> 
> I'm trying to understand what happens for the other counter, when the
> use_odd_counter is enabled? This change also doesn't add code that would
> make the other counter -EBUSY, should that be done?

When 2 32-bit counters are coupled to form a 64-bit counter using this setting,
one counter will hold the lower 32 bits while the other will hold the upper 32.
So the other counter (or syses corresponding to it) also needs to be accessed.

> For 64-bit counter, I suppose the userspace is expected to read the full
> counter from two sysfs files and combine the value (your documentation
> doesn't explain this)? That seems non-optimal, why cannot kernel just
> return the full combined 64-value directly in kernel?

I will add more clear comments for this.
While it is true that the driver could combine the 2 fields and present a
64-bit value via one of the sysfs, the reason for the current approach is that
there are other interfaces which expose the same counters for our platform
and there are tools that are expected to work on top of both interfaces for
the purpose of collecting performance stats. The other interfaces follow this
approach of having lower and upper 32-bits separately in each counter, and
the tools expect the same. Hence the driver follows this approach to keep
things consistent across the BlueField platform.

> 
> Similarly, are these cycle counters occupying the same space as non-cycle
> counters (so both can/cannot be used that the same time)? I'm asking this
> because you're adding a parallel interface to read the value and if it's
> either-or, I don't understand why the value needs to be read from
> different file depending on the counter counting in cycles or not.

It is the same file. The count_clock sysfs exposes 16 bits, one for each counter,
to allow the user to dedicate any of the 16 counters to counting cycles. Once
set, the corresponding counter can no longer monitor other events, and the
same sysfs can be accessed to read the cycle count. Again, I will try capture
this better and more elaborately in the documentation.

Thanks,
Shravan