[PATCH RFC v2 00/11] remoteproc: add support for any virtio device

[PATCH RFC v2 00/11] remoteproc: add support for any virtio device

[Francesco Valla]

Hello,

this is the second iteration of the patch series that introduces the
possibility to support generic virtio devices over the remoteproc
transport, whereas today only rpmsg and rproc-serial are supported.

Some introduction and motivation can be found in V1 [1].

This new proposal leverages the existing virtio infrastructure and its
optional usage of the DMA APIs to obtain the goal in two cases:

 - if the remote device is able to access the whole DMA space of the
   host, no special arrangement is needed; the DMA APIs will take care
   of cache management and (if required) DMA translation, optionally
   resorting to swiotlb if/when the buffers allocated by a virtio driver
   are not DMA-able;

 - in case the remote device and the host share only a small portion of
   memory (often the case for e.g. mixed-criticality systems), the DMA
   APIs can be coupled with a restricted-dma-pool area to force swiotlb
   translation for all the transmitted buffers.

The new functionality is introduced by setting the
VIRTIO_F_ACCESS_PLATFORM feature flag for all non-remoteproc-specific
virtio devices (i.e., all except the aforementioned rpmsg and
rproc-serial); this flag instructs the virtio framework to use the DMA
APIs when mapping and unmapping buffers to and from the remote
processor.

The series was tested against a custom Zephyr application [2] running on
the Cortex-M33 processor of an i.MX93 and exposing six different virtio
devices:

  - rpmsg
  - entropy (rng)
  - gpio
  - i2c
  - spi
  - can

== Open Points ==

There are still the following open points:

  - some virtio devices (e.g.: virtio-input) communicate through the
    device's config space. The remoteproc transport expects this config
    space to be somewhat constant, and there is no mechanism to sync
    changes made by the driver with the remote device.  

  - device de-registration on remoteproc stop is causing oopses (under
    investigation - might not be strictly tied to the series).

  - I still have to wrap my mind around DMA ranges and IOMMUs, and the
    way the remoteproc framework uses them; the current series keeps the
    legacy behavior for the first (i.e., DMA properties of the parent
    remoteproc device are copied to vdevs) and assumes da == pa for
    remote processor behind an IOMMU.

  - [i.MX-specific] the imx-rproc driver was sending messages in
    blocking mode; the change proposed in the series, required for
    proper operation in atomi contexts is sub-optimal and needs more
    work.

== Patches breakdown ==

Patches 1 and 2 are cleanups to the remoteproc-virtio driver and could
be applied independently of this series.

Patch 3 was submitted a couple of months ago [6] and paves the road for
the actual support of generic virtio devices, removing the fixed number
of 2 for the vrings associated to a vdev.

Patch 4 defines the VIRTIO_F_VERSION_1 feature for all
non-rproc-specific vdevs. This is required to support some virtio device
types, and there is no other mean of defining it, since the field
reserved for features inside the resource table is limited to 32 bits.
Given that the 1.x virtio specifications are ~10 years old this still
seems reasonable.

Patch 5 defines the VIRTIO_F_ACCESS_PLATFORM feature for all
non-rproc-specific vdevs. This is the first step required to obtain the
full virtio functionality.

Patch 6 introduces new devicetree bindings for declaring virtio
devices exposed by a remote processor inside a devicetree. This is not
required for some devices (e.g.: can, net, gpu), but for others (e.g.:
i2c, gpio) is necessary to declare child devices and link them.

Patch 7 assigns a fwnode (if existent) to each new virtio device and
assigns to it the memory region defined for such node.

Patch 8 expands existing remoteproc-virtio documentation to explain the
possibilities introduced by the previous changes.

Patches 9 and 10 are i.MX-specific and enable the usage of the newly
introduced support on this family of platforms. The first one might
probably be submitted independently of the series, since it aligns
the behavior of imx-rproc to the other platforms in relation to
mailbox usage. However, having a kmalloc in the hot send path is
sub-optimal. **Advice needed here.**

Finally, patch 11 is the PoC that has been used to develop and test the
series and shall not be merged.

======

Thanks to everyone that commented on v1 - the functionality evolved in a
much more flexible and no-nonsense way. Thanks in advance to whom will
do the same on this revision.

Regards,
Francesco

[1] https://patch.msgid.link/20260916-remoteproc_virtio_map-v1-0-dac8c5eb4aa9@valla.it
[2] https://github.com/WallaceIT/zephyr/tree/multi_vdev

---
Changes in v2:
- Rebased on v7.4-rc1
- Drop the custom .map APIs and rely on the virtio framework to perform
  DMA management and (if required) the bounce buffering [Robin].
- Set the VERSION_1 and ACCESS_PLATFORM features only for
  non-rproc-specific virtio devices to preserve existing usecases.
- Add the possibility to have a memory-region property in each vdev's
  node and use it for DMA allocations / swiotlb.
- Drop the new DMA subsystem APIs (no more required).
- Drop the spi-virtio bindings, which are not critical for the series
  and have been submitted independently [Mathieu].
- PoC i.MX93 FRDM dts: drop the virtio/spi node and child devices and
  move from gpio-keys-polled to gpio-keys.
- Re-introduced the rsc-table in PoC i.MX93 FRDM dts.
- Fixes in the remoteproc-virtio bindings [Rob].
- imx-rproc: drop unneeded rework of reserved memory assignment.
- imx-rproc: do not send async mailbox messages with payload living in
  the stack.
- Rephrase and expand commit messages.
- Document the memory allocation behavior.
- Scattered fixes for minor issues [Mathieu and sashiko].
- Link to v1: https://patch.msgid.link/20260916-remoteproc_virtio_map-v1-0-dac8c5eb4aa9@valla.it

To: Bjorn Andersson <andersson@kernel.org>
To: Mathieu Poirier <mathieu.poirier@linaro.org>
To: Kees Cook <kees@kernel.org>
To: "Gustavo A. R. Silva" <gustavoars@kernel.org>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Francesco Valla <francesco@valla.it>
To: Jonathan Corbet <corbet@lwn.net>
To: Shuah Khan <skhan@linuxfoundation.org>
To: Randy Dunlap <rdunlap@infradead.org>
To: Frank Li <Frank.Li@nxp.com>
To: Sascha Hauer <s.hauer@pengutronix.de>
To: Pengutronix Kernel Team <kernel@pengutronix.de>
To: Fabio Estevam <festevam@gmail.com>
To: Peng Fan <peng.fan@nxp.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: linux-remoteproc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-hardening@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-doc@vger.kernel.org
Cc: imx@lists.linux.dev
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Francesco Valla <francesco@valla.it>

---
Francesco Valla (11):
      remoteproc: virtio: cleanup rproc_add_virtio_dev error path
      remoteproc: virtio: replace commas with semicolons
      remoteproc: virtio: support dynamic number of vrings
      remoteproc: report VIRTIO_F_VERSION_1 for non-rproc-specific vdevs
      remoteproc: virtio: report VIRTIO_F_ACCESS_PLATFORM when required
      dt-bindings: remoteproc: add remoteproc-virtio
      remoteproc: assign fwnode during vdev registration
      Documentation: remoteproc: expand virtio support section
      remoteproc: imx_rproc: always use non-blocking mailboxes
      dt-bindings: remoteproc: imx-rproc: support virtio
      PoC: arm64: dts: imx93-11x11-frdm: add multiple vdevs

 .../bindings/remoteproc/fsl,imx-rproc.yaml         |   3 +-
 .../bindings/remoteproc/remoteproc-virtio.yaml     | 106 ++++++++++++
 Documentation/staging/remoteproc.rst               |  99 ++++++++++-
 arch/arm64/boot/dts/freescale/imx93-11x11-frdm.dts | 188 +++++++++++++++++++++
 drivers/remoteproc/imx_rproc.c                     |  76 +++------
 drivers/remoteproc/imx_rproc.h                     |   1 -
 drivers/remoteproc/remoteproc_core.c               |  55 ++++--
 drivers/remoteproc/remoteproc_virtio.c             | 110 +++++++++---
 include/linux/remoteproc.h                         |  10 +-
 9 files changed, 558 insertions(+), 90 deletions(-)
---
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
change-id: 20260915-remoteproc_virtio_map-bcf32a5fab54

Best regards,
--  
Francesco Valla <francesco@valla.it>

[PATCH RFC v2 01/11] remoteproc: virtio: cleanup rproc_add_virtio_dev error path

[Francesco Valla]

There is no cleanup to perform in case an error occurs inside
rproc_add_virtio_dev, so remove the goto label and just return in such
cases.

Signed-off-by: Francesco Valla <francesco@valla.it>
---
 drivers/remoteproc/remoteproc_virtio.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
index 12f55a81cfa0..a86c9a949e4c 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -386,9 +386,8 @@ static int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
 	int ret;
 
 	if (rproc->ops->kick == NULL) {
-		ret = -EINVAL;
 		dev_err(dev, ".kick method not defined for %s\n", rproc->name);
-		goto out;
+		return -EINVAL;
 	}
 
 	/* Try to find dedicated vdev buffer carveout */
