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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <VI1PR0402MB284781F3750D60017160690794B80@VI1PR0402MB2847.eurprd04.prod.outlook.com>
Date:   Thu, 10 Nov 2016 15:03:35 +0000
From:   Ruxandra Ioana Radulescu <ruxandra.radulescu@....com>
To:     Stuart Yoder <stuart.yoder@....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>,
        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.

> +	/*
> +	 * 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.

> +
> +	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.

Thanks,
Ioana

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