From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 5B948A034F; Wed, 13 May 2020 06:11:09 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id E500F1C0AF; Wed, 13 May 2020 06:11:07 +0200 (CEST) Received: from EUR05-AM6-obe.outbound.protection.outlook.com (mail-am6eur05on2078.outbound.protection.outlook.com [40.107.22.78]) by dpdk.org (Postfix) with ESMTP id 3B0171BFF8; Wed, 13 May 2020 06:11:06 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=cMPMrovnPtBl7wZY+tZAj4WBeL5HFQExwkMIlzHCOH5lP8YyU/rHaAHlO/uGNT1hYg40wBRpgWMcWJslB+VoHcfhWaI0SK3l3W31S4YlmQM5BraxVQQSVwZxrxX/JxTbtNNLZrVb/SZdp4AFtk2qUSB6MLOYDIzsnL9AicRMd0Dz2Dn+T7pBD5IDBKdiSbdNt81Vlvffitw+5goMds5Q+ZHFe8kKwlUVtZMoSqeVXRsjUE9b6aK8/Ut7TaOruITdnMsYiGelF216jO+d2sW1en3itNzLc+SD8HZTH/JcyEj/sSYlDM72z2KfMvmKNZ3OYBhxw95t/HrJ1Y5BlEOSYA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=+ZTg2Iif5xMOQBhCtgH2jggdMr7nJNXbQ7EhtcNmQio=; b=QxzZvY1Z8oB/kENl6JvFv+LlwfuNio4YtJuqZ+2PQe11QhI7Lm5EKOmDcPeaEOVqUAJUMPDxiDwgMrWnSVqHp2zeNfSR8JZnUU6yj7zPoZvKZmpJrZxEfQbEHdVIcOd3XQW2xgqwu9lQ/KiZIILpDL90pWcsZopsaoXJBpXY3cuZfwDj4L68Nc+oOtcuczMb0NIX+x+XG4QFXoIiANQ2sX5gHBW+e7qepu6POF4CggGXn2DwGAvIvwLQghzakmhIqWKjJEoSOX3SGw1H/nKhqjJ2I+VyTVLTvf5Yhp6UdwHUO0Qz0m13n8WHKjb3H51UV+ucxQrKT3+cRgI6uwiOsg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nxp.com; dmarc=pass action=none header.from=nxp.com; dkim=pass header.d=nxp.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nxp.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=+ZTg2Iif5xMOQBhCtgH2jggdMr7nJNXbQ7EhtcNmQio=; b=YFYh5orst7Y2rK5W02miQvszj7DyXTNIxIBpipdppaMJJvPu1NWHEhDyGHNlUHtwIb8v9BfVLlzx1SkASG+BWHNvypyzMEjID1/JVBsg9cqgLCdCD72hoBxmB8oa+UXD4E5R8R4P1Rac3ab5aGNO7JT113L+vnELScr3SYwkFlw= Received: from VI1PR04MB6960.eurprd04.prod.outlook.com (2603:10a6:803:12d::10) by VI1PR04MB3167.eurprd04.prod.outlook.com (2603:10a6:802:8::24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2979.34; Wed, 13 May 2020 04:11:04 +0000 Received: from VI1PR04MB6960.eurprd04.prod.outlook.com ([fe80::dd6:d975:588f:1c1c]) by VI1PR04MB6960.eurprd04.prod.outlook.com ([fe80::dd6:d975:588f:1c1c%3]) with mapi id 15.20.2979.033; Wed, 13 May 2020 04:11:04 +0000 From: Gagandeep Singh To: wangyunjian , "dev@dpdk.org" CC: Hemant Agrawal , "Lilijun (Jerry)" , xudingke , "stable@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v2] crypto/caam_jr: fix wrong check of fd Thread-Index: AQHWJ03x1MJHFZ/+60apzbPWDpz6P6ijzlEwgAB7ZgCAAR+OIA== Date: Wed, 13 May 2020 04:11:04 +0000 Message-ID: References: <1589171838-18076-1-git-send-email-wangyunjian@huawei.com> <34EFBCA9F01B0748BEB6B629CE643AE60CFD82D5@DGGEMM533-MBX.china.huawei.com> In-Reply-To: <34EFBCA9F01B0748BEB6B629CE643AE60CFD82D5@DGGEMM533-MBX.china.huawei.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: huawei.com; dkim=none (message not signed) header.d=none;huawei.com; dmarc=none action=none header.from=nxp.com; x-originating-ip: [106.211.72.7] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: 05a8c4f2-26a7-4749-b83a-08d7f6f3a7ad x-ms-traffictypediagnostic: VI1PR04MB3167:|VI1PR04MB3167: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:5236; x-forefront-prvs: 0402872DA1 x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: Fg0uXAJ+F4ocBzu2RzB0yqN7pOFFAERy+XWQUsC88Nd0P+rB1RDMJrklvGuOki5kRVjgGsmBYMCcE9npnEOijRIdC7nAsR0leHBS6F5W9jl4PeG9Xk1mdL8jloolBUVGBjut3K/zXjOhRX8H8XZEYaBs/W861SAtMBxsGLUUY3Qfl/1gcKXGY1hU5J2j7AoRZS0H3h2IEK0nY2sSAO/R2xMTvm8nWoL1A0aIUrP5WWe2FtJUP/9LZTM2+2OTDkonRlosWnN+EFw01nOE85dhgh3x+FRyTN+lJIeE9vE2ixvOyrnI8iW8BNRJ1Yojakm2+LQdjgwQdvJebhUi4oZ/iMnnq8xvQd0RaYricvGhGdBasojw/EyVaRst2G76YrH4LuEwIVWRXfGUqU4achGnkB5iN8oTliNhUbtJsMf6nVruQJdTpB7c//NNY3a0NWQVqo+vhS4HqJrlikjN+aR/Ka0U1tukCRGSxAPHXVlZ6FvfKt0HMkHCuBZfGZSAAj/SDHPSU7UYzo4MXZ4oOWDb/w== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:VI1PR04MB6960.eurprd04.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(4636009)(366004)(376002)(39860400002)(346002)(136003)(396003)(33430700001)(26005)(71200400001)(4326008)(2906002)(6506007)(186003)(478600001)(33440700001)(53546011)(55236004)(9686003)(86362001)(55016002)(8676002)(33656002)(54906003)(110136005)(76116006)(316002)(66946007)(66476007)(8936002)(64756008)(66556008)(66446008)(7696005)(5660300002)(52536014); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata: zj5HEh1AkgDxj9suy4jL3+ILxQSGPbie9Zib/Ge4voawFuzTHYKezgFrQPy7PUFqzbiHbt50zcYM5uFMSFVL0z4+tAinKfMl8TYR5cY7NnF+pjok5O3/vnTSBS4JD0PSaysLiGSuF2mV10/MYjXVCRrnxZrakEGoEZaZJ26BsHEcG1tij4eE722hbgC2YjeR3AOsOzyR0hy6Ayn6R8Or4O82/MMPkOyQQlHUaRwfkCysA/gVoYbknh8iM9Et4Mmw0hrPhfXu84evDxTTiP+rzydPusNlLAmvK8Gq8Otj6rwBjJ1LBCw2Q2jyTqY2hcFgjrLDG3tuyVvmXMyKYchWqO1XAyghsXSGIUgA1AXP3XqcVobQOFp3OdQKRzG7nHMZz6SFoH54QpYOtxQnkJpByIappYKEEpuIt1ACbEbCuFHrR8VPUgRb+yE7lKT1s5obRTZCljL/RDhAzYym5T7WzTzkh6ShF1t71faMvuwNbUc= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: nxp.com X-MS-Exchange-CrossTenant-Network-Message-Id: 05a8c4f2-26a7-4749-b83a-08d7f6f3a7ad X-MS-Exchange-CrossTenant-originalarrivaltime: 13 May 2020 04:11:04.6073 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: ppSmZHER3ZMaPBFYOjOh2ef+ItxKnYOA+HSE1rIyel0/DnQqgwkbuPaXFuAFQ062 X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR04MB3167 Subject: Re: [dpdk-dev] [PATCH v2] crypto/caam_jr: fix wrong check of fd X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > -----Original Message----- > From: wangyunjian > Sent: Tuesday, May 12, 2020 4:28 PM > To: Gagandeep Singh ; dev@dpdk.org > Cc: Hemant Agrawal ; Lilijun (Jerry) > ; xudingke ; > stable@dpdk.org > Subject: RE: [dpdk-dev] [PATCH v2] crypto/caam_jr: fix wrong check of fd >=20 > > -----Original Message----- > > From: Gagandeep Singh [mailto:G.Singh@nxp.com] > > Sent: Tuesday, May 12, 2020 11:49 AM > > To: wangyunjian ; dev@dpdk.org > > Cc: Hemant Agrawal ; Lilijun (Jerry) > > ; xudingke ; > > stable@dpdk.org > > Subject: RE: [dpdk-dev] [PATCH v2] crypto/caam_jr: fix wrong check of f= d > > > > > > > > > -----Original Message----- > > > From: wangyunjian > > > Sent: Monday, May 11, 2020 10:07 AM > > > To: dev@dpdk.org > > > Cc: Gagandeep Singh ; Hemant Agrawal > > > ; jerry.lilijun@huawei.com; > > > xudingke@huawei.com; Yunjian Wang ; > > > stable@dpdk.org > > > Subject: [dpdk-dev] [PATCH v2] crypto/caam_jr: fix wrong check of fd > > > > > > From: Yunjian Wang > > > > > > Zero is a valid fd. It will fail to check the fd if the fd is zero. > > > The "job_ring->uio_fd" is an fd, so define it as "int". > > > > > > Fixes: e7a45f3cc245 ("crypto/caam_jr: add UIO specific operations") > > > Fixes: a5e1018d5e67 ("crypto/caam_jr: add routines to configure HW") > > > Cc: stable@dpdk.org > > > > > > Signed-off-by: Yunjian Wang > > > --- > > > v2: > > > * Change "job_ring->uio_fd" type suggested by Gagandeep Singh > > > --- > > > drivers/crypto/caam_jr/caam_jr.c | 6 ++++-- > > > drivers/crypto/caam_jr/caam_jr_hw_specific.h | 2 +- > > > drivers/crypto/caam_jr/caam_jr_pvt.h | 5 +++-- > > > drivers/crypto/caam_jr/caam_jr_uio.c | 21 ++++++++++++++----= -- > > > 4 files changed, 23 insertions(+), 11 deletions(-) > > > > > > diff --git a/drivers/crypto/caam_jr/caam_jr.c > > > b/drivers/crypto/caam_jr/caam_jr.c > > > index 5a29dd169..f62d46546 100644 > > > --- a/drivers/crypto/caam_jr/caam_jr.c > > > +++ b/drivers/crypto/caam_jr/caam_jr.c > > > @@ -2074,7 +2074,7 @@ static struct rte_security_ops > > > caam_jr_security_ops =3D { static void close_job_ring(struct > > > sec_job_ring_t *job_ring) { > > > - if (job_ring->irq_fd) { > > > + if (job_ring->irq_fd !=3D -1) { > > > /* Producer index is frozen. If consumer index is not equal > > > * with producer index, then we have descs to flush. > > > */ > > > @@ -2187,7 +2187,7 @@ caam_jr_dev_uninit(struct rte_cryptodev *dev) > > > * > > > */ > > > static void * > > > -init_job_ring(void *reg_base_addr, uint32_t irq_id) > > > +init_job_ring(void *reg_base_addr, int irq_id) > > > { > > > struct sec_job_ring_t *job_ring =3D NULL; > > > int i, ret =3D 0; > > > @@ -2466,6 +2466,8 @@ > > > RTE_PMD_REGISTER_CRYPTO_DRIVER(caam_jr_crypto_drv, > > > cryptodev_caam_jr_drv.driver, > > > > > > RTE_INIT(caam_jr_init_log) > > > { > > > + sec_init(); > > > + > > Why are you calling a fd initialization function in log registration co= nstructor? > > This is not the right place for any initialization other than logs. > > Also, sec_init function will not work here, as you are using "g_uio_jr= _num" > > whose value will always be 0 during this function call. >=20 > Thanks, my bad. I should use 'MAX_SEC_JOB_RINGS'. > The default values of 'g_job_rings' and 'g_uio_job_ring' are 0. > As a result, the validity cannot be checked. what about following: >=20 > void sec_uio_job_rings_init(void) > { > int i; >=20 > for (i =3D 0; i < MAX_SEC_JOB_RINGS; i++) > g_uio_job_ring[i].uio_fd =3D -1; > } >=20 > void sec_job_rings_init(void) > { > int i; >=20 > for (i =3D 0; i < MAX_SEC_JOB_RINGS; i++) > g_job_rings[i].irq_fd =3D -1; > } >=20 > RTE_INIT(caam_jr_init) > { > sec_uio_job_rings_init(); > sec_job_rings_init(); > } >=20 This looks ok to me. You can combine both init functions into single one.=20 > > > > > caam_jr_logtype =3D rte_log_register("pmd.crypto.caam"); > > > if (caam_jr_logtype >=3D 0) > > > rte_log_set_level(caam_jr_logtype, RTE_LOG_NOTICE); diff -- > git > > > a/drivers/crypto/caam_jr/caam_jr_hw_specific.h > > > b/drivers/crypto/caam_jr/caam_jr_hw_specific.h > > > index 5f58a585d..ec4539d1b 100644 > > > --- a/drivers/crypto/caam_jr/caam_jr_hw_specific.h > > > +++ b/drivers/crypto/caam_jr/caam_jr_hw_specific.h > > > @@ -360,7 +360,7 @@ struct sec_job_ring_t { > > > * bitwise operations. > > > */ > > > > > > - uint32_t irq_fd; /* The file descriptor used for polling from > > > + int irq_fd; /* The file descriptor used for polling from > > > * user space for interrupts notifications > > > */ > > > uint32_t jr_mode; /* Model used by SEC Driver to receive > > > diff --git a/drivers/crypto/caam_jr/caam_jr_pvt.h > > > b/drivers/crypto/caam_jr/caam_jr_pvt.h > > > index 98cd4438a..fa9e86ea6 100644 > > > --- a/drivers/crypto/caam_jr/caam_jr_pvt.h > > > +++ b/drivers/crypto/caam_jr/caam_jr_pvt.h > > > @@ -216,16 +216,17 @@ calc_chksum(void *buffer, int len) } struct > > > uio_job_ring { > > > uint32_t jr_id; > > > - uint32_t uio_fd; > > > + int uio_fd; > > > void *register_base_addr; > > > int map_size; > > > int uio_minor_number; > > > }; > > > > > > int sec_cleanup(void); > > > +void sec_init(void); > > > int sec_configure(void); > > > struct uio_job_ring *config_job_ring(void); -void > > > free_job_ring(uint32_t uio_fd); > > > +void free_job_ring(int uio_fd); > > > > > > /* For Dma memory allocation of specified length and alignment */ > > > static inline void * diff --git a/drivers/crypto/caam_jr/caam_jr_uio.= c > > > b/drivers/crypto/caam_jr/caam_jr_uio.c > > > index b1bb44ca4..133b32084 100644 > > > --- a/drivers/crypto/caam_jr/caam_jr_uio.c > > > +++ b/drivers/crypto/caam_jr/caam_jr_uio.c > > > @@ -145,7 +145,7 @@ file_read_first_line(const char root[], const cha= r > > > subdir[], > > > "%s/%s/%s", root, subdir, filename); > > > > > > fd =3D open(absolute_file_name, O_RDONLY); > > > - SEC_ASSERT(fd > 0, fd, "Error opening file %s", > > > + SEC_ASSERT(fd >=3D 0, fd, "Error opening file %s", > > > absolute_file_name); > > > > > > /* read UIO device name from first line in file */ @@ -322,7 +322,7 > > > @@ uio_map_registers(int uio_device_fd, int uio_device_id, } > > > > > > void > > > -free_job_ring(uint32_t uio_fd) > > > +free_job_ring(int uio_fd) > > > { > > > struct uio_job_ring *job_ring =3D NULL; > > > int i; > > > > Please update the check "if (!uio_fd)". >=20 > OK, I will fix it. >=20 > > > > > @@ -347,7 +347,7 @@ free_job_ring(uint32_t uio_fd) > > > job_ring->jr_id, job_ring->uio_fd); > > > close(job_ring->uio_fd); > > > g_uio_jr_num--; > > > - job_ring->uio_fd =3D 0; > > > + job_ring->uio_fd =3D -1; > > > if (job_ring->register_base_addr =3D=3D NULL) > > > return; > > > > > > @@ -370,7 +370,7 @@ uio_job_ring *config_job_ring(void) > > > int i; > > > > > > for (i =3D 0; i < MAX_SEC_JOB_RINGS; i++) { > > > - if (g_uio_job_ring[i].uio_fd =3D=3D 0) { > > > + if (g_uio_job_ring[i].uio_fd =3D=3D -1) { > > > job_ring =3D &g_uio_job_ring[i]; > > > g_uio_jr_num++; > > > break; > > > @@ -389,7 +389,7 @@ uio_job_ring *config_job_ring(void) > > > > > > /* Open device file */ > > > job_ring->uio_fd =3D open(uio_device_file_name, O_RDWR); > > > - SEC_ASSERT(job_ring->uio_fd > 0, NULL, > > > + SEC_ASSERT(job_ring->uio_fd >=3D 0, NULL, > > > "Failed to open UIO device file for job ring %d", > > > job_ring->jr_id); > > > > > > @@ -488,12 +488,21 @@ sec_cleanup(void) > > > /* I need to close the fd after shutdown UIO commands need to > be > > > * sent using the fd > > > */ > > > - if (job_ring->uio_fd !=3D 0) { > > > + if (job_ring->uio_fd !=3D -1) { > > > CAAM_JR_INFO( > > > "Closed device file for job ring %d , fd =3D %d", > > > job_ring->jr_id, job_ring->uio_fd); > > > close(job_ring->uio_fd); > > > + job_ring->uio_fd =3D -1; > > > } > > > } > > > return 0; > > > } > > > + > > > +void sec_init(void) > > > +{ > > > + int i; > > > + > > > + for (i =3D 0; i < g_uio_jr_num; i++) > > > + g_uio_job_ring[i].uio_fd =3D -1; > > > +} > > > -- > > > 2.19.1 > > > > > > > There are still some functions(sec_uio_send_command, caam_jr_enable_irq= s, > > caam_jr_disable_irqs) in caam_jr_uio.c, which are accepting uio_fd para= meter > > as uint32_t, please update them also. > > > I will update them in next version. >=20