* Re: [Intel-wired-lan] [PATCH net-next v3] idpf: add support for IDPF PCI programming interface
[not found] <20250829172453.2059973-1-madhu.chittim@intel.com>
@ 2025-08-29 22:07 ` Paul Menzel
2025-09-02 18:10 ` Linga, Pavan Kumar
0 siblings, 1 reply; 2+ messages in thread
From: Paul Menzel @ 2025-08-29 22:07 UTC (permalink / raw)
To: Madhu Chittim; +Cc: intel-wired-lan, netdev, horms, linux-pci
[Cc: +linux-pci@vger.kernel.org]
Dear Madhu, dear Pavan,
Thank you for the patch.
Am 29.08.25 um 19:24 schrieb Madhu Chittim:
> From: Pavan Kumar Linga <pavan.kumar.linga@intel.com>
>
> At present IDPF supports only 0x1452 and 0x145C as PF and VF device IDs
> on our current generation hardware. Future hardware exposes a new set of
> device IDs for each generation. To avoid adding a new device ID for each
> generation and to make the driver forward and backward compatible,
> make use of the IDPF PCI programming interface to load the driver.
>
> Write and read the VF_ARQBAL mailbox register to find if the current
> device is a PF or a VF.
>
> PCI SIG allocated a new programming interface for the IDPF compliant
> ethernet network controller devices. It can be found at:
> https://members.pcisig.com/wg/PCI-SIG/document/20113
> with the document titled as 'PCI Code and ID Assignment Revision 1.16'
> or any latest revisions.
Could you please add some information, how you tested this?
> Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> Signed-off-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com>
> Signed-off-by: Madhu Chittim <madhu.chittim@intel.com>
>
> ---
> v3:
> - reworked logic to avoid gotos
>
> v2:
> - replace *u8 with *bool in idpf_is_vf_device function parameter
> - use ~0 instead of 0xffffff in PCI_DEVICE_CLASS parameter
>
> ---
>
> Signed-off-by: Madhu Chittim <madhu.chittim@intel.com>
This looks like a stray line, but will probably be ignored, when applied.
> ---
> drivers/net/ethernet/intel/idpf/idpf.h | 1 +
> drivers/net/ethernet/intel/idpf/idpf_main.c | 73 ++++++++++++++-----
> drivers/net/ethernet/intel/idpf/idpf_vf_dev.c | 37 ++++++++++
> 3 files changed, 94 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/idpf/idpf.h b/drivers/net/ethernet/intel/idpf/idpf.h
> index c56abf8b4c92..4a16e481faf7 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf.h
> +++ b/drivers/net/ethernet/intel/idpf/idpf.h
> @@ -1041,6 +1041,7 @@ void idpf_mbx_task(struct work_struct *work);
> void idpf_vc_event_task(struct work_struct *work);
> void idpf_dev_ops_init(struct idpf_adapter *adapter);
> void idpf_vf_dev_ops_init(struct idpf_adapter *adapter);
> +int idpf_is_vf_device(struct pci_dev *pdev, bool *is_vf);
> int idpf_intr_req(struct idpf_adapter *adapter);
> void idpf_intr_rel(struct idpf_adapter *adapter);
> u16 idpf_get_max_tx_hdr_size(struct idpf_adapter *adapter);
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_main.c b/drivers/net/ethernet/intel/idpf/idpf_main.c
> index 8c46481d2e1f..493604d50143 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_main.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_main.c
> @@ -7,11 +7,57 @@
>
> #define DRV_SUMMARY "Intel(R) Infrastructure Data Path Function Linux Driver"
>
> +#define IDPF_NETWORK_ETHERNET_PROGIF 0x01
> +#define IDPF_CLASS_NETWORK_ETHERNET_PROGIF \
> + (PCI_CLASS_NETWORK_ETHERNET << 8 | IDPF_NETWORK_ETHERNET_PROGIF)
> +
> MODULE_DESCRIPTION(DRV_SUMMARY);
> MODULE_IMPORT_NS("LIBETH");
> MODULE_IMPORT_NS("LIBETH_XDP");
> MODULE_LICENSE("GPL");
>
> +/**
> + * idpf_dev_init - Initialize device specific parameters
> + * @adapter: adapter to initialize
> + * @ent: entry in idpf_pci_tbl
> + *
> + * Return: %0 on success, -%errno on failure.
> + */
> +static int idpf_dev_init(struct idpf_adapter *adapter,
> + const struct pci_device_id *ent)
> +{
> + bool is_vf = false;
> + int err;
> +
> + if (ent->class == IDPF_CLASS_NETWORK_ETHERNET_PROGIF) {
> + err = idpf_is_vf_device(adapter->pdev, &is_vf);
> + if (err)
> + return err;
> + if (is_vf) {
> + idpf_vf_dev_ops_init(adapter);
> + adapter->crc_enable = true;
> + } else {
> + idpf_dev_ops_init(adapter);
> + }
> +
> + return 0;
> + }
> +
> + switch (ent->device) {
> + case IDPF_DEV_ID_PF:
> + idpf_dev_ops_init(adapter);
> + break;
> + case IDPF_DEV_ID_VF:
> + idpf_vf_dev_ops_init(adapter);
> + adapter->crc_enable = true;
> + break;
> + default:
> + return -ENODEV;
> + }
> +
> + return 0;
> +}
> +
> /**
> * idpf_remove - Device removal routine
> * @pdev: PCI device information struct
> @@ -165,21 +211,6 @@ static int idpf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> adapter->req_tx_splitq = true;
> adapter->req_rx_splitq = true;
>
> - switch (ent->device) {
> - case IDPF_DEV_ID_PF:
> - idpf_dev_ops_init(adapter);
> - break;
> - case IDPF_DEV_ID_VF:
> - idpf_vf_dev_ops_init(adapter);
> - adapter->crc_enable = true;
> - break;
> - default:
> - err = -ENODEV;
> - dev_err(&pdev->dev, "Unexpected dev ID 0x%x in idpf probe\n",
> - ent->device);
> - goto err_free;
> - }
> -
> adapter->pdev = pdev;
> err = pcim_enable_device(pdev);
> if (err)
> @@ -259,11 +290,18 @@ static int idpf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> /* setup msglvl */
> adapter->msg_enable = netif_msg_init(-1, IDPF_AVAIL_NETIF_M);
>
> + err = idpf_dev_init(adapter, ent);
> + if (err) {
> + dev_err(&pdev->dev, "Unexpected dev ID 0x%x in idpf probe\n",
> + ent->device);
> + goto destroy_vc_event_wq;
> + }
> +
> err = idpf_cfg_hw(adapter);
> if (err) {
> dev_err(dev, "Failed to configure HW structure for adapter: %d\n",
> err);
> - goto err_cfg_hw;
> + goto destroy_vc_event_wq;
> }
>
> mutex_init(&adapter->vport_ctrl_lock);
> @@ -284,7 +322,7 @@ static int idpf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>
> return 0;
>
> -err_cfg_hw:
> +destroy_vc_event_wq:
> destroy_workqueue(adapter->vc_event_wq);
> err_vc_event_wq_alloc:
> destroy_workqueue(adapter->stats_wq);
> @@ -304,6 +342,7 @@ static int idpf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> static const struct pci_device_id idpf_pci_tbl[] = {
> { PCI_VDEVICE(INTEL, IDPF_DEV_ID_PF)},
> { PCI_VDEVICE(INTEL, IDPF_DEV_ID_VF)},
> + { PCI_DEVICE_CLASS(IDPF_CLASS_NETWORK_ETHERNET_PROGIF, ~0)},
> { /* Sentinel */ }
> };
> MODULE_DEVICE_TABLE(pci, idpf_pci_tbl);
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_vf_dev.c b/drivers/net/ethernet/intel/idpf/idpf_vf_dev.c
> index 7527b967e2e7..09cccdf45b50 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_vf_dev.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_vf_dev.c
> @@ -7,6 +7,43 @@
>
> #define IDPF_VF_ITR_IDX_SPACING 0x40
>
> +#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 pci_dev *pdev, bool *is_vf)
> +{
> + struct resource mbx_region;
> + resource_size_t mbx_start;
> + void __iomem *mbx_addr;
> + long len;
Use size_t?
include/linux/ioport.h:static inline resource_size_t
resource_size(const struct resource *res)
> +
> + resource_set_range(&mbx_region, VF_BASE, IDPF_VF_MBX_REGION_SZ);
> +
> + mbx_start = pci_resource_start(pdev, 0) + mbx_region.start;
> + len = resource_size(&mbx_region);
> +
> + mbx_addr = ioremap(mbx_start, len);
> + if (!mbx_addr)
> + return -EIO;
Should some kind of error be printed with a hint, what the user could do?
> +
> + writel(IDPF_VF_TEST_VAL, mbx_addr + VF_ARQBAL - VF_BASE);
> +
> + /* Force memory write to complete before reading it back */
> + wmb();
> +
> + *is_vf = readl(mbx_addr + VF_ARQBAL - VF_BASE) == IDPF_VF_TEST_VAL;
> +
> + iounmap(mbx_addr);
> +
> + return 0;
> +}
> +
> /**
> * idpf_vf_ctlq_reg_init - initialize default mailbox registers
> * @adapter: adapter structure
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v3] idpf: add support for IDPF PCI programming interface
2025-08-29 22:07 ` [Intel-wired-lan] [PATCH net-next v3] idpf: add support for IDPF PCI programming interface Paul Menzel
@ 2025-09-02 18:10 ` Linga, Pavan Kumar
0 siblings, 0 replies; 2+ messages in thread
From: Linga, Pavan Kumar @ 2025-09-02 18:10 UTC (permalink / raw)
To: Paul Menzel, Madhu Chittim; +Cc: intel-wired-lan, netdev, horms, linux-pci
On 8/29/2025 3:07 PM, Paul Menzel wrote:
> [Cc: +linux-pci@vger.kernel.org]
>
> Dear Madhu, dear Pavan,
>
>
> Thank you for the patch.
>
>
> Am 29.08.25 um 19:24 schrieb Madhu Chittim:
>> From: Pavan Kumar Linga <pavan.kumar.linga@intel.com>
>>
>> At present IDPF supports only 0x1452 and 0x145C as PF and VF device IDs
>> on our current generation hardware. Future hardware exposes a new set of
>> device IDs for each generation. To avoid adding a new device ID for each
>> generation and to make the driver forward and backward compatible,
>> make use of the IDPF PCI programming interface to load the driver.
>>
>> Write and read the VF_ARQBAL mailbox register to find if the current
>> device is a PF or a VF.
>>
>> PCI SIG allocated a new programming interface for the IDPF compliant
>> ethernet network controller devices. It can be found at:
>> https://members.pcisig.com/wg/PCI-SIG/document/20113
>> with the document titled as 'PCI Code and ID Assignment Revision 1.16'
>> or any latest revisions.
>
> Could you please add some information, how you tested this?
>
Thanks for the review.
I verified the changes in this patch by loading the driver on the
existing hardware which supports the 0x1452 and 0x145C device IDs and
new hardware which supports IDPF PCI programming interface.
Sure, I will add this testing info to the commit message.
>> Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
>> Signed-off-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com>
>> Signed-off-by: Madhu Chittim <madhu.chittim@intel.com>
>>
>> ---
>> v3:
>> - reworked logic to avoid gotos
>>
>> v2:
>> - replace *u8 with *bool in idpf_is_vf_device function parameter
>> - use ~0 instead of 0xffffff in PCI_DEVICE_CLASS parameter
>>
>> ---
>>
>> Signed-off-by: Madhu Chittim <madhu.chittim@intel.com>
>
> This looks like a stray line, but will probably be ignored, when applied.
>
Will fix it.
>> ---
>> drivers/net/ethernet/intel/idpf/idpf.h | 1 +
>> drivers/net/ethernet/intel/idpf/idpf_main.c | 73 ++++++++++++++-----
>> drivers/net/ethernet/intel/idpf/idpf_vf_dev.c | 37 ++++++++++
>> 3 files changed, 94 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/idpf/idpf.h b/drivers/net/
>> ethernet/intel/idpf/idpf.h
>> index c56abf8b4c92..4a16e481faf7 100644
>> --- a/drivers/net/ethernet/intel/idpf/idpf.h
>> +++ b/drivers/net/ethernet/intel/idpf/idpf.h
>> @@ -1041,6 +1041,7 @@ void idpf_mbx_task(struct work_struct *work);
>> void idpf_vc_event_task(struct work_struct *work);
>> void idpf_dev_ops_init(struct idpf_adapter *adapter);
>> void idpf_vf_dev_ops_init(struct idpf_adapter *adapter);
>> +int idpf_is_vf_device(struct pci_dev *pdev, bool *is_vf);
>> int idpf_intr_req(struct idpf_adapter *adapter);
>> void idpf_intr_rel(struct idpf_adapter *adapter);
>> u16 idpf_get_max_tx_hdr_size(struct idpf_adapter *adapter);
>> diff --git a/drivers/net/ethernet/intel/idpf/idpf_main.c b/drivers/
>> net/ethernet/intel/idpf/idpf_main.c
>> index 8c46481d2e1f..493604d50143 100644
>> --- a/drivers/net/ethernet/intel/idpf/idpf_main.c
>> +++ b/drivers/net/ethernet/intel/idpf/idpf_main.c
>> @@ -7,11 +7,57 @@
>> #define DRV_SUMMARY "Intel(R) Infrastructure Data Path Function
>> Linux Driver"
>> +#define IDPF_NETWORK_ETHERNET_PROGIF 0x01
>> +#define IDPF_CLASS_NETWORK_ETHERNET_PROGIF \
>> + (PCI_CLASS_NETWORK_ETHERNET << 8 | IDPF_NETWORK_ETHERNET_PROGIF)
>> +
>> MODULE_DESCRIPTION(DRV_SUMMARY);
>> MODULE_IMPORT_NS("LIBETH");
>> MODULE_IMPORT_NS("LIBETH_XDP");
>> MODULE_LICENSE("GPL");
>> +/**
>> + * idpf_dev_init - Initialize device specific parameters
>> + * @adapter: adapter to initialize
>> + * @ent: entry in idpf_pci_tbl
>> + *
>> + * Return: %0 on success, -%errno on failure.
>> + */
>> +static int idpf_dev_init(struct idpf_adapter *adapter,
>> + const struct pci_device_id *ent)
>> +{
>> + bool is_vf = false;
>> + int err;
>> +
>> + if (ent->class == IDPF_CLASS_NETWORK_ETHERNET_PROGIF) {
>> + err = idpf_is_vf_device(adapter->pdev, &is_vf);
>> + if (err)
>> + return err;
>> + if (is_vf) {
>> + idpf_vf_dev_ops_init(adapter);
>> + adapter->crc_enable = true;
>> + } else {
>> + idpf_dev_ops_init(adapter);
>> + }
>> +
>> + return 0;
>> + }
>> +
>> + switch (ent->device) {
>> + case IDPF_DEV_ID_PF:
>> + idpf_dev_ops_init(adapter);
>> + break;
>> + case IDPF_DEV_ID_VF:
>> + idpf_vf_dev_ops_init(adapter);
>> + adapter->crc_enable = true;
>> + break;
>> + default:
>> + return -ENODEV;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> /**
>> * idpf_remove - Device removal routine
>> * @pdev: PCI device information struct
>> @@ -165,21 +211,6 @@ static int idpf_probe(struct pci_dev *pdev, const
>> struct pci_device_id *ent)
>> adapter->req_tx_splitq = true;
>> adapter->req_rx_splitq = true;
>> - switch (ent->device) {
>> - case IDPF_DEV_ID_PF:
>> - idpf_dev_ops_init(adapter);
>> - break;
>> - case IDPF_DEV_ID_VF:
>> - idpf_vf_dev_ops_init(adapter);
>> - adapter->crc_enable = true;
>> - break;
>> - default:
>> - err = -ENODEV;
>> - dev_err(&pdev->dev, "Unexpected dev ID 0x%x in idpf probe\n",
>> - ent->device);
>> - goto err_free;
>> - }
>> -
>> adapter->pdev = pdev;
>> err = pcim_enable_device(pdev);
>> if (err)
>> @@ -259,11 +290,18 @@ static int idpf_probe(struct pci_dev *pdev,
>> const struct pci_device_id *ent)
>> /* setup msglvl */
>> adapter->msg_enable = netif_msg_init(-1, IDPF_AVAIL_NETIF_M);
>> + err = idpf_dev_init(adapter, ent);
>> + if (err) {
>> + dev_err(&pdev->dev, "Unexpected dev ID 0x%x in idpf probe\n",
>> + ent->device);
>> + goto destroy_vc_event_wq;
>> + }
>> +
>> err = idpf_cfg_hw(adapter);
>> if (err) {
>> dev_err(dev, "Failed to configure HW structure for adapter:
>> %d\n",
>> err);
>> - goto err_cfg_hw;
>> + goto destroy_vc_event_wq;
>> }
>> mutex_init(&adapter->vport_ctrl_lock);
>> @@ -284,7 +322,7 @@ static int idpf_probe(struct pci_dev *pdev, const
>> struct pci_device_id *ent)
>> return 0;
>> -err_cfg_hw:
>> +destroy_vc_event_wq:
>> destroy_workqueue(adapter->vc_event_wq);
>> err_vc_event_wq_alloc:
>> destroy_workqueue(adapter->stats_wq);
>> @@ -304,6 +342,7 @@ static int idpf_probe(struct pci_dev *pdev, const
>> struct pci_device_id *ent)
>> static const struct pci_device_id idpf_pci_tbl[] = {
>> { PCI_VDEVICE(INTEL, IDPF_DEV_ID_PF)},
>> { PCI_VDEVICE(INTEL, IDPF_DEV_ID_VF)},
>> + { PCI_DEVICE_CLASS(IDPF_CLASS_NETWORK_ETHERNET_PROGIF, ~0)},
>> { /* Sentinel */ }
>> };
>> MODULE_DEVICE_TABLE(pci, idpf_pci_tbl);
>> diff --git a/drivers/net/ethernet/intel/idpf/idpf_vf_dev.c b/drivers/
>> net/ethernet/intel/idpf/idpf_vf_dev.c
>> index 7527b967e2e7..09cccdf45b50 100644
>> --- a/drivers/net/ethernet/intel/idpf/idpf_vf_dev.c
>> +++ b/drivers/net/ethernet/intel/idpf/idpf_vf_dev.c
>> @@ -7,6 +7,43 @@
>> #define IDPF_VF_ITR_IDX_SPACING 0x40
>> +#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 pci_dev *pdev, bool *is_vf)
>> +{
>> + struct resource mbx_region;
>> + resource_size_t mbx_start;
>> + void __iomem *mbx_addr;
>> + long len;
>
> Use size_t?
>
> include/linux/ioport.h:static inline resource_size_t
> resource_size(const struct resource *res)
>
>
Will fix it.
>> +
>> + resource_set_range(&mbx_region, VF_BASE, IDPF_VF_MBX_REGION_SZ);
>> +
>> + mbx_start = pci_resource_start(pdev, 0) + mbx_region.start;
>> + len = resource_size(&mbx_region);
>> +
>> + mbx_addr = ioremap(mbx_start, len);
>> + if (!mbx_addr)
>> + return -EIO;
>
> Should some kind of error be printed with a hint, what the user could do?
>
Sure, will add a print here.
>> +
>> + writel(IDPF_VF_TEST_VAL, mbx_addr + VF_ARQBAL - VF_BASE);
>> +
>> + /* Force memory write to complete before reading it back */
>> + wmb();
>> +
>> + *is_vf = readl(mbx_addr + VF_ARQBAL - VF_BASE) == IDPF_VF_TEST_VAL;
>> +
>> + iounmap(mbx_addr);
>> +
>> + return 0;
>> +}
>> +
>> /**
>> * idpf_vf_ctlq_reg_init - initialize default mailbox registers
>> * @adapter: adapter structure
>
Thanks,
Pavan
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-09-02 18:10 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20250829172453.2059973-1-madhu.chittim@intel.com>
2025-08-29 22:07 ` [Intel-wired-lan] [PATCH net-next v3] idpf: add support for IDPF PCI programming interface Paul Menzel
2025-09-02 18:10 ` Linga, Pavan Kumar
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).