On Tue, 25 Apr 2023 14:25:53 -0700, Shannon Nelson <shannon.nelson@xxxxxxx> wrote:
> To add a bit of flexibility with various virtio based devices, allow
> the caller to specify a different device id and DMA mask. This adds
> fields to struct virtio_pci_modern_device to specify an override device
> id check and a DMA mask.
>
> int (*device_id_check)(struct pci_dev *pdev);
> If defined by the driver, this function will be called to check
> that the PCI device is the vendor's expected device, and will
> return the found device id to be stored in mdev->id.device.
> This allows vendors with alternative vendor device ids to use
> this library on their own device BAR.
>
> u64 dma_mask;
> If defined by the driver, this mask will be used in a call to
> dma_set_mask_and_coherent() instead of the traditional
> DMA_BIT_MASK(64). This allows limiting the DMA space on
> vendor devices with address limitations.
>
> Signed-off-by: Shannon Nelson <shannon.nelson@xxxxxxx>
> ---
> drivers/virtio/virtio_pci_modern_dev.c | 37 +++++++++++++++++---------
> include/linux/virtio_pci_modern.h | 6 +++++
> 2 files changed, 31 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/virtio/virtio_pci_modern_dev.c b/drivers/virtio/virtio_pci_modern_dev.c
> index 869cb46bef96..1f2db76e8f91 100644
> --- a/drivers/virtio/virtio_pci_modern_dev.c
> +++ b/drivers/virtio/virtio_pci_modern_dev.c
> @@ -218,21 +218,29 @@ int vp_modern_probe(struct virtio_pci_modern_device *mdev)
> int err, common, isr, notify, device;
> u32 notify_length;
> u32 notify_offset;
> + int devid;
>
> check_offsets();
>
> - /* We only own devices >= 0x1000 and <= 0x107f: leave the rest. */
> - if (pci_dev->device < 0x1000 || pci_dev->device > 0x107f)
> - return -ENODEV;
> -
> - if (pci_dev->device < 0x1040) {
> - /* Transitional devices: use the PCI subsystem device id as
> - * virtio device id, same as legacy driver always did.
> - */
> - mdev->id.device = pci_dev->subsystem_device;
> + if (mdev->device_id_check) {
> + devid = mdev->device_id_check(pci_dev);
> + if (devid < 0)
> + return devid;
I would want to know is there any other reason to return the errno?
How about return -ENODEV directly?
Thanks.
> + mdev->id.device = devid;
> } else {
> - /* Modern devices: simply use PCI device id, but start from 0x1040. */
> - mdev->id.device = pci_dev->device - 0x1040;
> + /* We only own devices >= 0x1000 and <= 0x107f: leave the rest. */
> + if (pci_dev->device < 0x1000 || pci_dev->device > 0x107f)
> + return -ENODEV;
> +
> + if (pci_dev->device < 0x1040) {
> + /* Transitional devices: use the PCI subsystem device id as
> + * virtio device id, same as legacy driver always did.
> + */
> + mdev->id.device = pci_dev->subsystem_device;
> + } else {
> + /* Modern devices: simply use PCI device id, but start from 0x1040. */
> + mdev->id.device = pci_dev->device - 0x1040;
> + }
> }
> mdev->id.vendor = pci_dev->subsystem_vendor;
>
> @@ -260,7 +268,12 @@ int vp_modern_probe(struct virtio_pci_modern_device *mdev)
> return -EINVAL;
> }
>
> - err = dma_set_mask_and_coherent(&pci_dev->dev, DMA_BIT_MASK(64));
> + if (mdev->dma_mask)
> + err = dma_set_mask_and_coherent(&pci_dev->dev,
> + mdev->dma_mask);
> + else
> + err = dma_set_mask_and_coherent(&pci_dev->dev,
> + DMA_BIT_MASK(64));
> if (err)
> err = dma_set_mask_and_coherent(&pci_dev->dev,
> DMA_BIT_MASK(32));
> diff --git a/include/linux/virtio_pci_modern.h b/include/linux/virtio_pci_modern.h
> index c4eeb79b0139..067ac1d789bc 100644
> --- a/include/linux/virtio_pci_modern.h
> +++ b/include/linux/virtio_pci_modern.h
> @@ -38,6 +38,12 @@ struct virtio_pci_modern_device {
> int modern_bars;
>
> struct virtio_device_id id;
> +
> + /* optional check for vendor virtio device, returns dev_id or -ERRNO */
> + int (*device_id_check)(struct pci_dev *pdev);
> +
> + /* optional mask for devices with limited DMA space */
> + u64 dma_mask;
> };
>
> /*
> --
> 2.17.1
>
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
