Re: [PATCH V4 1/7] vDPA/ifcvf: get_vendor_id returns a device specific vendor id

Re: [PATCH V4 1/7] vDPA/ifcvf: get_vendor_id returns a device specific vendor id

[Jason Wang]

在 2021/3/15 下午3:44, Zhu Lingshan 写道:
> In this commit, ifcvf_get_vendor_id() will return
> a device specific vendor id of the probed pci device
> than a hard code.
>
> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>


Acked-by: Jason Wang <jasowang@redhat.com>


> ---
>   drivers/vdpa/ifcvf/ifcvf_main.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
> index fa1af301cf55..e501ee07de17 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_main.c
> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
> @@ -324,7 +324,10 @@ static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev)
>   
>   static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev)
>   {
> -	return IFCVF_SUBSYS_VENDOR_ID;
> +	struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev);
> +	struct pci_dev *pdev = adapter->pdev;
> +
> +	return pdev->subsystem_vendor;
>   }
>   
>   static u32 ifcvf_vdpa_get_vq_align(struct vdpa_device *vdpa_dev)

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH V4 2/7] vDPA/ifcvf: enable Intel C5000X-PL virtio-net for vDPA

[Jason Wang]

在 2021/3/15 下午3:44, Zhu Lingshan 写道:
> This commit enabled Intel FPGA SmartNIC C5000X-PL virtio-net
> for vDPA
>
> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>


Acked-by: Jason Wang <jasowang@redhat.com>


> ---
>   drivers/vdpa/ifcvf/ifcvf_base.h | 5 +++++
>   drivers/vdpa/ifcvf/ifcvf_main.c | 5 +++++
>   2 files changed, 10 insertions(+)
>
> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h
> index 64696d63fe07..75d9a8052039 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_base.h
> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h
> @@ -23,6 +23,11 @@
>   #define IFCVF_SUBSYS_VENDOR_ID	0x8086
>   #define IFCVF_SUBSYS_DEVICE_ID	0x001A
>   
> +#define C5000X_PL_VENDOR_ID		0x1AF4
> +#define C5000X_PL_DEVICE_ID		0x1000
> +#define C5000X_PL_SUBSYS_VENDOR_ID	0x8086
> +#define C5000X_PL_SUBSYS_DEVICE_ID	0x0001
> +
>   #define IFCVF_SUPPORTED_FEATURES \
>   		((1ULL << VIRTIO_NET_F_MAC)			| \
>   		 (1ULL << VIRTIO_F_ANY_LAYOUT)			| \
> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
> index e501ee07de17..26a2dab7ca66 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_main.c
> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
> @@ -484,6 +484,11 @@ static struct pci_device_id ifcvf_pci_ids[] = {
>   		IFCVF_DEVICE_ID,
>   		IFCVF_SUBSYS_VENDOR_ID,
>   		IFCVF_SUBSYS_DEVICE_ID) },
> +	{ PCI_DEVICE_SUB(C5000X_PL_VENDOR_ID,
> +			 C5000X_PL_DEVICE_ID,
> +			 C5000X_PL_SUBSYS_VENDOR_ID,
> +			 C5000X_PL_SUBSYS_DEVICE_ID) },
> +
>   	{ 0 },
>   };
>   MODULE_DEVICE_TABLE(pci, ifcvf_pci_ids);

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH V4 3/7] vDPA/ifcvf: rename original IFCVF dev ids to N3000 ids

[Jason Wang]

在 2021/3/15 下午3:44, Zhu Lingshan 写道:
> IFCVF driver probes multiple types of devices now,
> to distinguish the original device driven by IFCVF
> from others, it is renamed as "N3000".
>
> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>


Acked-by: Jason Wang <jasowang@redhat.com>

If you want to have a general driver, you probaby need to rename the driver.

Thanks


