[RFC PATCH 03/10] remoteproc: add remoteproc virtio transport driver

Simon Glass sjg at chromium.org
Thu Jul 27 02:50:13 CEST 2023


Hi Tanmay,

On Tue, 25 Jul 2023 at 08:08, Tanmay Shah <tanmay.shah at amd.com> wrote:
>
> Remoteproc virtio is virtio transport layer driver for remoteproc.
> Implement virtio config ops used by actual virtio
> driver using remoteproc virtio device
>
> Ported from the Linux kernel version 6.4-rc2 (d848a4819d85),
> Introduced in kernel version v3.10 (ac8954a41393)
> file: drivers/remoteproc/remoteproc_virtio.c.
>
> Signed-off-by: Tanmay Shah <tanmay.shah at amd.com>
> ---
>  MAINTAINERS                       |   6 +
>  drivers/remoteproc/Kconfig        |  11 +
>  drivers/remoteproc/Makefile       |   1 +
>  drivers/remoteproc/rproc-uclass.c |  42 ++-
>  drivers/remoteproc/rproc_virtio.c | 422 ++++++++++++++++++++++++++++++
>  drivers/virtio/virtio_ring.c      |  16 ++
>  include/remoteproc.h              |  66 +++--
>  include/rproc_virtio.h            |  29 ++
>  include/virtio.h                  |   3 +
>  include/virtio_ring.h             |  17 ++
>  10 files changed, 593 insertions(+), 20 deletions(-)
>  create mode 100644 drivers/remoteproc/rproc_virtio.c
>  create mode 100644 include/rproc_virtio.h

Can you split this patch up a bit? It seems to add a new method and
lots of other stuff.

