diff mbox

[v2,5/6] remoteproc/MIPS: Add a remoteproc driver for MIPS

Message ID 1474361249-31064-6-git-send-email-matt.redfearn@imgtec.com
State Superseded
Headers show

Commit Message

Matt Redfearn Sept. 20, 2016, 8:47 a.m. UTC
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

Comments

Thomas Gleixner Sept. 20, 2016, 9:47 a.m. UTC | #1
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
--
To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Matt Redfearn Sept. 20, 2016, 3:31 p.m. UTC | #2
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

--
To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Matt Redfearn Oct. 3, 2016, 8:35 a.m. UTC | #3
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");

--
To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjorn Andersson Oct. 3, 2016, 10:16 p.m. UTC | #4
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
--
To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Matt Redfearn Oct. 5, 2016, 4:22 p.m. UTC | #5
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

--
To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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");