Re: [dpdk-dev] [dpdk-dev,3/3] nfp: set device dma mask

[Jan Viktorin <viktorin@rehivetech.com>]

On Thu, 12 May 2016 16:13:55 +0100
Alejandro Lucero <alejandro.lucero@netronome.com> wrote:

> On Thu, May 12, 2016 at 4:03 PM, Jan Viktorin <viktorin@rehivetech.com>
> wrote:
> 
> > On Thu, 12 May 2016 15:34:00 +0100
> > "Alejandro.Lucero" <alejandro.lucero@netronome.com> wrote:
> >  
> > > - Just hugepages within the supported range will be available.  
> >
> > Again the hyphen is redundant here.
> >
> > By the way, this text does not describe the change well. If I understood
> > the whole patch set (I am not quite sure now), the initialization would
> > fail if there are hugepages out of the given DMA mask. Am I wrong?
> >
> >  
> You are right.
> 
> 
> > I'd expect something like "NFP supports DMA address in range ...".
> >
> >  
> That is a good idea. I was thinking on adding a memseg dump info as well
> which would help to understand this issue and other related to memory
> allocation.
> 
> 
> > >
> > > Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
> > >
> > > ---
> > > drivers/net/nfp/nfp_net.c | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > >
> > > diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
> > > index ea5a2a3..e0e444a 100644
> > > --- a/drivers/net/nfp/nfp_net.c
> > > +++ b/drivers/net/nfp/nfp_net.c
> > > @@ -115,6 +115,14 @@ enum nfp_qcp_ptr {
> > >       NFP_QCP_WRITE_PTR
> > >  };
> > >
> > > +#ifndef DMA_64BIT_MASK
> > > +#define DMA_64BIT_MASK  0xffffffffffffffffULL
> > > +#endif
> > > +
> > > +#ifndef DMA_BIT_MASK
> > > +#define DMA_BIT_MASK(n) (((n) == 64) ? DMA_64BIT_MASK : ((1ULL<<(n))-1))
> > > +#endif  
> >
> > This is quite a generic code, I'd put it into the EAL. Probably, it should
> > be renamed to something like RTE_DMA_BIT_MASK.  
> 
> 
> OK.

By the way:

CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#33: FILE: drivers/net/nfp/nfp_net.c:123:
+#define DMA_BIT_MASK(n) (((n) == 64) ? DMA_64BIT_MASK : ((1ULL<<(n))-1))
                                                               ^

CHECK:SPACING: spaces preferred around that '-' (ctx:VxV)
#33: FILE: drivers/net/nfp/nfp_net.c:123:
+#define DMA_BIT_MASK(n) (((n) == 64) ? DMA_64BIT_MASK : ((1ULL<<(n))-1))
                                                                     ^

total: 0 errors, 0 warnings, 2 checks, 23 lines checked

> 
> 
> >
> >
> > +  
> > >  /*
> > >   * nfp_qcp_ptr_add - Add the value to the selected pointer of a queue
> > >   * @q: Base address for queue structure
> > > @@ -2441,6 +2449,9 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
> > >       /* Recording current stats counters values */
> > >       nfp_net_stats_reset(eth_dev);
> > >
> > > +     /* Setting dma_mask */
> > > +     eth_dev->data->dma_mask = DMA_BIT_MASK(40);  
> >
> > Can we read this from /sys/bus/pci/devices/*/dma_mask_bits? I am not sure
> > whether is this generic enough but I can see dma_mask_bits for the PCI
> > devices on my PC.
> >
> >  
> The kernel adds a default dma mask when device scanning (at least for PCI
> devices). It is a device driver who knows about specific DMA addressing
> limitations. For example, this is done with UIO (igb_uio) and the using
> sysfs would be fine (but then you should add support for specifying a dma
> mask in igb_uio as a module param) but this is not true for VFIO.
> 

Makes sense...

Jan

> 
> 
> > Regards
> > Jan
> >  
> > > +
> > >       return 0;
> > >  }
> > >  
> >
> >
> >
> >
> > --
> >    Jan Viktorin                  E-mail: Viktorin@RehiveTech.com
> >    System Architect              Web:    www.RehiveTech.com
> >    RehiveTech
> >    Brno, Czech Republic
> >  



-- 
   Jan Viktorin                  E-mail: Viktorin@RehiveTech.com
   System Architect              Web:    www.RehiveTech.com
   RehiveTech
   Brno, Czech Republic