linux-kernel - Re: [RFC v1 1/2] uio: Add SVA support for PCI devices via uio_pci_generic

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <2025092518-urologist-image-12ab@gregkh>
Date: Thu, 25 Sep 2025 08:02:36 +0200
From: Greg KH <gregkh@...uxfoundation.org>
To: Yaxing Guo <guoyaxing@...c.ac.cn>
Cc: linux-kernel@...r.kernel.org, anxu@...c.ac.cn, zhangjian@...c.ac.cn,
	wangran@...c.an.cn
Subject: Re: [RFC v1 1/2] uio: Add SVA support for PCI devices via
 uio_pci_generic_sva.c

On Thu, Sep 25, 2025 at 12:06:36PM +0800, Yaxing Guo wrote:
> This patch introduces a new UIO driver, uio_pci_generic_sva, which
> extends the functionality of uio_pci_generic by adding support for
> Shared Virtual Addressing (SVA) when IOMMU is enabled in the system.
> 
> The key enhancement allows PCI devices to directly use user-space virtual
> addresses for DMA operations, eliminating the need for bounce buffers or
> explicit IOVA mapping. This is achieved by leveraging the kernel's IOMMU-SVA
> subsystem, including process address space attachment, page fault handling,
> and shared context management between CPU and device.
> 
> With this driver, userspace applications can perform zero-copy DMA using
> native pointers:
> 
>     void *addr = malloc(N);
>     set_dma_addr((uint64_t)addr);  // Pass user VA directly
>     start_dma();
> 
> The device can now access 'addr' through the IOMMU's PASID-based translation,
> provided that the underlying IOMMU hardware (e.g., Intel VT-d 3.1+, AMD-Vi,
> ARM SMMU, RISCV IOMMU) and platform support SVA.
> 
> Dependencies:
> - CONFIG_IOMMU_SVA must be enabled.
> - The platform must support PRI (Page Request Interface) and PASID.
> - Device drivers/userspace must handle page faults if demand-paging is used.
> 
> The implementation reuses core logic from uio_pci_generic.c while adding
> PASID setting, and integration with the IOMMU SVA APIs.
> 
> Signed-off-by: Yaxing Guo <guoyaxing@...c.ac.cn>
> ---
>  drivers/uio/uio_pci_generic_sva.c | 188 ++++++++++++++++++++++++++++++
>  1 file changed, 188 insertions(+)
>  create mode 100644 drivers/uio/uio_pci_generic_sva.c
> 
> diff --git a/drivers/uio/uio_pci_generic_sva.c b/drivers/uio/uio_pci_generic_sva.c
> new file mode 100644
> index 000000000000..6a20c3487fed
> --- /dev/null
> +++ b/drivers/uio/uio_pci_generic_sva.c
> @@ -0,0 +1,188 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * UIO PCI Express sva driver
> + *
> + * Copyright (c) 2024 Beijing Institute of Open Source Chip (BOSC)

It is now 2025 :)


> + */
> +
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/uio_driver.h>
> +#include <linux/iommu.h>
> +
> +struct uio_pci_sva_dev {
> +	struct pci_dev *pdev;
> +	struct uio_info info;
> +	struct iommu_sva *sva_handle;
> +	int pasid;
> +};
> +
> +static ssize_t pasid_show(struct device *dev,
> +			  struct device_attribute *attr, char *buf)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	struct uio_pci_sva_dev *udev = pci_get_drvdata(pdev);
> +
> +	return sprintf(buf, "%d\n", udev->pasid);

sysfs_emit() please.

> +}
> +static DEVICE_ATTR_RO(pasid);

All sysfs attributes need to be documented in Documentation/ABI/

> +static int probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +{
> +	struct uio_pci_sva_dev *udev;
> +	int ret, i, irq;
> +
> +	ret = pci_enable_device(pdev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "pci_enable_device failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
> +	if (ret)
> +		goto out_disable;
> +
> +	pci_set_master(pdev);
> +
> +	ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSIX);
> +	if (ret) {
> +		irq = msi_get_virq(&pdev->dev, 0);
> +		if (irq == -ENXIO)
> +			ret = UIO_IRQ_NONE;
> +		else if (irq < 0) {
> +			ret = irq;
> +			goto out_disable;
> +		}
> +	} else {
> +		dev_warn(&pdev->dev, "Failed to allocate irq vectors\n");

Why doesn't this fail probe?

> +	}
> +
> +	udev = devm_kzalloc(&pdev->dev, sizeof(struct uio_pci_sva_dev),
> +			    GFP_KERNEL);
> +	if (!udev) {
> +		ret =  -ENOMEM;
> +		goto out_disable;
> +	}
> +
> +	udev->pdev = pdev;
> +	udev->info.name = "uio_pci_sva";
> +	udev->info.version = "0.0.1";
> +	udev->info.open = uio_pci_sva_open;
> +	udev->info.release = uio_pci_sva_release;
> +	udev->info.irq = irq;
> +	udev->info.handler = irq_handler;
> +	udev->info.priv = udev;
> +
> +	for (i = 0; i < MAX_UIO_MAPS; i++) {
> +		struct resource *r = &pdev->resource[i];
> +		struct uio_mem *uiomem = &udev->info.mem[i];
> +
> +		if (r->flags != (IORESOURCE_SIZEALIGN | IORESOURCE_MEM))
> +			continue;
> +
> +		if (uiomem >= &udev->info.mem[MAX_UIO_MAPS]) {
> +			dev_warn(&pdev->dev, "Do not support more than %d iomem\n",
> +				 MAX_UIO_MAPS);
> +			break;
> +		}
> +
> +		uiomem->memtype = UIO_MEM_PHYS;
> +		uiomem->addr = r->start & PAGE_MASK;
> +		uiomem->offs = r->start & ~PAGE_MASK;
> +		uiomem->size =
> +			(uiomem->offs + resource_size(r) + PAGE_SIZE - 1) &
> +			PAGE_MASK;
> +		uiomem->name = r->name;
> +	}
> +
> +	ret = sysfs_create_file(&pdev->dev.kobj, &dev_attr_pasid.attr);

You just raced with userspace and lost.  Please use the default
attribute list instead.

> +module_pci_driver(uio_pci_generic_sva_driver);
> +MODULE_LICENSE("GPL");

You now also need a MODULE_DESCRIPTION(), didn't you get a build warning
without it?

thanks,

greg k-h