2020-07-31 11:50:02

by Arnaud POULIQUEN

[permalink] [raw]
Subject: [PATCH 0/9] introduce name service announcement rpmsg driver

The NS announcement is implemented by several backends, but could be
considered as part the RPMsg protocol.
In this case it should be managed as a reserved rpmsg service and so
implemented on top of the rpmsg protocol.

This series introduces the rpmsg_ns driver that handles the name service
announcement. The virtio backend is updated in consequence to use this
service.

Applies cleanly on Bjorn rpmsg-next branch (ddd1930d6e3e)

Arnaud Pouliquen (9):
rpmsg: virtio: rename rpmsg_create_channel
rpmsg: core: add channel creation internal API
rpmsg: virtio: add rpmsg channel device ops
rpmsg: define the name service announcement as reserved address
rpmsg: introduce reserved rpmsg driver for ns announcement
rpmsg: virtio: use rpmsg ns device for the ns announcement
rpmsg: ns: add name service announcement service
rpmsg: virtio: use rpmsg_ns driver to manage ns announcement
rpmsg: ns: name service announcement endianness

drivers/rpmsg/Kconfig | 9 ++
drivers/rpmsg/Makefile | 1 +
drivers/rpmsg/rpmsg_core.c | 37 ++++++
drivers/rpmsg/rpmsg_internal.h | 32 +++++
drivers/rpmsg/rpmsg_ns.c | 175 +++++++++++++++++++++++++
drivers/rpmsg/virtio_rpmsg_bus.c | 213 +++++++++----------------------
include/linux/rpmsg.h | 9 ++
7 files changed, 325 insertions(+), 151 deletions(-)
create mode 100644 drivers/rpmsg/rpmsg_ns.c

--
2.17.1


2020-07-31 11:50:10

by Arnaud POULIQUEN

[permalink] [raw]
Subject: [PATCH 1/9] rpmsg: virtio: rename rpmsg_create_channel

Rename the internal function as it is internal, and as
the name will be used in rpmsg_core.

Signed-off-by: Arnaud Pouliquen <[email protected]>
---
drivers/rpmsg/virtio_rpmsg_bus.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index 9006fc7f73d0..736bc7b9dea8 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -390,8 +390,9 @@ static void virtio_rpmsg_release_device(struct device *dev)
* this function will be used to create both static and dynamic
* channels.
*/
-static struct rpmsg_device *rpmsg_create_channel(struct virtproc_info *vrp,
- struct rpmsg_channel_info *chinfo)
+static struct rpmsg_device *
+__rpmsg_create_channel(struct virtproc_info *vrp,
+ struct rpmsg_channel_info *chinfo)
{
struct virtio_rpmsg_channel *vch;
struct rpmsg_device *rpdev;
@@ -864,7 +865,7 @@ static int rpmsg_ns_cb(struct rpmsg_device *rpdev, void *data, int len,
if (ret)
dev_err(dev, "rpmsg_destroy_channel failed: %d\n", ret);
} else {
- newch = rpmsg_create_channel(vrp, &chinfo);
+ newch = __rpmsg_create_channel(vrp, &chinfo);
if (!newch)
dev_err(dev, "rpmsg_create_channel failed\n");
}
--
2.17.1

2020-07-31 11:50:21

by Arnaud POULIQUEN

[permalink] [raw]
Subject: [PATCH 7/9] rpmsg: ns: add name service announcement service

As the RPMsg driver is in charge of the name service announcement,
create an API to send channel creation and destruction to the remote
processor.
Notice that the source address of the message sent is now RPMSG_NS_ADDR.
Legacy implementation was to send the message with source address
corresponding to the ept created.
RPMSG_NS_ADDR as source address make sense as we want to send a message
belonging to the NS announcement service and the created ept address is
already in the message payload.

Signed-off-by: Arnaud Pouliquen <[email protected]>
---
drivers/rpmsg/rpmsg_internal.h | 3 +++
drivers/rpmsg/rpmsg_ns.c | 39 ++++++++++++++++++++++++++++++++++
2 files changed, 42 insertions(+)

diff --git a/drivers/rpmsg/rpmsg_internal.h b/drivers/rpmsg/rpmsg_internal.h
index 641b48f6bf2a..d1549e5cb607 100644
--- a/drivers/rpmsg/rpmsg_internal.h
+++ b/drivers/rpmsg/rpmsg_internal.h
@@ -119,4 +119,7 @@ static inline int rpmsg_ns_register_device(struct rpmsg_device *rpdev)
return rpmsg_register_device(rpdev);
}

