linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/6] MIPS: Remote processor driver
@ 2016-09-20  8:47 Matt Redfearn
  2016-09-20  8:47 ` [PATCH v2 1/6] irqchip: mips-gic: Add context saving for MIPS_REMOTEPROC Matt Redfearn
                   ` (5 more replies)
  0 siblings, 6 replies; 17+ messages in thread
From: Matt Redfearn @ 2016-09-20  8:47 UTC (permalink / raw)
  To: Ralf Baechle, Bjorn Andersson, Ohad Ben-Cohen, Thomas Gleixner
  Cc: linux-mips, linux-remoteproc, lisa.parratt, linux-kernel,
	Matt Redfearn, Hugh Dickins, Qais Yousef, Masahiro Yamada,
	Huacai Chen, Jason Cooper, Lisa Parratt, James Hogan,
	Andrew Morton, Qais Yousef, Marc Zyngier, David S. Miller,
	Paul Burton


The MIPS remote processor driver allows non-Linux firmware to take
control of and execute on one of the systems VPEs. The CPU must be
offlined from Linux first. A sysfs interface is created which allows
firmware to be loaded and changed at runtime. A full description is
available at [1]. An example firmware that can be used with this driver
is available at [2].

This is useful to allow running bare metal code, or an RTOS, on one or
more CPUs while allowing Linux to continue running on those remaining.

The remote processor framework allows for firmwares to provide any
virtio device for communication between the firmware running on the
remote VP and Linux. For example [1] demonstrates a simple firmware
which provides a virtual serial port. Any string sent to the port is
case inverted and returned.

This is conceptually similar to the VPE loader functionality, but is
more standard as it fits into the remoteproc subsystem.

The first patches in this series lay the groundwork for the driver
before it is added. The last series deprecates the VPE loader.

This functionality is supported on:
- MIPS32r2 devices implementing the MIPS MT ASE for multithreading, such
  as interAptiv.
- MIPS32r6 devices implementing VPs, such as I6400.

Limitations:
- The remoteproc core supports only 32bit ELFs. Therefore it is only
  possible to run 32bit firmware on the remote processor. Also, for
  virtio communication, pointers are passed from the kernel to firmware.
  There can be no mismatch in pointer sizes between the kernel and
  firmware, so this limits the host kernel to 32bit as well.

The functionality has been tested on the Ci40 board which has a 2 core 2
thread interAptiv.

This series is based on v4.8-rc6

Depends on James Hogan's ebase series:
MIPS: traps: 64bit kernels should read CP0_EBase 64bit
MIPS: traps: Convert ebase to KSeg0
MIPS: traps: Ensure full EBase is written

Without these patches, if firmware modifies ebase to allow handling
exceptions / interrupts, then when the VPE is returned to Linux the
kernel exception handlers won't be reinstated properly.

[1] http://wiki.prplfoundation.org/w/images/d/df/MIPS_OS_Remote_Processor_Driver_Whitepaper_1.0.9.pdf
[2] https://github.com/MIPS/mips-rproc-example


Changes in v2:
Add dependence on additional patches to mips-gic in commit log
Incorporate changes from Marc Zynger's review:
- Remove CONTEXT_SAVING define.
- Make saved local state a per-cpu variable
- Make gic_save_* static functions when enabled, and do { } while(0)
  otherwise

Lisa Parratt (1):
  MIPS: CPS: Add VP(E) stealing

Matt Redfearn (5):
  irqchip: mips-gic: Add context saving for MIPS_REMOTEPROC
  MIPS: tlb-r4k: If there are wired entries, don't use TLBINVF
  MIPS: smp.c: Introduce mechanism for freeing and allocating IPIs
  remoteproc/MIPS: Add a remoteproc driver for MIPS
  MIPS: Deprecate VPE Loader

 Documentation/ABI/testing/sysfs-class-mips-rproc |  24 +
 arch/mips/Kconfig                                |  12 +-
 arch/mips/include/asm/smp-cps.h                  |   8 +
 arch/mips/include/asm/smp.h                      |  15 +
 arch/mips/kernel/smp-cps.c                       | 162 +++++-
 arch/mips/kernel/smp.c                           |  73 ++-
 arch/mips/mm/tlb-r4k.c                           |   7 +-
 drivers/irqchip/irq-mips-gic.c                   | 217 +++++++-
 drivers/remoteproc/Kconfig                       |  11 +
 drivers/remoteproc/Makefile                      |   1 +
 drivers/remoteproc/mips_remoteproc.c             | 651 +++++++++++++++++++++++
 11 files changed, 1156 insertions(+), 25 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-mips-rproc
 create mode 100644 drivers/remoteproc/mips_remoteproc.c

-- 
2.7.4

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2 1/6] irqchip: mips-gic: Add context saving for MIPS_REMOTEPROC
  2016-09-20  8:47 [PATCH v2 0/6] MIPS: Remote processor driver Matt Redfearn
@ 2016-09-20  8:47 ` Matt Redfearn
  2016-09-20  9:40   ` Thomas Gleixner
  2016-09-20  8:47 ` [PATCH v2 2/6] MIPS: tlb-r4k: If there are wired entries, don't use TLBINVF Matt Redfearn
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 17+ messages in thread
From: Matt Redfearn @ 2016-09-20  8:47 UTC (permalink / raw)
  To: Ralf Baechle, Bjorn Andersson, Ohad Ben-Cohen, Thomas Gleixner
  Cc: linux-mips, linux-remoteproc, lisa.parratt, linux-kernel,
	Matt Redfearn, Marc Zyngier, Jason Cooper

The MIPS remote processor driver allows non-Linux firmware to take
control of and execute on one of the systems VPEs. If that VPE is
brought back under Linux, it is necessary to ensure that all GIC
interrupts are routed and masked as Linux expects them, as the firmware
can have done anything it likes with the GIC configuration (hopefully
just for that VPEs local interrupt sources, but allow for shared
external interrupts as well).

The configuration of shared and local CPU interrupts is maintained and
updated every time a change is made. When a CPU is brought online, the
saved configuration is restored.

These functions will also be useful for restoring GIC context after a
suspend to RAM.

This patch depends on Paul Burton's recent patches:
irqchip: mips-gic: Implement activate op for device domain
irqchip: mips-gic: Cleanup chip & handler setup

Without these patches, the context saving will restore an incorrect map
to VPE value, since there is a route to not having the interrupt
affinity set.

Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com>
---

Changes in v2:
Add dependence on additional patches to mips-gic in commit log
Incorporate changes from Marc Zynger's review:
- Remove CONTEXT_SAVING define.
- Make saved local state a per-cpu variable
- Make gic_save_* static functions when enabled, and do { } while(0)
  otherwise

 drivers/irqchip/irq-mips-gic.c | 217 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 210 insertions(+), 7 deletions(-)

diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c
index 83f498393a7f..4aafe09849c1 100644
--- a/drivers/irqchip/irq-mips-gic.c
+++ b/drivers/irqchip/irq-mips-gic.c
@@ -8,6 +8,7 @@
  */
 #include <linux/bitmap.h>
 #include <linux/clocksource.h>
+#include <linux/cpu.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
@@ -56,6 +57,79 @@ static unsigned int timer_cpu_pin;
 static struct irq_chip gic_level_irq_controller, gic_edge_irq_controller;
 DECLARE_BITMAP(ipi_resrv, GIC_MAX_INTRS);
 
+#ifdef CONFIG_MIPS_REMOTEPROC
+struct gic_local_state_t {
+	u8 mask;
+};
+
+DEFINE_PER_CPU(struct gic_local_state_t, gic_local_state);
+
+static void gic_save_local_rmask(int vpe, int mask)
+{
+	struct gic_local_state_t *state = per_cpu_ptr(&gic_local_state, vpe);
+
+	state->mask &= mask;
+}
+
+static void gic_save_local_smask(int vpe, int mask)
+{
+	struct gic_local_state_t *state = per_cpu_ptr(&gic_local_state, vpe);
+
+	state->mask |= mask;
+}
+
+static struct {
+	unsigned vpe:		8;
+	unsigned pin:		4;
+
+	unsigned polarity:	1;
+	unsigned trigger:	1;
+	unsigned dual_edge:	1;
+	unsigned mask:		1;
+} gic_shared_state[GIC_MAX_INTRS];
+
+static void gic_save_shared_vpe(int intr, int vpe)
+{
+	gic_shared_state[intr].vpe = vpe;
+}
+
+static void gic_save_shared_pin(int intr, int pin)
+{
+	gic_shared_state[intr].pin = pin;
+}
+
+static void gic_save_shared_polarity(int intr, int polarity)
+{
+	gic_shared_state[intr].polarity = polarity;
+}
+
+static void gic_save_shared_trigger(int intr, int trigger)
+{
+	gic_shared_state[intr].trigger = trigger;
+}
+
+static void gic_save_shared_dual_edge(int intr, int dual_edge)
+{
+	gic_shared_state[intr].dual_edge = dual_edge;
+}
+
+static void gic_save_shared_mask(int intr, int mask)
+{
+	gic_shared_state[intr].mask = mask;
+}
+
+#else
+#define gic_save_local_rmask(vpe, i)	do { } while (0)
+#define gic_save_local_smask(vpe, i)	do { } while (0)
+
+#define gic_save_shared_vpe(i, v)	do { } while (0)
+#define gic_save_shared_pin(i, p)	do { } while (0)
+#define gic_save_shared_polarity(i, p)	do { } while (0)
+#define gic_save_shared_trigger(i, t)	do { } while (0)
+#define gic_save_shared_dual_edge(i, d)	do { } while (0)
+#define gic_save_shared_mask(i, m)	do { } while (0)
+#endif /* CONFIG_MIPS_REMOTEPROC */
+
 static void __gic_irq_dispatch(void);
 
 static inline u32 gic_read32(unsigned int reg)
@@ -105,52 +179,94 @@ static inline void gic_update_bits(unsigned int reg, unsigned long mask,
 	gic_write(reg, regval);
 }
 
-static inline void gic_reset_mask(unsigned int intr)
+static inline void gic_write_reset_mask(unsigned int intr)
 {
 	gic_write(GIC_REG(SHARED, GIC_SH_RMASK) + GIC_INTR_OFS(intr),
 		  1ul << GIC_INTR_BIT(intr));
 }
 
-static inline void gic_set_mask(unsigned int intr)
+static inline void gic_reset_mask(unsigned int intr)
+{
+	gic_save_shared_mask(intr, 0);
+	gic_write_reset_mask(intr);
+}
+
+static inline void gic_write_set_mask(unsigned int intr)
 {
 	gic_write(GIC_REG(SHARED, GIC_SH_SMASK) + GIC_INTR_OFS(intr),
 		  1ul << GIC_INTR_BIT(intr));
 }
 
-static inline void gic_set_polarity(unsigned int intr, unsigned int pol)
+static inline void gic_set_mask(unsigned int intr)
+{
+	gic_save_shared_mask(intr, 1);
+	gic_write_set_mask(intr);
+}
+
+static inline void gic_write_polarity(unsigned int intr, unsigned int pol)
 {
 	gic_update_bits(GIC_REG(SHARED, GIC_SH_SET_POLARITY) +
 			GIC_INTR_OFS(intr), 1ul << GIC_INTR_BIT(intr),
 			(unsigned long)pol << GIC_INTR_BIT(intr));
 }
 
-static inline void gic_set_trigger(unsigned int intr, unsigned int trig)
+static inline void gic_set_polarity(unsigned int intr, unsigned int pol)
+{
+	gic_save_shared_polarity(intr, pol);
+	gic_write_polarity(intr, pol);
+}
+
+static inline void gic_write_trigger(unsigned int intr, unsigned int trig)
 {
 	gic_update_bits(GIC_REG(SHARED, GIC_SH_SET_TRIGGER) +
 			GIC_INTR_OFS(intr), 1ul << GIC_INTR_BIT(intr),
 			(unsigned long)trig << GIC_INTR_BIT(intr));
 }
 
-static inline void gic_set_dual_edge(unsigned int intr, unsigned int dual)
+static inline void gic_set_trigger(unsigned int intr, unsigned int trig)
+{
+	gic_save_shared_trigger(intr, trig);
+	gic_write_trigger(intr, trig);
+}
+
+static inline void gic_write_dual_edge(unsigned int intr, unsigned int dual)
 {
 	gic_update_bits(GIC_REG(SHARED, GIC_SH_SET_DUAL) + GIC_INTR_OFS(intr),
 			1ul << GIC_INTR_BIT(intr),
 			(unsigned long)dual << GIC_INTR_BIT(intr));
 }
 
-static inline void gic_map_to_pin(unsigned int intr, unsigned int pin)
+static inline void gic_set_dual_edge(unsigned int intr, unsigned int dual)
+{
+	gic_save_shared_dual_edge(intr, dual);
+	gic_write_dual_edge(intr, dual);
+}
+
+static inline void gic_write_map_to_pin(unsigned int intr, unsigned int pin)
 {
 	gic_write32(GIC_REG(SHARED, GIC_SH_INTR_MAP_TO_PIN_BASE) +
 		    GIC_SH_MAP_TO_PIN(intr), GIC_MAP_TO_PIN_MSK | pin);
 }
 
-static inline void gic_map_to_vpe(unsigned int intr, unsigned int vpe)
+static inline void gic_map_to_pin(unsigned int intr, unsigned int pin)
+{
+	gic_save_shared_pin(intr, pin);
+	gic_write_map_to_pin(intr, pin);
+}
+
+static inline void gic_write_map_to_vpe(unsigned int intr, unsigned int vpe)
 {
 	gic_write(GIC_REG(SHARED, GIC_SH_INTR_MAP_TO_VPE_BASE) +
 		  GIC_SH_MAP_TO_VPE_REG_OFF(intr, vpe),
 		  GIC_SH_MAP_TO_VPE_REG_BIT(vpe));
 }
 
+static inline void gic_map_to_vpe(unsigned int intr, unsigned int vpe)
+{
+	gic_save_shared_vpe(intr, vpe);
+	gic_write_map_to_vpe(intr, vpe);
+}
+
 #ifdef CONFIG_CLKSRC_MIPS_GIC
 cycle_t gic_read_count(void)
 {
@@ -537,6 +653,7 @@ static void gic_mask_local_irq(struct irq_data *d)
 {
 	int intr = GIC_HWIRQ_TO_LOCAL(d->hwirq);
 
+	gic_save_local_rmask(smp_processor_id(), (1 << intr));
 	gic_write32(GIC_REG(VPE_LOCAL, GIC_VPE_RMASK), 1 << intr);
 }
 
@@ -544,6 +661,7 @@ static void gic_unmask_local_irq(struct irq_data *d)
 {
 	int intr = GIC_HWIRQ_TO_LOCAL(d->hwirq);
 
+	gic_save_local_smask(smp_processor_id(), (1 << intr));
 	gic_write32(GIC_REG(VPE_LOCAL, GIC_VPE_SMASK), 1 << intr);
 }
 
@@ -561,6 +679,7 @@ static void gic_mask_local_irq_all_vpes(struct irq_data *d)
 
 	spin_lock_irqsave(&gic_lock, flags);
 	for (i = 0; i < gic_vpes; i++) {
+		gic_save_local_rmask(mips_cm_vp_id(i), 1 << intr);
 		gic_write(GIC_REG(VPE_LOCAL, GIC_VPE_OTHER_ADDR),
 			  mips_cm_vp_id(i));
 		gic_write32(GIC_REG(VPE_OTHER, GIC_VPE_RMASK), 1 << intr);
@@ -576,6 +695,7 @@ static void gic_unmask_local_irq_all_vpes(struct irq_data *d)
 
 	spin_lock_irqsave(&gic_lock, flags);
 	for (i = 0; i < gic_vpes; i++) {
+		gic_save_local_smask(mips_cm_vp_id(i), 1 << intr);
 		gic_write(GIC_REG(VPE_LOCAL, GIC_VPE_OTHER_ADDR),
 			  mips_cm_vp_id(i));
 		gic_write32(GIC_REG(VPE_OTHER, GIC_VPE_SMASK), 1 << intr);
@@ -983,6 +1103,85 @@ static struct irq_domain_ops gic_ipi_domain_ops = {
 	.match = gic_ipi_domain_match,
 };
 
+#ifdef CONFIG_MIPS_REMOTEPROC
+static void gic_restore_shared(void)
+{
+	unsigned long flags;
+	int i;
+
+	spin_lock_irqsave(&gic_lock, flags);
+	for (i = 0; i < gic_shared_intrs; i++) {
+		gic_write_polarity(i, gic_shared_state[i].polarity);
+		gic_write_trigger(i, gic_shared_state[i].trigger);
+		gic_write_dual_edge(i, gic_shared_state[i].dual_edge);
+		gic_write_map_to_vpe(i, gic_shared_state[i].vpe);
+		gic_write_map_to_pin(i, gic_shared_state[i].pin);
+
+		if (gic_shared_state[i].mask)
+			gic_write_set_mask(i);
+		else
+			gic_write_reset_mask(i);
+	}
+	spin_unlock_irqrestore(&gic_lock, flags);
+}
+
+static void gic_restore_local(unsigned int vpe)
+{
+	struct gic_local_state_t state;
+	int hw, virq, intr, mask;
+	unsigned long flags;
+
+	for (hw = 0; hw < GIC_NUM_LOCAL_INTRS; hw++) {
+		intr = GIC_LOCAL_TO_HWIRQ(hw);
+		virq = irq_linear_revmap(gic_irq_domain, intr);
+		gic_local_irq_domain_map(gic_irq_domain, virq, hw);
+	}
+
+	local_irq_save(flags);
+	gic_write(GIC_REG(VPE_LOCAL, GIC_VPE_OTHER_ADDR), vpe);
+
+	/* Enable EIC mode if necessary */
+	gic_write32(GIC_REG(VPE_OTHER, GIC_VPE_CTL), cpu_has_veic);
+
+	/* Restore interrupt masks */
+	state = per_cpu(gic_local_state, vpe);
+	mask = state.mask;
+	gic_write32(GIC_REG(VPE_OTHER, GIC_VPE_RMASK), ~mask);
+	gic_write32(GIC_REG(VPE_OTHER, GIC_VPE_SMASK), mask);
+
+	local_irq_restore(flags);
+}
+
+/*
+ * The MIPS remote processor driver allows non-Linux firmware to take control
+ * of and execute on one of the systems VPEs. If that VPE is brought back under
+ * Linux, it is necessary to ensure that all GIC interrupts are routed and
+ * masked as Linux expects them, as the firmware can have done anything it
+ * likes with the GIC configuration (hopefully just for that VPEs local
+ * interrupt sources, but allow for shared external interrupts as well).
+ */
+static int gic_cpu_notify(struct notifier_block *nfb, unsigned long action,
+			       void *hcpu)
+{
+	unsigned int cpu = mips_cm_vp_id((unsigned long)hcpu);
+
+	switch (action) {
+	case CPU_UP_PREPARE:
+	case CPU_DOWN_FAILED:
+		gic_restore_shared();
+		gic_restore_local(cpu);
+	default:
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block gic_cpu_notifier __refdata = {
+	.notifier_call = gic_cpu_notify
+};
+#endif /* CONFIG_MIPS_REMOTEPROC */
+
 static void __init __gic_init(unsigned long gic_base_addr,
 			      unsigned long gic_addrspace_size,
 			      unsigned int cpu_vec, unsigned int irqbase,
@@ -1082,6 +1281,10 @@ static void __init __gic_init(unsigned long gic_base_addr,
 	}
 
 	gic_basic_init();
+
+#ifdef CONFIG_MIPS_REMOTEPROC
+	register_hotcpu_notifier(&gic_cpu_notifier);
+#endif /* CONFIG_MIPS_REMOTEPROC */
 }
 
 void __init gic_init(unsigned long gic_base_addr,
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 2/6] MIPS: tlb-r4k: If there are wired entries, don't use TLBINVF
  2016-09-20  8:47 [PATCH v2 0/6] MIPS: Remote processor driver Matt Redfearn
  2016-09-20  8:47 ` [PATCH v2 1/6] irqchip: mips-gic: Add context saving for MIPS_REMOTEPROC Matt Redfearn
