Received: by 2002:a05:6a10:f3d0:0:0:0:0 with SMTP id a16csp18958pxv; Wed, 30 Jun 2021 13:49:11 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyQ8jDwof5Bg+2u+WLEzpCVSMNuv3pqzzIjlv9KFOJ2gB6+n3M5AmuRFIzyvWxr9VgRCL3M X-Received: by 2002:a6b:6205:: with SMTP id f5mr9137534iog.60.1625086151730; Wed, 30 Jun 2021 13:49:11 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1625086151; cv=none; d=google.com; s=arc-20160816; b=V2CwPA3DJiB0mJZeT119lwCYeNNHvLpG8CNw2XH80z/IIr0sD/ocN+0EzXTt8bNpCL 2niGQ2FuBP5rTIB5WTz7t8QWrykAb7J/2w4Z+d2T2vA2cA9KCO3zXFbpeHDxoCwiwws3 bam6ZVBVvsgoRBeEaft0AGhmnpZZLxsoWRHANAg8E/+CDuHPvyqc1Ahn/F57sQcWUXwx yNbj3Qy0/251WYQnWfemeRjAilvrFxwHbQPLfewq4sePe9hPzyNoa/zrG63x3zBHTVZF yN2l47JgME0mxkZQvpBREombxO9g8DMuNoPQC6jBnTUosnWxgna2TjdtS7JQYJPK2Wy9 f6OQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature; bh=3V3OFykueXALQcb8bHaOuERxTOWs4Agns0W740chGTg=; b=jqdx5BMQ8ylp4sRlb1s3M1nZOmmcxyj6SDrOP/ilW944+U+TsZCDVTL64XSIhW3erS DyXNUKFCU1voctAUFbnmyx75eD4DlUtpuQk4ImTqAA1gjsUCG1wus2C8AuH5TKVMZNWi XwQIt3gob1i0oyxmluOgQqGKpJcCb2fc07gSCK4BM+HbUPE4kiZVAPh7fVip/YBqC+kt 7oi8X+Vjns9FGRkFy2NTRqcNKY1Mj6iAllxwEsxWDBnxjeT6+Gbjizdt6RcgrC0jNHQo aT4hRXyielGH+hd5X8ZRpv4B/gKfkv/2CVCz9YXIO5g8sTDOcl2nlWmdhHEbxL1s5vVf BWMw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=dXFdaB5e; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id r4si1632290ilt.108.2021.06.30.13.48.57; Wed, 30 Jun 2021 13:49:11 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=dXFdaB5e; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234090AbhF3Uu4 (ORCPT + 99 others); Wed, 30 Jun 2021 16:50:56 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54858 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229700AbhF3Uuz (ORCPT ); Wed, 30 Jun 2021 16:50:55 -0400 Received: from mail-pg1-x529.google.com (mail-pg1-x529.google.com [IPv6:2607:f8b0:4864:20::529]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B4A9DC0617A8 for ; Wed, 30 Jun 2021 13:48:26 -0700 (PDT) Received: by mail-pg1-x529.google.com with SMTP id w15so3543899pgk.13 for ; Wed, 30 Jun 2021 13:48:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=3V3OFykueXALQcb8bHaOuERxTOWs4Agns0W740chGTg=; b=dXFdaB5eK79hyNZjeXXsAOEOK1YUd22gnr/ukEbvG5B/Cj0fkM+Vihe1cNwY+Aed/S YiqwQ8eUD2Un/L66bOnABrG0xmvMZOydY9l9QRQ8aRB2SQqPAKQjVkWUGgcwtrGS0KZZ 2MjkWulYiPLphm4SQOJ/iKEm1kBZxL9LrjtiQMcLjIiW6CyLDOR0ECn9Tp0cWjwMKvij BG6LAP6BNgSaq3oeK3QyaEjZQdfmi/LZ8l0DwXUFBwWm7ba2nyRY1fHs3t1oNLdHwj3d FqTmnDNlRgEcK0beAB76IkESPtnILtCF+6816JOPQ6X6ijIUdgkWO73EgQolp8jyH2GJ +MpA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=3V3OFykueXALQcb8bHaOuERxTOWs4Agns0W740chGTg=; b=U0tfh2dLR/rU9UiT4vcZ4TRimebgY5kgdFAzuvsz0qCls72bjbU4WpBqG+yuggAYYh N6Ul2v9N4FapQx9L2lvH/n4hxtGRcZ8anMIbOxJSSKQlu/WItNFPsizSN5fT8RPO63td kRn/JajdQ5PZUs1P4KRN4BnURGu9qjxGSdNrn/KGHdQqAfkYeUY9gwuipUoq8j95S2s6 lZVdfKT0Q+xBpo+1Su5ELpivGgb2axO0klTasgjx3L21ElGDdx636BSYWxLRch0T7g+t U0ZULGqzU/7pdMQ2vgsEvYUaIJoHqSb5Um64h+WfHiyOiqNiM9+fgE76L9MOW91P4E9v 9SnA== X-Gm-Message-State: AOAM533bOe7ZoNqvwUcbRGwC7uPyJk5lnd16teRe/7PWuH78FhvmR5/T bbZST+Sn3Rq1rQ8Ag+eiIYkqmA== X-Received: by 2002:a63:5057:: with SMTP id q23mr35744377pgl.271.1625086106139; Wed, 30 Jun 2021 13:48:26 -0700 (PDT) Received: from p14s (S0106889e681aac74.cg.shawcable.net. [68.147.0.187]) by smtp.gmail.com with ESMTPSA id m21sm7339455pjz.57.2021.06.30.13.48.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 30 Jun 2021 13:48:25 -0700 (PDT) Date: Wed, 30 Jun 2021 14:48:23 -0600 From: Mathieu Poirier To: Arnaud Pouliquen Cc: Bjorn Andersson , Ohad Ben-Cohen , linux-remoteproc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, julien.massot@iot.bzh Subject: Re: [PATCH v2 3/4] rpmsg: char: Add possibility to use default endpoint of the rpmsg device. Message-ID: <20210630204823.GC1290178@p14s> References: <20210623150504.14450-1-arnaud.pouliquen@foss.st.com> <20210623150504.14450-4-arnaud.pouliquen@foss.st.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210623150504.14450-4-arnaud.pouliquen@foss.st.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jun 23, 2021 at 05:05:03PM +0200, Arnaud Pouliquen wrote: > Current implementation create/destroy a new endpoint on each > rpmsg_eptdev_open/rpmsg_eptdev_release calls. > > For a rpmsg device created by the NS announcement mechanism we need to > use a unique static endpoint that is the default rpmsg device endpoint > associated to the channel. > > This patch prepares the introduction of a rpmsg channel device for the > char device. The rpmsg channel device will require a default endpoint to > communicate to the remote processor. > > Add the static_ept field in rpmsg_eptdev structure. This boolean > determines the behavior on rpmsg_eptdev_open and rpmsg_eptdev_release call. > > - If static_ept == false: > Use the legacy behavior by creating a new endpoint each time > rpmsg_eptdev_open is called and release it when rpmsg_eptdev_release > is called on /dev/rpmsgX device open/close. > > - If static_ept == true: > use the rpmsg device default endpoint for the communication. > - Address the update of _rpmsg_chrdev_eptdev_create in e separate patch for readability. > > Add protection in rpmsg_eptdev_ioctl to prevent to destroy a default endpoint. > > Signed-off-by: Arnaud Pouliquen > --- > update vs V1: > - remove the management of the default endpoint creation from rpmsg_eptdev_open. > > --- > drivers/rpmsg/rpmsg_char.c | 21 +++++++++++++++++++-- > 1 file changed, 19 insertions(+), 2 deletions(-) > > diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c > index 50b7d4b00175..a75dce1e29d8 100644 > --- a/drivers/rpmsg/rpmsg_char.c > +++ b/drivers/rpmsg/rpmsg_char.c > @@ -45,6 +45,8 @@ static DEFINE_IDA(rpmsg_minor_ida); > * @queue_lock: synchronization of @queue operations > * @queue: incoming message queue > * @readq: wait object for incoming queue > + * @static_ept: specify if the endpoint has to be created at each device opening or > + * if the default endpoint should be used. > */ > struct rpmsg_eptdev { > struct device dev; > @@ -59,6 +61,8 @@ struct rpmsg_eptdev { > spinlock_t queue_lock; > struct sk_buff_head queue; > wait_queue_head_t readq; > + > + bool static_ept; > }; > > int rpmsg_chrdev_eptdev_destroy(struct device *dev, void *data) > @@ -116,7 +120,15 @@ static int rpmsg_eptdev_open(struct inode *inode, struct file *filp) > > get_device(dev); > > - ept = rpmsg_create_ept(rpdev, rpmsg_ept_cb, eptdev, eptdev->chinfo); > + /* > + * If the static_ept is set to true, the rpmsg device default endpoint is used. > + * Else a new endpoint is created on open that will be destroyed on release. > + */ > + if (eptdev->static_ept) > + ept = rpdev->ept; > + else > + ept = rpmsg_create_ept(rpdev, rpmsg_ept_cb, eptdev, eptdev->chinfo); > + > if (!ept) { > dev_err(dev, "failed to open %s\n", eptdev->chinfo.name); > put_device(dev); > @@ -137,7 +149,8 @@ static int rpmsg_eptdev_release(struct inode *inode, struct file *filp) > /* Close the endpoint, if it's not already destroyed by the parent */ > mutex_lock(&eptdev->ept_lock); > if (eptdev->ept) { > - rpmsg_destroy_ept(eptdev->ept); > + if (!eptdev->static_ept) > + rpmsg_destroy_ept(eptdev->ept); > eptdev->ept = NULL; > } > mutex_unlock(&eptdev->ept_lock); > @@ -264,6 +277,10 @@ static long rpmsg_eptdev_ioctl(struct file *fp, unsigned int cmd, > if (cmd != RPMSG_DESTROY_EPT_IOCTL) > return -EINVAL; > > + /* Don't allow to destroy a default endpoint. */ > + if (!eptdev->rpdev || eptdev->ept == eptdev->rpdev->ept) Did you find a scenario where eptdev->rpdev would not be valid when this is called? To me if this code is called __rpmsg_chrdev_eptdev_create() has setup the rpdev pointer and therefore it will be valid. If there is a scenario where it is possible that eptdev->rpdev is invalid then please add a comment with the details. Otherwise simply remove the first part of the condition. Reviewed-by: Mathieu Poirier > + return -EPERM; > + > return rpmsg_chrdev_eptdev_destroy(&eptdev->dev, NULL); > } > > -- > 2.17.1 >