@@ -404,7 +403,7 @@ static int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
 								 mem->of_resm_idx);
 			if (ret) {
 				dev_err(dev, "Can't associate reserved memory\n");
-				goto out;
+				return ret;
 			}
 		} else {
 			if (mem->va) {
@@ -422,7 +421,7 @@ static int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
 							   mem->len);
 			if (ret < 0) {
 				dev_err(dev, "Failed to associate buffer\n");
-				goto out;
+				return ret;
 			}
 		}
 	} else {
@@ -441,10 +440,9 @@ static int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
 
 	/* Allocate virtio device */
 	vdev = kzalloc_obj(*vdev);
-	if (!vdev) {
-		ret = -ENOMEM;
-		goto out;
-	}
+	if (!vdev)
+		return -ENOMEM;
+
 	vdev->id.device	= id,
 	vdev->config = &rproc_virtio_config_ops,
 	vdev->dev.parent = dev;
@@ -457,13 +455,12 @@ static int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
 	if (ret) {
 		put_device(&vdev->dev);
 		dev_err(dev, "failed to register vdev: %d\n", ret);
-		goto out;
+		return ret;
 	}
 
 	dev_info(dev, "registered %s (type %d)\n", dev_name(&vdev->dev), id);
 
-out:
-	return ret;
+	return 0;
 }
 
 /**

-- 
2.56.0

[PATCH RFC v2 02/11] remoteproc: virtio: replace commas with semicolons

[Francesco Valla]

While not technically wrong, the comma at the end of an assignment
directive can lead to nasty bugs. Just replace the two occurrences with
semicolons.

Signed-off-by: Francesco Valla <francesco@valla.it>
---
 drivers/remoteproc/remoteproc_virtio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
index a86c9a949e4c..29a3a4c18097 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -443,8 +443,8 @@ static int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
 	if (!vdev)
 		return -ENOMEM;
 
-	vdev->id.device	= id,
-	vdev->config = &rproc_virtio_config_ops,
+	vdev->id.device = id;
+	vdev->config = &rproc_virtio_config_ops;
 	vdev->dev.parent = dev;
 	vdev->dev.release = rproc_virtio_dev_release;
 

-- 
2.56.0

[PATCH RFC v2 03/11] remoteproc: virtio: support dynamic number of vrings

[Francesco Valla]

The number of vrings for each vdev has been fixed to 2 since the
introduction of multi-vdev support [1]; this is completely fine for the
rpmsg usecase, but can conflict with other virtio devices (CAN for
example requires 3 virtqueues, entropy only 1, network a variable number
and so on).

Remove the static vring allocation, transforming it to a flex array that
is allocated at vdev probe time; for the existing usecases (i.e.: mainly
rpmsg) this leads to no functional change, except the additional memory
used for the counter associated to the new array.

The maximum number of virtqueues is limited to 255 due to the u8 value
used inside the resource table to indicate the number of vring to
allocate; for this reason, no additional plausibility check is performed
on the number of vrings indicated by the resource table.

This also fixes the single virtqueue usecase, which was apparently
supported also before but for which the remove action caused an error
(because the remove action was trying to unmap also the second vring,
which was in fact not mapped).

[1] https://lore.kernel.org/all/1330589497-4139-5-git-send-email-ohad@wizery.com/

Signed-off-by: Francesco Valla <francesco@valla.it>
---
 drivers/remoteproc/remoteproc_core.c   |  7 -------
 drivers/remoteproc/remoteproc_virtio.c | 12 ++++++------
 include/linux/remoteproc.h             | 10 ++++------
 3 files changed, 10 insertions(+), 19 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 23f8539a99bd..c946ecd980a5 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -473,7 +473,6 @@ static int rproc_handle_vdev(struct rproc *rproc, void *ptr,
 {
 	struct fw_rsc_vdev *rsc = ptr;
 	struct device *dev = &rproc->dev;
-	struct rproc_vdev *rvdev;
 	size_t rsc_size;
 	struct rproc_vdev_data rvdev_data;
 	struct platform_device *pdev;
@@ -494,12 +493,6 @@ static int rproc_handle_vdev(struct rproc *rproc, void *ptr,
 	dev_dbg(dev, "vdev rsc: id %d, dfeatures 0x%x, cfg len %d, %d vrings\n",
 		rsc->id, rsc->dfeatures, rsc->config_len, rsc->num_of_vrings);
 
-	/* we currently support only two vrings per rvdev */
-	if (rsc->num_of_vrings > ARRAY_SIZE(rvdev->vring)) {
-		dev_err(dev, "too many vrings: %d\n", rsc->num_of_vrings);
-		return -EINVAL;
-	}
-
 	rvdev_data.id = rsc->id;
 	rvdev_data.index = rproc->nb_vdev++;
 	rvdev_data.rsc_offset = offset;
diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
index 29a3a4c18097..c56d3330adfe 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -135,8 +135,7 @@ static struct virtqueue *rp_find_vq(struct virtio_device *vdev,
 	void *addr;
 	int num, size;
 
-	/* we're temporarily limited to two virtqueues per rvdev */
-	if (id >= ARRAY_SIZE(rvdev->vring))
+	if (id >= rvdev->num_vrings)
 		return ERR_PTR(-EINVAL);
 
 	if (!name)
@@ -510,13 +509,16 @@ static int rproc_virtio_probe(struct platform_device *pdev)
 	if (!rvdev_data)
 		return -EINVAL;
 
-	rvdev = devm_kzalloc(dev, sizeof(*rvdev), GFP_KERNEL);
+	rsc = rvdev_data->rsc;
+
+	rvdev = devm_kzalloc_flex(dev, *rvdev, vring, rsc->num_of_vrings);
 	if (!rvdev)
 		return -ENOMEM;
 
 	rvdev->id = rvdev_data->id;
 	rvdev->rproc = rproc;
 	rvdev->index = rvdev_data->index;
+	rvdev->num_vrings = rsc->num_of_vrings;
 
 	ret = copy_dma_range_map(dev, rproc->dev.parent);
 	if (ret)
@@ -534,8 +536,6 @@ static int rproc_virtio_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, rvdev);
 	rvdev->pdev = pdev;
 
