linux-kernel - Re: [PATCHv2 2/5] arm: mvebu: support for SMP on 98DX3336 SoC

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: <5edb6751-d15b-6654-f4ae-adf38fafb8a4@gmail.com>
Date:   Wed, 4 Jan 2017 20:04:16 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Chris Packham <chris.packham@...iedtelesis.co.nz>,
        linux-arm-kernel@...ts.infradead.org
Cc:     Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Jason Cooper <jason@...edaemon.net>,
        Andrew Lunn <andrew@...n.ch>,
        Gregory Clement <gregory.clement@...e-electrons.com>,
        Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
        Russell King <linux@...linux.org.uk>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Chris Brand <chris.brand@...adcom.com>,
        Sudeep Holla <sudeep.holla@....com>,
        Jayachandran C <jchandra@...adcom.com>,
        Juri Lelli <juri.lelli@....com>, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCHv2 2/5] arm: mvebu: support for SMP on 98DX3336 SoC

Le 01/04/17 à 19:36, Chris Packham a écrit :
> Compared to the armada-xp the 98DX3336 uses different registers to set
> the boot address for the secondary CPU so a new enable-method is needed.
> This will only work if the machine definition doesn't define an overall
> smp_ops because there is not currently a way of overriding this from the
> device tree if it is set in the machine definition.

Not sure I follow you here, in theory, each individual CPU could have a
different enable-method property, and considering how you leverage
existing DTS include files, you should be able to override the DTS with
the appropriate enable-method, or do you have a different problem?

 mv98dx3236_resume_set_cpu_boot_addr(int hw_cpu, void *boot_addr)
> +{
> +	WARN_ON(hw_cpu != 1);
> +
> +	writel(0, mv98dx3236_resume_base + MV98DX3236_CPU_RESUME_CTRL_OFFSET);
> +	writel(virt_to_phys(boot_addr), mv98dx3236_resume_base +
> +	       MV98DX3236_CPU_RESUME_ADDR_OFFSET);

I just submitted a patch series that switches such users to
__pa_symbol() instead of virt_to_phys() because this is a kernel image
symbol. For now this will work as-is, but depending on which patch
series makes it first, it may be a good idea to keep this on someone's
TODO list (yours or mine). I do expect having to make a second pass of
conversions anyway.

[1]: https://lkml.org/lkml/2017/1/4/1101

> +}
> +
> +static int __init mv98dx3236_resume_init(void)
> +{
> +	struct device_node *np;
> +	struct resource res;
> +	int ret = 0;
> +
> +	np = of_find_matching_node(NULL, of_mv98dx3236_resume_table);
> +	if (!np)
> +		return 0;
> +
> +	pr_info("Initializing 98DX3236 Resume\n");

This can be probably be dropped, you should know fairly quickly if this
succeeded or not.

Can't this function be implemented as a smp_ops::smp_init_cpus instead
of having this initialization done at arch_initcall time?

> +
> +	if (of_address_to_resource(np, 0, &res)) {
> +		pr_err("unable to get resource\n");
> +		ret = -ENOENT;
> +		goto out;
> +	}
> +
> +	if (!request_mem_region(res.start, resource_size(&res),
> +				np->full_name)) {
> +		pr_err("unable to request region\n");
> +		ret = -EBUSY;
> +		goto out;
> +	}

You should be able to use of_io_request_and_map() and here.
-- 
Florian