Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 8 Apr 2015 23:14:52 +0000
From:      "Bjoern A. Zeeb" <bz@FreeBSD.org>
To:        Jim Harris <jimharris@FreeBSD.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r281281 - head/sys/dev/nvme
Message-ID:  <6E785386-3746-48A7-87B5-D33CDD999767@FreeBSD.org>
In-Reply-To: <201504082149.t38LnjBZ058856@svn.freebsd.org>
References:  <201504082149.t38LnjBZ058856@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help

> On 08 Apr 2015, at 21:49 , Jim Harris <jimharris@FreeBSD.org> wrote:
>=20
> Author: jimharris
> Date: Wed Apr  8 21:49:45 2015
> New Revision: 281281
> URL: https://svnweb.freebsd.org/changeset/base/281281
>=20
> Log:
>  nvme: create separate DMA tag for non-payload DMA buffers
>=20
>  Submission and completion queue memory need to use a
>  separate DMA tag for mappings than payload buffers,
>  to ensure mappings remain contiguous even with DMAR
>  enabled.
>=20
>  Submitted by:	kib
>  MFC after:	1 week
>  Sponsored by:	Intel
>=20
> Modified:
>  head/sys/dev/nvme/nvme_private.h
>  head/sys/dev/nvme/nvme_qpair.c

I think this one break i386 PAE and XEN kernels at least:

=
/scratch/tmp/bz/head.svn/sys/modules/nvme/../../dev/nvme/nvme_qpair.c:502:=
6: error: implicit conversion from 'unsigned long long' to 'bus_size_t' =
(aka 'unsigned int') changes value from 18446744073709551615 to =
4294967295 [-Werr
or,-Wconstant-conversion]
            BUS_SPACE_MAXADDR, 1, BUS_SPACE_MAXSIZE, 0,
            ^~~~~~~~~~~~~~~~~
./x86/bus.h:121:27: note: expanded from macro 'BUS_SPACE_MAXADDR'
#define BUS_SPACE_MAXADDR       0xFFFFFFFFFFFFFFFFULL
                                ^~~~~~~~~~~~~~~~~~~~~
1 error generated.
--- nvme_qpair.o ---
*** [nvme_qpair.o] Error code 1



=
/scratch/tmp/bz/head.svn/sys/modules/nvme/../../dev/nvme/nvme_qpair.c:502:=
6: error: implicit conversion from 'unsigned long long' to 'bus_size_t' =
(aka 'unsigned int') changes value from 18446744073709551615 to =
4294967295 [-Werr
or,-Wconstant-conversion]
            BUS_SPACE_MAXADDR, 1, BUS_SPACE_MAXSIZE, 0,
            ^~~~~~~~~~~~~~~~~
./x86/bus.h:121:27: note: expanded from macro 'BUS_SPACE_MAXADDR'
#define BUS_SPACE_MAXADDR       0xFFFFFFFFFFFFFFFFULL
                                ^~~~~~~~~~~~~~~~~~~~~
1 error generated.
--- nvme_qpair.o ---




