[2/3] net/ark: remove RQ pacing firmware from PMD
Checks
Commit Message
features and function have been removed from FPGA firmware
Signed-off-by: Ed Czeck <ed.czeck@atomicrules.com>
---
drivers/net/ark/ark_ethdev.c | 62 ++++++++------------------------
drivers/net/ark/ark_global.h | 3 --
drivers/net/ark/ark_rqp.c | 70 ------------------------------------
drivers/net/ark/ark_rqp.h | 58 ------------------------------
drivers/net/ark/meson.build | 1 -
5 files changed, 15 insertions(+), 179 deletions(-)
delete mode 100644 drivers/net/ark/ark_rqp.c
delete mode 100644 drivers/net/ark/ark_rqp.h
Comments
On 10/5/2023 9:52 PM, Ed Czeck wrote:
> features and function have been removed from FPGA firmware
>
I am always a little confused how you manage the deployment, if a
customer requires RQ pacing, how you manage it, at least should it be
documented in driver documentation that RQ pacing supported before
v23.11, or something like that?
If this doesn't make sense for your deployment model, scratch it, this
is just a reminder if it is useful.
> Signed-off-by: Ed Czeck <ed.czeck@atomicrules.com>
> ---
> drivers/net/ark/ark_ethdev.c | 62 ++++++++------------------------
> drivers/net/ark/ark_global.h | 3 --
> drivers/net/ark/ark_rqp.c | 70 ------------------------------------
> drivers/net/ark/ark_rqp.h | 58 ------------------------------
> drivers/net/ark/meson.build | 1 -
> 5 files changed, 15 insertions(+), 179 deletions(-)
> delete mode 100644 drivers/net/ark/ark_rqp.c
> delete mode 100644 drivers/net/ark/ark_rqp.h
>
> diff --git a/drivers/net/ark/ark_ethdev.c b/drivers/net/ark/ark_ethdev.c
> index 90d3c8abe6..306121ba31 100644
> --- a/drivers/net/ark/ark_ethdev.c
> +++ b/drivers/net/ark/ark_ethdev.c
> @@ -17,7 +17,6 @@
> #include "ark_mpu.h"
> #include "ark_ddm.h"
> #include "ark_udm.h"
> -#include "ark_rqp.h"
> #include "ark_pktdir.h"
> #include "ark_pktgen.h"
> #include "ark_pktchkr.h"
> @@ -107,36 +106,32 @@ static const struct rte_pci_id pci_id_ark_map[] = {
> * This structure is used to statically define the capabilities
> * of supported devices.
> * Capabilities:
> - * rqpacing -
> - * Some HW variants require that PCIe read-requests be correctly throttled.
> - * This is called "rqpacing" and has to do with credit and flow control
> - * on certain Arkville implementations.
> + * isvf -- defined for function id that are virtual
> */
> struct ark_caps {
> - bool rqpacing;
> bool isvf;
> };
> struct ark_dev_caps {
> uint32_t device_id;
> struct ark_caps caps;
> };
> -#define SET_DEV_CAPS(id, rqp, vf) \
> - {id, {.rqpacing = rqp, .isvf = vf} }
> +#define SET_DEV_CAPS(id, vf) \
> + {id, {.isvf = vf} }
>
> static const struct ark_dev_caps
> ark_device_caps[] = {
> - SET_DEV_CAPS(0x100d, true, false),
> - SET_DEV_CAPS(0x100e, true, false),
> - SET_DEV_CAPS(0x100f, true, false),
> - SET_DEV_CAPS(0x1010, false, false),
> - SET_DEV_CAPS(0x1017, true, false),
> - SET_DEV_CAPS(0x1018, true, false),
> - SET_DEV_CAPS(0x1019, true, false),
> - SET_DEV_CAPS(0x101a, true, false),
> - SET_DEV_CAPS(0x101b, true, false),
> - SET_DEV_CAPS(0x101c, true, true),
> - SET_DEV_CAPS(0x101e, false, false),
> - SET_DEV_CAPS(0x101f, false, false),
> + SET_DEV_CAPS(0x100d, false),
> + SET_DEV_CAPS(0x100e, false),
> + SET_DEV_CAPS(0x100f, false),
> + SET_DEV_CAPS(0x1010, false),
> + SET_DEV_CAPS(0x1017, false),
> + SET_DEV_CAPS(0x1018, false),
> + SET_DEV_CAPS(0x1019, false),
> + SET_DEV_CAPS(0x101a, false),
> + SET_DEV_CAPS(0x101b, false),
> + SET_DEV_CAPS(0x101c, true),
> + SET_DEV_CAPS(0x101e, false),
> + SET_DEV_CAPS(0x101f, false),
> {.device_id = 0,}
> };
>
> @@ -301,9 +296,6 @@ eth_ark_dev_init(struct rte_eth_dev *dev)
> int port_count = 1;
> int p;
> uint16_t num_queues;
> - bool rqpacing = false;
> -
> - ark->eth_dev = dev;
>
Above "ark->eth_dev" assignment doesn't look directly related with RQ
pacing, I just want to double check if it is removed intentionally?
On Tue, Oct 10, 2023 at 9:51 AM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
>
> On 10/5/2023 9:52 PM, Ed Czeck wrote:
> > features and function have been removed from FPGA firmware
> >
>
> I am always a little confused how you manage the deployment, if a
> customer requires RQ pacing, how you manage it, at least should it be
> documented in driver documentation that RQ pacing supported before
> v23.11, or something like that?
> If this doesn't make sense for your deployment model, scratch it, this
> is just a reminder if it is useful.
Our deployment needs to balance the DPDK release, our FPGA firmware, our
(not yet
published) DPDKpatches and external FPGA-IP firmware from AMD (Xilinx) and
Intel
(Altera). We have safety code to ensure that these fall into a valid
alignment. We also
try to maintain SW/FPGA compatibility and evolve without breaking things
unnecessarily.
Our releases follow DPDK's and we update other tools as they are released.
For RQ pacing, it was an internal feature needed for older Xilinx PCIE IP,
with a
narrow exposure via our PMD. The Xilinx IP no longer requires this module,
our
firmware no longer includes it, and the PMD can drop. It was not user
controllable
nor an advertised feature.
>
>
> > Signed-off-by: Ed Czeck <ed.czeck@atomicrules.com>
> > ---
> > drivers/net/ark/ark_ethdev.c | 62 ++++++++------------------------
> > drivers/net/ark/ark_global.h | 3 --
> > drivers/net/ark/ark_rqp.c | 70 ------------------------------------
> > drivers/net/ark/ark_rqp.h | 58 ------------------------------
> > drivers/net/ark/meson.build | 1 -
> > 5 files changed, 15 insertions(+), 179 deletions(-)
> > delete mode 100644 drivers/net/ark/ark_rqp.c
> > delete mode 100644 drivers/net/ark/ark_rqp.h
> >
> > diff --git a/drivers/net/ark/ark_ethdev.c b/drivers/net/ark/ark_ethdev.c
> > index 90d3c8abe6..306121ba31 100644
> > --- a/drivers/net/ark/ark_ethdev.c
> > +++ b/drivers/net/ark/ark_ethdev.c
> > @@ -17,7 +17,6 @@
> > #include "ark_mpu.h"
> > #include "ark_ddm.h"
> > #include "ark_udm.h"
> > -#include "ark_rqp.h"
> > #include "ark_pktdir.h"
> > #include "ark_pktgen.h"
> > #include "ark_pktchkr.h"
> > @@ -107,36 +106,32 @@ static const struct rte_pci_id pci_id_ark_map[] =
{
> > * This structure is used to statically define the capabilities
> > * of supported devices.
> > * Capabilities:
> > - * rqpacing -
> > - * Some HW variants require that PCIe read-requests be correctly
throttled.
> > - * This is called "rqpacing" and has to do with credit and flow control
> > - * on certain Arkville implementations.
> > + * isvf -- defined for function id that are virtual
> > */
> > struct ark_caps {
> > - bool rqpacing;
> > bool isvf;
> > };
> > struct ark_dev_caps {
> > uint32_t device_id;
> > struct ark_caps caps;
> > };
> > -#define SET_DEV_CAPS(id, rqp, vf) \
> > - {id, {.rqpacing = rqp, .isvf = vf} }
> > +#define SET_DEV_CAPS(id, vf) \
> > + {id, {.isvf = vf} }
> >
> > static const struct ark_dev_caps
> > ark_device_caps[] = {
> > - SET_DEV_CAPS(0x100d, true, false),
> > - SET_DEV_CAPS(0x100e, true, false),
> > - SET_DEV_CAPS(0x100f, true, false),
> > - SET_DEV_CAPS(0x1010, false, false),
> > - SET_DEV_CAPS(0x1017, true, false),
> > - SET_DEV_CAPS(0x1018, true, false),
> > - SET_DEV_CAPS(0x1019, true, false),
> > - SET_DEV_CAPS(0x101a, true, false),
> > - SET_DEV_CAPS(0x101b, true, false),
> > - SET_DEV_CAPS(0x101c, true, true),
> > - SET_DEV_CAPS(0x101e, false, false),
> > - SET_DEV_CAPS(0x101f, false, false),
> > + SET_DEV_CAPS(0x100d, false),
> > + SET_DEV_CAPS(0x100e, false),
> > + SET_DEV_CAPS(0x100f, false),
> > + SET_DEV_CAPS(0x1010, false),
> > + SET_DEV_CAPS(0x1017, false),
> > + SET_DEV_CAPS(0x1018, false),
> > + SET_DEV_CAPS(0x1019, false),
> > + SET_DEV_CAPS(0x101a, false),
> > + SET_DEV_CAPS(0x101b, false),
> > + SET_DEV_CAPS(0x101c, true),
> > + SET_DEV_CAPS(0x101e, false),
> > + SET_DEV_CAPS(0x101f, false),
> > {.device_id = 0,}
> > };
> >
> > @@ -301,9 +296,6 @@ eth_ark_dev_init(struct rte_eth_dev *dev)
> > int port_count = 1;
> > int p;
> > uint16_t num_queues;
> > - bool rqpacing = false;
> > -
> > - ark->eth_dev = dev;
> >
>
> Above "ark->eth_dev" assignment doesn't look directly related with RQ
> pacing, I just want to double check if it is removed intentionally?
This change is in error. Thanks for catching it. New patch to follow.
>
>
@@ -17,7 +17,6 @@
#include "ark_mpu.h"
#include "ark_ddm.h"
#include "ark_udm.h"
-#include "ark_rqp.h"
#include "ark_pktdir.h"
#include "ark_pktgen.h"
#include "ark_pktchkr.h"
@@ -107,36 +106,32 @@ static const struct rte_pci_id pci_id_ark_map[] = {
* This structure is used to statically define the capabilities
* of supported devices.
* Capabilities:
- * rqpacing -
- * Some HW variants require that PCIe read-requests be correctly throttled.
- * This is called "rqpacing" and has to do with credit and flow control
- * on certain Arkville implementations.
+ * isvf -- defined for function id that are virtual
*/
struct ark_caps {
- bool rqpacing;
bool isvf;
};
struct ark_dev_caps {
uint32_t device_id;
struct ark_caps caps;
};
-#define SET_DEV_CAPS(id, rqp, vf) \
- {id, {.rqpacing = rqp, .isvf = vf} }
+#define SET_DEV_CAPS(id, vf) \
+ {id, {.isvf = vf} }
static const struct ark_dev_caps
ark_device_caps[] = {
- SET_DEV_CAPS(0x100d, true, false),
- SET_DEV_CAPS(0x100e, true, false),
- SET_DEV_CAPS(0x100f, true, false),
- SET_DEV_CAPS(0x1010, false, false),
- SET_DEV_CAPS(0x1017, true, false),
- SET_DEV_CAPS(0x1018, true, false),
- SET_DEV_CAPS(0x1019, true, false),
- SET_DEV_CAPS(0x101a, true, false),
- SET_DEV_CAPS(0x101b, true, false),
- SET_DEV_CAPS(0x101c, true, true),
- SET_DEV_CAPS(0x101e, false, false),
- SET_DEV_CAPS(0x101f, false, false),
+ SET_DEV_CAPS(0x100d, false),
+ SET_DEV_CAPS(0x100e, false),
+ SET_DEV_CAPS(0x100f, false),
+ SET_DEV_CAPS(0x1010, false),
+ SET_DEV_CAPS(0x1017, false),
+ SET_DEV_CAPS(0x1018, false),
+ SET_DEV_CAPS(0x1019, false),
+ SET_DEV_CAPS(0x101a, false),
+ SET_DEV_CAPS(0x101b, false),
+ SET_DEV_CAPS(0x101c, true),
+ SET_DEV_CAPS(0x101e, false),
+ SET_DEV_CAPS(0x101f, false),
{.device_id = 0,}
};
@@ -301,9 +296,6 @@ eth_ark_dev_init(struct rte_eth_dev *dev)
int port_count = 1;
int p;
uint16_t num_queues;
- bool rqpacing = false;
-
- ark->eth_dev = dev;
ARK_PMD_LOG(DEBUG, "\n");
@@ -319,7 +311,6 @@ eth_ark_dev_init(struct rte_eth_dev *dev)
p = 0;
while (ark_device_caps[p].device_id != 0) {
if (pci_dev->id.device_id == ark_device_caps[p].device_id) {
- rqpacing = ark_device_caps[p].caps.rqpacing;
ark->isvf = ark_device_caps[p].caps.isvf;
break;
}
@@ -344,12 +335,6 @@ eth_ark_dev_init(struct rte_eth_dev *dev)
ark->pktgen.v = (void *)&ark->bar0[ARK_PKTGEN_BASE];
ark->pktchkr.v = (void *)&ark->bar0[ARK_PKTCHKR_BASE];
- if (rqpacing) {
- ark->rqpacing =
- (struct ark_rqpace_t *)(ark->bar0 + ARK_RCPACING_BASE);
- } else {
- ark->rqpacing = NULL;
- }
ark->started = 0;
ark->pkt_dir_v = ARK_PKT_DIR_INIT_VAL;
@@ -368,17 +353,6 @@ eth_ark_dev_init(struct rte_eth_dev *dev)
ark->sysctrl.t32[4], __func__);
return -1;
}
- if (ark->sysctrl.t32[3] != 0) {
- if (ark->rqpacing) {
- if (ark_rqp_lasped(ark->rqpacing)) {
- ARK_PMD_LOG(ERR, "Arkville Evaluation System - "
- "Timer has Expired\n");
- return -1;
- }
- ARK_PMD_LOG(WARNING, "Arkville Evaluation System - "
- "Timer is Running\n");
- }
- }
ARK_PMD_LOG(DEBUG,
"HW Sanity test has PASSED, expected constant"
@@ -550,9 +524,6 @@ ark_config_device(struct rte_eth_dev *dev)
mpu = RTE_PTR_ADD(mpu, ARK_MPU_QOFFSET);
}
- if (!ark->isvf && ark->rqpacing)
- ark_rqp_stats_reset(ark->rqpacing);
-
return 0;
}
@@ -709,9 +680,6 @@ eth_ark_dev_close(struct rte_eth_dev *dev)
/*
* This should only be called once for the device during shutdown
*/
- if (ark->rqpacing)
- ark_rqp_dump(ark->rqpacing);
-
/* return to power-on state */
if (ark->pd)
ark_pktdir_setup(ark->pd, ARK_PKT_DIR_INIT_VAL);
@@ -32,7 +32,6 @@
#define ARK_CMAC_BASE 0x80000
#define ARK_PKTDIR_BASE 0xa0000
#define ARK_PKTCHKR_BASE 0x90000
-#define ARK_RCPACING_BASE 0xb0000
#define ARK_EXTERNAL_BASE 0x100000
#define ARK_MPU_QOFFSET 0x00100
#define ARK_MAX_PORTS RTE_MAX_ETHPORTS
@@ -150,8 +149,6 @@ struct ark_adapter {
int started;
uint16_t rx_queues;
uint16_t tx_queues;
-
- struct ark_rqpace_t *rqpacing;
};
typedef uint32_t *ark_t;
deleted file mode 100644
@@ -1,70 +0,0 @@
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright (c) 2015-2018 Atomic Rules LLC
- */
-
-#include <unistd.h>
-
-#include "ark_rqp.h"
-#include "ark_logs.h"
-
-/* ************************************************************************* */
-void
-ark_rqp_stats_reset(struct ark_rqpace_t *rqp)
-{
- rqp->stats_clear = 1;
- /* POR 992 */
- /* rqp->cpld_max = 992; */
- /* POR 64 */
- /* rqp->cplh_max = 64; */
-}
-
-/* ************************************************************************* */
-void
-ark_rqp_dump(struct ark_rqpace_t *rqp)
-{
- if (rqp->err_count_other || rqp->cmpl_errors)
- ARK_PMD_LOG(ERR,
- "RQP Errors noted: ctrl: %d cplh_hmax %d cpld_max %d"
- ARK_SU32
- ARK_SU32
- ARK_SU32 "\n",
- rqp->ctrl, rqp->cplh_max, rqp->cpld_max,
- "Error Count", rqp->err_cnt,
- "Error General", rqp->err_count_other,
- "Cmpl Errors", rqp->cmpl_errors);
-
- ARK_PMD_LOG(INFO, "RQP Dump: ctrl: %d cplh_hmax %d cpld_max %d"
- ARK_SU32
- ARK_SU32 ARK_SU32 ARK_SU32 ARK_SU32 ARK_SU32 ARK_SU32
- ARK_SU32 ARK_SU32 ARK_SU32 ARK_SU32 ARK_SU32 ARK_SU32
- ARK_SU32 ARK_SU32 ARK_SU32
- ARK_SU32 ARK_SU32 ARK_SU32 ARK_SU32 ARK_SU32 "\n",
- rqp->ctrl, rqp->cplh_max, rqp->cpld_max,
- "Error Count", rqp->err_cnt,
- "Error General", rqp->err_count_other,
- "stall_pS", rqp->stall_ps,
- "stall_pS Min", rqp->stall_ps_min,
- "stall_pS Max", rqp->stall_ps_max,
- "req_pS", rqp->req_ps,
- "req_pS Min", rqp->req_ps_min,
- "req_pS Max", rqp->req_ps_max,
- "req_dWPS", rqp->req_dw_ps,
- "req_dWPS Min", rqp->req_dw_ps_min,
- "req_dWPS Max", rqp->req_dw_ps_max,
- "cpl_pS", rqp->cpl_ps,
- "cpl_pS Min", rqp->cpl_ps_min,
- "cpl_pS Max", rqp->cpl_ps_max,
- "cpl_dWPS", rqp->cpl_dw_ps,
- "cpl_dWPS Min", rqp->cpl_dw_ps_min,
- "cpl_dWPS Max", rqp->cpl_dw_ps_max,
- "cplh pending", rqp->cplh_pending,
- "cpld pending", rqp->cpld_pending,
- "cplh pending max", rqp->cplh_pending_max,
- "cpld pending max", rqp->cpld_pending_max);
-}
-
-int
-ark_rqp_lasped(struct ark_rqpace_t *rqp)
-{
- return rqp->lasped;
-}
deleted file mode 100644
@@ -1,58 +0,0 @@
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright (c) 2015-2018 Atomic Rules LLC
- */
-
-#ifndef _ARK_RQP_H_
-#define _ARK_RQP_H_
-
-#include <stdint.h>
-
-#include <rte_memory.h>
-
-/* The RQP or ReQuest Pacer is an internal Arkville hardware module
- * which limits the PCIE data flow to insure correct operation for the
- * particular hardware PCIE endpoint.
- * This module is *not* intended for end-user manipulation, hence
- * there is minimal documentation.
- */
-
-/*
- * RQ Pacing core hardware structure
- * This is an overlay structures to a memory mapped FPGA device. These
- * structs will never be instantiated in ram memory
- */
-struct ark_rqpace_t {
- volatile uint32_t ctrl;
- volatile uint32_t stats_clear;
- volatile uint32_t cplh_max;
- volatile uint32_t cpld_max;
- volatile uint32_t err_cnt;
- volatile uint32_t stall_ps;
- volatile uint32_t stall_ps_min;
- volatile uint32_t stall_ps_max;
- volatile uint32_t req_ps;
- volatile uint32_t req_ps_min;
- volatile uint32_t req_ps_max;
- volatile uint32_t req_dw_ps;
- volatile uint32_t req_dw_ps_min;
- volatile uint32_t req_dw_ps_max;
- volatile uint32_t cpl_ps;
- volatile uint32_t cpl_ps_min;
- volatile uint32_t cpl_ps_max;
- volatile uint32_t cpl_dw_ps;
- volatile uint32_t cpl_dw_ps_min;
- volatile uint32_t cpl_dw_ps_max;
- volatile uint32_t cplh_pending;
- volatile uint32_t cpld_pending;
- volatile uint32_t cplh_pending_max;
- volatile uint32_t cpld_pending_max;
- volatile uint32_t err_count_other;
- char eval[4];
- volatile int32_t lasped;
- volatile uint32_t cmpl_errors;
-};
-
-void ark_rqp_dump(struct ark_rqpace_t *rqp);
-void ark_rqp_stats_reset(struct ark_rqpace_t *rqp);
-int ark_rqp_lasped(struct ark_rqpace_t *rqp);
-#endif
@@ -16,6 +16,5 @@ sources = files(
'ark_pktchkr.c',
'ark_pktdir.c',
'ark_pktgen.c',
- 'ark_rqp.c',
'ark_udm.c',
)