| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <2ad6c912-69fe-9c2e-146d-cdca9b8b1c7b@quicinc.com>
Date: Mon, 13 Feb 2023 18:36:39 -0800
From: Chris Lew <quic_clew@...cinc.com>
To: Bjorn Andersson <quic_bjorande@...cinc.com>,
Bjorn Andersson <andersson@...nel.org>,
Mathieu Poirier <mathieu.poirier@...aro.org>
CC: <linux-arm-msm@...r.kernel.org>,
<linux-remoteproc@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/6] rpmsg: glink: smem: Wrap driver context
On 2/13/2023 7:52 AM, Bjorn Andersson wrote:
> The Glink SMEM driver allocates a struct device and hangs two
> devres-allocated pipe objects thereon. To facilitate the move of
> interrupt and mailbox handling to the driver, introduce a wrapper object
> capturing the device, glink reference and remote processor id.
>
> The type of the remoteproc reference is updated, as these are
> specifically targeting the SMEM implementation.
>
> Signed-off-by: Bjorn Andersson <quic_bjorande@...cinc.com>
> ---
>
> Changes since v1:
> - Revert back to use a local struct device * in register, to reduce size of
> diff.
> - Reverse xmas tree of variables in register
> - Fix spelling of targeting in commit message.
>
> drivers/remoteproc/qcom_common.h | 3 ++-
> drivers/rpmsg/qcom_glink_smem.c | 43 ++++++++++++++++++++++++--------
> include/linux/rpmsg/qcom_glink.h | 12 ++++-----
> 3 files changed, 40 insertions(+), 18 deletions(-)
>
Thanks for the changes, LGTM.
Reviewed-by: Chris Lew <quic_clew@...cinc.com>
> diff --git a/drivers/remoteproc/qcom_common.h b/drivers/remoteproc/qcom_common.h
> index c35adf730be0..2747c7d9ba44 100644
> --- a/drivers/remoteproc/qcom_common.h
> +++ b/drivers/remoteproc/qcom_common.h
> @@ -6,6 +6,7 @@
> #include "remoteproc_internal.h"
> #include <linux/soc/qcom/qmi.h>
>
> +struct qcom_glink_smem;
> struct qcom_sysmon;
>
> struct qcom_rproc_glink {
> @@ -15,7 +16,7 @@ struct qcom_rproc_glink {
>
> struct device *dev;
> struct device_node *node;
> - struct qcom_glink *edge;
> + struct qcom_glink_smem *edge;
> };
>
> struct qcom_rproc_subdev {
> diff --git a/drivers/rpmsg/qcom_glink_smem.c b/drivers/rpmsg/qcom_glink_smem.c
> index 579bc4443f6d..a9c477df4d68 100644
> --- a/drivers/rpmsg/qcom_glink_smem.c
> +++ b/drivers/rpmsg/qcom_glink_smem.c
> @@ -33,6 +33,14 @@
> #define SMEM_GLINK_NATIVE_XPRT_FIFO_0 479
> #define SMEM_GLINK_NATIVE_XPRT_FIFO_1 480
>
> +struct qcom_glink_smem {
> + struct device dev;
> +
> + struct qcom_glink *glink;
> +
> + u32 remote_pid;
> +};
> +
> struct glink_smem_pipe {
> struct qcom_glink_pipe native;
>
> @@ -41,7 +49,7 @@ struct glink_smem_pipe {
>
> void *fifo;
>
> - int remote_pid;
> + struct qcom_glink_smem *smem;
> };
>
> #define to_smem_pipe(p) container_of(p, struct glink_smem_pipe, native)
> @@ -49,13 +57,14 @@ struct glink_smem_pipe {
> static size_t glink_smem_rx_avail(struct qcom_glink_pipe *np)
> {
> struct glink_smem_pipe *pipe = to_smem_pipe(np);
> + struct qcom_glink_smem *smem = pipe->smem;
> size_t len;
> void *fifo;
> u32 head;
> u32 tail;
>
> if (!pipe->fifo) {
> - fifo = qcom_smem_get(pipe->remote_pid,
> + fifo = qcom_smem_get(smem->remote_pid,
> SMEM_GLINK_NATIVE_XPRT_FIFO_1, &len);
> if (IS_ERR(fifo)) {
> pr_err("failed to acquire RX fifo handle: %ld\n",
> @@ -179,14 +188,17 @@ static void glink_smem_tx_write(struct qcom_glink_pipe *glink_pipe,
>
> static void qcom_glink_smem_release(struct device *dev)
> {
> - kfree(dev);
> + struct qcom_glink_smem *smem = container_of(dev, struct qcom_glink_smem, dev);
> +
> + kfree(smem);
> }
>
> -struct qcom_glink *qcom_glink_smem_register(struct device *parent,
> - struct device_node *node)
> +struct qcom_glink_smem *qcom_glink_smem_register(struct device *parent,
> + struct device_node *node)
> {
> struct glink_smem_pipe *rx_pipe;
> struct glink_smem_pipe *tx_pipe;
> + struct qcom_glink_smem *smem;
> struct qcom_glink *glink;
> struct device *dev;
> u32 remote_pid;
> @@ -194,10 +206,12 @@ struct qcom_glink *qcom_glink_smem_register(struct device *parent,
> size_t size;
> int ret;
>
> - dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> - if (!dev)
> + smem = kzalloc(sizeof(*smem), GFP_KERNEL);
> + if (!smem)
> return ERR_PTR(-ENOMEM);
>
> + dev = &smem->dev;
> +
> dev->parent = parent;
> dev->of_node = node;
> dev->release = qcom_glink_smem_release;
> @@ -216,6 +230,8 @@ struct qcom_glink *qcom_glink_smem_register(struct device *parent,
> goto err_put_dev;
> }
>
> + smem->remote_pid = remote_pid;
> +
> rx_pipe = devm_kzalloc(dev, sizeof(*rx_pipe), GFP_KERNEL);
> tx_pipe = devm_kzalloc(dev, sizeof(*tx_pipe), GFP_KERNEL);
> if (!rx_pipe || !tx_pipe) {
> @@ -264,14 +280,14 @@ struct qcom_glink *qcom_glink_smem_register(struct device *parent,
> goto err_put_dev;
> }
>
> + rx_pipe->smem = smem;
> rx_pipe->native.avail = glink_smem_rx_avail;
> rx_pipe->native.peak = glink_smem_rx_peak;
> rx_pipe->native.advance = glink_smem_rx_advance;
> - rx_pipe->remote_pid = remote_pid;
>
> + tx_pipe->smem = smem;
> tx_pipe->native.avail = glink_smem_tx_avail;
> tx_pipe->native.write = glink_smem_tx_write;
> - tx_pipe->remote_pid = remote_pid;
>
> *rx_pipe->tail = 0;
> *tx_pipe->head = 0;
> @@ -285,7 +301,10 @@ struct qcom_glink *qcom_glink_smem_register(struct device *parent,
> goto err_put_dev;
> }
>
> - return glink;
> + smem->glink = glink;
> +
> + return smem;
> +
>
> err_put_dev:
> device_unregister(dev);
> @@ -294,8 +313,10 @@ struct qcom_glink *qcom_glink_smem_register(struct device *parent,
> }
> EXPORT_SYMBOL_GPL(qcom_glink_smem_register);
>
> -void qcom_glink_smem_unregister(struct qcom_glink *glink)
> +void qcom_glink_smem_unregister(struct qcom_glink_smem *smem)
> {
> + struct qcom_glink *glink = smem->glink;
> +
> qcom_glink_native_remove(glink);
> qcom_glink_native_unregister(glink);
> }
> diff --git a/include/linux/rpmsg/qcom_glink.h b/include/linux/rpmsg/qcom_glink.h
> index 22fc3a69b683..bfbd48f435fa 100644
> --- a/include/linux/rpmsg/qcom_glink.h
> +++ b/include/linux/rpmsg/qcom_glink.h
> @@ -5,7 +5,7 @@
>
> #include <linux/device.h>
>
> -struct qcom_glink;
> +struct qcom_glink_smem;
>
> #if IS_ENABLED(CONFIG_RPMSG_QCOM_GLINK)
> void qcom_glink_ssr_notify(const char *ssr_name);
> @@ -15,20 +15,20 @@ static inline void qcom_glink_ssr_notify(const char *ssr_name) {}
>
> #if IS_ENABLED(CONFIG_RPMSG_QCOM_GLINK_SMEM)
>
> -struct qcom_glink *qcom_glink_smem_register(struct device *parent,
> - struct device_node *node);
> -void qcom_glink_smem_unregister(struct qcom_glink *glink);
> +struct qcom_glink_smem *qcom_glink_smem_register(struct device *parent,
> + struct device_node *node);
> +void qcom_glink_smem_unregister(struct qcom_glink_smem *glink);
>
> #else
>
> -static inline struct qcom_glink *
> +static inline struct qcom_glink_smem *
> qcom_glink_smem_register(struct device *parent,
> struct device_node *node)
> {
> return NULL;
> }
>
> -static inline void qcom_glink_smem_unregister(struct qcom_glink *glink) {}
> +static inline void qcom_glink_smem_unregister(struct qcom_glink_smem *glink) {}
> #endif
>
> #endif
Powered by blists - more mailing lists