>=20
> Modified: head/sys/dev/nvme/nvme_private.h
> =
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D
> --- head/sys/dev/nvme/nvme_private.h	Wed Apr  8 21:46:18 2015	=
(r281280)
> +++ head/sys/dev/nvme/nvme_private.h	Wed Apr  8 21:49:45 2015	=
(r281281)
> @@ -211,6 +211,7 @@ struct nvme_qpair {
> 	struct nvme_completion	*cpl;
>=20
> 	bus_dma_tag_t		dma_tag;
> +	bus_dma_tag_t		dma_tag_payload;
>=20
> 	bus_dmamap_t		cmd_dma_map;
> 	uint64_t		cmd_bus_addr;
> @@ -491,6 +492,8 @@ nvme_single_map(void *arg, bus_dma_segme
> {
> 	uint64_t *bus_addr =3D (uint64_t *)arg;
>=20
> +	if (error !=3D 0)
> +		printf("nvme_single_map err %d\n", error);
> 	*bus_addr =3D seg[0].ds_addr;
> }
>=20
>=20
> Modified: head/sys/dev/nvme/nvme_qpair.c
> =
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D
> --- head/sys/dev/nvme/nvme_qpair.c	Wed Apr  8 21:46:18 2015	=
(r281280)
> +++ head/sys/dev/nvme/nvme_qpair.c	Wed Apr  8 21:49:45 2015	=
(r281281)
> @@ -294,7 +294,7 @@ nvme_qpair_construct_tracker(struct nvme
>     uint16_t cid)
> {
>=20
> -	bus_dmamap_create(qpair->dma_tag, 0, &tr->payload_dma_map);
> +	bus_dmamap_create(qpair->dma_tag_payload, 0, =
&tr->payload_dma_map);
> 	bus_dmamap_create(qpair->dma_tag, 0, &tr->prp_dma_map);
>=20
> 	bus_dmamap_load(qpair->dma_tag, tr->prp_dma_map, tr->prp,
> @@ -337,7 +337,7 @@ nvme_qpair_complete_tracker(struct nvme_
> 		nvme_qpair_submit_tracker(qpair, tr);
> 	} else {
> 		if (req->type !=3D NVME_REQUEST_NULL)
> -			bus_dmamap_unload(qpair->dma_tag,
> +			bus_dmamap_unload(qpair->dma_tag_payload,
> 			    tr->payload_dma_map);
>=20
> 		nvme_free_request(req);
> @@ -464,6 +464,7 @@ nvme_qpair_construct(struct nvme_qpair *
> {
> 	struct nvme_tracker	*tr;
> 	uint32_t		i;
> +	int			err;
>=20
> 	qpair->id =3D id;
> 	qpair->vector =3D vector;
> @@ -497,11 +498,20 @@ nvme_qpair_construct(struct nvme_qpair *
> 	mtx_init(&qpair->lock, "nvme qpair lock", NULL, MTX_DEF);
>=20
> 	/* Note: NVMe PRP format is restricted to 4-byte alignment. */
> -	bus_dma_tag_create(bus_get_dma_tag(ctrlr->dev),
> +	err =3D bus_dma_tag_create(bus_get_dma_tag(ctrlr->dev),
> 	    4, PAGE_SIZE, BUS_SPACE_MAXADDR,
> 	    BUS_SPACE_MAXADDR, NULL, NULL, NVME_MAX_XFER_SIZE,
> 	    (NVME_MAX_XFER_SIZE/PAGE_SIZE)+1, PAGE_SIZE, 0,
> +	    NULL, NULL, &qpair->dma_tag_payload);
> +	if (err !=3D 0)
> +		nvme_printf(ctrlr, "payload tag create failed %d\n", =
err);
> +
> +	err =3D bus_dma_tag_create(bus_get_dma_tag(ctrlr->dev),
> +	    4, 0, BUS_SPACE_MAXADDR, BUS_SPACE_MAXADDR, NULL, NULL,
> +	    BUS_SPACE_MAXADDR, 1, BUS_SPACE_MAXSIZE, 0,
> 	    NULL, NULL, &qpair->dma_tag);
> +	if (err !=3D 0)
> +		nvme_printf(ctrlr, "tag create failed %d\n", err);
>=20
> 	qpair->num_cmds =3D 0;
> 	qpair->num_intr_handler_calls =3D 0;
> @@ -513,8 +523,13 @@ nvme_qpair_construct(struct nvme_qpair *
> 	    sizeof(struct nvme_completion), M_NVME, M_ZERO,
> 	    0, BUS_SPACE_MAXADDR, PAGE_SIZE, 0);
>=20
> -	bus_dmamap_create(qpair->dma_tag, 0, &qpair->cmd_dma_map);
> -	bus_dmamap_create(qpair->dma_tag, 0, &qpair->cpl_dma_map);
> +	err =3D bus_dmamap_create(qpair->dma_tag, 0, =
&qpair->cmd_dma_map);
> +	if (err !=3D 0)
> +		nvme_printf(ctrlr, "cmd_dma_map create failed %d\n", =
err);
> +
> +	err =3D bus_dmamap_create(qpair->dma_tag, 0, =
&qpair->cpl_dma_map);
> +	if (err !=3D 0)
> +		nvme_printf(ctrlr, "cpl_dma_map create failed %d\n", =
err);
>=20
> 	bus_dmamap_load(qpair->dma_tag, qpair->cmd_dma_map,
> 	    qpair->cmd, qpair->num_entries * sizeof(struct =
nvme_command),
> @@ -570,6 +585,9 @@ nvme_qpair_destroy(struct nvme_qpair *qp
> 	if (qpair->dma_tag)
> 		bus_dma_tag_destroy(qpair->dma_tag);
>=20
> +	if (qpair->dma_tag_payload)
> +		bus_dma_tag_destroy(qpair->dma_tag_payload);
> +
> 	if (qpair->act_tr)
> 		free(qpair->act_tr, M_NVME);
>=20
> @@ -707,8 +725,11 @@ nvme_payload_map(void *arg, bus_dma_segm
> 	 *  is responsible for detecting the error status and failing =
the
> 	 *  tracker manually.
> 	 */
> -	if (error !=3D 0)
> +	if (error !=3D 0) {
> +		nvme_printf(tr->qpair->ctrlr,
> +		    "nvme_payload_map err %d\n", error);
> 		return;
> +	}
>=20
> 	/*
> 	 * Note that we specified PAGE_SIZE for alignment and max
> @@ -728,6 +749,13 @@ nvme_payload_map(void *arg, bus_dma_segm
> 			    (uint64_t)seg[cur_nseg].ds_addr;
> 			cur_nseg++;
> 		}
> +	} else {
> +		/*
> +		 * prp2 should not be used by the controller
> +		 *  since there is only one segment, but set
> +		 *  to 0 just to be safe.
> +		 */
> +		tr->req->cmd.prp2 =3D 0;
> 	}
>=20
> 	nvme_qpair_submit_tracker(tr->qpair, tr);
> @@ -780,8 +808,9 @@ _nvme_qpair_submit_request(struct nvme_q
> 		KASSERT(req->payload_size <=3D =
qpair->ctrlr->max_xfer_size,
> 		    ("payload_size (%d) exceeds max_xfer_size (%d)\n",
> 		    req->payload_size, qpair->ctrlr->max_xfer_size));
> -		err =3D bus_dmamap_load(tr->qpair->dma_tag, =
tr->payload_dma_map,
> -		    req->u.payload, req->payload_size, nvme_payload_map, =
tr, 0);
> +		err =3D bus_dmamap_load(tr->qpair->dma_tag_payload,
> +		    tr->payload_dma_map, req->u.payload, =
req->payload_size,
> +		    nvme_payload_map, tr, 0);
> 		if (err !=3D 0)
> 			nvme_printf(qpair->ctrlr,
> 			    "bus_dmamap_load returned 0x%x!\n", err);
> @@ -795,7 +824,7 @@ _nvme_qpair_submit_request(struct nvme_q
> 		    ("bio->bio_bcount (%jd) exceeds max_xfer_size =
(%d)\n",
> 		    (intmax_t)req->u.bio->bio_bcount,
> 		    qpair->ctrlr->max_xfer_size));
> -		err =3D bus_dmamap_load_bio(tr->qpair->dma_tag,
> +		err =3D bus_dmamap_load_bio(tr->qpair->dma_tag_payload,
> 		    tr->payload_dma_map, req->u.bio, nvme_payload_map, =
tr, 0);
> 		if (err !=3D 0)
> 			nvme_printf(qpair->ctrlr,
>=20

=E2=80=94=20
Bjoern A. Zeeb                                  Charles Haddon Spurgeon:
"Friendship is one of the sweetest joys of life.  Many might have failed
 beneath the bitterness of their trial  had they not found a friend."




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6E785386-3746-48A7-87B5-D33CDD999767>