Re: [dpdk-dev] [PATCH 02/28] common/cnxk: add support for communication with atf

[Jerin Jacob <jerinjacobk@gmail.com>]

On Tue, Jun 1, 2021 at 3:12 AM Tomasz Duszynski <tduszynski@marvell.com> wrote:
>
> Messages can be exchanged between userspace software and firmware
> via set of two dedicated registers, namely scratch1 and scratch0.
>
> scratch1 acts as a command register i.e message is sent to firmware,
> while scratch0 holds response to previously sent message.


Observed following build issue with gcc 11.

FAILED: drivers/libtmp_rte_common_cnxk.a.p/common_cnxk_roc_bphy_cgx.c.o
ccache gcc -Idrivers/libtmp_rte_common_cnxk.a.p -Idrivers -I../drivers
-Idrivers/common/cnxk -I../drivers/common/cnxk -Idrivers/bus/pci
-I../drivers/bus/pci -I. -I.. -Iconfig -I../config -Ilib/eal/include
-I../lib/eal/include -Ilib/eal/linu
x/include -I../lib/eal/linux/include -Ilib/eal/x86/include
-I../lib/eal/x86/include -Ilib/eal/common -I../lib/eal/common
-Ilib/eal -I../lib/eal -Ilib/kvargs -I../lib/kvargs -Ilib/metrics
-I../lib/metrics -Ilib/telemetry -I../lib/telemetry -
Ilib/pci -I../lib/pci -I../drivers/bus/pci/linux -Ilib/mbuf
-I../lib/mbuf -Ilib/mempool -I../lib/mempool -Ilib/ring -I../lib/ring
-fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch
-Werror -O2 -g -include rte_config.h -We
xtra -Wcast-qual -Wdeprecated -Wformat -Wformat-nonliteral
-Wformat-security -Wmissing-declarations -Wmissing-prototypes
-Wnested-externs -Wold-style-definition -Wpointer-arith -Wsign-compare
-Wstrict-prototypes -Wundef -Wwrite-strings -Wno
-address-of-packed-member -Wno-packed-not-aligned
-Wno-missing-field-initializers -Wno-zero-length-bounds -D_GNU_SOURCE
-fPIC -march=native -DALLOW_EXPERIMENTAL_API -DALLOW_INTERNAL_API
-Wno-format-truncation -DRTE_LOG_DEFAULT_LOGTYPE=pmd.c
ommon.cnxk -MD -MQ
drivers/libtmp_rte_common_cnxk.a.p/common_cnxk_roc_bphy_cgx.c.o -MF
drivers/libtmp_rte_common_cnxk.a.p/common_cnxk_roc_bphy_cgx.c.o.d -o
drivers/libtmp_rte_common_cnxk.a.p/common_cnxk_roc_bphy_cgx.c.o -c
../drivers/common
/cnxk/roc_bphy_cgx.c
../drivers/common/cnxk/roc_bphy_cgx.c:107:1: error:
‘roc_bphy_cgx_intf_req’ defined but not used [-Werror=unused-function]
  107 | roc_bphy_cgx_intf_req(struct roc_bphy_cgx *roc_cgx, unsigned int lmac,
      | ^~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors


>
> Signed-off-by: Tomasz Duszynski <tduszynski@marvell.com>
> Signed-off-by: Jakub Palider <jpalider@marvell.com>
> ---
>  drivers/common/cnxk/roc_bphy_cgx.c      | 145 ++++++++++++++++++++++++
>  drivers/common/cnxk/roc_bphy_cgx.h      |   4 +
>  drivers/common/cnxk/roc_bphy_cgx_priv.h |  54 +++++++++
>  drivers/common/cnxk/roc_priv.h          |   3 +
>  4 files changed, 206 insertions(+)
>  create mode 100644 drivers/common/cnxk/roc_bphy_cgx_priv.h
>
> diff --git a/drivers/common/cnxk/roc_bphy_cgx.c b/drivers/common/cnxk/roc_bphy_cgx.c
> index 029d4102e..5048a90de 100644
> --- a/drivers/common/cnxk/roc_bphy_cgx.c
> +++ b/drivers/common/cnxk/roc_bphy_cgx.c
> @@ -2,8 +2,13 @@
>   * Copyright(C) 2021 Marvell.
>   */
>
> +#include <pthread.h>
> +
>  #include "roc_api.h"
> +#include "roc_priv.h"
>
> +#define CGX_CMRX_INT                  0x40
> +#define CGX_CMRX_INT_OVERFLW          BIT_ULL(1)
>  /*
>   * CN10K stores number of lmacs in 4 bit filed
>   * in contraty to CN9K which uses only 3 bits.
> @@ -15,6 +20,8 @@
>   */
>  #define CGX_CMRX_RX_LMACS      0x128
>  #define CGX_CMRX_RX_LMACS_LMACS GENMASK_ULL(3, 0)
> +#define CGX_CMRX_SCRATCH0      0x1050
> +#define CGX_CMRX_SCRATCH1      0x1058
>
>  static uint64_t
>  roc_bphy_cgx_read(struct roc_bphy_cgx *roc_cgx, uint64_t lmac, uint64_t offset)
> @@ -25,6 +32,137 @@ roc_bphy_cgx_read(struct roc_bphy_cgx *roc_cgx, uint64_t lmac, uint64_t offset)
>         return plt_read64(base + (lmac << shift) + offset);
>  }
>
> +static void
> +roc_bphy_cgx_write(struct roc_bphy_cgx *roc_cgx, uint64_t lmac, uint64_t offset,
> +                  uint64_t value)
> +{
> +       int shift = roc_model_is_cn10k() ? 20 : 18;
> +       uint64_t base = (uint64_t)roc_cgx->bar0_va;
> +
> +       plt_write64(value, base + (lmac << shift) + offset);
> +}
> +
> +static void
> +roc_bphy_cgx_ack(struct roc_bphy_cgx *roc_cgx, unsigned int lmac,
> +                uint64_t *scr0)
> +{
> +       uint64_t val;
> +
> +       /* clear interrupt */
> +       val = roc_bphy_cgx_read(roc_cgx, lmac, CGX_CMRX_INT);
> +       val |= FIELD_PREP(CGX_CMRX_INT_OVERFLW, 1);
> +       roc_bphy_cgx_write(roc_cgx, lmac, CGX_CMRX_INT, val);
> +
> +       /* ack fw response */
> +       *scr0 &= ~SCR0_ETH_EVT_STS_S_ACK;
> +       roc_bphy_cgx_write(roc_cgx, lmac, CGX_CMRX_SCRATCH0, *scr0);
> +}
> +
> +static int
> +roc_bphy_cgx_wait_for_ownership(struct roc_bphy_cgx *roc_cgx, unsigned int lmac)
> +{
> +       uint64_t scr0, scr1;
> +       int tries = 5000;

Maybe #define for 5000.

> +
> +       do {
> +               scr0 = roc_bphy_cgx_read(roc_cgx, lmac, CGX_CMRX_SCRATCH0);
> +               scr1 = roc_bphy_cgx_read(roc_cgx, lmac, CGX_CMRX_SCRATCH1);
> +
> +               if (FIELD_GET(SCR1_OWN_STATUS, scr1) == ETH_OWN_NON_SECURE_SW &&
> +                   FIELD_GET(SCR0_ETH_EVT_STS_S_ACK, scr0) == 0)
> +                       break;
> +
> +               /* clear async events if any */
> +               if (FIELD_GET(SCR0_ETH_EVT_STS_S_EVT_TYPE, scr0) == ETH_EVT_ASYNC &&
> +                   FIELD_GET(SCR0_ETH_EVT_STS_S_ACK, scr0))
> +                       roc_bphy_cgx_ack(roc_cgx, lmac, &scr0);
> +
> +               plt_delay_ms(1);
> +       } while (--tries);
> +
> +       return tries ? 0 : -ETIMEDOUT;
> +}
> +
> +static int
> +roc_bphy_cgx_wait_for_ack(struct roc_bphy_cgx *roc_cgx, unsigned int lmac)
> +{
> +       uint64_t scr0, scr1;
> +       int tries = 5000;

Maybe #define for 5000.


> +
> +       do {
> +               scr0 = roc_bphy_cgx_read(roc_cgx, lmac, CGX_CMRX_SCRATCH0);
> +               scr1 = roc_bphy_cgx_read(roc_cgx, lmac, CGX_CMRX_SCRATCH1);
> +
> +               if (FIELD_GET(SCR1_OWN_STATUS, scr1) == ETH_OWN_NON_SECURE_SW &&
> +                   FIELD_GET(SCR0_ETH_EVT_STS_S_ACK, scr0))
> +                       break;
> +
> +               plt_delay_ms(1);
> +       } while (--tries);
> +
> +       return tries ? 0 : -ETIMEDOUT;
> +}
> +
> +static int
> +roc_bphy_cgx_intf_req(struct roc_bphy_cgx *roc_cgx, unsigned int lmac,
> +                     uint64_t scr1, uint64_t *scr0)
> +{
> +       uint8_t cmd_id = FIELD_GET(SCR1_ETH_CMD_ID, scr1);
> +       int ret;
> +
> +       pthread_mutex_lock(&roc_cgx->lock);
> +
> +       /* wait for ownership */
> +       ret = roc_bphy_cgx_wait_for_ownership(roc_cgx, lmac);
> +       if (ret) {
> +               plt_err("timed out waiting for ownership");
> +               goto out;
> +       }
> +
> +       /* write command */
> +       scr1 |= FIELD_PREP(SCR1_OWN_STATUS, ETH_OWN_FIRMWARE);
> +       roc_bphy_cgx_write(roc_cgx, lmac, CGX_CMRX_SCRATCH1, scr1);
> +
> +       /* wait for command ack */
> +       ret = roc_bphy_cgx_wait_for_ack(roc_cgx, lmac);
> +       if (ret) {
> +               plt_err("timed out waiting for response");
> +               goto out;
> +       }
> +
> +       if (cmd_id == ETH_CMD_INTF_SHUTDOWN)
> +               goto out;
> +
> +       *scr0 = roc_bphy_cgx_read(roc_cgx, lmac, CGX_CMRX_SCRATCH0);
> +
> +       if (FIELD_GET(SCR0_ETH_EVT_STS_S_EVT_TYPE, *scr0) != ETH_EVT_CMD_RESP) {
> +               plt_err("received async event instead of cmd resp event");
> +               ret = -EIO;
> +               goto out;
> +       }
> +
> +       if (FIELD_GET(SCR0_ETH_EVT_STS_S_ID, *scr0) != cmd_id) {
> +               plt_err("received resp for cmd %d expected for cmd %d",
> +                       (int)FIELD_GET(SCR0_ETH_EVT_STS_S_ID, *scr0), cmd_id);
> +               ret = -EIO;
> +               goto out;
> +       }
> +
> +       if (FIELD_GET(SCR0_ETH_EVT_STS_S_STAT, *scr0) != ETH_STAT_SUCCESS) {
> +               plt_err("cmd %d failed on cgx%u lmac%u with errcode %d", cmd_id,
> +                       roc_cgx->id, lmac,
> +                       (int)FIELD_GET(SCR0_ETH_LNK_STS_S_ERR_TYPE, *scr0));
> +               ret = -EIO;
> +       }
> +
> +out:
> +       roc_bphy_cgx_ack(roc_cgx, lmac, scr0);
> +
> +       pthread_mutex_unlock(&roc_cgx->lock);
> +
> +       return ret;
> +}
> +
>  static unsigned int
>  roc_bphy_cgx_dev_id(struct roc_bphy_cgx *roc_cgx)
>  {
> @@ -38,10 +176,15 @@ int
>  roc_bphy_cgx_dev_init(struct roc_bphy_cgx *roc_cgx)
>  {
>         uint64_t val;
> +       int ret;
>
>         if (!roc_cgx || !roc_cgx->bar0_va || !roc_cgx->bar0_pa)
>                 return -EINVAL;
>
> +       ret = pthread_mutex_init(&roc_cgx->lock, NULL);
> +       if (ret)
> +               return ret;
> +
>         val = roc_bphy_cgx_read(roc_cgx, 0, CGX_CMRX_RX_LMACS);
>         val = FIELD_GET(CGX_CMRX_RX_LMACS_LMACS, val);
>         if (roc_model_is_cn9k())
> @@ -58,5 +201,7 @@ roc_bphy_cgx_dev_fini(struct roc_bphy_cgx *roc_cgx)
>         if (!roc_cgx)
>                 return -EINVAL;
>
> +       pthread_mutex_destroy(&roc_cgx->lock);
> +
>         return 0;
>  }
> diff --git a/drivers/common/cnxk/roc_bphy_cgx.h b/drivers/common/cnxk/roc_bphy_cgx.h
> index aac2c262c..37b5c2742 100644
> --- a/drivers/common/cnxk/roc_bphy_cgx.h
> +++ b/drivers/common/cnxk/roc_bphy_cgx.h
> @@ -5,6 +5,8 @@
>  #ifndef _ROC_BPHY_CGX_H_
>  #define _ROC_BPHY_CGX_H_
>
> +#include <pthread.h>
> +
>  #include "roc_api.h"
>
>  struct roc_bphy_cgx {
> @@ -12,6 +14,8 @@ struct roc_bphy_cgx {
>         void *bar0_va;
>         uint64_t lmac_bmap;
>         unsigned int id;
> +       /* serialize access to the whole structure */
> +       pthread_mutex_t lock;
>  } __plt_cache_aligned;
>
>  __roc_api int roc_bphy_cgx_dev_init(struct roc_bphy_cgx *roc_cgx);
> diff --git a/drivers/common/cnxk/roc_bphy_cgx_priv.h b/drivers/common/cnxk/roc_bphy_cgx_priv.h
> new file mode 100644
> index 000000000..42d0bce7a
> --- /dev/null
> +++ b/drivers/common/cnxk/roc_bphy_cgx_priv.h
> @@ -0,0 +1,54 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(C) 2021 Marvell.
> + */
> +
> +#ifndef _ROC_BPHY_CGX_PRIV_H_
> +#define _ROC_BPHY_CGX_PRIV_H_
> +
> +/* REQUEST ID types. Input to firmware */
> +enum eth_cmd_id {
> +       ETH_CMD_INTF_SHUTDOWN = 12,
> +};
> +
> +/* event types - cause of interrupt */
> +enum eth_evt_type {
> +       ETH_EVT_ASYNC,
> +       ETH_EVT_CMD_RESP,
> +};
> +
> +enum eth_stat {
> +       ETH_STAT_SUCCESS,
> +       ETH_STAT_FAIL,
> +};
> +
> +enum eth_cmd_own {
> +       /* default ownership with kernel/uefi/u-boot */
> +       ETH_OWN_NON_SECURE_SW,
> +       /* set by kernel/uefi/u-boot after posting a new request to ATF */
> +       ETH_OWN_FIRMWARE,
> +};
> +
> +/* scratchx(0) CSR used for ATF->non-secure SW communication.
> + * This acts as the status register
> + * Provides details on command ack/status, link status, error details
> + */
> +
> +/* struct eth_evt_sts_s */
> +#define SCR0_ETH_EVT_STS_S_ACK     BIT_ULL(0)
> +#define SCR0_ETH_EVT_STS_S_EVT_TYPE BIT_ULL(1)
> +#define SCR0_ETH_EVT_STS_S_STAT            BIT_ULL(2)
> +#define SCR0_ETH_EVT_STS_S_ID      GENMASK_ULL(8, 3)
> +
> +/* struct eth_lnk_sts_s */
> +#define SCR0_ETH_LNK_STS_S_ERR_TYPE    GENMASK_ULL(24, 15)
> +
> +/* scratchx(1) CSR used for non-secure SW->ATF communication
> + * This CSR acts as a command register
> + */
> +
> +/* struct eth_cmd */
> +#define SCR1_ETH_CMD_ID GENMASK_ULL(7, 2)
> +
> +#define SCR1_OWN_STATUS GENMASK_ULL(1, 0)
> +
> +#endif /* _ROC_BPHY_CGX_PRIV_H_ */
> diff --git a/drivers/common/cnxk/roc_priv.h b/drivers/common/cnxk/roc_priv.h
> index 5e7564ce5..feca732a9 100644
> --- a/drivers/common/cnxk/roc_priv.h
> +++ b/drivers/common/cnxk/roc_priv.h
> @@ -32,4 +32,7 @@
>  /* TIM */
>  #include "roc_tim_priv.h"
>
> +/* BPHY CGX */
> +#include "roc_bphy_cgx_priv.h"
> +
>  #endif /* _ROC_PRIV_H_ */
> --
> 2.25.1
>