-	rsc = rvdev_data->rsc;
-
 	/* parse the vrings */
 	for (i = 0; i < rsc->num_of_vrings; i++) {
 		ret = rproc_parse_vring(rvdev, rsc, i);
@@ -586,7 +586,7 @@ static void rproc_virtio_remove(struct platform_device *pdev)
 	struct rproc_vring *rvring;
 	int id;
 
-	for (id = 0; id < ARRAY_SIZE(rvdev->vring); id++) {
+	for (id = 0; id < rvdev->num_vrings; id++) {
 		rvring = &rvdev->vring[id];
 		rproc_free_vring(rvring);
 	}
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 00019b1895e6..52e1eccbe42e 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -322,10 +322,6 @@ struct rproc_subdev {
 	void (*unprepare)(struct rproc_subdev *subdev);
 };
 
-/* we currently support only two vrings per rvdev */
-
-#define RVDEV_NUM_VRINGS 2
-
 /**
  * struct rproc_vring - remoteproc vring state
  * @va:	virtual address
@@ -353,9 +349,10 @@ struct rproc_vring {
  * @id: virtio device id (as in virtio_ids.h)
  * @node: list node
  * @rproc: the rproc handle
- * @vring: the vrings for this vdev
  * @rsc_offset: offset of the vdev's resource entry
  * @index: vdev position versus other vdev declared in resource table
+ * @num_vrings: the number of vrings for this vdev
+ * @vring: the vrings for this vdev
  */
 struct rproc_vdev {
 	struct rproc_subdev subdev;
@@ -364,9 +361,10 @@ struct rproc_vdev {
 	unsigned int id;
 	struct list_head node;
 	struct rproc *rproc;
-	struct rproc_vring vring[RVDEV_NUM_VRINGS];
 	u32 rsc_offset;
 	u32 index;
+	unsigned int num_vrings;
+	struct rproc_vring vring[] __counted_by(num_vrings);
 };
 
 struct rproc *rproc_get_by_phandle(phandle phandle);

-- 
2.56.0

[PATCH RFC v2 04/11] remoteproc: report VIRTIO_F_VERSION_1 for non-rproc-specific vdevs

[Francesco Valla]

There is currently no way to report the VIRTIO_F_VERSION_1 using the
resource table alone, as the per-vdev feature array is limited to 32
bits.

Considering that the VIRTIO 1.0 specification is now ~10 years old,
always report the VIRTIO_F_VERSION_1 feature for virtio devices that
are not specific for remoteproc usage. These latter, which as of today
are rpmsg and rproc-serial, are excluded to avoid impacting the existing
usecases, since VIRTIO_F_VERSION_1 changes the semantics of the
communication (e.g., the configuration space is assumed to be
little-endian rather than follow the device's endianness).

While at it, replace BUG_ON() with WARN_ON() and return an appropriate
error in case of features that would overflow the 32 bits limit.

Signed-off-by: Francesco Valla <francesco@valla.it>
---
 drivers/remoteproc/remoteproc_virtio.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
index c56d3330adfe..f1b362158a9c 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -16,6 +16,7 @@
 #include <linux/of_reserved_mem.h>
 #include <linux/platform_device.h>
 #include <linux/remoteproc.h>
+#include <linux/rsc_table.h>
 #include <linux/virtio.h>
 #include <linux/virtio_config.h>
 #include <linux/virtio_ids.h>
@@ -27,6 +28,11 @@
 
 #include "remoteproc_internal.h"
 
+static bool rproc_virtio_rsc_vdev_rproc_specific(struct fw_rsc_vdev *rsc)
+{
+	return (rsc->id == VIRTIO_ID_RPMSG || rsc->id == VIRTIO_ID_RPROC_SERIAL);
+}
+
 static int copy_dma_range_map(struct device *to, struct device *from)
 {
 	const struct bus_dma_region *map = from->dma_range_map, *new_map, *r;
@@ -268,8 +274,17 @@ static void rproc_virtio_synchronize_cbs(struct virtio_device *vdev)
 static u64 rproc_virtio_get_features(struct virtio_device *vdev)
 {
 	struct fw_rsc_vdev *rsc = vdev_to_rsc(vdev);
+	u64 features = (u64)rsc->dfeatures;
 
-	return rsc->dfeatures;
+	/*
+	 * All non-remoteproc-specific virtio devices are assumed to be
+	 * compliant with virtio specifications 1.x; there is no other method
+	 * to transmit this capability from the remote processor to the host.
+	 */
+	if (!rproc_virtio_rsc_vdev_rproc_specific(rsc))
+		features |= (1ULL << VIRTIO_F_VERSION_1);
+
+	return features;
 }
 
 static void rproc_transport_features(struct virtio_device *vdev)
@@ -285,6 +300,7 @@ static void rproc_transport_features(struct virtio_device *vdev)
 static int rproc_virtio_finalize_features(struct virtio_device *vdev)
 {
 	struct fw_rsc_vdev *rsc = vdev_to_rsc(vdev);
+	u64 features;
 
 	/* Give virtio_ring a chance to accept features */
 	vring_transport_features(vdev);
@@ -292,8 +308,14 @@ static int rproc_virtio_finalize_features(struct virtio_device *vdev)
 	/* Give virtio_rproc a chance to accept features. */
 	rproc_transport_features(vdev);
 
-	/* Make sure we don't have any features > 32 bits! */
-	BUG_ON((u32)vdev->features != vdev->features);
+	/* Make sure we don't have any features > 32 bits except the expected ones. */
+	features = vdev->features;
+
+	if (!rproc_virtio_rsc_vdev_rproc_specific(rsc))
+		features &= ~(1ULL << VIRTIO_F_VERSION_1);
+
+	if (WARN_ON((u32)vdev->features != features))
+		return -EINVAL;
 
 	/*
 	 * Remember the finalized features of our vdev, and provide it

-- 
2.56.0

[PATCH RFC v2 05/11] remoteproc: virtio: report VIRTIO_F_ACCESS_PLATFORM when required

[Francesco Valla]

Depending on the driver originating them, data buffers used for virtio
communication can either:

  - already be allocated from the coherent memory area that is
    accessible by the remote processor; this is the case of
    remoteproc-specific virtio devices, i.e., rpmsg and rproc-serial;
  - be allocated from generic kmem, and thus not necessarily accessible
    directly by the remote processor.

In the second case, the DMA APIs can be exploited to obtain a
functional communication, since they will take care of cache maintenance
and - if required - bounce buffering. The VIRTIO_F_ACCESS_PLATFORM
feature flag can be used for this purpose: with it set, the virtio
framework will use the aforementioned DMA APIs to map and unmap buffers.

Bounce buffering will be transparently used in two cases:

  - if the buffer address supplied by the virtio driver is not suitable
    for DMA transfers, or
  - if the reserved memory associated to the vdev is a
    restricted-dma-pool; in this case, *all* buffers will be bounced to
    and from this pool.

With this support in place, any kind of virtio device can be supported
through the remoteproc-virtio transport, at least from a data exchange
standpoint.

Note that remote processors behind an IOMMU need da == pa for the memory
region(s) the buffers are allocated from.

Existing use cases (e.g., rpmsg and virtio-console) are not impacted as
they are explicitly excluded.

Signed-off-by: Francesco Valla <francesco@valla.it>
---
 drivers/remoteproc/remoteproc_virtio.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
index f1b362158a9c..1496659ddff9 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -280,9 +280,14 @@ static u64 rproc_virtio_get_features(struct virtio_device *vdev)
 	 * All non-remoteproc-specific virtio devices are assumed to be
 	 * compliant with virtio specifications 1.x; there is no other method
 	 * to transmit this capability from the remote processor to the host.
+	 *
+	 * Virtio devices that are specific for remoteproc usage take care on
+	 * their own to allocate memory from the device's coherent pool; use
+	 * the DMA APIs for the others through VIRTIO_F_ACCESS_PLATFORM.
 	 */
 	if (!rproc_virtio_rsc_vdev_rproc_specific(rsc))
-		features |= (1ULL << VIRTIO_F_VERSION_1);
+		features |= ((1ULL << VIRTIO_F_VERSION_1) |
+			     (1ULL << VIRTIO_F_ACCESS_PLATFORM));
 
 	return features;
 }
@@ -312,7 +317,8 @@ static int rproc_virtio_finalize_features(struct virtio_device *vdev)
 	features = vdev->features;
 
 	if (!rproc_virtio_rsc_vdev_rproc_specific(rsc))
-		features &= ~(1ULL << VIRTIO_F_VERSION_1);
+		features &= ~((1ULL << VIRTIO_F_VERSION_1) |
+			      (1ULL << VIRTIO_F_ACCESS_PLATFORM));
 
 	if (WARN_ON((u32)vdev->features != features))
 		return -EINVAL;

-- 
2.56.0

[PATCH RFC v2 06/11] dt-bindings: remoteproc: add remoteproc-virtio

[Francesco Valla]

Add a new binding to describe remoteproc-provided virtio devices; while
these are discovered through a resource table parsed by the remoteproc
infrastructure at runtime, their description can be needed to probe
non-discoverable buses (such as I2C) or to link consumers and suppliers.

Each vdev is described by a dedicated "group" node, which then includes
a virtio-device node. Each vdev shall be statically linked to a "group"
node using its index inside the resource table as the reg property of
the node; this permits to have multiple instances of the same type of
device.

The binding is intended to be generic and adopted by any remoteproc
provider.

Signed-off-by: Francesco Valla <francesco@valla.it>
---
 .../bindings/remoteproc/remoteproc-virtio.yaml     | 106 +++++++++++++++++++++
 1 file changed, 106 insertions(+)

diff --git a/Documentation/devicetree/bindings/remoteproc/remoteproc-virtio.yaml b/Documentation/devicetree/bindings/remoteproc/remoteproc-virtio.yaml
new file mode 100644
index 000000000000..8cdfa8d0f936
--- /dev/null
+++ b/Documentation/devicetree/bindings/remoteproc/remoteproc-virtio.yaml
@@ -0,0 +1,106 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/remoteproc/remoteproc-virtio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Virtio devices over remoteproc
+
+description: |
+  Virtio devices ("vdevs") can be exposed using the remoteproc infrastructure
+  and its resource table. For some of them, a device tree node might be needed
+  to describe remote undiscoverable hardware and/or connect consumers and
+  providers.
+
+maintainers:
+  - Francesco Valla <francesco@valla.it>
+
+properties:
+  virtio:
+    type: object
+    description: |
+      Contains a group of virtio devices exposed by the firmware running on
+      the remote processor through its resource table.
+
+    additionalProperties: false
+
+    properties:
+      '#address-cells':
+        const: 1
+
+      '#size-cells':
+        const: 0
+
+    patternProperties:
+      "@[0-9a-f]+$":
+        type: object
+        description: |
+          A single virtio device exposed by the remote processor. It can
+          contain at most one child node, describing the virtio device itself
+          (e.g., to declare devices on a non-discoverable bus or to act as a
+          provider for other nodes).
+
+        properties:
+          reg:
+            description: |
+              Position of the virtio device among the VDEV resources contained
+              inside the resource table.
+            maxItems: 1
+
+          memory-region:
+            description: |
+              Memory region used for the buffers exchanged with the remote
+              processor. A restricted-dma-pool region forces all buffers to
+              be bounced through it.
+            maxItems: 1
+
+        additionalProperties:
+          type: object
+          $ref: /schemas/virtio/virtio-device.yaml#
+
+        required:
+          - reg
+
+    required:
+      - '#address-cells'
+      - '#size-cells'
+
+additionalProperties: true
+
+examples:
+  - |
+    remoteproc-cm33 {
+        virtio {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            vdev@0 {
+                reg = <0>;
+
+                gpio {
+                    compatible = "virtio,device29";
+
+                    gpio-controller;
+                    #gpio-cells = <2>;
+                };
+            };
+
+            vdev@1 {
+                reg = <1>;
+                memory-region = <&vdev1_buffer>;
+
+                i2c {
+                    compatible = "virtio,device22";
+
+                    #address-cells = <1>;
+                    #size-cells = <0>;
+
+                    eeprom@50 {
+                        compatible = "atmel,24c1025";
+                        reg = <0x50>;
+                    };
+                };
+            };
+        };
+    };
+...

-- 
2.56.0

[PATCH RFC v2 07/11] remoteproc: assign fwnode during vdev registration

[Francesco Valla]

The virtio subsystem already supports a fwnode describing a vdev; search
for a specific node during vdev registration and supply it to the
rproc-virtio platform device that is being registered.

A memory-region specified inside that node is also used for the virtio
device if a carveout was not supplied for it through the parent device's
node.

Signed-off-by: Francesco Valla <francesco@valla.it>
---
 drivers/remoteproc/remoteproc_core.c   | 48 +++++++++++++++++++++++++++++++---
 drivers/remoteproc/remoteproc_virtio.c | 41 +++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+), 4 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index c946ecd980a5..e833853ae7f6 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -440,6 +440,33 @@ void rproc_remove_rvdev(struct rproc_vdev *rvdev)
 	if (rvdev)
 		list_del(&rvdev->node);
 }
+
+static struct fwnode_handle *rproc_get_vdev_fwnode(struct device *dev, u32 idx)
+{
+	struct fwnode_handle *group, *child;
+	u32 reg;
+	int ret;
+
+	group = device_get_named_child_node(dev, "virtio");
+	if (!fwnode_device_is_available(group)) {
+		fwnode_handle_put(group);
+		return NULL;
+	}
+
+	fwnode_for_each_available_child_node(group, child) {
+		ret = fwnode_property_read_u32(child, "reg", &reg);
+		if (ret)
+			continue;
+
+		if (idx == reg)
+			break;
+	}
+
+	fwnode_handle_put(group);
+
+	return child;
+}
+
 /**
  * rproc_handle_vdev() - handle a vdev fw resource
  * @rproc: the remote processor
@@ -475,7 +502,16 @@ static int rproc_handle_vdev(struct rproc *rproc, void *ptr,
 	struct device *dev = &rproc->dev;
 	size_t rsc_size;
 	struct rproc_vdev_data rvdev_data;
+	struct platform_device_info pdev_info = {
+		.parent = dev,
+		.name = "rproc-virtio",
+		.id = PLATFORM_DEVID_AUTO,
+		.data = &rvdev_data,
+		.size_data = sizeof(rvdev_data),
+	};
 	struct platform_device *pdev;
+	struct fwnode_handle *fwnode;
+	int ret = 0;
 
 	/* make sure resource isn't truncated */
 	rsc_size = struct_size(rsc, vring, rsc->num_of_vrings);
@@ -498,20 +534,24 @@ static int rproc_handle_vdev(struct rproc *rproc, void *ptr,
 	rvdev_data.rsc_offset = offset;
 	rvdev_data.rsc = rsc;
 
+	fwnode = rproc_get_vdev_fwnode(dev->parent, rvdev_data.index);
+	pdev_info.fwnode = fwnode;
+
 	/*
 	 * When there is more than one remote processor, rproc->nb_vdev number is
 	 * same for each separate instances of "rproc". If rvdev_data.index is used
 	 * as device id, then we get duplication in sysfs, so need to use
 	 * PLATFORM_DEVID_AUTO to auto select device id.
 	 */
-	pdev = platform_device_register_data(dev, "rproc-virtio", PLATFORM_DEVID_AUTO, &rvdev_data,
-					     sizeof(rvdev_data));
+	pdev = platform_device_register_full(&pdev_info);
 	if (IS_ERR(pdev)) {
 		dev_err(dev, "failed to create rproc-virtio device\n");
-		return PTR_ERR(pdev);
+		ret = PTR_ERR(pdev);
 	}
 
-	return 0;
+	fwnode_handle_put(fwnode);
+
+	return ret;
 }
 
 /**
diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
index 1496659ddff9..950d65480be5 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -13,10 +13,12 @@
 #include <linux/dma-map-ops.h>
 #include <linux/dma-mapping.h>
 #include <linux/export.h>
+#include <linux/of.h>
 #include <linux/of_reserved_mem.h>
 #include <linux/platform_device.h>
 #include <linux/remoteproc.h>
 #include <linux/rsc_table.h>
+#include <linux/swiotlb.h>
 #include <linux/virtio.h>
 #include <linux/virtio_config.h>
 #include <linux/virtio_ids.h>
@@ -394,6 +396,23 @@ static void rproc_virtio_dev_release(struct device *dev)
 	put_device(&rvdev->pdev->dev);
 }
 
+/*
+ * Attach the memory region declared inside the vdev's own devicetree node.
+ *
+ * A restricted-dma-pool region is attached by of_dma_configure() when the
+ * platform device is probed, but it is released together with the virtio
+ * device (see rproc_virtio_dev_release()); attach it again if needed, e.g.
+ * when the remote processor is restarted during a crash recovery.
+ * Any other kind of region (e.g., shared-dma-pool) is attached only here.
+ */
+static int rproc_virtio_init_vdev_mem(struct device *dev)
+{
+	if (is_swiotlb_for_alloc(dev))
+		return 0;
+
+	return of_reserved_mem_device_init_by_idx(dev, dev->of_node, 0);
+}
+
 /**
  * rproc_add_virtio_dev() - register an rproc-induced virtio device
  * @rvdev: the remote vdev
@@ -410,6 +429,7 @@ static int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
 	struct device *dev = &rvdev->pdev->dev;
 	struct virtio_device *vdev;
 	struct rproc_mem_entry *mem;
+	bool has_mem_region;
 	int ret;
 
 	if (rproc->ops->kick == NULL) {
@@ -417,11 +437,24 @@ static int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
 		return -EINVAL;
 	}
 
+	has_mem_region = of_property_present(dev->of_node, "memory-region");
+
 	/* Try to find dedicated vdev buffer carveout */
 	mem = rproc_find_carveout_by_name(rproc, "vdev%dbuffer", rvdev->index);
 	if (mem) {
 		phys_addr_t pa;
 
+		if (has_mem_region) {
+			/*
+			 * The carveout takes precedence: drop the region that
+			 * of_dma_configure() might have attached at probe time.
+			 */
+			dev_warn(dev,
+				 "vdev%dbuffer carveout found, ignoring memory-region\n",
+				 rvdev->index);
+			of_reserved_mem_device_release(dev);
+		}
+
 		if (mem->of_resm_idx != -1) {
 			struct device_node *np = rproc->dev.parent->of_node;
 
@@ -451,6 +484,14 @@ static int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
 				return ret;
 			}
 		}
+	} else if (has_mem_region) {
+		/* Use the memory region declared in the vdev's own node */
+		ret = rproc_virtio_init_vdev_mem(dev);
+		if (ret) {
+			dev_err(dev, "Can't associate vdev memory-region: %d\n",
+				ret);
+			return ret;
+		}
 	} else {
 		struct device_node *np = rproc->dev.parent->of_node;
 

-- 
2.56.0

[PATCH RFC v2 08/11] Documentation: remoteproc: expand virtio support section

[Francesco Valla]

Expand the section on virtio support over remoteproc with details on
memory allocation setup and constraints.

Signed-off-by: Francesco Valla <francesco@valla.it>
---
 Documentation/staging/remoteproc.rst | 99 +++++++++++++++++++++++++++++++++++-
 1 file changed, 98 insertions(+), 1 deletion(-)

diff --git a/Documentation/staging/remoteproc.rst b/Documentation/staging/remoteproc.rst
index c117b060e76c..4b42ed582fb4 100644
--- a/Documentation/staging/remoteproc.rst
+++ b/Documentation/staging/remoteproc.rst
@@ -354,7 +354,104 @@ it supports. A firmware may support any number of virtio devices, and
 of any type (a single remote processor can also easily support several
 rpmsg virtio devices this way, if desired).
 
+Memory carveouts
+----------------
+
+For a virtio device at position ``X`` among the RSC_VDEV resources declared by
+the resource table, the remoteproc framework will search for memory carveouts
+for:
+
+* all the vrings the specific virtio device requires (as ``vdevXvringY``);
+* the memory area to allocate communication buffers from (as ``vdevXbuffer``).
+
+The carveouts can be provided by the remote processor itself as RSC_CARVEOUT
+resources inside the resource table and/or supplied by the remoteproc driver
+and will be indexed by their name.
+If a carveout with the same name is provided through both means, the one
+supplied by the remote processor should be marked as dynamically allocable,
+i.e., with the device address set to ``FW_RSC_ADDR_ANY`` (-1).
+
+If a ``vdevXvringY`` carveout is not provided, the remoteproc framework will
+try to allocate it using dma_alloc_coherent(), which will resort to allocate it
+either from the first ``restricted-dma-pool`` region associated to the virtio
+device's parent (i.e., the remote processor device), if present, or the global
+DMA pool.
+
+.. WARNING::
+
+    This fallback mechanism can result in a vring being allocated using a
+    ``vdevXbuffer`` area meant for another vdev, so a particular care shall be
+    adopted when defining the carveouts. In particular, ``restricted-dma-pool``
+    memory regions should be specified inside the virtio device's devicetree
+    nodes and not attached to the parent remote processor.
+
+If a ``vdevXbuffer`` carveout is not provided, the remoteproc framework will
+use the memory region declared by the virtio device's own devicetree node (see
+Documentation/devicetree/bindings/remoteproc/remoteproc-virtio.yaml), if any.
+Otherwise, it will try to replace it with the first memory region (of any type)
+associated to the virtio device's parent (i.e., the remote processor device);
+if no such memory region is present, it will be replaced with the global DMA
+pool.
+
+If both a ``vdevXbuffer`` carveout and a memory region inside the virtio
+device's devicetree node are present, the carveout takes precedence and the
+memory region is ignored.
+
+Declaring the memory region inside the virtio device's devicetree node is the
+preferred method for ``restricted-dma-pool`` regions, since listing them among
+the memory regions of the remote processor device would make the first of them
+the (forced) bounce buffer for the remote processor device itself as well.
+
+Buffer allocation
+-----------------
+
+There are different kinds of buffer allocation scenarios:
+
+* if the remote processor can access the whole DMA memory area of the host, no
+  special arrangement needs to be performed, since either the drivers (in case
+  of rpmsg and rproc-serial devices) or the DMA APIs (in case of any other
+  virtio device type) will take care of any necessary translation;
+* if the remote processor has limited access to the host memory space, for
+  example because of a different addressing scheme or for memory isolation
+  constraints, the memory reserved for buffer allocation shall be selected
+  depending on the virtio device type:
+
+  - for rpmsg and rproc-serial devices, a simple contiguous memory region
+    (e.g.: one declared in devicetree as ``shared-dma-pool``) is sufficient and
+    should be preferred, since it allows zero-copy message transmission between
+    the host and the remote processor;
+  - for any other virtio device type, a swiotlb-enabled memory region (e.g.:
+    one declared in devicetree as ``restricted-dma-pool``) is required, because
+    such devices need bounce buffering to work correctly.
+
+For virtio devices other than rpmsg and rproc-serial, the remoteproc framework
+offers the ``VIRTIO_F_ACCESS_PLATFORM`` feature, so the buffer addresses written
+in the vring descriptors are the DMA addresses returned by the DMA APIs for the
+virtio device. The virtio device inherits the DMA mask and the DMA range map
+(i.e., the translation described by the ``dma-ranges`` devicetree property) of
+the remote processor device, and is never attached to an IOMMU: these DMA
+addresses are therefore host physical addresses, translated only through the
+DMA range map. The remote processor must be able to access the buffers at
+these addresses.
+
+.. WARNING::
+
+    For remote processors that access memory through an IOMMU managed by the
+    remoteproc framework (i.e., with ``has_iommu`` set), the IOMMU domain only
+    contains the mappings created for the RSC_CARVEOUT and RSC_DEVMEM entries
+    of the resource table, at the device addresses they specify; no mapping is
+    created for the buffers of the virtio devices. Such remote processors are
+    supported only if all the memory that can hold virtio buffers (e.g., the
+    ``restricted-dma-pool`` region associated to the virtio device) is mapped
+    with its device address equal to its physical address, for example through
+    an RSC_DEVMEM entry. The same constraint applies to any fixed address
+    translation applied by the remote processor that is not described by the
+    DMA range map of the remote processor device.
+
+Allocation of virtio devices
+----------------------------
+
 Of course, RSC_VDEV resource entries are only good enough for static
 allocation of virtio devices. Dynamic allocations will also be made possible
 using the rpmsg bus (similar to how we already do dynamic allocations of
-rpmsg channels; read more about it in rpmsg.txt).
+rpmsg channels; read more about it in Documentation/staging/rpmsg.rst).

-- 
2.56.0

[PATCH RFC v2 09/11] remoteproc: imx_rproc: always use non-blocking mailboxes

[Francesco Valla]

Blocking mailboxes may lead to sleep during the kick operation, which
conflicts with some virtio devices (e.g.: virtio-can) that kicks their
virtqueues with spinlock(s) held.

Align to the behavior of all the other remoteproc drivers that use a
mailbox to perform the kick (i.e.: omap, st, stm32, ti_k3_*, xlnx_r5)
and move to a non-blocking mailbox here as well. Messages need to be
allocated and freed on-the-fly, since they might be sent after
imx_rproc_kick() returns and cannot thus live on the stack.

As a side effect of this modification, the close-reopen action on
the mailboxes during restart and shutdown can be removed, since it was
performed only to have a non-blocking mailbox.

Signed-off-by: Francesco Valla <francesco@valla.it>
---
 drivers/remoteproc/imx_rproc.c | 76 ++++++++++++++++--------------------------
 drivers/remoteproc/imx_rproc.h |  1 -
 2 files changed, 28 insertions(+), 49 deletions(-)

diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index 581bb07f5893..a792b9ca5edd 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -96,7 +96,7 @@ struct imx_rproc_mem {
 /* Linux has permission to handle the Logical Machine of remote cores */
 #define IMX_RPROC_FLAGS_SM_LMM_CTRL	BIT(0)
 
-static int imx_rproc_xtr_mbox_init(struct rproc *rproc, bool tx_block);
+static int imx_rproc_xtr_mbox_init(struct rproc *rproc);
 static void imx_rproc_free_mbox(void *data);
 
 /* Forward declarations for platform operations */
@@ -441,7 +441,7 @@ static int imx_rproc_start(struct rproc *rproc)
 	struct device *dev = priv->dev;
 	int ret;
 
-	ret = imx_rproc_xtr_mbox_init(rproc, true);
+	ret = imx_rproc_xtr_mbox_init(rproc);
 	if (ret)
 		return ret;
 
@@ -696,28 +696,45 @@ static void imx_rproc_kick(struct rproc *rproc, int vqid)
 {
 	struct imx_rproc *priv = rproc->priv;
 	int err;
-	__u32 mmsg;
+	__u32 *mmsg;
 
 	if (!priv->tx_ch) {
 		dev_err(priv->dev, "No initialized mbox tx channel\n");
 		return;
 	}
 
+	mmsg = kzalloc_obj(*mmsg, GFP_ATOMIC);
+	if (!mmsg) {
+		dev_err_ratelimited(priv->dev, "cannot allocate kick message\n");
+		return;
+	}
+
 	/*
 	 * Send the index of the triggered virtqueue as the mu payload.
 	 * Let remote processor know which virtqueue is used.
 	 */
-	mmsg = vqid << 16;
+	*mmsg = vqid << 16;
 
-	err = mbox_send_message(priv->tx_ch, (void *)&mmsg);
-	if (err < 0)
+	err = mbox_send_message(priv->tx_ch, (void *)mmsg);
+	if (err < 0) {
 		dev_err(priv->dev, "%s: failed (%d, err:%d)\n",
 			__func__, vqid, err);
+		kfree(mmsg);
+	}
+}
+
+static void imx_rproc_kick_tx_done(struct mbox_client *cl, void *mmsg, int r)
+{
+	if (r != 0)
+		dev_err_ratelimited(cl->dev, "failed to kick vqid %u: %d\n",
+				    *(__u32 *)mmsg >> 16, r);
+
+	kfree(mmsg);
 }
 
 static int imx_rproc_attach(struct rproc *rproc)
 {
-	return imx_rproc_xtr_mbox_init(rproc, true);
+	return imx_rproc_xtr_mbox_init(rproc);
 }
 
 static int imx_rproc_scu_api_detach(struct rproc *rproc)
@@ -872,7 +889,7 @@ static void imx_rproc_rx_callback(struct mbox_client *cl, void *msg)
 	idr_for_each(&rproc->notifyids, imx_rproc_notified_idr_cb, rproc);
 }
 
-static int imx_rproc_xtr_mbox_init(struct rproc *rproc, bool tx_block)
+static int imx_rproc_xtr_mbox_init(struct rproc *rproc)
 {
 	struct imx_rproc *priv = rproc->priv;
 	struct device *dev = priv->dev;
@@ -895,10 +912,10 @@ static int imx_rproc_xtr_mbox_init(struct rproc *rproc, bool tx_block)
 
 	cl = &priv->cl;
 	cl->dev = dev;
-	cl->tx_block = tx_block;
-	cl->tx_tout = 100;
+	cl->tx_block = false;
 	cl->knows_txdone = false;
 	cl->rx_callback = imx_rproc_rx_callback;
+	cl->tx_done = imx_rproc_kick_tx_done;
 
 	priv->tx_ch = mbox_request_channel_byname(cl, "tx");
 	if (IS_ERR(priv->tx_ch))
@@ -1218,22 +1235,6 @@ static int imx_rproc_detect_mode(struct imx_rproc *priv)
 	return priv->ops->detect_mode(priv->rproc);
 }
 
-static int imx_rproc_sys_off_handler(struct sys_off_data *data)
-{
-	struct rproc *rproc = data->cb_data;
-	int ret;
-
-	imx_rproc_free_mbox(rproc);
-
-	ret = imx_rproc_xtr_mbox_init(rproc, false);
-	if (ret) {
-		dev_err(&rproc->dev, "Failed to request non-blocking mbox\n");
-		return NOTIFY_BAD;
-	}
-
-	return NOTIFY_DONE;
-}
-
 static int imx_rproc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -1262,7 +1263,7 @@ static int imx_rproc_probe(struct platform_device *pdev)
 
 	dev_set_drvdata(dev, rproc);
 
-	ret = imx_rproc_xtr_mbox_init(rproc, true);
+	ret = imx_rproc_xtr_mbox_init(rproc);
 	if (ret)
 		return ret;
 
@@ -1292,26 +1293,6 @@ static int imx_rproc_probe(struct platform_device *pdev)
 	if (rproc->state != RPROC_DETACHED)
 		rproc->auto_boot = of_property_read_bool(np, "fsl,auto-boot");
 
-	if (dcfg->flags & IMX_RPROC_NEED_SYSTEM_OFF) {
-		/*
-		 * setup mailbox to non-blocking mode in
-		 * [SYS_OFF_MODE_POWER_OFF_PREPARE, SYS_OFF_MODE_RESTART_PREPARE]
-		 * phase before invoking [SYS_OFF_MODE_POWER_OFF, SYS_OFF_MODE_RESTART]
-		 * atomic chain, see kernel/reboot.c.
-		 */
-		ret = devm_register_sys_off_handler(dev, SYS_OFF_MODE_POWER_OFF_PREPARE,
-						    SYS_OFF_PRIO_DEFAULT,
-						    imx_rproc_sys_off_handler, rproc);
-		if (ret)
-			return dev_err_probe(dev, ret, "register power off handler failure\n");
-
-		ret = devm_register_sys_off_handler(dev, SYS_OFF_MODE_RESTART_PREPARE,
-						    SYS_OFF_PRIO_DEFAULT,
-						    imx_rproc_sys_off_handler, rproc);
-		if (ret)
-			return dev_err_probe(dev, ret, "register restart handler failure\n");
-	}
-
 	pm_runtime_enable(dev);
 	ret = pm_runtime_resume_and_get(dev);
 	if (ret)
@@ -1424,7 +1405,6 @@ static const struct imx_rproc_dcfg imx_rproc_cfg_imx8ulp = {
 static const struct imx_rproc_dcfg imx_rproc_cfg_imx7ulp = {
 	.att		= imx_rproc_att_imx7ulp,
 	.att_size	= ARRAY_SIZE(imx_rproc_att_imx7ulp),
-	.flags		= IMX_RPROC_NEED_SYSTEM_OFF,
 };
 
 static const struct imx_rproc_dcfg imx_rproc_cfg_imx7d = {
diff --git a/drivers/remoteproc/imx_rproc.h b/drivers/remoteproc/imx_rproc.h
index 0d7d48352a10..9a1400d0d081 100644
--- a/drivers/remoteproc/imx_rproc.h
+++ b/drivers/remoteproc/imx_rproc.h
@@ -16,7 +16,6 @@ struct imx_rproc_att {
 };
 
 /* dcfg flags */
-#define IMX_RPROC_NEED_SYSTEM_OFF	BIT(0)
 #define IMX_RPROC_NEED_CLKS		BIT(1)
 
 struct imx_rproc_plat_ops {

-- 
2.56.0

[PATCH RFC v2 10/11] dt-bindings: remoteproc: imx-rproc: support virtio

[Francesco Valla]

Add support for virtio devices description for i.MX remoteproc devices.

Signed-off-by: Francesco Valla <francesco@valla.it>
---
 Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
index c18f71b64889..32fc415c7028 100644
--- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
@@ -92,6 +92,7 @@ required:
   - compatible
 
 allOf:
+  - $ref: remoteproc-virtio.yaml#
   - if:
       properties:
         compatible:
@@ -118,7 +119,7 @@ allOf:
       properties:
         power-domains: false
 
-additionalProperties: false
+unevaluatedProperties: false
 
 examples:
   - |

-- 
2.56.0

[PATCH RFC v2 11/11] PoC: arm64: dts: imx93-11x11-frdm: add multiple vdevs

[Francesco Valla]

Add rings and buffers for multiple virtio devices exposed by the
firmware running on the Cortex-M33 processor which is part of the i.MX93.
It is expected that such firmware exposes, through its resource table,
the following vdevs:

  - vdev0: rpmsg
  - vdev1: entropy
  - vdev2: gpio
  - vdev3: i2c
  - vdev4: spi
  - vdev5: can

The order in which the devices are contained inside the resource table
is critical, since it is used both to associate vrings and buffers to a
vdev and to map additional dt nodes to them.

All vdev buffers except the one for the rpmsg bus are declared as
restricted-dma-pool, because the corresponding drivers need bounce
buffering for the communication to work correctly.

Nodes for the gpio and i2c vdevs with example children devices are
included.

NOTE: this is a Proof-Of-Concept, not meant to be integrated!

Signed-off-by: Francesco Valla <francesco@valla.it>

---

Notable diff from RFC v1: the rsc-table node has been re-introduced to
focus on the changes that are truly in scope for the series. I still
believe something is off with the current implementation, but I'll
tackle it separately.
---
 arch/arm64/boot/dts/freescale/imx93-11x11-frdm.dts | 188 +++++++++++++++++++++
 1 file changed, 188 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx93-11x11-frdm.dts b/arch/arm64/boot/dts/freescale/imx93-11x11-frdm.dts
index 6e8351d12016..84324b02c45d 100644
--- a/arch/arm64/boot/dts/freescale/imx93-11x11-frdm.dts
+++ b/arch/arm64/boot/dts/freescale/imx93-11x11-frdm.dts
@@ -137,3 +137,191 @@ MX93_PAD_SD3_DATA3__GPIO3_IO25		0x31e
 		>;
 	};
 };
+
+/* Multi-vdev PoC */
+
+#include <dt-bindings/input/linux-event-codes.h>
+
+/ {
+	reserved-memory {
+		rsc_table: rsc-table@2021e000 {
+			reg = <0 0x2021e000 0 0x1000>;
+			no-map;
+		};
+
+		/* vdev0: rpmsg */
+		vdev0vring0: vdev0vring0@a4000000 {
+			reg = <0 0xa4000000 0 0x8000>;
+			no-map;
+		};
+
+		vdev0vring1: vdev0vring1@a4008000 {
+			reg = <0 0xa4008000 0 0x8000>;
+			no-map;
+		};
+
+		vdev0buffer: vdev0buffer@a4020000 {
+			compatible = "shared-dma-pool";
+			reg = <0 0xa4020000 0 0x100000>;
+			no-map;
+		};
+
+		/* vdev1: entropy */
+		vdev1vring0: vdev1vring0@a4010000 {
+			reg = <0 0xa4010000 0 0x1000>;
+			no-map;
+		};
+
+		/delete-node/ vdev1vring1@a4018000;
+
+		vdev1buffer: vdev1buffer@a4120000 {
+			compatible = "restricted-dma-pool";
+			reg = <0 0xa4120000 0 0x10000>;
+		};
+
+		/* vdev2: GPIO */
+		vdev2vring0: vdev2vring0@a4011000 {
+			reg = <0 0xa4011000 0 0x2000>;
+			no-map;
+		};
+
+		vdev2vring1: vdev2vring1@a4013000 {
+			reg = <0 0xa4013000 0 0x2000>;
+			no-map;
+		};
+
+		vdev2buffer: vdev2buffer@a4130000 {
+			compatible = "restricted-dma-pool";
+			reg = <0 0xa4130000 0 0x20000>;
+		};
+
+		/* vdev3: I2C */
+		vdev3vring0: vdev3vring0@a4015000 {
+			reg = <0 0xa4015000 0 0x2000>;
+			no-map;
+		};
+
+		vdev3buffer: vdev3buffer@a4150000 {
+			compatible = "restricted-dma-pool";
+			reg = <0 0xa4150000 0 0x20000>;
+		};
+
+		/* vdev4: SPI */
+		vdev4vring0: vdev4vring0@a4017000 {
+			reg = <0 0xa4017000 0 0x4000>;
+			no-map;
+		};
+
+		vdev4buffer: vdev4buffer@a4170000 {
+			compatible = "restricted-dma-pool";
+			reg = <0 0xa4170000 0 0x90000>;
+		};
+
+		/* vdev5: CAN */
+		vdev5vring0: vdev5vring0@a401b000 {
+			reg = <0 0xa401b000 0 0x2000>;
+			no-map;
+		};
+
+		vdev5vring1: vdev5vring1@a401d000 {
+			reg = <0 0xa401d000 0 0x2000>;
+			no-map;
+		};
+
+		vdev5vring2: vdev5vring2@a401f000 {
+			reg = <0 0xa401f000 0 0x1000>;
+			no-map;
+		};
+
+		vdev5buffer: vdev5buffer@a4200000 {
+			compatible = "restricted-dma-pool";
+			reg = <0 0xa4200000 0 0x80000>;
+		};
+	};
+
+	gpio-keys-virtio {
+		compatible = "gpio-keys";
+
+		button-v1 {
+			label = "Button V1";
+			linux,code = <BTN_3>;
+			gpios = <&v_gpio 23 GPIO_ACTIVE_LOW>;
+		};
+
+		button-v2 {
+			label = "Button V2";
+			linux,code = <BTN_4>;
+			gpios = <&v_gpio 24 GPIO_ACTIVE_LOW>;
+		};
+	};
+
+	leds {
+		compatible = "gpio-leds";
+
+		led {
+			gpios = <&v_gpio 18 GPIO_ACTIVE_HIGH>;
+			label = "LED V";
+		};
+	};
+};
+
+&cm33 {
+	memory-region = <&vdev0buffer>, <&vdev0vring0>, <&vdev0vring1>,
+			<&vdev1vring0>, <&vdev2vring0>, <&vdev2vring1>,
+			<&vdev3vring0>, <&vdev4vring0>, <&vdev5vring0>,
+			<&vdev5vring1>, <&vdev5vring2>, <&rsc_table>;
+
+	virtio {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		/* vdev1: entropy */
+		vdev@1 {
+			reg = <1>;
+			memory-region = <&vdev1buffer>;
+		};
+
+		/* vdev2: GPIO */
+		vdev@2 {
+			reg = <2>;
+			memory-region = <&vdev2buffer>;
+
+			v_gpio: gpio {
+				compatible = "virtio,device29";
+				gpio-controller;
+				#gpio-cells = <2>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+			};
+		};
+
+		/* vdev3: I2C */
+		vdev@3 {
+			reg = <3>;
+			memory-region = <&vdev3buffer>;
+
+			i2c {
+				compatible = "virtio,device22";
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				eeprom@50 {
+					compatible = "atmel,24c1025";
+					reg = <0x50>;
+				};
+			};
+		};
+
+		/* vdev4: SPI */
+		vdev@4 {
+			reg = <4>;
+			memory-region = <&vdev4buffer>;
+		};
+
+		/* vdev5: CAN */
+		vdev@5 {
+			reg = <5>;
+			memory-region = <&vdev5buffer>;
+		};
+	};
+};

-- 
2.56.0

Re: [PATCH RFC v2 07/11] remoteproc: assign fwnode during vdev registration

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] remoteproc: unreleased reserved memory before register_virtio_device()
--

commit 3f46613846e39fe7244229a8a47044d9f38bc9cb
Author: Francesco Valla <francesco@valla.it>

remoteproc: assign fwnode during vdev registration

This commit looks up a child firmware node describing a virtio device
during vdev registration and assigns it to the rproc-virtio platform
device, using its memory-region when no parent carveout is provided.

> diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
> index 1496659ddff9e..950d65480be59 100644
> --- a/drivers/remoteproc/remoteproc_virtio.c
> +++ b/drivers/remoteproc/remoteproc_virtio.c
[ ... ]
> @@ -394,6 +396,23 @@ static void rproc_virtio_dev_release(struct device *dev)
>  	put_device(&rvdev->pdev->dev);
>  }
>  
> +/*
> + * Attach the memory region declared inside the vdev's own devicetree node.
> + *
> + * A restricted-dma-pool region is attached by of_dma_configure() when the
> + * platform device is probed, but it is released together with the virtio
> + * device (see rproc_virtio_dev_release()); attach it again if needed, e.g.
> + * when the remote processor is restarted during a crash recovery.
> + * Any other kind of region (e.g., shared-dma-pool) is attached only here.
> + */
> +static int rproc_virtio_init_vdev_mem(struct device *dev)
> +{
> +	if (is_swiotlb_for_alloc(dev))
> +		return 0;
> +
> +	return of_reserved_mem_device_init_by_idx(dev, dev->of_node, 0);
> +}
> +
>  /**
>   * rproc_add_virtio_dev() - register an rproc-induced virtio device
>   * @rvdev: the remote vdev
> @@ -410,6 +429,7 @@ static int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
>  	struct device *dev = &rvdev->pdev->dev;
>  	struct virtio_device *vdev;
>  	struct rproc_mem_entry *mem;
> +	bool has_mem_region;
>  	int ret;
>  
>  	if (rproc->ops->kick == NULL) {
> @@ -417,11 +437,24 @@ static int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
>  		return -EINVAL;
>  	}
>  
> +	has_mem_region = of_property_present(dev->of_node, "memory-region");
> +
>  	/* Try to find dedicated vdev buffer carveout */
>  	mem = rproc_find_carveout_by_name(rproc, "vdev%dbuffer", rvdev->index);
>  	if (mem) {
>  		phys_addr_t pa;
>  
> +		if (has_mem_region) {
> +			/*
> +			 * The carveout takes precedence: drop the region that
> +			 * of_dma_configure() might have attached at probe time.
> +			 */
> +			dev_warn(dev,
> +				 "vdev%dbuffer carveout found, ignoring memory-region\n",
> +				 rvdev->index);
> +			of_reserved_mem_device_release(dev);
> +		}
> +
>  		if (mem->of_resm_idx != -1) {
>  			struct device_node *np = rproc->dev.parent->of_node;
>  
> @@ -451,6 +484,14 @@ static int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
>  				return ret;
>  			}
>  		}
> +	} else if (has_mem_region) {
> +		/* Use the memory region declared in the vdev's own node */
> +		ret = rproc_virtio_init_vdev_mem(dev);
> +		if (ret) {
> +			dev_err(dev, "Can't associate vdev memory-region: %d\n",
> +				ret);
> +			return ret;
> +		}

[Severity: High]
What happens to the reserved memory attached to &rvdev->pdev->dev if the
virtio device is never registered?

When rproc_handle_vdev() sets pdev_info.fwnode and registers the platform
device via platform_device_register_full(), a restricted-dma-pool region
in the vdev node is attached during probe by:

  really_probe()
    -> platform_dma_configure()
      -> of_dma_configure()
        -> of_dma_configure_id()
          -> of_dma_set_restricted_buffer()
            -> of_reserved_mem_device_init_by_idx()

This allocates a struct rmem_assigned_device entry pointing to &pdev->dev
and links it into the global of_rmem_assigned_device_list.

Similarly, in rproc_add_virtio_dev(), rproc_virtio_init_vdev_mem() (or the
carveout / parent fallback paths) attaches reserved or coherent memory to
&rvdev->pdev->dev before vdev = kzalloc_obj(*vdev) is called:

  rproc_add_virtio_dev() {
      ...
      } else if (has_mem_region) {
          ret = rproc_virtio_init_vdev_mem(dev);
          ...
      }
      vdev = kzalloc_obj(*vdev);
      if (!vdev)
          return -ENOMEM;
  }

