linux-kernel - Re: [PATCH] driver/perf: Add PMU driver for the ARM DMC-620 memory controller.

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: <20200318202356.GB7463@willie-the-truck>
Date:   Wed, 18 Mar 2020 20:23:57 +0000
From:   Will Deacon <will@...nel.org>
To:     Jonathan Cameron <Jonathan.Cameron@...wei.com>
Cc:     Tuan Phan <tuanphan@...amperecomputing.com>,
        Mark Rutland <mark.rutland@....com>,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH] driver/perf: Add PMU driver for the ARM DMC-620 memory
 controller.

On Wed, Mar 18, 2020 at 04:02:02PM +0000, Jonathan Cameron wrote:
> On Tue, 17 Mar 2020 17:29:38 -0700
> Tuan Phan <tuanphan@...amperecomputing.com> wrote:
> > +/* User ABI */
> > +#define ATTR_CFG_FLD_mask_CFG		config
> > +#define ATTR_CFG_FLD_mask_LO		0
> > +#define ATTR_CFG_FLD_mask_HI		44
> > +#define ATTR_CFG_FLD_match_CFG		config1
> > +#define ATTR_CFG_FLD_match_LO		0
> > +#define ATTR_CFG_FLD_match_HI		44
> > +#define ATTR_CFG_FLD_invert_CFG		config2
> > +#define ATTR_CFG_FLD_invert_LO		0
> > +#define ATTR_CFG_FLD_invert_HI		0
> > +#define ATTR_CFG_FLD_incr_CFG		config2
> > +#define ATTR_CFG_FLD_incr_LO		1
> > +#define ATTR_CFG_FLD_incr_HI		2
> > +#define ATTR_CFG_FLD_event_CFG		config2
> > +#define ATTR_CFG_FLD_event_LO		3
> > +#define ATTR_CFG_FLD_event_HI		8
> > +
> > +#define __GEN_PMU_FORMAT_ATTR(cfg, lo, hi)			\
> > +	(lo) == (hi) ? #cfg ":" #lo "\n" : #cfg ":" #lo "-" #hi
> > +
> > +#define _GEN_PMU_FORMAT_ATTR(cfg, lo, hi)			\
> > +	__GEN_PMU_FORMAT_ATTR(cfg, lo, hi)
> > +
> > +#define GEN_PMU_FORMAT_ATTR(name)				\
> > +	PMU_FORMAT_ATTR(name,					\
> > +	_GEN_PMU_FORMAT_ATTR(ATTR_CFG_FLD_##name##_CFG,		\
> > +			     ATTR_CFG_FLD_##name##_LO,		\
> > +			     ATTR_CFG_FLD_##name##_HI))
> > +
> > +#define _ATTR_CFG_GET_FLD(attr, cfg, lo, hi)			\
> > +	((((attr)->cfg) >> lo) & GENMASK(hi - lo, 0))
> 
> Hmm. I see this came form SPE pmu.
> 
> Personally I'd argue this makes the code harder to read than doing
> most of it long hand.  Ah well.

I agree that it's harder to read, but I did it this way in the SPE driver
so that the user ABI is always in sync with what the driver thinks, because
the accessors and the sysfs bits are all generated from the same constants.
If you screw that up, then it's really hard to fix without breaking
userspace.

Will