| 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: <VI1PR0401MB263805FEF8B0055215D8E2688DB80@VI1PR0401MB2638.eurprd04.prod.outlook.com>
Date: Thu, 10 Nov 2016 17:25:38 +0000
From: Stuart Yoder <stuart.yoder@....com>
To: Ruxandra Ioana Radulescu <ruxandra.radulescu@....com>,
"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
CC: German Rivera <german.rivera@....com>,
"devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"agraf@...e.de" <agraf@...e.de>, "arnd@...db.de" <arnd@...db.de>,
Leo Li <leoyang.li@....com>, Roy Pledge <roy.pledge@....com>,
Haiying Wang <haiying.wang@....com>
Subject: RE: [PATCH 6/9] bus: fsl-mc: dpio: add QBMan portal APIs for DPAA2
> -----Original Message-----
> From: Ruxandra Ioana Radulescu
> Sent: Thursday, November 10, 2016 9:04 AM
> To: Stuart Yoder <stuart.yoder@....com>; gregkh@...uxfoundation.org
> Cc: German Rivera <german.rivera@....com>; devel@...verdev.osuosl.org; linux-kernel@...r.kernel.org;
> agraf@...e.de; arnd@...db.de; Leo Li <leoyang.li@....com>; Roy Pledge <roy.pledge@....com>; Haiying Wang
> <haiying.wang@....com>; Stuart Yoder <stuart.yoder@....com>
> Subject: RE: [PATCH 6/9] bus: fsl-mc: dpio: add QBMan portal APIs for DPAA2
>
>
> > -----Original Message-----
> > From: Stuart Yoder [mailto:stuart.yoder@....com]
> > Sent: Friday, October 21, 2016 9:02 AM
> > To: gregkh@...uxfoundation.org
> > Cc: German Rivera <german.rivera@....com>; devel@...verdev.osuosl.org;
> > linux-kernel@...r.kernel.org; agraf@...e.de; arnd@...db.de; Leo Li
> > <leoyang.li@....com>; Roy Pledge <roy.pledge@....com>; Roy Pledge
> > <roy.pledge@....com>; Haiying Wang <haiying.wang@....com>; Stuart
> > Yoder <stuart.yoder@....com>
> > Subject: [PATCH 6/9] bus: fsl-mc: dpio: add QBMan portal APIs for DPAA2
> >
> > From: Roy Pledge <Roy.Pledge@....com>
> >
> > Add QBman APIs for frame queue and buffer pool operations.
> >
> > Signed-off-by: Roy Pledge <roy.pledge@....com>
> > Signed-off-by: Haiying Wang <haiying.wang@....com>
> > Signed-off-by: Stuart Yoder <stuart.yoder@....com>
> > ---
> > 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
> > @@ -0,0 +1,1009 @@
>
> [...]
>
> > +/**
> > + * qbman_swp_release() - Issue a buffer release command
> > + * @s: the software portal object
> > + * @d: the release descriptor
> > + * @buffers: a pointer pointing to the buffer address to be released
> > + * @num_buffers: number of buffers to be released, must be less than 8
> > + *
> > + * Return 0 for success, -EBUSY if the release command ring is not ready.
> > + */
> > +int qbman_swp_release(struct qbman_swp *s, const struct
> > qbman_release_desc *d,
> > + const u64 *buffers, unsigned int num_buffers)
> > +{
> > + int i;
> > + struct qbman_release_desc *p;
> > + u32 rar;
> > +
> > + if (!num_buffers || (num_buffers > 7))
> > + return -EINVAL;
> > +
> > + rar = qbman_read_register(s, QBMAN_CINH_SWP_RAR);
> > + if (!RAR_SUCCESS(rar))
> > + return -EBUSY;
> > +
> > + /* Start the release command */
> > + p = qbman_get_cmd(s, QBMAN_CENA_SWP_RCR(RAR_IDX(rar)));
> > + /* Copy the caller's buffer pointers to the command */
> > + for (i = 0; i < num_buffers; i++)
> > + p->buf[i] = cpu_to_le64(buffers[i]);
> > +
>
> Hi Stuart,
> We also need to set BPID field in the buffer release command, something like:
> + p->bpid = d->bpid;
> Without this all buffers will be released to buffer pool id 0, which is incorrect.
Will fix on next respin.
> > + /*
> > + * Set the verb byte, have to substitute in the valid-bit and the
> > number
> > + * of buffers.
> > + */
> > + dma_wmb();
> > + p->verb = d->verb | RAR_VB(rar) | num_buffers;
> > +
> > + return 0;
> > +}
> > +
> > +struct qbman_acquire_desc {
> > + u8 verb;
> > + u8 reserved;
> > + u16 bpid;
> > + u8 num;
> > + u8 reserved2[59];
> > +};
> > +
> > +struct qbman_acquire_rslt {
> > + u8 verb;
> > + u8 rslt;
> > + u16 reserved;
> > + u8 num;
> > + u8 reserved2[3];
> > + u64 buf[7];
> > +};
> > +
> > +/**
> > + * qbman_swp_acquire() - Issue a buffer acquire command
> > + * @s: the software portal object
> > + * @bpid: the buffer pool index
> > + * @buffers: a pointer pointing to the acquired buffer addresses
> > + * @num_buffers: number of buffers to be acquired, must be less than 8
> > + *
> > + * Return 0 for success, or negative error code if the acquire command
> > + * fails.
> > + */
> > +int qbman_swp_acquire(struct qbman_swp *s, u16 bpid, u64 *buffers,
> > + unsigned int num_buffers)
> > +{
> > + struct qbman_acquire_desc *p;
> > + struct qbman_acquire_rslt *r;
> > + int i;
> > +
> > + if (!num_buffers || (num_buffers > 7))
> > + return -EINVAL;
> > +
> > + /* Start the management command */
> > + p = qbman_swp_mc_start(s);
>
> qbman_swp_mc_start() returns a pointer to where the QBMan
> management command must be written, but doesn't clear any
> previous values found there.
> We should memset the area to zero before using it.
> Same comment applies to other places where this function is used.
Ok, will fix.
> > +
> > + if (!p)
> > + return -EBUSY;
> > +
> > + /* Encode the caller-provided attributes */
> > + p->bpid = cpu_to_le16(bpid);
> > + p->num = num_buffers;
> > +
> > + /* Complete the management command */
> > + r = qbman_swp_mc_complete(s, p, p->verb |
> > QBMAN_MC_ACQUIRE);
>
> qbman_swp_mc_complete() may return NULL, if for instance the hardware
> is unresponsive. We need to check this before dereferencing r.
> Same for other instances of usage throughout the code.
Ok, will fix.
Thanks for the review.
Stuart
Powered by blists - more mailing lists