@ 2016-09-20  8:47 ` Matt Redfearn
  2016-09-22 12:11   ` Ralf Baechle
  2016-09-20  8:47 ` [PATCH v2 3/6] MIPS: smp.c: Introduce mechanism for freeing and allocating IPIs Matt Redfearn
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 17+ messages in thread
From: Matt Redfearn @ 2016-09-20  8:47 UTC (permalink / raw)
  To: Ralf Baechle, Bjorn Andersson, Ohad Ben-Cohen, Thomas Gleixner
  Cc: linux-mips, linux-remoteproc, lisa.parratt, linux-kernel,
	Matt Redfearn, Hugh Dickins, Huacai Chen, David S. Miller,
	James Hogan, Paul Burton, Andrew Morton

When adding a wired entry to the TLB via add_wired_entry, the tlb is
flushed with local_flush_tlb_all, which on CPUs with TLBINV results in
the new wired entry being flushed again.

Behavior of the TLBINV instruction applies to all applicable TLB entries
and is unaffected by the setting of the Wired register. Therefore if
the TLB has any wired entries, fall back to iterating over the entries
rather than blasting them all using TLBINVF.

Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com>
---

Changes in v2: None

 arch/mips/mm/tlb-r4k.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/mips/mm/tlb-r4k.c b/arch/mips/mm/tlb-r4k.c
index e8b335c16295..4953c1a8cdfd 100644
--- a/arch/mips/mm/tlb-r4k.c
+++ b/arch/mips/mm/tlb-r4k.c
@@ -67,8 +67,11 @@ void local_flush_tlb_all(void)
 
 	entry = read_c0_wired();
 
-	/* Blast 'em all away. */
-	if (cpu_has_tlbinv) {
+	/*
+	 * Blast 'em all away.
+	 * If there are any wired entries, fall back to iterating
+	 */
+	if (cpu_has_tlbinv && !entry) {
 		if (current_cpu_data.tlbsizevtlb) {
 			write_c0_index(0);
 			mtc0_tlbw_hazard();
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 3/6] MIPS: smp.c: Introduce mechanism for freeing and allocating IPIs
  2016-09-20  8:47 [PATCH v2 0/6] MIPS: Remote processor driver Matt Redfearn
  2016-09-20  8:47 ` [PATCH v2 1/6] irqchip: mips-gic: Add context saving for MIPS_REMOTEPROC Matt Redfearn
  2016-09-20  8:47 ` [PATCH v2 2/6] MIPS: tlb-r4k: If there are wired entries, don't use TLBINVF Matt Redfearn
@ 2016-09-20  8:47 ` Matt Redfearn
  2016-09-20  8:47 ` [PATCH v2 4/6] MIPS: CPS: Add VP(E) stealing Matt Redfearn
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 17+ messages in thread
From: Matt Redfearn @ 2016-09-20  8:47 UTC (permalink / raw)
  To: Ralf Baechle, Bjorn Andersson, Ohad Ben-Cohen, Thomas Gleixner
  Cc: linux-mips, linux-remoteproc, lisa.parratt, linux-kernel,
	Matt Redfearn, Lisa Parratt, James Hogan, Qais Yousef,
	Paul Burton

For the MIPS remote processor implementation, we need additional IPIs to
talk to the remote processor. Since MIPS GIC reserves exactly the right
number of IPI IRQs required by Linux for the number of VPs in the
system, this is not possible without releasing some recources.

This commit introduces mips_smp_ipi_allocate() which allocates IPIs to a
given cpumask. It is called as normal with the cpu_possible_mask at
bootup to initialise IPIs to all CPUs. mips_smp_ipi_free() may then be
used to free IPIs to a subset of those CPUs so that their hardware
resources can be reused.

Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com>
---

Changes in v2: None

 arch/mips/include/asm/smp.h | 14 +++++++++++
 arch/mips/kernel/smp.c      | 61 +++++++++++++++++++++++++++++++++++++++------
 2 files changed, 67 insertions(+), 8 deletions(-)

diff --git a/arch/mips/include/asm/smp.h b/arch/mips/include/asm/smp.h
index 8bc6c70a4030..060f23ff1817 100644
--- a/arch/mips/include/asm/smp.h
+++ b/arch/mips/include/asm/smp.h
@@ -85,6 +85,20 @@ static inline void __cpu_die(unsigned int cpu)
 extern void play_dead(void);
 #endif
 
+/*
+ * This function will set up the necessary IPIs for Linux to communicate
+ * with the CPUs in mask.
+ * Return 0 on success.
+ */
+int mips_smp_ipi_allocate(const struct cpumask *mask);
+
+/*
+ * This function will free up IPIs allocated with mips_smp_ipi_allocate to the
+ * CPUs in mask, which must be a subset of the IPIs that have been configured.
+ * Return 0 on success.
+ */
+int mips_smp_ipi_free(const struct cpumask *mask);
+
 static inline void arch_send_call_function_single_ipi(int cpu)
 {
 	extern struct plat_smp_ops *mp_ops;	/* private */
diff --git a/arch/mips/kernel/smp.c b/arch/mips/kernel/smp.c
index f95f094f36e4..afa06c2bb019 100644
--- a/arch/mips/kernel/smp.c
+++ b/arch/mips/kernel/smp.c
@@ -229,7 +229,7 @@ static struct irqaction irq_call = {
 	.name		= "IPI call"
 };
 
-static __init void smp_ipi_init_one(unsigned int virq,
+static void smp_ipi_init_one(unsigned int virq,
 				    struct irqaction *action)
 {
 	int ret;
@@ -239,9 +239,11 @@ static __init void smp_ipi_init_one(unsigned int virq,
 	BUG_ON(ret);
 }
 
-static int __init mips_smp_ipi_init(void)
+static unsigned int call_virq, sched_virq;
+
+int mips_smp_ipi_allocate(const struct cpumask *mask)
 {
-	unsigned int call_virq, sched_virq;
+	int virq;
 	struct irq_domain *ipidomain;
 	struct device_node *node;
 
@@ -268,16 +270,20 @@ static int __init mips_smp_ipi_init(void)
 	if (!ipidomain)
 		return 0;
 
-	call_virq = irq_reserve_ipi(ipidomain, cpu_possible_mask);
-	BUG_ON(!call_virq);
+	virq = irq_reserve_ipi(ipidomain, mask);
+	BUG_ON(!virq);
+	if (!call_virq)
+		call_virq = virq;
 
-	sched_virq = irq_reserve_ipi(ipidomain, cpu_possible_mask);
-	BUG_ON(!sched_virq);
+	virq = irq_reserve_ipi(ipidomain, mask);
+	BUG_ON(!virq);
+	if (!sched_virq)
+		sched_virq = virq;
 
 	if (irq_domain_is_ipi_per_cpu(ipidomain)) {
 		int cpu;
 
-		for_each_cpu(cpu, cpu_possible_mask) {
+		for_each_cpu(cpu, mask) {
 			smp_ipi_init_one(call_virq + cpu, &irq_call);
 			smp_ipi_init_one(sched_virq + cpu, &irq_resched);
 		}
@@ -286,6 +292,45 @@ static int __init mips_smp_ipi_init(void)
 		smp_ipi_init_one(sched_virq, &irq_resched);
 	}
 
+	return 0;
+}
+
+int mips_smp_ipi_free(const struct cpumask *mask)
+{
+	struct irq_domain *ipidomain;
+	struct device_node *node;
+
+	node = of_irq_find_parent(of_root);
+	ipidomain = irq_find_matching_host(node, DOMAIN_BUS_IPI);
+
+	/*
+	 * Some platforms have half DT setup. So if we found irq node but
+	 * didn't find an ipidomain, try to search for one that is not in the
+	 * DT.
+	 */
+	if (node && !ipidomain)
+		ipidomain = irq_find_matching_host(NULL, DOMAIN_BUS_IPI);
+
+	BUG_ON(!ipidomain);
+
+	if (irq_domain_is_ipi_per_cpu(ipidomain)) {
+		int cpu;
+
+		for_each_cpu(cpu, mask) {
+			remove_irq(call_virq + cpu, &irq_call);
+			remove_irq(sched_virq + cpu, &irq_resched);
+		}
+	}
+	irq_destroy_ipi(call_virq, mask);
+	irq_destroy_ipi(sched_virq, mask);
+	return 0;
+}
+
+
+static int __init mips_smp_ipi_init(void)
+{
+	mips_smp_ipi_allocate(cpu_possible_mask);
+
 	call_desc = irq_to_desc(call_virq);
 	sched_desc = irq_to_desc(sched_virq);
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 4/6] MIPS: CPS: Add VP(E) stealing
  2016-09-20  8:47 [PATCH v2 0/6] MIPS: Remote processor driver Matt Redfearn
                   ` (2 preceding siblings ...)
  2016-09-20  8:47 ` [PATCH v2 3/6] MIPS: smp.c: Introduce mechanism for freeing and allocating IPIs Matt Redfearn
@ 2016-09-20  8:47 ` Matt Redfearn
  2016-09-20  8:47 ` [PATCH v2 5/6] remoteproc/MIPS: Add a remoteproc driver for MIPS Matt Redfearn
  2016-09-20  8:47 ` [PATCH v2 6/6] MIPS: Deprecate VPE Loader Matt Redfearn
  5 siblings, 0 replies; 17+ messages in thread
From: Matt Redfearn @ 2016-09-20  8:47 UTC (permalink / raw)
  To: Ralf Baechle, Bjorn Andersson, Ohad Ben-Cohen, Thomas Gleixner
  Cc: linux-mips, linux-remoteproc, lisa.parratt, linux-kernel,
	Lisa Parratt, Matt Redfearn, Qais Yousef, Masahiro Yamada,
	James Hogan, Paul Burton

From: Lisa Parratt <Lisa.Parratt@imgtec.com>

VP(E) stealing provides a mechanism for removing an offline Virtual
Processor from the Linux kernel such that it is available to run bare
metal code.
Once the CPU has been offlined from Linux, the CPU can be given a task
to run via mips_cps_steal_cpu_and_execute(). The CPU is removed from the
cpu_present mask and is set up to execute from address entry_fn. Stack
space is assigned via the tsk task_struct so that C initialisation code
may be used.
To return the CPU back to Linux control, mips_cps_halt_and_return_cpu
will arrange to halt the CPU and return it to the cpu_present mask. It
is then available to be brought online again via CPU hotplug.

This mechanism is used by the MIPS remote processor driver to allow
CPUs within the system to execute bare metal code, not under control of
the kernel.

Signed-off-by: Lisa Parratt <Lisa.Parratt@imgtec.com>
Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com>
---

Changes in v2: None

 arch/mips/Kconfig               |   7 ++
 arch/mips/include/asm/smp-cps.h |   8 ++
 arch/mips/include/asm/smp.h     |   1 +
 arch/mips/kernel/smp-cps.c      | 162 ++++++++++++++++++++++++++++++++++++++--
 arch/mips/kernel/smp.c          |  12 +++
 5 files changed, 183 insertions(+), 7 deletions(-)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 26388562e300..2094cbcea0d4 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -2341,6 +2341,13 @@ config MIPS_CPS
 	  no external assistance. It is safe to enable this when hardware
 	  support is unavailable.
 
+config MIPS_STEAL
+	bool "VPE stealing"
+	depends on HOTPLUG_CPU && MIPS_CPS
+	help
+	  Select this is you wish to be able to run bare metal code on offline
+	  VPEs.
+
 config MIPS_CPS_PM
 	depends on MIPS_CPS
 	select MIPS_CPC
diff --git a/arch/mips/include/asm/smp-cps.h b/arch/mips/include/asm/smp-cps.h
index 2ae1f61a4a95..4f6cd5b14185 100644
--- a/arch/mips/include/asm/smp-cps.h
+++ b/arch/mips/include/asm/smp-cps.h
@@ -34,6 +34,14 @@ extern void mips_cps_boot_vpes(struct core_boot_config *cfg, unsigned vpe);
 extern void mips_cps_pm_save(void);
 extern void mips_cps_pm_restore(void);
 
+#ifdef CONFIG_MIPS_STEAL
+
+extern int mips_cps_steal_cpu_and_execute(unsigned int cpu, void *entry_fn,
+					  struct task_struct *tsk);
+extern int mips_cps_halt_and_return_cpu(unsigned int cpu);
+
+#endif /* CONFIG_MIPS_STEAL */
+
 #ifdef CONFIG_MIPS_CPS
 
 extern bool mips_cps_smp_in_use(void);
diff --git a/arch/mips/include/asm/smp.h b/arch/mips/include/asm/smp.h
index 060f23ff1817..3c62a1958af5 100644
--- a/arch/mips/include/asm/smp.h
+++ b/arch/mips/include/asm/smp.h
@@ -117,4 +117,5 @@ static inline void arch_send_call_function_ipi_mask(const struct cpumask *mask)
 extern void (*dump_ipi_function_ptr)(void *);
 void dump_send_ipi(void (*dump_ipi_callback)(void *));
 #endif
+
 #endif /* __ASM_SMP_H */
diff --git a/arch/mips/kernel/smp-cps.c b/arch/mips/kernel/smp-cps.c
index e9d9fc6c754c..bcb9b62816b1 100644
--- a/arch/mips/kernel/smp-cps.c
+++ b/arch/mips/kernel/smp-cps.c
@@ -8,6 +8,7 @@
  * option) any later version.
  */
 
+#include <linux/cpu.h>
 #include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/irqchip/mips-gic.h>
