| 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
| ||
|
Message-ID: <54D335F0.5080508@linaro.org>
Date: Thu, 05 Feb 2015 17:20:48 +0800
From: Hanjun Guo <hanjun.guo@...aro.org>
To: Mark Rutland <mark.rutland@....com>
CC: Catalin Marinas <Catalin.Marinas@....com>,
"Rafael J. Wysocki" <rjw@...ysocki.net>,
Olof Johansson <olof@...om.net>, Arnd Bergmann <arnd@...db.de>,
"grant.likely@...aro.org" <grant.likely@...aro.org>,
Will Deacon <Will.Deacon@....com>,
Lorenzo Pieralisi <Lorenzo.Pieralisi@....com>,
"graeme.gregory@...aro.org" <graeme.gregory@...aro.org>,
Sudeep Holla <Sudeep.Holla@....com>,
"jcm@...hat.com" <jcm@...hat.com>,
Jason Cooper <jason@...edaemon.net>,
Marc Zyngier <Marc.Zyngier@....com>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Daniel Lezcano <daniel.lezcano@...aro.org>,
Mark Brown <broonie@...nel.org>, Rob Herring <robh@...nel.org>,
Robert Richter <rric@...nel.org>,
Randy Dunlap <rdunlap@...radead.org>,
Charles Garcia-Tobin <Charles.Garcia-Tobin@....com>,
"phoenix.liyi@...wei.com" <phoenix.liyi@...wei.com>,
Timur Tabi <timur@...eaurora.org>,
Ashwin Chaugule <ashwinc@...eaurora.org>,
"suravee.suthikulpanit@....com" <suravee.suthikulpanit@....com>,
Mark Langsdorf <mlangsdo@...hat.com>,
"wangyijing@...wei.com" <wangyijing@...wei.com>,
"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linaro-acpi@...ts.linaro.org" <linaro-acpi@...ts.linaro.org>,
Tomasz Nowicki <tomasz.nowicki@...aro.org>
Subject: Re: [PATCH v8 13/21] ARM64 / ACPI: Parse MADT for SMP initialization
On 2015年02月04日 18:30, Mark Rutland wrote:
> On Wed, Feb 04, 2015 at 09:05:13AM +0000, Hanjun Guo wrote:
>> On 2015年02月03日 21:53, Mark Rutland wrote:
>>> On Mon, Feb 02, 2015 at 12:45:41PM +0000, Hanjun Guo wrote:
>>>> MADT contains the information for MPIDR which is essential for
>>>> SMP initialization, parse the GIC cpu interface structures to
>>>> get the MPIDR value and map it to cpu_logical_map(), and add
>>>> enabled cpu with valid MPIDR into cpu_possible_map.
>>>>
>>>> ACPI 5.1 only has two explicit methods to boot up SMP, PSCI and
>>>> Parking protocol, but the Parking protocol is only specified for
>>>> ARMv7 now, so make PSCI as the only way for the SMP boot protocol
>>>> before some updates for the ACPI spec or the Parking protocol spec.
>>>>
>>>> Parking protocol patches for SMP boot will be sent to upstream when
>>>> the new version of Parking protocol is ready.
[...]
>>>> + /* No need to check duplicate MPIDRs for the first CPU */
>>>> + if (enabled_cpus) {
>>>> + /*
>>>> + * Duplicate MPIDRs are a recipe for disaster. Scan
>>>> + * all initialized entries and check for
>>>> + * duplicates. If any is found just ignore the CPU.
>>>> + */
>>>> + for_each_possible_cpu(cpu) {
>>>> + if (cpu_logical_map(cpu) == mpidr) {
>>>> + pr_err("Firmware bug, duplicate CPU MPIDR: 0x%llx in MADT\n",
>>>> + mpidr);
>>>> + return -EINVAL;
>>>> + }
>>>> + }
>>>> +
>>>> + /* allocate a logical cpu id for the new comer */
>>>> + cpu = cpumask_next_zero(-1, cpu_possible_mask);
>>>> + } else {
>>>> + /*
>>>> + * First GICC entry must be BSP as ACPI spec said
>>>> + * in section 5.2.12.15
>>>> + */
>>>> + if (cpu_logical_map(0) != mpidr) {
>>>> + pr_err("First GICC entry with MPIDR 0x%llx is not BSP\n",
>>>> + mpidr);
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + /*
>>>> + * boot_cpu_init() already hold bit 0 in cpu_possible_mask
>>>> + * for BSP, no need to allocate again.
>>>> + */
>>>> + cpu = 0;
>>>> + }
>>>
>>> If/when kexec comes, on systems where CPU0 can be hotplugged the next
>>> kernel might boot on an AP rather than the BSP.
>>
>> so cpu_logical_map(0) will be the MPIDR of AP which boot the kernel,
>> then it will not equal to mpidr provided in the first entry of MADT,
>> right?
>
> Yes.
>
>> It seems that DT smp init will have the same problem, could you give me
>> some guidance how it solved?
>
> For DT we don't rely on the first entry we see in /cpus/ being CPU0 --
> we loop over all entries and expect one of them to be CPU0. I that what
> you're asking about, or have I misunderstood the question?
That's what I asked, thanks for the explain. I think I need to rework
this code a little bit and modify the logic as well.
>
>
>>> Is there a requirement
>>> Linux-side that CPU0 is the BSP, or is this just intended as a sanity
>>> check of the tables the FW provided?
>>
>> It is just the check of the table that the FW provided, so in this
>> kexec case, I think this code need to be reworked.
>>
>> On x86, no check for the first LAPIC entry must be BSP, I think we
>> need to remove the check for ARM64 too if it makes sense.
>
> Ok. It would be nice to know that there's no implicit assumption that
> ACPI makes about code executing on the BSP elsewhere; if so we may need
> to prevent CPU0 hotplug.
>
> On x86 CPU0 hotplug is typically inhibited for suspend/resume and
> PIC-specific issues, and it's not clear to me if there are other
> requirements for CPU0 to stay online.
>
> If the FW requires a particular CPU to stay online, then hopefully that
> will be reported through PSCI MIGRATE_INFO_UP_CPU, but we don't
> currently check that that in the PSCI code.
>
>>
>>>
>>>> +
>>>> + if (!acpi_psci_present())
>>>> + return -EOPNOTSUPP;
>>>> +
>>>> + cpu_ops[cpu] = cpu_get_ops("psci");
>>>> + /* CPU 0 was already initialized */
>>>> + if (cpu) {
>>>> + if (!cpu_ops[cpu])
>>>> + return -EINVAL;
>>>> +
>>>> + if (cpu_ops[cpu]->cpu_init(NULL, cpu))
>>>> + return -EOPNOTSUPP;
>>>> +
>>>> + /* map the logical cpu id to cpu MPIDR */
>>>> + cpu_logical_map(cpu) = mpidr;
>>>> +
>>>> + set_cpu_possible(cpu, true);
>>>> + }
>>>
>>> In the OF case we only set CPUs possible once we've scanned all the
>>> nodes, and only when the boot CPU was actually found in a table. We
>>> should keep the ACPI case consistent with that.
>>>
>>> Can we not handle all of this in a later call once we've scanned all of
>>> the GICC structures?
>>
>> we can. the code will be same as DT ones, when all the structures
>> are scanned, we can add the init code in acpi_init_cpus():
>>
>> for (i = 0; i < NR_CPUS; i++)
>> if (cpu_logical_map(i) != INVALID_HWID)
>> set_cpu_possible(i, true);
>>
>> but I think there is no difference for the logic, maybe I missed
>> something.
>
> With the ACPI code above, we mark each CPU possible as we scan it. In
> the DT case, if we fail to find the current CPU in the DTB, we don't
> mark any other nodes as possible. So in the DT case you don't get SMP
> if the current CPU is not in the table provided by FW, but in the ACPI
> case you would (when the CPU0 == BSP test is removed).
>
> I would prefer that we have a strong requirement that the current CPU is
> in the tables in the ACPI case. It safeguards against obviously wrong
> tables.
OK, make sense to me too, I will update the code.
Thanks
Hanjun
--
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/
Powered by blists - more mailing lists