However, of_reserved_mem_device_release(&rvdev->pdev->dev) and
dma_release_coherent_memory(&rvdev->pdev->dev) are only called in
rproc_virtio_dev_release(), not in rproc_virtio_remove() or in the error
paths of rproc_virtio_probe() and rproc_add_virtio_dev().

If rproc_virtio_probe() fails, if rproc_fw_boot() or rproc_attach() fails
after rproc_handle_vdev() (for example in rproc_alloc_registered_carveouts(),
rproc_load_segments(), rproc_prepare_subdevices(), or rproc->ops->start()),
or if rproc_add_virtio_dev() fails when rproc->ops->kick is NULL or when
kzalloc_obj(*vdev) returns NULL, rproc_virtio_dev_release() never runs.

When rproc_resource_cleanup() then unregisters and frees rvdev->pdev,
doesn't that leak the struct rmem_assigned_device entry on
of_rmem_assigned_device_list with a dangling rd->dev pointer to the freed
platform device (and leave the memory region attached across crash
recovery attempts so dma_assign_coherent_memory() fails with -EBUSY)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102-remoteproc_virtio_map-v2-0-26b300e01c62@valla.it?part=7

Re: [PATCH RFC v2 09/11] remoteproc: imx_rproc: always use non-blocking mailboxes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] remoteproc: imx_rproc: memory leak of mmsg in imx_rproc_kick() on mbox abort
--

