On Thu, 12 May 2016 16:13:55 +0100
Alejandro Lucero <alejandro.lucero at netronome.com> wrote:
> On Thu, May 12, 2016 at 4:03 PM, Jan Viktorin <viktorin at rehivetech.com>
> wrote:
>
> > On Thu, 12 May 2016 15:34:00 +0100
> > "Alejandro.Lucero" <alejandro.lucero at 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 at 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 at RehiveTech.com
> > System Architect Web: www.RehiveTech.com
> > RehiveTech
> > Brno, Czech Republic
> >
--
Jan Viktorin E-mail: Viktorin at RehiveTech.com
System Architect Web: www.RehiveTech.com
RehiveTech
Brno, Czech Republic