+int rpmsg_ns_announce_create(struct rpmsg_device *rpdev);
+int rpmsg_ns_announce_destroy(struct rpmsg_device *rpdev);
+
#endif
diff --git a/drivers/rpmsg/rpmsg_ns.c b/drivers/rpmsg/rpmsg_ns.c
index fe7713e737c2..cc2bd47c415a 100644
--- a/drivers/rpmsg/rpmsg_ns.c
+++ b/drivers/rpmsg/rpmsg_ns.c
@@ -37,6 +37,45 @@ struct rpmsg_ns_msg {
u32 flags;
} __packed;

+/**
+ * rpmsg_ns_announce_create() -announce to the remote processor
+ * the service creation or destruction
+ * @rpdev: the rpmsg channel
+ * @ns_flag: related to the @rpmsg_ns_flags enum
+ *
+ * This function can be called on a channel creation or destruction to inform
+ * the remote processor, using the reserved name service anouncement channel.
+ */
+static int rpmsg_ns_channel_announce(struct rpmsg_device *rpdev, int ns_flag)
+{
+ struct rpmsg_ns_msg nsm;
+
+ if (!rpdev->announce || !rpdev->ept)
+ return 0;
+
+ if (ns_flag != RPMSG_NS_CREATE && ns_flag != RPMSG_NS_DESTROY)
+ return -EINVAL;
+
+ strncpy(nsm.name, rpdev->id.name, RPMSG_NAME_SIZE);
+ nsm.addr = rpdev->ept->addr;
+ nsm.flags = ns_flag;
+
+ return rpmsg_send_offchannel(rpdev->ept, RPMSG_NS_ADDR, RPMSG_NS_ADDR,
+ &nsm, sizeof(nsm));
+}
+
+int rpmsg_ns_announce_create(struct rpmsg_device *rpdev)
+{
+ return rpmsg_ns_channel_announce(rpdev, RPMSG_NS_CREATE);
+}
+EXPORT_SYMBOL(rpmsg_ns_announce_create);
+
+int rpmsg_ns_announce_destroy(struct rpmsg_device *rpdev)
+{
+ return rpmsg_ns_channel_announce(rpdev, RPMSG_NS_DESTROY);
+}
+EXPORT_SYMBOL(rpmsg_ns_announce_destroy);
+
/* invoked when a name service announcement arrives */
static int rpmsg_ns_cb(struct rpmsg_device *rpdev, void *data, int len,
void *priv, u32 src)
--
2.17.1

2020-07-31 11:51:13

by Arnaud POULIQUEN

[permalink] [raw]
Subject: [PATCH 2/9] rpmsg: core: add channel creation internal API

Add the channel creation API as a first step to be able to define the
name service announcement as a rpmsg driver independent from the RPMsg
virtio bus.

Signed-off-by: Arnaud Pouliquen <[email protected]>
---
drivers/rpmsg/rpmsg_core.c | 37 ++++++++++++++++++++++++++++++++++
drivers/rpmsg/rpmsg_internal.h | 12 +++++++++++
2 files changed, 49 insertions(+)

diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
index a6361cad608b..ae7da4a2e528 100644
--- a/drivers/rpmsg/rpmsg_core.c
+++ b/drivers/rpmsg/rpmsg_core.c
@@ -20,6 +20,43 @@

#include "rpmsg_internal.h"

