linux-kernel - Re: [PATCH v1] platform/mellanox: mlxbf-pmc: Add Mellanox BlueField PMC driver

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]

Date:   Mon, 27 Jul 2020 13:53:50 +0300
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Shravan Kumar Ramani <sramani@...lanox.com>
Cc:     Andy Shevchenko <andy@...radead.org>,
        Darren Hart <dvhart@...radead.org>,
        Vadim Pasternak <vadimp@...lanox.com>,
        Jiri Pirko <jiri@...lanox.com>,
        Platform Driver <platform-driver-x86@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1] platform/mellanox: mlxbf-pmc: Add Mellanox BlueField
 PMC driver

On Mon, Jul 27, 2020 at 12:02 PM Shravan Kumar Ramani
<sramani@...lanox.com> wrote:
>
> The performance modules in BlueField are present in several hardware
> blocks and each block provides access to these stats either through
> counters that can be programmed to monitor supported events or
> through memory-mapped registers that hold the relevant information.
> The hardware blocks that include a performance module are:
>  * Tile (block containing 2 cores and a shared L2 cache)
>  * TRIO (PCIe root complex)
>  * MSS (Memory Sub-system containing the Memory Controller and L3 cache)
>  * GIC (Interrupt controller)
>  * SMMU (System Memory Management Unit)
> The mlx_pmc driver provides access to all of these performance modules
> through a hwmon sysfs interface.

Just brief comments:
- consider to revisit header block to see what is really necessary and
what can be dropped
- add comma to the arrays where last line is not a termination
- look at match_string() / sysfs_match_string() API, I think they can
be utilised here
- UUID manipulations (esp. with that GUID_INIT() against non-constant)
seems too much, consider refactoring and cleaning up these pieces
- use kstroto*() API instead of sscanf. It has a range check


-- 
With Best Regards,
Andy Shevchenko