@@ -39,6 +40,31 @@ static int __init setup_nothreads(char *s)
 }
 early_param("nothreads", setup_nothreads);
 
+#ifdef CONFIG_MIPS_STEAL
+struct cpumask cpu_stolen_mask;
+
+static inline bool cpu_stolen(int cpu)
+{
+	return cpumask_test_cpu(cpu, &cpu_stolen_mask);
+}
+
+static inline void set_cpu_stolen(int cpu, bool state)
+{
+	if (state)
+		cpumask_set_cpu(cpu, &cpu_stolen_mask);
+	else
+		cpumask_clear_cpu(cpu, &cpu_stolen_mask);
+}
+#else
+static inline bool cpu_stolen(int cpu)
+{
+	return false;
+}
+
+static inline void set_cpu_stolen(int cpu, bool state) { }
+
+#endif /* CONFIG_MIPS_STEAL */
+
 static unsigned core_vpe_count(unsigned core)
 {
 	unsigned cfg;
@@ -109,6 +135,10 @@ static void __init cps_smp_setup(void)
 		write_gcr_bev_base(core_entry);
 	}
 
+#ifdef CONFIG_MIPS_STEAL
+	cpumask_clear(&cpu_stolen_mask);
+#endif /* CONFIG_MIPS_STEAL */
+
 #ifdef CONFIG_MIPS_MT_FPAFF
 	/* If we have an FPU, enroll ourselves in the FPU-full mask */
 	if (cpu_has_fpu)
@@ -287,7 +317,7 @@ static void remote_vpe_boot(void *dummy)
 	mips_cps_boot_vpes(core_cfg, cpu_vpe_id(&current_cpu_data));
 }
 
-static void cps_boot_secondary(int cpu, struct task_struct *idle)
+static void cps_start_secondary(int cpu, void *entry_fn, struct task_struct *tsk)
 {
 	unsigned core = cpu_data[cpu].core;
 	unsigned vpe_id = cpu_vpe_id(&cpu_data[cpu]);
@@ -297,9 +327,9 @@ static void cps_boot_secondary(int cpu, struct task_struct *idle)
 	unsigned int remote;
 	int err;
 
-	vpe_cfg->pc = (unsigned long)&smp_bootstrap;
-	vpe_cfg->sp = __KSTK_TOS(idle);
-	vpe_cfg->gp = (unsigned long)task_thread_info(idle);
+	vpe_cfg->pc = (unsigned long)entry_fn;
+	vpe_cfg->sp = __KSTK_TOS(tsk);
+	vpe_cfg->gp = (unsigned long)task_thread_info(tsk);
 
 	atomic_or(1 << cpu_vpe_id(&cpu_data[cpu]), &core_cfg->vpe_mask);
 
@@ -343,6 +373,11 @@ out:
 	preempt_enable();
 }
 
+static void cps_boot_secondary(int cpu, struct task_struct *idle)
+{
+	cps_start_secondary(cpu, &smp_bootstrap, idle);
+}
+
 static void cps_init_secondary(void)
 {
 	/* Disable MT - we only want to run 1 TC per VPE */
@@ -394,6 +429,28 @@ static int cps_cpu_disable(void)
 	if (!cps_pm_support_state(CPS_PM_POWER_GATED))
 		return -EINVAL;
 
+#ifdef CONFIG_MIPS_STEAL
+	/*
+	 * With the MT ASE only VPEs in the same core may read / write the
+	 * control registers of other VPEs. Therefore to maintain control of
+	 * any stolen VPEs at least one sibling VPE must be kept online.
+	 */
+	if (cpu_has_mipsmt) {
+		int stolen, siblings = 0;
+
+		for_each_cpu((stolen), &cpu_stolen_mask)
+			if (cpu_data[stolen].core == cpu_data[cpu].core)
+				siblings++;
+
+		if (siblings == 1)
+			/*
+			 * When a VPE has been stolen, keep at least one of it's
+			 * siblings around in order to control it.
+			 */
+			return -EBUSY;
+	}
+#endif /* CONFIG_MIPS_STEAL */
+
 	core_cfg = &mips_cps_core_bootcfg[current_cpu_data.core];
 	atomic_sub(1 << cpu_vpe_id(&current_cpu_data), &core_cfg->vpe_mask);
 	smp_mb__after_atomic();
@@ -426,7 +483,7 @@ void play_dead(void)
 		core = cpu_data[cpu].core;
 
 		/* Look for another online VPE within the core */
-		for_each_online_cpu(cpu_death_sibling) {
+		for_each_possible_cpu(cpu_death_sibling) {
 			if (cpu_data[cpu_death_sibling].core != core)
 				continue;
 
@@ -434,8 +491,11 @@ void play_dead(void)
 			 * There is an online VPE within the core. Just halt
 			 * this TC and leave the core alone.
 			 */
-			cpu_death = CPU_DEATH_HALT;
-			break;
+			if (cpu_online(cpu_death_sibling) ||
+			    cpu_stolen(cpu_death_sibling))
+				cpu_death = CPU_DEATH_HALT;
+			if (cpu_online(cpu_death_sibling))
+				break;
 		}
 	}
 
@@ -466,6 +526,94 @@ void play_dead(void)
 	panic("Failed to offline CPU %u", cpu);
 }
 
