linux-kernel - Re: [PATCH v5 3/7] powerpc/powernv: Nest PMU detection and device tree parser

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:	Thu, 23 Jul 2015 19:16:55 +1000
From:	Michael Ellerman <mpe@...erman.id.au>
To:	Madhavan Srinivasan <maddy@...ux.vnet.ibm.com>
Cc:	Daniel Axtens <dja@...ens.net>, linux-kernel@...r.kernel.org,
	Stephane Eranian <eranian@...gle.com>,
	Paul Mackerras <paulus@...ba.org>,
	Anton Blanchard <anton@...ba.org>,
	Sukadev Bhattiprolu <sukadev@...ux.vnet.ibm.com>,
	linuxppc-dev@...ts.ozlabs.org,
	Anshuman Khandual <khandual@...ux.vnet.ibm.com>
Subject: Re: [PATCH v5 3/7] powerpc/powernv: Nest PMU detection and device
 tree parser

On Thu, 2015-07-23 at 11:24 +0530, Madhavan Srinivasan wrote:
> 
> On Wednesday 22 July 2015 09:19 AM, Daniel Axtens wrote:
> > Hi,
> >
> >> +static struct perchip_nest_info p8_nest_perchip_info[P8_NEST_MAX_CHIPS];
> >> +
> >> +static int nest_ima_dt_parser(void)
> >> +{
> >> +	const __be32 *gcid;
> >> +	const __be64 *chip_ima_reg;
> >> +	const __be64 *chip_ima_size;
> >> +	struct device_node *dev;
> >> +	struct perchip_nest_info *p8ni;
> >> +	int idx;
> >> +
> >> +	/*
> >> +	 * "nest-ima" folder contains two things,
> >> +	 * a) per-chip reserved memory region for Nest PMU Counter data
> >> +	 * b) Support Nest PMU units and their event files
> >> +	 */
> >> +	for_each_node_with_property(dev, "ibm,ima-chip") {
> >> +		gcid = of_get_property(dev, "ibm,chip-id", NULL);
> >> +		chip_ima_reg = of_get_property(dev, "reg", NULL);
> >> +		chip_ima_size = of_get_property(dev, "size", NULL);
> >> +
> >> +		if ((!gcid) || (!chip_ima_reg) || (!chip_ima_size)) {
> >> +			pr_err("Nest_PMU: device %s missing property\n",
> >> +							dev->full_name);
> >> +			return -ENODEV;
> >> +		}
> >> +
> >> +		/* chip id to save reserve memory region */
> >> +		idx = (uint32_t)be32_to_cpup(gcid);
> > So be32_to_cpup returns a __u32. You're casting to a uint32_t and then
> > assigning to an int.
> >  - Do you need the intermediate cast?
> >  - Should idx be an unsigned type?
> 
> my bad, sorry abt type case of uint to int.
> And your are right, idx can be __u32 (__u32 and uint32_t are same i
> guess). 

It should be u32. Don't use the uintx_t types in kernel code unless there's
some good reason for it.

The __u32 etc. types are for things that are exposed to userspace, which this
is not, so u32 is correct.

Having said that, this code should be using of_property_read_u32() etc.

And having said that, this is all based on a device tree binding that hasn't
been reviewed yet on the OPAL side, so it's subject to change too.

cheers


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/