* [PATCH 03/16] ARM: b.L: introduce helpers for platform coherency exit/setup
From: Dave Martin @ 2013-01-14 17:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357777251-13541-4-git-send-email-nicolas.pitre@linaro.org>
On Wed, Jan 09, 2013 at 07:20:38PM -0500, Nicolas Pitre wrote:
> From: Dave Martin <dave.martin@linaro.org>
>
> This provides helper methods to coordinate between CPUs coming down
> and CPUs going up, as well as documentation on the used algorithms,
> so that cluster teardown and setup
> operations are not done for a cluster simultaneously.
[...]
In response to the incorrectness of the outer cache handling,
here's a supplementary patch:
>From b64f305c90e7ea585992df2d710f62ec6a7b5395 Mon Sep 17 00:00:00 2001
From: Dave Martin <dave.martin@linaro.org>
Date: Mon, 14 Jan 2013 16:25:47 +0000
Subject: [PATCH] ARM: b.L: Fix outer cache handling for coherency setup/exit helpers
This patch addresses the following issues:
* When invalidating stale data from the cache before a read,
outer caches must be invalidated _before_ inner caches, not
after, otherwise stale data may be re-filled from outer to
inner after the inner cache is flushed.
We still retain an inner clean before touching the outer cache,
to avoid stale data being rewritten from there into the outer
cache after the outer cache is flushed.
* All the sync_mem() calls synchronise either reads or writes,
but not both. This patch splits sync_mem() into separate
functions for reads and writes, to avoid excessive inner
flushes in the write case.
The two functions are different from the original sync_mem(),
to fix the above issues.
Signed-off-by: Dave Martin <dave.martin@linaro.org>
---
NOTE: This patch is build-tested only.
arch/arm/common/bL_entry.c | 57 ++++++++++++++++++++++++++++++++++----------
1 files changed, 44 insertions(+), 13 deletions(-)
diff --git a/arch/arm/common/bL_entry.c b/arch/arm/common/bL_entry.c
index 1ea4ec9..3e1a404 100644
--- a/arch/arm/common/bL_entry.c
+++ b/arch/arm/common/bL_entry.c
@@ -119,16 +119,47 @@ int bL_cpu_powered_up(void)
struct bL_sync_struct bL_sync;
-static void __sync_range(volatile void *p, size_t size)
+/*
+ * Ensure preceding writes to *p by this CPU are visible to
+ * subsequent reads by other CPUs:
+ */
+static void __sync_range_w(volatile void *p, size_t size)
{
char *_p = (char *)p;
__cpuc_flush_dcache_area(_p, size);
- outer_flush_range(__pa(_p), __pa(_p + size));
+ outer_clean_range(__pa(_p), __pa(_p + size));
outer_sync();
}
-#define sync_mem(ptr) __sync_range(ptr, sizeof *(ptr))
+/*
+ * Ensure preceding writes to *p by other CPUs are visible to
+ * subsequent reads by this CPU:
+ */
+static void __sync_range_r(volatile void *p, size_t size)
+{
+ char *_p = (char *)p;
+
+#ifdef CONFIG_OUTER_CACHE
+ if (outer_cache.flush_range) {
+ /*
+ * Ensure ditry data migrated from other CPUs into our cache
+ * are cleaned out safely before the outer cache is cleaned:
+ */
+ __cpuc_flush_dcache_area(_p, size);
+
+ /* Clean and invalidate stale data for *p from outer ... */
+ outer_flush_range(__pa(_p), __pa(_p + size));
+ outer_sync();
+ }
+#endif
+
+ /* ... and inner cache: */
+ __cpuc_flush_dcache_area(_p, size);
+}
+
+#define sync_w(ptr) __sync_range_w(ptr, sizeof *(ptr))
+#define sync_r(ptr) __sync_range_r(ptr, sizeof *(ptr))
/*
* __bL_cpu_going_down: Indicates that the cpu is being torn down.
@@ -138,7 +169,7 @@ static void __sync_range(volatile void *p, size_t size)
void __bL_cpu_going_down(unsigned int cpu, unsigned int cluster)
{
bL_sync.clusters[cluster].cpus[cpu].cpu = CPU_GOING_DOWN;
- sync_mem(&bL_sync.clusters[cluster].cpus[cpu].cpu);
+ sync_w(&bL_sync.clusters[cluster].cpus[cpu].cpu);
}
/*
@@ -151,7 +182,7 @@ void __bL_cpu_down(unsigned int cpu, unsigned int cluster)
{
dsb();
bL_sync.clusters[cluster].cpus[cpu].cpu = CPU_DOWN;
- sync_mem(&bL_sync.clusters[cluster].cpus[cpu].cpu);
+ sync_w(&bL_sync.clusters[cluster].cpus[cpu].cpu);
sev();
}
@@ -167,7 +198,7 @@ void __bL_outbound_leave_critical(unsigned int cluster, int state)
{
dsb();
bL_sync.clusters[cluster].cluster = state;
- sync_mem(&bL_sync.clusters[cluster].cluster);
+ sync_w(&bL_sync.clusters[cluster].cluster);
sev();
}
@@ -189,10 +220,10 @@ bool __bL_outbound_enter_critical(unsigned int cpu, unsigned int cluster)
/* Warn inbound CPUs that the cluster is being torn down: */
c->cluster = CLUSTER_GOING_DOWN;
- sync_mem(&c->cluster);
+ sync_w(&c->cluster);
/* Back out if the inbound cluster is already in the critical region: */
- sync_mem(&c->inbound);
+ sync_r(&c->inbound);
if (c->inbound == INBOUND_COMING_UP)
goto abort;
@@ -203,7 +234,7 @@ bool __bL_outbound_enter_critical(unsigned int cpu, unsigned int cluster)
* If any CPU has been woken up again from the DOWN state, then we
* shouldn't be taking the cluster down at all: abort in that case.
*/
- sync_mem(&c->cpus);
+ sync_r(&c->cpus);
for (i = 0; i < BL_CPUS_PER_CLUSTER; i++) {
int cpustate;
@@ -216,7 +247,7 @@ bool __bL_outbound_enter_critical(unsigned int cpu, unsigned int cluster)
break;
wfe();
- sync_mem(&c->cpus[i].cpu);
+ sync_r(&c->cpus[i].cpu);
}
switch (cpustate) {
@@ -239,7 +270,7 @@ abort:
int __bL_cluster_state(unsigned int cluster)
{
- sync_mem(&bL_sync.clusters[cluster].cluster);
+ sync_r(&bL_sync.clusters[cluster].cluster);
return bL_sync.clusters[cluster].cluster;
}
@@ -267,11 +298,11 @@ int __init bL_cluster_sync_init(void (*power_up_setup)(void))
for_each_online_cpu(i)
bL_sync.clusters[this_cluster].cpus[i].cpu = CPU_UP;
bL_sync.clusters[this_cluster].cluster = CLUSTER_UP;
- sync_mem(&bL_sync);
+ sync_w(&bL_sync);
if (power_up_setup) {
bL_power_up_setup_phys = virt_to_phys(power_up_setup);
- sync_mem(&bL_power_up_setup_phys);
+ sync_w(&bL_power_up_setup_phys);
}
return 0;
--
1.7.4.1
^ permalink raw reply related
* [PATCHv3 4/4] arm: Add generic timer broadcast support
From: Mark Rutland @ 2013-01-14 17:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1358183124-28461-1-git-send-email-mark.rutland@arm.com>
Implement timer_broadcast for the arm architecture, allowing for the use
of clock_event_device_drivers decoupled from the timer tick broadcast
mechanism.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Tested-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
arch/arm/Kconfig | 1 +
arch/arm/kernel/smp.c | 3 +--
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 67874b8..65ae737 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -4,6 +4,7 @@ config ARM
select ARCH_BINFMT_ELF_RANDOMIZE_PIE
select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
select ARCH_HAVE_CUSTOM_GPIO_H
+ select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
select ARCH_WANT_IPC_PARSE_VERSION
select BUILDTIME_EXTABLE_SORT if MMU
select CPU_PM if (SUSPEND || CPU_IDLE)
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 9308519..b7e3b50 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -476,7 +476,7 @@ u64 smp_irq_stat_cpu(unsigned int cpu)
static DEFINE_PER_CPU(struct clock_event_device, percpu_clockevent);
#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
-static void smp_timer_broadcast(const struct cpumask *mask)
+void tick_broadcast(const struct cpumask *mask)
{
smp_cross_call(mask, IPI_TIMER);
}
@@ -524,7 +524,6 @@ static void __cpuinit percpu_timer_setup(void)
struct clock_event_device *evt = &per_cpu(percpu_clockevent, cpu);
evt->cpumask = cpumask_of(cpu);
- evt->broadcast = smp_timer_broadcast;
if (!lt_ops || lt_ops->setup(evt))
broadcast_timer_setup(evt);
--
1.7.0.4
^ permalink raw reply related
* [PATCHv3 3/4] arm: Use generic timer broadcast receiver
From: Mark Rutland @ 2013-01-14 17:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1358183124-28461-1-git-send-email-mark.rutland@arm.com>
Currently, the ARM backend must maintain a redundant list of timers for
the purpose of centralising timer broadcast functionality. This prevents
sharing timer drivers across architectures.
This patch moves the pain of dealing with timer broadcasts to the core
clockevents tick broadcast code, which already maintains its own list
of timers.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Tested-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
arch/arm/kernel/smp.c | 10 +++-------
1 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 84f4cbf..9308519 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -475,12 +475,6 @@ u64 smp_irq_stat_cpu(unsigned int cpu)
*/
static DEFINE_PER_CPU(struct clock_event_device, percpu_clockevent);
-static void ipi_timer(void)
-{
- struct clock_event_device *evt = &__get_cpu_var(percpu_clockevent);
- evt->event_handler(evt);
-}
-
#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
static void smp_timer_broadcast(const struct cpumask *mask)
{
@@ -596,11 +590,13 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
case IPI_WAKEUP:
break;
+#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
case IPI_TIMER:
irq_enter();
- ipi_timer();
+ tick_receive_broadcast();
irq_exit();
break;
+#endif
case IPI_RESCHEDULE:
scheduler_ipi();
--
1.7.0.4
^ permalink raw reply related
* [PATCHv3 2/4] clockevents: Add generic timer broadcast function
From: Mark Rutland @ 2013-01-14 17:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1358183124-28461-1-git-send-email-mark.rutland@arm.com>
Currently, the timer broadcast mechanism is defined by a function
pointer on struct clock_event_device. As the fundamental mechanism for
broadcast is architecture-specific, this means that clock_event_device
drivers cannot be shared across multiple architectures.
This patch adds an (optional) architecture-specific function for timer
tick broadcast, allowing drivers which may require broadcast
functionality to be shared across multiple architectures.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Tested-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
include/linux/clockchips.h | 5 +++++
kernel/time/Kconfig | 4 ++++
kernel/time/tick-broadcast.c | 13 +++++++++++++
3 files changed, 22 insertions(+), 0 deletions(-)
diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h
index e1089aa..6634652 100644
--- a/include/linux/clockchips.h
+++ b/include/linux/clockchips.h
@@ -162,6 +162,11 @@ extern void clockevents_suspend(void);
extern void clockevents_resume(void);
#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
+#ifdef CONFIG_ARCH_HAS_TICK_BROADCAST
+extern void tick_broadcast(const struct cpumask *mask);
+#else
+#define tick_broadcast NULL
+#endif
extern int tick_receive_broadcast(void);
#endif
diff --git a/kernel/time/Kconfig b/kernel/time/Kconfig
index 8601f0d..b696922 100644
--- a/kernel/time/Kconfig
+++ b/kernel/time/Kconfig
@@ -38,6 +38,10 @@ config GENERIC_CLOCKEVENTS_BUILD
default y
depends on GENERIC_CLOCKEVENTS
+# Architecture can handle broadcast in a driver-agnostic way
+config ARCH_HAS_TICK_BROADCAST
+ bool
+
# Clockevents broadcasting infrastructure
config GENERIC_CLOCKEVENTS_BROADCAST
bool
diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index 1d6767d..9a70c6c 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -18,6 +18,7 @@
#include <linux/percpu.h>
#include <linux/profile.h>
#include <linux/sched.h>
+#include <linux/smp.h>
#include "tick-internal.h"
@@ -86,6 +87,11 @@ int tick_is_broadcast_device(struct clock_event_device *dev)
return (dev && tick_broadcast_device.evtdev == dev);
}
+static void err_broadcast(const struct cpumask *mask)
+{
+ pr_crit_once("Failed to broadcast timer tick. Some CPUs may be unresponsive.\n");
+}
+
/*
* Check, if the device is disfunctional and a place holder, which
* needs to be handled by the broadcast device.
@@ -105,6 +111,13 @@ int tick_device_uses_broadcast(struct clock_event_device *dev, int cpu)
*/
if (!tick_device_is_functional(dev)) {
dev->event_handler = tick_handle_periodic;
+ if (!dev->broadcast)
+ dev->broadcast = tick_broadcast;
+ if (!dev->broadcast) {
+ pr_warn_once("%s depends on broadcast, but no broadcast function available\n",
+ dev->name);
+ dev->broadcast = err_broadcast;
+ }
cpumask_set_cpu(cpu, tick_get_broadcast_mask());
tick_broadcast_start_periodic(tick_broadcast_device.evtdev);
ret = 1;
--
1.7.0.4
^ permalink raw reply related
* [PATCHv3 1/4] clockevents: Add generic timer broadcast receiver
From: Mark Rutland @ 2013-01-14 17:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1358183124-28461-1-git-send-email-mark.rutland@arm.com>
Currently the broadcast mechanism used for timers is abstracted by a
function pointer on struct clock_event_device. As the fundamental
mechanism for broadcast is architecture-specific, this ties each
clock_event_device driver to a single architecture, even where the
driver is otherwise generic.
This patch adds a standard path for the receipt of timer broadcasts, so
drivers and/or architecture backends need not manage redundant lists of
timers for the purpose of routing broadcast timer ticks.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Tested-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
include/linux/clockchips.h | 4 ++++
kernel/time/tick-broadcast.c | 15 +++++++++++++++
2 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h
index 8a7096f..e1089aa 100644
--- a/include/linux/clockchips.h
+++ b/include/linux/clockchips.h
@@ -161,6 +161,10 @@ clockevents_calc_mult_shift(struct clock_event_device *ce, u32 freq, u32 minsec)
extern void clockevents_suspend(void);
extern void clockevents_resume(void);
+#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
+extern int tick_receive_broadcast(void);
+#endif
+
#ifdef CONFIG_GENERIC_CLOCKEVENTS
extern void clockevents_notify(unsigned long reason, void *arg);
#else
diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index f113755..1d6767d 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -125,6 +125,21 @@ int tick_device_uses_broadcast(struct clock_event_device *dev, int cpu)
return ret;
}
+int tick_receive_broadcast(void)
+{
+ struct tick_device *td = this_cpu_ptr(&tick_cpu_device);
+ struct clock_event_device *evt = td->evtdev;
+
+ if (!evt)
+ return -ENODEV;
+
+ if (!evt->evt_handler)
+ return -EINVAL;
+
+ evt->event_handler(evt);
+ return 0;
+}
+
/*
* Broadcast the event to the cpus, which are set in the mask (mangled).
*/
--
1.7.0.4
^ permalink raw reply related
* [PATCHv3 0/4] clockevents: decouple broadcast mechanism from drivers
From: Mark Rutland @ 2013-01-14 17:05 UTC (permalink / raw)
To: linux-arm-kernel
This is an updated version of the series I posted earlier this month:
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/140528.html
The core patches can be found at:
git://linux-arm.org/linux-mr.git tags/timer-broadcast-v3-core
And the full series can be found at:
git://linux-arm.org/linux-mr.git tags/timer-broadcast-v3-arm
Changes since v2:
* Add evt->event_handler check in tick_receive_broadcast
* Remove tick_receive_broadcast stub for !GENERIC_CLOCKEVENTS_BROADCAST
* #ifdef IPI_TIMER handler for unexpected IPI warning
* Reorder patches (generic first, then arm implementation)
Changes since v1:
* Drop removal of guards in smp.c
* Removed useless evt->evt_handler check in tick_receive_broadcast
* Fix up tick_receive_broadcast when !GENERIC_CLOCKEVENTS_BROADCAST
* Fix checkpatch issues (multi-line strings)
Thanks to Stephen Boyd, Santosh Shilimkar, and Thomas Gleixner for their
commments.
In some SMP systems, cpu-local timers may stop delivering interrupts
when in low power states, or not all CPUs may have local timers. To
support these systems we have a mechanism for broadcasting timer ticks
to other CPUs. This mechanism relies on the struct
clock_event_device::broadcast function pointer, which is a
driver-specific mechanism for broadcasting ticks to other CPUs.
As the broadcast mechanism is architecture-specific, placing the
broadcast function on struct clock_event_device ties each driver to a
single architecture. Additionally the driver or architecture backend
must handle the routing of broadcast ticks to the correct
clock_event_device, leading to duplication of the list of active
clock_event_devices.
These patches introduce a generic mechanism for handling the receipt of
timer broadcasts, and an optional architecture-specific broadcast
function which allows drivers to be decoupled from a particular
architecture will retaining support for timer tick broadcasts. These
mechanisms are wired up for the arm port, and have been boot-tested on a
pandaboard.
Thanks,
Mark.
Mark Rutland (4):
clockevents: Add generic timer broadcast receiver
clockevents: Add generic timer broadcast function
arm: Use generic timer broadcast receiver
arm: Add generic timer broadcast support
arch/arm/Kconfig | 1 +
arch/arm/kernel/smp.c | 13 ++++---------
include/linux/clockchips.h | 9 +++++++++
kernel/time/Kconfig | 4 ++++
kernel/time/tick-broadcast.c | 28 ++++++++++++++++++++++++++++
5 files changed, 46 insertions(+), 9 deletions(-)
^ permalink raw reply
* [PATCH 08/16] ARM: bL_platsmp.c: make sure the GIC interface of a dying CPU is disabled
From: Will Deacon @ 2013-01-14 17:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <alpine.LFD.2.02.1301141154130.6300@xanadu.home>
On Mon, Jan 14, 2013 at 04:54:52PM +0000, Nicolas Pitre wrote:
> On Mon, 14 Jan 2013, Will Deacon wrote:
>
> > On Thu, Jan 10, 2013 at 12:20:43AM +0000, Nicolas Pitre wrote:
> > > Otherwise there might be some interrupts or IPIs becoming pending and the
> > > CPU will not enter low power mode when doing a WFI. The effect of this
> > > is a CPU that loops back into the kernel, go through the first man
> > > election, signals itself as alive, and prevent the cluster from being
> > > shut down.
> > >
> > > This could benefit from a better solution.
> > >
> > > Signed-off-by: Nicolas Pitre <nico@linaro.org>
> > > ---
> > > arch/arm/common/bL_platsmp.c | 1 +
> > > arch/arm/common/gic.c | 6 ++++++
> > > arch/arm/include/asm/hardware/gic.h | 2 ++
> > > 3 files changed, 9 insertions(+)
> > >
> > > diff --git a/arch/arm/common/bL_platsmp.c b/arch/arm/common/bL_platsmp.c
> > > index 0ae44123bf..6a3b251b97 100644
> > > --- a/arch/arm/common/bL_platsmp.c
> > > +++ b/arch/arm/common/bL_platsmp.c
> > > @@ -68,6 +68,7 @@ static void __ref bL_cpu_die(unsigned int cpu)
> > > pcpu = mpidr & 0xff;
> > > pcluster = (mpidr >> 8) & 0xff;
> > > bL_set_entry_vector(pcpu, pcluster, NULL);
> > > + gic_cpu_if_down();
> >
> > I'm starting to sound like a stuck record (and not a very tuneful one at
> > that) but... I think you need a barrier here.
>
> And I'm getting puzzled at the repetition. ;-)
Sorry! This case is more interesting though, because you also want to order
the cpu_if_down GIC write so that it completes before we do the power_off.
Will
^ permalink raw reply
* [PATCH 07/16] ARM: bL_platsmp.c: close the kernel entry gate before hot-unplugging a CPU
From: Will Deacon @ 2013-01-14 17:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <alpine.LFD.2.02.1301141151510.6300@xanadu.home>
On Mon, Jan 14, 2013 at 04:53:41PM +0000, Nicolas Pitre wrote:
> On Mon, 14 Jan 2013, Will Deacon wrote:
>
> > On Thu, Jan 10, 2013 at 12:20:42AM +0000, Nicolas Pitre wrote:
> > > If for whatever reason a CPU is unexpectedly awaken, it shouldn't
> > > re-enter the kernel by using whatever entry vector that might have
> > > been set by a previous operation.
> > >
> > > Signed-off-by: Nicolas Pitre <nico@linaro.org>
> > > ---
> > > arch/arm/common/bL_platsmp.c | 5 +++++
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/arch/arm/common/bL_platsmp.c b/arch/arm/common/bL_platsmp.c
> > > index 0acb9f4685..0ae44123bf 100644
> > > --- a/arch/arm/common/bL_platsmp.c
> > > +++ b/arch/arm/common/bL_platsmp.c
> > > @@ -63,6 +63,11 @@ static int bL_cpu_disable(unsigned int cpu)
> > >
> > > static void __ref bL_cpu_die(unsigned int cpu)
> > > {
> > > + unsigned int mpidr, pcpu, pcluster;
> > > + asm ("mrc p15, 0, %0, c0, c0, 5" : "=r" (mpidr));
> > > + pcpu = mpidr & 0xff;
> > > + pcluster = (mpidr >> 8) & 0xff;
> >
> > Usual comment about helper functions :)
> >
> > > + bL_set_entry_vector(pcpu, pcluster, NULL);
> >
> > Similar to the power_on story, you need a barrier here (unless you change
> > your platform_ops API to require barriers).
>
> The bL_set_entry_vector() includes a cache flush which itself has a DSB.
> Hence my previous interrogation.
For L1, sure, we always have the dsb for v7. However, for the outer-cache we
only have a dsb by virtue of a spin_unlock in l2x0.c... it seems a bit risky
to rely on that for ordering your entry_vector write with the power_on.
I think the best bet is to put a barrier in power_on, before invoking the
platform_ops function and similarly for power_off.
What do you reckon?
Will
^ permalink raw reply
* [PATCH] ARM: imx: disable cpu in .cpu_kill hook
From: Nicolas Pitre @ 2013-01-14 16:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130114164642.GI23505@n2100.arm.linux.org.uk>
On Mon, 14 Jan 2013, Russell King - ARM Linux wrote:
> On Mon, Jan 14, 2013 at 11:29:13AM -0500, Nicolas Pitre wrote:
> > On Mon, 14 Jan 2013, Rob Herring wrote:
> >
> > > Setting or clearing the SMP bit is showing up in multiple places
> > > (big.LITTLE series, Tegra cpuidle). I did inline versions of
> > > set_auxcr/get_auxcr for highbank cpuidle. We should be make those common.
> >
> > Could you do that like now and send it to RMK for his stable branch?
> > This way I could rebase the b.L series on top.
>
> Just be clear: I don't think this is something for the stable trees.
Right, not the stable tree but more precisely the devel-stable branch.
Nicolas
^ permalink raw reply
* [PATCH RFT 3/3] ARM: tegra: dts: seaboard: enable keyboard
From: Russell King - ARM Linux @ 2013-01-14 16:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50F43640.7060308@wwwdotorg.org>
On Mon, Jan 14, 2013 at 09:45:52AM -0700, Stephen Warren wrote:
> On 01/12/2013 03:34 AM, Laxman Dewangan wrote:
> ...
> > Which is the tree on which Prashant changes are applied. I can debug
> > from that tree.
>
> git://nv-tegra.nvidia.com/user/swarren/linux-2.6 test-ccf-rework-v4
>
> But as I said in some other email in response to Prashant, I'll go and
> find out why the clk driver is returning NULL.
Exactly - the rule is, the rest of the clk API should accept as valid
anything which clk_get() produces _for that implementation_ which
IS_ERR() is not equal to 1.
What that means is:
- if a platform's clk_get() never returns NULL, then the rest of the clk
API need not test for that value.
(If a NULL pointer is passed to one of the clk API functions, crashing
_is_ _okay_. Consider what memset(NULL, 0, 4096) would do from the
kernel - it doesn't test for a NULL pointer just because it can - it
crashes so you can debug why it was passed a NULL pointer.)
- if a platform's clk_get() does return NULL, then the rest of the clk
API is expected to deal with a NULL pointer in a way that does _not_
result in the system crashing.
Or, to put it another way:
clk = clk_get(...);
if (IS_ERR(clk))
return PTR_ERR(clk);
clk_foo(clk);
for all possible return values of clk_get() that an implementation
_actually_ _intentionally_ returns[*] should never cause an oops.
[*] - bugs excluded. No bugs were squashed in the creation of this email.
^ permalink raw reply
* [PATCH 08/16] ARM: bL_platsmp.c: make sure the GIC interface of a dying CPU is disabled
From: Nicolas Pitre @ 2013-01-14 16:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130114163902.GC31341@mudshark.cambridge.arm.com>
On Mon, 14 Jan 2013, Will Deacon wrote:
> On Thu, Jan 10, 2013 at 12:20:43AM +0000, Nicolas Pitre wrote:
> > Otherwise there might be some interrupts or IPIs becoming pending and the
> > CPU will not enter low power mode when doing a WFI. The effect of this
> > is a CPU that loops back into the kernel, go through the first man
> > election, signals itself as alive, and prevent the cluster from being
> > shut down.
> >
> > This could benefit from a better solution.
> >
> > Signed-off-by: Nicolas Pitre <nico@linaro.org>
> > ---
> > arch/arm/common/bL_platsmp.c | 1 +
> > arch/arm/common/gic.c | 6 ++++++
> > arch/arm/include/asm/hardware/gic.h | 2 ++
> > 3 files changed, 9 insertions(+)
> >
> > diff --git a/arch/arm/common/bL_platsmp.c b/arch/arm/common/bL_platsmp.c
> > index 0ae44123bf..6a3b251b97 100644
> > --- a/arch/arm/common/bL_platsmp.c
> > +++ b/arch/arm/common/bL_platsmp.c
> > @@ -68,6 +68,7 @@ static void __ref bL_cpu_die(unsigned int cpu)
> > pcpu = mpidr & 0xff;
> > pcluster = (mpidr >> 8) & 0xff;
> > bL_set_entry_vector(pcpu, pcluster, NULL);
> > + gic_cpu_if_down();
>
> I'm starting to sound like a stuck record (and not a very tuneful one at
> that) but... I think you need a barrier here.
And I'm getting puzzled at the repetition. ;-)
Nicolas
^ permalink raw reply
* [PATCH 07/16] ARM: bL_platsmp.c: close the kernel entry gate before hot-unplugging a CPU
From: Nicolas Pitre @ 2013-01-14 16:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130114163755.GB31341@mudshark.cambridge.arm.com>
On Mon, 14 Jan 2013, Will Deacon wrote:
> On Thu, Jan 10, 2013 at 12:20:42AM +0000, Nicolas Pitre wrote:
> > If for whatever reason a CPU is unexpectedly awaken, it shouldn't
> > re-enter the kernel by using whatever entry vector that might have
> > been set by a previous operation.
> >
> > Signed-off-by: Nicolas Pitre <nico@linaro.org>
> > ---
> > arch/arm/common/bL_platsmp.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/arch/arm/common/bL_platsmp.c b/arch/arm/common/bL_platsmp.c
> > index 0acb9f4685..0ae44123bf 100644
> > --- a/arch/arm/common/bL_platsmp.c
> > +++ b/arch/arm/common/bL_platsmp.c
> > @@ -63,6 +63,11 @@ static int bL_cpu_disable(unsigned int cpu)
> >
> > static void __ref bL_cpu_die(unsigned int cpu)
> > {
> > + unsigned int mpidr, pcpu, pcluster;
> > + asm ("mrc p15, 0, %0, c0, c0, 5" : "=r" (mpidr));
> > + pcpu = mpidr & 0xff;
> > + pcluster = (mpidr >> 8) & 0xff;
>
> Usual comment about helper functions :)
>
> > + bL_set_entry_vector(pcpu, pcluster, NULL);
>
> Similar to the power_on story, you need a barrier here (unless you change
> your platform_ops API to require barriers).
The bL_set_entry_vector() includes a cache flush which itself has a DSB.
Hence my previous interrogation.
Nicolas
^ permalink raw reply
* [PATCH 06/16] ARM: b.L: generic SMP secondary bringup and hotplug support
From: Nicolas Pitre @ 2013-01-14 16:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130114163530.GA31341@mudshark.cambridge.arm.com>
On Mon, 14 Jan 2013, Will Deacon wrote:
> On Thu, Jan 10, 2013 at 12:20:41AM +0000, Nicolas Pitre wrote:
> > Now that the b.L power API is in place, we can use it for SMP secondary
> > bringup and CPU hotplug in a generic fashion.
>
> [...]
>
> > diff --git a/arch/arm/common/bL_platsmp.c b/arch/arm/common/bL_platsmp.c
> > new file mode 100644
> > index 0000000000..0acb9f4685
> > --- /dev/null
> > +++ b/arch/arm/common/bL_platsmp.c
> > @@ -0,0 +1,79 @@
> > +/*
> > + * linux/arch/arm/mach-vexpress/bL_platsmp.c
> > + *
> > + * Created by: Nicolas Pitre, November 2012
> > + * Copyright: (C) 2012 Linaro Limited
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * Code to handle secondary CPU bringup and hotplug for the bL power API.
> > + */
> > +
> > +#include <linux/init.h>
> > +#include <linux/smp.h>
> > +
> > +#include <asm/bL_entry.h>
> > +#include <asm/smp_plat.h>
> > +#include <asm/hardware/gic.h>
> > +
> > +static void __init simple_smp_init_cpus(void)
> > +{
> > + set_smp_cross_call(gic_raise_softirq);
> > +}
> > +
> > +static int __cpuinit bL_boot_secondary(unsigned int cpu, struct task_struct *idle)
> > +{
> > + unsigned int pcpu, pcluster, ret;
> > + extern void secondary_startup(void);
> > +
> > + pcpu = cpu_logical_map(cpu) & 0xff;
> > + pcluster = (cpu_logical_map(cpu) >> 8) & 0xff;
>
> Again, you can probably use Lorenzo's helpers here.
Yes, that goes for the whole series.
> > + pr_debug("%s: logical CPU %d is physical CPU %d cluster %d\n",
> > + __func__, cpu, pcpu, pcluster);
> > +
> > + bL_set_entry_vector(pcpu, pcluster, NULL);
>
> Now that you don't have a barrier in this function, you need one here.
Hmmm... Why?
> > + ret = bL_cpu_power_up(pcpu, pcluster);
> > + if (ret)
> > + return ret;
>
> and here, although I confess to not understanding why you write NULL the
> first time.
If for some reasons the bL_cpu_power_up() call fails, I don't want this
CPU to suddenly decide to enter the kernel if it wakes up at a later
time when secondary_startup is not ready to deal with it anymore.
> > + bL_set_entry_vector(pcpu, pcluster, secondary_startup);
> > + gic_raise_softirq(cpumask_of(cpu), 0);
> > + sev();
>
> This relise on the event register being able to be set if the target is in a
> low-power (wfi) state. I'd feel safer with a dsb before the sev...
Sure.
Nicolas
^ permalink raw reply
* [PATCH] ARM: imx: disable cpu in .cpu_kill hook
From: Russell King - ARM Linux @ 2013-01-14 16:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <alpine.LFD.2.02.1301141127370.6300@xanadu.home>
On Mon, Jan 14, 2013 at 11:29:13AM -0500, Nicolas Pitre wrote:
> On Mon, 14 Jan 2013, Rob Herring wrote:
>
> > Setting or clearing the SMP bit is showing up in multiple places
> > (big.LITTLE series, Tegra cpuidle). I did inline versions of
> > set_auxcr/get_auxcr for highbank cpuidle. We should be make those common.
>
> Could you do that like now and send it to RMK for his stable branch?
> This way I could rebase the b.L series on top.
Just be clear: I don't think this is something for the stable trees.
^ permalink raw reply
* [PATCH RFT 3/3] ARM: tegra: dts: seaboard: enable keyboard
From: Stephen Warren @ 2013-01-14 16:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50F13C34.3020808@nvidia.com>
On 01/12/2013 03:34 AM, Laxman Dewangan wrote:
...
> Which is the tree on which Prashant changes are applied. I can debug
> from that tree.
git://nv-tegra.nvidia.com/user/swarren/linux-2.6 test-ccf-rework-v4
But as I said in some other email in response to Prashant, I'll go and
find out why the clk driver is returning NULL.
(Russell, while the clk API definition may allow NULL to be a legal clk
value, the actual implementation on Tegra at least doesn't consider NULL
to be a legal clk value, so it shouldn't be returned in the first
place). There's no issue here in the comprehension of the clk API.
^ permalink raw reply
* [PATCH v5 13/14] KVM: ARM: Handle I/O aborts
From: Russell King - ARM Linux @ 2013-01-14 16:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130108184005.46302.38495.stgit@ubuntu>
On Tue, Jan 08, 2013 at 01:40:05PM -0500, Christoffer Dall wrote:
> diff --git a/arch/arm/kvm/decode.c b/arch/arm/kvm/decode.c
> new file mode 100644
> index 0000000..469cf14
> --- /dev/null
> +++ b/arch/arm/kvm/decode.c
> @@ -0,0 +1,462 @@
> +/*
> + * Copyright (C) 2012 - Virtual Open Systems and Columbia University
> + * Author: Christoffer Dall <c.dall@virtualopensystems.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License, version 2, as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
> + */
> +#include <linux/kvm_host.h>
> +#include <asm/kvm_mmio.h>
> +#include <asm/kvm_emulate.h>
> +#include <asm/kvm_decode.h>
> +#include <trace/events/kvm.h>
> +
> +#include "trace.h"
> +
> +struct arm_instr {
> + /* Instruction decoding */
> + u32 opc;
> + u32 opc_mask;
> +
> + /* Decoding for the register write back */
> + bool register_form;
> + u32 imm;
> + u8 Rm;
> + u8 type;
> + u8 shift_n;
> +
> + /* Common decoding */
> + u8 len;
> + bool sign_extend;
> + bool w;
> +
> + bool (*decode)(struct kvm_decode *decode, struct kvm_exit_mmio *mmio,
> + unsigned long instr, struct arm_instr *ai);
> +};
> +
> +enum SRType {
> + SRType_LSL,
> + SRType_LSR,
> + SRType_ASR,
> + SRType_ROR,
> + SRType_RRX
> +};
> +
> +/* Modelled after DecodeImmShift() in the ARM ARM */
> +static enum SRType decode_imm_shift(u8 type, u8 imm5, u8 *amount)
> +{
> + switch (type) {
> + case 0x0:
> + *amount = imm5;
> + return SRType_LSL;
> + case 0x1:
> + *amount = (imm5 == 0) ? 32 : imm5;
> + return SRType_LSR;
> + case 0x2:
> + *amount = (imm5 == 0) ? 32 : imm5;
> + return SRType_ASR;
> + case 0x3:
> + if (imm5 == 0) {
> + *amount = 1;
> + return SRType_RRX;
> + } else {
> + *amount = imm5;
> + return SRType_ROR;
> + }
> + }
> +
> + return SRType_LSL;
> +}
> +
> +/* Modelled after Shift() in the ARM ARM */
> +static u32 shift(u32 value, u8 N, enum SRType type, u8 amount, bool carry_in)
> +{
> + u32 mask = (1 << N) - 1;
> + s32 svalue = (s32)value;
> +
> + BUG_ON(N > 32);
> + BUG_ON(type == SRType_RRX && amount != 1);
> + BUG_ON(amount > N);
> +
> + if (amount == 0)
> + return value;
> +
> + switch (type) {
> + case SRType_LSL:
> + value <<= amount;
> + break;
> + case SRType_LSR:
> + value >>= amount;
> + break;
> + case SRType_ASR:
> + if (value & (1 << (N - 1)))
> + svalue |= ((-1UL) << N);
> + value = svalue >> amount;
> + break;
> + case SRType_ROR:
> + value = (value >> amount) | (value << (N - amount));
> + break;
> + case SRType_RRX: {
> + u32 C = (carry_in) ? 1 : 0;
> + value = (value >> 1) | (C << (N - 1));
> + break;
> + }
> + }
> +
> + return value & mask;
> +}
> +
> +static bool decode_arm_wb(struct kvm_decode *decode, struct kvm_exit_mmio *mmio,
> + unsigned long instr, const struct arm_instr *ai)
> +{
> + u8 Rt = (instr >> 12) & 0xf;
> + u8 Rn = (instr >> 16) & 0xf;
> + u8 W = (instr >> 21) & 1;
> + u8 U = (instr >> 23) & 1;
> + u8 P = (instr >> 24) & 1;
> + u32 base_addr = *kvm_decode_reg(decode, Rn);
> + u32 offset_addr, offset;
> +
> + /*
> + * Technically this is allowed in certain circumstances,
> + * but we don't support it.
> + */
> + if (Rt == 15 || Rn == 15)
> + return false;
> +
> + if (P && !W) {
> + kvm_err("Decoding operation with valid ISV?\n");
> + return false;
> + }
> +
> + decode->rt = Rt;
> +
> + if (ai->register_form) {
> + /* Register operation */
> + enum SRType s_type;
> + u8 shift_n = 0;
> + bool c_bit = *kvm_decode_cpsr(decode) & PSR_C_BIT;
> + u32 s_reg = *kvm_decode_reg(decode, ai->Rm);
> +
> + s_type = decode_imm_shift(ai->type, ai->shift_n, &shift_n);
> + offset = shift(s_reg, 5, s_type, shift_n, c_bit);
> + } else {
> + /* Immediate operation */
> + offset = ai->imm;
> + }
> +
> + /* Handle Writeback */
> + if (U)
> + offset_addr = base_addr + offset;
> + else
> + offset_addr = base_addr - offset;
> + *kvm_decode_reg(decode, Rn) = offset_addr;
> + return true;
> +}
> +
> +static bool decode_arm_ls(struct kvm_decode *decode, struct kvm_exit_mmio *mmio,
> + unsigned long instr, struct arm_instr *ai)
> +{
> + u8 A = (instr >> 25) & 1;
> +
> + mmio->is_write = ai->w;
> + mmio->len = ai->len;
> + decode->sign_extend = false;
> +
> + ai->register_form = A;
> + ai->imm = instr & 0xfff;
> + ai->Rm = instr & 0xf;
> + ai->type = (instr >> 5) & 0x3;
> + ai->shift_n = (instr >> 7) & 0x1f;
> +
> + return decode_arm_wb(decode, mmio, instr, ai);
> +}
> +
> +static bool decode_arm_extra(struct kvm_decode *decode,
> + struct kvm_exit_mmio *mmio,
> + unsigned long instr, struct arm_instr *ai)
> +{
> + mmio->is_write = ai->w;
> + mmio->len = ai->len;
> + decode->sign_extend = ai->sign_extend;
> +
> + ai->register_form = !((instr >> 22) & 1);
> + ai->imm = ((instr >> 4) & 0xf0) | (instr & 0xf);
> + ai->Rm = instr & 0xf;
> + ai->type = 0; /* SRType_LSL */
> + ai->shift_n = 0;
> +
> + return decode_arm_wb(decode, mmio, instr, ai);
> +}
> +
> +/*
> + * The encodings in this table assumes that a fault was generated where the
> + * ISV field in the HSR was clear, and the decoding information was invalid,
> + * which means that a register write-back occurred, the PC was used as the
> + * destination or a load/store multiple operation was used. Since the latter
> + * two cases are crazy for MMIO on the guest side, we simply inject a fault
> + * when this happens and support the common case.
> + *
> + * We treat unpriviledged loads and stores of words and bytes like all other
> + * loads and stores as their encodings mandate the W bit set and the P bit
> + * clear.
> + */
> +static const struct arm_instr arm_instr[] = {
> + /**************** Load/Store Word and Byte **********************/
> + /* Store word with writeback */
> + { .opc = 0x04000000, .opc_mask = 0x0c500000, .len = 4, .w = true,
> + .sign_extend = false, .decode = decode_arm_ls },
> + /* Store byte with writeback */
> + { .opc = 0x04400000, .opc_mask = 0x0c500000, .len = 1, .w = true,
> + .sign_extend = false, .decode = decode_arm_ls },
> + /* Load word with writeback */
> + { .opc = 0x04100000, .opc_mask = 0x0c500000, .len = 4, .w = false,
> + .sign_extend = false, .decode = decode_arm_ls },
> + /* Load byte with writeback */
> + { .opc = 0x04500000, .opc_mask = 0x0c500000, .len = 1, .w = false,
> + .sign_extend = false, .decode = decode_arm_ls },
> +
> + /*************** Extra load/store instructions ******************/
> +
> + /* Store halfword with writeback */
> + { .opc = 0x000000b0, .opc_mask = 0x0c1000f0, .len = 2, .w = true,
> + .sign_extend = false, .decode = decode_arm_extra },
> + /* Load halfword with writeback */
> + { .opc = 0x001000b0, .opc_mask = 0x0c1000f0, .len = 2, .w = false,
> + .sign_extend = false, .decode = decode_arm_extra },
> +
> + /* Load dual with writeback */
> + { .opc = 0x000000d0, .opc_mask = 0x0c1000f0, .len = 8, .w = false,
> + .sign_extend = false, .decode = decode_arm_extra },
> + /* Load signed byte with writeback */
> + { .opc = 0x001000d0, .opc_mask = 0x0c1000f0, .len = 1, .w = false,
> + .sign_extend = true, .decode = decode_arm_extra },
> +
> + /* Store dual with writeback */
> + { .opc = 0x000000f0, .opc_mask = 0x0c1000f0, .len = 8, .w = true,
> + .sign_extend = false, .decode = decode_arm_extra },
> + /* Load signed halfword with writeback */
> + { .opc = 0x001000f0, .opc_mask = 0x0c1000f0, .len = 2, .w = false,
> + .sign_extend = true, .decode = decode_arm_extra },
> +
> + /* Store halfword unprivileged */
> + { .opc = 0x002000b0, .opc_mask = 0x0f3000f0, .len = 2, .w = true,
> + .sign_extend = false, .decode = decode_arm_extra },
> + /* Load halfword unprivileged */
> + { .opc = 0x003000b0, .opc_mask = 0x0f3000f0, .len = 2, .w = false,
> + .sign_extend = false, .decode = decode_arm_extra },
> + /* Load signed byte unprivileged */
> + { .opc = 0x003000d0, .opc_mask = 0x0f3000f0, .len = 1, .w = false,
> + .sign_extend = true , .decode = decode_arm_extra },
> + /* Load signed halfword unprivileged */
> + { .opc = 0x003000d0, .opc_mask = 0x0f3000f0, .len = 2, .w = false,
> + .sign_extend = true , .decode = decode_arm_extra },
So here, yet again, we end up with more code decoding the ARM load/store
instructions so that we can do something with them. How many places do
we now have in the ARM kernel doing this exact same thing? Do we really
need to keep rewriting this functionality each time a feature that needs
it gets implemented, or is _someone_ going to sort this out once and for
all?
^ permalink raw reply
* [PATCH 08/16] ARM: bL_platsmp.c: make sure the GIC interface of a dying CPU is disabled
From: Will Deacon @ 2013-01-14 16:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357777251-13541-9-git-send-email-nicolas.pitre@linaro.org>
On Thu, Jan 10, 2013 at 12:20:43AM +0000, Nicolas Pitre wrote:
> Otherwise there might be some interrupts or IPIs becoming pending and the
> CPU will not enter low power mode when doing a WFI. The effect of this
> is a CPU that loops back into the kernel, go through the first man
> election, signals itself as alive, and prevent the cluster from being
> shut down.
>
> This could benefit from a better solution.
>
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
> ---
> arch/arm/common/bL_platsmp.c | 1 +
> arch/arm/common/gic.c | 6 ++++++
> arch/arm/include/asm/hardware/gic.h | 2 ++
> 3 files changed, 9 insertions(+)
>
> diff --git a/arch/arm/common/bL_platsmp.c b/arch/arm/common/bL_platsmp.c
> index 0ae44123bf..6a3b251b97 100644
> --- a/arch/arm/common/bL_platsmp.c
> +++ b/arch/arm/common/bL_platsmp.c
> @@ -68,6 +68,7 @@ static void __ref bL_cpu_die(unsigned int cpu)
> pcpu = mpidr & 0xff;
> pcluster = (mpidr >> 8) & 0xff;
> bL_set_entry_vector(pcpu, pcluster, NULL);
> + gic_cpu_if_down();
I'm starting to sound like a stuck record (and not a very tuneful one at
that) but... I think you need a barrier here.
> bL_cpu_power_down();
Will
^ permalink raw reply
* [PATCH 07/16] ARM: bL_platsmp.c: close the kernel entry gate before hot-unplugging a CPU
From: Will Deacon @ 2013-01-14 16:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357777251-13541-8-git-send-email-nicolas.pitre@linaro.org>
On Thu, Jan 10, 2013 at 12:20:42AM +0000, Nicolas Pitre wrote:
> If for whatever reason a CPU is unexpectedly awaken, it shouldn't
> re-enter the kernel by using whatever entry vector that might have
> been set by a previous operation.
>
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
> ---
> arch/arm/common/bL_platsmp.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm/common/bL_platsmp.c b/arch/arm/common/bL_platsmp.c
> index 0acb9f4685..0ae44123bf 100644
> --- a/arch/arm/common/bL_platsmp.c
> +++ b/arch/arm/common/bL_platsmp.c
> @@ -63,6 +63,11 @@ static int bL_cpu_disable(unsigned int cpu)
>
> static void __ref bL_cpu_die(unsigned int cpu)
> {
> + unsigned int mpidr, pcpu, pcluster;
> + asm ("mrc p15, 0, %0, c0, c0, 5" : "=r" (mpidr));
> + pcpu = mpidr & 0xff;
> + pcluster = (mpidr >> 8) & 0xff;
Usual comment about helper functions :)
> + bL_set_entry_vector(pcpu, pcluster, NULL);
Similar to the power_on story, you need a barrier here (unless you change
your platform_ops API to require barriers).
> bL_cpu_power_down();
Will
^ permalink raw reply
* [PATCH v5 08/14] KVM: ARM: Emulation framework and CP15 emulation
From: Russell King - ARM Linux @ 2013-01-14 16:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130108183931.46302.60921.stgit@ubuntu>
On Tue, Jan 08, 2013 at 01:39:31PM -0500, Christoffer Dall wrote:
> + /*
> + * Check whether this vcpu requires the cache to be flushed on
> + * this physical CPU. This is a consequence of doing dcache
> + * operations by set/way on this vcpu. We do it here to be in
> + * a non-preemptible section.
> + */
> + if (cpumask_test_cpu(cpu, &vcpu->arch.require_dcache_flush)) {
> + cpumask_clear_cpu(cpu, &vcpu->arch.require_dcache_flush);
There is cpumask_test_and_clear_cpu() which may be better for this.
^ permalink raw reply
* [PATCH 06/16] ARM: b.L: generic SMP secondary bringup and hotplug support
From: Will Deacon @ 2013-01-14 16:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357777251-13541-7-git-send-email-nicolas.pitre@linaro.org>
On Thu, Jan 10, 2013 at 12:20:41AM +0000, Nicolas Pitre wrote:
> Now that the b.L power API is in place, we can use it for SMP secondary
> bringup and CPU hotplug in a generic fashion.
[...]
> diff --git a/arch/arm/common/bL_platsmp.c b/arch/arm/common/bL_platsmp.c
> new file mode 100644
> index 0000000000..0acb9f4685
> --- /dev/null
> +++ b/arch/arm/common/bL_platsmp.c
> @@ -0,0 +1,79 @@
> +/*
> + * linux/arch/arm/mach-vexpress/bL_platsmp.c
> + *
> + * Created by: Nicolas Pitre, November 2012
> + * Copyright: (C) 2012 Linaro Limited
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * Code to handle secondary CPU bringup and hotplug for the bL power API.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/smp.h>
> +
> +#include <asm/bL_entry.h>
> +#include <asm/smp_plat.h>
> +#include <asm/hardware/gic.h>
> +
> +static void __init simple_smp_init_cpus(void)
> +{
> + set_smp_cross_call(gic_raise_softirq);
> +}
> +
> +static int __cpuinit bL_boot_secondary(unsigned int cpu, struct task_struct *idle)
> +{
> + unsigned int pcpu, pcluster, ret;
> + extern void secondary_startup(void);
> +
> + pcpu = cpu_logical_map(cpu) & 0xff;
> + pcluster = (cpu_logical_map(cpu) >> 8) & 0xff;
Again, you can probably use Lorenzo's helpers here.
> + pr_debug("%s: logical CPU %d is physical CPU %d cluster %d\n",
> + __func__, cpu, pcpu, pcluster);
> +
> + bL_set_entry_vector(pcpu, pcluster, NULL);
Now that you don't have a barrier in this function, you need one here.
> + ret = bL_cpu_power_up(pcpu, pcluster);
> + if (ret)
> + return ret;
and here, although I confess to not understanding why you write NULL the
first time.
> + bL_set_entry_vector(pcpu, pcluster, secondary_startup);
> + gic_raise_softirq(cpumask_of(cpu), 0);
> + sev();
This relise on the event register being able to be set if the target is in a
low-power (wfi) state. I'd feel safer with a dsb before the sev...
Will
^ permalink raw reply
* [PATCH v5 04/14] KVM: ARM: Hypervisor initialization
From: Christoffer Dall @ 2013-01-14 16:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130114151106.GB18935@mudshark.cambridge.arm.com>
On Mon, Jan 14, 2013 at 10:11 AM, Will Deacon <will.deacon@arm.com> wrote:
> On Tue, Jan 08, 2013 at 06:39:03PM +0000, Christoffer Dall wrote:
>> Sets up KVM code to handle all exceptions taken to Hyp mode.
>>
>> When the kernel is booted in Hyp mode, calling an hvc instruction with r0
>> pointing to the new vectors, the HVBAR is changed to the the vector pointers.
>> This allows subsystems (like KVM here) to execute code in Hyp-mode with the
>> MMU disabled.
>>
>> We initialize other Hyp-mode registers and enables the MMU for Hyp-mode from
>> the id-mapped hyp initialization code. Afterwards, the HVBAR is changed to
>> point to KVM Hyp vectors used to catch guest faults and to switch to Hyp mode
>> to perform a world-switch into a KVM guest.
>>
>> Also provides memory mapping code to map required code pages, data structures,
>> and I/O regions accessed in Hyp mode at the same virtual address as the host
>> kernel virtual addresses, but which conforms to the architectural requirements
>> for translations in Hyp mode. This interface is added in arch/arm/kvm/arm_mmu.c
>> and comprises:
>> - create_hyp_mappings(from, to);
>> - create_hyp_io_mappings(from, to, phys_addr);
>> - free_hyp_pmds();
>
> [...]
>
>> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
>> index 82cb338..2dddc58 100644
>> --- a/arch/arm/kvm/arm.c
>> +++ b/arch/arm/kvm/arm.c
>> @@ -34,11 +34,21 @@
>> #include <asm/ptrace.h>
>> #include <asm/mman.h>
>> #include <asm/cputype.h>
>> +#include <asm/tlbflush.h>
>> +#include <asm/virt.h>
>> +#include <asm/kvm_arm.h>
>> +#include <asm/kvm_asm.h>
>> +#include <asm/kvm_mmu.h>
>>
>> #ifdef REQUIRES_VIRT
>> __asm__(".arch_extension virt");
>> #endif
>>
>> +static DEFINE_PER_CPU(unsigned long, kvm_arm_hyp_stack_page);
>> +static struct vfp_hard_struct __percpu *kvm_host_vfp_state;
>> +static unsigned long hyp_default_vectors;
>> +
>> +
>> int kvm_arch_hardware_enable(void *garbage)
>> {
>> return 0;
>> @@ -336,9 +346,176 @@ long kvm_arch_vm_ioctl(struct file *filp,
>> return -EINVAL;
>> }
>>
>> +static void cpu_init_hyp_mode(void *vector)
>> +{
>> + unsigned long long pgd_ptr;
>> + unsigned long hyp_stack_ptr;
>> + unsigned long stack_page;
>> + unsigned long vector_ptr;
>> +
>> + /* Switch from the HYP stub to our own HYP init vector */
>> + __hyp_set_vectors((unsigned long)vector);
>> +
>> + pgd_ptr = (unsigned long long)kvm_mmu_get_httbr();
>> + stack_page = __get_cpu_var(kvm_arm_hyp_stack_page);
>> + hyp_stack_ptr = stack_page + PAGE_SIZE;
>> + vector_ptr = (unsigned long)__kvm_hyp_vector;
>> +
>> + /*
>> + * Call initialization code, and switch to the full blown
>> + * HYP code. The init code corrupts r12, so set the clobber
>> + * list accordingly.
>> + */
>> + asm volatile (
>> + "mov r0, %[pgd_ptr_low]\n\t"
>> + "mov r1, %[pgd_ptr_high]\n\t"
>> + "mov r2, %[hyp_stack_ptr]\n\t"
>> + "mov r3, %[vector_ptr]\n\t"
>> + "hvc #0\n\t" : :
>> + [pgd_ptr_low] "r" ((unsigned long)(pgd_ptr & 0xffffffff)),
>> + [pgd_ptr_high] "r" ((unsigned long)(pgd_ptr >> 32ULL)),
>> + [hyp_stack_ptr] "r" (hyp_stack_ptr),
>> + [vector_ptr] "r" (vector_ptr) :
>> + "r0", "r1", "r2", "r3", "r12");
>> +}
>
> Use kvm_call_hyp here instead.
>
good idea:
commit 00e22196205800ce9caa561e7c806023f4915138
Author: Christoffer Dall <c.dall@virtualopensystems.com>
Date: Mon Jan 14 11:32:36 2013 -0500
KVM: ARM: Reuse kvm_call_hyp in vcpu_init_hyp_mode
Instead of directly and manually callin the hypercall into the KVM init
code, use the kvm_call_hyp function, which only requires a small abuse
of the prototype in exchange for much nicer C code.
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Christoffer Dall <c.dall@virtualopensystems.com>
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 6997326..b5c6ab1 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -971,6 +971,7 @@ long kvm_arch_vm_ioctl(struct file *filp,
static void cpu_init_hyp_mode(void *vector)
{
unsigned long long pgd_ptr;
+ unsigned long pgd_low, pgd_high;
unsigned long hyp_stack_ptr;
unsigned long stack_page;
unsigned long vector_ptr;
@@ -979,26 +980,20 @@ static void cpu_init_hyp_mode(void *vector)
__hyp_set_vectors((unsigned long)vector);
pgd_ptr = (unsigned long long)kvm_mmu_get_httbr();
+ pgd_low = (pgd_ptr & ((1ULL << 32) - 1));
+ pgd_high = (pgd_ptr >> 32ULL);
stack_page = __get_cpu_var(kvm_arm_hyp_stack_page);
hyp_stack_ptr = stack_page + PAGE_SIZE;
vector_ptr = (unsigned long)__kvm_hyp_vector;
/*
* Call initialization code, and switch to the full blown
- * HYP code. The init code corrupts r12, so set the clobber
- * list accordingly.
+ * HYP code. The init code doesn't need to preserve these registers as
+ * r1-r3 and r12 are already callee save according to the AAPCS.
+ * Note that we slightly misuse the prototype by casing the pgd_low to
+ * a void *.
*/
- asm volatile (
- "mov r0, %[pgd_ptr_low]\n\t"
- "mov r1, %[pgd_ptr_high]\n\t"
- "mov r2, %[hyp_stack_ptr]\n\t"
- "mov r3, %[vector_ptr]\n\t"
- "hvc #0\n\t" : :
- [pgd_ptr_low] "r" ((unsigned long)(pgd_ptr & 0xffffffff)),
- [pgd_ptr_high] "r" ((unsigned long)(pgd_ptr >> 32ULL)),
- [hyp_stack_ptr] "r" (hyp_stack_ptr),
- [vector_ptr] "r" (vector_ptr) :
- "r0", "r1", "r2", "r3", "r12");
+ kvm_call_hyp((void *)pgd_low, pgd_high, hyp_stack_ptr, vector_ptr);
}
/**
--
Thanks,
-Christoffer
^ permalink raw reply related
* [PATCH] timer: vt8500: Move timer code to drivers/clocksource
From: Stephen Warren @ 2013-01-14 16:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1358140171-25390-2-git-send-email-linux@prisktech.co.nz>
On 01/13/2013 10:09 PM, Tony Prisk wrote:
> This patch moves arch-vt8500/timer.c into drivers/clocksource and
> updates the necessary Kconfig/Makefile options.
> diff --git a/include/linux/vt8500_timer.h b/include/linux/vt8500_timer.h
> +#ifndef __VT8500_TIMER_H
> +#define __VT8500_TIMER_H
> +
> +#include <asm/mach/time.h>
> +
> +void vt8500_timer_init(void);
> +
> +#endif
Is VT8500 DT-only? If so, it'd be nice not to add this header, but
instead use CLOCKSOURCE_OF_DECLARE() inside the driver C file.
^ permalink raw reply
* [PATCH] ARM: imx: disable cpu in .cpu_kill hook
From: Nicolas Pitre @ 2013-01-14 16:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50F40E58.4030808@gmail.com>
On Mon, 14 Jan 2013, Rob Herring wrote:
> Setting or clearing the SMP bit is showing up in multiple places
> (big.LITTLE series, Tegra cpuidle). I did inline versions of
> set_auxcr/get_auxcr for highbank cpuidle. We should be make those common.
Could you do that like now and send it to RMK for his stable branch?
This way I could rebase the b.L series on top.
Nicolas
^ permalink raw reply
* [PATCH v5 03/14] KVM: ARM: Initial skeleton to compile KVM support
From: Russell King - ARM Linux @ 2013-01-14 16:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130108183855.46302.40539.stgit@ubuntu>
On Tue, Jan 08, 2013 at 01:38:55PM -0500, Christoffer Dall wrote:
> + /* -ENOENT for unknown features, -EINVAL for invalid combinations. */
> + for (i = 0; i < sizeof(init->features)*8; i++) {
> + if (init->features[i / 32] & (1 << (i % 32))) {
Isn't this an open-coded version of test_bit() ?
^ permalink raw reply
* [PATCH v5 02/14] ARM: Section based HYP idmap
From: Russell King - ARM Linux @ 2013-01-14 16:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130108183848.46302.77369.stgit@ubuntu>
On Tue, Jan 08, 2013 at 01:38:48PM -0500, Christoffer Dall wrote:
> + pr_info("Setting up static %sidentity map for 0x%llx - 0x%llx\n",
> + prot ? "HYP " : "",
> + (long long)addr, (long long)end);
There's no point using 0x%llx and casting to 64-bit longs if the arguments
are always going to be 32-bit.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox