[PATCH] net/gve: remove gve-specific PCI revision ID macros

[PATCH] net/gve: remove gve-specific PCI revision ID macros

[Joshua Washington]

Commit 5feee68de99e ("pci: define more standard register offsets") adds
a generic offset macro for the PCI revision ID, among a number of other
standard PCI config fields. Update GVE to make use of these macros
instead of defining its own.

Signed-off-by: Joshua Washington <joshwash@google.com>
---
 drivers/net/gve/base/gve.h        |  2 --
 drivers/net/gve/base/gve_adminq.c | 21 +++++++++------------
 2 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/net/gve/base/gve.h b/drivers/net/gve/base/gve.h
index 0c5f5e9ce4..99514cb5f9 100644
--- a/drivers/net/gve/base/gve.h
+++ b/drivers/net/gve/base/gve.h
@@ -14,8 +14,6 @@
 #endif
 
 #define GVE_DEV_ID		0x0042
-#define GVE_PCI_REV_OFFSET	0x8
-#define GVE_PCI_REV_SIZE	1
 
 #define GVE_REG_BAR		0
 #define GVE_DB_BAR		2
diff --git a/drivers/net/gve/base/gve_adminq.c b/drivers/net/gve/base/gve_adminq.c
index 2c5cfa2aa1..25f4481c1b 100644
--- a/drivers/net/gve/base/gve_adminq.c
+++ b/drivers/net/gve/base/gve_adminq.c
@@ -196,18 +196,10 @@ gve_process_device_options(struct gve_priv *priv,
 	return 0;
 }
 
-static uint8_t
-gve_get_pci_revision_id(struct gve_priv *priv)
-{
-	uint8_t rev_id;
-
-	rte_pci_read_config(priv->pci_dev, &rev_id, GVE_PCI_REV_SIZE,
-			    GVE_PCI_REV_OFFSET);
-	return rev_id;
-}
-
 int gve_adminq_alloc(struct gve_priv *priv)
 {
+	uint8_t pci_rev_id;
+
 	priv->adminq = gve_alloc_dma_mem(&priv->adminq_dma_mem, PAGE_SIZE);
 	if (unlikely(!priv->adminq))
 		return -ENOMEM;
@@ -231,7 +223,9 @@ int gve_adminq_alloc(struct gve_priv *priv)
 	priv->adminq_get_ptype_map_cnt = 0;
 
 	/* Setup Admin queue with the device */
-	if (gve_get_pci_revision_id(priv) < 0x1) { /* Use AQ PFN. */
+	rte_pci_read_config(priv->pci_dev, &pci_rev_id, sizeof(pci_rev_id),
+			    RTE_PCI_REVISION_ID);
+	if (pci_rev_id < 0x1) { /* Use AQ PFN. */
 		iowrite32be(priv->adminq_dma_mem.pa / PAGE_SIZE,
 			    &priv->reg_bar0->adminq_pfn);
 	} else { /* Use full AQ address. */
@@ -251,10 +245,13 @@ int gve_adminq_alloc(struct gve_priv *priv)
 
 void gve_adminq_release(struct gve_priv *priv)
 {
+	uint8_t pci_rev_id;
 	int i = 0;
 
 	/* Tell the device the adminq is leaving */
-	if (gve_get_pci_revision_id(priv) < 0x1) {
+	rte_pci_read_config(priv->pci_dev, &pci_rev_id, sizeof(pci_rev_id),
+			    RTE_PCI_REVISION_ID);
+	if (pci_rev_id < 0x1) {
 		iowrite32be(0x0, &priv->reg_bar0->adminq_pfn);
 		while (ioread32be(&priv->reg_bar0->adminq_pfn)) {
 			/* If this is reached the device is unrecoverable and still
-- 
2.49.0.1045.g170613ef41-goog

Re: [PATCH] net/gve: remove gve-specific PCI revision ID macros

[Joshua Washington]

Patch doesn't apply to the main branch of dpdk-next-net:
https://mails.dpdk.org/archives/test-report/2025-May/877522.html

It seems like the diffbase patch only exists in the for-main branch.
When is for-main expected to be merged into main, and should we be
building against for-main or main? Similarly, what should this
compilation test build against?

Thanks,
Josh

Re: [PATCH] net/gve: remove gve-specific PCI revision ID macros

[David Marchand]

Hello,

Thanks for the cleanup.

On Wed, May 14, 2025 at 2:46 AM Joshua Washington <joshwash@google.com> wrote:
>
> Patch doesn't apply to the main branch of dpdk-next-net:
> https://mails.dpdk.org/archives/test-report/2025-May/877522.html
>
> It seems like the diffbase patch only exists in the for-main branch.
> When is for-main expected to be merged into main, and should we be
> building against for-main or main? Similarly, what should this
> compilation test build against?

Stephen is out for some time (until next week?).

Thomas may pull next-net soon.
(@Thomas it would be great if this additional change was squashed in
96a1290b6258 ("net/gve: support for gVNIC PCI revision 1 and later")
while pulling.)
But otherwise this cleanup can be merged after.

In any case, for net drivers update, please continue basing your work
on next-net.

Thanks.

-- 
David Marchand