> -----Original Message----- > From: Ruxandra Ioana Radulescu > Sent: Monday, November 28, 2016 12:10 PM > To: Stuart Yoder <stuart.yoder@xxxxxxx>; gregkh@xxxxxxxxxxxxxxxxxxx > Cc: German Rivera <german.rivera@xxxxxxx>; devel@xxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > agraf@xxxxxxx; arnd@xxxxxxxx; Leo Li <leoyang.li@xxxxxxx>; Roy Pledge <roy.pledge@xxxxxxx>; Haiying Wang > <haiying.wang@xxxxxxx> > Subject: RE: [PATCH 6/9] bus: fsl-mc: dpio: add QBMan portal APIs for DPAA2 > > > -----Original Message----- > > From: Stuart Yoder [mailto:stuart.yoder@xxxxxxx] > > Sent: Friday, October 21, 2016 9:02 AM > > To: gregkh@xxxxxxxxxxxxxxxxxxx > > Cc: German Rivera <german.rivera@xxxxxxx>; devel@xxxxxxxxxxxxxxxxxxxx; > > linux-kernel@xxxxxxxxxxxxxxx; agraf@xxxxxxx; arnd@xxxxxxxx; Leo Li > > <leoyang.li@xxxxxxx>; Roy Pledge <roy.pledge@xxxxxxx>; Roy Pledge > > <roy.pledge@xxxxxxx>; Haiying Wang <haiying.wang@xxxxxxx>; Stuart > > Yoder <stuart.yoder@xxxxxxx> > > Subject: [PATCH 6/9] bus: fsl-mc: dpio: add QBMan portal APIs for DPAA2 > > > > From: Roy Pledge <Roy.Pledge@xxxxxxx> > > > > Add QBman APIs for frame queue and buffer pool operations. > > > > Signed-off-by: Roy Pledge <roy.pledge@xxxxxxx> > > Signed-off-by: Haiying Wang <haiying.wang@xxxxxxx> > > Signed-off-by: Stuart Yoder <stuart.yoder@xxxxxxx> > > --- > > 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. Will fix this on v3 respin. Stuart _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
