[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <60e77932-9be7-443d-aed9-2b54945fdce6@foss.st.com>
Date: Wed, 23 Apr 2025 09:15:15 +0200
From: Arnaud POULIQUEN <arnaud.pouliquen@...s.st.com>
To: <linux@...blig.org>, <andersson@...nel.org>, <mathieu.poirier@...aro.org>
CC: <corbet@....net>, <linux-remoteproc@...r.kernel.org>,
<linux-doc@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] rpmsg: core: Remove deadcode
Hello,
On 4/20/25 22:41, linux@...blig.org wrote:
> From: "Dr. David Alan Gilbert" <linux@...blig.org>
>
> rpmsg_send_offchannel() and rpmsg_trysend_offchannel() have been
> unused since they were added in 2011's
> commit bcabbccabffe ("rpmsg: add virtio-based remote processor messaging
> bus")
>
> Remove them and associated docs.
>
> I suspect this means the trysend_offchannel and send_offchannel
> member function pointers and the virtio implementation of them can be
> removed as well, but haven't yet gone that far.
It seems to me that this API is not safe as it allows an endpoint to usurp the
identity of another one thanks to the source address.
So, +1 to remove it.
That said, to complete the remove it would be nice to also remove associated ops
declared in rpmsg_endpoint_ops and implemented in virtio_rpmsg_bus.c.
Regards,
Arnaud
>
> Signed-off-by: Dr. David Alan Gilbert <linux@...blig.org>
> ---
> Documentation/staging/rpmsg.rst | 46 ------------------------
> drivers/rpmsg/rpmsg_core.c | 63 ---------------------------------
> include/linux/rpmsg.h | 22 ------------
> 3 files changed, 131 deletions(-)
>
> diff --git a/Documentation/staging/rpmsg.rst b/Documentation/staging/rpmsg.rst
> index 3713adaa1608..40282cca86ca 100644
> --- a/Documentation/staging/rpmsg.rst
> +++ b/Documentation/staging/rpmsg.rst
> @@ -110,31 +110,6 @@ or a timeout of 15 seconds elapses. When the latter happens,
> The function can only be called from a process context (for now).
> Returns 0 on success and an appropriate error value on failure.
>
> -::
> -
> - int rpmsg_send_offchannel(struct rpmsg_endpoint *ept, u32 src, u32 dst,
> - void *data, int len);
> -
> -
> -sends a message across to the remote processor, using the src and dst
> -addresses provided by the user.
> -
> -The caller should specify the endpoint, the data it wants to send,
> -its length (in bytes), and explicit source and destination addresses.
> -The message will then be sent to the remote processor to which the
> -endpoint's channel belongs, but the endpoint's src and channel dst
> -addresses will be ignored (and the user-provided addresses will
> -be used instead).
> -
> -In case there are no TX buffers available, the function will block until
> -one becomes available (i.e. until the remote processor consumes
> -a tx buffer and puts it back on virtio's used descriptor ring),
> -or a timeout of 15 seconds elapses. When the latter happens,
> --ERESTARTSYS is returned.
> -
> -The function can only be called from a process context (for now).
> -Returns 0 on success and an appropriate error value on failure.
> -
> ::
>
> int rpmsg_trysend(struct rpmsg_endpoint *ept, void *data, int len);
> @@ -173,27 +148,6 @@ return -ENOMEM without waiting until one becomes available.
> The function can only be called from a process context (for now).
> Returns 0 on success and an appropriate error value on failure.
>
> -::
> -
> - int rpmsg_trysend_offchannel(struct rpmsg_endpoint *ept, u32 src, u32 dst,
> - void *data, int len);
> -
> -
> -sends a message across to the remote processor, using source and
> -destination addresses provided by the user.
> -
> -The user should specify the channel, the data it wants to send,
> -its length (in bytes), and explicit source and destination addresses.
> -The message will then be sent to the remote processor to which the
> -channel belongs, but the channel's src and dst addresses will be
> -ignored (and the user-provided addresses will be used instead).
> -
> -In case there are no TX buffers available, the function will immediately
> -return -ENOMEM without waiting until one becomes available.
> -
> -The function can only be called from a process context (for now).
> -Returns 0 on success and an appropriate error value on failure.
> -
> ::
>
> struct rpmsg_endpoint *rpmsg_create_ept(struct rpmsg_device *rpdev,
> diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
> index 207b64c0a2fe..6ee36adcbdba 100644
> --- a/drivers/rpmsg/rpmsg_core.c
> +++ b/drivers/rpmsg/rpmsg_core.c
> @@ -193,38 +193,6 @@ int rpmsg_sendto(struct rpmsg_endpoint *ept, void *data, int len, u32 dst)
> }
> EXPORT_SYMBOL(rpmsg_sendto);
>
> -/**
> - * rpmsg_send_offchannel() - send a message using explicit src/dst addresses
> - * @ept: the rpmsg endpoint
> - * @src: source address
> - * @dst: destination address
> - * @data: payload of message
> - * @len: length of payload
> - *
> - * This function sends @data of length @len to the remote @dst address,
> - * and uses @src as the source address.
> - * The message will be sent to the remote processor which the @ept
> - * endpoint belongs to.
> - * In case there are no TX buffers available, the function will block until
> - * one becomes available, or a timeout of 15 seconds elapses. When the latter
> - * happens, -ERESTARTSYS is returned.
> - *
> - * Can only be called from process context (for now).
> - *
> - * Return: 0 on success and an appropriate error value on failure.
> - */
> -int rpmsg_send_offchannel(struct rpmsg_endpoint *ept, u32 src, u32 dst,
> - void *data, int len)
> -{
> - if (WARN_ON(!ept))
> - return -EINVAL;
> - if (!ept->ops->send_offchannel)
> - return -ENXIO;
> -
> - return ept->ops->send_offchannel(ept, src, dst, data, len);
> -}
> -EXPORT_SYMBOL(rpmsg_send_offchannel);
> -
> /**
> * rpmsg_trysend() - send a message across to the remote processor
> * @ept: the rpmsg endpoint
> @@ -301,37 +269,6 @@ __poll_t rpmsg_poll(struct rpmsg_endpoint *ept, struct file *filp,
> }
> EXPORT_SYMBOL(rpmsg_poll);
>
> -/**
> - * rpmsg_trysend_offchannel() - send a message using explicit src/dst addresses
> - * @ept: the rpmsg endpoint
> - * @src: source address
> - * @dst: destination address
> - * @data: payload of message
> - * @len: length of payload
> - *
> - * This function sends @data of length @len to the remote @dst address,
> - * and uses @src as the source address.
> - * The message will be sent to the remote processor which the @ept
> - * endpoint belongs to.
> - * In case there are no TX buffers available, the function will immediately
> - * return -ENOMEM without waiting until one becomes available.
> - *
> - * Can only be called from process context (for now).
> - *
> - * Return: 0 on success and an appropriate error value on failure.
> - */
> -int rpmsg_trysend_offchannel(struct rpmsg_endpoint *ept, u32 src, u32 dst,
> - void *data, int len)
> -{
> - if (WARN_ON(!ept))
> - return -EINVAL;
> - if (!ept->ops->trysend_offchannel)
> - return -ENXIO;
> -
> - return ept->ops->trysend_offchannel(ept, src, dst, data, len);
> -}
> -EXPORT_SYMBOL(rpmsg_trysend_offchannel);
> -
> /**
> * rpmsg_set_flow_control() - request remote to pause/resume transmission
> * @ept: the rpmsg endpoint
> diff --git a/include/linux/rpmsg.h b/include/linux/rpmsg.h
> index 90d8e4475f80..fb7ab9165645 100644
> --- a/include/linux/rpmsg.h
> +++ b/include/linux/rpmsg.h
> @@ -184,13 +184,9 @@ struct rpmsg_endpoint *rpmsg_create_ept(struct rpmsg_device *,
>
> int rpmsg_send(struct rpmsg_endpoint *ept, void *data, int len);
> int rpmsg_sendto(struct rpmsg_endpoint *ept, void *data, int len, u32 dst);
> -int rpmsg_send_offchannel(struct rpmsg_endpoint *ept, u32 src, u32 dst,
> - void *data, int len);
>
> int rpmsg_trysend(struct rpmsg_endpoint *ept, void *data, int len);
> int rpmsg_trysendto(struct rpmsg_endpoint *ept, void *data, int len, u32 dst);
> -int rpmsg_trysend_offchannel(struct rpmsg_endpoint *ept, u32 src, u32 dst,
> - void *data, int len);
>
> __poll_t rpmsg_poll(struct rpmsg_endpoint *ept, struct file *filp,
> poll_table *wait);
> @@ -271,15 +267,6 @@ static inline int rpmsg_sendto(struct rpmsg_endpoint *ept, void *data, int len,
>
> }
>
> -static inline int rpmsg_send_offchannel(struct rpmsg_endpoint *ept, u32 src,
> - u32 dst, void *data, int len)
> -{
> - /* This shouldn't be possible */
> - WARN_ON(1);
> -
> - return -ENXIO;
> -}
> -
> static inline int rpmsg_trysend(struct rpmsg_endpoint *ept, void *data, int len)
> {
> /* This shouldn't be possible */
> @@ -297,15 +284,6 @@ static inline int rpmsg_trysendto(struct rpmsg_endpoint *ept, void *data,
> return -ENXIO;
> }
>
> -static inline int rpmsg_trysend_offchannel(struct rpmsg_endpoint *ept, u32 src,
> - u32 dst, void *data, int len)
> -{
> - /* This shouldn't be possible */
> - WARN_ON(1);
> -
> - return -ENXIO;
> -}
> -
> static inline __poll_t rpmsg_poll(struct rpmsg_endpoint *ept,
> struct file *filp, poll_table *wait)
> {
Powered by blists - more mailing lists