diff mbox series

[v3,virtio,1/8] virtio: allow caller to override device id and DMA mask

Message ID 20230322191038.44037-2-shannon.nelson@amd.com (mailing list archive)
State Not Applicable
Headers show
Series pds_vdpa driver | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Nelson, Shannon March 22, 2023, 7:10 p.m. UTC
To allow 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 XXX to specify an override device id check and a DMA mask.

Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
---
 drivers/virtio/virtio_pci_modern_dev.c | 36 +++++++++++++++++---------
 include/linux/virtio_pci_modern.h      |  6 +++++
 2 files changed, 30 insertions(+), 12 deletions(-)

Comments

Jason Wang March 23, 2023, 4:05 a.m. UTC | #1
On Thu, Mar 23, 2023 at 3:11 AM Shannon Nelson <shannon.nelson@amd.com> wrote:
>
> To allow 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 XXX to specify an override device id check and a DMA mask.
>
> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
> ---
>  drivers/virtio/virtio_pci_modern_dev.c | 36 +++++++++++++++++---------
>  include/linux/virtio_pci_modern.h      |  6 +++++
>  2 files changed, 30 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/virtio/virtio_pci_modern_dev.c b/drivers/virtio/virtio_pci_modern_dev.c
> index 869cb46bef96..6ad1bb9ae8fa 100644
> --- a/drivers/virtio/virtio_pci_modern_dev.c
> +++ b/drivers/virtio/virtio_pci_modern_dev.c
> @@ -221,18 +221,25 @@ int vp_modern_probe(struct virtio_pci_modern_device *mdev)
>
>         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_override) {
> +               err = mdev->device_id_check_override(pci_dev);
> +               if (err)
> +                       return err;
> +               mdev->id.device = pci_dev->device;

While at this, would it be better to let the device_id_check_override
to return the mdev->id.device ?

Others look good.

Thanks

>         } 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 +267,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_override)
> +               err = dma_set_mask_and_coherent(&pci_dev->dev,
> +                                               mdev->dma_mask_override);
> +       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..84765bbd8dc5 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;
> +
> +       /* alt. check for vendor virtio device, return 0 or -ERRNO */
> +       int (*device_id_check_override)(struct pci_dev *pdev);
> +
> +       /* alt. mask for devices with limited DMA space */
> +       u64 dma_mask_override;
>  };
>
>  /*
> --
> 2.17.1
>
Nelson, Shannon March 25, 2023, 12:26 a.m. UTC | #2
On 3/22/23 9:05 PM, Jason Wang wrote:
> On Thu, Mar 23, 2023 at 3:11 AM Shannon Nelson <shannon.nelson@amd.com> wrote:
>>
>> To allow 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 XXX to specify an override device id check and a DMA mask.
>>
>> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
>> ---
>>   drivers/virtio/virtio_pci_modern_dev.c | 36 +++++++++++++++++---------
>>   include/linux/virtio_pci_modern.h      |  6 +++++
>>   2 files changed, 30 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/virtio/virtio_pci_modern_dev.c b/drivers/virtio/virtio_pci_modern_dev.c
>> index 869cb46bef96..6ad1bb9ae8fa 100644
>> --- a/drivers/virtio/virtio_pci_modern_dev.c
>> +++ b/drivers/virtio/virtio_pci_modern_dev.c
>> @@ -221,18 +221,25 @@ int vp_modern_probe(struct virtio_pci_modern_device *mdev)
>>
>>          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_override) {
>> +               err = mdev->device_id_check_override(pci_dev);
>> +               if (err)
>> +                       return err;
>> +               mdev->id.device = pci_dev->device;
> 
> While at this, would it be better to let the device_id_check_override
> to return the mdev->id.device ?

Good idea - I'll add that.
sln


> 
> Others look good.
> 
> Thanks
> 
>>          } 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 +267,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_override)
>> +               err = dma_set_mask_and_coherent(&pci_dev->dev,
>> +                                               mdev->dma_mask_override);
>> +       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..84765bbd8dc5 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;
>> +
>> +       /* alt. check for vendor virtio device, return 0 or -ERRNO */
>> +       int (*device_id_check_override)(struct pci_dev *pdev);
>> +
>> +       /* alt. mask for devices with limited DMA space */
>> +       u64 dma_mask_override;
>>   };
>>
>>   /*
>> --
>> 2.17.1
>>
>
diff mbox series

Patch

diff --git a/drivers/virtio/virtio_pci_modern_dev.c b/drivers/virtio/virtio_pci_modern_dev.c
index 869cb46bef96..6ad1bb9ae8fa 100644
--- a/drivers/virtio/virtio_pci_modern_dev.c
+++ b/drivers/virtio/virtio_pci_modern_dev.c
@@ -221,18 +221,25 @@  int vp_modern_probe(struct virtio_pci_modern_device *mdev)
 
 	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_override) {
+		err = mdev->device_id_check_override(pci_dev);
+		if (err)
+			return err;
+		mdev->id.device = pci_dev->device;
 	} 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 +267,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_override)
+		err = dma_set_mask_and_coherent(&pci_dev->dev,
+						mdev->dma_mask_override);
+	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..84765bbd8dc5 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;
+
+	/* alt. check for vendor virtio device, return 0 or -ERRNO */
+	int (*device_id_check_override)(struct pci_dev *pdev);
+
+	/* alt. mask for devices with limited DMA space */
+	u64 dma_mask_override;
 };
 
 /*