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] [day] [month] [year] [list]

Message-ID: <HE1PR0501MB28125EED224EE4F487862974CE730@HE1PR0501MB2812.eurprd05.prod.outlook.com>
Date:   Tue, 28 Jul 2020 05:04:29 +0000
From:   Shravan Ramani <sramani@...lanox.com>
To:     Andy Shevchenko <andy.shevchenko@...il.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



> -----Original Message-----
> From: Andy Shevchenko <andy.shevchenko@...il.com>
> Sent: Monday, July 27, 2020 4:24 PM
> To: Shravan 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

Could you please elaborate on what approach you'd like me to take with the UUID manipulation?
I used the same approach as in drivers/platform/mellanox/mlxbf-bootctl.c which seemed like an appropriate example.
Any other pointers would be helpful.

Thanks for the feedback. Will address all the other comments in v2.

Regards,
Shravan

> - use kstroto*() API instead of sscanf. It has a range check
> 
> 
> --
> With Best Regards,
> Andy Shevchenko