* [dpdk-dev] [PATCH v4 2/2] examples/ethtool: use rte_eth_dev_get_reg_info for reg params
2016-06-23 13:26 [dpdk-dev] [PATCH v4 1/2] ethdev: remove get_reg_length callback zr
@ 2016-06-23 13:26 ` zr
2016-06-27 10:46 ` Remy Horton
2016-06-27 10:46 ` [dpdk-dev] [PATCH v4 1/2] ethdev: remove get_reg_length callback Remy Horton
` (3 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: zr @ 2016-06-23 13:26 UTC (permalink / raw)
To: remy.horton, thomas.monjalon, wenzhuo.lu, helin.zhang,
konstantin.ananyev, jingjing.wu
Cc: dev, Zyta Szpak
From: Zyta Szpak <zr@semihalf.com>
Version 4 of fixing the fixed register width assumption.
The app was allocating too little space for 64-bit registers
which resulted in memory corruption. This commit resolves
this by getting the number of registers and size of register
by rte_eth_dev_get_reg_info function called first time
with data=NULL.
Signed-off-by: Zyta Szpak <zr@semihalf.com>
---
examples/ethtool/lib/rte_ethtool.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/examples/ethtool/lib/rte_ethtool.c b/examples/ethtool/lib/rte_ethtool.c
index 54391f2..a1f91d4 100644
--- a/examples/ethtool/lib/rte_ethtool.c
+++ b/examples/ethtool/lib/rte_ethtool.c
@@ -46,6 +46,7 @@ int
rte_ethtool_get_drvinfo(uint8_t port_id, struct ethtool_drvinfo *drvinfo)
{
struct rte_eth_dev_info dev_info;
+ struct rte_dev_reg_info reg_info;
int n;
if (drvinfo == NULL)
@@ -65,7 +66,9 @@ rte_ethtool_get_drvinfo(uint8_t port_id, struct ethtool_drvinfo *drvinfo)
dev_info.pci_dev->addr.domain, dev_info.pci_dev->addr.bus,
dev_info.pci_dev->addr.devid, dev_info.pci_dev->addr.function);
- n = rte_eth_dev_get_reg_length(port_id);
+ memset(®_info, 0, sizeof(reg_info));
+ rte_eth_dev_get_reg_info(port_id, ®_info);
+ n = reg_info.length;
if (n > 0)
drvinfo->regdump_len = n;
else
@@ -86,12 +89,16 @@ rte_ethtool_get_drvinfo(uint8_t port_id, struct ethtool_drvinfo *drvinfo)
int
rte_ethtool_get_regs_len(uint8_t port_id)
{
- int count_regs;
+ struct rte_dev_reg_info reg_info;
+ int ret;
+
+ memset(®_info, 0, sizeof(reg_info));
+
+ ret = rte_eth_dev_get_reg_info(port_id, ®_info);
+ if (ret)
+ return ret;
- count_regs = rte_eth_dev_get_reg_length(port_id);
- if (count_regs > 0)
- return count_regs * sizeof(uint32_t);
- return count_regs;
+ return reg_info.length * reg_info.width;
}
int
--
1.9.1
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [dpdk-dev] [PATCH v4 2/2] examples/ethtool: use rte_eth_dev_get_reg_info for reg params
2016-06-23 13:26 ` [dpdk-dev] [PATCH v4 2/2] examples/ethtool: use rte_eth_dev_get_reg_info for reg params zr
@ 2016-06-27 10:46 ` Remy Horton
0 siblings, 0 replies; 17+ messages in thread
From: Remy Horton @ 2016-06-27 10:46 UTC (permalink / raw)
To: zr, thomas.monjalon, wenzhuo.lu, helin.zhang, konstantin.ananyev,
jingjing.wu
Cc: dev
On 23/06/2016 14:26, zr@semihalf.com wrote:
> From: Zyta Szpak <zr@semihalf.com>
>
> Version 4 of fixing the fixed register width assumption.
> The app was allocating too little space for 64-bit registers
> which resulted in memory corruption. This commit resolves
> this by getting the number of registers and size of register
> by rte_eth_dev_get_reg_info function called first time
> with data=NULL.
>
> Signed-off-by: Zyta Szpak <zr@semihalf.com>
Acked-by: Remy Horton <remy.horton@intel.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH v4 1/2] ethdev: remove get_reg_length callback
2016-06-23 13:26 [dpdk-dev] [PATCH v4 1/2] ethdev: remove get_reg_length callback zr
2016-06-23 13:26 ` [dpdk-dev] [PATCH v4 2/2] examples/ethtool: use rte_eth_dev_get_reg_info for reg params zr
@ 2016-06-27 10:46 ` Remy Horton
2016-06-28 16:05 ` Zyta Szpak
2016-06-27 15:19 ` Thomas Monjalon
` (2 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Remy Horton @ 2016-06-27 10:46 UTC (permalink / raw)
To: zr, thomas.monjalon, wenzhuo.lu, helin.zhang, konstantin.ananyev,
jingjing.wu
Cc: dev
Morning,
On 23/06/2016 14:26, zr@semihalf.com wrote:
> From: Zyta Szpak <zr@semihalf.com>
>
> Version 4 of fixing the assumption of that device registers
> are always 32 bits long. rte_eth_dev_get_reg_length and
> rte_eth_dev_get_reg_info callbacks did not provide register size
> to the app in any way. It is needed to allocate proper number
> of bytes before retrieving registers content with
> rte_eth_dev_get_reg. This commit remove rte_eth_dev_get_reg_length
> callback and adds width parameter to reg_info struct which makes
> it possible to call rte_eth_dev_get_reg_info to get attributes
> first. The drivers using this callback fill width and length
> when call to function made with data=NULL.
I think this would read better as a commit message:
Removes hard-coded assumption that device registers are always 32 bits
wide. The rte_eth_dev_get_reg_length and rte_eth_dev_get_reg_info
callbacks did not provide register size to the app in any way, which is
needed to allocate correct number of bytes before retrieving registers
using rte_eth_dev_get_reg.
This commit changes rte_eth_dev_get_reg_info so that it can be used to
retrieve both the number of registers and their width, and removes the
now-redundant rte_eth_dev_get_reg_length.
> -/**
> - * Retrieve device registers and register attributes
> + * Retrieve device registers and register attributes (nb of regs and reg size)
> *
> * @param port_id
> * The port identifier of the Ethernet device.
Need detail regarding how *info->data affects function behaviour.
Something along the lines of:
/**
* Retrieve device registers and register attributes (number of
* registers and register size)
*
* @param port_id
* The port identifier of the Ethernet device.
* @param info
* Pointer to rte_dev_reg_info structure to fill in. If info->data is
* NULL the function fills in the width and length fields. If non-NULL
* the registers are put into the buffer pointed at by the data field.
Program code itself looks good to me.
..Remy
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [dpdk-dev] [PATCH v4 1/2] ethdev: remove get_reg_length callback
2016-06-27 10:46 ` [dpdk-dev] [PATCH v4 1/2] ethdev: remove get_reg_length callback Remy Horton
@ 2016-06-28 16:05 ` Zyta Szpak
0 siblings, 0 replies; 17+ messages in thread
From: Zyta Szpak @ 2016-06-28 16:05 UTC (permalink / raw)
To: Remy Horton, thomas.monjalon, wenzhuo.lu, helin.zhang,
konstantin.ananyev, jingjing.wu
Cc: dev
OK
On 27.06.2016 12:46, Remy Horton wrote:
> Morning,
>
>
> On 23/06/2016 14:26, zr@semihalf.com wrote:
>> From: Zyta Szpak <zr@semihalf.com>
>>
>> Version 4 of fixing the assumption of that device registers
>> are always 32 bits long. rte_eth_dev_get_reg_length and
>> rte_eth_dev_get_reg_info callbacks did not provide register size
>> to the app in any way. It is needed to allocate proper number
>> of bytes before retrieving registers content with
>> rte_eth_dev_get_reg. This commit remove rte_eth_dev_get_reg_length
>> callback and adds width parameter to reg_info struct which makes
>> it possible to call rte_eth_dev_get_reg_info to get attributes
>> first. The drivers using this callback fill width and length
>> when call to function made with data=NULL.
>
> I think this would read better as a commit message:
>
> Removes hard-coded assumption that device registers are always 32 bits
> wide. The rte_eth_dev_get_reg_length and rte_eth_dev_get_reg_info
> callbacks did not provide register size to the app in any way, which
> is needed to allocate correct number of bytes before retrieving
> registers using rte_eth_dev_get_reg.
>
> This commit changes rte_eth_dev_get_reg_info so that it can be used to
> retrieve both the number of registers and their width, and removes the
> now-redundant rte_eth_dev_get_reg_length.
>
>
>> -/**
>> - * Retrieve device registers and register attributes
>> + * Retrieve device registers and register attributes (nb of regs and
>> reg size)
>> *
>> * @param port_id
>> * The port identifier of the Ethernet device.
>
> Need detail regarding how *info->data affects function behaviour.
> Something along the lines of:
>
> /**
> * Retrieve device registers and register attributes (number of
> * registers and register size)
> *
> * @param port_id
> * The port identifier of the Ethernet device.
> * @param info
> * Pointer to rte_dev_reg_info structure to fill in. If info->data is
> * NULL the function fills in the width and length fields. If non-NULL
> * the registers are put into the buffer pointed at by the data field.
>
> Program code itself looks good to me.
>
> ..Remy
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH v4 1/2] ethdev: remove get_reg_length callback
2016-06-23 13:26 [dpdk-dev] [PATCH v4 1/2] ethdev: remove get_reg_length callback zr
2016-06-23 13:26 ` [dpdk-dev] [PATCH v4 2/2] examples/ethtool: use rte_eth_dev_get_reg_info for reg params zr
2016-06-27 10:46 ` [dpdk-dev] [PATCH v4 1/2] ethdev: remove get_reg_length callback Remy Horton
@ 2016-06-27 15:19 ` Thomas Monjalon
2016-06-28 16:05 ` Zyta Szpak
2016-07-04 6:51 ` [dpdk-dev] [PATCH v5 " Zyta Szpak
2016-07-04 11:36 ` [dpdk-dev] [PATCH v6 " Zyta Szpak
4 siblings, 1 reply; 17+ messages in thread
From: Thomas Monjalon @ 2016-06-27 15:19 UTC (permalink / raw)
To: zr
Cc: remy.horton, wenzhuo.lu, helin.zhang, konstantin.ananyev,
jingjing.wu, dev
2016-06-23 15:26, zr@semihalf.com:
> From: Zyta Szpak <zr@semihalf.com>
>
> Version 4 of fixing the assumption of that device registers
> are always 32 bits long. rte_eth_dev_get_reg_length and
> rte_eth_dev_get_reg_info callbacks did not provide register size
> to the app in any way. It is needed to allocate proper number
> of bytes before retrieving registers content with
> rte_eth_dev_get_reg. This commit remove rte_eth_dev_get_reg_length
> callback and adds width parameter to reg_info struct which makes
> it possible to call rte_eth_dev_get_reg_info to get attributes
> first. The drivers using this callback fill width and length
> when call to function made with data=NULL.
>
> Signed-off-by: Zyta Szpak <zr@semihalf.com>
Please do not mention patch revision in the commit log.
However you can add a changelog below the three dashes (with --annotate).
And please use --in-reply-to to keep revisions in the same thread.
Thanks
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH v4 1/2] ethdev: remove get_reg_length callback
2016-06-27 15:19 ` Thomas Monjalon
@ 2016-06-28 16:05 ` Zyta Szpak
0 siblings, 0 replies; 17+ messages in thread
From: Zyta Szpak @ 2016-06-28 16:05 UTC (permalink / raw)
To: Thomas Monjalon
Cc: remy.horton, wenzhuo.lu, helin.zhang, konstantin.ananyev,
jingjing.wu, dev
OK
On 27.06.2016 17:19, Thomas Monjalon wrote:
> 2016-06-23 15:26, zr@semihalf.com:
>> From: Zyta Szpak <zr@semihalf.com>
>>
>> Version 4 of fixing the assumption of that device registers
>> are always 32 bits long. rte_eth_dev_get_reg_length and
>> rte_eth_dev_get_reg_info callbacks did not provide register size
>> to the app in any way. It is needed to allocate proper number
>> of bytes before retrieving registers content with
>> rte_eth_dev_get_reg. This commit remove rte_eth_dev_get_reg_length
>> callback and adds width parameter to reg_info struct which makes
>> it possible to call rte_eth_dev_get_reg_info to get attributes
>> first. The drivers using this callback fill width and length
>> when call to function made with data=NULL.
>>
>> Signed-off-by: Zyta Szpak <zr@semihalf.com>
> Please do not mention patch revision in the commit log.
> However you can add a changelog below the three dashes (with --annotate).
> And please use --in-reply-to to keep revisions in the same thread.
> Thanks
^ permalink raw reply [flat|nested] 17+ messages in thread
* [dpdk-dev] [PATCH v5 1/2] ethdev: remove get_reg_length callback
2016-06-23 13:26 [dpdk-dev] [PATCH v4 1/2] ethdev: remove get_reg_length callback zr
` (2 preceding siblings ...)
2016-06-27 15:19 ` Thomas Monjalon
@ 2016-07-04 6:51 ` Zyta Szpak
2016-07-04 6:51 ` [dpdk-dev] [PATCH v5 2/2] examples/ethtool: use rte_eth_dev_get_reg_info for reg params Zyta Szpak
2016-07-04 10:38 ` [dpdk-dev] [PATCH v5 1/2] ethdev: remove get_reg_length callback Remy Horton
2016-07-04 11:36 ` [dpdk-dev] [PATCH v6 " Zyta Szpak
4 siblings, 2 replies; 17+ messages in thread
From: Zyta Szpak @ 2016-07-04 6:51 UTC (permalink / raw)
To: remy.horton, thomas.monjalon, wenzhuo.lu, helin.zhang,
konstantin.ananyev, jingjing.wu, jerin.jacob, rahul.lakkireddy
Cc: dev, Zyta Szpak
From: Zyta Szpak <zr@semihalf.com>
Removes hard-coded assumption that device registers
are always 32 bits wide. The rte_eth_dev_get_reg_length
and rte_eth_dev_get_reg_info callbacks did not
provide register size to the app in any way while is
needed to allocate correct number of bytes before
retrieving registers using rte_eth_dev_get_reg.
This commit changes rte_eth_dev_get_reg_info so that
it can be used to retrieve both the number of registers
and their width, and removes the now-redundant
rte_eth_dev_get_reg_length.
Signed-off-by: Zyta Szpak <zr@semihalf.com>
---
drivers/net/cxgbe/cxgbe_ethdev.c | 11 +++++++++--
drivers/net/e1000/igb_ethdev.c | 14 ++++++++++++--
drivers/net/i40e/i40e_ethdev.c | 15 ++++++---------
drivers/net/ixgbe/ixgbe_ethdev.c | 14 ++++++++++++--
drivers/net/thunderx/nicvf_ethdev.c | 14 +++++---------
drivers/net/thunderx/nicvf_ethdev.h | 1 +
lib/librte_ether/rte_dev_info.h | 1 +
lib/librte_ether/rte_ethdev.c | 12 ------------
lib/librte_ether/rte_ethdev.h | 25 +++++--------------------
lib/librte_ether/rte_ether_version.map | 1 -
10 files changed, 51 insertions(+), 57 deletions(-)
diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
index 6c130ed..90098f9 100644
--- a/drivers/net/cxgbe/cxgbe_ethdev.c
+++ b/drivers/net/cxgbe/cxgbe_ethdev.c
@@ -934,7 +934,15 @@ static int cxgbe_get_regs(struct rte_eth_dev *eth_dev,
struct port_info *pi = (struct port_info *)(eth_dev->data->dev_private);
struct adapter *adapter = pi->adapter;
- regs->length = cxgbe_get_regs_len(eth_dev);
+ if (regs->data == NULL) {
+ regs->length = cxgbe_get_regs_len(eth_dev);
+ regs->width = sizeof(uint32_t);
+ regs->version = CHELSIO_CHIP_VERSION(adapter->params.chip) |
+ (CHELSIO_CHIP_RELEASE(adapter->params.chip) << 10) |
+ (1 << 16);
+ return 0;
+ }
+
regs->version = CHELSIO_CHIP_VERSION(adapter->params.chip) |
(CHELSIO_CHIP_RELEASE(adapter->params.chip) << 10) |
(1 << 16);
@@ -971,7 +979,6 @@ static const struct eth_dev_ops cxgbe_eth_dev_ops = {
.get_eeprom_length = cxgbe_get_eeprom_length,
.get_eeprom = cxgbe_get_eeprom,
.set_eeprom = cxgbe_set_eeprom,
- .get_reg_length = cxgbe_get_regs_len,
.get_reg = cxgbe_get_regs,
};
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index dea50f3..594655d 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -386,7 +386,6 @@ static const struct eth_dev_ops eth_igb_ops = {
.timesync_disable = igb_timesync_disable,
.timesync_read_rx_timestamp = igb_timesync_read_rx_timestamp,
.timesync_read_tx_timestamp = igb_timesync_read_tx_timestamp,
- .get_reg_length = eth_igb_get_reg_length,
.get_reg = eth_igb_get_regs,
.get_eeprom_length = eth_igb_get_eeprom_length,
.get_eeprom = eth_igb_get_eeprom,
@@ -426,7 +425,6 @@ static const struct eth_dev_ops igbvf_eth_dev_ops = {
.rxq_info_get = igb_rxq_info_get,
.txq_info_get = igb_txq_info_get,
.mac_addr_set = igbvf_default_mac_addr_set,
- .get_reg_length = igbvf_get_reg_length,
.get_reg = igbvf_get_regs,
};
@@ -4947,6 +4945,12 @@ eth_igb_get_regs(struct rte_eth_dev *dev,
int count = 0;
const struct reg_info *reg_group;
+ if (data == NULL) {
+ regs->length = eth_igb_get_reg_length(dev);
+ regs->width = sizeof(uint32_t);
+ return 0;
+ }
+
/* Support only full register dump */
if ((regs->length == 0) ||
(regs->length == (uint32_t)eth_igb_get_reg_length(dev))) {
@@ -4971,6 +4975,12 @@ igbvf_get_regs(struct rte_eth_dev *dev,
int count = 0;
const struct reg_info *reg_group;
+ if (data == NULL) {
+ regs->length = igbvf_get_reg_length(dev);
+ regs->width = sizeof(uint32_t);
+ return 0;
+ }
+
/* Support only full register dump */
if ((regs->length == 0) ||
(regs->length == (uint32_t)igbvf_get_reg_length(dev))) {
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index c07da1b..8e3e27d 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -440,8 +440,6 @@ static int i40e_dev_rx_queue_intr_enable(struct rte_eth_dev *dev,
static int i40e_dev_rx_queue_intr_disable(struct rte_eth_dev *dev,
uint16_t queue_id);
-static int i40e_get_reg_length(struct rte_eth_dev *dev);
-
static int i40e_get_regs(struct rte_eth_dev *dev,
struct rte_dev_reg_info *regs);
@@ -524,7 +522,6 @@ static const struct eth_dev_ops i40e_eth_dev_ops = {
.timesync_adjust_time = i40e_timesync_adjust_time,
.timesync_read_time = i40e_timesync_read_time,
.timesync_write_time = i40e_timesync_write_time,
- .get_reg_length = i40e_get_reg_length,
.get_reg = i40e_get_regs,
.get_eeprom_length = i40e_get_eeprom_length,
.get_eeprom = i40e_get_eeprom,
@@ -9350,12 +9347,6 @@ i40e_dev_rx_queue_intr_disable(struct rte_eth_dev *dev, uint16_t queue_id)
return 0;
}
-static int i40e_get_reg_length(__rte_unused struct rte_eth_dev *dev)
-{
- /* Highest base addr + 32-bit word */
- return I40E_GLGEN_STAT_CLEAR + 4;
-}
-
static int i40e_get_regs(struct rte_eth_dev *dev,
struct rte_dev_reg_info *regs)
{
@@ -9364,6 +9355,12 @@ static int i40e_get_regs(struct rte_eth_dev *dev,
uint32_t reg_idx, arr_idx, arr_idx2, reg_offset;
const struct i40e_reg_info *reg_info;
+ if (ptr_data == NULL) {
+ regs->length = I40E_GLGEN_STAT_CLEAR + 4;
+ regs->width = sizeof(uint32_t);
+ return 0;
+ }
+
/* The first few registers have to be read using AQ operations */
reg_idx = 0;
while (i40e_regs_adminq[reg_idx].name) {
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index d61af37..09109ed 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -538,7 +538,6 @@ static const struct eth_dev_ops ixgbe_eth_dev_ops = {
.timesync_disable = ixgbe_timesync_disable,
.timesync_read_rx_timestamp = ixgbe_timesync_read_rx_timestamp,
.timesync_read_tx_timestamp = ixgbe_timesync_read_tx_timestamp,
- .get_reg_length = ixgbe_get_reg_length,
.get_reg = ixgbe_get_regs,
.get_eeprom_length = ixgbe_get_eeprom_length,
.get_eeprom = ixgbe_get_eeprom,
@@ -589,7 +588,6 @@ static const struct eth_dev_ops ixgbevf_eth_dev_ops = {
.rxq_info_get = ixgbe_rxq_info_get,
.txq_info_get = ixgbe_txq_info_get,
.mac_addr_set = ixgbevf_set_default_mac_addr,
- .get_reg_length = ixgbevf_get_reg_length,
.get_reg = ixgbevf_get_regs,
.reta_update = ixgbe_dev_rss_reta_update,
.reta_query = ixgbe_dev_rss_reta_query,
@@ -6323,6 +6321,12 @@ ixgbe_get_regs(struct rte_eth_dev *dev,
const struct reg_info **reg_set = (hw->mac.type == ixgbe_mac_82598EB) ?
ixgbe_regs_mac_82598EB : ixgbe_regs_others;
+ if (data == NULL) {
+ regs->length = ixgbe_get_reg_length(dev);
+ regs->width = sizeof(uint32_t);
+ return 0;
+ }
+
/* Support only full register dump */
if ((regs->length == 0) ||
(regs->length == (uint32_t)ixgbe_get_reg_length(dev))) {
@@ -6347,6 +6351,12 @@ ixgbevf_get_regs(struct rte_eth_dev *dev,
int count = 0;
const struct reg_info *reg_group;
+ if (data == NULL) {
+ regs->length = ixgbevf_get_reg_length(dev);
+ regs->width = sizeof(uint32_t);
+ return 0;
+ }
+
/* Support only full register dump */
if ((regs->length == 0) ||
(regs->length == (uint32_t)ixgbevf_get_reg_length(dev))) {
diff --git a/drivers/net/thunderx/nicvf_ethdev.c b/drivers/net/thunderx/nicvf_ethdev.c
index d534312..5b2c605 100644
--- a/drivers/net/thunderx/nicvf_ethdev.c
+++ b/drivers/net/thunderx/nicvf_ethdev.c
@@ -189,19 +189,16 @@ nicvf_dev_set_mtu(struct rte_eth_dev *dev, uint16_t mtu)
}
static int
-nicvf_dev_get_reg_length(struct rte_eth_dev *dev __rte_unused)
-{
- return nicvf_reg_get_count();
-}
-
-static int
nicvf_dev_get_regs(struct rte_eth_dev *dev, struct rte_dev_reg_info *regs)
{
uint64_t *data = regs->data;
struct nicvf *nic = nicvf_pmd_priv(dev);
- if (data == NULL)
- return -EINVAL;
+ if (data == NULL) {
+ regs->length = nicvf_reg_get_count();
+ regs->width = THUNDERX_REG_BYTES;
+ return 0;
+ }
/* Support only full register dump */
if ((regs->length == 0) ||
@@ -1623,7 +1620,6 @@ static const struct eth_dev_ops nicvf_eth_dev_ops = {
.rx_queue_count = nicvf_dev_rx_queue_count,
.tx_queue_setup = nicvf_dev_tx_queue_setup,
.tx_queue_release = nicvf_dev_tx_queue_release,
- .get_reg_length = nicvf_dev_get_reg_length,
.get_reg = nicvf_dev_get_regs,
};
diff --git a/drivers/net/thunderx/nicvf_ethdev.h b/drivers/net/thunderx/nicvf_ethdev.h
index 59fa19c..34447e0 100644
--- a/drivers/net/thunderx/nicvf_ethdev.h
+++ b/drivers/net/thunderx/nicvf_ethdev.h
@@ -36,6 +36,7 @@
#include <rte_ethdev.h>
#define THUNDERX_NICVF_PMD_VERSION "1.0"
+#define THUNDERX_REG_BYTES 8
#define NICVF_INTR_POLL_INTERVAL_MS 50
#define NICVF_HALF_DUPLEX 0x00
diff --git a/lib/librte_ether/rte_dev_info.h b/lib/librte_ether/rte_dev_info.h
index 291bd4d..574683d 100644
--- a/lib/librte_ether/rte_dev_info.h
+++ b/lib/librte_ether/rte_dev_info.h
@@ -41,6 +41,7 @@ struct rte_dev_reg_info {
void *data; /**< Buffer for return registers */
uint32_t offset; /**< Start register table location for access */
uint32_t length; /**< Number of registers to fetch */
+ uint32_t width; /**< Size of device register */
uint32_t version; /**< Device version */
};
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 42aaef7..2d73758 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3308,18 +3308,6 @@ rte_eth_timesync_write_time(uint8_t port_id, const struct timespec *timestamp)
}
int
-rte_eth_dev_get_reg_length(uint8_t port_id)
-{
- struct rte_eth_dev *dev;
-
- RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
-
- dev = &rte_eth_devices[port_id];
- RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->get_reg_length, -ENOTSUP);
- return (*dev->dev_ops->get_reg_length)(dev);
-}
-
-int
rte_eth_dev_get_reg_info(uint8_t port_id, struct rte_dev_reg_info *info)
{
struct rte_eth_dev *dev;
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index ebe6578..2ad7de5 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1322,9 +1322,6 @@ typedef int (*eth_timesync_write_time)(struct rte_eth_dev *dev,
const struct timespec *timestamp);
/**< @internal Function used to get time from the device clock */
-typedef int (*eth_get_reg_length_t)(struct rte_eth_dev *dev);
-/**< @internal Retrieve device register count */
-
typedef int (*eth_get_reg_t)(struct rte_eth_dev *dev,
struct rte_dev_reg_info *info);
/**< @internal Retrieve registers */
@@ -1488,8 +1485,6 @@ struct eth_dev_ops {
/** Query redirection table. */
reta_query_t reta_query;
- eth_get_reg_length_t get_reg_length;
- /**< Get # of registers */
eth_get_reg_t get_reg;
/**< Get registers */
eth_get_eeprom_length_t get_eeprom_length;
@@ -4047,25 +4042,15 @@ int rte_eth_tx_queue_info_get(uint8_t port_id, uint16_t queue_id,
struct rte_eth_txq_info *qinfo);
/**
- * Retrieve number of available registers for access
- *
- * @param port_id
- * The port identifier of the Ethernet device.
- * @return
- * - (>=0) number of registers if successful.
- * - (-ENOTSUP) if hardware doesn't support.
- * - (-ENODEV) if *port_id* invalid.
- * - others depends on the specific operations implementation.
- */
-int rte_eth_dev_get_reg_length(uint8_t port_id);
-
-/**
- * Retrieve device registers and register attributes
+ * Retrieve device registers and register attributes (number of registers and
+ * register size)
*
* @param port_id
* The port identifier of the Ethernet device.
* @param info
- * The template includes buffer for register data and attribute to be filled.
+ * Pointer to rte_dev_reg_info structure to fill in. If info->data is
+ * NULL the function fills in the width and length fields. If non-NULL
+ * the registers are put into the buffer pointed at by the data field.
* @return
* - (0) if successful.
* - (-ENOTSUP) if hardware doesn't support.
diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map
index 97ed0b0..bbc50d1 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -36,7 +36,6 @@ DPDK_2.2 {
rte_eth_dev_get_eeprom_length;
rte_eth_dev_get_mtu;
rte_eth_dev_get_reg_info;
- rte_eth_dev_get_reg_length;
rte_eth_dev_get_vlan_offload;
rte_eth_devices;
rte_eth_dev_info_get;
--
2.7.4
^ permalink raw reply [flat|nested] 17+ messages in thread* [dpdk-dev] [PATCH v5 2/2] examples/ethtool: use rte_eth_dev_get_reg_info for reg params
2016-07-04 6:51 ` [dpdk-dev] [PATCH v5 " Zyta Szpak
@ 2016-07-04 6:51 ` Zyta Szpak
2016-07-04 10:39 ` Remy Horton
2016-07-04 10:38 ` [dpdk-dev] [PATCH v5 1/2] ethdev: remove get_reg_length callback Remy Horton
1 sibling, 1 reply; 17+ messages in thread
From: Zyta Szpak @ 2016-07-04 6:51 UTC (permalink / raw)
To: remy.horton, thomas.monjalon, wenzhuo.lu, helin.zhang,
konstantin.ananyev, jingjing.wu, jerin.jacob, rahul.lakkireddy
Cc: dev, Zyta Szpak
From: Zyta Szpak <zr@semihalf.com>
Version 4 of fixing the fixed register width assumption.
The app was allocating too little space for 64-bit registers
which resulted in memory corruption. This commit resolves
this by getting the number of registers and size of register
by rte_eth_dev_get_reg_info function called first time
with data=NULL.
Signed-off-by: Zyta Szpak <zr@semihalf.com>
---
examples/ethtool/lib/rte_ethtool.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/examples/ethtool/lib/rte_ethtool.c b/examples/ethtool/lib/rte_ethtool.c
index 54391f2..a1f91d4 100644
--- a/examples/ethtool/lib/rte_ethtool.c
+++ b/examples/ethtool/lib/rte_ethtool.c
@@ -46,6 +46,7 @@ int
rte_ethtool_get_drvinfo(uint8_t port_id, struct ethtool_drvinfo *drvinfo)
{
struct rte_eth_dev_info dev_info;
+ struct rte_dev_reg_info reg_info;
int n;
if (drvinfo == NULL)
@@ -65,7 +66,9 @@ rte_ethtool_get_drvinfo(uint8_t port_id, struct ethtool_drvinfo *drvinfo)
dev_info.pci_dev->addr.domain, dev_info.pci_dev->addr.bus,
dev_info.pci_dev->addr.devid, dev_info.pci_dev->addr.function);
- n = rte_eth_dev_get_reg_length(port_id);
+ memset(®_info, 0, sizeof(reg_info));
+ rte_eth_dev_get_reg_info(port_id, ®_info);
+ n = reg_info.length;
if (n > 0)
drvinfo->regdump_len = n;
else
@@ -86,12 +89,16 @@ rte_ethtool_get_drvinfo(uint8_t port_id, struct ethtool_drvinfo *drvinfo)
int
rte_ethtool_get_regs_len(uint8_t port_id)
{
- int count_regs;
+ struct rte_dev_reg_info reg_info;
+ int ret;
+
+ memset(®_info, 0, sizeof(reg_info));
+
+ ret = rte_eth_dev_get_reg_info(port_id, ®_info);
+ if (ret)
+ return ret;
- count_regs = rte_eth_dev_get_reg_length(port_id);
- if (count_regs > 0)
- return count_regs * sizeof(uint32_t);
- return count_regs;
+ return reg_info.length * reg_info.width;
}
int
--
2.7.4
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [dpdk-dev] [PATCH v5 2/2] examples/ethtool: use rte_eth_dev_get_reg_info for reg params
2016-07-04 6:51 ` [dpdk-dev] [PATCH v5 2/2] examples/ethtool: use rte_eth_dev_get_reg_info for reg params Zyta Szpak
@ 2016-07-04 10:39 ` Remy Horton
0 siblings, 0 replies; 17+ messages in thread
From: Remy Horton @ 2016-07-04 10:39 UTC (permalink / raw)
To: Zyta Szpak, thomas.monjalon, wenzhuo.lu, helin.zhang,
konstantin.ananyev, jingjing.wu, jerin.jacob, rahul.lakkireddy
Cc: dev
On 04/07/2016 07:51, Zyta Szpak wrote:
> From: Zyta Szpak <zr@semihalf.com>
>
> Version 4 of fixing the fixed register width assumption.
> The app was allocating too little space for 64-bit registers
> which resulted in memory corruption. This commit resolves
> this by getting the number of registers and size of register
> by rte_eth_dev_get_reg_info function called first time
> with data=NULL.
Comments regarding commit message on v4 1/2 also apply here..
It is also not v4.. :)
> Signed-off-by: Zyta Szpak <zr@semihalf.com>
> ---
> examples/ethtool/lib/rte_ethtool.c | 19 +++++++++++++------
> 1 file changed, 13 insertions(+), 6 deletions(-)
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH v5 1/2] ethdev: remove get_reg_length callback
2016-07-04 6:51 ` [dpdk-dev] [PATCH v5 " Zyta Szpak
2016-07-04 6:51 ` [dpdk-dev] [PATCH v5 2/2] examples/ethtool: use rte_eth_dev_get_reg_info for reg params Zyta Szpak
@ 2016-07-04 10:38 ` Remy Horton
2016-07-04 11:34 ` Zyta Szpak
1 sibling, 1 reply; 17+ messages in thread
From: Remy Horton @ 2016-07-04 10:38 UTC (permalink / raw)
To: Zyta Szpak, thomas.monjalon, wenzhuo.lu, helin.zhang,
konstantin.ananyev, jingjing.wu, jerin.jacob, rahul.lakkireddy
Cc: dev
> +++ b/drivers/net/cxgbe/cxgbe_ethdev.c
> @@ -934,7 +934,15 @@ static int cxgbe_get_regs(struct rte_eth_dev *eth_dev,
> struct port_info *pi = (struct port_info *)(eth_dev->data->dev_private);
> struct adapter *adapter = pi->adapter;
>
> - regs->length = cxgbe_get_regs_len(eth_dev);
> + if (regs->data == NULL) {
> + regs->length = cxgbe_get_regs_len(eth_dev);
> + regs->width = sizeof(uint32_t);
> + regs->version = CHELSIO_CHIP_VERSION(adapter->params.chip) |
> + (CHELSIO_CHIP_RELEASE(adapter->params.chip) << 10) |
> + (1 << 16);
> + return 0;
> + }
> +
> regs->version = CHELSIO_CHIP_VERSION(adapter->params.chip) |
> (CHELSIO_CHIP_RELEASE(adapter->params.chip) << 10) |
> (1 << 16);
Code duplication..
Rest looks ok and passed a quick compile test. Might need to keep an eye
out for other driver changes.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [dpdk-dev] [PATCH v5 1/2] ethdev: remove get_reg_length callback
2016-07-04 10:38 ` [dpdk-dev] [PATCH v5 1/2] ethdev: remove get_reg_length callback Remy Horton
@ 2016-07-04 11:34 ` Zyta Szpak
0 siblings, 0 replies; 17+ messages in thread
From: Zyta Szpak @ 2016-07-04 11:34 UTC (permalink / raw)
To: Remy Horton
Cc: Thomas Monjalon, wenzhuo.lu, helin.zhang, konstantin.ananyev,
jingjing.wu, Jacob, Jerin, rahul.lakkireddy, dev
Very good point.. this one and the other :)
THere comes fixed version.
2016-07-04 12:38 GMT+02:00 Remy Horton <remy.horton@intel.com>:
>
> +++ b/drivers/net/cxgbe/cxgbe_ethdev.c
>> @@ -934,7 +934,15 @@ static int cxgbe_get_regs(struct rte_eth_dev
>> *eth_dev,
>> struct port_info *pi = (struct port_info
>> *)(eth_dev->data->dev_private);
>> struct adapter *adapter = pi->adapter;
>>
>> - regs->length = cxgbe_get_regs_len(eth_dev);
>> + if (regs->data == NULL) {
>> + regs->length = cxgbe_get_regs_len(eth_dev);
>> + regs->width = sizeof(uint32_t);
>> + regs->version =
>> CHELSIO_CHIP_VERSION(adapter->params.chip) |
>> + (CHELSIO_CHIP_RELEASE(adapter->params.chip) <<
>> 10) |
>> + (1 << 16);
>> + return 0;
>> + }
>> +
>> regs->version = CHELSIO_CHIP_VERSION(adapter->params.chip) |
>> (CHELSIO_CHIP_RELEASE(adapter->params.chip) <<
>> 10) |
>> (1 << 16);
>>
>
> Code duplication..
>
> Rest looks ok and passed a quick compile test. Might need to keep an eye
> out for other driver changes.
>
--
Zyta Szpak
Software Development Engineer
Semihalf sp. z o.o.
ul. Krakusa 11
30-535 Kraków
^ permalink raw reply [flat|nested] 17+ messages in thread
* [dpdk-dev] [PATCH v6 1/2] ethdev: remove get_reg_length callback
2016-06-23 13:26 [dpdk-dev] [PATCH v4 1/2] ethdev: remove get_reg_length callback zr
` (3 preceding siblings ...)
2016-07-04 6:51 ` [dpdk-dev] [PATCH v5 " Zyta Szpak
@ 2016-07-04 11:36 ` Zyta Szpak
2016-07-04 11:36 ` [dpdk-dev] [PATCH v6 2/2] examples/ethtool: use rte_eth_dev_get_reg_info for reg params Zyta Szpak
2016-07-04 13:24 ` [dpdk-dev] [PATCH v6 1/2] ethdev: remove get_reg_length callback Remy Horton
4 siblings, 2 replies; 17+ messages in thread
From: Zyta Szpak @ 2016-07-04 11:36 UTC (permalink / raw)
To: remy.horton, thomas.monjalon, wenzhuo.lu, helin.zhang,
konstantin.ananyev, jingjing.wu, jerin.jacob, rahul.lakkireddy
Cc: dev, Zyta Szpak
From: Zyta Szpak <zr@semihalf.com>
Removes hard-coded assumption that device registers
are always 32 bits wide. The rte_eth_dev_get_reg_length
and rte_eth_dev_get_reg_info callbacks did not
provide register size to the app in any way while is
needed to allocate correct number of bytes before
retrieving registers using rte_eth_dev_get_reg.
This commit changes rte_eth_dev_get_reg_info so that
it can be used to retrieve both the number of registers
and their width, and removes the now-redundant
rte_eth_dev_get_reg_length.
Signed-off-by: Zyta Szpak <zr@semihalf.com>
---
drivers/net/cxgbe/cxgbe_ethdev.c | 14 ++++++++++----
drivers/net/e1000/igb_ethdev.c | 14 ++++++++++++--
drivers/net/i40e/i40e_ethdev.c | 15 ++++++---------
drivers/net/ixgbe/ixgbe_ethdev.c | 14 ++++++++++++--
drivers/net/thunderx/nicvf_ethdev.c | 14 +++++---------
drivers/net/thunderx/nicvf_ethdev.h | 1 +
lib/librte_ether/rte_dev_info.h | 1 +
lib/librte_ether/rte_ethdev.c | 12 ------------
lib/librte_ether/rte_ethdev.h | 25 +++++--------------------
lib/librte_ether/rte_ether_version.map | 1 -
10 files changed, 52 insertions(+), 59 deletions(-)
diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
index 6c130ed..f822d8f 100644
--- a/drivers/net/cxgbe/cxgbe_ethdev.c
+++ b/drivers/net/cxgbe/cxgbe_ethdev.c
@@ -934,10 +934,17 @@ static int cxgbe_get_regs(struct rte_eth_dev *eth_dev,
struct port_info *pi = (struct port_info *)(eth_dev->data->dev_private);
struct adapter *adapter = pi->adapter;
- regs->length = cxgbe_get_regs_len(eth_dev);
regs->version = CHELSIO_CHIP_VERSION(adapter->params.chip) |
- (CHELSIO_CHIP_RELEASE(adapter->params.chip) << 10) |
- (1 << 16);
+ (CHELSIO_CHIP_RELEASE(adapter->params.chip) << 10) |
+ (1 << 16);
+
+ if (regs->data == NULL) {
+ regs->length = cxgbe_get_regs_len(eth_dev);
+ regs->width = sizeof(uint32_t);
+
+ return 0;
+ }
+
t4_get_regs(adapter, regs->data, (regs->length * sizeof(uint32_t)));
return 0;
@@ -971,7 +978,6 @@ static const struct eth_dev_ops cxgbe_eth_dev_ops = {
.get_eeprom_length = cxgbe_get_eeprom_length,
.get_eeprom = cxgbe_get_eeprom,
.set_eeprom = cxgbe_set_eeprom,
- .get_reg_length = cxgbe_get_regs_len,
.get_reg = cxgbe_get_regs,
};
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index dea50f3..594655d 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -386,7 +386,6 @@ static const struct eth_dev_ops eth_igb_ops = {
.timesync_disable = igb_timesync_disable,
.timesync_read_rx_timestamp = igb_timesync_read_rx_timestamp,
.timesync_read_tx_timestamp = igb_timesync_read_tx_timestamp,
- .get_reg_length = eth_igb_get_reg_length,
.get_reg = eth_igb_get_regs,
.get_eeprom_length = eth_igb_get_eeprom_length,
.get_eeprom = eth_igb_get_eeprom,
@@ -426,7 +425,6 @@ static const struct eth_dev_ops igbvf_eth_dev_ops = {
.rxq_info_get = igb_rxq_info_get,
.txq_info_get = igb_txq_info_get,
.mac_addr_set = igbvf_default_mac_addr_set,
- .get_reg_length = igbvf_get_reg_length,
.get_reg = igbvf_get_regs,
};
@@ -4947,6 +4945,12 @@ eth_igb_get_regs(struct rte_eth_dev *dev,
int count = 0;
const struct reg_info *reg_group;
+ if (data == NULL) {
+ regs->length = eth_igb_get_reg_length(dev);
+ regs->width = sizeof(uint32_t);
+ return 0;
+ }
+
/* Support only full register dump */
if ((regs->length == 0) ||
(regs->length == (uint32_t)eth_igb_get_reg_length(dev))) {
@@ -4971,6 +4975,12 @@ igbvf_get_regs(struct rte_eth_dev *dev,
int count = 0;
const struct reg_info *reg_group;
+ if (data == NULL) {
+ regs->length = igbvf_get_reg_length(dev);
+ regs->width = sizeof(uint32_t);
+ return 0;
+ }
+
/* Support only full register dump */
if ((regs->length == 0) ||
(regs->length == (uint32_t)igbvf_get_reg_length(dev))) {
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index c07da1b..8e3e27d 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -440,8 +440,6 @@ static int i40e_dev_rx_queue_intr_enable(struct rte_eth_dev *dev,
static int i40e_dev_rx_queue_intr_disable(struct rte_eth_dev *dev,
uint16_t queue_id);
-static int i40e_get_reg_length(struct rte_eth_dev *dev);
-
static int i40e_get_regs(struct rte_eth_dev *dev,
struct rte_dev_reg_info *regs);
@@ -524,7 +522,6 @@ static const struct eth_dev_ops i40e_eth_dev_ops = {
.timesync_adjust_time = i40e_timesync_adjust_time,
.timesync_read_time = i40e_timesync_read_time,
.timesync_write_time = i40e_timesync_write_time,
- .get_reg_length = i40e_get_reg_length,
.get_reg = i40e_get_regs,
.get_eeprom_length = i40e_get_eeprom_length,
.get_eeprom = i40e_get_eeprom,
@@ -9350,12 +9347,6 @@ i40e_dev_rx_queue_intr_disable(struct rte_eth_dev *dev, uint16_t queue_id)
return 0;
}
-static int i40e_get_reg_length(__rte_unused struct rte_eth_dev *dev)
-{
- /* Highest base addr + 32-bit word */
- return I40E_GLGEN_STAT_CLEAR + 4;
-}
-
static int i40e_get_regs(struct rte_eth_dev *dev,
struct rte_dev_reg_info *regs)
{
@@ -9364,6 +9355,12 @@ static int i40e_get_regs(struct rte_eth_dev *dev,
uint32_t reg_idx, arr_idx, arr_idx2, reg_offset;
const struct i40e_reg_info *reg_info;
+ if (ptr_data == NULL) {
+ regs->length = I40E_GLGEN_STAT_CLEAR + 4;
+ regs->width = sizeof(uint32_t);
+ return 0;
+ }
+
/* The first few registers have to be read using AQ operations */
reg_idx = 0;
while (i40e_regs_adminq[reg_idx].name) {
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index d61af37..09109ed 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -538,7 +538,6 @@ static const struct eth_dev_ops ixgbe_eth_dev_ops = {
.timesync_disable = ixgbe_timesync_disable,
.timesync_read_rx_timestamp = ixgbe_timesync_read_rx_timestamp,
.timesync_read_tx_timestamp = ixgbe_timesync_read_tx_timestamp,
- .get_reg_length = ixgbe_get_reg_length,
.get_reg = ixgbe_get_regs,
.get_eeprom_length = ixgbe_get_eeprom_length,
.get_eeprom = ixgbe_get_eeprom,
@@ -589,7 +588,6 @@ static const struct eth_dev_ops ixgbevf_eth_dev_ops = {
.rxq_info_get = ixgbe_rxq_info_get,
.txq_info_get = ixgbe_txq_info_get,
.mac_addr_set = ixgbevf_set_default_mac_addr,
- .get_reg_length = ixgbevf_get_reg_length,
.get_reg = ixgbevf_get_regs,
.reta_update = ixgbe_dev_rss_reta_update,
.reta_query = ixgbe_dev_rss_reta_query,
@@ -6323,6 +6321,12 @@ ixgbe_get_regs(struct rte_eth_dev *dev,
const struct reg_info **reg_set = (hw->mac.type == ixgbe_mac_82598EB) ?
ixgbe_regs_mac_82598EB : ixgbe_regs_others;
+ if (data == NULL) {
+ regs->length = ixgbe_get_reg_length(dev);
+ regs->width = sizeof(uint32_t);
+ return 0;
+ }
+
/* Support only full register dump */
if ((regs->length == 0) ||
(regs->length == (uint32_t)ixgbe_get_reg_length(dev))) {
@@ -6347,6 +6351,12 @@ ixgbevf_get_regs(struct rte_eth_dev *dev,
int count = 0;
const struct reg_info *reg_group;
+ if (data == NULL) {
+ regs->length = ixgbevf_get_reg_length(dev);
+ regs->width = sizeof(uint32_t);
+ return 0;
+ }
+
/* Support only full register dump */
if ((regs->length == 0) ||
(regs->length == (uint32_t)ixgbevf_get_reg_length(dev))) {
diff --git a/drivers/net/thunderx/nicvf_ethdev.c b/drivers/net/thunderx/nicvf_ethdev.c
index d534312..5b2c605 100644
--- a/drivers/net/thunderx/nicvf_ethdev.c
+++ b/drivers/net/thunderx/nicvf_ethdev.c
@@ -189,19 +189,16 @@ nicvf_dev_set_mtu(struct rte_eth_dev *dev, uint16_t mtu)
}
static int
-nicvf_dev_get_reg_length(struct rte_eth_dev *dev __rte_unused)
-{
- return nicvf_reg_get_count();
-}
-
-static int
nicvf_dev_get_regs(struct rte_eth_dev *dev, struct rte_dev_reg_info *regs)
{
uint64_t *data = regs->data;
struct nicvf *nic = nicvf_pmd_priv(dev);
- if (data == NULL)
- return -EINVAL;
+ if (data == NULL) {
+ regs->length = nicvf_reg_get_count();
+ regs->width = THUNDERX_REG_BYTES;
+ return 0;
+ }
/* Support only full register dump */
if ((regs->length == 0) ||
@@ -1623,7 +1620,6 @@ static const struct eth_dev_ops nicvf_eth_dev_ops = {
.rx_queue_count = nicvf_dev_rx_queue_count,
.tx_queue_setup = nicvf_dev_tx_queue_setup,
.tx_queue_release = nicvf_dev_tx_queue_release,
- .get_reg_length = nicvf_dev_get_reg_length,
.get_reg = nicvf_dev_get_regs,
};
diff --git a/drivers/net/thunderx/nicvf_ethdev.h b/drivers/net/thunderx/nicvf_ethdev.h
index 59fa19c..34447e0 100644
--- a/drivers/net/thunderx/nicvf_ethdev.h
+++ b/drivers/net/thunderx/nicvf_ethdev.h
@@ -36,6 +36,7 @@
#include <rte_ethdev.h>
#define THUNDERX_NICVF_PMD_VERSION "1.0"
+#define THUNDERX_REG_BYTES 8
#define NICVF_INTR_POLL_INTERVAL_MS 50
#define NICVF_HALF_DUPLEX 0x00
diff --git a/lib/librte_ether/rte_dev_info.h b/lib/librte_ether/rte_dev_info.h
index 291bd4d..574683d 100644
--- a/lib/librte_ether/rte_dev_info.h
+++ b/lib/librte_ether/rte_dev_info.h
@@ -41,6 +41,7 @@ struct rte_dev_reg_info {
void *data; /**< Buffer for return registers */
uint32_t offset; /**< Start register table location for access */
uint32_t length; /**< Number of registers to fetch */
+ uint32_t width; /**< Size of device register */
uint32_t version; /**< Device version */
};
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 42aaef7..2d73758 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3308,18 +3308,6 @@ rte_eth_timesync_write_time(uint8_t port_id, const struct timespec *timestamp)
}
int
-rte_eth_dev_get_reg_length(uint8_t port_id)
-{
- struct rte_eth_dev *dev;
-
- RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
-
- dev = &rte_eth_devices[port_id];
- RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->get_reg_length, -ENOTSUP);
- return (*dev->dev_ops->get_reg_length)(dev);
-}
-
-int
rte_eth_dev_get_reg_info(uint8_t port_id, struct rte_dev_reg_info *info)
{
struct rte_eth_dev *dev;
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index ebe6578..2ad7de5 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1322,9 +1322,6 @@ typedef int (*eth_timesync_write_time)(struct rte_eth_dev *dev,
const struct timespec *timestamp);
/**< @internal Function used to get time from the device clock */
-typedef int (*eth_get_reg_length_t)(struct rte_eth_dev *dev);
-/**< @internal Retrieve device register count */
-
typedef int (*eth_get_reg_t)(struct rte_eth_dev *dev,
struct rte_dev_reg_info *info);
/**< @internal Retrieve registers */
@@ -1488,8 +1485,6 @@ struct eth_dev_ops {
/** Query redirection table. */
reta_query_t reta_query;
- eth_get_reg_length_t get_reg_length;
- /**< Get # of registers */
eth_get_reg_t get_reg;
/**< Get registers */
eth_get_eeprom_length_t get_eeprom_length;
@@ -4047,25 +4042,15 @@ int rte_eth_tx_queue_info_get(uint8_t port_id, uint16_t queue_id,
struct rte_eth_txq_info *qinfo);
/**
- * Retrieve number of available registers for access
- *
- * @param port_id
- * The port identifier of the Ethernet device.
- * @return
- * - (>=0) number of registers if successful.
- * - (-ENOTSUP) if hardware doesn't support.
- * - (-ENODEV) if *port_id* invalid.
- * - others depends on the specific operations implementation.
- */
-int rte_eth_dev_get_reg_length(uint8_t port_id);
-
-/**
- * Retrieve device registers and register attributes
+ * Retrieve device registers and register attributes (number of registers and
+ * register size)
*
* @param port_id
* The port identifier of the Ethernet device.
* @param info
- * The template includes buffer for register data and attribute to be filled.
+ * Pointer to rte_dev_reg_info structure to fill in. If info->data is
+ * NULL the function fills in the width and length fields. If non-NULL
+ * the registers are put into the buffer pointed at by the data field.
* @return
* - (0) if successful.
* - (-ENOTSUP) if hardware doesn't support.
diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map
index 97ed0b0..bbc50d1 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -36,7 +36,6 @@ DPDK_2.2 {
rte_eth_dev_get_eeprom_length;
rte_eth_dev_get_mtu;
rte_eth_dev_get_reg_info;
- rte_eth_dev_get_reg_length;
rte_eth_dev_get_vlan_offload;
rte_eth_devices;
rte_eth_dev_info_get;
--
2.7.4
^ permalink raw reply [flat|nested] 17+ messages in thread* [dpdk-dev] [PATCH v6 2/2] examples/ethtool: use rte_eth_dev_get_reg_info for reg params
2016-07-04 11:36 ` [dpdk-dev] [PATCH v6 " Zyta Szpak
@ 2016-07-04 11:36 ` Zyta Szpak
2016-07-04 13:24 ` Remy Horton
2016-07-04 13:24 ` [dpdk-dev] [PATCH v6 1/2] ethdev: remove get_reg_length callback Remy Horton
1 sibling, 1 reply; 17+ messages in thread
From: Zyta Szpak @ 2016-07-04 11:36 UTC (permalink / raw)
To: remy.horton, thomas.monjalon, wenzhuo.lu, helin.zhang,
konstantin.ananyev, jingjing.wu, jerin.jacob, rahul.lakkireddy
Cc: dev, Zyta Szpak
From: Zyta Szpak <zr@semihalf.com>
This change deals with hard-coded register width.
The app was allocating too little space for 64-bit registers
which resulted in memory corruption. This commit resolves
this by getting the number of registers and size of register
by rte_eth_dev_get_reg_info function called first time
with data=NULL.
Signed-off-by: Zyta Szpak <zr@semihalf.com>
---
examples/ethtool/lib/rte_ethtool.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/examples/ethtool/lib/rte_ethtool.c b/examples/ethtool/lib/rte_ethtool.c
index 54391f2..a1f91d4 100644
--- a/examples/ethtool/lib/rte_ethtool.c
+++ b/examples/ethtool/lib/rte_ethtool.c
@@ -46,6 +46,7 @@ int
rte_ethtool_get_drvinfo(uint8_t port_id, struct ethtool_drvinfo *drvinfo)
{
struct rte_eth_dev_info dev_info;
+ struct rte_dev_reg_info reg_info;
int n;
if (drvinfo == NULL)
@@ -65,7 +66,9 @@ rte_ethtool_get_drvinfo(uint8_t port_id, struct ethtool_drvinfo *drvinfo)
dev_info.pci_dev->addr.domain, dev_info.pci_dev->addr.bus,
dev_info.pci_dev->addr.devid, dev_info.pci_dev->addr.function);
- n = rte_eth_dev_get_reg_length(port_id);
+ memset(®_info, 0, sizeof(reg_info));
+ rte_eth_dev_get_reg_info(port_id, ®_info);
+ n = reg_info.length;
if (n > 0)
drvinfo->regdump_len = n;
else
@@ -86,12 +89,16 @@ rte_ethtool_get_drvinfo(uint8_t port_id, struct ethtool_drvinfo *drvinfo)
int
rte_ethtool_get_regs_len(uint8_t port_id)
{
- int count_regs;
+ struct rte_dev_reg_info reg_info;
+ int ret;
+
+ memset(®_info, 0, sizeof(reg_info));
+
+ ret = rte_eth_dev_get_reg_info(port_id, ®_info);
+ if (ret)
+ return ret;
- count_regs = rte_eth_dev_get_reg_length(port_id);
- if (count_regs > 0)
- return count_regs * sizeof(uint32_t);
- return count_regs;
+ return reg_info.length * reg_info.width;
}
int
--
2.7.4
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [dpdk-dev] [PATCH v6 2/2] examples/ethtool: use rte_eth_dev_get_reg_info for reg params
2016-07-04 11:36 ` [dpdk-dev] [PATCH v6 2/2] examples/ethtool: use rte_eth_dev_get_reg_info for reg params Zyta Szpak
@ 2016-07-04 13:24 ` Remy Horton
2016-07-08 17:02 ` Thomas Monjalon
0 siblings, 1 reply; 17+ messages in thread
From: Remy Horton @ 2016-07-04 13:24 UTC (permalink / raw)
To: Zyta Szpak, thomas.monjalon, wenzhuo.lu, helin.zhang,
konstantin.ananyev, jingjing.wu, jerin.jacob, rahul.lakkireddy
Cc: dev
On 04/07/2016 12:36, Zyta Szpak wrote:
> From: Zyta Szpak <zr@semihalf.com>
>
> This change deals with hard-coded register width.
> The app was allocating too little space for 64-bit registers
> which resulted in memory corruption. This commit resolves
> this by getting the number of registers and size of register
> by rte_eth_dev_get_reg_info function called first time
> with data=NULL.
>
> Signed-off-by: Zyta Szpak <zr@semihalf.com>
Acked-by: Remy Horton <remy.horton@intel.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH v6 2/2] examples/ethtool: use rte_eth_dev_get_reg_info for reg params
2016-07-04 13:24 ` Remy Horton
@ 2016-07-08 17:02 ` Thomas Monjalon
0 siblings, 0 replies; 17+ messages in thread
From: Thomas Monjalon @ 2016-07-08 17:02 UTC (permalink / raw)
To: Zyta Szpak
Cc: Remy Horton, wenzhuo.lu, helin.zhang, konstantin.ananyev,
jingjing.wu, jerin.jacob, rahul.lakkireddy, dev
2016-07-04 14:24, Remy Horton:
> On 04/07/2016 12:36, Zyta Szpak wrote:
> > From: Zyta Szpak <zr@semihalf.com>
> >
> > This change deals with hard-coded register width.
> > The app was allocating too little space for 64-bit registers
> > which resulted in memory corruption. This commit resolves
> > this by getting the number of registers and size of register
> > by rte_eth_dev_get_reg_info function called first time
> > with data=NULL.
> >
> > Signed-off-by: Zyta Szpak <zr@semihalf.com>
>
> Acked-by: Remy Horton <remy.horton@intel.com>
Series squashed (to avoid breaking example build) and applied, thanks
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH v6 1/2] ethdev: remove get_reg_length callback
2016-07-04 11:36 ` [dpdk-dev] [PATCH v6 " Zyta Szpak
2016-07-04 11:36 ` [dpdk-dev] [PATCH v6 2/2] examples/ethtool: use rte_eth_dev_get_reg_info for reg params Zyta Szpak
@ 2016-07-04 13:24 ` Remy Horton
1 sibling, 0 replies; 17+ messages in thread
From: Remy Horton @ 2016-07-04 13:24 UTC (permalink / raw)
To: Zyta Szpak, thomas.monjalon, wenzhuo.lu, helin.zhang,
konstantin.ananyev, jingjing.wu, jerin.jacob, rahul.lakkireddy
Cc: dev
On 04/07/2016 12:36, Zyta Szpak wrote:
> From: Zyta Szpak <zr@semihalf.com>
>
> Removes hard-coded assumption that device registers
> are always 32 bits wide. The rte_eth_dev_get_reg_length
> and rte_eth_dev_get_reg_info callbacks did not
> provide register size to the app in any way while is
> needed to allocate correct number of bytes before
> retrieving registers using rte_eth_dev_get_reg.
>
> This commit changes rte_eth_dev_get_reg_info so that
> it can be used to retrieve both the number of registers
> and their width, and removes the now-redundant
> rte_eth_dev_get_reg_length.
>
> Signed-off-by: Zyta Szpak <zr@semihalf.com>
Acked-by: Remy Horton <remy.horton@intel.com>
^ permalink raw reply [flat|nested] 17+ messages in thread