Re: [dpdk-dev] [PATCH v6 2/2] common/octeontx2: add polling based response mbox message

[Jerin Jacob <jerinjacobk@gmail.com>]

On Fri, Dec 20, 2019 at 12:27 PM Sunil Kumar Kori <skori@marvell.com> wrote:
>
> Currently otx2_mbox_get_rsp_xxx get response once AF driver
> interrupts after completion. But this funciton will get into

s/funciton/function

> deadlock if called in another interrupt context.
>
> To avoid it, implemented another version of this function which polls
> on dedicated memory for a given timeout.
>
> Also after clearing interrupt, there could UP messages available for
> processing. So irq handler must check mbox messages.
>
> Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
> Signed-off-by: Harman Kalra <hkalra@marvell.com>

With the above change:
Acked-by: Jerin Jacob <jerinj@marvell.com>

@Thomas Monjalon  Since this patch has a dependency on an eal
patch(1/2  eal: add API to check if its interrupt context), I am
delegating this patch to EAL maintainer.



> ---
> v6:
>  - Removed unnecessary code.
> v5:
>  - Fix shared library compilation error
> v4:
>  - used rte_io_rmb instead of rte_rmb in mbox_poll.
> v3:
>  - Remove experimental tag as API is defined static.
>  - Merge all changes to single patch.
> v2:
>  - Included Makefile and meson build changes.
>  - Rebased patch on 19.11-rc4
>
>  drivers/common/octeontx2/otx2_dev.c  | 41 +++++++++++-----------
>  drivers/common/octeontx2/otx2_mbox.c | 51 ++++++++++++++++++++++++----
>  drivers/common/octeontx2/otx2_mbox.h |  5 +--
>  3 files changed, 70 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/common/octeontx2/otx2_dev.c b/drivers/common/octeontx2/otx2_dev.c
> index 0fc799e4a..d61c712fa 100644
> --- a/drivers/common/octeontx2/otx2_dev.c
> +++ b/drivers/common/octeontx2/otx2_dev.c
> @@ -577,17 +577,16 @@ otx2_pf_vf_mbox_irq(void *param)
>
>         intr = otx2_read64(dev->bar2 + RVU_VF_INT);
>         if (intr == 0)
> -               return;
> +               otx2_base_dbg("Proceeding to check mbox UP messages if any");
>
>         otx2_write64(intr, dev->bar2 + RVU_VF_INT);
>         otx2_base_dbg("Irq 0x%" PRIx64 "(pf:%d,vf:%d)", intr, dev->pf, dev->vf);
> -       if (intr) {
> -               /* First process all configuration messages */
> -               otx2_process_msgs(dev, dev->mbox);
>
> -               /* Process Uplink messages */
> -               otx2_process_msgs_up(dev, &dev->mbox_up);
> -       }
> +       /* First process all configuration messages */
> +       otx2_process_msgs(dev, dev->mbox);
> +
> +       /* Process Uplink messages */
> +       otx2_process_msgs_up(dev, &dev->mbox_up);
>  }
>
>  static void
> @@ -598,18 +597,16 @@ otx2_af_pf_mbox_irq(void *param)
>
>         intr = otx2_read64(dev->bar2 + RVU_PF_INT);
>         if (intr == 0)
> -               return;
> +               otx2_base_dbg("Proceeding to check mbox UP messages if any");
>
>         otx2_write64(intr, dev->bar2 + RVU_PF_INT);
> -
>         otx2_base_dbg("Irq 0x%" PRIx64 "(pf:%d,vf:%d)", intr, dev->pf, dev->vf);
> -       if (intr) {
> -               /* First process all configuration messages */
> -               otx2_process_msgs(dev, dev->mbox);
>
> -               /* Process Uplink messages */
> -               otx2_process_msgs_up(dev, &dev->mbox_up);
> -       }
> +       /* First process all configuration messages */
> +       otx2_process_msgs(dev, dev->mbox);
> +
> +       /* Process Uplink messages */
> +       otx2_process_msgs_up(dev, &dev->mbox_up);
>  }
>
>  static int
> @@ -900,6 +897,7 @@ otx2_dev_priv_init(struct rte_pci_device *pci_dev, void *otx2_dev)
>  {
>         int up_direction = MBOX_DIR_PFAF_UP;
>         int rc, direction = MBOX_DIR_PFAF;
> +       uint64_t intr_offset = RVU_PF_INT;
>         struct otx2_dev *dev = otx2_dev;
>         uintptr_t bar2, bar4;
>         uint64_t bar4_addr;
> @@ -924,15 +922,18 @@ otx2_dev_priv_init(struct rte_pci_device *pci_dev, void *otx2_dev)
>         if (otx2_dev_is_vf(dev)) {
>                 direction = MBOX_DIR_VFPF;
>                 up_direction = MBOX_DIR_VFPF_UP;
> +               intr_offset = RVU_VF_INT;
>         }
>
>         /* Initialize the local mbox */
> -       rc = otx2_mbox_init(&dev->mbox_local, bar4, bar2, direction, 1);
> +       rc = otx2_mbox_init(&dev->mbox_local, bar4, bar2, direction, 1,
> +                           intr_offset);
>         if (rc)
>                 goto error;
>         dev->mbox = &dev->mbox_local;
>
> -       rc = otx2_mbox_init(&dev->mbox_up, bar4, bar2, up_direction, 1);
> +       rc = otx2_mbox_init(&dev->mbox_up, bar4, bar2, up_direction, 1,
> +                           intr_offset);
>         if (rc)
>                 goto error;
>
> @@ -967,13 +968,15 @@ otx2_dev_priv_init(struct rte_pci_device *pci_dev, void *otx2_dev)
>                 }
>                 /* Init mbox object */
>                 rc = otx2_mbox_init(&dev->mbox_vfpf, (uintptr_t)hwbase,
> -                                   bar2, MBOX_DIR_PFVF, pci_dev->max_vfs);
> +                                   bar2, MBOX_DIR_PFVF, pci_dev->max_vfs,
> +                                   intr_offset);
>                 if (rc)
>                         goto iounmap;
>
>                 /* PF -> VF UP messages */
>                 rc = otx2_mbox_init(&dev->mbox_vfpf_up, (uintptr_t)hwbase,
> -                                   bar2, MBOX_DIR_PFVF_UP, pci_dev->max_vfs);
> +                                   bar2, MBOX_DIR_PFVF_UP, pci_dev->max_vfs,
> +                                   intr_offset);
>                 if (rc)
>                         goto mbox_fini;
>         }
> diff --git a/drivers/common/octeontx2/otx2_mbox.c b/drivers/common/octeontx2/otx2_mbox.c
> index c359bf42f..1ec0d6f69 100644
> --- a/drivers/common/octeontx2/otx2_mbox.c
> +++ b/drivers/common/octeontx2/otx2_mbox.c
> @@ -11,6 +11,7 @@
>  #include <rte_cycles.h>
>
>  #include "otx2_mbox.h"
> +#include "otx2_dev.h"
>
>  #define RVU_AF_AFPF_MBOX0      (0x02000)
>  #define RVU_AF_AFPF_MBOX1      (0x02008)
> @@ -59,12 +60,13 @@ otx2_mbox_reset(struct otx2_mbox *mbox, int devid)
>  }
>
>  int
> -otx2_mbox_init(struct otx2_mbox *mbox, uintptr_t hwbase,
> -              uintptr_t reg_base, int direction, int ndevs)
> +otx2_mbox_init(struct otx2_mbox *mbox, uintptr_t hwbase, uintptr_t reg_base,
> +              int direction, int ndevs, uint64_t intr_offset)
>  {
>         struct otx2_mbox_dev *mdev;
>         int devid;
>
> +       mbox->intr_offset = intr_offset;
>         mbox->reg_base = reg_base;
>         mbox->hwbase = hwbase;
>
> @@ -244,6 +246,39 @@ otx2_mbox_get_rsp(struct otx2_mbox *mbox, int devid, void **msg)
>         return msghdr->rc;
>  }
>
> +/**
> + * Polling for given wait time to get mailbox response
> + */
> +static int
> +mbox_poll(struct otx2_mbox *mbox, uint32_t wait)
> +{
> +       uint32_t timeout = 0, sleep = 1;
> +       uint32_t wait_us = wait * 1000;
> +       uint64_t rsp_reg = 0;
> +       uintptr_t reg_addr;
> +
> +       reg_addr = mbox->reg_base + mbox->intr_offset;
> +       do {
> +               rsp_reg = otx2_read64(reg_addr);
> +
> +               if (timeout >= wait_us)
> +                       return -ETIMEDOUT;
> +
> +               rte_delay_us(sleep);
> +               timeout += sleep;
> +       } while (!rsp_reg);
> +
> +       rte_smp_rmb();
> +
> +       /* Clear interrupt */
> +       otx2_write64(rsp_reg, reg_addr);
> +
> +       /* Reset mbox */
> +       otx2_mbox_reset(mbox, 0);
> +
> +       return 0;
> +}
> +
>  /**
>   * @internal
>   * Wait and get mailbox response with timeout
> @@ -321,11 +356,15 @@ otx2_mbox_wait_for_rsp_tmo(struct otx2_mbox *mbox, int devid, uint32_t tmo)
>         }
>
>         /* Wait message */
> -       rc = mbox_wait(mbox, devid, tmo);
> -       if (rc)
> -               return rc;
> +       if (rte_thread_is_intr())
> +               rc = mbox_poll(mbox, tmo);
> +       else
> +               rc = mbox_wait(mbox, devid, tmo);
>
> -       return mdev->msgs_acked;
> +       if (!rc)
> +               rc = mdev->num_msgs;
> +
> +       return rc;
>  }
>
>  /**
> diff --git a/drivers/common/octeontx2/otx2_mbox.h b/drivers/common/octeontx2/otx2_mbox.h
> index e0e4e2f63..0535cec36 100644
> --- a/drivers/common/octeontx2/otx2_mbox.h
> +++ b/drivers/common/octeontx2/otx2_mbox.h
> @@ -73,6 +73,7 @@ struct otx2_mbox {
>         uint16_t tx_size;  /* Size of Tx region */
>         uint16_t ndevs;    /* The number of peers */
>         struct otx2_mbox_dev *dev;
> +       uint64_t intr_offset; /* Offset to interrupt register */
>  };
>
>  /* Header which precedes all mbox messages */
> @@ -1562,8 +1563,8 @@ struct tim_enable_rsp {
>  const char *otx2_mbox_id2name(uint16_t id);
>  int otx2_mbox_id2size(uint16_t id);
>  void otx2_mbox_reset(struct otx2_mbox *mbox, int devid);
> -int otx2_mbox_init(struct otx2_mbox *mbox, uintptr_t hwbase,
> -                  uintptr_t reg_base, int direction, int ndevs);
> +int otx2_mbox_init(struct otx2_mbox *mbox, uintptr_t hwbase, uintptr_t reg_base,
> +                  int direction, int ndevsi, uint64_t intr_offset);
>  void otx2_mbox_fini(struct otx2_mbox *mbox);
>  void otx2_mbox_msg_send(struct otx2_mbox *mbox, int devid);
>  int otx2_mbox_wait_for_rsp(struct otx2_mbox *mbox, int devid);
> --
> 2.17.1
>