linux-kernel - Re: [PATCH V3 16/17] irqchip/gic: Prepare for adding platform 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:	Fri, 6 May 2016 15:09:51 +0100
From:	Jon Hunter <jonathanh@...dia.com>
To:	Marc Zyngier <marc.zyngier@....com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Jason Cooper <jason@...edaemon.net>,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Stephen Warren <swarren@...dotorg.org>,
	Thierry Reding <thierry.reding@...il.com>
CC:	Kevin Hilman <khilman@...nel.org>,
	Geert Uytterhoeven <geert@...ux-m68k.org>,
	Grygorii Strashko <grygorii.strashko@...com>,
	Lars-Peter Clausen <lars@...afoo.de>,
	Linus Walleij <linus.walleij@...aro.org>,
	<linux-tegra@...r.kernel.org>, <linux-omap@...r.kernel.org>,
	<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V3 16/17] irqchip/gic: Prepare for adding platform driver

Hi Marc,

On 05/05/16 15:13, Marc Zyngier wrote:

[...]

> Gahhh. No. Please. Last time we did that, it took 6 months to untangle
> the mess people made by adding their own hacks in this structure, 
> so I definitely want to keep it completely private, forever. Same goes
> for the gic_{dist,cpu.pm}_init() functions.

OK.

> I've had a go at this, and came up with the following patch. I've only
> briefly tested it on a host and a VM, so it is likely to break some stuff
> somewhere, but you'll get the idea: The gic_chip_data struct is entirely
> opaque, allocated by the GIC driver itself, with a few new fields in
> it so that it becomes self-contained. This applies on top of your series.
> 
> It should also make it easy to switch to a model where we allocate
> the structure dynamically instead of the old static crap.
> 
> Thoughts?

Yes I have been doing some testing and with a couple tweaks we can make
something like this work. One thing that caught me out was ...

> +int gic_of_setup(struct device_node *node, struct device *dev,
> +		 struct gic_chip_data **gicp)
> +{
> +	struct gic_chip_data *gic;
>  
> -	*cpu_base = of_iomap(node, 1);
> -	if (WARN(!*cpu_base, "unable to map gic cpu registers\n")) {
> -		iounmap(*dist_base);
> -		return -ENOMEM;
> +	if (!node || !gicp)
> +		return -EINVAL;
> +
> +	if (dev) {
> +		*gicp = devm_kzalloc(dev, sizeof(*gic), GFP_KERNEL);
> +		if (!*gicp)
> +			return -ENOMEM;
>  	}
>  
> -	if (of_property_read_u32(node, "cpu-offset", percpu_offset))
> -		*percpu_offset = 0;
> +	gic = *gicp;
> +
> +	gic->raw_dist_base = of_iomap(node, 0);
> +	if (WARN(!gic->raw_dist_base, "unable to map gic dist registers\n"))
> +		goto err;
> +
> +	gic->raw_cpu_base = of_iomap(node, 1);
> +	if (WARN(!gic->raw_cpu_base, "unable to map gic cpu registers\n"))
> +		goto err;
> +
> +	if (of_property_read_u32(node, "cpu-offset", &gic->percpu_offset))
> +		gic->percpu_offset = 0;
>  
> +	gic->chip.parent_device = dev;

We can't initialise the device here as it gets overwritten in the
gic_init_bases. So I have had to re-organise things a bit. Good news is
that I have eliminated the call from the platform driver to
gic_init_bases so we only have a single call to initialise the GIC.

Cheers
Jon