[PATCH v2] net/intel: add IDPF PCI class ID support
Vemula, Venkatesh
venkatesh.vemula at intel.com
Thu Sep 25 12:28:53 CEST 2025
Thanks, Bruce, for all your comments ,
Noted, will address in V3 patch,
Thanks,
Venkatesh
-----Original Message-----
From: Richardson, Bruce <bruce.richardson at intel.com>
Sent: 24 September 2025 20:05
To: Vemula, Venkatesh <venkatesh.vemula at intel.com>
Cc: dev at dpdk.org; Singh, Aman Deep <aman.deep.singh at intel.com>; Wani, Shaiq <shaiq.wani at intel.com>; Marchand, David <david.marchand at redhat.com>
Subject: Re: [PATCH v2] net/intel: add IDPF PCI class ID support
On Wed, Sep 24, 2025 at 06:27:49PM +0530, Vemula Venkatesh wrote:
> Current IDPF supports only the MEV device IDs. MMG has new set of
> device IDs and same might be the case for the future devices. Instead
> of adding new device IDs every time, make use of the IDPF PCI class
> ID(0x20001) to differentiate between PF and VF.
>
> Write and read the VF_ARQBAL register to find if the current device is
> a PF or a VF.
>
> v2: addressed reviewers comments.
>
> Signed-off-by: Vemula Venkatesh <venkatesh.vemula at intel.com>
> ---
> drivers/net/intel/cpfl/cpfl_ethdev.c | 3 +-
> drivers/net/intel/cpfl/cpfl_ethdev.h | 3 +-
> .../net/intel/idpf/base/idpf_controlq_api.h | 1 +
> drivers/net/intel/idpf/idpf_common_device.c | 37 +++++++++++++++++--
> drivers/net/intel/idpf/idpf_common_device.h | 17 +++++++++
> drivers/net/intel/idpf/idpf_ethdev.c | 2 +
> 6 files changed, 57 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/intel/cpfl/cpfl_ethdev.c
> b/drivers/net/intel/cpfl/cpfl_ethdev.c
> index 6d7b23ad7a..8cec977ecf 100644
> --- a/drivers/net/intel/cpfl/cpfl_ethdev.c
> +++ b/drivers/net/intel/cpfl/cpfl_ethdev.c
> @@ -2606,7 +2606,8 @@ cpfl_dev_vport_init(struct rte_eth_dev *dev,
> void *init_params) }
>
> static const struct rte_pci_id pci_id_cpfl_map[] = {
> - { RTE_PCI_DEVICE(IDPF_INTEL_VENDOR_ID, IDPF_DEV_ID_CPF) },
> + { RTE_PCI_DEVICE(IDPF_INTEL_VENDOR_ID, CPFL_DEV_ID_MEV) },
> + { RTE_PCI_DEVICE(IDPF_INTEL_VENDOR_ID, CPFL_DEV_ID_MMG) },
> { .vendor_id = 0, /* sentinel */ },
> };
>
> diff --git a/drivers/net/intel/cpfl/cpfl_ethdev.h
> b/drivers/net/intel/cpfl/cpfl_ethdev.h
> index d4e1176ab1..566b395dae 100644
> --- a/drivers/net/intel/cpfl/cpfl_ethdev.h
> +++ b/drivers/net/intel/cpfl/cpfl_ethdev.h
> @@ -59,7 +59,8 @@
> #define CPFL_ALARM_INTERVAL 50000 /* us */
>
> /* Device IDs */
> -#define IDPF_DEV_ID_CPF 0x1453
> +#define CPFL_DEV_ID_MMG 0x11E0
> +#define CPFL_DEV_ID_MEV 0x1453
> #define VIRTCHNL2_QUEUE_GROUP_P2P 0x100
>
> #define CPFL_HOST_ID_NUM 2
> diff --git a/drivers/net/intel/idpf/base/idpf_controlq_api.h
> b/drivers/net/intel/idpf/base/idpf_controlq_api.h
> index 8a90258099..7c25aabb28 100644
> --- a/drivers/net/intel/idpf/base/idpf_controlq_api.h
> +++ b/drivers/net/intel/idpf/base/idpf_controlq_api.h
> @@ -184,6 +184,7 @@ struct idpf_hw {
> u16 subsystem_device_id;
> u16 subsystem_vendor_id;
> u8 revision_id;
> + uint32_t cls_id;
> bool adapter_stopped;
>
This is a poor location to add a new 32-bit field, right between two 8-bit fields. Adding it here will likely cause more padding than necessary to be added. Add the new field alongside other 32-bit fields to minimize padding.
> LIST_HEAD_TYPE(list_head, idpf_ctlq_info) cq_list_head; diff --git
> a/drivers/net/intel/idpf/idpf_common_device.c
> b/drivers/net/intel/idpf/idpf_common_device.c
> index ff1fbcd2b4..6f083770d8 100644
> --- a/drivers/net/intel/idpf/idpf_common_device.c
> +++ b/drivers/net/intel/idpf/idpf_common_device.c
> @@ -125,12 +125,12 @@ struct idpf_ctlq_create_info
> vf_ctlq_info[IDPF_CTLQ_NUM] = { };
>
> static int
> -idpf_init_mbx(struct idpf_hw *hw)
> +idpf_init_mbx(struct idpf_hw *hw, bool is_vf)
> {
> struct idpf_ctlq_info *ctlq;
> int ret = 0;
>
> - if (hw->device_id == IDPF_DEV_ID_SRIOV)
> + if (is_vf == 1)
> ret = idpf_ctlq_init(hw, IDPF_CTLQ_NUM, vf_ctlq_info);
> else
> ret = idpf_ctlq_init(hw, IDPF_CTLQ_NUM, pf_ctlq_info); @@ -388,8
> +388,21 @@ idpf_adapter_init(struct idpf_adapter *adapter) {
> struct idpf_hw *hw = &adapter->hw;
> int ret;
> + int err;
> + bool is_vf = 0;
>
> - if (hw->device_id == IDPF_DEV_ID_SRIOV) {
> + switch (hw->device_id) {
> + case IDPF_DEV_ID_SRIOV:
> + is_vf = 1;
> + break;
> + default:
> + if (hw->cls_id == IDPF_CLASS_NETWORK_ETHERNET_PROGIF) {
> + err = idpf_is_vf_device(hw, &is_vf);
> + if (err)
> + return err;
> + }
> + }
There is no need for a switch statement here, I think. Simplify the code by just adding a "(hw->device_id == IDPF_DEV_ID_SRIOV)" check at the start of idpf_is_vf_device function, and use that in all cases.
> + if (is_vf) {
> ret = idpf_check_vf_reset_done(hw);
> } else {
> idpf_reset_pf(hw);
> @@ -400,7 +413,7 @@ idpf_adapter_init(struct idpf_adapter *adapter)
> goto err_check_reset;
> }
>
> - ret = idpf_init_mbx(hw);
> + ret = idpf_init_mbx(hw, is_vf);
> if (ret != 0) {
> DRV_LOG(ERR, "Failed to init mailbox");
> goto err_check_reset;
> @@ -443,6 +456,22 @@ idpf_adapter_init(struct idpf_adapter *adapter)
> return ret;
> }
>
> +#define IDPF_VF_TEST_VAL 0xFEED0000
> +
> +/**
> + * idpf_is_vf_device - Helper to find if it is a VF device
> + * @pdev: PCI device information struct
> + * @is_vf: used to update VF device status
> + *
> + * Return: 0 on success, errno on failure.
> + */
> +int idpf_is_vf_device(struct idpf_hw *hw, bool *is_vf) {
> + IDPF_WRITE_REG(hw, VF_ARQBAL, IDPF_VF_TEST_VAL);
> + *is_vf = (IDPF_READ_REG(hw, VF_ARQBAL) == IDPF_VF_TEST_VAL);
> + return 0;
> +}
> +
Why pass in is_vf as parameter and return int here, given that there is no possible error. I would rewrite as:
bool idpf_is_vf_device(struct idpf_hw *hw) {
if (hw->device_id == IDPF_DEV_ID_SRIOV)
return 1;
IDPF_WRITE_REG(hw, VF_ARQBAL, IDPF_VF_TEST_VAL);
return IDPF_READ_REG(hw, VF_ARQBAL) == IDPF_VF_TEST_VAL; }
While only one line longer, it removes the need for the switch statement above and all the error handling for a non-zero result which can never happen.
> RTE_EXPORT_INTERNAL_SYMBOL(idpf_adapter_deinit)
> int
> idpf_adapter_deinit(struct idpf_adapter *adapter) diff --git
> a/drivers/net/intel/idpf/idpf_common_device.h
> b/drivers/net/intel/idpf/idpf_common_device.h
> index 5f3e4a4fcf..51da8214cb 100644
> --- a/drivers/net/intel/idpf/idpf_common_device.h
> +++ b/drivers/net/intel/idpf/idpf_common_device.h
> @@ -44,6 +44,23 @@
> (sizeof(struct virtchnl2_ptype) + \
> (((p)->proto_id_count ? ((p)->proto_id_count - 1) : 0) *
> sizeof((p)->proto_id[0])))
>
> +/** Macro used to help building up tables of device IDs with PCI class */
> +#define IDPF_PCI_CLASS(cls) \
> + .class_id = (cls), \
> + .vendor_id = RTE_PCI_ANY_ID, \
> + .device_id = RTE_PCI_ANY_ID, \
> + .subsystem_vendor_id = RTE_PCI_ANY_ID, \
> + .subsystem_device_id = RTE_PCI_ANY_ID
> +
> +
> +/* PCI Class network ethernet */
> +#define PCI_CLASS_NETWORK_ETHERNET 0x0200
> +#define IDPF_NETWORK_ETHERNET_PROGIF 0x01
> +#define IDPF_CLASS_NETWORK_ETHERNET_PROGIF \
> + (PCI_CLASS_NETWORK_ETHERNET << 8 | IDPF_NETWORK_ETHERNET_PROGIF)
> +
So the device does not identify itself as an ethernet device because it has it's class id set as the ethernet-class shifted by 8?
> +int idpf_is_vf_device(struct idpf_hw *hw, bool *is_vf);
> +
> struct idpf_adapter {
> struct idpf_hw hw;
> struct virtchnl2_version_info virtchnl_version; diff --git
> a/drivers/net/intel/idpf/idpf_ethdev.c
> b/drivers/net/intel/idpf/idpf_ethdev.c
> index 90720909bf..80bf53db0f 100644
> --- a/drivers/net/intel/idpf/idpf_ethdev.c
> +++ b/drivers/net/intel/idpf/idpf_ethdev.c
> @@ -1201,6 +1201,7 @@ idpf_adapter_ext_init(struct rte_pci_device *pci_dev, struct idpf_adapter_ext *a
> hw->vendor_id = pci_dev->id.vendor_id;
> hw->device_id = pci_dev->id.device_id;
> hw->subsystem_vendor_id = pci_dev->id.subsystem_vendor_id;
> + hw->cls_id = pci_dev->id.class_id;
>
> strncpy(adapter->name, pci_dev->device.name, PCI_PRI_STR_SIZE);
>
> @@ -1313,6 +1314,7 @@ idpf_dev_vport_init(struct rte_eth_dev *dev,
> void *init_params) static const struct rte_pci_id pci_id_idpf_map[] = {
> { RTE_PCI_DEVICE(IDPF_INTEL_VENDOR_ID, IDPF_DEV_ID_PF) },
> { RTE_PCI_DEVICE(IDPF_INTEL_VENDOR_ID, IDPF_DEV_ID_SRIOV) },
> + { IDPF_PCI_CLASS(IDPF_CLASS_NETWORK_ETHERNET_PROGIF) },
> { .vendor_id = 0, /* sentinel */ },
> };
>
> --
> 2.34.1
>
More information about the dev
mailing list