commit 059fe2437ad2cd1bd7152b14297e2a685bc5d803
Author: Francesco Valla <francesco@valla.it>

remoteproc: imx_rproc: always use non-blocking mailboxes

This commit switches the i.MX remoteproc mailbox client to non-blocking
mode, dynamically allocating and freeing kick messages via the tx_done
callback, and removes the sys_off handler that previously reopened the
mailbox in non-blocking mode during system shutdown and restart.

> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index 581bb07f5893f..a792b9ca5edd1 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c

[ ... ]

> @@ -696,28 +696,45 @@ static void imx_rproc_kick(struct rproc *rproc, int vqid)
>  {
>  	struct imx_rproc *priv = rproc->priv;
>  	int err;
> -	__u32 mmsg;
> +	__u32 *mmsg;
>  
>  	if (!priv->tx_ch) {
>  		dev_err(priv->dev, "No initialized mbox tx channel\n");
>  		return;
>  	}
>  
> +	mmsg = kzalloc_obj(*mmsg, GFP_ATOMIC);
> +	if (!mmsg) {
> +		dev_err_ratelimited(priv->dev, "cannot allocate kick message\n");
> +		return;
> +	}
> +
>  	/*
>  	 * Send the index of the triggered virtqueue as the mu payload.
>  	 * Let remote processor know which virtqueue is used.
>  	 */
> -	mmsg = vqid << 16;
> +	*mmsg = vqid << 16;
>  
> -	err = mbox_send_message(priv->tx_ch, (void *)&mmsg);
> -	if (err < 0)
> +	err = mbox_send_message(priv->tx_ch, (void *)mmsg);
> +	if (err < 0) {
>  		dev_err(priv->dev, "%s: failed (%d, err:%d)\n",
>  			__func__, vqid, err);
> +		kfree(mmsg);
> +	}
> +}
> +
> +static void imx_rproc_kick_tx_done(struct mbox_client *cl, void *mmsg, int r)
> +{
> +	if (r != 0)
> +		dev_err_ratelimited(cl->dev, "failed to kick vqid %u: %d\n",
> +				    *(__u32 *)mmsg >> 16, r);
> +
> +	kfree(mmsg);
>  }

