linux-kernel - Re: [PATCH V2 16/19] csky: SMP support

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 Jul 2018 06:24:33 +0100
From:   Mark Rutland <mark.rutland@....com>
To:     Guo Ren <ren_guo@...ky.com>
Cc:     linux-arch@...r.kernel.org, linux-kernel@...r.kernel.org,
        tglx@...utronix.de, daniel.lezcano@...aro.org,
        jason@...edaemon.net, arnd@...db.de, c-sky_gcc_upstream@...ky.com,
        gnu-csky@...tor.com, thomas.petazzoni@...tlin.com,
        wbx@...ibc-ng.org, green.hu@...il.com
Subject: Re: [PATCH V2 16/19] csky: SMP support

On Mon, Jul 02, 2018 at 01:30:19AM +0800, Guo Ren wrote:
> +static int csky_of_cpu(struct device_node *node)
> +{
> +	const char *status;
> +	int cpu;
> +
> +	if (of_property_read_u32(node, "reg", &cpu))
> +		goto error;
> +
> +	if (cpu >= NR_CPUS)
> +		goto error;
> +
> +	if (of_property_read_string(node, "status", &status))
> +		status = "enable";
> +
> +	if (strcmp(status, "disable") == 0)
> +		goto error;

Please use of_device_is_available(node); "enable" is not a sensible value for
the status property, and "disable" (rather than "disabled") is simply unusual.

Neither "enable" nor "disable" are correct values for the status property.

What is the value in the reg property, exactly? Is there a unique ID in
hardware for each CPU in the system?

It would be good to document this, e.g. as arm does in
Documentation/devicetree/bindings/arm/cpus.txt

> +
> +	return cpu;
> +error:
> +	return -ENODEV;
> +}
> +
> +void __init setup_smp(void)
> +{
> +	struct device_node *node = NULL;
> +	int cpu;
> +
> +	while ((node = of_find_node_by_type(node, "cpu"))) {
> +		cpu = csky_of_cpu(node);
> +		if (cpu >= 0) {
> +			set_cpu_possible(cpu, true);
> +			set_cpu_present(cpu, true);
> +		}
> +	}
> +}

What happens if/when the value in the reg property is larger than NR_CPUS?

> +int __cpu_up(unsigned int cpu, struct task_struct *tidle)
> +{
> +	unsigned int tmp;
> +
> +	secondary_stack = (unsigned int)tidle->stack + THREAD_SIZE;
> +
> +	secondary_hint = mfcr("cr31");
> +
> +	secondary_ccr  = mfcr("cr18");
> +
> +	pr_info("%s: CPU%u\n", __func__, cpu);
> +
> +	tmp = mfcr("cr<29, 0>");
> +	tmp |= 1 << cpu;
> +	mtcr("cr<29, 0>", tmp);
> +
> +	while (!cpu_online(cpu));
> +
> +	secondary_stack = 0;
> +
> +	return 0;
> +}

I don't see a start address being setup here, so I assume that CPUs branch to a
fixed address out-of-reset. Does that mean that the kernel has to be loaded at
a particular physical address on a given platform?

Thanks,
Mark.