>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 87991cccdd..c4a32a0956 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1319,6 +1319,12 @@ S:       Maintained
>  T:     git https://source.denx.de/u-boot/custodians/u-boot-nand-flash.git
>  F:     drivers/mtd/nand/raw/
>
> +REMOTEPROC
> +M:     Tanmay Shah <tanmay.shah at amd.com>
> +S:     Maintained
> +F:     drivers/remoteproc/rproc_virtio.c
> +F:     include/rproc_virtio.h
> +
>  RISC-V
>  M:     Rick Chen <rick at andestech.com>
>  M:     Leo <ycliang at andestech.com>
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index 27e4a60ff5..b758c248e4 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -102,4 +102,15 @@ config REMOTEPROC_TI_IPU
>         help
>           Say 'y' here to add support for TI' K3 remoteproc driver.
>
> +config REMOTEPROC_VIRTIO
> +       bool "Support remoteproc virtio devices"
> +       select REMOTEPROC
> +       select VIRTIO
> +       depends on DM
> +       help
> +         Say 'y' here to add support of remoteproc virtio devices.
> +         rproc_virtio is virtio transport layer driver. The transport
> +         drivers provide a set of ops for the real virtio device
> +         driver to call.
> +
>  endmenu
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index fbe9c172bc..61fdb87efb 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -16,3 +16,4 @@ obj-$(CONFIG_REMOTEPROC_TI_K3_R5F) += ti_k3_r5f_rproc.o
>  obj-$(CONFIG_REMOTEPROC_TI_POWER) += ti_power_proc.o
>  obj-$(CONFIG_REMOTEPROC_TI_PRU) += pru_rproc.o
>  obj-$(CONFIG_REMOTEPROC_TI_IPU) += ipu_rproc.o
> +obj-$(CONFIG_REMOTEPROC_VIRTIO) += rproc_virtio.o
> diff --git a/drivers/remoteproc/rproc-uclass.c b/drivers/remoteproc/rproc-uclass.c
> index d697639cdd..3aebaf6187 100644
> --- a/drivers/remoteproc/rproc-uclass.c
> +++ b/drivers/remoteproc/rproc-uclass.c
> @@ -14,6 +14,7 @@
>  #include <malloc.h>
>  #include <virtio_ring.h>
>  #include <remoteproc.h>
> +#include <rproc_virtio.h>
>  #include <asm/io.h>
>  #include <dm/device-internal.h>
>  #include <dm.h>
> @@ -279,6 +280,33 @@ static int rproc_config_pagetable(struct udevice *dev, unsigned int virt,
>         return 0;
>  }
>
> +/**
> + * rproc_find_res_by_name() - After parsing the resource table add the mappings
> + * @dev:       device we finished probing
> + * @name: name of rproc_mem_entry resource
> + *
> + * Return: If failed NULL, else first carveout entry with matching name.
> + */
> +struct rproc_mem_entry *rproc_find_res_by_name(struct udevice *dev,
> +                                              const char *name)
> +{
> +       struct rproc *rproc = rproc_get_cfg(dev);
> +       struct rproc_mem_entry *mapping = NULL;
> +       int ret;
> +
> +       if (!rproc)
> +               return NULL;
> +
> +       list_for_each_entry(mapping, &rproc->mappings.node, node) {
> +               ret = strcmp(mapping->name, name);
> +               if (!ret)

Do we need ret?

> +                       return mapping;
> +       }
> +
> +       debug("%s: %s carveout not found\n", dev->name, name);

We typically put a blank line before the final return

> +       return NULL;
> +}
> +
>  UCLASS_DRIVER(rproc) = {
>         .id = UCLASS_REMOTEPROC,
>         .name = "remoteproc",
> @@ -688,6 +716,7 @@ static int alloc_vring(struct udevice *dev, struct fw_rsc_vdev *rsc, int i)
>  static int handle_vdev(struct udevice *dev, struct fw_rsc_vdev *rsc,
>                        int offset, int avail)
>  {
> +       struct rproc *rproc;
>         int i, ret;
>         void *pa;
>
> @@ -720,7 +749,18 @@ static int handle_vdev(struct udevice *dev, struct fw_rsc_vdev *rsc,
>         }
>
>         /*
> -        * allocate the vrings
> +        * If virtio device creation is supported, then prefer to get vrings
> +        * during find_vq op
> +        */
> +       rproc = rproc_get_cfg(dev);
> +
> +       if (rproc && rproc->support_rpmsg_virtio &&
> +           !(IS_ENABLED(CONFIG_SANDBOX))) {

Why the special case for sandbox? Can't it work like real hardware?

> +               return rproc_virtio_create_dev(dev, rsc, offset);
> +       }
> +
> +       /*
> +        * allocate the vrings traditional way
>          */

Single-line comment style

>         for (i = 0; i < rsc->num_of_vrings; i++) {
>                 ret = alloc_vring(dev, rsc, i);
> diff --git a/drivers/remoteproc/rproc_virtio.c b/drivers/remoteproc/rproc_virtio.c
> new file mode 100644
> index 0000000000..5e7cdfa12f
> --- /dev/null
> +++ b/drivers/remoteproc/rproc_virtio.c
> @@ -0,0 +1,422 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Remote Processor Messaging transport
> + *
> + * Copyright (C) 2011 Texas Instruments, Inc.
> + * Copyright (C) 2011 Google, Inc.
> + * Copyright (C) 2023, Advanced Micro Devices Inc.
> + *
> + * VirtIO RPMsg transport driver
> + * Ported from Linux drivers/remoteproc/remoteproc_virtio.c
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <log.h>
> +#include <remoteproc.h>
> +#include <rproc_virtio.h>
> +#include <virtio.h>
> +#include <virtio_ring.h>
> +#include <virtio_types.h>
> +#include <asm/io.h>
> +#include <dm/device_compat.h>
> +#include <dm/device-internal.h>
> +#include <linux/bug.h>
> +#include <linux/compat.h>
> +#include <linux/err.h>
> +
> +/**
> + * rproc_flush_dcache() - flush shared memory regions
> + * @dev: valid remoteproc virtio device
> + *
> + * Some platforms have dcache enabled. If dcache is on then data written
> + * to shared memory is actually written to cache memory. This function
> + * checks if cache is on or not and if it is on, then it performs
> + * flush and invalidate operation on address range or reserved memory
> + * created by platform driver for remoteproc
> + *
> + * Return: none
> + */
> +void rproc_flush_dcache(struct udevice *dev)
> +{
> +       struct rproc *rproc = rproc_get_cfg(dev->parent);
> +       struct rproc_mem_entry *mapping = NULL;
> +       struct list_head *mapping_node = NULL;
> +
> +       /* If dcache is off, don't perform cache operation */
> +       if (dcache_status() == false)

if (!dcache_status())

> +               return;
> +
> +       /*
> +        * If cache is on, then flush cache
> +        * specially reserved mem regions for,
> +        * resource table, vrings and vdevbuffer of platform dev
> +        */
> +       list_for_each(mapping_node, &rproc->mappings.node) {
> +               mapping = container_of(mapping_node, struct rproc_mem_entry,
> +                                      node);
> +
> +               /*
> +                * First flush data so current data from cache is written
> +                * to actual memory from cache. Then invalidate cache so
> +                * next time data will be accessed from actual memory
> +                */
> +               flush_dcache_range(mapping->da, mapping->da + mapping->len);
> +               invalidate_dcache_range(mapping->da, mapping->da + mapping->len);
> +       }
> +}

[..]

Regards,
Simon


More information about the U-Boot mailing list