+#ifdef CONFIG_MIPS_STEAL
+
+/* Find an online sibling CPU (another VPE in the same core) */
+static inline int mips_cps_get_online_sibling(unsigned int cpu)
+{
+	int sibling;
+
+	for_each_online_cpu(sibling)
+		if (cpu_data[sibling].core == cpu_data[cpu].core)
+			return sibling;
+
+	return -1;
+}
+
+int mips_cps_steal_cpu_and_execute(unsigned int cpu, void *entry_fn,
+				   struct task_struct *tsk)
+{
+	int err = -EINVAL;
+
+	preempt_disable();
+
+	if (!cpu_present(cpu) || cpu_online(cpu) || cpu_stolen(cpu))
+		goto out;
+
+	if (cpu_has_mipsmt && (mips_cps_get_online_sibling(cpu) < 0))
+		pr_warn("CPU%d has no online siblings to control it\n", cpu);
+	else {
+		set_cpu_present(cpu, false);
+		set_cpu_stolen(cpu, true);
+
+		cps_start_secondary(cpu, entry_fn, tsk);
+		err = 0;
+	}
+out:
+	preempt_enable();
+	return err;
+}
+
+static void mips_cps_halt_sibling(void *ptr_cpu)
+{
+	unsigned int cpu = (unsigned long)ptr_cpu;
+	unsigned int vpe_id = cpu_vpe_id(&cpu_data[cpu]);
+	unsigned long flags;
+	int vpflags;
+
+	local_irq_save(flags);
+	vpflags = dvpe();
+	settc(vpe_id);
+	write_tc_c0_tchalt(TCHALT_H);
+	evpe(vpflags);
+	local_irq_restore(flags);
+}
+
+int mips_cps_halt_and_return_cpu(unsigned int cpu)
+{
+	unsigned int core = cpu_data[cpu].core;
+	unsigned int vpe_id = cpu_vpe_id(&cpu_data[cpu]);
+
+	if (!cpu_stolen(cpu))
+		return -EINVAL;
+
+	if (cpu_has_mipsmt && (core == cpu_data[smp_processor_id()].core))
+		mips_cps_halt_sibling((void *)(unsigned long)cpu);
+	else if (cpu_has_mipsmt) {
+		int sibling = mips_cps_get_online_sibling(cpu);
+
+		if (sibling < 0) {
+			pr_warn("CPU%d has no online siblings\n", cpu);
+			return -EINVAL;
+		}
+
+		if (smp_call_function_single(sibling, mips_cps_halt_sibling,
+						(void *)(unsigned long)cpu, 1))
+			panic("Failed to call sibling CPU\n");
+
+	} else if (cpu_has_vp) {
+		mips_cm_lock_other(core, vpe_id);
+		write_cpc_co_vp_stop(1 << vpe_id);
+		mips_cm_unlock_other();
+	}
+
+	set_cpu_stolen(cpu, false);
+	set_cpu_present(cpu, true);
+	return 0;
+}
+
+#endif /* CONFIG_MIPS_STEAL */
+
 static void wait_for_sibling_halt(void *ptr_cpu)
 {
 	unsigned cpu = (unsigned long)ptr_cpu;
diff --git a/arch/mips/kernel/smp.c b/arch/mips/kernel/smp.c
index afa06c2bb019..f3d01f556fe2 100644
--- a/arch/mips/kernel/smp.c
+++ b/arch/mips/kernel/smp.c
@@ -233,6 +233,18 @@ static void smp_ipi_init_one(unsigned int virq,
 				    struct irqaction *action)
 {
 	int ret;
+#ifdef CONFIG_MIPS_STEAL
+	struct irq_data *data;
+	/*
+	 * A bit of a hack to ensure that the ipi_offset is 0.
+	 * This is to deal with removing / reallocating IPIs
+	 * to subsets of the possible CPUs, where the IPI IRQ domain
+	 * will set ipi_offset to the first cpu in the cpumask when the
+	 * IPI is reallocated.
+	 */
+	data = irq_get_irq_data(virq);
+	data->common->ipi_offset = 0;
+#endif /* CONFIG_MIPS_STEAL */
 
 	irq_set_handler(virq, handle_percpu_irq);
 	ret = setup_irq(virq, action);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 5/6] remoteproc/MIPS: Add a remoteproc driver for MIPS
  2016-09-20  8:47 [PATCH v2 0/6] MIPS: Remote processor driver Matt Redfearn
                   ` (3 preceding siblings ...)
  2016-09-20  8:47 ` [PATCH v2 4/6] MIPS: CPS: Add VP(E) stealing Matt Redfearn
@ 2016-09-20  8:47 ` Matt Redfearn
  2016-09-20  9:47   ` Thomas Gleixner
                     ` (2 more replies)
  2016-09-20  8:47 ` [PATCH v2 6/6] MIPS: Deprecate VPE Loader Matt Redfearn
  5 siblings, 3 replies; 17+ messages in thread
From: Matt Redfearn @ 2016-09-20  8:47 UTC (permalink / raw)
  To: Ralf Baechle, Bjorn Andersson, Ohad Ben-Cohen, Thomas Gleixner
  Cc: linux-mips, linux-remoteproc, lisa.parratt, linux-kernel, Matt Redfearn

Add a remoteproc driver to steal, load the firmware, and boot an offline
MIPS core, turning it into a coprocessor.

This driver provides a sysfs to allow arbitrary firmware to be loaded
onto a core, which may expose virtio devices. Coprocessor firmware must
abide by the UHI coprocessor boot protocol.

Signed-off-by: Lisa Parratt <lisa.parratt@imgtec.com>
Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com>

---

Changes in v2: None

 Documentation/ABI/testing/sysfs-class-mips-rproc |  24 +
 drivers/remoteproc/Kconfig                       |  11 +
 drivers/remoteproc/Makefile                      |   1 +
 drivers/remoteproc/mips_remoteproc.c             | 651 +++++++++++++++++++++++
 4 files changed, 687 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-class-mips-rproc
 create mode 100644 drivers/remoteproc/mips_remoteproc.c

diff --git a/Documentation/ABI/testing/sysfs-class-mips-rproc b/Documentation/ABI/testing/sysfs-class-mips-rproc
new file mode 100644
index 000000000000..c09d02a755e4
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-mips-rproc
@@ -0,0 +1,24 @@
+What:		/sys/class/mips-rproc/rproc#/firmware
+Date:		August 2015
+Contact:	Lisa Parratt <lisa.parratt@imgtec.com>
+Description:	MIPS VPE remoteproc start
+
+	This node only exists when a VPE is considered offline by Linux. Writes
+	to this file will start firmware running on a VPE.
+
+	If the VPE is idle, specifying a name will cause a remoteproc instance
+	to be allocated, which will cause the core to be stolen, the firmware
+	image to be loaded, and the remoteproc instance to be started.
+	Otherwise, the operation will fail.
+
+What:		/sys/class/mips-rproc/rproc#/stop
+Date:		August 2015
+Contact:	Lisa Parratt <lisa.parratt@imgtec.com>
+Description:	MIPS VPE remoteproc stop
+
+	This node only exists when a VPE is considered offline by Linux. Writes
+	to this file will stop firmware running on a VPE.
+
+	If the VPE is running a remote proc instance, the instance will be
+	stopped, the core returned, and the instance freed.
+	Otherwise, the operation will fail.
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 1a8bf76a925f..05db52e0e668 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -100,4 +100,15 @@ config ST_REMOTEPROC
 	  processor framework.
 	  This can be either built-in or a loadable module.
 
+config MIPS_REMOTEPROC
+	tristate "MIPS remoteproc support"
+	depends on MIPS_CPS && HAS_DMA
+	select CMA
+	select REMOTEPROC
+	select MIPS_STEAL
+	help
+	  Say y here to support using offline cores/VPEs as remote processors
+	  via the remote processor framework.
+	  If unsure say N.
+
 endmenu
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 92d3758bd15c..de19cd320f3a 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -14,3 +14,4 @@ obj-$(CONFIG_DA8XX_REMOTEPROC)		+= da8xx_remoteproc.o
 obj-$(CONFIG_QCOM_MDT_LOADER)		+= qcom_mdt_loader.o
 obj-$(CONFIG_QCOM_Q6V5_PIL)		+= qcom_q6v5_pil.o
 obj-$(CONFIG_ST_REMOTEPROC)		+= st_remoteproc.o
+obj-$(CONFIG_MIPS_REMOTEPROC)		+= mips_remoteproc.o
diff --git a/drivers/remoteproc/mips_remoteproc.c b/drivers/remoteproc/mips_remoteproc.c
new file mode 100644
index 000000000000..944ad66280b4
--- /dev/null
+++ b/drivers/remoteproc/mips_remoteproc.c
@@ -0,0 +1,651 @@
+/*
+ * MIPS Remote Processor driver
+ *
+ * Copyright (C) 2016 Imagination Technologies
+ * Lisa Parratt <lisa.parratt@imgtec.com>
+ * Matt Redfearn <matt.redfearn@imgtec.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+#include <linux/cpu.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/remoteproc.h>
+
+#include <asm/dma-coherence.h>
+#include <asm/smp-cps.h>
+#include <asm/tlbflush.h>
+#include <asm/tlbmisc.h>
+
+#include "remoteproc_internal.h"
+
+struct mips_rproc {
+	struct rproc		*rproc;
+	char			*firmware;
+	struct task_struct	*tsk;
+	struct device		dev;
+	unsigned int		cpu;
+	int			ipi_linux;
+	int			ipi_remote;
+};
+
+static struct rproc *mips_rprocs[NR_CPUS];
+
+#define to_mips_rproc(d) container_of(d, struct mips_rproc, dev)
+
+
+/* Compute the largest page mask a physical address can be mapped with */
+static unsigned long mips_rproc_largest_pm(unsigned long pa,
+					   unsigned long maxmask)
+{
+	unsigned long mask;
+	/* Find address bits limiting alignment */
+	unsigned long shift = ffs(pa);
+
+	/* Obey MIPS restrictions on page sizes */
+	if (pa) {
+		if (shift & 1)
+			shift -= 2;
+		else
+			shift--;
+	}
+	mask = ULONG_MAX << shift;
+	return maxmask & ~mask;
+}
+
+/* Compute the next largest page mask for a given page mask */
+static unsigned long mips_rproc_next_pm(unsigned long pm, unsigned long maxmask)
+{
+	if (pm != PM_4K)
+		return ((pm << 2) | pm) & maxmask;
+	else
+		return PM_16K;
+}
+
+static void mips_map_page(unsigned long da, unsigned long pa, int c,
+			  unsigned long pagemask, unsigned long pagesize)
+{
+	unsigned long pa2 = pa + (pagesize / 2);
+	unsigned long entryhi, entrylo0, entrylo1;
+
+	/* Compute the mapping */
+	pa = (pa >> 6) & (ULONG_MAX << MIPS_ENTRYLO_PFN_SHIFT);
+	pa2 = (pa2 >> 6) & (ULONG_MAX << MIPS_ENTRYLO_PFN_SHIFT);
+	entryhi = da & 0xfffffe000;
+	entrylo0 = (c << ENTRYLO_C_SHIFT) | ENTRYLO_D | ENTRYLO_V | pa;
+	entrylo1 = (c << ENTRYLO_C_SHIFT) | ENTRYLO_D | ENTRYLO_V | pa2;
+
+	pr_debug("Create wired entry %d, CCA %d\n", read_c0_wired(), c);
+	pr_debug(" EntryHi: 0x%016lx\n", entryhi);
+	pr_debug(" EntryLo0: 0x%016lx\n", entrylo0);
+	pr_debug(" EntryLo1: 0x%016lx\n", entrylo1);
+	pr_debug(" Pagemask: 0x%016lx\n", pagemask);
+	pr_debug("\n");
+
+	add_wired_entry(entrylo0, entrylo1, entryhi, pagemask);
+}
+
+/*
+ * Compute the page required to fulfill a mapping. Escapes alignment derived
+ * page size limitations before using biggest fit to map the remainder.
+ */
+static inline void mips_rproc_fit_page(unsigned long da, unsigned long pa,
+					int c, unsigned long size,
+					unsigned long maxmask)
+{
+	unsigned long bigmask, nextmask;
+	unsigned long pagemask, pagesize;
+	unsigned long distance, target;
+
+	do {
+		/* Compute the current largest page mask */
+		bigmask = mips_rproc_largest_pm(pa, maxmask);
+		/* Compute the next largest pagesize */
+		nextmask = mips_rproc_next_pm(bigmask, maxmask);
+		/*
+		 * Compute the distance from our current physical address to
+		 * the next page boundary.
+		 */
+		distance = (nextmask + 0x2000) - (pa & nextmask);
+		/*
+		 * Decide between searching to get to the next highest page
+		 * boundary or finishing.
+		 */
+		target = distance < size ? distance : size;
+		/* Fit */
+		while (target) {
+			/* Find the largest supported page size that will fit */
+			for (pagesize = maxmask + 0x2000;
+			     (pagesize > 0x2000) && (pagesize > target);
+			     pagesize /= 4) {
+			}
+			/* Convert it to a page mask */
+			pagemask = pagesize - 0x2000;
+			/* Emit it */
+			mips_map_page(da, pa, c, pagemask, pagesize);
+			/* Move to next step */
+			size -= pagesize;
+			da += pagesize;
+			pa += pagesize;
+			target -= pagesize;
+		}
+	} while (size);
+}
+
+
+static int mips_rproc_carveouts(struct rproc *rproc, int max_pagemask)
+{
+	struct rproc_mem_entry *carveout;
+
+	list_for_each_entry(carveout, &rproc->carveouts, node) {
+		int c = CONF_CM_CACHABLE_COW;
+
+		dev_dbg(&rproc->dev,
+			"carveout mapping da 0x%x -> %pad length 0x%x, CCA %d",
+			carveout->da, &carveout->dma, carveout->len, c);
+
+		mips_rproc_fit_page(carveout->da, carveout->dma, c,
+				    carveout->len, max_pagemask);
+	}
+	return 0;
+}
+
+
+static int mips_rproc_vdevs(struct rproc *rproc, int max_pagemask)
+{
+	struct rproc_vdev *rvdev;
+
+	list_for_each_entry(rvdev, &rproc->rvdevs, node) {
+		int i, size;
+
+		for (i = 0; i < ARRAY_SIZE(rvdev->vring); i++) {
+			struct rproc_vring *vring = &rvdev->vring[i];
+			unsigned long pa = vring->dma;
+			int c;
+
+			if (hw_coherentio) {
+				/*
+				 * The DMA API will allocate cacheable buffers
+				 * for shared resources, so the firmware should
+				 * also access those buffers cached
+				 */
+				c = (_page_cachable_default >> _CACHE_SHIFT);
+			} else {
+				/*
+				 * Otherwise, shared buffers should be accessed
+				 * uncached
+				 */
+				c = CONF_CM_UNCACHED;
+			}
+
+			/* actual size of vring (in bytes) */
+			size = PAGE_ALIGN(vring_size(vring->len, vring->align));
+
+			dev_dbg(&rproc->dev,
+				"vring mapping da %pad -> %pad length 0x%x, CCA %d",
+				&vring->dma, &vring->dma, size, c);
+
+			mips_rproc_fit_page(pa, pa, c, size, max_pagemask);
+		}
+	}
+	return 0;
+}
+
+static void mips_rproc_cpu_entry(void)
+{
+	struct rproc *rproc = mips_rprocs[smp_processor_id()];
+	struct mips_rproc *mproc = *(struct mips_rproc **)rproc->priv;
+	int ipi_to_remote = ipi_get_hwirq(mproc->ipi_remote, mproc->cpu);
+	int ipi_from_remote = ipi_get_hwirq(mproc->ipi_linux, 0);
+	unsigned long old_pagemask, max_pagemask;
+
+	if (!rproc)
+		return;
+
+	dev_info(&rproc->dev, "Starting %s on MIPS CPU%d\n",
+		 mproc->firmware, mproc->cpu);
+
+	/* Get the maximum pagemask supported on this CPU */
+	old_pagemask = read_c0_pagemask();
+	write_c0_pagemask(PM_HUGE_MASK);
+	mtc0_tlbw_hazard();
+	max_pagemask = read_c0_pagemask();
+	write_c0_pagemask(old_pagemask);
+	mtc0_tlbw_hazard();
+
+	/* Start with no wired entries */
+	write_c0_wired(0);
+
+	/* Flush all previous TLB entries */
+	local_flush_tlb_all();
+
+	/* Map firmware resources into virtual memory */
+	mips_rproc_carveouts(rproc, max_pagemask);
+	mips_rproc_vdevs(rproc, max_pagemask);
+
+	dev_dbg(&rproc->dev, "IPI to remote: %d\n", ipi_to_remote);
+	dev_dbg(&rproc->dev, "IPI from remote: %d\n", ipi_from_remote);
+
+	/* Hand off the CPU to the firmware */
+	dev_dbg(&rproc->dev, "Jumping to firmware at 0x%x\n", rproc->bootaddr);
+
+	write_c0_entryhi(0); /* Set ASID 0 */
+	tlbw_use_hazard();
+
+	/* Firmware protocol */
+	__asm__("addiu $a0, $zero, -3");
+	__asm__("move $a1, %0" :: "r" (ipi_to_remote));
+	__asm__("move $a2, %0" :: "r" (ipi_from_remote));
+	__asm__("move $a3, $zero");
+	__asm__("jr %0" :: "r" (rproc->bootaddr));
+}
+
+
+static irqreturn_t mips_rproc_ipi_handler(int irq, void *dev_id)
+{
+	/* Synthetic interrupts shouldn't need acking */
+	return IRQ_WAKE_THREAD;
+}
+
+
+static irqreturn_t mips_rproc_vq_int(int irq, void *p)
+{
+	struct rproc *rproc = (struct rproc *)p;
+	void *entry;
+	int id;
+
+	/* We don't have a mailbox, so iterate over all vqs and kick them. */
+	idr_for_each_entry(&rproc->notifyids, entry, id)
+		rproc_vq_interrupt(rproc, id);
+
+	return IRQ_HANDLED;
+}
+
+
+/* Helper function to find the IPI domain */
+static struct irq_domain *ipi_domain(void)
+{
+	struct device_node *node = of_irq_find_parent(of_root);
+	struct irq_domain *ipidomain;
+
+	ipidomain = irq_find_matching_host(node, DOMAIN_BUS_IPI);
+	/*
+	 * Some platforms have half DT setup. So if we found irq node but
+	 * didn't find an ipidomain, try to search for one that is not in the
+	 * DT.
+	 */
+	if (node && !ipidomain)
+		ipidomain = irq_find_matching_host(NULL, DOMAIN_BUS_IPI);
+
+	return ipidomain;
+}
+
+
+int mips_rproc_op_start(struct rproc *rproc)
+{
+	struct mips_rproc *mproc = *(struct mips_rproc **)rproc->priv;
+	int err;
+	int cpu = mproc->cpu;
+
+	if (mips_rprocs[cpu]) {
+		dev_err(&rproc->dev, "CPU%d in use\n", cpu);
+		return -EBUSY;
+	}
+	mips_rprocs[cpu] = rproc;
+
+	/* Create task for the CPU to use before handing off to firmware */
+	mproc->tsk = fork_idle(cpu);
+	if (IS_ERR(mproc->tsk)) {
+		dev_err(&rproc->dev, "fork_idle() failed for CPU%d\n", cpu);
+		return -ENOMEM;
+	}
+
+	/* We won't be needing the Linux IPIs anymore */
+	if (mips_smp_ipi_free(get_cpu_mask(cpu)))
+		return -EINVAL;
+
+	/*
+	 * Direct IPIs from the remote processor to CPU0 since that can't be
+	 * offlined while the remote CPU is running.
+	 */
+	mproc->ipi_linux = irq_reserve_ipi(ipi_domain(), get_cpu_mask(0));
+	if (!mproc->ipi_linux) {
+		dev_err(&mproc->dev, "Failed to reserve incoming kick\n");
+		goto exit_rproc_nofrom;
+	}
+
+	mproc->ipi_remote = irq_reserve_ipi(ipi_domain(), get_cpu_mask(cpu));
+	if (!mproc->ipi_remote) {
+		dev_err(&mproc->dev, "Failed to reserve outgoing kick\n");
+		goto exit_rproc_noto;
+	}
+
+	/* register incoming ipi */
+	err = devm_request_threaded_irq(&mproc->dev, mproc->ipi_linux,
+					mips_rproc_ipi_handler,
+					mips_rproc_vq_int, 0,
+					"mips-rproc IPI in", mproc->rproc);
+	if (err) {
+		dev_err(&mproc->dev, "Failed to register incoming kick: %d\n",
+			err);
+		goto exit_rproc_noint;
+	}
+
+	if (!mips_cps_steal_cpu_and_execute(cpu, &mips_rproc_cpu_entry,
+						mproc->tsk))
+		return 0;
+
+	dev_err(&mproc->dev, "Failed to steal CPU%d for remote\n", cpu);
+	devm_free_irq(&mproc->dev, mproc->ipi_linux, mproc->rproc);
+exit_rproc_noint:
+	irq_destroy_ipi(mproc->ipi_remote, get_cpu_mask(cpu));
+exit_rproc_noto:
+	irq_destroy_ipi(mproc->ipi_linux, get_cpu_mask(0));
+exit_rproc_nofrom:
+	free_task(mproc->tsk);
+	mips_rprocs[cpu] = NULL;
+
+	/* Set up the Linux IPIs again */
+	mips_smp_ipi_allocate(get_cpu_mask(cpu));
+	return -EINVAL;
+}
+
+int mips_rproc_op_stop(struct rproc *rproc)
+{
+	struct mips_rproc *mproc = *(struct mips_rproc **)rproc->priv;
+
+	if (mproc->ipi_linux)
+		devm_free_irq(&mproc->dev, mproc->ipi_linux, mproc->rproc);
+
+	irq_destroy_ipi(mproc->ipi_linux, get_cpu_mask(0));
+	irq_destroy_ipi(mproc->ipi_remote, get_cpu_mask(mproc->cpu));
+
+	/* Set up the Linux IPIs again */
+	mips_smp_ipi_allocate(get_cpu_mask(mproc->cpu));
+
+	free_task(mproc->tsk);
+
+	mips_rprocs[mproc->cpu] = NULL;
+
+	return mips_cps_halt_and_return_cpu(mproc->cpu);
+}
+
+
+void mips_rproc_op_kick(struct rproc *rproc, int vqid)
+{
+	struct mips_rproc *mproc = *(struct mips_rproc **)rproc->priv;
+
+	ipi_send_single(mproc->ipi_remote, mproc->cpu);
+}
+
+struct rproc_ops mips_rproc_proc_ops = {
+	.start	= mips_rproc_op_start,
+	.stop	= mips_rproc_op_stop,
+	.kick	= mips_rproc_op_kick,
+};
+
+
+static int mips_rproc_probe(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static int mips_rproc_remove(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static struct platform_driver mips_rproc_driver = {
+	.probe = mips_rproc_probe,
+	.remove = mips_rproc_remove,
+	.driver = {
+		.name = "mips-rproc"
+	},
+};
+
+
+/* Steal a core and run some firmware on it */
+int mips_rproc_start(struct mips_rproc *mproc, const char *firmware, size_t len)
+{
+	int err = -EINVAL;
+	struct mips_rproc **priv;
+
+	/* Duplicate the filename, dropping whitespace from the end via len */
+	mproc->firmware = kstrndup(firmware, len, GFP_KERNEL);
+	if (!mproc->firmware)
+		return -ENOMEM;
+
+	mproc->rproc = rproc_alloc(&mproc->dev, "mips", &mips_rproc_proc_ops,
+				   mproc->firmware,
+				   sizeof(struct mips_rproc *));
+	if (!mproc->rproc)
+		return -ENOMEM;
+
+	priv = mproc->rproc->priv;
+	*priv = mproc;
+
+	/* go live! */
+	err = rproc_add(mproc->rproc);
+	if (err) {
+		dev_err(&mproc->dev, "Failed to add rproc: %d\n", err);
+		rproc_put(mproc->rproc);
+		kfree(mproc->firmware);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/* Stop a core, and return it to being offline */
+int mips_rproc_stop(struct mips_rproc *mproc)
+{
+	rproc_shutdown(mproc->rproc);
+	rproc_del(mproc->rproc);
+	rproc_put(mproc->rproc);
+	mproc->rproc = NULL;
+	return 0;
+}
+
+/* sysfs interface to mips_rproc_start */
+static ssize_t firmware_store(struct device *dev,
+			      struct device_attribute *attr,
+			      const char *buf, size_t count)
+{
+	struct mips_rproc *mproc = to_mips_rproc(dev);
+	size_t len = count;
+	int err = -EINVAL;
+
+	if (buf[count - 1] == '\n')
+		len--;
+
+	if (!mproc->rproc && len)
+		err = mips_rproc_start(mproc, buf, len);
+	else if (len)
+		err = -EBUSY;
+
+	return err ? err : count;
+}
+static DEVICE_ATTR_WO(firmware);
+
+/* sysfs interface to mips_rproc_stop */
+static ssize_t stop_store(struct device *dev,
+			      struct device_attribute *attr,
+			      const char *buf, size_t count)
+{
+	struct mips_rproc *mproc = to_mips_rproc(dev);
+	int err = -EINVAL;
+
+
+	if (mproc->rproc)
+		err = mips_rproc_stop(mproc);
+	else
+		err = -EBUSY;
+
+	return err ? err : count;
+}
+static DEVICE_ATTR_WO(stop);
+
+/* Boiler plate for devclarng mips-rproc sysfs devices */
+static struct attribute *mips_rproc_attrs[] = {
+	&dev_attr_firmware.attr,
+	&dev_attr_stop.attr,
+	NULL
+};
+
+static struct attribute_group mips_rproc_devgroup = {
+	.attrs = mips_rproc_attrs
+};
+
+static const struct attribute_group *mips_rproc_devgroups[] = {
+	&mips_rproc_devgroup,
+	NULL
+};
+
+static char *mips_rproc_devnode(struct device *dev, umode_t *mode)
+{
+	return kasprintf(GFP_KERNEL, "mips-rproc/%s", dev_name(dev));
+}
+
+static struct class mips_rproc_class = {
+	.name		= "mips-rproc",
+	.devnode	= mips_rproc_devnode,
+	.dev_groups	= mips_rproc_devgroups,
+};
+
+static void mips_rproc_release(struct device *dev)
+{
+}
+
+static int mips_rproc_uevent(struct device *dev, struct kobj_uevent_env *env)
+{
+	struct mips_rproc *mproc = to_mips_rproc(dev);
+
+	if (!mproc)
+		return -ENODEV;
+
+	return 0;
+}
+
+static struct device_type mips_rproc_type = {
+	.release	= mips_rproc_release,
+	.uevent		= mips_rproc_uevent
+};
+
+/* Helper function for locating the device for a CPU */
+int mips_rproc_device_rproc_match(struct device *dev, const void *data)
+{
+	struct mips_rproc *mproc = to_mips_rproc(dev);
+	unsigned int cpu = *(unsigned int *)data;
+
+	return mproc->cpu == cpu;
+}
+
+/* Create a sysfs device in response to CPU down */
+int mips_rproc_device_register(unsigned int cpu)
+{
+	struct mips_rproc *dev;
+
+	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+	if (!dev)
+		return -EINVAL;
+
+	dev->dev.driver = &mips_rproc_driver.driver;
+	dev->dev.type = &mips_rproc_type;
+	dev->dev.class = &mips_rproc_class;
+	dev->dev.id = cpu;
+	dev_set_name(&dev->dev, "rproc%u", cpu);
+	dev->cpu = cpu;
+
+	return device_register(&dev->dev);
+}
+
+/* Destroy a sysfs device in response to CPU up */
+int mips_rproc_device_unregister(unsigned int cpu)
+{
+	struct device *dev = class_find_device(&mips_rproc_class, NULL, &cpu,
+					       mips_rproc_device_rproc_match);
+	struct mips_rproc *mproc = to_mips_rproc(dev);
+
+	if (mips_rprocs[cpu])
+		mips_rproc_stop(mproc);
+
+	put_device(dev);
+	device_unregister(dev);
+	kfree(mproc);
+	return 0;
+}
+
+/* Intercept CPU hotplug events for syfs purposes */
+static int mips_rproc_callback(struct notifier_block *nfb, unsigned long action,
+			       void *hcpu)
+{
+	unsigned int cpu = (unsigned long)hcpu;
+
+	switch (action) {
+	case CPU_UP_PREPARE:
+	case CPU_DOWN_FAILED:
+		mips_rproc_device_unregister(cpu);
+		break;
+	case CPU_DOWN_PREPARE:
+		mips_rproc_device_register(cpu);
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block mips_rproc_notifier __refdata = {
+	.notifier_call = mips_rproc_callback
+};
+
+static int __init mips_rproc_init(void)
+{
+	int cpu;
+	/* create mips-rproc device class for sysfs */
+	int err = class_register(&mips_rproc_class);
+
+	if (err) {
+		pr_err("mips-proc: unable to register mips-rproc class\n");
+		return err;
+	}
+
+	/* Dynamically create mips-rproc class devices based on hotplug data */
+	get_online_cpus();
+	for_each_possible_cpu(cpu)
+		if (!cpu_online(cpu))
+			mips_rproc_device_register(cpu);
+	register_hotcpu_notifier(&mips_rproc_notifier);
+	put_online_cpus();
+
+	return 0;
+}
+
+static void __exit mips_rproc_exit(void)
+{
+	int cpu;
+	/* Destroy mips-rproc class devices */
+	get_online_cpus();
+	unregister_hotcpu_notifier(&mips_rproc_notifier);
+	for_each_possible_cpu(cpu)
+		if (!cpu_online(cpu))
+			mips_rproc_device_unregister(cpu);
+	put_online_cpus();
+
+	class_unregister(&mips_rproc_class);
+}
+
+subsys_initcall(mips_rproc_init);
+module_exit(mips_rproc_exit);
+
+module_platform_driver(mips_rproc_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("MIPS Remote Processor control driver");
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 6/6] MIPS: Deprecate VPE Loader
  2016-09-20  8:47 [PATCH v2 0/6] MIPS: Remote processor driver Matt Redfearn
                   ` (4 preceding siblings ...)
  2016-09-20  8:47 ` [PATCH v2 5/6] remoteproc/MIPS: Add a remoteproc driver for MIPS Matt Redfearn
@ 2016-09-20  8:47 ` Matt Redfearn
  2016-09-20 10:25   ` Sergei Shtylyov
  2016-09-20 13:19   ` Ralf Baechle
  5 siblings, 2 replies; 17+ messages in thread
From: Matt Redfearn @ 2016-09-20  8:47 UTC (permalink / raw)
  To: Ralf Baechle, Bjorn Andersson, Ohad Ben-Cohen, Thomas Gleixner
  Cc: linux-mips, linux-remoteproc, lisa.parratt, linux-kernel, Matt Redfearn

The MIPS remote processor driver (CONFIG_MIPS_RPROC) provides a more
standard mechanism for using one or more VPs as coprocessors running
separate firmware.

Here we deprecate this mechanism before it is removed.

Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com>
---

Changes in v2: None

 arch/mips/Kconfig | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 2094cbcea0d4..071fc4585265 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -2263,7 +2263,7 @@ comment "MIPS R2-to-R6 emulator is only available for UP kernels"
 	depends on SMP && CPU_MIPSR6
 
 config MIPS_VPE_LOADER
-	bool "VPE loader support."
+	bool "VPE loader support. (DEPRECATED)"
 	depends on SYS_SUPPORTS_MULTITHREADING && MODULES
 	select CPU_MIPSR2_IRQ_VI
 	select CPU_MIPSR2_IRQ_EI
@@ -2272,6 +2272,9 @@ config MIPS_VPE_LOADER
 	  Includes a loader for loading an elf relocatable object
 	  onto another VPE and running it.
 
+	  Unless you have a specific need, you should use CONFIG_MIPS_RPROC
+          instead of this.
+
 config MIPS_VPE_LOADER_CMP
 	bool
 	default "y"
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 1/6] irqchip: mips-gic: Add context saving for MIPS_REMOTEPROC
  2016-09-20  8:47 ` [PATCH v2 1/6] irqchip: mips-gic: Add context saving for MIPS_REMOTEPROC Matt Redfearn
@ 2016-09-20  9:40   ` Thomas Gleixner
  0 siblings, 0 replies; 17+ messages in thread
From: Thomas Gleixner @ 2016-09-20  9:40 UTC (permalink / raw)
  To: Matt Redfearn
  Cc: Ralf Baechle, Bjorn Andersson, Ohad Ben-Cohen, linux-mips,
	linux-remoteproc, lisa.parratt, linux-kernel, Marc Zyngier,
	Jason Cooper

On Tue, 20 Sep 2016, Matt Redfearn wrote:
> +/*
> + * The MIPS remote processor driver allows non-Linux firmware to take control
> + * of and execute on one of the systems VPEs. If that VPE is brought back under
> + * Linux, it is necessary to ensure that all GIC interrupts are routed and
> + * masked as Linux expects them, as the firmware can have done anything it
> + * likes with the GIC configuration (hopefully just for that VPEs local
> + * interrupt sources, but allow for shared external interrupts as well).
> + */
> +static int gic_cpu_notify(struct notifier_block *nfb, unsigned long action,
> +			       void *hcpu)
> +{
> +	unsigned int cpu = mips_cm_vp_id((unsigned long)hcpu);
> +
> +	switch (action) {
> +	case CPU_UP_PREPARE:
> +	case CPU_DOWN_FAILED:
> +		gic_restore_shared();
> +		gic_restore_local(cpu);
> +	default:
> +		break;
> +	}
> +
> +	return NOTIFY_OK;
> +}

Please use the new state machine based infrastructure.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 5/6] remoteproc/MIPS: Add a remoteproc driver for MIPS
  2016-09-20  8:47 ` [PATCH v2 5/6] remoteproc/MIPS: Add a remoteproc driver for MIPS Matt Redfearn
@ 2016-09-20  9:47   ` Thomas Gleixner
  2016-09-20 15:31     ` Matt Redfearn
  2016-10-03  8:35   ` Matt Redfearn
  2016-10-03 22:16   ` Bjorn Andersson
  2 siblings, 1 reply; 17+ messages in thread
From: Thomas Gleixner @ 2016-09-20  9:47 UTC (permalink / raw)
  To: Matt Redfearn
  Cc: Ralf Baechle, Bjorn Andersson, Ohad Ben-Cohen, linux-mips,
	linux-remoteproc, lisa.parratt, linux-kernel

On Tue, 20 Sep 2016, Matt Redfearn wrote:
> +/* Intercept CPU hotplug events for syfs purposes */
> +static int mips_rproc_callback(struct notifier_block *nfb, unsigned long action,
> +			       void *hcpu)
> +{

Please convert to cpu hotplug state machine.

> +	unsigned int cpu = (unsigned long)hcpu;
> +
> +	switch (action) {
> +	case CPU_UP_PREPARE:
> +	case CPU_DOWN_FAILED:
> +		mips_rproc_device_unregister(cpu);
> +		break;
> +	case CPU_DOWN_PREPARE:
> +		mips_rproc_device_register(cpu);
> +		break;
> +	}

There is no reason why you need to setup the rproc device on
DOWN_PREPARE. It's sufficient to do that when the CPU is dead, so you can
use a symetric callback prep/dead.

> +	/* Dynamically create mips-rproc class devices based on hotplug data */
> +	get_online_cpus();
> +	for_each_possible_cpu(cpu)
> +		if (!cpu_online(cpu))
> +			mips_rproc_device_register(cpu);
> +	register_hotcpu_notifier(&mips_rproc_notifier);
> +	put_online_cpus();

Perhaps we should add support for "reverse" functionality to the state
machine core. I'll have a look later how hard that'd be.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 6/6] MIPS: Deprecate VPE Loader
  2016-09-20  8:47 ` [PATCH v2 6/6] MIPS: Deprecate VPE Loader Matt Redfearn
@ 2016-09-20 10:25   ` Sergei Shtylyov
  2016-09-20 13:19   ` Ralf Baechle
  1 sibling, 0 replies; 17+ messages in thread
From: Sergei Shtylyov @ 2016-09-20 10:25 UTC (permalink / raw)
  To: Matt Redfearn, Ralf Baechle, Bjorn Andersson, Ohad Ben-Cohen,
	Thomas Gleixner
  Cc: linux-mips, linux-remoteproc, lisa.parratt, linux-kernel

Hello.

On 9/20/2016 11:47 AM, Matt Redfearn wrote:

> The MIPS remote processor driver (CONFIG_MIPS_RPROC) provides a more
> standard mechanism for using one or more VPs as coprocessors running
> separate firmware.
>
> Here we deprecate this mechanism before it is removed.
>
> Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com>
> ---
>
> Changes in v2: None
>
>  arch/mips/Kconfig | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index 2094cbcea0d4..071fc4585265 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -2263,7 +2263,7 @@ comment "MIPS R2-to-R6 emulator is only available for UP kernels"
>  	depends on SMP && CPU_MIPSR6
>
>  config MIPS_VPE_LOADER
> -	bool "VPE loader support."
> +	bool "VPE loader support. (DEPRECATED)"

    I think the period should be after (DEPRECATED), if at all.

[...]

MBR,Sergei

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 6/6] MIPS: Deprecate VPE Loader
  2016-09-20  8:47 ` [PATCH v2 6/6] MIPS: Deprecate VPE Loader Matt Redfearn
  2016-09-20 10:25   ` Sergei Shtylyov
@ 2016-09-20 13:19   ` Ralf Baechle
  2016-09-20 13:53     ` Matt Redfearn
  1 sibling, 1 reply; 17+ messages in thread
From: Ralf Baechle @ 2016-09-20 13:19 UTC (permalink / raw)
  To: Matt Redfearn
  Cc: Bjorn Andersson, Ohad Ben-Cohen, Thomas Gleixner, linux-mips,
	linux-remoteproc, lisa.parratt, linux-kernel

On Tue, Sep 20, 2016 at 09:47:29AM +0100, Matt Redfearn wrote:

> The MIPS remote processor driver (CONFIG_MIPS_RPROC) provides a more
> standard mechanism for using one or more VPs as coprocessors running
> separate firmware.
> 
> Here we deprecate this mechanism before it is removed.

The world will be a better place once this is removed.

I receive the occasional minor cleanup or robopatch (coccinelle or similar)
for the VPE loader but I have no indication this is actually being used
by anybody, so is thee any reason why not to delete it right away?

  Ralf

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 6/6] MIPS: Deprecate VPE Loader
  2016-09-20 13:19   ` Ralf Baechle
@ 2016-09-20 13:53     ` Matt Redfearn
  0 siblings, 0 replies; 17+ messages in thread
From: Matt Redfearn @ 2016-09-20 13:53 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: Bjorn Andersson, Ohad Ben-Cohen, Thomas Gleixner, linux-mips,
	linux-remoteproc, lisa.parratt, linux-kernel

Hi Ralf,


On 20/09/16 14:19, Ralf Baechle wrote:
> On Tue, Sep 20, 2016 at 09:47:29AM +0100, Matt Redfearn wrote:
>
>> The MIPS remote processor driver (CONFIG_MIPS_RPROC) provides a more
>> standard mechanism for using one or more VPs as coprocessors running
>> separate firmware.
>>
>> Here we deprecate this mechanism before it is removed.
> The world will be a better place once this is removed.

Indeed :-)

>
> I receive the occasional minor cleanup or robopatch (coccinelle or similar)
> for the VPE loader but I have no indication this is actually being used
> by anybody, so is thee any reason why not to delete it right away?

I'd like to get the remote processor implementation in, then delete the 
VPE loader in the next kernel revision, if that's ok with you. Once it 
is removed, we should also be able to remove the CMP SMP implementation, 
and other bits of infrastructure that'll no longer be needed.

Thanks,
Matt

>
>    Ralf

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 5/6] remoteproc/MIPS: Add a remoteproc driver for MIPS
  2016-09-20  9:47   ` Thomas Gleixner
@ 2016-09-20 15:31     ` Matt Redfearn
  0 siblings, 0 replies; 17+ messages in thread
From: Matt Redfearn @ 2016-09-20 15:31 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Ralf Baechle, Bjorn Andersson, Ohad Ben-Cohen, linux-mips,
	linux-remoteproc, lisa.parratt, linux-kernel

Hi Thomas,


On 20/09/16 10:47, Thomas Gleixner wrote:
> On Tue, 20 Sep 2016, Matt Redfearn wrote:
>> +/* Intercept CPU hotplug events for syfs purposes */
>> +static int mips_rproc_callback(struct notifier_block *nfb, unsigned long action,
>> +			       void *hcpu)
>> +{
> Please convert to cpu hotplug state machine.

OK, I've done that for this and the MIPS GIC patch, using the dynamic 
CPUHP_AP_ONLINE_DYN state - I hope that is ok.

>
>> +	unsigned int cpu = (unsigned long)hcpu;
>> +
>> +	switch (action) {
>> +	case CPU_UP_PREPARE:
>> +	case CPU_DOWN_FAILED:
>> +		mips_rproc_device_unregister(cpu);
>> +		break;
>> +	case CPU_DOWN_PREPARE:
>> +		mips_rproc_device_register(cpu);
>> +		break;
>> +	}
> There is no reason why you need to setup the rproc device on
> DOWN_PREPARE. It's sufficient to do that when the CPU is dead, so you can
> use a symetric callback prep/dead.

Sure, the new state machine makes this much nicer registering the 
on/offline callbacks on one state.

>
>> +	/* Dynamically create mips-rproc class devices based on hotplug data */
>> +	get_online_cpus();
>> +	for_each_possible_cpu(cpu)
>> +		if (!cpu_online(cpu))
>> +			mips_rproc_device_register(cpu);
>> +	register_hotcpu_notifier(&mips_rproc_notifier);
>> +	put_online_cpus();
> Perhaps we should add support for "reverse" functionality to the state
> machine core. I'll have a look later how hard that'd be.

Yeah - I've had to work around the framework a little here since we 
require the opposite sense to the callbacks - activate the driver when 
the cpu is offlined and vice versa. In practice the only issue this gave 
me was that by default the framework invokes the teardown callback when 
the state is removed, so I used __cpuhp_remove_state() to avoid creating 
a remote processor device as the driver is being removed.

Thanks,
Matt

>
> Thanks,
>
> 	tglx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/6] MIPS: tlb-r4k: If there are wired entries, don't use TLBINVF
  2016-09-20  8:47 ` [PATCH v2 2/6] MIPS: tlb-r4k: If there are wired entries, don't use TLBINVF Matt Redfearn
@ 2016-09-22 12:11   ` Ralf Baechle
  0 siblings, 0 replies; 17+ messages in thread
From: Ralf Baechle @ 2016-09-22 12:11 UTC (permalink / raw)
  To: Matt Redfearn
  Cc: Bjorn Andersson, Ohad Ben-Cohen, Thomas Gleixner, linux-mips,
	linux-remoteproc, lisa.parratt, linux-kernel, Hugh Dickins,
	Huacai Chen, David S. Miller, James Hogan, Paul Burton,
	Andrew Morton

On Tue, Sep 20, 2016 at 09:47:25AM +0100, Matt Redfearn wrote:

> When adding a wired entry to the TLB via add_wired_entry, the tlb is
> flushed with local_flush_tlb_all, which on CPUs with TLBINV results in
> the new wired entry being flushed again.
> 
> Behavior of the TLBINV instruction applies to all applicable TLB entries
> and is unaffected by the setting of the Wired register. Therefore if
> the TLB has any wired entries, fall back to iterating over the entries
> rather than blasting them all using TLBINVF.
> 
> Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com>

Queued for 4.9..

  Ralf

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 5/6] remoteproc/MIPS: Add a remoteproc driver for MIPS
  2016-09-20  8:47 ` [PATCH v2 5/6] remoteproc/MIPS: Add a remoteproc driver for MIPS Matt Redfearn
  2016-09-20  9:47   ` Thomas Gleixner
@ 2016-10-03  8:35   ` Matt Redfearn
  2016-10-03 22:16   ` Bjorn Andersson
  2 siblings, 0 replies; 17+ messages in thread
From: Matt Redfearn @ 2016-10-03  8:35 UTC (permalink / raw)
  To: Ralf Baechle, Bjorn Andersson, Ohad Ben-Cohen, Thomas Gleixner
  Cc: linux-mips, linux-remoteproc, lisa.parratt, linux-kernel

Hi Bjorn / Ohad,

Please could you give some feedback on this patch?

Thanks,

Matt


On 20/09/16 09:47, Matt Redfearn wrote:
> Add a remoteproc driver to steal, load the firmware, and boot an offline
> MIPS core, turning it into a coprocessor.
>
> This driver provides a sysfs to allow arbitrary firmware to be loaded
> onto a core, which may expose virtio devices. Coprocessor firmware must
> abide by the UHI coprocessor boot protocol.
>
> Signed-off-by: Lisa Parratt <lisa.parratt@imgtec.com>
> Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com>
>
> ---
>
> Changes in v2: None
>
>   Documentation/ABI/testing/sysfs-class-mips-rproc |  24 +
>   drivers/remoteproc/Kconfig                       |  11 +
>   drivers/remoteproc/Makefile                      |   1 +
>   drivers/remoteproc/mips_remoteproc.c             | 651 +++++++++++++++++++++++
>   4 files changed, 687 insertions(+)
>   create mode 100644 Documentation/ABI/testing/sysfs-class-mips-rproc
>   create mode 100644 drivers/remoteproc/mips_remoteproc.c
>
> diff --git a/Documentation/ABI/testing/sysfs-class-mips-rproc b/Documentation/ABI/testing/sysfs-class-mips-rproc
> new file mode 100644
> index 000000000000..c09d02a755e4
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-mips-rproc
> @@ -0,0 +1,24 @@
> +What:		/sys/class/mips-rproc/rproc#/firmware
> +Date:		August 2015
> +Contact:	Lisa Parratt <lisa.parratt@imgtec.com>
> +Description:	MIPS VPE remoteproc start
> +
> +	This node only exists when a VPE is considered offline by Linux. Writes
> +	to this file will start firmware running on a VPE.
> +
> +	If the VPE is idle, specifying a name will cause a remoteproc instance
> +	to be allocated, which will cause the core to be stolen, the firmware
> +	image to be loaded, and the remoteproc instance to be started.
> +	Otherwise, the operation will fail.
> +
> +What:		/sys/class/mips-rproc/rproc#/stop
> +Date:		August 2015
> +Contact:	Lisa Parratt <lisa.parratt@imgtec.com>
> +Description:	MIPS VPE remoteproc stop
> +
> +	This node only exists when a VPE is considered offline by Linux. Writes
> +	to this file will stop firmware running on a VPE.
> +
> +	If the VPE is running a remote proc instance, the instance will be
> +	stopped, the core returned, and the instance freed.
> +	Otherwise, the operation will fail.
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index 1a8bf76a925f..05db52e0e668 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -100,4 +100,15 @@ config ST_REMOTEPROC
>   	  processor framework.
>   	  This can be either built-in or a loadable module.
>   
> +config MIPS_REMOTEPROC
> +	tristate "MIPS remoteproc support"
> +	depends on MIPS_CPS && HAS_DMA
> +	select CMA
> +	select REMOTEPROC
> +	select MIPS_STEAL
> +	help
> +	  Say y here to support using offline cores/VPEs as remote processors
> +	  via the remote processor framework.
> +	  If unsure say N.
> +
>   endmenu
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index 92d3758bd15c..de19cd320f3a 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -14,3 +14,4 @@ obj-$(CONFIG_DA8XX_REMOTEPROC)		+= da8xx_remoteproc.o
>   obj-$(CONFIG_QCOM_MDT_LOADER)		+= qcom_mdt_loader.o
>   obj-$(CONFIG_QCOM_Q6V5_PIL)		+= qcom_q6v5_pil.o
>   obj-$(CONFIG_ST_REMOTEPROC)		+= st_remoteproc.o
> +obj-$(CONFIG_MIPS_REMOTEPROC)		+= mips_remoteproc.o
> diff --git a/drivers/remoteproc/mips_remoteproc.c b/drivers/remoteproc/mips_remoteproc.c
> new file mode 100644
> index 000000000000..944ad66280b4
> --- /dev/null
> +++ b/drivers/remoteproc/mips_remoteproc.c
> @@ -0,0 +1,651 @@
> +/*
> + * MIPS Remote Processor driver
> + *
> + * Copyright (C) 2016 Imagination Technologies
> + * Lisa Parratt <lisa.parratt@imgtec.com>
> + * Matt Redfearn <matt.redfearn@imgtec.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation;  either version 2 of the  License, or (at your
> + * option) any later version.
> + */
> +
> +#include <linux/cpu.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/irq.h>
> +#include <linux/module.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/remoteproc.h>
> +
> +#include <asm/dma-coherence.h>
> +#include <asm/smp-cps.h>
> +#include <asm/tlbflush.h>
> +#include <asm/tlbmisc.h>
> +
> +#include "remoteproc_internal.h"
> +
> +struct mips_rproc {
> +	struct rproc		*rproc;
> +	char			*firmware;
> +	struct task_struct	*tsk;
> +	struct device		dev;
> +	unsigned int		cpu;
> +	int			ipi_linux;
> +	int			ipi_remote;
> +};
> +
> +static struct rproc *mips_rprocs[NR_CPUS];
> +
> +#define to_mips_rproc(d) container_of(d, struct mips_rproc, dev)
> +
> +
> +/* Compute the largest page mask a physical address can be mapped with */
> +static unsigned long mips_rproc_largest_pm(unsigned long pa,
> +					   unsigned long maxmask)
> +{
> +	unsigned long mask;
> +	/* Find address bits limiting alignment */
> +	unsigned long shift = ffs(pa);
> +
> +	/* Obey MIPS restrictions on page sizes */
> +	if (pa) {
> +		if (shift & 1)
> +			shift -= 2;
> +		else
> +			shift--;
> +	}
> +	mask = ULONG_MAX << shift;
> +	return maxmask & ~mask;
> +}
> +
> +/* Compute the next largest page mask for a given page mask */
> +static unsigned long mips_rproc_next_pm(unsigned long pm, unsigned long maxmask)
> +{
> +	if (pm != PM_4K)
> +		return ((pm << 2) | pm) & maxmask;
> +	else
> +		return PM_16K;
> +}
> +
> +static void mips_map_page(unsigned long da, unsigned long pa, int c,
> +			  unsigned long pagemask, unsigned long pagesize)
> +{
> +	unsigned long pa2 = pa + (pagesize / 2);
> +	unsigned long entryhi, entrylo0, entrylo1;
> +
> +	/* Compute the mapping */
> +	pa = (pa >> 6) & (ULONG_MAX << MIPS_ENTRYLO_PFN_SHIFT);
> +	pa2 = (pa2 >> 6) & (ULONG_MAX << MIPS_ENTRYLO_PFN_SHIFT);
> +	entryhi = da & 0xfffffe000;
> +	entrylo0 = (c << ENTRYLO_C_SHIFT) | ENTRYLO_D | ENTRYLO_V | pa;
> +	entrylo1 = (c << ENTRYLO_C_SHIFT) | ENTRYLO_D | ENTRYLO_V | pa2;
> +
> +	pr_debug("Create wired entry %d, CCA %d\n", read_c0_wired(), c);
> +	pr_debug(" EntryHi: 0x%016lx\n", entryhi);
> +	pr_debug(" EntryLo0: 0x%016lx\n", entrylo0);
> +	pr_debug(" EntryLo1: 0x%016lx\n", entrylo1);
> +	pr_debug(" Pagemask: 0x%016lx\n", pagemask);
> +	pr_debug("\n");
> +
> +	add_wired_entry(entrylo0, entrylo1, entryhi, pagemask);
> +}
> +
> +/*
> + * Compute the page required to fulfill a mapping. Escapes alignment derived
> + * page size limitations before using biggest fit to map the remainder.
> + */
> +static inline void mips_rproc_fit_page(unsigned long da, unsigned long pa,
> +					int c, unsigned long size,
> +					unsigned long maxmask)
> +{
> +	unsigned long bigmask, nextmask;
> +	unsigned long pagemask, pagesize;
> +	unsigned long distance, target;
> +
> +	do {
> +		/* Compute the current largest page mask */
> +		bigmask = mips_rproc_largest_pm(pa, maxmask);
> +		/* Compute the next largest pagesize */
> +		nextmask = mips_rproc_next_pm(bigmask, maxmask);
> +		/*
> +		 * Compute the distance from our current physical address to
> +		 * the next page boundary.
> +		 */
> +		distance = (nextmask + 0x2000) - (pa & nextmask);
> +		/*
> +		 * Decide between searching to get to the next highest page
> +		 * boundary or finishing.
> +		 */
> +		target = distance < size ? distance : size;
> +		/* Fit */
> +		while (target) {
> +			/* Find the largest supported page size that will fit */
> +			for (pagesize = maxmask + 0x2000;
> +			     (pagesize > 0x2000) && (pagesize > target);
> +			     pagesize /= 4) {
> +			}
> +			/* Convert it to a page mask */
> +			pagemask = pagesize - 0x2000;
> +			/* Emit it */
> +			mips_map_page(da, pa, c, pagemask, pagesize);
> +			/* Move to next step */
> +			size -= pagesize;
> +			da += pagesize;
> +			pa += pagesize;
> +			target -= pagesize;
> +		}
> +	} while (size);
> +}
> +
> +
> +static int mips_rproc_carveouts(struct rproc *rproc, int max_pagemask)
> +{
> +	struct rproc_mem_entry *carveout;
> +
> +	list_for_each_entry(carveout, &rproc->carveouts, node) {
> +		int c = CONF_CM_CACHABLE_COW;
> +
> +		dev_dbg(&rproc->dev,
> +			"carveout mapping da 0x%x -> %pad length 0x%x, CCA %d",
> +			carveout->da, &carveout->dma, carveout->len, c);
> +
> +		mips_rproc_fit_page(carveout->da, carveout->dma, c,
> +				    carveout->len, max_pagemask);
> +	}
> +	return 0;
> +}
> +
> +
> +static int mips_rproc_vdevs(struct rproc *rproc, int max_pagemask)
> +{
> +	struct rproc_vdev *rvdev;
> +
> +	list_for_each_entry(rvdev, &rproc->rvdevs, node) {
> +		int i, size;
> +
> +		for (i = 0; i < ARRAY_SIZE(rvdev->vring); i++) {
> +			struct rproc_vring *vring = &rvdev->vring[i];
> +			unsigned long pa = vring->dma;
> +			int c;
> +
> +			if (hw_coherentio) {
> +				/*
> +				 * The DMA API will allocate cacheable buffers
> +				 * for shared resources, so the firmware should
> +				 * also access those buffers cached
> +				 */
> +				c = (_page_cachable_default >> _CACHE_SHIFT);
> +			} else {
> +				/*
> +				 * Otherwise, shared buffers should be accessed
> +				 * uncached
> +				 */
> +				c = CONF_CM_UNCACHED;
> +			}
> +
> +			/* actual size of vring (in bytes) */
> +			size = PAGE_ALIGN(vring_size(vring->len, vring->align));
> +
> +			dev_dbg(&rproc->dev,
> +				"vring mapping da %pad -> %pad length 0x%x, CCA %d",
> +				&vring->dma, &vring->dma, size, c);
> +
> +			mips_rproc_fit_page(pa, pa, c, size, max_pagemask);
> +		}
> +	}
> +	return 0;
> +}
> +
> +static void mips_rproc_cpu_entry(void)
> +{
> +	struct rproc *rproc = mips_rprocs[smp_processor_id()];
> +	struct mips_rproc *mproc = *(struct mips_rproc **)rproc->priv;
> +	int ipi_to_remote = ipi_get_hwirq(mproc->ipi_remote, mproc->cpu);
> +	int ipi_from_remote = ipi_get_hwirq(mproc->ipi_linux, 0);
> +	unsigned long old_pagemask, max_pagemask;
> +
> +	if (!rproc)
> +		return;
> +
> +	dev_info(&rproc->dev, "Starting %s on MIPS CPU%d\n",
> +		 mproc->firmware, mproc->cpu);
> +
> +	/* Get the maximum pagemask supported on this CPU */
> +	old_pagemask = read_c0_pagemask();
> +	write_c0_pagemask(PM_HUGE_MASK);
> +	mtc0_tlbw_hazard();
> +	max_pagemask = read_c0_pagemask();
> +	write_c0_pagemask(old_pagemask);
> +	mtc0_tlbw_hazard();
> +
> +	/* Start with no wired entries */
> +	write_c0_wired(0);
> +
> +	/* Flush all previous TLB entries */
> +	local_flush_tlb_all();
> +
> +	/* Map firmware resources into virtual memory */
> +	mips_rproc_carveouts(rproc, max_pagemask);
> +	mips_rproc_vdevs(rproc, max_pagemask);
> +
> +	dev_dbg(&rproc->dev, "IPI to remote: %d\n", ipi_to_remote);
> +	dev_dbg(&rproc->dev, "IPI from remote: %d\n", ipi_from_remote);
> +
> +	/* Hand off the CPU to the firmware */
> +	dev_dbg(&rproc->dev, "Jumping to firmware at 0x%x\n", rproc->bootaddr);
> +
> +	write_c0_entryhi(0); /* Set ASID 0 */
> +	tlbw_use_hazard();
> +
> +	/* Firmware protocol */
> +	__asm__("addiu $a0, $zero, -3");
> +	__asm__("move $a1, %0" :: "r" (ipi_to_remote));
> +	__asm__("move $a2, %0" :: "r" (ipi_from_remote));
> +	__asm__("move $a3, $zero");
> +	__asm__("jr %0" :: "r" (rproc->bootaddr));
> +}
> +
> +
> +static irqreturn_t mips_rproc_ipi_handler(int irq, void *dev_id)
> +{
> +	/* Synthetic interrupts shouldn't need acking */
> +	return IRQ_WAKE_THREAD;
> +}
> +
> +
> +static irqreturn_t mips_rproc_vq_int(int irq, void *p)
> +{
> +	struct rproc *rproc = (struct rproc *)p;
> +	void *entry;
> +	int id;
> +
> +	/* We don't have a mailbox, so iterate over all vqs and kick them. */
> +	idr_for_each_entry(&rproc->notifyids, entry, id)
> +		rproc_vq_interrupt(rproc, id);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +
> +/* Helper function to find the IPI domain */
> +static struct irq_domain *ipi_domain(void)
> +{
> +	struct device_node *node = of_irq_find_parent(of_root);
> +	struct irq_domain *ipidomain;
> +
> +	ipidomain = irq_find_matching_host(node, DOMAIN_BUS_IPI);
> +	/*
> +	 * Some platforms have half DT setup. So if we found irq node but
> +	 * didn't find an ipidomain, try to search for one that is not in the
> +	 * DT.
> +	 */
> +	if (node && !ipidomain)
> +		ipidomain = irq_find_matching_host(NULL, DOMAIN_BUS_IPI);
> +
> +	return ipidomain;
> +}
> +
> +
> +int mips_rproc_op_start(struct rproc *rproc)
> +{
> +	struct mips_rproc *mproc = *(struct mips_rproc **)rproc->priv;
> +	int err;
> +	int cpu = mproc->cpu;
> +
> +	if (mips_rprocs[cpu]) {
> +		dev_err(&rproc->dev, "CPU%d in use\n", cpu);
> +		return -EBUSY;
> +	}
> +	mips_rprocs[cpu] = rproc;
> +
> +	/* Create task for the CPU to use before handing off to firmware */
> +	mproc->tsk = fork_idle(cpu);
> +	if (IS_ERR(mproc->tsk)) {
> +		dev_err(&rproc->dev, "fork_idle() failed for CPU%d\n", cpu);
> +		return -ENOMEM;
> +	}
> +
> +	/* We won't be needing the Linux IPIs anymore */
> +	if (mips_smp_ipi_free(get_cpu_mask(cpu)))
> +		return -EINVAL;
> +
> +	/*
> +	 * Direct IPIs from the remote processor to CPU0 since that can't be
> +	 * offlined while the remote CPU is running.
> +	 */
> +	mproc->ipi_linux = irq_reserve_ipi(ipi_domain(), get_cpu_mask(0));
> +	if (!mproc->ipi_linux) {
> +		dev_err(&mproc->dev, "Failed to reserve incoming kick\n");
> +		goto exit_rproc_nofrom;
> +	}
> +
> +	mproc->ipi_remote = irq_reserve_ipi(ipi_domain(), get_cpu_mask(cpu));
> +	if (!mproc->ipi_remote) {
> +		dev_err(&mproc->dev, "Failed to reserve outgoing kick\n");
> +		goto exit_rproc_noto;
> +	}
> +
> +	/* register incoming ipi */
> +	err = devm_request_threaded_irq(&mproc->dev, mproc->ipi_linux,
> +					mips_rproc_ipi_handler,
> +					mips_rproc_vq_int, 0,
> +					"mips-rproc IPI in", mproc->rproc);
> +	if (err) {
> +		dev_err(&mproc->dev, "Failed to register incoming kick: %d\n",
> +			err);
> +		goto exit_rproc_noint;
> +	}
> +
> +	if (!mips_cps_steal_cpu_and_execute(cpu, &mips_rproc_cpu_entry,
> +						mproc->tsk))
> +		return 0;
> +
> +	dev_err(&mproc->dev, "Failed to steal CPU%d for remote\n", cpu);
> +	devm_free_irq(&mproc->dev, mproc->ipi_linux, mproc->rproc);
> +exit_rproc_noint:
> +	irq_destroy_ipi(mproc->ipi_remote, get_cpu_mask(cpu));
> +exit_rproc_noto:
> +	irq_destroy_ipi(mproc->ipi_linux, get_cpu_mask(0));
> +exit_rproc_nofrom:
> +	free_task(mproc->tsk);
> +	mips_rprocs[cpu] = NULL;
> +
> +	/* Set up the Linux IPIs again */
> +	mips_smp_ipi_allocate(get_cpu_mask(cpu));
> +	return -EINVAL;
> +}
> +
> +int mips_rproc_op_stop(struct rproc *rproc)
> +{
> +	struct mips_rproc *mproc = *(struct mips_rproc **)rproc->priv;
> +
> +	if (mproc->ipi_linux)
> +		devm_free_irq(&mproc->dev, mproc->ipi_linux, mproc->rproc);
> +
> +	irq_destroy_ipi(mproc->ipi_linux, get_cpu_mask(0));
> +	irq_destroy_ipi(mproc->ipi_remote, get_cpu_mask(mproc->cpu));
> +
> +	/* Set up the Linux IPIs again */
> +	mips_smp_ipi_allocate(get_cpu_mask(mproc->cpu));
> +
> +	free_task(mproc->tsk);
> +
> +	mips_rprocs[mproc->cpu] = NULL;
> +
> +	return mips_cps_halt_and_return_cpu(mproc->cpu);
> +}
> +
> +
> +void mips_rproc_op_kick(struct rproc *rproc, int vqid)
> +{
> +	struct mips_rproc *mproc = *(struct mips_rproc **)rproc->priv;
> +
> +	ipi_send_single(mproc->ipi_remote, mproc->cpu);
> +}
> +
> +struct rproc_ops mips_rproc_proc_ops = {
> +	.start	= mips_rproc_op_start,
> +	.stop	= mips_rproc_op_stop,
> +	.kick	= mips_rproc_op_kick,
> +};
> +
> +
> +static int mips_rproc_probe(struct platform_device *pdev)
> +{
> +	return 0;
> +}
> +
> +static int mips_rproc_remove(struct platform_device *pdev)
> +{
> +	return 0;
> +}
> +
> +static struct platform_driver mips_rproc_driver = {
> +	.probe = mips_rproc_probe,
> +	.remove = mips_rproc_remove,
> +	.driver = {
> +		.name = "mips-rproc"
> +	},
> +};
> +
> +
> +/* Steal a core and run some firmware on it */
> +int mips_rproc_start(struct mips_rproc *mproc, const char *firmware, size_t len)
> +{
> +	int err = -EINVAL;
> +	struct mips_rproc **priv;
> +
> +	/* Duplicate the filename, dropping whitespace from the end via len */
> +	mproc->firmware = kstrndup(firmware, len, GFP_KERNEL);
> +	if (!mproc->firmware)
> +		return -ENOMEM;
> +
> +	mproc->rproc = rproc_alloc(&mproc->dev, "mips", &mips_rproc_proc_ops,
> +				   mproc->firmware,
> +				   sizeof(struct mips_rproc *));
> +	if (!mproc->rproc)
> +		return -ENOMEM;
> +
> +	priv = mproc->rproc->priv;
> +	*priv = mproc;
> +
> +	/* go live! */
> +	err = rproc_add(mproc->rproc);
> +	if (err) {
> +		dev_err(&mproc->dev, "Failed to add rproc: %d\n", err);
> +		rproc_put(mproc->rproc);
> +		kfree(mproc->firmware);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +/* Stop a core, and return it to being offline */
> +int mips_rproc_stop(struct mips_rproc *mproc)
> +{
> +	rproc_shutdown(mproc->rproc);
> +	rproc_del(mproc->rproc);
> +	rproc_put(mproc->rproc);
> +	mproc->rproc = NULL;
> +	return 0;
> +}
> +
> +/* sysfs interface to mips_rproc_start */
> +static ssize_t firmware_store(struct device *dev,
> +			      struct device_attribute *attr,
> +			      const char *buf, size_t count)
> +{
> +	struct mips_rproc *mproc = to_mips_rproc(dev);
> +	size_t len = count;
> +	int err = -EINVAL;
> +
> +	if (buf[count - 1] == '\n')
> +		len--;
> +
> +	if (!mproc->rproc && len)
> +		err = mips_rproc_start(mproc, buf, len);
> +	else if (len)
> +		err = -EBUSY;
> +
> +	return err ? err : count;
> +}
> +static DEVICE_ATTR_WO(firmware);
> +
> +/* sysfs interface to mips_rproc_stop */
> +static ssize_t stop_store(struct device *dev,
> +			      struct device_attribute *attr,
> +			      const char *buf, size_t count)
> +{
> +	struct mips_rproc *mproc = to_mips_rproc(dev);
> +	int err = -EINVAL;
> +
> +
> +	if (mproc->rproc)
> +		err = mips_rproc_stop(mproc);
> +	else
> +		err = -EBUSY;
> +
> +	return err ? err : count;
> +}
> +static DEVICE_ATTR_WO(stop);
> +
> +/* Boiler plate for devclarng mips-rproc sysfs devices */
> +static struct attribute *mips_rproc_attrs[] = {
> +	&dev_attr_firmware.attr,
> +	&dev_attr_stop.attr,
> +	NULL
> +};
> +
> +static struct attribute_group mips_rproc_devgroup = {
> +	.attrs = mips_rproc_attrs
> +};
> +
> +static const struct attribute_group *mips_rproc_devgroups[] = {
> +	&mips_rproc_devgroup,
> +	NULL
> +};
> +
> +static char *mips_rproc_devnode(struct device *dev, umode_t *mode)
> +{
> +	return kasprintf(GFP_KERNEL, "mips-rproc/%s", dev_name(dev));
> +}
> +
> +static struct class mips_rproc_class = {
> +	.name		= "mips-rproc",
> +	.devnode	= mips_rproc_devnode,
> +	.dev_groups	= mips_rproc_devgroups,
> +};
> +
> +static void mips_rproc_release(struct device *dev)
> +{
> +}
> +
> +static int mips_rproc_uevent(struct device *dev, struct kobj_uevent_env *env)
> +{
> +	struct mips_rproc *mproc = to_mips_rproc(dev);
> +
> +	if (!mproc)
> +		return -ENODEV;
> +
> +	return 0;
> +}
> +
> +static struct device_type mips_rproc_type = {
> +	.release	= mips_rproc_release,
> +	.uevent		= mips_rproc_uevent
> +};
> +
> +/* Helper function for locating the device for a CPU */
> +int mips_rproc_device_rproc_match(struct device *dev, const void *data)
> +{
> +	struct mips_rproc *mproc = to_mips_rproc(dev);
> +	unsigned int cpu = *(unsigned int *)data;
> +
> +	return mproc->cpu == cpu;
> +}
> +
> +/* Create a sysfs device in response to CPU down */
> +int mips_rproc_device_register(unsigned int cpu)
> +{
> +	struct mips_rproc *dev;
> +
> +	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> +	if (!dev)
> +		return -EINVAL;
> +
> +	dev->dev.driver = &mips_rproc_driver.driver;
> +	dev->dev.type = &mips_rproc_type;
> +	dev->dev.class = &mips_rproc_class;
> +	dev->dev.id = cpu;
> +	dev_set_name(&dev->dev, "rproc%u", cpu);
> +	dev->cpu = cpu;
> +
> +	return device_register(&dev->dev);
> +}
> +
> +/* Destroy a sysfs device in response to CPU up */
> +int mips_rproc_device_unregister(unsigned int cpu)
> +{
> +	struct device *dev = class_find_device(&mips_rproc_class, NULL, &cpu,
> +					       mips_rproc_device_rproc_match);
> +	struct mips_rproc *mproc = to_mips_rproc(dev);
> +
> +	if (mips_rprocs[cpu])
> +		mips_rproc_stop(mproc);
> +
> +	put_device(dev);
> +	device_unregister(dev);
> +	kfree(mproc);
> +	return 0;
> +}
> +
> +/* Intercept CPU hotplug events for syfs purposes */
> +static int mips_rproc_callback(struct notifier_block *nfb, unsigned long action,
> +			       void *hcpu)
> +{
> +	unsigned int cpu = (unsigned long)hcpu;
> +
> +	switch (action) {
> +	case CPU_UP_PREPARE:
> +	case CPU_DOWN_FAILED:
> +		mips_rproc_device_unregister(cpu);
> +		break;
> +	case CPU_DOWN_PREPARE:
> +		mips_rproc_device_register(cpu);
> +		break;
> +	}
> +
> +	return NOTIFY_OK;
> +}
> +
> +static struct notifier_block mips_rproc_notifier __refdata = {
> +	.notifier_call = mips_rproc_callback
> +};
> +
> +static int __init mips_rproc_init(void)
> +{
> +	int cpu;
> +	/* create mips-rproc device class for sysfs */
> +	int err = class_register(&mips_rproc_class);
> +
> +	if (err) {
> +		pr_err("mips-proc: unable to register mips-rproc class\n");
> +		return err;
> +	}
> +
> +	/* Dynamically create mips-rproc class devices based on hotplug data */
> +	get_online_cpus();
> +	for_each_possible_cpu(cpu)
> +		if (!cpu_online(cpu))
> +			mips_rproc_device_register(cpu);
> +	register_hotcpu_notifier(&mips_rproc_notifier);
> +	put_online_cpus();
> +
> +	return 0;
> +}
> +
> +static void __exit mips_rproc_exit(void)
> +{
> +	int cpu;
> +	/* Destroy mips-rproc class devices */
> +	get_online_cpus();
> +	unregister_hotcpu_notifier(&mips_rproc_notifier);
> +	for_each_possible_cpu(cpu)
> +		if (!cpu_online(cpu))
> +			mips_rproc_device_unregister(cpu);
> +	put_online_cpus();
> +
> +	class_unregister(&mips_rproc_class);
> +}
> +
> +subsys_initcall(mips_rproc_init);
> +module_exit(mips_rproc_exit);
> +
> +module_platform_driver(mips_rproc_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("MIPS Remote Processor control driver");

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 5/6] remoteproc/MIPS: Add a remoteproc driver for MIPS
  2016-09-20  8:47 ` [PATCH v2 5/6] remoteproc/MIPS: Add a remoteproc driver for MIPS Matt Redfearn
  2016-09-20  9:47   ` Thomas Gleixner
  2016-10-03  8:35   ` Matt Redfearn
@ 2016-10-03 22:16   ` Bjorn Andersson
  2016-10-05 16:22     ` Matt Redfearn
  2 siblings, 1 reply; 17+ messages in thread
From: Bjorn Andersson @ 2016-10-03 22:16 UTC (permalink / raw)
  To: Matt Redfearn
  Cc: Ralf Baechle, Ohad Ben-Cohen, Thomas Gleixner, linux-mips,
	linux-remoteproc, lisa.parratt, linux-kernel

On Tue 20 Sep 01:47 PDT 2016, Matt Redfearn wrote:

> Add a remoteproc driver to steal, load the firmware, and boot an offline
> MIPS core, turning it into a coprocessor.
> 
> This driver provides a sysfs to allow arbitrary firmware to be loaded
> onto a core, which may expose virtio devices. Coprocessor firmware must
> abide by the UHI coprocessor boot protocol.

Hi Matt,

Sorry for my very slow response, I kept getting side tracked on the
sysfs part every time I attempted to review this. After discussing with
others it's obvious that being able to boot a remoteproc with a specific
firmware image for some amount of time is a very common request.

Rather than adding a MIPS specific interface for controlling this rproc
I would like for us to bring this to the core.


I would also appreciate if Ralf had some input on the MIPS specifics.


Also regarding the 32-bit requirement, have you investigated what is
needed to support 64-bit ELFs?

[..]
> diff --git a/drivers/remoteproc/mips_remoteproc.c b/drivers/remoteproc/mips_remoteproc.c
[..]
> +int mips_rproc_op_start(struct rproc *rproc)
> +{
> +	struct mips_rproc *mproc = *(struct mips_rproc **)rproc->priv;
> +	int err;
> +	int cpu = mproc->cpu;
> +
> +	if (mips_rprocs[cpu]) {
> +		dev_err(&rproc->dev, "CPU%d in use\n", cpu);
> +		return -EBUSY;
> +	}
> +	mips_rprocs[cpu] = rproc;
> +
> +	/* Create task for the CPU to use before handing off to firmware */
> +	mproc->tsk = fork_idle(cpu);
> +	if (IS_ERR(mproc->tsk)) {
> +		dev_err(&rproc->dev, "fork_idle() failed for CPU%d\n", cpu);
> +		return -ENOMEM;
> +	}
> +
> +	/* We won't be needing the Linux IPIs anymore */
> +	if (mips_smp_ipi_free(get_cpu_mask(cpu)))
> +		return -EINVAL;
> +
> +	/*
> +	 * Direct IPIs from the remote processor to CPU0 since that can't be
> +	 * offlined while the remote CPU is running.
> +	 */
> +	mproc->ipi_linux = irq_reserve_ipi(ipi_domain(), get_cpu_mask(0));
> +	if (!mproc->ipi_linux) {
> +		dev_err(&mproc->dev, "Failed to reserve incoming kick\n");
> +		goto exit_rproc_nofrom;
> +	}
> +
> +	mproc->ipi_remote = irq_reserve_ipi(ipi_domain(), get_cpu_mask(cpu));
> +	if (!mproc->ipi_remote) {
> +		dev_err(&mproc->dev, "Failed to reserve outgoing kick\n");
> +		goto exit_rproc_noto;
> +	}
> +
> +	/* register incoming ipi */
> +	err = devm_request_threaded_irq(&mproc->dev, mproc->ipi_linux,
> +					mips_rproc_ipi_handler,
> +					mips_rproc_vq_int, 0,
> +					"mips-rproc IPI in", mproc->rproc);

Based on how you've designed this I think it makes sense to just depend
on the fact that stop() will always be called and hence you do not
benefit from the devm_ version of this api.

> +	if (err) {
> +		dev_err(&mproc->dev, "Failed to register incoming kick: %d\n",
> +			err);
> +		goto exit_rproc_noint;
> +	}
> +
> +	if (!mips_cps_steal_cpu_and_execute(cpu, &mips_rproc_cpu_entry,
> +						mproc->tsk))
> +		return 0;

Please flip this around, to follow the pattern of the others, like:

	if (mips_cps_steal_cpu_and_execute()) {
		dev_err()
		goto exit_free_irq;
	}

	return 0;

exit_free_irq:

> +
> +	dev_err(&mproc->dev, "Failed to steal CPU%d for remote\n", cpu);
> +	devm_free_irq(&mproc->dev, mproc->ipi_linux, mproc->rproc);
> +exit_rproc_noint:
> +	irq_destroy_ipi(mproc->ipi_remote, get_cpu_mask(cpu));
> +exit_rproc_noto:
> +	irq_destroy_ipi(mproc->ipi_linux, get_cpu_mask(0));
> +exit_rproc_nofrom:
> +	free_task(mproc->tsk);
> +	mips_rprocs[cpu] = NULL;
> +
> +	/* Set up the Linux IPIs again */
> +	mips_smp_ipi_allocate(get_cpu_mask(cpu));
> +	return -EINVAL;
> +}
> +
> +int mips_rproc_op_stop(struct rproc *rproc)
> +{
> +	struct mips_rproc *mproc = *(struct mips_rproc **)rproc->priv;
> +
> +	if (mproc->ipi_linux)

stop() should not be called unless start() succeeded, so ipi_linux
should not be able to be 0.

> +		devm_free_irq(&mproc->dev, mproc->ipi_linux, mproc->rproc);
> +
> +	irq_destroy_ipi(mproc->ipi_linux, get_cpu_mask(0));
> +	irq_destroy_ipi(mproc->ipi_remote, get_cpu_mask(mproc->cpu));
> +
> +	/* Set up the Linux IPIs again */
> +	mips_smp_ipi_allocate(get_cpu_mask(mproc->cpu));
> +
> +	free_task(mproc->tsk);
> +
> +	mips_rprocs[mproc->cpu] = NULL;
> +
> +	return mips_cps_halt_and_return_cpu(mproc->cpu);
> +}
> +
> +
[..]
> +
> +/* Steal a core and run some firmware on it */
> +int mips_rproc_start(struct mips_rproc *mproc, const char *firmware, size_t len)
> +{
> +	int err = -EINVAL;
> +	struct mips_rproc **priv;
> +
> +	/* Duplicate the filename, dropping whitespace from the end via len */
> +	mproc->firmware = kstrndup(firmware, len, GFP_KERNEL);
> +	if (!mproc->firmware)
> +		return -ENOMEM;
> +
> +	mproc->rproc = rproc_alloc(&mproc->dev, "mips", &mips_rproc_proc_ops,
> +				   mproc->firmware,
> +				   sizeof(struct mips_rproc *));
> +	if (!mproc->rproc)
> +		return -ENOMEM;
> +
> +	priv = mproc->rproc->priv;
> +	*priv = mproc;

If we move the class into the core, everyone will share the same
interface for setting firmware, booting and shutting down remoteproc.

I think you should set rproc->auto_boot to false and move this code into
the probe function above. It would make there be an remoteproc instance
whenever the cpu is offlined, do you see any problems with this?

> +
> +	/* go live! */
> +	err = rproc_add(mproc->rproc);
> +	if (err) {
> +		dev_err(&mproc->dev, "Failed to add rproc: %d\n", err);
> +		rproc_put(mproc->rproc);
> +		kfree(mproc->firmware);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +/* Stop a core, and return it to being offline */
> +int mips_rproc_stop(struct mips_rproc *mproc)
> +{
> +	rproc_shutdown(mproc->rproc);

I presume this shutdown is related to the implicit boot happening in
rproc_add() if you have virtio devices; I've changed this for v4.9 so
that rproc_del() shuts down the core if rproc_add() booted it.

There needs to be some more work done in this area though, because there
are plenty of corner cases that we don't handle properly today...

> +	rproc_del(mproc->rproc);
> +	rproc_put(mproc->rproc);
> +	mproc->rproc = NULL;
> +	return 0;
> +}
> +
[..]
> +
> +/* sysfs interface to mips_rproc_stop */
> +static ssize_t stop_store(struct device *dev,
> +			      struct device_attribute *attr,
> +			      const char *buf, size_t count)
> +{
> +	struct mips_rproc *mproc = to_mips_rproc(dev);
> +	int err = -EINVAL;
> +
> +
> +	if (mproc->rproc)
> +		err = mips_rproc_stop(mproc);
> +	else
> +		err = -EBUSY;
> +
> +	return err ? err : count;
> +}
> +static DEVICE_ATTR_WO(stop);

Please move this control into the core, preferably as "state" which can
be passed "boot" or "shutdown" - i.e. what we today have in debugfs.

> +
> +/* Boiler plate for devclarng mips-rproc sysfs devices */
> +static struct attribute *mips_rproc_attrs[] = {
> +	&dev_attr_firmware.attr,
> +	&dev_attr_stop.attr,
> +	NULL
> +};
> +

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 5/6] remoteproc/MIPS: Add a remoteproc driver for MIPS
  2016-10-03 22:16   ` Bjorn Andersson
@ 2016-10-05 16:22     ` Matt Redfearn
  0 siblings, 0 replies; 17+ messages in thread
From: Matt Redfearn @ 2016-10-05 16:22 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Ralf Baechle, Ohad Ben-Cohen, Thomas Gleixner, linux-mips,
	linux-remoteproc, lisa.parratt, linux-kernel

Hi Bjorn,

Thanks for the review and comments, much appreciated :-)


On 03/10/16 23:16, Bjorn Andersson wrote:
> On Tue 20 Sep 01:47 PDT 2016, Matt Redfearn wrote:
>
>> Add a remoteproc driver to steal, load the firmware, and boot an offline
>> MIPS core, turning it into a coprocessor.
>>
>> This driver provides a sysfs to allow arbitrary firmware to be loaded
>> onto a core, which may expose virtio devices. Coprocessor firmware must
>> abide by the UHI coprocessor boot protocol.
> Hi Matt,
>
> Sorry for my very slow response, I kept getting side tracked on the
> sysfs part every time I attempted to review this. After discussing with
> others it's obvious that being able to boot a remoteproc with a specific
> firmware image for some amount of time is a very common request.
>
> Rather than adding a MIPS specific interface for controlling this rproc
> I would like for us to bring this to the core.

Yes, makes perfect sense. I had a bit of fun allowing for the firmware 
name / information to be changed in an allocated struct rproc, but I 
have a proof of concept working now and will post it soon. The main 
issue remaining is that when the virtio devices are removed in the 
context of the sysfs write, a warning from dma-mapping.h is triggered 
because interrupts are disabled:

[   28.413958] WARNING: CPU: 0 PID: 121 at 
./include/linux/dma-mapping.h:433 free_buf+0x1a8/0x288
[   28.423542] Modules linked in:
[   28.426951] CPU: 0 PID: 121 Comm: sh Tainted: G        W 4.8.0+ #729
[   28.434600] Stack : 00000000 00000000 00000000 00000000 80d26cea 
0000003e 80c50000 00000000
       00000000 80c50000 80c50000 00040800 80c50000 8f658bc8 80b992dc 
00000079
       00000000 80d23824 00000000 0067544c 0067542c 8048af58 80c50000 
00000001
       80c50000 00000000 80b9fcd0 8f7c7b74 80d23824 8051d8bc 00000000 
0067544c
       00000007 80c50000 8f7c7b74 00040800 00000000 00000000 00000000 
00000000
       ...
[   28.474274] Call Trace:
[   28.477005] [<8040c538>] show_stack+0x74/0xc0
[   28.481860] [<80757240>] dump_stack+0xd0/0x110
[   28.486813] [<80430d98>] __warn+0xfc/0x130
[   28.491379] [<80430ee0>] warn_slowpath_null+0x2c/0x3c
[   28.497007] [<807e7c6c>] free_buf+0x1a8/0x288
[   28.501862] [<807ea590>] remove_port_data+0x50/0xac
[   28.507298] [<807ea6a0>] unplug_port+0xb4/0x1bc
[   28.512346] [<807ea858>] virtcons_remove+0xb0/0xfc
[   28.517689] [<807b6734>] virtio_dev_remove+0x58/0xc0
[   28.523223] [<807f918c>] __device_release_driver+0xac/0x134
[   28.529433] [<807f924c>] device_release_driver+0x38/0x50
[   28.535352] [<807f7edc>] bus_remove_device+0xfc/0x130
[   28.540980] [<807f4b74>] device_del+0x17c/0x21c
[   28.546027] [<807f4c38>] device_unregister+0x24/0x38
[   28.551562] [<807b6b50>] unregister_virtio_device+0x28/0x44
[   28.557777] [<80948ab0>] rproc_change_firmware+0xb4/0x114
[   28.563795] [<80949464>] firmware_store+0x2c/0x40
[   28.569039] [<8060186c>] kernfs_fop_write+0x154/0x1dc
[   28.574669] [<805823f0>] __vfs_write+0x5c/0x17c
[   28.579719] [<805834ac>] vfs_write+0xe0/0x190
[   28.584574] [<80584520>] SyS_write+0x80/0xf4
[   28.589336] [<80415908>] syscall_common+0x34/0x58
[   28.594570]
[   28.596228] ---[ end trace 3f6cae675f2fcee9 ]---


I'm still investigating how to decouple removal of the virtio devices 
from interrupts being disabled.


>
>
> I would also appreciate if Ralf had some input on the MIPS specifics.
>
>
> Also regarding the 32-bit requirement, have you investigated what is
> needed to support 64-bit ELFs?

Not yet - I wanted to get the 32bit version accepted first, then we can 
look at the required changes to support 64bit since no doubt that will 
necessitate quite a few changes to the core code.

>
> [..]
>> diff --git a/drivers/remoteproc/mips_remoteproc.c b/drivers/remoteproc/mips_remoteproc.c
> [..]
>> +int mips_rproc_op_start(struct rproc *rproc)
>> +{
>> +	struct mips_rproc *mproc = *(struct mips_rproc **)rproc->priv;
>> +	int err;
>> +	int cpu = mproc->cpu;
>> +
>> +	if (mips_rprocs[cpu]) {
>> +		dev_err(&rproc->dev, "CPU%d in use\n", cpu);
>> +		return -EBUSY;
>> +	}
>> +	mips_rprocs[cpu] = rproc;
>> +
>> +	/* Create task for the CPU to use before handing off to firmware */
>> +	mproc->tsk = fork_idle(cpu);
>> +	if (IS_ERR(mproc->tsk)) {
>> +		dev_err(&rproc->dev, "fork_idle() failed for CPU%d\n", cpu);
>> +		return -ENOMEM;
>> +	}
>> +
>> +	/* We won't be needing the Linux IPIs anymore */
>> +	if (mips_smp_ipi_free(get_cpu_mask(cpu)))
>> +		return -EINVAL;
>> +
>> +	/*
>> +	 * Direct IPIs from the remote processor to CPU0 since that can't be
>> +	 * offlined while the remote CPU is running.
>> +	 */
>> +	mproc->ipi_linux = irq_reserve_ipi(ipi_domain(), get_cpu_mask(0));
>> +	if (!mproc->ipi_linux) {
>> +		dev_err(&mproc->dev, "Failed to reserve incoming kick\n");
>> +		goto exit_rproc_nofrom;
>> +	}
>> +
>> +	mproc->ipi_remote = irq_reserve_ipi(ipi_domain(), get_cpu_mask(cpu));
>> +	if (!mproc->ipi_remote) {
>> +		dev_err(&mproc->dev, "Failed to reserve outgoing kick\n");
>> +		goto exit_rproc_noto;
>> +	}
>> +
>> +	/* register incoming ipi */
>> +	err = devm_request_threaded_irq(&mproc->dev, mproc->ipi_linux,
>> +					mips_rproc_ipi_handler,
>> +					mips_rproc_vq_int, 0,
>> +					"mips-rproc IPI in", mproc->rproc);
> Based on how you've designed this I think it makes sense to just depend
> on the fact that stop() will always be called and hence you do not
> benefit from the devm_ version of this api.

Yeah, makes sense.

>
>> +	if (err) {
>> +		dev_err(&mproc->dev, "Failed to register incoming kick: %d\n",
>> +			err);
>> +		goto exit_rproc_noint;
>> +	}
>> +
>> +	if (!mips_cps_steal_cpu_and_execute(cpu, &mips_rproc_cpu_entry,
>> +						mproc->tsk))
>> +		return 0;
> Please flip this around, to follow the pattern of the others, like:
>
> 	if (mips_cps_steal_cpu_and_execute()) {
> 		dev_err()
> 		goto exit_free_irq;
> 	}
>
> 	return 0;
>
> exit_free_irq:

OK.

>
>> +
>> +	dev_err(&mproc->dev, "Failed to steal CPU%d for remote\n", cpu);
>> +	devm_free_irq(&mproc->dev, mproc->ipi_linux, mproc->rproc);
>> +exit_rproc_noint:
>> +	irq_destroy_ipi(mproc->ipi_remote, get_cpu_mask(cpu));
>> +exit_rproc_noto:
>> +	irq_destroy_ipi(mproc->ipi_linux, get_cpu_mask(0));
>> +exit_rproc_nofrom:
>> +	free_task(mproc->tsk);
>> +	mips_rprocs[cpu] = NULL;
>> +
>> +	/* Set up the Linux IPIs again */
>> +	mips_smp_ipi_allocate(get_cpu_mask(cpu));
>> +	return -EINVAL;
>> +}
>> +
>> +int mips_rproc_op_stop(struct rproc *rproc)
>> +{
>> +	struct mips_rproc *mproc = *(struct mips_rproc **)rproc->priv;
>> +
>> +	if (mproc->ipi_linux)
> stop() should not be called unless start() succeeded, so ipi_linux
> should not be able to be 0.

True.

>
>> +		devm_free_irq(&mproc->dev, mproc->ipi_linux, mproc->rproc);
>> +
>> +	irq_destroy_ipi(mproc->ipi_linux, get_cpu_mask(0));
>> +	irq_destroy_ipi(mproc->ipi_remote, get_cpu_mask(mproc->cpu));
>> +
>> +	/* Set up the Linux IPIs again */
>> +	mips_smp_ipi_allocate(get_cpu_mask(mproc->cpu));
>> +
>> +	free_task(mproc->tsk);
>> +
>> +	mips_rprocs[mproc->cpu] = NULL;
>> +
>> +	return mips_cps_halt_and_return_cpu(mproc->cpu);
>> +}
>> +
>> +
> [..]
>> +
>> +/* Steal a core and run some firmware on it */
>> +int mips_rproc_start(struct mips_rproc *mproc, const char *firmware, size_t len)
>> +{
>> +	int err = -EINVAL;
>> +	struct mips_rproc **priv;
>> +
>> +	/* Duplicate the filename, dropping whitespace from the end via len */
>> +	mproc->firmware = kstrndup(firmware, len, GFP_KERNEL);
>> +	if (!mproc->firmware)
>> +		return -ENOMEM;
>> +
>> +	mproc->rproc = rproc_alloc(&mproc->dev, "mips", &mips_rproc_proc_ops,
>> +				   mproc->firmware,
>> +				   sizeof(struct mips_rproc *));
>> +	if (!mproc->rproc)
>> +		return -ENOMEM;
>> +
>> +	priv = mproc->rproc->priv;
>> +	*priv = mproc;
> If we move the class into the core, everyone will share the same
> interface for setting firmware, booting and shutting down remoteproc.

Yes, that sounds like the best way forward.

>
> I think you should set rproc->auto_boot to false and move this code into
> the probe function above. It would make there be an remoteproc instance
> whenever the cpu is offlined, do you see any problems with this?

No problem with it being available any time the CPU is offline.


>
>> +
>> +	/* go live! */
>> +	err = rproc_add(mproc->rproc);
>> +	if (err) {
>> +		dev_err(&mproc->dev, "Failed to add rproc: %d\n", err);
>> +		rproc_put(mproc->rproc);
>> +		kfree(mproc->firmware);
>> +		return -EINVAL;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +/* Stop a core, and return it to being offline */
>> +int mips_rproc_stop(struct mips_rproc *mproc)
>> +{
>> +	rproc_shutdown(mproc->rproc);
> I presume this shutdown is related to the implicit boot happening in
> rproc_add() if you have virtio devices; I've changed this for v4.9 so
> that rproc_del() shuts down the core if rproc_add() booted it.

Yeah it is, ok great.

>
> There needs to be some more work done in this area though, because there
> are plenty of corner cases that we don't handle properly today...
>
>> +	rproc_del(mproc->rproc);
>> +	rproc_put(mproc->rproc);
>> +	mproc->rproc = NULL;
>> +	return 0;
>> +}
>> +
> [..]
>> +
>> +/* sysfs interface to mips_rproc_stop */
>> +static ssize_t stop_store(struct device *dev,
>> +			      struct device_attribute *attr,
>> +			      const char *buf, size_t count)
>> +{
>> +	struct mips_rproc *mproc = to_mips_rproc(dev);
>> +	int err = -EINVAL;
>> +
>> +
>> +	if (mproc->rproc)
>> +		err = mips_rproc_stop(mproc);
>> +	else
>> +		err = -EBUSY;
>> +
>> +	return err ? err : count;
>> +}
>> +static DEVICE_ATTR_WO(stop);
> Please move this control into the core, preferably as "state" which can
> be passed "boot" or "shutdown" - i.e. what we today have in debugfs.

OK, though I've stuck with "start" and "stop" as are in the debugfs version.

Thanks,
Matt


>
>> +
>> +/* Boiler plate for devclarng mips-rproc sysfs devices */
>> +static struct attribute *mips_rproc_attrs[] = {
>> +	&dev_attr_firmware.attr,
>> +	&dev_attr_stop.attr,
>> +	NULL
>> +};
>> +
> Regards,
> Bjorn

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2016-10-05 16:22 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-20  8:47 [PATCH v2 0/6] MIPS: Remote processor driver Matt Redfearn
2016-09-20  8:47 ` [PATCH v2 1/6] irqchip: mips-gic: Add context saving for MIPS_REMOTEPROC Matt Redfearn
2016-09-20  9:40   ` Thomas Gleixner
2016-09-20  8:47 ` [PATCH v2 2/6] MIPS: tlb-r4k: If there are wired entries, don't use TLBINVF Matt Redfearn
2016-09-22 12:11   ` Ralf Baechle
2016-09-20  8:47 ` [PATCH v2 3/6] MIPS: smp.c: Introduce mechanism for freeing and allocating IPIs Matt Redfearn
2016-09-20  8:47 ` [PATCH v2 4/6] MIPS: CPS: Add VP(E) stealing Matt Redfearn
2016-09-20  8:47 ` [PATCH v2 5/6] remoteproc/MIPS: Add a remoteproc driver for MIPS Matt Redfearn
2016-09-20  9:47   ` Thomas Gleixner
2016-09-20 15:31     ` Matt Redfearn
2016-10-03  8:35   ` Matt Redfearn
2016-10-03 22:16   ` Bjorn Andersson
2016-10-05 16:22     ` Matt Redfearn
2016-09-20  8:47 ` [PATCH v2 6/6] MIPS: Deprecate VPE Loader Matt Redfearn
2016-09-20 10:25   ` Sergei Shtylyov
2016-09-20 13:19   ` Ralf Baechle
2016-09-20 13:53     ` Matt Redfearn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).