+/**
+ * rpmsg_create_channel() - create a new rpmsg channel
+ * using its name and address info.
+ * @rpdev: rpmsg driver
+ * @chinfo: channel_info to bind
+ *
+ * Returns a pointer to the new rpmsg device on success, or NULL on error.
+ */
+struct rpmsg_device *
+ rpmsg_create_channel(struct rpmsg_device *rpdev,
+ struct rpmsg_channel_info *chinfo)
+{
+ if (WARN_ON(!rpdev) || !rpdev->ops->create_channel)
+ return NULL;
+
+ return rpdev->ops->create_channel(rpdev, chinfo);
+}
+EXPORT_SYMBOL(rpmsg_create_channel);
+
+/**
+ * rpmsg_release_channel() - release a rpmsg channel
+ * using its name and address info.
+ * @rpdev: rpmsg driver
+ * @chinfo: channel_info to bind
+ *
+ * Returns a pointer to the new rpmsg device on success, or NULL on error.
+ */
+int rpmsg_release_channel(struct rpmsg_device *rpdev,
+ struct rpmsg_channel_info *chinfo)
+{
+ if (WARN_ON(!rpdev) || !rpdev->ops->release_channel)
+ return 0;
+
+ return rpdev->ops->release_channel(rpdev, chinfo);
+}
+EXPORT_SYMBOL(rpmsg_release_channel);
+
/**
* rpmsg_create_ept() - create a new rpmsg_endpoint
* @rpdev: rpmsg channel device
diff --git a/drivers/rpmsg/rpmsg_internal.h b/drivers/rpmsg/rpmsg_internal.h
index 3fc83cd50e98..d5ab286d0e5e 100644
--- a/drivers/rpmsg/rpmsg_internal.h
+++ b/drivers/rpmsg/rpmsg_internal.h
@@ -20,6 +20,8 @@

/**
* struct rpmsg_device_ops - indirection table for the rpmsg_device operations
+ * @create_channel: create backend-specific channel, optional
+ * @release_channel: release backend-specific channel, optional
* @create_ept: create backend-specific endpoint, required
* @announce_create: announce presence of new channel, optional
* @announce_destroy: announce destruction of channel, optional
@@ -29,6 +31,11 @@
* advertise new channels implicitly by creating the endpoints.
*/
struct rpmsg_device_ops {
+ struct rpmsg_device *(*create_channel)(struct rpmsg_device *rpdev,
+ struct rpmsg_channel_info *chinfo);
+ int (*release_channel)(struct rpmsg_device *rpdev,
+ struct rpmsg_channel_info *chinfo);
+
struct rpmsg_endpoint *(*create_ept)(struct rpmsg_device *rpdev,
rpmsg_rx_cb_t cb, void *priv,
struct rpmsg_channel_info chinfo);
@@ -75,6 +82,11 @@ int rpmsg_unregister_device(struct device *parent,
struct device *rpmsg_find_device(struct device *parent,
struct rpmsg_channel_info *chinfo);

+struct rpmsg_device *
+ rpmsg_create_channel(struct rpmsg_device *rpdev,
+ struct rpmsg_channel_info *chinfo);
+int rpmsg_release_channel(struct rpmsg_device *rpdev,
+ struct rpmsg_channel_info *chinfo);
/**
* rpmsg_chrdev_register_device() - register chrdev device based on rpdev
* @rpdev: prepared rpdev to be used for creating endpoints
--
2.17.1

2020-08-21 00:01:34

by Mathieu Poirier

[permalink] [raw]
Subject: Re: [PATCH 0/9] introduce name service announcement rpmsg driver

On Fri, Jul 31, 2020 at 01:47:23PM +0200, Arnaud Pouliquen wrote:
> The NS announcement is implemented by several backends, but could be
> considered as part the RPMsg protocol.
> In this case it should be managed as a reserved rpmsg service and so
> implemented on top of the rpmsg protocol.
>
> This series introduces the rpmsg_ns driver that handles the name service
> announcement. The virtio backend is updated in consequence to use this
> service.

I have started to review this set, comments will be spread over a few days.

Thanks,
Mathieu

>
> Applies cleanly on Bjorn rpmsg-next branch (ddd1930d6e3e)
>
> Arnaud Pouliquen (9):
> rpmsg: virtio: rename rpmsg_create_channel
> rpmsg: core: add channel creation internal API
> rpmsg: virtio: add rpmsg channel device ops
> rpmsg: define the name service announcement as reserved address
> rpmsg: introduce reserved rpmsg driver for ns announcement
> rpmsg: virtio: use rpmsg ns device for the ns announcement
> rpmsg: ns: add name service announcement service
> rpmsg: virtio: use rpmsg_ns driver to manage ns announcement
> rpmsg: ns: name service announcement endianness
>
> drivers/rpmsg/Kconfig | 9 ++
> drivers/rpmsg/Makefile | 1 +
> drivers/rpmsg/rpmsg_core.c | 37 ++++++
> drivers/rpmsg/rpmsg_internal.h | 32 +++++
> drivers/rpmsg/rpmsg_ns.c | 175 +++++++++++++++++++++++++
> drivers/rpmsg/virtio_rpmsg_bus.c | 213 +++++++++----------------------
> include/linux/rpmsg.h | 9 ++
> 7 files changed, 325 insertions(+), 151 deletions(-)
> create mode 100644 drivers/rpmsg/rpmsg_ns.c
>
> --
> 2.17.1
>

2020-08-24 22:45:43

by Mathieu Poirier

[permalink] [raw]
Subject: Re: [PATCH 2/9] rpmsg: core: add channel creation internal API

Hi Arnaud,

On Fri, Jul 31, 2020 at 01:47:25PM +0200, Arnaud Pouliquen wrote:
> Add the channel creation API as a first step to be able to define the
> name service announcement as a rpmsg driver independent from the RPMsg
> virtio bus.
>
> Signed-off-by: Arnaud Pouliquen <[email protected]>
> ---
> drivers/rpmsg/rpmsg_core.c | 37 ++++++++++++++++++++++++++++++++++
> drivers/rpmsg/rpmsg_internal.h | 12 +++++++++++
> 2 files changed, 49 insertions(+)
>
> diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
> index a6361cad608b..ae7da4a2e528 100644
> --- a/drivers/rpmsg/rpmsg_core.c
> +++ b/drivers/rpmsg/rpmsg_core.c
> @@ -20,6 +20,43 @@
>
> #include "rpmsg_internal.h"
>
> +/**
> + * rpmsg_create_channel() - create a new rpmsg channel
> + * using its name and address info.
> + * @rpdev: rpmsg driver
> + * @chinfo: channel_info to bind
> + *
> + * Returns a pointer to the new rpmsg device on success, or NULL on error.
> + */
> +struct rpmsg_device *
> + rpmsg_create_channel(struct rpmsg_device *rpdev,
> + struct rpmsg_channel_info *chinfo)
> +{
> + if (WARN_ON(!rpdev) || !rpdev->ops->create_channel)
> + return NULL;

Ok for the WARN_ON(). In another if(), I would check for ops and
ops->create_channel(). Same for the release() operation.

> +
> + return rpdev->ops->create_channel(rpdev, chinfo);
> +}
> +EXPORT_SYMBOL(rpmsg_create_channel);
> +
> +/**
> + * rpmsg_release_channel() - release a rpmsg channel
> + * using its name and address info.
> + * @rpdev: rpmsg driver
> + * @chinfo: channel_info to bind
> + *
> + * Returns a pointer to the new rpmsg device on success, or NULL on error.
> + */
> +int rpmsg_release_channel(struct rpmsg_device *rpdev,
> + struct rpmsg_channel_info *chinfo)
> +{
> + if (WARN_ON(!rpdev) || !rpdev->ops->release_channel)
> + return 0;
> +
> + return rpdev->ops->release_channel(rpdev, chinfo);
> +}
> +EXPORT_SYMBOL(rpmsg_release_channel);
> +
> /**
> * rpmsg_create_ept() - create a new rpmsg_endpoint
> * @rpdev: rpmsg channel device
> diff --git a/drivers/rpmsg/rpmsg_internal.h b/drivers/rpmsg/rpmsg_internal.h
> index 3fc83cd50e98..d5ab286d0e5e 100644
> --- a/drivers/rpmsg/rpmsg_internal.h
> +++ b/drivers/rpmsg/rpmsg_internal.h
> @@ -20,6 +20,8 @@
>
> /**
> * struct rpmsg_device_ops - indirection table for the rpmsg_device operations
> + * @create_channel: create backend-specific channel, optional
> + * @release_channel: release backend-specific channel, optional
> * @create_ept: create backend-specific endpoint, required
> * @announce_create: announce presence of new channel, optional
> * @announce_destroy: announce destruction of channel, optional
> @@ -29,6 +31,11 @@
> * advertise new channels implicitly by creating the endpoints.
> */
> struct rpmsg_device_ops {
> + struct rpmsg_device *(*create_channel)(struct rpmsg_device *rpdev,
> + struct rpmsg_channel_info *chinfo);
> + int (*release_channel)(struct rpmsg_device *rpdev,
> + struct rpmsg_channel_info *chinfo);
> +
> struct rpmsg_endpoint *(*create_ept)(struct rpmsg_device *rpdev,
> rpmsg_rx_cb_t cb, void *priv,
> struct rpmsg_channel_info chinfo);
> @@ -75,6 +82,11 @@ int rpmsg_unregister_device(struct device *parent,
> struct device *rpmsg_find_device(struct device *parent,
> struct rpmsg_channel_info *chinfo);
>
> +struct rpmsg_device *
> + rpmsg_create_channel(struct rpmsg_device *rpdev,
> + struct rpmsg_channel_info *chinfo);
> +int rpmsg_release_channel(struct rpmsg_device *rpdev,
> + struct rpmsg_channel_info *chinfo);
> /**
> * rpmsg_chrdev_register_device() - register chrdev device based on rpdev
> * @rpdev: prepared rpdev to be used for creating endpoints
> --
> 2.17.1
>