Re: [dpdk-dev] [PATCH 2/8] app/testpmd: use PCI memory resource access APIs

Hi,

-----Original Message-----
From: Li, Xiaoyun <redacted>
Sent: Thursday, September 16, 2021 2:10 PM
To: Xia, Chenbo <redacted>; dev@dpdk.org
Subject: RE: [PATCH 2/8] app/testpmd: use PCI memory resource access APIs

Hi

quoted

-----Original Message-----
From: Xia, Chenbo <redacted>
Sent: Friday, September 10, 2021 10:24
To: dev@dpdk.org
Cc: Li, Xiaoyun <redacted>
Subject: [PATCH 2/8] app/testpmd: use PCI memory resource access APIs

Currently testpmd uses struct rte_pci_device to access PCI memory resource.
Since this structure will be internal later, this patch replaces use of
rte_pci_device with new PCI memory resource access APIs to read/write BAR 0.

Signed-off-by: Chenbo Xia <redacted>
---
 app/test-pmd/config.c  | 38 +++++++++++-----------------------
 app/test-pmd/testpmd.h | 46 +++++++++++++++++++++---------------------
 2 files changed, 35 insertions(+), 49 deletions(-)

<snip>

quoted

@@ -1081,7 +1063,9 @@ port_reg_bit_set(portid_t port_id, uint32_t reg_off,

uint8_t bit_pos,
 			(int) bit_v);
 		return;
 	}
-	reg_v = port_id_pci_reg_read(port_id, reg_off);
+	if (port_id_pci_reg_read(port_id, reg_off, &reg_v))
+		return;
+
 	if (bit_v == 0)
 		reg_v &= ~(1 << bit_pos);
 	else

@@ -1123,7 +1107,9 @@ port_reg_bit_field_set(portid_t port_id, uint32_t

reg_off,
 				(unsigned)max_v, (unsigned)max_v);
 		return;
 	}
-	reg_v = port_id_pci_reg_read(port_id, reg_off);
+	if (port_id_pci_reg_read(port_id, reg_off, &reg_v))
+		return;
+
 	reg_v &= ~(max_v << l_bit); /* Keep unchanged bits */
 	reg_v |= (value << l_bit); /* Set changed bits */
 	port_id_pci_reg_write(port_id, reg_off, reg_v); diff --git a/app/test-

In your implementation, port_id_pci_reg_write() can fail. Then the following
display shouldn't be called.
So change this function to have return value and check it before use it will
be better.

Make sense. Will fix in v2.

Thanks,
Chenbo

quoted

pmd/testpmd.h b/app/test-pmd/testpmd.h index 16a3598e48..7922807a6e
100644

--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h

<snip>

quoted

 static inline void
-port_pci_reg_write(struct rte_port *port, uint32_t reg_off, uint32_t reg_v)
+port_id_pci_reg_write(portid_t pt_id, uint32_t reg_off, uint32_t reg_v)
 {
-	const struct rte_pci_device *pci_dev;
+	struct rte_port *port = &ports[(pt_id)];
+	char name[RTE_ETH_NAME_MAX_LEN];
 	const struct rte_bus *bus;
-	void *reg_addr;

 	if (!port->dev_info.device) {
 		fprintf(stderr, "Invalid device\n");

@@ -721,19 +721,19 @@ port_pci_reg_write(struct rte_port *port, uint32_t

reg_off, uint32_t reg_v)

 	bus = rte_bus_find_by_device(port->dev_info.device);
 	if (bus && !strcmp(bus->name, "pci")) {
-		pci_dev = RTE_DEV_TO_PCI(port->dev_info.device);
+		rte_eth_dev_get_name_by_port(pt_id, name);
 	} else {
 		fprintf(stderr, "Not a PCI device\n");
 		return;
 	}

-	reg_addr = ((char *)pci_dev->mem_resource[0].addr + reg_off);
-	*((volatile uint32_t *)reg_addr) = rte_cpu_to_le_32(reg_v);
+	reg_v = rte_cpu_to_le_32(reg_v);
+	if (rte_pci_mem_wr32(name, 0, &reg_v, reg_off)) {
+		fprintf(stderr, "Failed to write register\n");
+		return;
+	}
 }

-#define port_id_pci_reg_write(pt_id, reg_off, reg_value) \
-	port_pci_reg_write(&ports[(pt_id)], (reg_off), (reg_value))
-
 static inline void
 get_start_cycles(uint64_t *start_tsc)
 {
--
2.17.1