On Mon, Jul 21, 2025 at 09:50:53AM +0200, Christoph Hellwig wrote:
> > if (entries == 1) {
> > - nvme_pci_sgl_set_data_sg(sg_list, sgl);
> > + iod->meta_total_len = iter.len;
> > + nvme_pci_sgl_set_data(sg_list, &iter);
> > + iod->nr_meta_descriptors = 0;
>
> This should probably just set up the linear metadata pointer instead
> of a single-segment SGL.
Okay, but we should still use SGL with user passthrough commands for
memory safety. Even if we have an iommu protecting access, there's still
a possibility of corrupting adjacent iova's if using MPTR.
> > + if (!iod->nr_meta_descriptors) {
> > + dma_unmap_page(dma_dev, le64_to_cpu(sg_list->addr),
> > + le32_to_cpu(sg_list->length), dir);
> > + return;
> > + }
> > +
> > + for (i = 1; i <= iod->nr_meta_descriptors; i++)
> > + dma_unmap_page(dma_dev, le64_to_cpu(sg_list[i].addr),
> > + le32_to_cpu(sg_list[i].length), dir);
> > +}
>
> The use of nr_meta_descriptors is still incorrect here. nr_descriptors
> counts the number of descriptors we got from the dma pools, which
> currently is always 1 for metadata SGLs. The length of the SGL
> descriptor simplify comes from le32_to_cpu(sg_list[0].length) divided
> by the sgl entry size.
In this patch, the nr_meta_descriptors value matches the sg_list length.
The only real reason I need this 'nr_' value is to distinguish the
single data descriptor condition from the segment descriptor use, but I
can just add an iod flag for that too and save some space.