[Severity: High]
Does this leak mmsg when the mailbox channel is freed or when transmission
does not complete with a tx_done callback?

In imx_rproc_kick(), once mbox_send_message() returns a non-negative ring
buffer index, freeing mmsg relies solely on imx_rproc_kick_tx_done() being
called from tx_tick().

However, there are several paths where imx_rproc_kick_tx_done() is never
invoked for a queued or active mmsg:

1. When imx_rproc_free_mbox() calls mbox_free_channel(priv->tx_ch) during
stop, detach, crash recovery, or driver teardown,
mbox_free_channel()->mbox_clean_and_put_channel() aborts the active request
and all queued requests without invoking cl->tx_done:

drivers/mailbox/mailbox.c:mbox_clean_and_put_channel() {
	/* The queued TX requests are simply aborted, no callbacks are made */
	scoped_guard(spinlock_irqsave, &chan->lock) {
		chan->cl = NULL;
		chan->active_req = MBOX_NO_MSG;
		if (chan->txdone_method == MBOX_TXDONE_BY_ACK)
			chan->txdone_method = MBOX_TXDONE_BY_POLL;
	}
...
}

Any in-flight kick in chan->active_req (such as kicks sent by virtio
subdevices during rproc_stop_subdevices() right before imx_rproc_stop()) or
queued kicks in chan->msg_data[] (for example, when the remote processor
crashes or stops reading the MU register) will be leaked.

