* [PATCH 0/3] vDPA/ifcvf: enables Intel C5000X-PL virtio-blk
@ 2021-04-14 9:18 Zhu Lingshan
2021-04-14 9:18 ` [PATCH 1/3] vDPA/ifcvf: deduce VIRTIO device ID when probe Zhu Lingshan
` (2 more replies)
0 siblings, 3 replies; 19+ messages in thread
From: Zhu Lingshan @ 2021-04-14 9:18 UTC (permalink / raw)
To: jasowang, mst, lulu, leonro
Cc: virtualization, netdev, kvm, linux-kernel, Zhu Lingshan
This series enabled Intel FGPA SmartNIC C5000X-PL virtio-blk for vDPA.
This series requires:
Stefano's vdpa block patchset: https://lkml.org/lkml/2021/3/15/2113
my patchset to enable Intel FGPA SmartNIC C5000X-PL virtio-net for vDPA:
https://lkml.org/lkml/2021/3/17/432
Thanks!
Zhu Lingshan (3):
vDPA/ifcvf: deduce VIRTIO device ID when probe
vDPA/ifcvf: enable Intel C5000X-PL virtio-block for vDPA
vDPA/ifcvf: get_config_size should return dev specific config size
drivers/vdpa/ifcvf/ifcvf_base.h | 18 +++++++++++++-
drivers/vdpa/ifcvf/ifcvf_main.c | 43 ++++++++++++++++++++++-----------
2 files changed, 46 insertions(+), 15 deletions(-)
--
2.27.0
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH 1/3] vDPA/ifcvf: deduce VIRTIO device ID when probe 2021-04-14 9:18 [PATCH 0/3] vDPA/ifcvf: enables Intel C5000X-PL virtio-blk Zhu Lingshan @ 2021-04-14 9:18 ` Zhu Lingshan 2021-04-15 3:30 ` Jason Wang 2021-04-14 9:18 ` [PATCH 2/3] vDPA/ifcvf: enable Intel C5000X-PL virtio-block for vDPA Zhu Lingshan 2021-04-14 9:18 ` [PATCH 3/3] vDPA/ifcvf: get_config_size should return dev specific config size Zhu Lingshan 2 siblings, 1 reply; 19+ messages in thread From: Zhu Lingshan @ 2021-04-14 9:18 UTC (permalink / raw) To: jasowang, mst, lulu, leonro Cc: virtualization, netdev, kvm, linux-kernel, Zhu Lingshan This commit deduces VIRTIO device ID as device type when probe, then ifcvf_vdpa_get_device_id() can simply return the ID. ifcvf_vdpa_get_features() and ifcvf_vdpa_get_config_size() can work properly based on the device ID. Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> --- drivers/vdpa/ifcvf/ifcvf_base.h | 1 + drivers/vdpa/ifcvf/ifcvf_main.c | 22 ++++++++++------------ 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index b2eeb16b9c2c..1c04cd256fa7 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -84,6 +84,7 @@ struct ifcvf_hw { u32 notify_off_multiplier; u64 req_features; u64 hw_features; + u32 dev_type; struct virtio_pci_common_cfg __iomem *common_cfg; void __iomem *net_cfg; struct vring_info vring[IFCVF_MAX_QUEUE_PAIRS * 2]; diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 44d7586019da..99b0a6b4c227 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -323,19 +323,9 @@ static u32 ifcvf_vdpa_get_generation(struct vdpa_device *vdpa_dev) static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev) { - struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev); - struct pci_dev *pdev = adapter->pdev; - u32 ret = -ENODEV; - - if (pdev->device < 0x1000 || pdev->device > 0x107f) - return ret; - - if (pdev->device < 0x1040) - ret = pdev->subsystem_device; - else - ret = pdev->device - 0x1040; + struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); - return ret; + return vf->dev_type; } static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev) @@ -466,6 +456,14 @@ static int ifcvf_probe(struct pci_dev *pdev, const struct pci_device_id *id) pci_set_drvdata(pdev, adapter); vf = &adapter->vf; + if (pdev->device < 0x1000 || pdev->device > 0x107f) + return -EOPNOTSUPP; + + if (pdev->device < 0x1040) + vf->dev_type = pdev->subsystem_device; + else + vf->dev_type = pdev->device - 0x1040; + vf->base = pcim_iomap_table(pdev); adapter->pdev = pdev; -- 2.27.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] vDPA/ifcvf: deduce VIRTIO device ID when probe 2021-04-14 9:18 ` [PATCH 1/3] vDPA/ifcvf: deduce VIRTIO device ID when probe Zhu Lingshan @ 2021-04-15 3:30 ` Jason Wang 2021-04-15 5:52 ` Zhu Lingshan 0 siblings, 1 reply; 19+ messages in thread From: Jason Wang @ 2021-04-15 3:30 UTC (permalink / raw) To: Zhu Lingshan, mst, lulu, leonro; +Cc: virtualization, netdev, kvm, linux-kernel 在 2021/4/14 下午5:18, Zhu Lingshan 写道: > This commit deduces VIRTIO device ID as device type when probe, > then ifcvf_vdpa_get_device_id() can simply return the ID. > ifcvf_vdpa_get_features() and ifcvf_vdpa_get_config_size() > can work properly based on the device ID. > > Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> > --- > drivers/vdpa/ifcvf/ifcvf_base.h | 1 + > drivers/vdpa/ifcvf/ifcvf_main.c | 22 ++++++++++------------ > 2 files changed, 11 insertions(+), 12 deletions(-) > > diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h > index b2eeb16b9c2c..1c04cd256fa7 100644 > --- a/drivers/vdpa/ifcvf/ifcvf_base.h > +++ b/drivers/vdpa/ifcvf/ifcvf_base.h > @@ -84,6 +84,7 @@ struct ifcvf_hw { > u32 notify_off_multiplier; > u64 req_features; > u64 hw_features; > + u32 dev_type; > struct virtio_pci_common_cfg __iomem *common_cfg; > void __iomem *net_cfg; > struct vring_info vring[IFCVF_MAX_QUEUE_PAIRS * 2]; > diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c > index 44d7586019da..99b0a6b4c227 100644 > --- a/drivers/vdpa/ifcvf/ifcvf_main.c > +++ b/drivers/vdpa/ifcvf/ifcvf_main.c > @@ -323,19 +323,9 @@ static u32 ifcvf_vdpa_get_generation(struct vdpa_device *vdpa_dev) > > static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev) > { > - struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev); > - struct pci_dev *pdev = adapter->pdev; > - u32 ret = -ENODEV; > - > - if (pdev->device < 0x1000 || pdev->device > 0x107f) > - return ret; > - > - if (pdev->device < 0x1040) > - ret = pdev->subsystem_device; > - else > - ret = pdev->device - 0x1040; > + struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); > > - return ret; > + return vf->dev_type; > } > > static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev) > @@ -466,6 +456,14 @@ static int ifcvf_probe(struct pci_dev *pdev, const struct pci_device_id *id) > pci_set_drvdata(pdev, adapter); > > vf = &adapter->vf; > + if (pdev->device < 0x1000 || pdev->device > 0x107f) > + return -EOPNOTSUPP; > + > + if (pdev->device < 0x1040) > + vf->dev_type = pdev->subsystem_device; > + else > + vf->dev_type = pdev->device - 0x1040; So a question here, is the device a transtional device or modern one? If it's a transitonal one, can it swtich endianess automatically or not? Thanks > + > vf->base = pcim_iomap_table(pdev); > > adapter->pdev = pdev; ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] vDPA/ifcvf: deduce VIRTIO device ID when probe 2021-04-15 3:30 ` Jason Wang @ 2021-04-15 5:52 ` Zhu Lingshan 2021-04-15 6:30 ` Jason Wang 0 siblings, 1 reply; 19+ messages in thread From: Zhu Lingshan @ 2021-04-15 5:52 UTC (permalink / raw) To: Jason Wang, Zhu Lingshan, mst, lulu, leonro Cc: virtualization, netdev, kvm, linux-kernel On 4/15/2021 11:30 AM, Jason Wang wrote: > > 在 2021/4/14 下午5:18, Zhu Lingshan 写道: >> This commit deduces VIRTIO device ID as device type when probe, >> then ifcvf_vdpa_get_device_id() can simply return the ID. >> ifcvf_vdpa_get_features() and ifcvf_vdpa_get_config_size() >> can work properly based on the device ID. >> >> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> >> --- >> drivers/vdpa/ifcvf/ifcvf_base.h | 1 + >> drivers/vdpa/ifcvf/ifcvf_main.c | 22 ++++++++++------------ >> 2 files changed, 11 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h >> b/drivers/vdpa/ifcvf/ifcvf_base.h >> index b2eeb16b9c2c..1c04cd256fa7 100644 >> --- a/drivers/vdpa/ifcvf/ifcvf_base.h >> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h >> @@ -84,6 +84,7 @@ struct ifcvf_hw { >> u32 notify_off_multiplier; >> u64 req_features; >> u64 hw_features; >> + u32 dev_type; >> struct virtio_pci_common_cfg __iomem *common_cfg; >> void __iomem *net_cfg; >> struct vring_info vring[IFCVF_MAX_QUEUE_PAIRS * 2]; >> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c >> b/drivers/vdpa/ifcvf/ifcvf_main.c >> index 44d7586019da..99b0a6b4c227 100644 >> --- a/drivers/vdpa/ifcvf/ifcvf_main.c >> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c >> @@ -323,19 +323,9 @@ static u32 ifcvf_vdpa_get_generation(struct >> vdpa_device *vdpa_dev) >> static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev) >> { >> - struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev); >> - struct pci_dev *pdev = adapter->pdev; >> - u32 ret = -ENODEV; >> - >> - if (pdev->device < 0x1000 || pdev->device > 0x107f) >> - return ret; >> - >> - if (pdev->device < 0x1040) >> - ret = pdev->subsystem_device; >> - else >> - ret = pdev->device - 0x1040; >> + struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); >> - return ret; >> + return vf->dev_type; >> } >> static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev) >> @@ -466,6 +456,14 @@ static int ifcvf_probe(struct pci_dev *pdev, >> const struct pci_device_id *id) >> pci_set_drvdata(pdev, adapter); >> vf = &adapter->vf; >> + if (pdev->device < 0x1000 || pdev->device > 0x107f) >> + return -EOPNOTSUPP; >> + >> + if (pdev->device < 0x1040) >> + vf->dev_type = pdev->subsystem_device; >> + else >> + vf->dev_type = pdev->device - 0x1040; > > > So a question here, is the device a transtional device or modern one? > > If it's a transitonal one, can it swtich endianess automatically or not? > > Thanks Hi Jason, This driver should drive both modern and transitional devices as we discussed before. If it's a transitional one, it will act as a modern device by default, legacy mode is a fail-over path. For vDPA, it has to support VIRTIO_1 and ACCESS_PLATFORM, so it must in modern mode. I think we don't need to worry about endianess for legacy mode. Thanks Zhu Lingshan > > >> + >> vf->base = pcim_iomap_table(pdev); >> adapter->pdev = pdev; > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] vDPA/ifcvf: deduce VIRTIO device ID when probe 2021-04-15 5:52 ` Zhu Lingshan @ 2021-04-15 6:30 ` Jason Wang 2021-04-15 6:36 ` Zhu Lingshan 0 siblings, 1 reply; 19+ messages in thread From: Jason Wang @ 2021-04-15 6:30 UTC (permalink / raw) To: Zhu Lingshan, Zhu Lingshan, mst, lulu, leonro Cc: virtualization, netdev, kvm, linux-kernel 在 2021/4/15 下午1:52, Zhu Lingshan 写道: > > > On 4/15/2021 11:30 AM, Jason Wang wrote: >> >> 在 2021/4/14 下午5:18, Zhu Lingshan 写道: >>> This commit deduces VIRTIO device ID as device type when probe, >>> then ifcvf_vdpa_get_device_id() can simply return the ID. >>> ifcvf_vdpa_get_features() and ifcvf_vdpa_get_config_size() >>> can work properly based on the device ID. >>> >>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> >>> --- >>> drivers/vdpa/ifcvf/ifcvf_base.h | 1 + >>> drivers/vdpa/ifcvf/ifcvf_main.c | 22 ++++++++++------------ >>> 2 files changed, 11 insertions(+), 12 deletions(-) >>> >>> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h >>> b/drivers/vdpa/ifcvf/ifcvf_base.h >>> index b2eeb16b9c2c..1c04cd256fa7 100644 >>> --- a/drivers/vdpa/ifcvf/ifcvf_base.h >>> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h >>> @@ -84,6 +84,7 @@ struct ifcvf_hw { >>> u32 notify_off_multiplier; >>> u64 req_features; >>> u64 hw_features; >>> + u32 dev_type; >>> struct virtio_pci_common_cfg __iomem *common_cfg; >>> void __iomem *net_cfg; >>> struct vring_info vring[IFCVF_MAX_QUEUE_PAIRS * 2]; >>> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c >>> b/drivers/vdpa/ifcvf/ifcvf_main.c >>> index 44d7586019da..99b0a6b4c227 100644 >>> --- a/drivers/vdpa/ifcvf/ifcvf_main.c >>> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c >>> @@ -323,19 +323,9 @@ static u32 ifcvf_vdpa_get_generation(struct >>> vdpa_device *vdpa_dev) >>> static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev) >>> { >>> - struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev); >>> - struct pci_dev *pdev = adapter->pdev; >>> - u32 ret = -ENODEV; >>> - >>> - if (pdev->device < 0x1000 || pdev->device > 0x107f) >>> - return ret; >>> - >>> - if (pdev->device < 0x1040) >>> - ret = pdev->subsystem_device; >>> - else >>> - ret = pdev->device -0x1040; >>> + struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); >>> - return ret; >>> + return vf->dev_type; >>> } >>> static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev) >>> @@ -466,6 +456,14 @@ static int ifcvf_probe(struct pci_dev *pdev, >>> const struct pci_device_id *id) >>> pci_set_drvdata(pdev, adapter); >>> vf = &adapter->vf; >>> + if (pdev->device < 0x1000 || pdev->device > 0x107f) >>> + return -EOPNOTSUPP; >>> + >>> + if (pdev->device < 0x1040) >>> + vf->dev_type = pdev->subsystem_device; >>> + else >>> + vf->dev_type = pdev->device - 0x1040; >> >> >> So a question here, is the device a transtional device or modern one? >> >> If it's a transitonal one, can it swtich endianess automatically or not? >> >> Thanks > Hi Jason, > > This driver should drive both modern and transitional devices as we > discussed before. > If it's a transitional one, it will act as a modern device by default, > legacy mode is a fail-over path. Note that legacy driver use native endian, support legacy driver requires the device to know native endian which I'm not sure your device can do that. Thanks > For vDPA, it has to support VIRTIO_1 and ACCESS_PLATFORM, so it must > in modern mode. > I think we don't need to worry about endianess for legacy mode. > > Thanks > Zhu Lingshan >> >> >>> + >>> vf->base = pcim_iomap_table(pdev); >>> adapter->pdev = pdev; >> > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] vDPA/ifcvf: deduce VIRTIO device ID when probe 2021-04-15 6:30 ` Jason Wang @ 2021-04-15 6:36 ` Zhu Lingshan 2021-04-15 7:16 ` Jason Wang 0 siblings, 1 reply; 19+ messages in thread From: Zhu Lingshan @ 2021-04-15 6:36 UTC (permalink / raw) To: Jason Wang, Zhu Lingshan, mst, lulu, leonro Cc: virtualization, netdev, kvm, linux-kernel On 4/15/2021 2:30 PM, Jason Wang wrote: > > 在 2021/4/15 下午1:52, Zhu Lingshan 写道: >> >> >> On 4/15/2021 11:30 AM, Jason Wang wrote: >>> >>> 在 2021/4/14 下午5:18, Zhu Lingshan 写道: >>>> This commit deduces VIRTIO device ID as device type when probe, >>>> then ifcvf_vdpa_get_device_id() can simply return the ID. >>>> ifcvf_vdpa_get_features() and ifcvf_vdpa_get_config_size() >>>> can work properly based on the device ID. >>>> >>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> >>>> --- >>>> drivers/vdpa/ifcvf/ifcvf_base.h | 1 + >>>> drivers/vdpa/ifcvf/ifcvf_main.c | 22 ++++++++++------------ >>>> 2 files changed, 11 insertions(+), 12 deletions(-) >>>> >>>> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h >>>> b/drivers/vdpa/ifcvf/ifcvf_base.h >>>> index b2eeb16b9c2c..1c04cd256fa7 100644 >>>> --- a/drivers/vdpa/ifcvf/ifcvf_base.h >>>> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h >>>> @@ -84,6 +84,7 @@ struct ifcvf_hw { >>>> u32 notify_off_multiplier; >>>> u64 req_features; >>>> u64 hw_features; >>>> + u32 dev_type; >>>> struct virtio_pci_common_cfg __iomem *common_cfg; >>>> void __iomem *net_cfg; >>>> struct vring_info vring[IFCVF_MAX_QUEUE_PAIRS * 2]; >>>> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c >>>> b/drivers/vdpa/ifcvf/ifcvf_main.c >>>> index 44d7586019da..99b0a6b4c227 100644 >>>> --- a/drivers/vdpa/ifcvf/ifcvf_main.c >>>> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c >>>> @@ -323,19 +323,9 @@ static u32 ifcvf_vdpa_get_generation(struct >>>> vdpa_device *vdpa_dev) >>>> static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev) >>>> { >>>> - struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev); >>>> - struct pci_dev *pdev = adapter->pdev; >>>> - u32 ret = -ENODEV; >>>> - >>>> - if (pdev->device < 0x1000 || pdev->device > 0x107f) >>>> - return ret; >>>> - >>>> - if (pdev->device < 0x1040) >>>> - ret = pdev->subsystem_device; >>>> - else >>>> - ret = pdev->device -0x1040; >>>> + struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); >>>> - return ret; >>>> + return vf->dev_type; >>>> } >>>> static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev) >>>> @@ -466,6 +456,14 @@ static int ifcvf_probe(struct pci_dev *pdev, >>>> const struct pci_device_id *id) >>>> pci_set_drvdata(pdev, adapter); >>>> vf = &adapter->vf; >>>> + if (pdev->device < 0x1000 || pdev->device > 0x107f) >>>> + return -EOPNOTSUPP; >>>> + >>>> + if (pdev->device < 0x1040) >>>> + vf->dev_type = pdev->subsystem_device; >>>> + else >>>> + vf->dev_type = pdev->device - 0x1040; >>> >>> >>> So a question here, is the device a transtional device or modern one? >>> >>> If it's a transitonal one, can it swtich endianess automatically or >>> not? >>> >>> Thanks >> Hi Jason, >> >> This driver should drive both modern and transitional devices as we >> discussed before. >> If it's a transitional one, it will act as a modern device by >> default, legacy mode is a fail-over path. > > > Note that legacy driver use native endian, support legacy driver > requires the device to know native endian which I'm not sure your > device can do that. > > Thanks Yes, legacy requires guest native endianess, I think we don't need to worry about this because our transitional device should work in modern mode by default(legacy mode is the failover path we will never reach, get_features will fail if no ACCESS_PLATFORM), we don't support legacy device in vDPA. Thanks > > >> For vDPA, it has to support VIRTIO_1 and ACCESS_PLATFORM, so it must >> in modern mode. >> I think we don't need to worry about endianess for legacy mode. >> >> Thanks >> Zhu Lingshan >>> >>> >>>> + >>>> vf->base = pcim_iomap_table(pdev); >>>> adapter->pdev = pdev; >>> >> > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] vDPA/ifcvf: deduce VIRTIO device ID when probe 2021-04-15 6:36 ` Zhu Lingshan @ 2021-04-15 7:16 ` Jason Wang 2021-04-15 7:23 ` Zhu Lingshan 0 siblings, 1 reply; 19+ messages in thread From: Jason Wang @ 2021-04-15 7:16 UTC (permalink / raw) To: Zhu Lingshan, Zhu Lingshan, mst, lulu, leonro Cc: virtualization, netdev, kvm, linux-kernel 在 2021/4/15 下午2:36, Zhu Lingshan 写道: > > > On 4/15/2021 2:30 PM, Jason Wang wrote: >> >> 在 2021/4/15 下午1:52, Zhu Lingshan 写道: >>> >>> >>> On 4/15/2021 11:30 AM, Jason Wang wrote: >>>> >>>> 在 2021/4/14 下午5:18, Zhu Lingshan 写道: >>>>> This commit deduces VIRTIO device ID as device type when probe, >>>>> then ifcvf_vdpa_get_device_id() can simply return the ID. >>>>> ifcvf_vdpa_get_features() and ifcvf_vdpa_get_config_size() >>>>> can work properly based on the device ID. >>>>> >>>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> >>>>> --- >>>>> drivers/vdpa/ifcvf/ifcvf_base.h | 1 + >>>>> drivers/vdpa/ifcvf/ifcvf_main.c | 22 ++++++++++------------ >>>>> 2 files changed, 11 insertions(+), 12 deletions(-) >>>>> >>>>> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h >>>>> b/drivers/vdpa/ifcvf/ifcvf_base.h >>>>> index b2eeb16b9c2c..1c04cd256fa7 100644 >>>>> --- a/drivers/vdpa/ifcvf/ifcvf_base.h >>>>> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h >>>>> @@ -84,6 +84,7 @@ struct ifcvf_hw { >>>>> u32 notify_off_multiplier; >>>>> u64 req_features; >>>>> u64 hw_features; >>>>> + u32 dev_type; >>>>> struct virtio_pci_common_cfg __iomem *common_cfg; >>>>> void __iomem *net_cfg; >>>>> struct vring_info vring[IFCVF_MAX_QUEUE_PAIRS * 2]; >>>>> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c >>>>> b/drivers/vdpa/ifcvf/ifcvf_main.c >>>>> index 44d7586019da..99b0a6b4c227 100644 >>>>> --- a/drivers/vdpa/ifcvf/ifcvf_main.c >>>>> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c >>>>> @@ -323,19 +323,9 @@ static u32 ifcvf_vdpa_get_generation(struct >>>>> vdpa_device *vdpa_dev) >>>>> static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev) >>>>> { >>>>> - struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev); >>>>> - struct pci_dev *pdev = adapter->pdev; >>>>> - u32 ret = -ENODEV; >>>>> - >>>>> - if (pdev->device < 0x1000 || pdev->device > 0x107f) >>>>> - return ret; >>>>> - >>>>> - if (pdev->device < 0x1040) >>>>> - ret = pdev->subsystem_device; >>>>> - else >>>>> - ret = pdev->device-0x1040; >>>>> + struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); >>>>> - return ret; >>>>> + return vf->dev_type; >>>>> } >>>>> static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev) >>>>> @@ -466,6 +456,14 @@ static int ifcvf_probe(struct pci_dev *pdev, >>>>> const struct pci_device_id *id) >>>>> pci_set_drvdata(pdev, adapter); >>>>> vf = &adapter->vf; >>>>> + if (pdev->device < 0x1000 || pdev->device > 0x107f) >>>>> + return -EOPNOTSUPP; >>>>> + >>>>> + if (pdev->device < 0x1040) >>>>> + vf->dev_type = pdev->subsystem_device; >>>>> + else >>>>> + vf->dev_type = pdev->device - 0x1040; >>>> >>>> >>>> So a question here, is the device a transtional device or modern one? >>>> >>>> If it's a transitonal one, can it swtich endianess automatically or >>>> not? >>>> >>>> Thanks >>> Hi Jason, >>> >>> This driver should drive both modern and transitional devices as we >>> discussed before. >>> If it's a transitional one, it will act as a modern device by >>> default, legacy mode is a fail-over path. >> >> >> Note that legacy driver use native endian, support legacy driver >> requires the device to know native endian which I'm not sure your >> device can do that. >> >> Thanks > Yes, legacy requires guest native endianess, I think we don't need to > worry about this because our transitional device should work in modern > mode by > default(legacy mode is the failover path we will never reach, > get_features will fail if no ACCESS_PLATFORM), we don't support legacy > device in vDPA. > > Thanks Ok, so I think it's better to add a comment here. Thanks >> >> >>> For vDPA, it has to support VIRTIO_1 and ACCESS_PLATFORM, so it must >>> in modern mode. >>> I think we don't need to worry about endianess for legacy mode. >>> >>> Thanks >>> Zhu Lingshan >>>> >>>> >>>>> + >>>>> vf->base = pcim_iomap_table(pdev); >>>>> adapter->pdev = pdev; >>>> >>> >> > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] vDPA/ifcvf: deduce VIRTIO device ID when probe 2021-04-15 7:16 ` Jason Wang @ 2021-04-15 7:23 ` Zhu Lingshan 0 siblings, 0 replies; 19+ messages in thread From: Zhu Lingshan @ 2021-04-15 7:23 UTC (permalink / raw) To: Jason Wang, Zhu Lingshan, mst, lulu, leonro Cc: virtualization, netdev, kvm, linux-kernel On 4/15/2021 3:16 PM, Jason Wang wrote: > > 在 2021/4/15 下午2:36, Zhu Lingshan 写道: >> >> >> On 4/15/2021 2:30 PM, Jason Wang wrote: >>> >>> 在 2021/4/15 下午1:52, Zhu Lingshan 写道: >>>> >>>> >>>> On 4/15/2021 11:30 AM, Jason Wang wrote: >>>>> >>>>> 在 2021/4/14 下午5:18, Zhu Lingshan 写道: >>>>>> This commit deduces VIRTIO device ID as device type when probe, >>>>>> then ifcvf_vdpa_get_device_id() can simply return the ID. >>>>>> ifcvf_vdpa_get_features() and ifcvf_vdpa_get_config_size() >>>>>> can work properly based on the device ID. >>>>>> >>>>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> >>>>>> --- >>>>>> drivers/vdpa/ifcvf/ifcvf_base.h | 1 + >>>>>> drivers/vdpa/ifcvf/ifcvf_main.c | 22 ++++++++++------------ >>>>>> 2 files changed, 11 insertions(+), 12 deletions(-) >>>>>> >>>>>> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h >>>>>> b/drivers/vdpa/ifcvf/ifcvf_base.h >>>>>> index b2eeb16b9c2c..1c04cd256fa7 100644 >>>>>> --- a/drivers/vdpa/ifcvf/ifcvf_base.h >>>>>> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h >>>>>> @@ -84,6 +84,7 @@ struct ifcvf_hw { >>>>>> u32 notify_off_multiplier; >>>>>> u64 req_features; >>>>>> u64 hw_features; >>>>>> + u32 dev_type; >>>>>> struct virtio_pci_common_cfg __iomem *common_cfg; >>>>>> void __iomem *net_cfg; >>>>>> struct vring_info vring[IFCVF_MAX_QUEUE_PAIRS * 2]; >>>>>> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c >>>>>> b/drivers/vdpa/ifcvf/ifcvf_main.c >>>>>> index 44d7586019da..99b0a6b4c227 100644 >>>>>> --- a/drivers/vdpa/ifcvf/ifcvf_main.c >>>>>> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c >>>>>> @@ -323,19 +323,9 @@ static u32 ifcvf_vdpa_get_generation(struct >>>>>> vdpa_device *vdpa_dev) >>>>>> static u32 ifcvf_vdpa_get_device_id(struct vdpa_device >>>>>> *vdpa_dev) >>>>>> { >>>>>> - struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev); >>>>>> - struct pci_dev *pdev = adapter->pdev; >>>>>> - u32 ret = -ENODEV; >>>>>> - >>>>>> - if (pdev->device < 0x1000 || pdev->device > 0x107f) >>>>>> - return ret; >>>>>> - >>>>>> - if (pdev->device < 0x1040) >>>>>> - ret = pdev->subsystem_device; >>>>>> - else >>>>>> - ret = pdev->device-0x1040; >>>>>> + struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); >>>>>> - return ret; >>>>>> + return vf->dev_type; >>>>>> } >>>>>> static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device >>>>>> *vdpa_dev) >>>>>> @@ -466,6 +456,14 @@ static int ifcvf_probe(struct pci_dev *pdev, >>>>>> const struct pci_device_id *id) >>>>>> pci_set_drvdata(pdev, adapter); >>>>>> vf = &adapter->vf; >>>>>> + if (pdev->device < 0x1000 || pdev->device > 0x107f) >>>>>> + return -EOPNOTSUPP; >>>>>> + >>>>>> + if (pdev->device < 0x1040) >>>>>> + vf->dev_type = pdev->subsystem_device; >>>>>> + else >>>>>> + vf->dev_type = pdev->device - 0x1040; >>>>> >>>>> >>>>> So a question here, is the device a transtional device or modern one? >>>>> >>>>> If it's a transitonal one, can it swtich endianess automatically >>>>> or not? >>>>> >>>>> Thanks >>>> Hi Jason, >>>> >>>> This driver should drive both modern and transitional devices as we >>>> discussed before. >>>> If it's a transitional one, it will act as a modern device by >>>> default, legacy mode is a fail-over path. >>> >>> >>> Note that legacy driver use native endian, support legacy driver >>> requires the device to know native endian which I'm not sure your >>> device can do that. >>> >>> Thanks >> Yes, legacy requires guest native endianess, I think we don't need to >> worry about this because our transitional device should work in >> modern mode by >> default(legacy mode is the failover path we will never reach, >> get_features will fail if no ACCESS_PLATFORM), we don't support >> legacy device in vDPA. >> >> Thanks > > > Ok, so I think it's better to add a comment here. sure, will add a comment in V2 Thanks > > Thanks > > >>> >>> >>>> For vDPA, it has to support VIRTIO_1 and ACCESS_PLATFORM, so it >>>> must in modern mode. >>>> I think we don't need to worry about endianess for legacy mode. >>>> >>>> Thanks >>>> Zhu Lingshan >>>>> >>>>> >>>>>> + >>>>>> vf->base = pcim_iomap_table(pdev); >>>>>> adapter->pdev = pdev; >>>>> >>>> >>> >> > ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/3] vDPA/ifcvf: enable Intel C5000X-PL virtio-block for vDPA 2021-04-14 9:18 [PATCH 0/3] vDPA/ifcvf: enables Intel C5000X-PL virtio-blk Zhu Lingshan 2021-04-14 9:18 ` [PATCH 1/3] vDPA/ifcvf: deduce VIRTIO device ID when probe Zhu Lingshan @ 2021-04-14 9:18 ` Zhu Lingshan 2021-04-15 3:34 ` Jason Wang 2021-04-14 9:18 ` [PATCH 3/3] vDPA/ifcvf: get_config_size should return dev specific config size Zhu Lingshan 2 siblings, 1 reply; 19+ messages in thread From: Zhu Lingshan @ 2021-04-14 9:18 UTC (permalink / raw) To: jasowang, mst, lulu, leonro Cc: virtualization, netdev, kvm, linux-kernel, Zhu Lingshan This commit enabled Intel FPGA SmartNIC C5000X-PL virtio-block for vDPA. Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> --- drivers/vdpa/ifcvf/ifcvf_base.h | 17 ++++++++++++++++- drivers/vdpa/ifcvf/ifcvf_main.c | 10 +++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index 1c04cd256fa7..8b403522bf06 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -15,6 +15,7 @@ #include <linux/pci_regs.h> #include <linux/vdpa.h> #include <uapi/linux/virtio_net.h> +#include <uapi/linux/virtio_blk.h> #include <uapi/linux/virtio_config.h> #include <uapi/linux/virtio_pci.h> @@ -28,7 +29,12 @@ #define C5000X_PL_SUBSYS_VENDOR_ID 0x8086 #define C5000X_PL_SUBSYS_DEVICE_ID 0x0001 -#define IFCVF_SUPPORTED_FEATURES \ +#define C5000X_PL_BLK_VENDOR_ID 0x1AF4 +#define C5000X_PL_BLK_DEVICE_ID 0x1001 +#define C5000X_PL_BLK_SUBSYS_VENDOR_ID 0x8086 +#define C5000X_PL_BLK_SUBSYS_DEVICE_ID 0x0002 + +#define IFCVF_NET_SUPPORTED_FEATURES \ ((1ULL << VIRTIO_NET_F_MAC) | \ (1ULL << VIRTIO_F_ANY_LAYOUT) | \ (1ULL << VIRTIO_F_VERSION_1) | \ @@ -37,6 +43,15 @@ (1ULL << VIRTIO_F_ACCESS_PLATFORM) | \ (1ULL << VIRTIO_NET_F_MRG_RXBUF)) +#define IFCVF_BLK_SUPPORTED_FEATURES \ + ((1ULL << VIRTIO_BLK_F_SIZE_MAX) | \ + (1ULL << VIRTIO_BLK_F_SEG_MAX) | \ + (1ULL << VIRTIO_BLK_F_BLK_SIZE) | \ + (1ULL << VIRTIO_BLK_F_TOPOLOGY) | \ + (1ULL << VIRTIO_BLK_F_MQ) | \ + (1ULL << VIRTIO_F_VERSION_1) | \ + (1ULL << VIRTIO_F_ACCESS_PLATFORM)) + /* Only one queue pair for now. */ #define IFCVF_MAX_QUEUE_PAIRS 1 diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 99b0a6b4c227..9b6a38b798fa 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -171,7 +171,11 @@ static u64 ifcvf_vdpa_get_features(struct vdpa_device *vdpa_dev) struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); u64 features; - features = ifcvf_get_features(vf) & IFCVF_SUPPORTED_FEATURES; + if (vf->dev_type == VIRTIO_ID_NET) + features = ifcvf_get_features(vf) & IFCVF_NET_SUPPORTED_FEATURES; + + if (vf->dev_type == VIRTIO_ID_BLOCK) + features = ifcvf_get_features(vf) & IFCVF_BLK_SUPPORTED_FEATURES; return features; } @@ -509,6 +513,10 @@ static struct pci_device_id ifcvf_pci_ids[] = { C5000X_PL_DEVICE_ID, C5000X_PL_SUBSYS_VENDOR_ID, C5000X_PL_SUBSYS_DEVICE_ID) }, + { PCI_DEVICE_SUB(C5000X_PL_BLK_VENDOR_ID, + C5000X_PL_BLK_DEVICE_ID, + C5000X_PL_BLK_SUBSYS_VENDOR_ID, + C5000X_PL_BLK_SUBSYS_DEVICE_ID) }, { 0 }, }; -- 2.27.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] vDPA/ifcvf: enable Intel C5000X-PL virtio-block for vDPA 2021-04-14 9:18 ` [PATCH 2/3] vDPA/ifcvf: enable Intel C5000X-PL virtio-block for vDPA Zhu Lingshan @ 2021-04-15 3:34 ` Jason Wang 2021-04-15 5:55 ` Zhu Lingshan 0 siblings, 1 reply; 19+ messages in thread From: Jason Wang @ 2021-04-15 3:34 UTC (permalink / raw) To: Zhu Lingshan, mst, lulu, leonro; +Cc: virtualization, netdev, kvm, linux-kernel 在 2021/4/14 下午5:18, Zhu Lingshan 写道: > This commit enabled Intel FPGA SmartNIC C5000X-PL virtio-block > for vDPA. > > Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> > --- > drivers/vdpa/ifcvf/ifcvf_base.h | 17 ++++++++++++++++- > drivers/vdpa/ifcvf/ifcvf_main.c | 10 +++++++++- > 2 files changed, 25 insertions(+), 2 deletions(-) > > diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h > index 1c04cd256fa7..8b403522bf06 100644 > --- a/drivers/vdpa/ifcvf/ifcvf_base.h > +++ b/drivers/vdpa/ifcvf/ifcvf_base.h > @@ -15,6 +15,7 @@ > #include <linux/pci_regs.h> > #include <linux/vdpa.h> > #include <uapi/linux/virtio_net.h> > +#include <uapi/linux/virtio_blk.h> > #include <uapi/linux/virtio_config.h> > #include <uapi/linux/virtio_pci.h> > > @@ -28,7 +29,12 @@ > #define C5000X_PL_SUBSYS_VENDOR_ID 0x8086 > #define C5000X_PL_SUBSYS_DEVICE_ID 0x0001 > > -#define IFCVF_SUPPORTED_FEATURES \ > +#define C5000X_PL_BLK_VENDOR_ID 0x1AF4 > +#define C5000X_PL_BLK_DEVICE_ID 0x1001 > +#define C5000X_PL_BLK_SUBSYS_VENDOR_ID 0x8086 > +#define C5000X_PL_BLK_SUBSYS_DEVICE_ID 0x0002 > + > +#define IFCVF_NET_SUPPORTED_FEATURES \ > ((1ULL << VIRTIO_NET_F_MAC) | \ > (1ULL << VIRTIO_F_ANY_LAYOUT) | \ > (1ULL << VIRTIO_F_VERSION_1) | \ > @@ -37,6 +43,15 @@ > (1ULL << VIRTIO_F_ACCESS_PLATFORM) | \ > (1ULL << VIRTIO_NET_F_MRG_RXBUF)) > > +#define IFCVF_BLK_SUPPORTED_FEATURES \ > + ((1ULL << VIRTIO_BLK_F_SIZE_MAX) | \ > + (1ULL << VIRTIO_BLK_F_SEG_MAX) | \ > + (1ULL << VIRTIO_BLK_F_BLK_SIZE) | \ > + (1ULL << VIRTIO_BLK_F_TOPOLOGY) | \ > + (1ULL << VIRTIO_BLK_F_MQ) | \ > + (1ULL << VIRTIO_F_VERSION_1) | \ > + (1ULL << VIRTIO_F_ACCESS_PLATFORM)) I think we've discussed this sometime in the past but what's the reason for such whitelist consider there's already a get_features() implemention? E.g Any reason to block VIRTIO_BLK_F_WRITE_ZEROS or VIRTIO_F_RING_PACKED? Thanks > + > /* Only one queue pair for now. */ > #define IFCVF_MAX_QUEUE_PAIRS 1 > > diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c > index 99b0a6b4c227..9b6a38b798fa 100644 > --- a/drivers/vdpa/ifcvf/ifcvf_main.c > +++ b/drivers/vdpa/ifcvf/ifcvf_main.c > @@ -171,7 +171,11 @@ static u64 ifcvf_vdpa_get_features(struct vdpa_device *vdpa_dev) > struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); > u64 features; > > - features = ifcvf_get_features(vf) & IFCVF_SUPPORTED_FEATURES; > + if (vf->dev_type == VIRTIO_ID_NET) > + features = ifcvf_get_features(vf) & IFCVF_NET_SUPPORTED_FEATURES; > + > + if (vf->dev_type == VIRTIO_ID_BLOCK) > + features = ifcvf_get_features(vf) & IFCVF_BLK_SUPPORTED_FEATURES; > > return features; > } > @@ -509,6 +513,10 @@ static struct pci_device_id ifcvf_pci_ids[] = { > C5000X_PL_DEVICE_ID, > C5000X_PL_SUBSYS_VENDOR_ID, > C5000X_PL_SUBSYS_DEVICE_ID) }, > + { PCI_DEVICE_SUB(C5000X_PL_BLK_VENDOR_ID, > + C5000X_PL_BLK_DEVICE_ID, > + C5000X_PL_BLK_SUBSYS_VENDOR_ID, > + C5000X_PL_BLK_SUBSYS_DEVICE_ID) }, > > { 0 }, > }; ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] vDPA/ifcvf: enable Intel C5000X-PL virtio-block for vDPA 2021-04-15 3:34 ` Jason Wang @ 2021-04-15 5:55 ` Zhu Lingshan 2021-04-15 6:31 ` Jason Wang 0 siblings, 1 reply; 19+ messages in thread From: Zhu Lingshan @ 2021-04-15 5:55 UTC (permalink / raw) To: Jason Wang, Zhu Lingshan, mst, lulu, leonro Cc: virtualization, netdev, kvm, linux-kernel On 4/15/2021 11:34 AM, Jason Wang wrote: > > 在 2021/4/14 下午5:18, Zhu Lingshan 写道: >> This commit enabled Intel FPGA SmartNIC C5000X-PL virtio-block >> for vDPA. >> >> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> >> --- >> drivers/vdpa/ifcvf/ifcvf_base.h | 17 ++++++++++++++++- >> drivers/vdpa/ifcvf/ifcvf_main.c | 10 +++++++++- >> 2 files changed, 25 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h >> b/drivers/vdpa/ifcvf/ifcvf_base.h >> index 1c04cd256fa7..8b403522bf06 100644 >> --- a/drivers/vdpa/ifcvf/ifcvf_base.h >> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h >> @@ -15,6 +15,7 @@ >> #include <linux/pci_regs.h> >> #include <linux/vdpa.h> >> #include <uapi/linux/virtio_net.h> >> +#include <uapi/linux/virtio_blk.h> >> #include <uapi/linux/virtio_config.h> >> #include <uapi/linux/virtio_pci.h> >> @@ -28,7 +29,12 @@ >> #define C5000X_PL_SUBSYS_VENDOR_ID 0x8086 >> #define C5000X_PL_SUBSYS_DEVICE_ID 0x0001 >> -#define IFCVF_SUPPORTED_FEATURES \ >> +#define C5000X_PL_BLK_VENDOR_ID 0x1AF4 >> +#define C5000X_PL_BLK_DEVICE_ID 0x1001 >> +#define C5000X_PL_BLK_SUBSYS_VENDOR_ID 0x8086 >> +#define C5000X_PL_BLK_SUBSYS_DEVICE_ID 0x0002 >> + >> +#define IFCVF_NET_SUPPORTED_FEATURES \ >> ((1ULL << VIRTIO_NET_F_MAC) | \ >> (1ULL << VIRTIO_F_ANY_LAYOUT) | \ >> (1ULL << VIRTIO_F_VERSION_1) | \ >> @@ -37,6 +43,15 @@ >> (1ULL << VIRTIO_F_ACCESS_PLATFORM) | \ >> (1ULL << VIRTIO_NET_F_MRG_RXBUF)) >> +#define IFCVF_BLK_SUPPORTED_FEATURES \ >> + ((1ULL << VIRTIO_BLK_F_SIZE_MAX) | \ >> + (1ULL << VIRTIO_BLK_F_SEG_MAX) | \ >> + (1ULL << VIRTIO_BLK_F_BLK_SIZE) | \ >> + (1ULL << VIRTIO_BLK_F_TOPOLOGY) | \ >> + (1ULL << VIRTIO_BLK_F_MQ) | \ >> + (1ULL << VIRTIO_F_VERSION_1) | \ >> + (1ULL << VIRTIO_F_ACCESS_PLATFORM)) > > > I think we've discussed this sometime in the past but what's the > reason for such whitelist consider there's already a get_features() > implemention? > > E.g Any reason to block VIRTIO_BLK_F_WRITE_ZEROS or VIRTIO_F_RING_PACKED? > > Thanks The reason is some feature bits are supported in the device but not supported by the driver, e.g, for virtio-net, mq & cq implementation is not ready in the driver. Thanks! > > >> + >> /* Only one queue pair for now. */ >> #define IFCVF_MAX_QUEUE_PAIRS 1 >> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c >> b/drivers/vdpa/ifcvf/ifcvf_main.c >> index 99b0a6b4c227..9b6a38b798fa 100644 >> --- a/drivers/vdpa/ifcvf/ifcvf_main.c >> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c >> @@ -171,7 +171,11 @@ static u64 ifcvf_vdpa_get_features(struct >> vdpa_device *vdpa_dev) >> struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); >> u64 features; >> - features = ifcvf_get_features(vf) & IFCVF_SUPPORTED_FEATURES; >> + if (vf->dev_type == VIRTIO_ID_NET) >> + features = ifcvf_get_features(vf) & >> IFCVF_NET_SUPPORTED_FEATURES; >> + >> + if (vf->dev_type == VIRTIO_ID_BLOCK) >> + features = ifcvf_get_features(vf) & >> IFCVF_BLK_SUPPORTED_FEATURES; >> return features; >> } >> @@ -509,6 +513,10 @@ static struct pci_device_id ifcvf_pci_ids[] = { >> C5000X_PL_DEVICE_ID, >> C5000X_PL_SUBSYS_VENDOR_ID, >> C5000X_PL_SUBSYS_DEVICE_ID) }, >> + { PCI_DEVICE_SUB(C5000X_PL_BLK_VENDOR_ID, >> + C5000X_PL_BLK_DEVICE_ID, >> + C5000X_PL_BLK_SUBSYS_VENDOR_ID, >> + C5000X_PL_BLK_SUBSYS_DEVICE_ID) }, >> { 0 }, >> }; > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] vDPA/ifcvf: enable Intel C5000X-PL virtio-block for vDPA 2021-04-15 5:55 ` Zhu Lingshan @ 2021-04-15 6:31 ` Jason Wang 2021-04-15 6:41 ` Zhu Lingshan 0 siblings, 1 reply; 19+ messages in thread From: Jason Wang @ 2021-04-15 6:31 UTC (permalink / raw) To: Zhu Lingshan, Zhu Lingshan, mst, lulu, leonro Cc: virtualization, netdev, kvm, linux-kernel 在 2021/4/15 下午1:55, Zhu Lingshan 写道: > > > On 4/15/2021 11:34 AM, Jason Wang wrote: >> >> 在 2021/4/14 下午5:18, Zhu Lingshan 写道: >>> This commit enabled Intel FPGA SmartNIC C5000X-PL virtio-block >>> for vDPA. >>> >>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> >>> --- >>> drivers/vdpa/ifcvf/ifcvf_base.h | 17 ++++++++++++++++- >>> drivers/vdpa/ifcvf/ifcvf_main.c | 10 +++++++++- >>> 2 files changed, 25 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h >>> b/drivers/vdpa/ifcvf/ifcvf_base.h >>> index 1c04cd256fa7..8b403522bf06 100644 >>> --- a/drivers/vdpa/ifcvf/ifcvf_base.h >>> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h >>> @@ -15,6 +15,7 @@ >>> #include <linux/pci_regs.h> >>> #include <linux/vdpa.h> >>> #include <uapi/linux/virtio_net.h> >>> +#include <uapi/linux/virtio_blk.h> >>> #include <uapi/linux/virtio_config.h> >>> #include <uapi/linux/virtio_pci.h> >>> @@ -28,7 +29,12 @@ >>> #define C5000X_PL_SUBSYS_VENDOR_ID 0x8086 >>> #define C5000X_PL_SUBSYS_DEVICE_ID 0x0001 >>> -#define IFCVF_SUPPORTED_FEATURES \ >>> +#define C5000X_PL_BLK_VENDOR_ID 0x1AF4 >>> +#define C5000X_PL_BLK_DEVICE_ID 0x1001 >>> +#define C5000X_PL_BLK_SUBSYS_VENDOR_ID 0x8086 >>> +#define C5000X_PL_BLK_SUBSYS_DEVICE_ID 0x0002 >>> + >>> +#define IFCVF_NET_SUPPORTED_FEATURES \ >>> ((1ULL << VIRTIO_NET_F_MAC) | \ >>> (1ULL << VIRTIO_F_ANY_LAYOUT) | \ >>> (1ULL << VIRTIO_F_VERSION_1) | \ >>> @@ -37,6 +43,15 @@ >>> (1ULL << VIRTIO_F_ACCESS_PLATFORM) | \ >>> (1ULL << VIRTIO_NET_F_MRG_RXBUF)) >>> +#define IFCVF_BLK_SUPPORTED_FEATURES \ >>> + ((1ULL << VIRTIO_BLK_F_SIZE_MAX) | \ >>> + (1ULL << VIRTIO_BLK_F_SEG_MAX) | \ >>> + (1ULL << VIRTIO_BLK_F_BLK_SIZE) | \ >>> + (1ULL << VIRTIO_BLK_F_TOPOLOGY) | \ >>> + (1ULL << VIRTIO_BLK_F_MQ) | \ >>> + (1ULL << VIRTIO_F_VERSION_1) | \ >>> + (1ULL << VIRTIO_F_ACCESS_PLATFORM)) >> >> >> I think we've discussed this sometime in the past but what's the >> reason for such whitelist consider there's already a get_features() >> implemention? >> >> E.g Any reason to block VIRTIO_BLK_F_WRITE_ZEROS or >> VIRTIO_F_RING_PACKED? >> >> Thanks > The reason is some feature bits are supported in the device but not > supported by the driver, e.g, for virtio-net, mq & cq implementation > is not ready in the driver. I understand the case of virtio-net but I wonder why we need this for block where we don't vq cvq. Thanks > > Thanks! > >> >> >>> + >>> /* Only one queue pair for now. */ >>> #define IFCVF_MAX_QUEUE_PAIRS 1 >>> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c >>> b/drivers/vdpa/ifcvf/ifcvf_main.c >>> index 99b0a6b4c227..9b6a38b798fa 100644 >>> --- a/drivers/vdpa/ifcvf/ifcvf_main.c >>> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c >>> @@ -171,7 +171,11 @@ static u64 ifcvf_vdpa_get_features(struct >>> vdpa_device *vdpa_dev) >>> struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); >>> u64 features; >>> - features = ifcvf_get_features(vf) & IFCVF_SUPPORTED_FEATURES; >>> + if (vf->dev_type == VIRTIO_ID_NET) >>> + features = ifcvf_get_features(vf) & >>> IFCVF_NET_SUPPORTED_FEATURES; >>> + >>> + if (vf->dev_type == VIRTIO_ID_BLOCK) >>> + features = ifcvf_get_features(vf) & >>> IFCVF_BLK_SUPPORTED_FEATURES; >>> return features; >>> } >>> @@ -509,6 +513,10 @@ static struct pci_device_id ifcvf_pci_ids[] = { >>> C5000X_PL_DEVICE_ID, >>> C5000X_PL_SUBSYS_VENDOR_ID, >>> C5000X_PL_SUBSYS_DEVICE_ID) }, >>> + { PCI_DEVICE_SUB(C5000X_PL_BLK_VENDOR_ID, >>> + C5000X_PL_BLK_DEVICE_ID, >>> + C5000X_PL_BLK_SUBSYS_VENDOR_ID, >>> + C5000X_PL_BLK_SUBSYS_DEVICE_ID) }, >>> { 0 }, >>> }; >> > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] vDPA/ifcvf: enable Intel C5000X-PL virtio-block for vDPA 2021-04-15 6:31 ` Jason Wang @ 2021-04-15 6:41 ` Zhu Lingshan 2021-04-15 7:17 ` Jason Wang 0 siblings, 1 reply; 19+ messages in thread From: Zhu Lingshan @ 2021-04-15 6:41 UTC (permalink / raw) To: Jason Wang, Zhu Lingshan, mst, lulu, leonro Cc: virtualization, netdev, kvm, linux-kernel On 4/15/2021 2:31 PM, Jason Wang wrote: > > 在 2021/4/15 下午1:55, Zhu Lingshan 写道: >> >> >> On 4/15/2021 11:34 AM, Jason Wang wrote: >>> >>> 在 2021/4/14 下午5:18, Zhu Lingshan 写道: >>>> This commit enabled Intel FPGA SmartNIC C5000X-PL virtio-block >>>> for vDPA. >>>> >>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> >>>> --- >>>> drivers/vdpa/ifcvf/ifcvf_base.h | 17 ++++++++++++++++- >>>> drivers/vdpa/ifcvf/ifcvf_main.c | 10 +++++++++- >>>> 2 files changed, 25 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h >>>> b/drivers/vdpa/ifcvf/ifcvf_base.h >>>> index 1c04cd256fa7..8b403522bf06 100644 >>>> --- a/drivers/vdpa/ifcvf/ifcvf_base.h >>>> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h >>>> @@ -15,6 +15,7 @@ >>>> #include <linux/pci_regs.h> >>>> #include <linux/vdpa.h> >>>> #include <uapi/linux/virtio_net.h> >>>> +#include <uapi/linux/virtio_blk.h> >>>> #include <uapi/linux/virtio_config.h> >>>> #include <uapi/linux/virtio_pci.h> >>>> @@ -28,7 +29,12 @@ >>>> #define C5000X_PL_SUBSYS_VENDOR_ID 0x8086 >>>> #define C5000X_PL_SUBSYS_DEVICE_ID 0x0001 >>>> -#define IFCVF_SUPPORTED_FEATURES \ >>>> +#define C5000X_PL_BLK_VENDOR_ID 0x1AF4 >>>> +#define C5000X_PL_BLK_DEVICE_ID 0x1001 >>>> +#define C5000X_PL_BLK_SUBSYS_VENDOR_ID 0x8086 >>>> +#define C5000X_PL_BLK_SUBSYS_DEVICE_ID 0x0002 >>>> + >>>> +#define IFCVF_NET_SUPPORTED_FEATURES \ >>>> ((1ULL << VIRTIO_NET_F_MAC) | \ >>>> (1ULL << VIRTIO_F_ANY_LAYOUT) | \ >>>> (1ULL << VIRTIO_F_VERSION_1) | \ >>>> @@ -37,6 +43,15 @@ >>>> (1ULL << VIRTIO_F_ACCESS_PLATFORM) | \ >>>> (1ULL << VIRTIO_NET_F_MRG_RXBUF)) >>>> +#define IFCVF_BLK_SUPPORTED_FEATURES \ >>>> + ((1ULL << VIRTIO_BLK_F_SIZE_MAX) | \ >>>> + (1ULL << VIRTIO_BLK_F_SEG_MAX) | \ >>>> + (1ULL << VIRTIO_BLK_F_BLK_SIZE) | \ >>>> + (1ULL << VIRTIO_BLK_F_TOPOLOGY) | \ >>>> + (1ULL << VIRTIO_BLK_F_MQ) | \ >>>> + (1ULL << VIRTIO_F_VERSION_1) | \ >>>> + (1ULL << VIRTIO_F_ACCESS_PLATFORM)) >>> >>> >>> I think we've discussed this sometime in the past but what's the >>> reason for such whitelist consider there's already a get_features() >>> implemention? >>> >>> E.g Any reason to block VIRTIO_BLK_F_WRITE_ZEROS or >>> VIRTIO_F_RING_PACKED? >>> >>> Thanks >> The reason is some feature bits are supported in the device but not >> supported by the driver, e.g, for virtio-net, mq & cq implementation >> is not ready in the driver. > > > I understand the case of virtio-net but I wonder why we need this for > block where we don't vq cvq. > > Thanks This is still a subset of the feature bits read from hardware, I leave it here to code consistently, and indicate what we support clearly. Are you suggesting remove this feature bits list and just use what we read from hardware? Thansk > > >> >> Thanks! >> >>> >>> >>>> + >>>> /* Only one queue pair for now. */ >>>> #define IFCVF_MAX_QUEUE_PAIRS 1 >>>> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c >>>> b/drivers/vdpa/ifcvf/ifcvf_main.c >>>> index 99b0a6b4c227..9b6a38b798fa 100644 >>>> --- a/drivers/vdpa/ifcvf/ifcvf_main.c >>>> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c >>>> @@ -171,7 +171,11 @@ static u64 ifcvf_vdpa_get_features(struct >>>> vdpa_device *vdpa_dev) >>>> struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); >>>> u64 features; >>>> - features = ifcvf_get_features(vf) & IFCVF_SUPPORTED_FEATURES; >>>> + if (vf->dev_type == VIRTIO_ID_NET) >>>> + features = ifcvf_get_features(vf) & >>>> IFCVF_NET_SUPPORTED_FEATURES; >>>> + >>>> + if (vf->dev_type == VIRTIO_ID_BLOCK) >>>> + features = ifcvf_get_features(vf) & >>>> IFCVF_BLK_SUPPORTED_FEATURES; >>>> return features; >>>> } >>>> @@ -509,6 +513,10 @@ static struct pci_device_id ifcvf_pci_ids[] = { >>>> C5000X_PL_DEVICE_ID, >>>> C5000X_PL_SUBSYS_VENDOR_ID, >>>> C5000X_PL_SUBSYS_DEVICE_ID) }, >>>> + { PCI_DEVICE_SUB(C5000X_PL_BLK_VENDOR_ID, >>>> + C5000X_PL_BLK_DEVICE_ID, >>>> + C5000X_PL_BLK_SUBSYS_VENDOR_ID, >>>> + C5000X_PL_BLK_SUBSYS_DEVICE_ID) }, >>>> { 0 }, >>>> }; >>> >> > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] vDPA/ifcvf: enable Intel C5000X-PL virtio-block for vDPA 2021-04-15 6:41 ` Zhu Lingshan @ 2021-04-15 7:17 ` Jason Wang 2021-04-15 7:23 ` Zhu Lingshan 0 siblings, 1 reply; 19+ messages in thread From: Jason Wang @ 2021-04-15 7:17 UTC (permalink / raw) To: Zhu Lingshan, Zhu Lingshan, mst, lulu, leonro Cc: virtualization, netdev, kvm, linux-kernel 在 2021/4/15 下午2:41, Zhu Lingshan 写道: >>>> >>>> I think we've discussed this sometime in the past but what's the >>>> reason for such whitelist consider there's already a get_features() >>>> implemention? >>>> >>>> E.g Any reason to block VIRTIO_BLK_F_WRITE_ZEROS or >>>> VIRTIO_F_RING_PACKED? >>>> >>>> Thanks >>> The reason is some feature bits are supported in the device but not >>> supported by the driver, e.g, for virtio-net, mq & cq implementation >>> is not ready in the driver. >> >> >> I understand the case of virtio-net but I wonder why we need this for >> block where we don't vq cvq. >> >> Thanks > This is still a subset of the feature bits read from hardware, I leave > it here to code consistently, and indicate what we support clearly. > Are you suggesting remove this feature bits list and just use what we > read from hardware? > > Thansk Yes, please do that. The whiltelist doesn't help in this case I think. Thanks ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] vDPA/ifcvf: enable Intel C5000X-PL virtio-block for vDPA 2021-04-15 7:17 ` Jason Wang @ 2021-04-15 7:23 ` Zhu Lingshan 0 siblings, 0 replies; 19+ messages in thread From: Zhu Lingshan @ 2021-04-15 7:23 UTC (permalink / raw) To: Jason Wang, Zhu Lingshan, mst, lulu, leonro Cc: virtualization, netdev, kvm, linux-kernel On 4/15/2021 3:17 PM, Jason Wang wrote: > > 在 2021/4/15 下午2:41, Zhu Lingshan 写道: >>>>> >>>>> I think we've discussed this sometime in the past but what's the >>>>> reason for such whitelist consider there's already a >>>>> get_features() implemention? >>>>> >>>>> E.g Any reason to block VIRTIO_BLK_F_WRITE_ZEROS or >>>>> VIRTIO_F_RING_PACKED? >>>>> >>>>> Thanks >>>> The reason is some feature bits are supported in the device but not >>>> supported by the driver, e.g, for virtio-net, mq & cq >>>> implementation is not ready in the driver. >>> >>> >>> I understand the case of virtio-net but I wonder why we need this >>> for block where we don't vq cvq. >>> >>> Thanks >> This is still a subset of the feature bits read from hardware, I >> leave it here to code consistently, and indicate what we support >> clearly. >> Are you suggesting remove this feature bits list and just use what we >> read from hardware? >> >> Thansk > > > Yes, please do that. > > The whiltelist doesn't help in this case I think. OK, will remove this in V2 Thanks > > Thanks ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3/3] vDPA/ifcvf: get_config_size should return dev specific config size 2021-04-14 9:18 [PATCH 0/3] vDPA/ifcvf: enables Intel C5000X-PL virtio-blk Zhu Lingshan 2021-04-14 9:18 ` [PATCH 1/3] vDPA/ifcvf: deduce VIRTIO device ID when probe Zhu Lingshan 2021-04-14 9:18 ` [PATCH 2/3] vDPA/ifcvf: enable Intel C5000X-PL virtio-block for vDPA Zhu Lingshan @ 2021-04-14 9:18 ` Zhu Lingshan 2021-04-15 3:36 ` Jason Wang 2021-04-15 8:12 ` Stefano Garzarella 2 siblings, 2 replies; 19+ messages in thread From: Zhu Lingshan @ 2021-04-14 9:18 UTC (permalink / raw) To: jasowang, mst, lulu, leonro Cc: virtualization, netdev, kvm, linux-kernel, Zhu Lingshan get_config_size() should return the size based on the decected device type. Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> --- drivers/vdpa/ifcvf/ifcvf_main.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 9b6a38b798fa..b48b9789b69e 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -347,7 +347,16 @@ static u32 ifcvf_vdpa_get_vq_align(struct vdpa_device *vdpa_dev) static size_t ifcvf_vdpa_get_config_size(struct vdpa_device *vdpa_dev) { - return sizeof(struct virtio_net_config); + struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); + size_t size; + + if (vf->dev_type == VIRTIO_ID_NET) + size = sizeof(struct virtio_net_config); + + if (vf->dev_type == VIRTIO_ID_BLOCK) + size = sizeof(struct virtio_blk_config); + + return size; } static void ifcvf_vdpa_get_config(struct vdpa_device *vdpa_dev, -- 2.27.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] vDPA/ifcvf: get_config_size should return dev specific config size 2021-04-14 9:18 ` [PATCH 3/3] vDPA/ifcvf: get_config_size should return dev specific config size Zhu Lingshan @ 2021-04-15 3:36 ` Jason Wang 2021-04-15 8:12 ` Stefano Garzarella 1 sibling, 0 replies; 19+ messages in thread From: Jason Wang @ 2021-04-15 3:36 UTC (permalink / raw) To: Zhu Lingshan, mst, lulu, leonro; +Cc: virtualization, netdev, kvm, linux-kernel 在 2021/4/14 下午5:18, Zhu Lingshan 写道: > get_config_size() should return the size based on the decected > device type. > > Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> Acked-by: Jason Wang <jasowang@redhat.com> > --- > drivers/vdpa/ifcvf/ifcvf_main.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c > index 9b6a38b798fa..b48b9789b69e 100644 > --- a/drivers/vdpa/ifcvf/ifcvf_main.c > +++ b/drivers/vdpa/ifcvf/ifcvf_main.c > @@ -347,7 +347,16 @@ static u32 ifcvf_vdpa_get_vq_align(struct vdpa_device *vdpa_dev) > > static size_t ifcvf_vdpa_get_config_size(struct vdpa_device *vdpa_dev) > { > - return sizeof(struct virtio_net_config); > + struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); > + size_t size; > + > + if (vf->dev_type == VIRTIO_ID_NET) > + size = sizeof(struct virtio_net_config); > + > + if (vf->dev_type == VIRTIO_ID_BLOCK) > + size = sizeof(struct virtio_blk_config); > + > + return size; > } > > static void ifcvf_vdpa_get_config(struct vdpa_device *vdpa_dev, ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] vDPA/ifcvf: get_config_size should return dev specific config size 2021-04-14 9:18 ` [PATCH 3/3] vDPA/ifcvf: get_config_size should return dev specific config size Zhu Lingshan 2021-04-15 3:36 ` Jason Wang @ 2021-04-15 8:12 ` Stefano Garzarella 2021-04-15 8:16 ` Jason Wang 1 sibling, 1 reply; 19+ messages in thread From: Stefano Garzarella @ 2021-04-15 8:12 UTC (permalink / raw) To: Zhu Lingshan Cc: jasowang, mst, lulu, leonro, virtualization, netdev, kvm, linux-kernel On Wed, Apr 14, 2021 at 05:18:32PM +0800, Zhu Lingshan wrote: >get_config_size() should return the size based on the decected >device type. > >Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> >--- > drivers/vdpa/ifcvf/ifcvf_main.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > >diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c >index 9b6a38b798fa..b48b9789b69e 100644 >--- a/drivers/vdpa/ifcvf/ifcvf_main.c >+++ b/drivers/vdpa/ifcvf/ifcvf_main.c >@@ -347,7 +347,16 @@ static u32 ifcvf_vdpa_get_vq_align(struct vdpa_device *vdpa_dev) > > static size_t ifcvf_vdpa_get_config_size(struct vdpa_device *vdpa_dev) > { >- return sizeof(struct virtio_net_config); >+ struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); >+ size_t size; >+ >+ if (vf->dev_type == VIRTIO_ID_NET) >+ size = sizeof(struct virtio_net_config); >+ >+ if (vf->dev_type == VIRTIO_ID_BLOCK) >+ size = sizeof(struct virtio_blk_config); >+ >+ return size; I'm not familiar with the ifcvf details, but can it happen that the device is not block or net? Should we set `size` to 0 by default to handle this case or are we sure it's one of the two? Maybe we should add a comment or a warning message in this case, to prevent some analysis tool or compiler from worrying that `size` might be uninitialized. I was thinking something like this: switch(vf->dev_type) { case VIRTIO_ID_NET: size = sizeof(struct virtio_net_config); break; case VIRTIO_ID_BLOCK: size = sizeof(struct virtio_blk_config); break; default: /* or WARN(1, "") if dev_warn() not apply */ dev_warn(... , "virtio ID [0x%x] not supported\n") size = 0; } Thanks, Stefano ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] vDPA/ifcvf: get_config_size should return dev specific config size 2021-04-15 8:12 ` Stefano Garzarella @ 2021-04-15 8:16 ` Jason Wang 0 siblings, 0 replies; 19+ messages in thread From: Jason Wang @ 2021-04-15 8:16 UTC (permalink / raw) To: Stefano Garzarella, Zhu Lingshan Cc: mst, lulu, leonro, virtualization, netdev, kvm, linux-kernel 在 2021/4/15 下午4:12, Stefano Garzarella 写道: > On Wed, Apr 14, 2021 at 05:18:32PM +0800, Zhu Lingshan wrote: >> get_config_size() should return the size based on the decected >> device type. >> >> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> >> --- >> drivers/vdpa/ifcvf/ifcvf_main.c | 11 ++++++++++- >> 1 file changed, 10 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c >> b/drivers/vdpa/ifcvf/ifcvf_main.c >> index 9b6a38b798fa..b48b9789b69e 100644 >> --- a/drivers/vdpa/ifcvf/ifcvf_main.c >> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c >> @@ -347,7 +347,16 @@ static u32 ifcvf_vdpa_get_vq_align(struct >> vdpa_device *vdpa_dev) >> >> static size_t ifcvf_vdpa_get_config_size(struct vdpa_device *vdpa_dev) >> { >> - return sizeof(struct virtio_net_config); >> + struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); >> + size_t size; >> + >> + if (vf->dev_type == VIRTIO_ID_NET) >> + size = sizeof(struct virtio_net_config); >> + >> + if (vf->dev_type == VIRTIO_ID_BLOCK) >> + size = sizeof(struct virtio_blk_config); >> + >> + return size; > > I'm not familiar with the ifcvf details, but can it happen that the > device is not block or net? > > Should we set `size` to 0 by default to handle this case or are we > sure it's one of the two? > > Maybe we should add a comment or a warning message in this case, to > prevent some analysis tool or compiler from worrying that `size` might > be uninitialized. > > I was thinking something like this: > > switch(vf->dev_type) { > case VIRTIO_ID_NET: > size = sizeof(struct virtio_net_config); > break; > case VIRTIO_ID_BLOCK: > size = sizeof(struct virtio_blk_config); > break; > default: > /* or WARN(1, "") if dev_warn() not apply */ > dev_warn(... , "virtio ID [0x%x] not supported\n") > size = 0; > > } > Yes, I agree. Thanks > Thanks, > Stefano > ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2021-04-15 8:16 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-14 9:18 [PATCH 0/3] vDPA/ifcvf: enables Intel C5000X-PL virtio-blk Zhu Lingshan 2021-04-14 9:18 ` [PATCH 1/3] vDPA/ifcvf: deduce VIRTIO device ID when probe Zhu Lingshan 2021-04-15 3:30 ` Jason Wang 2021-04-15 5:52 ` Zhu Lingshan 2021-04-15 6:30 ` Jason Wang 2021-04-15 6:36 ` Zhu Lingshan 2021-04-15 7:16 ` Jason Wang 2021-04-15 7:23 ` Zhu Lingshan 2021-04-14 9:18 ` [PATCH 2/3] vDPA/ifcvf: enable Intel C5000X-PL virtio-block for vDPA Zhu Lingshan 2021-04-15 3:34 ` Jason Wang 2021-04-15 5:55 ` Zhu Lingshan 2021-04-15 6:31 ` Jason Wang 2021-04-15 6:41 ` Zhu Lingshan 2021-04-15 7:17 ` Jason Wang 2021-04-15 7:23 ` Zhu Lingshan 2021-04-14 9:18 ` [PATCH 3/3] vDPA/ifcvf: get_config_size should return dev specific config size Zhu Lingshan 2021-04-15 3:36 ` Jason Wang 2021-04-15 8:12 ` Stefano Garzarella 2021-04-15 8:16 ` Jason Wang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).