Module Name: src Committed By: bouyer Date: Sun Nov 26 12:38:56 UTC 2023
Modified Files: src/sys/dev/pci [netbsd-10]: pciide_common.c Log Message: Pull up following revision(s) (requested by thorpej in ticket #470): sys/dev/pci/pciide_common.c: revision 1.70 pciide_dma_dmamap_setup(): If we end up with a DMA segment with an odd length or odd starting address, unload the map and return EINVAL. Some controllers get really upset if a DMA segment has an odd address or length. This can happen if a physio user performs a virtually-contiguous I/O that starts at an odd address and spans a page boundary where the resulting physical pages are discontiguous. The EINVAL return will cause the upper layers in the ATA code to re-try the I/O using PIO, which should (will in all of my tests) succeed. PR port-alpha/56434 To generate a diff of this commit: cvs rdiff -u -r1.67 -r1.67.20.1 src/sys/dev/pci/pciide_common.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/sys/dev/pci/pciide_common.c diff -u src/sys/dev/pci/pciide_common.c:1.67 src/sys/dev/pci/pciide_common.c:1.67.20.1 --- src/sys/dev/pci/pciide_common.c:1.67 Mon Aug 24 05:37:41 2020 +++ src/sys/dev/pci/pciide_common.c Sun Nov 26 12:38:56 2023 @@ -1,4 +1,4 @@ -/* $NetBSD: pciide_common.c,v 1.67 2020/08/24 05:37:41 msaitoh Exp $ */ +/* $NetBSD: pciide_common.c,v 1.67.20.1 2023/11/26 12:38:56 bouyer Exp $ */ /* @@ -70,7 +70,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: pciide_common.c,v 1.67 2020/08/24 05:37:41 msaitoh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: pciide_common.c,v 1.67.20.1 2023/11/26 12:38:56 bouyer Exp $"); #include <sys/param.h> @@ -721,25 +721,51 @@ pciide_dma_dmamap_setup(struct pciide_so BUS_DMASYNC_PREREAD : BUS_DMASYNC_PREWRITE); for (seg = 0; seg < dma_maps->dmamap_xfer->dm_nsegs; seg++) { + bus_addr_t phys = dma_maps->dmamap_xfer->dm_segs[seg].ds_addr; + bus_size_t len = dma_maps->dmamap_xfer->dm_segs[seg].ds_len; + #ifdef DIAGNOSTIC /* A segment must not cross a 64k boundary */ { - u_long phys = dma_maps->dmamap_xfer->dm_segs[seg].ds_addr; - u_long len = dma_maps->dmamap_xfer->dm_segs[seg].ds_len; if ((phys & ~IDEDMA_BYTE_COUNT_MASK) != ((phys + len - 1) & ~IDEDMA_BYTE_COUNT_MASK)) { - printf("pciide_dma: segment %d physical addr 0x%lx" - " len 0x%lx not properly aligned\n", - seg, phys, len); + printf("pciide_dma: seg %d addr 0x%" PRIx64 + " len 0x%" PRIx64 " not properly aligned\n", + seg, (uint64_t)phys, (uint64_t)len); panic("pciide_dma: buf align"); } } #endif - dma_maps->dma_table[seg].base_addr = - htole32(dma_maps->dmamap_xfer->dm_segs[seg].ds_addr); + /* + * Some controllers get really upset if the length + * of any DMA segment is odd. This isn't something + * that's going to happen in normal steady-state + * operation (reading VM pages, etc.), but physio users + * don't have as many guard rails. + * + * Consider an 8K read request that starts at an odd + * offset within a page. At first blush, all of the + * checks pass because it's a sector-rounded size, but + * unless the buffer spans 2 physically contiguous pages, + * it's going to result in 2 odd-length DMA segments. + * + * Odd start addresses are also frowned upon, so we + * catch those here, too. + * + * Returning EINVAL here will cause the upper layers to + * fall back onto PIO. + */ + if ((phys & 1) != 0 || (len & 1) != 0) { + aprint_verbose_dev(sc->sc_wdcdev.sc_atac.atac_dev, + "Invalid DMA segment: " + "seg %d addr 0x%" PRIx64 " len 0x%" PRIx64 "\n", + seg, (uint64_t)phys, (uint64_t)len); + bus_dmamap_unload(sc->sc_dmat, dma_maps->dmamap_xfer); + return EINVAL; + } + dma_maps->dma_table[seg].base_addr = htole32(phys); dma_maps->dma_table[seg].byte_count = - htole32(dma_maps->dmamap_xfer->dm_segs[seg].ds_len & - IDEDMA_BYTE_COUNT_MASK); + htole32(len & IDEDMA_BYTE_COUNT_MASK); ATADEBUG_PRINT(("\t seg %d len %d addr 0x%x\n", seg, le32toh(dma_maps->dma_table[seg].byte_count), le32toh(dma_maps->dma_table[seg].base_addr)), DEBUG_DMA);