Date: Fri, 19 Jan 2024 17:17:36 GMT From: Alexander Motin <mav@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: c3b67deaf514 - stable/14 - mpi3mr: Honor the dma mask from IOCFacts Message-ID: <202401191717.40JHHaHP097110@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/14 has been updated by mav: URL: https://cgit.FreeBSD.org/src/commit/?id=c3b67deaf51464ca4bdd1676ef279a67596715ee commit c3b67deaf51464ca4bdd1676ef279a67596715ee Author: Warner Losh <imp@FreeBSD.org> AuthorDate: 2023-11-29 01:49:49 +0000 Commit: Alexander Motin <mav@FreeBSD.org> CommitDate: 2024-01-19 17:16:50 +0000 mpi3mr: Honor the dma mask from IOCFacts The number of signficant bits that are decoded are returned in the flags field of the IOCFacts structure from the device. Rather than assume the worst with a pessimal 32-bit maximum, look at this value and pass it along to all the dma map creation requests. A lof of those creations are repetitive and could just inherit from the base tag if we moved to the templated interface. This is called out as desireable future work not done at this time. In addition, due to a chicken and an egg problem, we have to allocate some of the maps with a 32-bit loaddr. These are the ones we need to read iocfacts. And they are fine to be so restricted: they are little used after startup, and when they are used, bouncing is fine. Sponsored by: Netflix Reviewed by: mav Differential Revision: https://reviews.freebsd.org/D42559 (cherry picked from commit 91d961356d03465635c4784fab48acdd1304e1e0) --- sys/dev/mpi3mr/mpi3mr.c | 27 +++++++++++++++++++++++++++ sys/dev/mpi3mr/mpi3mr_pci.c | 10 ++++++++++ 2 files changed, 37 insertions(+) diff --git a/sys/dev/mpi3mr/mpi3mr.c b/sys/dev/mpi3mr/mpi3mr.c index f6c4bb4496e6..580f42ea0179 100644 --- a/sys/dev/mpi3mr/mpi3mr.c +++ b/sys/dev/mpi3mr/mpi3mr.c @@ -981,6 +981,11 @@ static int mpi3mr_setup_admin_qpair(struct mpi3mr_softc *sc) sc->admin_reply_ephase = 1; if (!sc->admin_req) { + /* + * We need to create the tag for the admin queue to get the + * iofacts to see how many bits the controller decodes. Solve + * this chicken and egg problem by only doing lower 4GB DMA. + */ if (bus_dma_tag_create(sc->mpi3mr_parent_dmat, /* parent */ 4, 0, /* algnmnt, boundary */ BUS_SPACE_MAXADDR_32BIT,/* lowaddr */ @@ -1435,6 +1440,12 @@ static int mpi3mr_issue_iocfacts(struct mpi3mr_softc *sc, MPI3_SGE_FLAGS_END_OF_LIST); + /* + * We can't use sc->dma_loaddr / hiaddr here. We set those only after + * we get the iocfacts. So allocate in the lower 4GB. The amount of + * data is tiny and we don't do this that often, so any bouncing we + * might have to do isn't a cause for concern. + */ if (bus_dma_tag_create(sc->mpi3mr_parent_dmat, /* parent */ 4, 0, /* algnmnt, boundary */ BUS_SPACE_MAXADDR_32BIT,/* lowaddr */ @@ -1669,6 +1680,22 @@ static int mpi3mr_process_factsdata(struct mpi3mr_softc *sc, sc->max_host_ios = sc->facts.max_reqs - (MPI3MR_INTERNALCMDS_RESVD + 1); + /* + * Set the DMA mask for the card. dma_mask is the number of bits that + * can have bits set in them. Translate this into bus_dma loaddr/hiaddr + * args. Add sanity for more bits than address space or other overflow + * situations. + */ + if (sc->facts.dma_mask == 0 || + (sc->facts.dma_mask >= sizeof(bus_addr_t) * 8)) + sc->dma_loaddr = BUS_SPACE_MAXADDR; + else + sc->dma_loaddr = ~((1ull << sc->facts.dma_mask) - 1); + sc->dma_hiaddr = BUS_SPACE_MAXADDR; + mpi3mr_dprint(sc, MPI3MR_INFO, + "dma_mask bits: %d loaddr 0x%jx hiaddr 0x%jx\n", + sc->facts.dma_mask, sc->dma_loaddr, sc->dma_hiaddr); + return retval; } diff --git a/sys/dev/mpi3mr/mpi3mr_pci.c b/sys/dev/mpi3mr/mpi3mr_pci.c index eaf73022291d..11ec98882949 100644 --- a/sys/dev/mpi3mr/mpi3mr_pci.c +++ b/sys/dev/mpi3mr/mpi3mr_pci.c @@ -273,6 +273,16 @@ static int mpi3mr_setup_resources(struct mpi3mr_softc *sc) sc->mpi3mr_btag = rman_get_bustag(sc->mpi3mr_regs_resource); sc->mpi3mr_bhandle = rman_get_bushandle(sc->mpi3mr_regs_resource); + /* + * XXX Perhaps we should move this to after we read iocfacts and use + * that to create the proper parent tag. However, to get the iocfacts + * we need to have a dmatag for both the admin queue and the iocfacts + * DMA transfer. So for now, we just create a 'no restriction' tag and + * use sc->dma_loaddr for all the other tag_create calls to get the + * right value. It would be nice if one could retroactively adjust a + * created tag. The Linux driver effectively does this by setting the + * dma_mask on the device. + */ /* Allocate the parent DMA tag */ if (bus_dma_tag_create(bus_get_dma_tag(dev), /* parent */ 1, 0, /* algnmnt, boundary */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202401191717.40JHHaHP097110>