Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756028AbcK2BqR (ORCPT ); Mon, 28 Nov 2016 20:46:17 -0500 Received: from mail-db5eur01on0045.outbound.protection.outlook.com ([104.47.2.45]:3136 "EHLO EUR01-DB5-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1755715AbcK2BqC (ORCPT ); Mon, 28 Nov 2016 20:46:02 -0500 From: Ruxandra Ioana Radulescu To: Stuart Yoder , "gregkh@linuxfoundation.org" CC: German Rivera , "devel@driverdev.osuosl.org" , "linux-kernel@vger.kernel.org" , "agraf@suse.de" , "arnd@arndb.de" , Leo Li , Roy Pledge , Haiying Wang Subject: RE: [PATCH 6/9] bus: fsl-mc: dpio: add QBMan portal APIs for DPAA2 Thread-Topic: [PATCH 6/9] bus: fsl-mc: dpio: add QBMan portal APIs for DPAA2 Thread-Index: AQHSK6SU/JJyAb48sEOHR53H+I1ID6DHaGwwgCd/JmA= Date: Mon, 28 Nov 2016 18:09:33 +0000 Message-ID: References: <1477058509-12547-1-git-send-email-stuart.yoder@nxp.com> <1477058509-12547-7-git-send-email-stuart.yoder@nxp.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=ruxandra.radulescu@nxp.com; x-originating-ip: [192.88.146.1] x-microsoft-exchange-diagnostics: 1;HE1PR0401MB2633;7:3vjowlHx+pHd3pSubRXqJWo/iK9exDt0/QKbCwYWm9jU093sRb4WO+XIYrauCGrZGaaEGVoBQn+imRauGqY/HBdrmc3q/24iraacSftjStb9CbZpR2C8dq2S/h1h4sXzJ2rxWWsVxSBAd70/PhW8opAwWqqU3tGW7dmnthhwTKIput/j7Zk6Rr6HB7MBj6PVIuARiNcSI1Vyf+zULVjF+z/MSBgD/PBcSQ7SLXb3ikMOlcQQzmxFtMXx8m0FSa95I8VNFNWBqXd0ctfcnPrKFPXBBL/HKtw51/TgRw18e8MsaIxCZTCMHwNQPKSiTjez8LvuE1gXs9HfCkVGzE7vBuA1PNtsma9dtoYVA5EW/f/OCaz4X3t56OqmMgKFuhcwSyiYQwK48QDwYC0L1uDbhTJoqsAhxBFXHJ9Izybwn5WPXvAbenMtihRXG5+7rV99xCrBOovRgdjmSqUJsZfvvA== x-forefront-antispam-report: SFV:SKI;SCL:-1SFV:NSPM;SFS:(10009020)(6009001)(7916002)(189002)(199003)(377454003)(13464003)(3660700001)(81166006)(8676002)(81156014)(8936002)(5660300001)(2906002)(2950100002)(3280700002)(68736007)(92566002)(122556002)(7696004)(189998001)(86362001)(102836003)(6116002)(3846002)(9686002)(97736004)(5001770100001)(2900100001)(77096006)(39450400002)(76576001)(106356001)(105586002)(7846002)(106116001)(2501003)(76176999)(54356999)(101416001)(4326007)(39380400001)(39410400001)(38730400001)(50986999)(33656002)(6506003)(66066001)(229853002)(39400400001)(7736002)(305945005)(74316002);DIR:OUT;SFP:1101;SCL:1;SRVR:HE1PR0401MB2633;H:VI1PR0402MB2847.eurprd04.prod.outlook.com;FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; x-ms-office365-filtering-correlation-id: 1011397e-91ea-4cad-83e2-08d417b9b4a5 x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001);SRVR:HE1PR0401MB2633; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(9452136761055)(185117386973197)(275809806118684); x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(6060326)(6040361)(6045199)(601004)(2401047)(5005006)(8121501046)(3002001)(10201501046)(6055026)(6041248)(6061324)(6046074)(20161123562025)(20161123555025)(20161123560025)(20161123564025);SRVR:HE1PR0401MB2633;BCL:0;PCL:0;RULEID:;SRVR:HE1PR0401MB2633; x-forefront-prvs: 01401330D1 spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 X-OriginatorOrg: nxp.com X-MS-Exchange-CrossTenant-originalarrivaltime: 28 Nov 2016 18:09:33.0815 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR0401MB2633 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id uAT1kSu1006146 Content-Length: 4550 Lines: 125 > -----Original Message----- > From: Stuart Yoder [mailto:stuart.yoder@nxp.com] > Sent: Friday, October 21, 2016 9:02 AM > To: gregkh@linuxfoundation.org > Cc: German Rivera ; devel@driverdev.osuosl.org; > linux-kernel@vger.kernel.org; agraf@suse.de; arnd@arndb.de; Leo Li > ; Roy Pledge ; Roy Pledge > ; Haiying Wang ; Stuart > Yoder > Subject: [PATCH 6/9] bus: fsl-mc: dpio: add QBMan portal APIs for DPAA2 > > From: Roy Pledge > > Add QBman APIs for frame queue and buffer pool operations. > > Signed-off-by: Roy Pledge > Signed-off-by: Haiying Wang > Signed-off-by: Stuart Yoder > --- > drivers/bus/fsl-mc/dpio/Makefile | 2 +- > drivers/bus/fsl-mc/dpio/qbman-portal.c | 1009 > ++++++++++++++++++++++++++++++++ > drivers/bus/fsl-mc/dpio/qbman-portal.h | 464 +++++++++++++++ > 3 files changed, 1474 insertions(+), 1 deletion(-) > create mode 100644 drivers/bus/fsl-mc/dpio/qbman-portal.c > create mode 100644 drivers/bus/fsl-mc/dpio/qbman-portal.h > > diff --git a/drivers/bus/fsl-mc/dpio/Makefile b/drivers/bus/fsl- > mc/dpio/Makefile > index 128befc..6588498 100644 > --- a/drivers/bus/fsl-mc/dpio/Makefile > +++ b/drivers/bus/fsl-mc/dpio/Makefile > @@ -6,4 +6,4 @@ subdir-ccflags-y := -Werror > > obj-$(CONFIG_FSL_MC_DPIO) += fsl-mc-dpio.o > > -fsl-mc-dpio-objs := dpio.o > +fsl-mc-dpio-objs := dpio.o qbman-portal.o > diff --git a/drivers/bus/fsl-mc/dpio/qbman-portal.c b/drivers/bus/fsl- > mc/dpio/qbman-portal.c > new file mode 100644 > index 0000000..1eb3dd9 > --- /dev/null > +++ b/drivers/bus/fsl-mc/dpio/qbman-portal.c [...] > +/** > + * qbman_swp_pull() - Issue the pull dequeue command > + * @s: the software portal object > + * @d: the software portal descriptor which has been configured with > + * the set of qbman_pull_desc_set_*() calls > + * > + * Return 0 for success, and -EBUSY if the software portal is not ready > + * to do pull dequeue. > + */ > +int qbman_swp_pull(struct qbman_swp *s, struct qbman_pull_desc *d) > +{ > + struct qbman_pull_desc *p; > + > + if (!atomic_dec_and_test(&s->vdq.available)) { > + atomic_inc(&s->vdq.available); > + return -EBUSY; > + } > + s->vdq.storage = (void *)d->rsp_addr_virt; > + d->tok = 1; > + p = qbman_get_cmd(s, QBMAN_CENA_SWP_VDQCR); > + *p = *d; > + dma_wmb(); > + > + /* Set the verb byte, have to substitute in the valid-bit */ > + p->verb |= s->vdq.valid_bit; > + s->vdq.valid_bit ^= QB_VALID_BIT; > + > + return 0; > +} > + [...] > + > +/** > + * qbman_result_has_new_result() - Check and get the dequeue response > from the > + * dq storage memory set in pull dequeue command > + * @s: the software portal object > + * @dq: the dequeue result read from the memory > + * > + * Return 1 for getting a valid dequeue result, or 0 for not getting a valid > + * dequeue result. > + * > + * Only used for user-provided storage of dequeue results, not DQRR. For > + * efficiency purposes, the driver will perform any required endianness > + * conversion to ensure that the user's dequeue result storage is in host- > endian > + * format. As such, once the user has called > qbman_result_has_new_result() and > + * been returned a valid dequeue result, they should not call it again on > + * the same memory location (except of course if another dequeue > command has > + * been executed to produce a new result to that location). > + */ > +int qbman_result_has_new_result(struct qbman_swp *s, const struct > dpaa2_dq *dq) > +{ > + if (!dq->dq.tok) > + return 0; While testing the Ethernet driver I discovered that sometimes the above check fails. When we check a store entry for the first time, if the hardware didn't manage to fill it with a valid respose yet, we may find a non-null value in the token field (because the stores have uninitialized memory). So by only checking that token is !=0, we risk processing an uninitialized memory area as a valid dequeue entry. We should always compare the token field against 1, the value that is given to the hardware on the dequeue command. It might also be a good idea to use a define here, to make it clear 1 is a magic value. And I think we should also zero the stores when they are first allocated, since even with the proposed fix there's still a (much smaller) risk of finding our exact token value in an uninitialized memory area. Thanks, Ioana