2. In mbox_send_message()->msg_submit(), if chan->mbox->ops->send_data()
returns an error, chan->active_req is not set and mmsg remains in
chan->msg_data[] without tx_done being called, while mbox_send_message()
still returns the non-negative slot index t to imx_rproc_kick().

3. For IMX_MU_TYPE_TXDB_V2 channels, imx_mu_xlate() configures
txdone_method as MBOX_TXDONE_BY_ACK, while imx_rproc_xtr_mbox_init() sets
cl->knows_txdone = false and never calls mbox_client_txdone(), so tx_tick()
and imx_rproc_kick_tx_done() are never called.

Since mbox_send_message() only stores the void * pointer in
chan->msg_data[] and passes it to send_data(), could vqid (or vqid << 16)
be encoded directly in the pointer via uintptr_t or stored in a preallocated
per-vq/per-device array instead of dynamically allocating mmsg on every
kick?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102-remoteproc_virtio_map-v2-0-26b300e01c62@valla.it?part=9

Re: [PATCH RFC v2 11/11] PoC: arm64: dts: imx93-11x11-frdm: add multiple vdevs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] remoteproc: virtio: unreleased reserved memory and dangling dev ptr
- [Medium] arm64: dts: imx93-11x11-frdm: PoC multi-vdev changes not meant for integration
- [Medium] remoteproc: missing dma_mask and dma_range_map leak on DT vdevs
- [Medium] arm64: dts: imx93-11x11-frdm: fw_devlink moves v_gpio consumers to cm33
--

