[dpdk-dev] [PATCH v10 1/5] net/bnxt: add support for aarch32

[Ruifeng Wang]

> -----Original Message-----
> From: Lance Richardson <lance.richardson at broadcom.com>
> Sent: Thursday, November 5, 2020 2:30 AM
> To: Juraj Linkeš <juraj.linkes at pantheon.tech>
> Cc: thomas at monjalon.net; Bruce Richardson <bruce.richardson at intel.com>;
> aconole at redhat.com; maicolgabriel at hotmail.com; dev at dpdk.org; Ruifeng
> Wang <Ruifeng.Wang at arm.com>
> Subject: Re: [dpdk-dev] [PATCH v10 1/5] net/bnxt: add support for aarch32
> 
> On Wed, Sep 16, 2020 at 4:21 AM Juraj Linkeš <juraj.linkes at pantheon.tech>
> wrote:
> >
> > From: Ruifeng Wang <ruifeng.wang at arm.com>
> >
> > Expand vector PMD support to aarch32.
> >
> > Signed-off-by: Ruifeng Wang <ruifeng.wang at arm.com>
> > Acked-by: Ajit Khaparde <ajit.khaparde at broadcom.com>
> > ---
> >  drivers/net/bnxt/bnxt_rxq.h | 2 +-
> >  drivers/net/bnxt/bnxt_rxr.h | 2 +-
> >  drivers/net/bnxt/bnxt_txr.h | 2 +-
> >  3 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/bnxt/bnxt_rxq.h b/drivers/net/bnxt/bnxt_rxq.h
> > index d5ce3b6d5..1c4027711 100644
> > --- a/drivers/net/bnxt/bnxt_rxq.h
> > +++ b/drivers/net/bnxt/bnxt_rxq.h
> > @@ -22,7 +22,7 @@ struct bnxt_rx_queue {
> >         uint16_t                nb_rx_hold; /* num held free RX desc */
> >         uint16_t                rx_free_thresh; /* max free RX desc to hold */
> >         uint16_t                queue_id; /* RX queue index */
> > -#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64)
> > +#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64) ||
> defined(RTE_ARCH_ARM)
> >         uint16_t                rxrearm_nb; /* number of descs to reinit. */
> >         uint16_t                rxrearm_start; /* next desc index to reinit. */
> >  #endif
> > diff --git a/drivers/net/bnxt/bnxt_rxr.h b/drivers/net/bnxt/bnxt_rxr.h
> > index 2bf46cd91..e2fba1647 100644
> > --- a/drivers/net/bnxt/bnxt_rxr.h
> > +++ b/drivers/net/bnxt/bnxt_rxr.h
> > @@ -221,7 +221,7 @@ int bnxt_init_one_rx_ring(struct bnxt_rx_queue
> *rxq);
> >  int bnxt_rx_queue_start(struct rte_eth_dev *dev, uint16_t rx_queue_id);
> >  int bnxt_rx_queue_stop(struct rte_eth_dev *dev, uint16_t rx_queue_id);
> >
> > -#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64)
> > +#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64) ||
> defined(RTE_ARCH_ARM)
> >  uint16_t bnxt_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
> >                             uint16_t nb_pkts);
> >  int bnxt_rxq_vec_setup(struct bnxt_rx_queue *rxq);
> > diff --git a/drivers/net/bnxt/bnxt_txr.h b/drivers/net/bnxt/bnxt_txr.h
> > index 7715c11b8..38e5ac9df 100644
> > --- a/drivers/net/bnxt/bnxt_txr.h
> > +++ b/drivers/net/bnxt/bnxt_txr.h
> > @@ -59,7 +59,7 @@ uint16_t bnxt_xmit_pkts(void *tx_queue, struct
> rte_mbuf **tx_pkts,
> >                                uint16_t nb_pkts);
> >  uint16_t bnxt_dummy_xmit_pkts(void *tx_queue, struct rte_mbuf
> **tx_pkts,
> >                               uint16_t nb_pkts);
> > -#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64)
> > +#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64) ||
> defined(RTE_ARCH_ARM)
> >  uint16_t bnxt_xmit_pkts_vec(void *tx_queue, struct rte_mbuf **tx_pkts,
> >                             uint16_t nb_pkts);
> >  #endif
> > --
> > 2.20.1
> >
> 
> Hi Juraj,
> 
> I might be missing something, but I don't
> believe these changes are sufficient to
> enable vector mode for ARM32. For one
> thing, bnxt_receive_function() in bnxt_ethdev.c
> would need to be changed to enable the
> selection of the vector receive function.

Hi Lance,

This patch set aimed to enable aarch32 compilation and run some basic unit tests.
So changes in this patch fixed some 'symbol not found' issues when building for aarch32.
However, it do need a respin because a patch that changed Arm flags has been merged. 

> 
> Also, meson.build has this condition
> for building bnxt_rxtx_vec_neon.c:
> 
> if arch_subdir == 'x86'
>         sources += files('bnxt_rxtx_vec_sse.c')
> elif arch_subdir == 'arm' and host_machine.cpu_family().startswith('aarch64')
>         sources += files('bnxt_rxtx_vec_neon.c')
> endif
> 
> Were you able to build with these changes

Yes. I was able to build with these changes.
As you can find in 3/5 of this patch set, aarch32 uses (cpu_family = 'aarch64').
So condition in meson.build is not a problem.

> and confirm that the vector mode functions
> are selected and used? (There is a log at INFO
> level to indicate which function is selected,
> as should "show rxq info ..." from the
> testpmd CLI.
> 
> Thanks,
Thanks for your review.
> 
>    Lance