From 0fe46944e2656b27cb8e941fb3752807847c25c2 Mon Sep 17 00:00:00 2001 From: sungjoo-XCENA Date: Wed, 12 Aug 2026 17:36:48 +0900 Subject: [PATCH] fix(core): split PRP chunks by DMA address, not user page offset mx_prp_first_chunk_len() derived the first chunk length from the CPU-side page offset (sg->offset), while the chunk itself is placed at sg_dma_address(). The device computes the first chunk length from the DMA address it receives, so whenever the mapping does not preserve the buffer's low address bits -- e.g. SWIOTLB bounce buffering -- every PRP entry after the first pointed at the wrong address. Transfers needing more than one PRP entry then silently corrupted data (shifted by the alignment delta) while the ioctl still returned success. Reproduced in a QEMU guest (no vIOMMU, SWIOTLB active): a 2576-byte DeviceInfo read at user page offset 1 was bounced to a 1 KiB-aligned DMA address; the driver emitted the desc list [base, base+1023, base+2047] instead of [base, base+1024, base+2048], shifting all data past byte 1023 by one. Direct mapping and IOMMU setups preserve the intra-page offset, which is why the bug stayed hidden on real hardware. Also branch create_mx_command_sg() (v2) on the DMA-side descriptor count instead of a host page count computed from the user virtual address, which could disagree with the DMA layout the same way. The SINGLE_DMA_SIZE == PAGE_SIZE static_assert only guarded that host-page branching and goes away with it. --- core_common.c | 9 ++++----- core_v2.c | 20 ++++++++------------ mx_dma.h | 4 ++-- 3 files changed, 14 insertions(+), 19 deletions(-) diff --git a/core_common.c b/core_common.c index 7ac0aa8..b665cfe 100644 --- a/core_common.c +++ b/core_common.c @@ -51,13 +51,12 @@ int mx_sg_locate(struct sg_table *sgt, size_t byte_offset, return -EINVAL; } -/* First PRP chunk length within an SG entry starting at intra_off; truncates so subsequent chunks - * land on dma_size boundaries. Returns dma_size when already aligned. Works for arbitrary - * dma_size (compiler folds the modulo to a bitmask when dma_size is a known power of 2). */ +/* First PRP chunk length at (sg, intra_off): distance to the next dma_size boundary of the + * mapped DMA address. Computed from sg_dma_address(), not the CPU page offset — the device + * splits by the address it receives, and SWIOTLB may not preserve the low address bits. */ size_t mx_prp_first_chunk_len(struct scatterlist *sg, size_t intra_off, size_t dma_size) { - size_t off_in_page = (sg->offset + intra_off) & (PAGE_SIZE - 1); - size_t rem = off_in_page % dma_size; + size_t rem = (sg_dma_address(sg) + intra_off) % dma_size; return rem ? (dma_size - rem) : dma_size; } diff --git a/core_v2.c b/core_v2.c index 4217e86..b0a7c07 100644 --- a/core_v2.c +++ b/core_v2.c @@ -226,11 +226,6 @@ static const struct mx_queue_ops v2_queue_ops = { #define SINGLE_DMA_SIZE PAGE_SIZE #define NUM_OF_DESC_PER_LIST (SINGLE_DMA_SIZE / sizeof(uint64_t)) -/* create_mx_command_sg branches on host page count (split_pages_nr) but emits PRP entries of dma_size - * (= SINGLE_DMA_SIZE). Branching is correct only while these match. */ -static_assert(SINGLE_DMA_SIZE == PAGE_SIZE, - "v2 PRP branching in create_mx_command_sg assumes SINGLE_DMA_SIZE == PAGE_SIZE"); - static struct mx_command *alloc_mx_command(struct mx_transfer *transfer, int opcode) { struct mx_command *comm = (struct mx_command *)transfer->cmd_inline; @@ -251,9 +246,7 @@ static void *create_mx_command_sg(struct mx_pci_dev *mx_pdev, struct mx_transfer struct sg_table *sgt = &transfer->sg_ctx->sgt; struct scatterlist *sg = NULL; size_t intra_off = 0; - unsigned int slice_offset_in_page = - offset_in_page((uintptr_t)transfer->sg_ctx->user_addr + transfer->sg_byte_offset); - int split_pages_nr = DIV_ROUND_UP(slice_offset_in_page + transfer->size, PAGE_SIZE); + size_t desc_cnt; int ret; comm = alloc_mx_command(transfer, opcode); @@ -274,19 +267,22 @@ static void *create_mx_command_sg(struct mx_pci_dev *mx_pdev, struct mx_transfer return NULL; } - if (split_pages_nr == 1) { + /* Branch on the DMA-side entry count, not host page count (alignments can differ). */ + desc_cnt = mx_get_total_desc_count(sg, intra_off, transfer->size, SINGLE_DMA_SIZE, false); + + if (desc_cnt == 1) { comm->prp_entry2 = 0; - } else if (split_pages_nr == 2) { + } else if (desc_cnt == 2) { size_t first_len = mx_prp_first_chunk_len(sg, intra_off, SINGLE_DMA_SIZE); - /* Second PRP entry points to the page after the first chunk. */ + /* Second PRP entry points to the chunk after the first. */ if (intra_off + first_len < sg_dma_len(sg)) { comm->prp_entry2 = comm->prp_entry1 + first_len; } else { struct scatterlist *next = sg_next(sg); if (!next) { - pr_warn("sg_next NULL in 2-page path (id=%u)\n", transfer->id); + pr_warn("sg_next NULL in 2-entry path (id=%u)\n", transfer->id); return NULL; } comm->prp_entry2 = sg_dma_address(next); diff --git a/mx_dma.h b/mx_dma.h index efacd28..e2db9e6 100644 --- a/mx_dma.h +++ b/mx_dma.h @@ -423,8 +423,8 @@ uint64_t mx_desc_list_init(struct mx_pci_dev *mx_pdev, struct mx_transfer *trans int mx_sg_locate(struct sg_table *sgt, size_t byte_offset, struct scatterlist **out_sg, size_t *out_intra); -/* First PRP chunk length when starting intra_off bytes into an SG entry; truncates so subsequent - * chunks land on dma_size boundaries. Returns dma_size when already aligned. See core_common.c. */ +/* First PRP chunk length at (sg, intra_off); splits fall on dma_size boundaries of the mapped + * DMA address. See core_common.c. */ size_t mx_prp_first_chunk_len(struct scatterlist *sg, size_t intra_off, size_t dma_size); void mx_stop_queue_threads(struct mx_pci_dev *mx_pdev);