Re: [dpdk-dev] [PATCH] net/mlx5: fix transmit doorbell register write barrier

[Slava Ovsiienko <viacheslavo@mellanox.com>]

> -----Original Message-----
> From: Matan Azrad <matan@mellanox.com>
> Sent: Sunday, September 22, 2019 9:59
> To: Slava Ovsiienko <viacheslavo@mellanox.com>; dev@dpdk.org
> Cc: Raslan Darawsheh <rasland@mellanox.com>
> Subject: RE: [PATCH] net/mlx5: fix transmit doorbell register write barrier
> 
> Hi Slava
> 
> Questions inline.
> 

[.. skipped ...]


> > +	/* Check the doorbell register mapping type. */
> > +	cmd = txq_ctrl->uar_mmap_offset / page_size;
> > +	cmd >>= MLX5_UAR_MMAP_CMD_SHIFT;
> > +	cmd &= MLX5_UAR_MMAP_CMD_MASK;
> > +	if (cmd == MLX5_MMAP_GET_NC_PAGES_CMD)
> > +		txq_ctrl->txq.db_nc = 1;
> 
> Are you sure we can't retrieve the value in compile time?
It is deduced by rdma_core library in runtime,
grounding on the MLX5_SHUT_UP_BF environment variable.
This variable is checked by rdma library in ibv device open calls
(both verbs/dv).

> 
> >  #ifndef RTE_ARCH_64
> >  	/* Assign an UAR lock according to UAR page number */
> >  	lock_idx = (txq_ctrl->uar_mmap_offset / page_size) & @@ -281,6
> > +289,7 @@
> >  	uintptr_t uar_va;
> >  	uintptr_t offset;
> >  	const size_t page_size = sysconf(_SC_PAGESIZE);
> > +	unsigned int cmd;
> >
> >  	assert(ppriv);
> >  	/*
> > @@ -300,6 +309,12 @@
> >  	}
> >  	addr = RTE_PTR_ADD(addr, offset);
> >  	ppriv->uar_table[txq->idx] = addr;
> > +	/* Check the doorbell register mapping type. */
> > +	cmd = txq_ctrl->uar_mmap_offset / page_size;
> > +	cmd >>= MLX5_UAR_MMAP_CMD_SHIFT;
> > +	cmd &= MLX5_UAR_MMAP_CMD_MASK;
> > +	if (cmd == MLX5_MMAP_GET_NC_PAGES_CMD)
> > +		txq_ctrl->txq.db_nc = 1;
> 
> Looks like double code.
> Maybe it is better to get the value by a MACRO?
Yes, it would be better.

WBR, Slava