commit 36664d55421d235cfeb718886d0f002fe5e55c01
Author: Francesco Valla <francesco@valla.it>

PoC: arm64: dts: imx93-11x11-frdm: add multiple vdevs

This commit adds reserved-memory regions and devicetree nodes to
imx93-11x11-frdm.dts for a proof-of-concept multi-vdev Cortex-M33 firmware
configuration exposing rpmsg, entropy, GPIO, I2C, SPI, and CAN virtio
devices.

> diff --git a/arch/arm64/boot/dts/freescale/imx93-11x11-frdm.dts b/arch/arm64/boot/dts/freescale/imx93-11x11-frdm.dts
> index 6e8351d120160..84324b02c45dc 100644
> --- a/arch/arm64/boot/dts/freescale/imx93-11x11-frdm.dts
> +++ b/arch/arm64/boot/dts/freescale/imx93-11x11-frdm.dts
> @@ -137,3 +137,191 @@ MX93_PAD_SD3_DATA3__GPIO3_IO25		0x31e
>  		>;
>  	};
>  };
> +
> +/* Multi-vdev PoC */
> +
> +#include <dt-bindings/input/linux-event-codes.h>
> +
> +/ {
> +	reserved-memory {
> +		rsc_table: rsc-table@2021e000 {
> +			reg = <0 0x2021e000 0 0x1000>;
> +			no-map;
> +		};
> +
> +		/* vdev0: rpmsg */
> +		vdev0vring0: vdev0vring0@a4000000 {
> +			reg = <0 0xa4000000 0 0x8000>;
> +			no-map;
> +		};
> +
> +		vdev0vring1: vdev0vring1@a4008000 {
> +			reg = <0 0xa4008000 0 0x8000>;
> +			no-map;
> +		};
> +
> +		vdev0buffer: vdev0buffer@a4020000 {
> +			compatible = "shared-dma-pool";
> +			reg = <0 0xa4020000 0 0x100000>;
> +			no-map;
> +		};
> +
> +		/* vdev1: entropy */
> +		vdev1vring0: vdev1vring0@a4010000 {
> +			reg = <0 0xa4010000 0 0x1000>;
> +			no-map;
> +		};
> +
> +		/delete-node/ vdev1vring1@a4018000;
> +
> +		vdev1buffer: vdev1buffer@a4120000 {
> +			compatible = "restricted-dma-pool";
> +			reg = <0 0xa4120000 0 0x10000>;
> +		};

[Severity: Medium]
As reported by Sashiko on RFC [1], the commit message notes that this is a
proof-of-concept not meant to be integrated, and applying it directly to
arch/arm64/boot/dts/freescale/imx93-11x11-frdm.dts (which is also included by
imx93-11x11-frdm-pixpaper.dts) modifies the production board devicetree.

In particular, this shrinks vdev1vring0@a4010000 from 0x8000 to 0x1000,
deletes vdev1vring1@a4018000, repurposes 0xa4011000..0xa4020000 for
vdev2..vdev5 vrings, duplicates rsc_table, vdev0vring0, vdev0vring1, and
vdev0buffer from imx93-11x11-frdm-common.dtsi, carves out an extra 1.375 MiB
of RAM (0xa4120000..0xa4280000) as non-reusable restricted-dma-pool regions,
and adds board-level gpio-keys-virtio, leds, and eeprom@50 nodes tied to a
specific Cortex-M33 firmware image.

Would this break backwards compatibility with standard NXP MCUXpresso SDK
Cortex-M33 RPMSG firmware if merged into the main board DTS?

[ ... ]

> +&cm33 {
> +	memory-region = <&vdev0buffer>, <&vdev0vring0>, <&vdev0vring1>,
> +			<&vdev1vring0>, <&vdev2vring0>, <&vdev2vring1>,
> +			<&vdev3vring0>, <&vdev4vring0>, <&vdev5vring0>,
> +			<&vdev5vring1>, <&vdev5vring2>, <&rsc_table>;
> +
> +	virtio {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		/* vdev1: entropy */
> +		vdev@1 {
> +			reg = <1>;
> +			memory-region = <&vdev1buffer>;
> +		};

[Severity: Medium]
When vdev@1..vdev@5 are registered in rproc_handle_vdev(), pdev_info.fwnode
is populated while pdev_info.dma_mask is left as 0:

drivers/remoteproc/remoteproc_core.c:rproc_handle_vdev() {
	struct platform_device_info pdev_info = {
		.parent = dev,
		.name = "rproc-virtio",
		.id = PLATFORM_DEVID_AUTO,
		.data = &rvdev_data,
		.size_data = sizeof(rvdev_data),
	};
	...
	fwnode = rproc_get_vdev_fwnode(dev->parent, rvdev_data.index);
	pdev_info.fwnode = fwnode;
	...
	pdev = platform_device_register_full(&pdev_info);
}

Because pdev_info.dma_mask is 0, pdev->dev.dma_mask remains NULL when
really_probe() -> platform_dma_configure() -> of_dma_configure() ->
of_dma_configure_id() runs before rproc_virtio_probe(). In
of_dma_configure_id(), !dev->dma_mask triggers dev_warn(dev, "DMA mask not
set\n") for every vdev with a matching DT node.

In addition, if an ancestor DT node defines dma-ranges, of_dma_configure_id()
allocates a struct bus_dma_region array via of_dma_get_range() and assigns it
to dev->dma_range_map, which rproc_virtio_probe() -> copy_dma_range_map()
then overwrites without freeing:

drivers/remoteproc/remoteproc_virtio.c:copy_dma_range_map() {
	new_map = kmemdup_array(map, num_ranges + 1, sizeof(*map), GFP_KERNEL);
	if (!new_map)
		return -ENOMEM;

	to->dma_range_map = new_map;
}

Does probing these DT-described vdevs trigger the "DMA mask not set" warning
and leak the dev->dma_range_map array allocated by of_dma_configure_id()?

[Severity: High]
Because vdev@1..vdev@5 have an OF fwnode whose memory-region points to a
restricted-dma-pool, really_probe() calls:

  platform_dma_configure()
    -> of_dma_configure_id()
      -> of_dma_set_restricted_buffer()
        -> of_reserved_mem_device_init_by_idx(&pdev->dev, of_node, i)

before rproc_virtio_probe() runs, adding a struct rmem_assigned_device entry
with rd->dev = &pdev->dev to the global of_rmem_assigned_device_list.

However, of_reserved_mem_device_release(&rvdev->pdev->dev) and
dma_release_coherent_memory(&rvdev->pdev->dev) are only called in
rproc_virtio_dev_release() when the child struct virtio_device is released:

drivers/remoteproc/remoteproc_virtio.c:rproc_virtio_dev_release() {
	of_reserved_mem_device_release(&rvdev->pdev->dev);
	dma_release_coherent_memory(&rvdev->pdev->dev);
}

Neither rproc_virtio_remove() nor the error paths in rproc_virtio_probe() and
rproc_add_virtio_dev() release the memory region attached to &pdev->dev.

If rproc_virtio_probe() fails after platform_dma_configure(), if
rproc_fw_boot() or rproc_attach() fails before rproc_start_subdevices()
reaches this rvdev and cleans up via rproc_resource_cleanup() ->
platform_device_unregister(rvdev->pdev) -> rproc_virtio_remove(), or if
kzalloc_obj(*vdev) in rproc_add_virtio_dev() returns -ENOMEM after
rproc_virtio_init_vdev_mem(), won't rvdev->pdev be freed while its
rmem_assigned_device entry remains on of_rmem_assigned_device_list with a
dangling rd->dev pointer?

> +
> +		/* vdev2: GPIO */
> +		vdev@2 {
> +			reg = <2>;
> +			memory-region = <&vdev2buffer>;
> +
> +			v_gpio: gpio {
> +				compatible = "virtio,device29";
> +				gpio-controller;
> +				#gpio-cells = <2>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +			};
> +		};

[Severity: Medium]
Here v_gpio is placed under &cm33/virtio/vdev@2 and referenced by the
top-level gpio-keys-virtio and leds consumer nodes.

When imx_rproc_probe() finishes probing &cm33, devm_rproc_add() ->
rproc_add() either schedules rproc->boot_work asynchronously or leaves the
remoteproc offline if auto_boot is false, returning 0 before
rproc_handle_vdev() or register_virtio_device() creates any child devices.

When imx_rproc_probe() returns 0, really_probe() -> driver_bound() ->
device_links_driver_bound(&pdev->dev) runs
fw_devlink_pickup_dangling_consumers(&pdev->dev) on &cm33. Because the
descendant fwnodes virtio, vdev@2, and gpio (v_gpio) all have
fwnode->dev == NULL at that point, __fw_devlink_pickup_dangling_consumers()
marks them with FWNODE_FLAG_NOT_DEVICE and moves v_gpio's consumer links
(gpio-keys-virtio and leds) onto &cm33.

When the remote processor is later stopped via rproc_shutdown() or undergoes
crash recovery via rproc_trigger_recovery(), &cm33 stays bound while
rproc_vdev_do_stop() unregisters the virtio-gpio device and gpiochip_remove()
tears down the gpio_chip and frees active GPIO IRQs. Because gpio-keys-virtio
and leds only have a managed device_link to &cm33 rather than to the GPIO
device, won't they remain bound to a dead gpio_device across remoteproc
stop/restart cycles?

[1] https://lore.kernel.org/r/20260916-remoteproc_virtio_map-v1-12-dac8c5eb4aa9@valla.it

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102-remoteproc_virtio_map-v2-0-26b300e01c62@valla.it?part=11