| 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: <546DF9AB.3080300@linaro.org>
Date: Thu, 20 Nov 2014 14:24:43 +0000
From: Daniel Thompson <daniel.thompson@...aro.org>
To: Lucas Stach <l.stach@...gutronix.de>
CC: Shawn Guo <shawn.guo@...aro.org>,
Sascha Hauer <kernel@...gutronix.de>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
Russell King <linux@....linux.org.uk>,
Will Deacon <will.deacon@....com>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>,
Paul Mackerras <paulus@...ba.org>,
Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
patches@...aro.org, linaro-kernel@...ts.linaro.org,
John Stultz <john.stultz@...aro.org>,
Sumit Semwal <sumit.semwal@...aro.org>
Subject: Re: [RFC PATCH] arm: imx: Workaround i.MX6 PMU interrupts muxed to
one SPI
On 20/11/14 11:52, Lucas Stach wrote:
> Am Donnerstag, den 20.11.2014, 11:42 +0000 schrieb Daniel Thompson:
>> All PMU interrupts on multi-core i.MX6 devices are muxed onto a single SPI.
>> Should the PMU of any core except 0 (the default affinity for the
>> interrupt) trigger the interrupt then it cannot be serviced and,
>> eventually, the spurious irq detection will forcefully disable the
>> interrupt.
>>
>> This can be worked around by getting the interrupt handler to change its
>> own affinity if it cannot figure out why it has been triggered. This patch
>> adds logic to rotate the affinity to i.MX6.
>>
>> Signed-off-by: Daniel Thompson <daniel.thompson@...aro.org>
>
> I've sent almost the same patch a while ago. At this time it was shot
> down due to fears of the measurements being too flaky to be useful with
> all that IRQ dance. While I don't think this is true (I did some
> measurements on a SOLO and a QUAD variants of the i.MX6 with the same
> workload, that were only minimally apart), I believe the IRQ affinity
> dance isn't the best way to handle this.
Cumulative statistics and time based sampling profilers should be fine
either way since a delay before the interrupt the asserted on the
affected core should have a low impact here.
A use case where the measurement would be flaky might be a profile
trying to highlight which functions (or opcodes) of a process are most
likely to miss the cache. In such a case delay before the interrupt is
asserted would result in the cache miss being attributed to the wrong
opcode.
Note also that for a single threaded processes, or any other workload
where one core's PMU raises interrupts much more frequently than any
other, then the dance remains a good approach since it leaves the
affinity set correctly for next time.
> I had planned to look into smp_call_function() to distribute the IRQ to
> all CPUs at the same time, but have not got around to it yet. Maybe you
> could investigate whether this is a viable solution to the problem at
> hand?
I'm a little sceptical, not least because smp_call_function() and its
friends can't be called from interrupt.
This means the steps to propagate the interrupt become rather complex
and therefore on systems with a small(ish) numbers of cores it is not
obvious that the delay before the interrupt is asserted on the affected
core will improve very much.
Hopefully systems with a large number of cores won't exhibit this
problem (because whatever the workaround we adopt there would be
significant problems).
>
> Regards,
> Lucas
>> ---
>>
>> Notes:
>> This patch adopts the approach used on the u8500 (db8500_pmu_handler)
>> but the logic has been generalized for any number of CPUs, mostly
>> because the i.MX6 has a both dual and quad core variants.
>>
>> However it might be better to include the generalized logic in the main
>> armpmu code. I think the logic could be deployed automatically on SMP
>> systems with only a single not-percpu IRQ, replacing the
>> plat->handle_irq dance we currently do to hook up this code.
>>
>> Thoughts? (or is there already shared logic to do this that I
>> overlooked)
>>
>>
>> arch/arm/mach-imx/mach-imx6q.c | 39 ++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 38 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
>> index d51c6e99a2e9..c056b7b97eaa 100644
>> --- a/arch/arm/mach-imx/mach-imx6q.c
>> +++ b/arch/arm/mach-imx/mach-imx6q.c
>> @@ -16,6 +16,7 @@
>> #include <linux/delay.h>
>> #include <linux/export.h>
>> #include <linux/init.h>
>> +#include <linux/interrupt.h>
>> #include <linux/io.h>
>> #include <linux/irq.h>
>> #include <linux/irqchip.h>
>> @@ -33,6 +34,7 @@
>> #include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
>> #include <asm/mach/arch.h>
>> #include <asm/mach/map.h>
>> +#include <asm/pmu.h>
>> #include <asm/system_misc.h>
>>
>> #include "common.h"
>> @@ -261,6 +263,40 @@ static void __init imx6q_axi_init(void)
>> }
>> }
>>
>> +/*
>> + * The PMU IRQ lines of all cores are muxed onto a single interrupt.
>> + * Rotate the interrupt around the cores if the current CPU cannot
>> + * figure out why the interrupt has been triggered.
>> + */
>> +static irqreturn_t imx6q_pmu_handler(int irq, void *dev, irq_handler_t handler)
>> +{
>> + irqreturn_t ret = handler(irq, dev);
>> + int next;
>> +
>> + if (ret == IRQ_NONE && num_online_cpus() > 1) {
>> + next = cpumask_next(smp_processor_id(), cpu_online_mask);
>> + if (next > nr_cpu_ids)
>> + next = cpumask_next(-1, cpu_online_mask);
>> + irq_set_affinity(irq, cpumask_of(next));
>> + }
>> +
>> + /*
>> + * We should be able to get away with the amount of IRQ_NONEs we give,
>> + * while still having the spurious IRQ detection code kick in if the
>> + * interrupt really starts hitting spuriously.
>> + */
>> + return ret;
>> +}
>> +
>> +static struct arm_pmu_platdata imx6q_pmu_platdata = {
>> + .handle_irq = imx6q_pmu_handler,
>> +};
>> +
>> +static struct of_dev_auxdata imx6q_auxdata_lookup[] __initdata = {
>> + OF_DEV_AUXDATA("arm,cortex-a9-pmu", 0, "arm-pmu", &imx6q_pmu_platdata),
>> + {},
>> +};
>> +
>> static void __init imx6q_init_machine(void)
>> {
>> struct device *parent;
>> @@ -276,7 +312,8 @@ static void __init imx6q_init_machine(void)
>>
>> imx6q_enet_phy_init();
>>
>> - of_platform_populate(NULL, of_default_bus_match_table, NULL, parent);
>> + of_platform_populate(NULL, of_default_bus_match_table,
>> + imx6q_auxdata_lookup, parent);
>>
>> imx_anatop_init();
>> cpu_is_imx6q() ? imx6q_pm_init() : imx6dl_pm_init();
>> --
>> 1.9.3
>>
>
--
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