> ---
>   drivers/vdpa/ifcvf/ifcvf_base.h | 8 ++++----
>   drivers/vdpa/ifcvf/ifcvf_main.c | 8 ++++----
>   2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h
> index 75d9a8052039..794d1505d857 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_base.h
> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h
> @@ -18,10 +18,10 @@
>   #include <uapi/linux/virtio_config.h>
>   #include <uapi/linux/virtio_pci.h>
>   
> -#define IFCVF_VENDOR_ID		0x1AF4
> -#define IFCVF_DEVICE_ID		0x1041
> -#define IFCVF_SUBSYS_VENDOR_ID	0x8086
> -#define IFCVF_SUBSYS_DEVICE_ID	0x001A
> +#define N3000_VENDOR_ID		0x1AF4
> +#define N3000_DEVICE_ID		0x1041
> +#define N3000_SUBSYS_VENDOR_ID	0x8086
> +#define N3000_SUBSYS_DEVICE_ID	0x001A
>   
>   #define C5000X_PL_VENDOR_ID		0x1AF4
>   #define C5000X_PL_DEVICE_ID		0x1000
> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
> index 26a2dab7ca66..fd5befc5cbcc 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_main.c
> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
> @@ -480,10 +480,10 @@ static void ifcvf_remove(struct pci_dev *pdev)
>   }
>   
>   static struct pci_device_id ifcvf_pci_ids[] = {
> -	{ PCI_DEVICE_SUB(IFCVF_VENDOR_ID,
> -		IFCVF_DEVICE_ID,
> -		IFCVF_SUBSYS_VENDOR_ID,
> -		IFCVF_SUBSYS_DEVICE_ID) },
> +	{ PCI_DEVICE_SUB(N3000_VENDOR_ID,
> +			 N3000_DEVICE_ID,
> +			 N3000_SUBSYS_VENDOR_ID,
> +			 N3000_SUBSYS_DEVICE_ID) },
>   	{ PCI_DEVICE_SUB(C5000X_PL_VENDOR_ID,
>   			 C5000X_PL_DEVICE_ID,
>   			 C5000X_PL_SUBSYS_VENDOR_ID,

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH V4 4/7] vDPA/ifcvf: remove the version number string

[Jason Wang]

在 2021/3/15 下午3:44, Zhu Lingshan 写道:
> This commit removes the version number string, using kernel
> version is enough.
>
> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
> Reviewed-by: Leon Romanovsky <leonro@nvidia.com>


Acked-by: Jason Wang <jasowang@redhat.com>


> ---
>   drivers/vdpa/ifcvf/ifcvf_main.c | 2 --
>   1 file changed, 2 deletions(-)
>
> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
> index fd5befc5cbcc..c34e1eec6b6c 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_main.c
> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
> @@ -14,7 +14,6 @@
>   #include <linux/sysfs.h>
>   #include "ifcvf_base.h"
>   
> -#define VERSION_STRING  "0.1"
>   #define DRIVER_AUTHOR   "Intel Corporation"
>   #define IFCVF_DRIVER_NAME       "ifcvf"
>   
> @@ -503,4 +502,3 @@ static struct pci_driver ifcvf_driver = {
>   module_pci_driver(ifcvf_driver);
>   
>   MODULE_LICENSE("GPL v2");
> -MODULE_VERSION(VERSION_STRING);

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH V4 5/7] vDPA/ifcvf: fetch device feature bits when probe

[Jason Wang]

在 2021/3/15 下午3:44, Zhu Lingshan 写道:
> This commit would read and store device feature
> bits when probe.
>
> rename ifcvf_get_features() to ifcvf_get_hw_features(),
> it reads and stores features of the probed device.
>
> new ifcvf_get_features() simply returns stored
> feature bits.
>
> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>


Acked-by: Jason Wang <jasowang@redhat.com>


> ---
>   drivers/vdpa/ifcvf/ifcvf_base.c | 12 ++++++++++--
>   drivers/vdpa/ifcvf/ifcvf_base.h |  2 ++
>   drivers/vdpa/ifcvf/ifcvf_main.c |  2 ++
>   3 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c
> index f2a128e56de5..ea6a78791c9b 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_base.c
> +++ b/drivers/vdpa/ifcvf/ifcvf_base.c
> @@ -202,10 +202,11 @@ static void ifcvf_add_status(struct ifcvf_hw *hw, u8 status)
>   	ifcvf_get_status(hw);
>   }
>   
> -u64 ifcvf_get_features(struct ifcvf_hw *hw)
> +u64 ifcvf_get_hw_features(struct ifcvf_hw *hw)
>   {
>   	struct virtio_pci_common_cfg __iomem *cfg = hw->common_cfg;
>   	u32 features_lo, features_hi;
> +	u64 features;
>   
>   	ifc_iowrite32(0, &cfg->device_feature_select);
>   	features_lo = ifc_ioread32(&cfg->device_feature);
> @@ -213,7 +214,14 @@ u64 ifcvf_get_features(struct ifcvf_hw *hw)
>   	ifc_iowrite32(1, &cfg->device_feature_select);
>   	features_hi = ifc_ioread32(&cfg->device_feature);
>   
> -	return ((u64)features_hi << 32) | features_lo;
> +	features = ((u64)features_hi << 32) | features_lo;
> +
> +	return features;
> +}
> +
> +u64 ifcvf_get_features(struct ifcvf_hw *hw)
> +{
> +	return hw->hw_features;
>   }
>   
>   void ifcvf_read_net_config(struct ifcvf_hw *hw, u64 offset,
> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h
> index 794d1505d857..dbb8c10aa3b1 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_base.h
> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h
> @@ -83,6 +83,7 @@ struct ifcvf_hw {
>   	void __iomem *notify_base;
>   	u32 notify_off_multiplier;
>   	u64 req_features;
> +	u64 hw_features;
>   	struct virtio_pci_common_cfg __iomem *common_cfg;
>   	void __iomem *net_cfg;
>   	struct vring_info vring[IFCVF_MAX_QUEUE_PAIRS * 2];
> @@ -121,6 +122,7 @@ void ifcvf_set_status(struct ifcvf_hw *hw, u8 status);
>   void io_write64_twopart(u64 val, u32 *lo, u32 *hi);
>   void ifcvf_reset(struct ifcvf_hw *hw);
>   u64 ifcvf_get_features(struct ifcvf_hw *hw);
> +u64 ifcvf_get_hw_features(struct ifcvf_hw *hw);
>   u16 ifcvf_get_vq_state(struct ifcvf_hw *hw, u16 qid);
>   int ifcvf_set_vq_state(struct ifcvf_hw *hw, u16 qid, u16 num);
>   struct ifcvf_adapter *vf_to_adapter(struct ifcvf_hw *hw);
> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
> index c34e1eec6b6c..25fb9dfe23f0 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_main.c
> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
> @@ -458,6 +458,8 @@ static int ifcvf_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   	for (i = 0; i < IFCVF_MAX_QUEUE_PAIRS * 2; i++)
>   		vf->vring[i].irq = -EINVAL;
>   
> +	vf->hw_features = ifcvf_get_hw_features(vf);
> +
>   	ret = vdpa_register_device(&adapter->vdpa);
>   	if (ret) {
>   		IFCVF_ERR(pdev, "Failed to register ifcvf to vdpa bus");

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH V4 6/7] vDPA/ifcvf: verify mandatory feature bits for vDPA

[Jason Wang]

在 2021/3/15 下午3:45, Zhu Lingshan 写道:
> vDPA requres VIRTIO_F_ACCESS_PLATFORM as a must, this commit
> examines this when set features.
>
> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
> ---
>   drivers/vdpa/ifcvf/ifcvf_base.c | 12 ++++++++++++
>   drivers/vdpa/ifcvf/ifcvf_base.h |  1 +
>   drivers/vdpa/ifcvf/ifcvf_main.c |  5 +++++
>   3 files changed, 18 insertions(+)
>
> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c
> index ea6a78791c9b..4f257c4b2f76 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_base.c
> +++ b/drivers/vdpa/ifcvf/ifcvf_base.c
> @@ -224,6 +224,18 @@ u64 ifcvf_get_features(struct ifcvf_hw *hw)
>   	return hw->hw_features;
>   }
>   
> +int ifcvf_verify_min_features(struct ifcvf_hw *hw, u64 features)
> +{
> +	struct ifcvf_adapter *ifcvf = vf_to_adapter(hw);
> +
> +	if (!(features & BIT_ULL(VIRTIO_F_ACCESS_PLATFORM)) && features) {
> +		IFCVF_ERR(ifcvf->pdev, "VIRTIO_F_ACCESS_PLATFORM not negotiated\n");


Should be "is not negotiated".

Otherwise:

Acked-by: Jason Wang <jasowang@redhat.com>


> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>   void ifcvf_read_net_config(struct ifcvf_hw *hw, u64 offset,
>   			   void *dst, int length)
>   {
> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h
> index dbb8c10aa3b1..f77239fc1644 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_base.h
> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h
> @@ -123,6 +123,7 @@ void io_write64_twopart(u64 val, u32 *lo, u32 *hi);
>   void ifcvf_reset(struct ifcvf_hw *hw);
>   u64 ifcvf_get_features(struct ifcvf_hw *hw);
>   u64 ifcvf_get_hw_features(struct ifcvf_hw *hw);
> +int ifcvf_verify_min_features(struct ifcvf_hw *hw, u64 features);
>   u16 ifcvf_get_vq_state(struct ifcvf_hw *hw, u16 qid);
>   int ifcvf_set_vq_state(struct ifcvf_hw *hw, u16 qid, u16 num);
>   struct ifcvf_adapter *vf_to_adapter(struct ifcvf_hw *hw);
> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
> index 25fb9dfe23f0..ea93ea7fd5df 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_main.c
> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
> @@ -179,6 +179,11 @@ static u64 ifcvf_vdpa_get_features(struct vdpa_device *vdpa_dev)
>   static int ifcvf_vdpa_set_features(struct vdpa_device *vdpa_dev, u64 features)
>   {
>   	struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev);
> +	int ret;
> +
> +	ret = ifcvf_verify_min_features(vf, features);
> +	if (ret)
> +		return ret;
>   
>   	vf->req_features = features;
>   

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH V4 7/7] vDPA/ifcvf: deduce VIRTIO device ID from pdev ids

[Jason Wang]

在 2021/3/15 下午3:45, Zhu Lingshan 写道:
>   static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev)
>   {
> -	return VIRTIO_ID_NET;
> +	struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev);
> +	u32 ret = -EOPNOTSUPP;
> +
> +	if (ifcvf_probed_virtio_net(vf))
> +		ret = VIRTIO_ID_NET;


So the point is to simplify the future extension.

How about simply?

if (device_id>0x1040)
     return devce_id - 0x1040;
else
     return device_id;

Since I don't think you plan to introduce device whose vendor id is not 
1AF4 and the subsys vendor/device id is not interesting to vDPA bus.

Thanks


> +
> +	return ret;
>   }

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization