DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] Testpmd returns error.
@ 2015-02-22  5:30 Tetsuya Mukawa
  2015-02-22 14:17 ` Thomas Monjalon
                   ` (2 more replies)
  0 siblings, 3 replies; 30+ messages in thread
From: Tetsuya Mukawa @ 2015-02-22  5:30 UTC (permalink / raw)
  To: dev

Hi,

In my environment, testpmd in latest master branch returns error like below.

$ sudo ./tools/dpdk_nic_bind.py -b igb_uio 0000:02:00.0
$ sudo ./x86_64-native-linuxapp-gcc/app/testpmd -c f -n 1 -- -i
EAL: Detected lcore 0 as core 0 on socket 0
EAL: Detected lcore 1 as core 1 on socket 0
EAL: Detected lcore 2 as core 2 on socket 0
EAL: Detected lcore 3 as core 3 on socket 0
EAL: Detected lcore 4 as core 4 on socket 0
EAL: Detected lcore 5 as core 5 on socket 0
EAL: Detected lcore 6 as core 6 on socket 0
EAL: Detected lcore 7 as core 7 on socket 0
EAL: Support maximum 128 logical core(s) by configuration.
EAL: Detected 8 lcore(s)
EAL: VFIO modules not all loaded, skip VFIO support...
EAL: Setting up memory...
EAL: Ask a virtual area of 0x280000000 bytes
EAL: Virtual area found at 0x7ffd40000000 (size = 0x280000000)
EAL: Requesting 10 pages of size 1024MB from socket 0
EAL: TSC frequency is ~3991450 KHz
EAL: Master core 0 is ready (tid=f7fd6840)
PMD: ENICPMD trace: rte_enic_pmd_init
EAL: Core 3 is ready (tid=f58e0700)
EAL: Core 2 is ready (tid=f60e1700)
EAL: Core 1 is ready (tid=f68e2700)
EAL: PCI device 0000:02:00.0 on NUMA socket -1
EAL:   probe driver: 8086:10b9 rte_em_pmd
EAL:   PCI memory mapped at 0x7fffc0000000
EAL: pci_map_resource(): cannot mmap(23, 0x7fffc0020000, 0x20000,
0x1000): Invalid argument (0xffffffffffffffff)
EAL: Error - exiting with code: 1
  Cause: Requested device 0000:02:00.0 cannot be used


I've run git-bisect, and it seems following commit cause this error.

commit 4a499c64959074ba6fa6a5a2b3a2a6aa10627fa1
Author: Danny Zhou <danny.zhou@intel.com>
Date:   Fri Feb 20 16:59:15 2015 +0000

    eal/linux: enable uio_pci_generic support

Someone, could you please check it?

Thanks,
Tetsuya

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [dpdk-dev] Testpmd returns error.
  2015-02-22  5:30 [dpdk-dev] Testpmd returns error Tetsuya Mukawa
@ 2015-02-22 14:17 ` Thomas Monjalon
  2015-02-22 16:19   ` David Marchand
  2015-02-22 20:46 ` Bruce Richardson
  2015-02-23 14:57 ` [dpdk-dev] [PATCH] eal: mmap uio resources using resourceX files Bruce Richardson
  2 siblings, 1 reply; 30+ messages in thread
From: Thomas Monjalon @ 2015-02-22 14:17 UTC (permalink / raw)
  To: Tetsuya Mukawa, david.marchand; +Cc: dev

Hi Tetsuya,

2015-02-22 14:30, Tetsuya Mukawa:
> Hi,
> 
> In my environment, testpmd in latest master branch returns error like below.
> 
> $ sudo ./tools/dpdk_nic_bind.py -b igb_uio 0000:02:00.0
> $ sudo ./x86_64-native-linuxapp-gcc/app/testpmd -c f -n 1 -- -i
> EAL: Detected lcore 0 as core 0 on socket 0
> EAL: Detected lcore 1 as core 1 on socket 0
> EAL: Detected lcore 2 as core 2 on socket 0
> EAL: Detected lcore 3 as core 3 on socket 0
> EAL: Detected lcore 4 as core 4 on socket 0
> EAL: Detected lcore 5 as core 5 on socket 0
> EAL: Detected lcore 6 as core 6 on socket 0
> EAL: Detected lcore 7 as core 7 on socket 0
> EAL: Support maximum 128 logical core(s) by configuration.
> EAL: Detected 8 lcore(s)
> EAL: VFIO modules not all loaded, skip VFIO support...
> EAL: Setting up memory...
> EAL: Ask a virtual area of 0x280000000 bytes
> EAL: Virtual area found at 0x7ffd40000000 (size = 0x280000000)
> EAL: Requesting 10 pages of size 1024MB from socket 0
> EAL: TSC frequency is ~3991450 KHz
> EAL: Master core 0 is ready (tid=f7fd6840)
> PMD: ENICPMD trace: rte_enic_pmd_init
> EAL: Core 3 is ready (tid=f58e0700)
> EAL: Core 2 is ready (tid=f60e1700)
> EAL: Core 1 is ready (tid=f68e2700)
> EAL: PCI device 0000:02:00.0 on NUMA socket -1
> EAL:   probe driver: 8086:10b9 rte_em_pmd
> EAL:   PCI memory mapped at 0x7fffc0000000
> EAL: pci_map_resource(): cannot mmap(23, 0x7fffc0020000, 0x20000,
> 0x1000): Invalid argument (0xffffffffffffffff)
> EAL: Error - exiting with code: 1
>   Cause: Requested device 0000:02:00.0 cannot be used
> 
> 
> I've run git-bisect, and it seems following commit cause this error.
> 
> commit 4a499c64959074ba6fa6a5a2b3a2a6aa10627fa1
> Author: Danny Zhou <danny.zhou@intel.com>
> Date:   Fri Feb 20 16:59:15 2015 +0000
> 
>     eal/linux: enable uio_pci_generic support

Thank you Tetsuya for reporting and sorry to disturb you with this new problem.
Please ignore this patchset in your hotplug development.

> Someone, could you please check it?

It is possible that this patchset was not correctly tested.
We might revert it or try to fix it.
I think the decision should be done by its authors (Danny, Bruce),
or the Linux EAL maintainer (David).

David, any opinion?

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [dpdk-dev] Testpmd returns error.
  2015-02-22 14:17 ` Thomas Monjalon
@ 2015-02-22 16:19   ` David Marchand
  2015-02-22 18:51     ` Richardson, Bruce
  0 siblings, 1 reply; 30+ messages in thread
From: David Marchand @ 2015-02-22 16:19 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

Hello,

On Sun, Feb 22, 2015 at 3:17 PM, Thomas Monjalon <thomas.monjalon@6wind.com>
wrote:

> Hi Tetsuya,
>
> > Someone, could you please check it?
>
> It is possible that this patchset was not correctly tested.
> We might revert it or try to fix it.
> I think the decision should be done by its authors (Danny, Bruce),
> or the Linux EAL maintainer (David).
>

Well, I need to have a deeper look at this change.
I did not find time before my holidays.

- I think there may be an issue with the use of resource0 instead of
/dev/uio.
I am not sure uio mmap will be happy or I overlooked something trivial.

- Testing proc type in pci_uio_map_resource() looks wrong to me, since we
validated earlier in this same function that we are in primary process.

- uio_res->maps indexes are not the same as the pci resources, might
trigger problems (and it clearly does not make it easy to read ...).
And now we are reading sysfs twice.
I would prefer this code is reworked so that we avoid those loops in
eal_pci_uio.c.


Anyway, I am still on holiday (I should have kept my laptop away ...), I
will be back tomorrow.
I suppose Bruce or Danny will come with a fix, let's decide what the best
solution is at this moment.


-- 
David Marchand

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [dpdk-dev] Testpmd returns error.
  2015-02-22 16:19   ` David Marchand
@ 2015-02-22 18:51     ` Richardson, Bruce
  2015-02-22 20:31       ` Bruce Richardson
  0 siblings, 1 reply; 30+ messages in thread
From: Richardson, Bruce @ 2015-02-22 18:51 UTC (permalink / raw)
  To: David Marchand; +Cc: dev





On 22 Feb 2015, at 16:19, David Marchand <david.marchand@6wind.com<mailto:david.marchand@6wind.com>> wrote:

Hello,

On Sun, Feb 22, 2015 at 3:17 PM, Thomas Monjalon <thomas.monjalon@6wind.com<mailto:thomas.monjalon@6wind.com>> wrote:
Hi Tetsuya,

> Someone, could you please check it?

It is possible that this patchset was not correctly tested.
We might revert it or try to fix it.
I think the decision should be done by its authors (Danny, Bruce),
or the Linux EAL maintainer (David).

Well, I need to have a deeper look at this change.
I did not find time before my holidays.

- I think there may be an issue with the use of resource0 instead of /dev/uio.
I am not sure uio mmap will be happy or I overlooked something trivial.

- Testing proc type in pci_uio_map_resource() looks wrong to me, since we validated earlier in this same function that we are in primary process.

- uio_res->maps indexes are not the same as the pci resources, might trigger problems (and it clearly does not make it easy to read ...).
And now we are reading sysfs twice.
I would prefer this code is reworked so that we avoid those loops in eal_pci_uio.c.


Anyway, I am still on holiday (I should have kept my laptop away ...), I will be back tomorrow.
I suppose Bruce or Danny will come with a fix, let's decide what the best solution is at this moment.



I'll take another look at this but unfortunately I don't think I can necessarily reproduce this issue as it seems to occur with enic driver, but I will try.

--
David Marchand

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [dpdk-dev] Testpmd returns error.
  2015-02-22 18:51     ` Richardson, Bruce
@ 2015-02-22 20:31       ` Bruce Richardson
  0 siblings, 0 replies; 30+ messages in thread
From: Bruce Richardson @ 2015-02-22 20:31 UTC (permalink / raw)
  To: David Marchand; +Cc: dev

On Sun, Feb 22, 2015 at 06:51:58PM +0000, Richardson, Bruce wrote:
> 
> 
> 
> 
> On 22 Feb 2015, at 16:19, David Marchand <david.marchand@6wind.com<mailto:david.marchand@6wind.com>> wrote:
> 
> Hello,
> 
> On Sun, Feb 22, 2015 at 3:17 PM, Thomas Monjalon <thomas.monjalon@6wind.com<mailto:thomas.monjalon@6wind.com>> wrote:
> Hi Tetsuya,
> 
> > Someone, could you please check it?
> 
> It is possible that this patchset was not correctly tested.
> We might revert it or try to fix it.
> I think the decision should be done by its authors (Danny, Bruce),
> or the Linux EAL maintainer (David).
> 
> Well, I need to have a deeper look at this change.
> I did not find time before my holidays.
> 
> - I think there may be an issue with the use of resource0 instead of /dev/uio.
> I am not sure uio mmap will be happy or I overlooked something trivial.
> 
> - Testing proc type in pci_uio_map_resource() looks wrong to me, since we validated earlier in this same function that we are in primary process.
> 
> - uio_res->maps indexes are not the same as the pci resources, might trigger problems (and it clearly does not make it easy to read ...).
> And now we are reading sysfs twice.
> I would prefer this code is reworked so that we avoid those loops in eal_pci_uio.c.
> 
> 
> Anyway, I am still on holiday (I should have kept my laptop away ...), I will be back tomorrow.
> I suppose Bruce or Danny will come with a fix, let's decide what the best solution is at this moment.
> 
> 
> 
> I'll take another look at this but unfortunately I don't think I can necessarily reproduce this issue as it seems to occur with enic driver, but I will try.

Please ignore above from me, now that I'm able to read my mail on more than a phone
screen, I can actually read things properly... :-)

I'll look into this now, see if it's something simple I can spot to fix.

/Bruce
> 
> --
> David Marchand

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [dpdk-dev] Testpmd returns error.
  2015-02-22  5:30 [dpdk-dev] Testpmd returns error Tetsuya Mukawa
  2015-02-22 14:17 ` Thomas Monjalon
@ 2015-02-22 20:46 ` Bruce Richardson
  2015-02-23  2:33   ` Tetsuya Mukawa
  2015-02-23 14:57 ` [dpdk-dev] [PATCH] eal: mmap uio resources using resourceX files Bruce Richardson
  2 siblings, 1 reply; 30+ messages in thread
From: Bruce Richardson @ 2015-02-22 20:46 UTC (permalink / raw)
  To: Tetsuya Mukawa; +Cc: dev

On Sun, Feb 22, 2015 at 02:30:02PM +0900, Tetsuya Mukawa wrote:
> Hi,
> 
> In my environment, testpmd in latest master branch returns error like below.
> 
> $ sudo ./tools/dpdk_nic_bind.py -b igb_uio 0000:02:00.0
> $ sudo ./x86_64-native-linuxapp-gcc/app/testpmd -c f -n 1 -- -i
> EAL: Detected lcore 0 as core 0 on socket 0
> EAL: Detected lcore 1 as core 1 on socket 0
> EAL: Detected lcore 2 as core 2 on socket 0
> EAL: Detected lcore 3 as core 3 on socket 0
> EAL: Detected lcore 4 as core 4 on socket 0
> EAL: Detected lcore 5 as core 5 on socket 0
> EAL: Detected lcore 6 as core 6 on socket 0
> EAL: Detected lcore 7 as core 7 on socket 0
> EAL: Support maximum 128 logical core(s) by configuration.
> EAL: Detected 8 lcore(s)
> EAL: VFIO modules not all loaded, skip VFIO support...
> EAL: Setting up memory...
> EAL: Ask a virtual area of 0x280000000 bytes
> EAL: Virtual area found at 0x7ffd40000000 (size = 0x280000000)
> EAL: Requesting 10 pages of size 1024MB from socket 0
> EAL: TSC frequency is ~3991450 KHz
> EAL: Master core 0 is ready (tid=f7fd6840)
> PMD: ENICPMD trace: rte_enic_pmd_init
> EAL: Core 3 is ready (tid=f58e0700)
> EAL: Core 2 is ready (tid=f60e1700)
> EAL: Core 1 is ready (tid=f68e2700)
> EAL: PCI device 0000:02:00.0 on NUMA socket -1
> EAL:   probe driver: 8086:10b9 rte_em_pmd
> EAL:   PCI memory mapped at 0x7fffc0000000
> EAL: pci_map_resource(): cannot mmap(23, 0x7fffc0020000, 0x20000,
> 0x1000): Invalid argument (0xffffffffffffffff)
> EAL: Error - exiting with code: 1
>   Cause: Requested device 0000:02:00.0 cannot be used
> 
> 
> I've run git-bisect, and it seems following commit cause this error.
> 
> commit 4a499c64959074ba6fa6a5a2b3a2a6aa10627fa1
> Author: Danny Zhou <danny.zhou@intel.com>
> Date:   Fri Feb 20 16:59:15 2015 +0000
> 
>     eal/linux: enable uio_pci_generic support
> 
> Someone, could you please check it?
> 
> Thanks,
> Tetsuya
> 

Hi Tetsuya,

trying to reproduce the problem here, with no success so far with a mix of 1G
and 10G ports. Is there anything special about your environment that might 
especially trigger this issue? Is it a VM or running on the host machine etc.?

Regards,
/Bruce

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [dpdk-dev] Testpmd returns error.
  2015-02-22 20:46 ` Bruce Richardson
@ 2015-02-23  2:33   ` Tetsuya Mukawa
  2015-02-23 11:12     ` Bruce Richardson
  0 siblings, 1 reply; 30+ messages in thread
From: Tetsuya Mukawa @ 2015-02-23  2:33 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev

On 2015/02/23 5:46, Bruce Richardson wrote:
> On Sun, Feb 22, 2015 at 02:30:02PM +0900, Tetsuya Mukawa wrote:
>> Hi,
>>
>> In my environment, testpmd in latest master branch returns error like below.
>>
>> $ sudo ./tools/dpdk_nic_bind.py -b igb_uio 0000:02:00.0
>> $ sudo ./x86_64-native-linuxapp-gcc/app/testpmd -c f -n 1 -- -i
>> EAL: Detected lcore 0 as core 0 on socket 0
>> EAL: Detected lcore 1 as core 1 on socket 0
>> EAL: Detected lcore 2 as core 2 on socket 0
>> EAL: Detected lcore 3 as core 3 on socket 0
>> EAL: Detected lcore 4 as core 4 on socket 0
>> EAL: Detected lcore 5 as core 5 on socket 0
>> EAL: Detected lcore 6 as core 6 on socket 0
>> EAL: Detected lcore 7 as core 7 on socket 0
>> EAL: Support maximum 128 logical core(s) by configuration.
>> EAL: Detected 8 lcore(s)
>> EAL: VFIO modules not all loaded, skip VFIO support...
>> EAL: Setting up memory...
>> EAL: Ask a virtual area of 0x280000000 bytes
>> EAL: Virtual area found at 0x7ffd40000000 (size = 0x280000000)
>> EAL: Requesting 10 pages of size 1024MB from socket 0
>> EAL: TSC frequency is ~3991450 KHz
>> EAL: Master core 0 is ready (tid=f7fd6840)
>> PMD: ENICPMD trace: rte_enic_pmd_init
>> EAL: Core 3 is ready (tid=f58e0700)
>> EAL: Core 2 is ready (tid=f60e1700)
>> EAL: Core 1 is ready (tid=f68e2700)
>> EAL: PCI device 0000:02:00.0 on NUMA socket -1
>> EAL:   probe driver: 8086:10b9 rte_em_pmd
>> EAL:   PCI memory mapped at 0x7fffc0000000
>> EAL: pci_map_resource(): cannot mmap(23, 0x7fffc0020000, 0x20000,
>> 0x1000): Invalid argument (0xffffffffffffffff)
>> EAL: Error - exiting with code: 1
>>   Cause: Requested device 0000:02:00.0 cannot be used
>>
>>
>> I've run git-bisect, and it seems following commit cause this error.
>>
>> commit 4a499c64959074ba6fa6a5a2b3a2a6aa10627fa1
>> Author: Danny Zhou <danny.zhou@intel.com>
>> Date:   Fri Feb 20 16:59:15 2015 +0000
>>
>>     eal/linux: enable uio_pci_generic support
>>
>> Someone, could you please check it?
>>
>> Thanks,
>> Tetsuya
>>
> Hi Tetsuya,
>
> trying to reproduce the problem here, with no success so far with a mix of 1G
> and 10G ports. Is there anything special about your environment that might 
> especially trigger this issue? Is it a VM or running on the host machine etc.?

Hi Bruce,

I appreciate for your testing.

I've tried it on an another system, and I couldn't reproduce it.
Could you please see below?

- The system I can reproduce the issue
OS: Ubuntu14.04
Kernel: Linux eris 3.13.0-30-generic
CPU: AMD FX(tm)-8350 Eight-Core Processor
NIC: Ethernet controller: Intel Corporation 82572EI Gigabit Ethernet
Controller (Copper) (rev 06)

- The system I cannot reproduce the issue
OS: ubuntu14.04
Kernel: Linux ubuntu-igel 3.13.0-30-generic
Intel(R) Xeon(R) CPU E5-2697 v2 @ 2.70GHz
NIC: Ethernet controller: Intel Corporation I350 Gigabit Network
Connection (rev 01)

I will check it more in next testing phase.
Until then, I will use the system I cannot reproduce the issue.

Regards,
Tetsuya

> Regards,
> /Bruce
>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [dpdk-dev] Testpmd returns error.
  2015-02-23  2:33   ` Tetsuya Mukawa
@ 2015-02-23 11:12     ` Bruce Richardson
  2015-02-23 11:36       ` Tetsuya Mukawa
  0 siblings, 1 reply; 30+ messages in thread
From: Bruce Richardson @ 2015-02-23 11:12 UTC (permalink / raw)
  To: Tetsuya Mukawa; +Cc: dev

On Mon, Feb 23, 2015 at 11:33:45AM +0900, Tetsuya Mukawa wrote:
> On 2015/02/23 5:46, Bruce Richardson wrote:
> > On Sun, Feb 22, 2015 at 02:30:02PM +0900, Tetsuya Mukawa wrote:
> >> Hi,
> >>
> >> In my environment, testpmd in latest master branch returns error like below.
> >>
> >> $ sudo ./tools/dpdk_nic_bind.py -b igb_uio 0000:02:00.0
> >> $ sudo ./x86_64-native-linuxapp-gcc/app/testpmd -c f -n 1 -- -i
> >> EAL: Detected lcore 0 as core 0 on socket 0
> >> EAL: Detected lcore 1 as core 1 on socket 0
> >> EAL: Detected lcore 2 as core 2 on socket 0
> >> EAL: Detected lcore 3 as core 3 on socket 0
> >> EAL: Detected lcore 4 as core 4 on socket 0
> >> EAL: Detected lcore 5 as core 5 on socket 0
> >> EAL: Detected lcore 6 as core 6 on socket 0
> >> EAL: Detected lcore 7 as core 7 on socket 0
> >> EAL: Support maximum 128 logical core(s) by configuration.
> >> EAL: Detected 8 lcore(s)
> >> EAL: VFIO modules not all loaded, skip VFIO support...
> >> EAL: Setting up memory...
> >> EAL: Ask a virtual area of 0x280000000 bytes
> >> EAL: Virtual area found at 0x7ffd40000000 (size = 0x280000000)
> >> EAL: Requesting 10 pages of size 1024MB from socket 0
> >> EAL: TSC frequency is ~3991450 KHz
> >> EAL: Master core 0 is ready (tid=f7fd6840)
> >> PMD: ENICPMD trace: rte_enic_pmd_init
> >> EAL: Core 3 is ready (tid=f58e0700)
> >> EAL: Core 2 is ready (tid=f60e1700)
> >> EAL: Core 1 is ready (tid=f68e2700)
> >> EAL: PCI device 0000:02:00.0 on NUMA socket -1
> >> EAL:   probe driver: 8086:10b9 rte_em_pmd
> >> EAL:   PCI memory mapped at 0x7fffc0000000
> >> EAL: pci_map_resource(): cannot mmap(23, 0x7fffc0020000, 0x20000,
> >> 0x1000): Invalid argument (0xffffffffffffffff)
> >> EAL: Error - exiting with code: 1
> >>   Cause: Requested device 0000:02:00.0 cannot be used
> >>
> >>
> >> I've run git-bisect, and it seems following commit cause this error.
> >>
> >> commit 4a499c64959074ba6fa6a5a2b3a2a6aa10627fa1
> >> Author: Danny Zhou <danny.zhou@intel.com>
> >> Date:   Fri Feb 20 16:59:15 2015 +0000
> >>
> >>     eal/linux: enable uio_pci_generic support
> >>
> >> Someone, could you please check it?
> >>
> >> Thanks,
> >> Tetsuya
> >>
> > Hi Tetsuya,
> >
> > trying to reproduce the problem here, with no success so far with a mix of 1G
> > and 10G ports. Is there anything special about your environment that might 
> > especially trigger this issue? Is it a VM or running on the host machine etc.?
> 
> Hi Bruce,
> 
> I appreciate for your testing.
> 
> I've tried it on an another system, and I couldn't reproduce it.
> Could you please see below?
> 
> - The system I can reproduce the issue
> OS: Ubuntu14.04
> Kernel: Linux eris 3.13.0-30-generic
> CPU: AMD FX(tm)-8350 Eight-Core Processor
> NIC: Ethernet controller: Intel Corporation 82572EI Gigabit Ethernet
> Controller (Copper) (rev 06)
> 
> - The system I cannot reproduce the issue
> OS: ubuntu14.04
> Kernel: Linux ubuntu-igel 3.13.0-30-generic
> Intel(R) Xeon(R) CPU E5-2697 v2 @ 2.70GHz
> NIC: Ethernet controller: Intel Corporation I350 Gigabit Network
> Connection (rev 01)
> 
> I will check it more in next testing phase.
> Until then, I will use the system I cannot reproduce the issue.
> 
> Regards,
> Tetsuya
> 
> > Regards,
> > /Bruce
> >
>
Thanks Tetsuya,
Declan has managed to find a board here that can reproduce the issue so we 
are now investigating possible solutions.
/Bruce

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [dpdk-dev] Testpmd returns error.
  2015-02-23 11:12     ` Bruce Richardson
@ 2015-02-23 11:36       ` Tetsuya Mukawa
  0 siblings, 0 replies; 30+ messages in thread
From: Tetsuya Mukawa @ 2015-02-23 11:36 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev

On 2015/02/23 20:12, Bruce Richardson wrote:
> On Mon, Feb 23, 2015 at 11:33:45AM +0900, Tetsuya Mukawa wrote:
>> On 2015/02/23 5:46, Bruce Richardson wrote:
>>> On Sun, Feb 22, 2015 at 02:30:02PM +0900, Tetsuya Mukawa wrote:
>>>> Hi,
>>>>
>>>> In my environment, testpmd in latest master branch returns error like below.
>>>>
>>>> $ sudo ./tools/dpdk_nic_bind.py -b igb_uio 0000:02:00.0
>>>> $ sudo ./x86_64-native-linuxapp-gcc/app/testpmd -c f -n 1 -- -i
>>>> EAL: Detected lcore 0 as core 0 on socket 0
>>>> EAL: Detected lcore 1 as core 1 on socket 0
>>>> EAL: Detected lcore 2 as core 2 on socket 0
>>>> EAL: Detected lcore 3 as core 3 on socket 0
>>>> EAL: Detected lcore 4 as core 4 on socket 0
>>>> EAL: Detected lcore 5 as core 5 on socket 0
>>>> EAL: Detected lcore 6 as core 6 on socket 0
>>>> EAL: Detected lcore 7 as core 7 on socket 0
>>>> EAL: Support maximum 128 logical core(s) by configuration.
>>>> EAL: Detected 8 lcore(s)
>>>> EAL: VFIO modules not all loaded, skip VFIO support...
>>>> EAL: Setting up memory...
>>>> EAL: Ask a virtual area of 0x280000000 bytes
>>>> EAL: Virtual area found at 0x7ffd40000000 (size = 0x280000000)
>>>> EAL: Requesting 10 pages of size 1024MB from socket 0
>>>> EAL: TSC frequency is ~3991450 KHz
>>>> EAL: Master core 0 is ready (tid=f7fd6840)
>>>> PMD: ENICPMD trace: rte_enic_pmd_init
>>>> EAL: Core 3 is ready (tid=f58e0700)
>>>> EAL: Core 2 is ready (tid=f60e1700)
>>>> EAL: Core 1 is ready (tid=f68e2700)
>>>> EAL: PCI device 0000:02:00.0 on NUMA socket -1
>>>> EAL:   probe driver: 8086:10b9 rte_em_pmd
>>>> EAL:   PCI memory mapped at 0x7fffc0000000
>>>> EAL: pci_map_resource(): cannot mmap(23, 0x7fffc0020000, 0x20000,
>>>> 0x1000): Invalid argument (0xffffffffffffffff)
>>>> EAL: Error - exiting with code: 1
>>>>   Cause: Requested device 0000:02:00.0 cannot be used
>>>>
>>>>
>>>> I've run git-bisect, and it seems following commit cause this error.
>>>>
>>>> commit 4a499c64959074ba6fa6a5a2b3a2a6aa10627fa1
>>>> Author: Danny Zhou <danny.zhou@intel.com>
>>>> Date:   Fri Feb 20 16:59:15 2015 +0000
>>>>
>>>>     eal/linux: enable uio_pci_generic support
>>>>
>>>> Someone, could you please check it?
>>>>
>>>> Thanks,
>>>> Tetsuya
>>>>
>>> Hi Tetsuya,
>>>
>>> trying to reproduce the problem here, with no success so far with a mix of 1G
>>> and 10G ports. Is there anything special about your environment that might 
>>> especially trigger this issue? Is it a VM or running on the host machine etc.?
>> Hi Bruce,
>>
>> I appreciate for your testing.
>>
>> I've tried it on an another system, and I couldn't reproduce it.
>> Could you please see below?
>>
>> - The system I can reproduce the issue
>> OS: Ubuntu14.04
>> Kernel: Linux eris 3.13.0-30-generic
>> CPU: AMD FX(tm)-8350 Eight-Core Processor
>> NIC: Ethernet controller: Intel Corporation 82572EI Gigabit Ethernet
>> Controller (Copper) (rev 06)
>>
>> - The system I cannot reproduce the issue
>> OS: ubuntu14.04
>> Kernel: Linux ubuntu-igel 3.13.0-30-generic
>> Intel(R) Xeon(R) CPU E5-2697 v2 @ 2.70GHz
>> NIC: Ethernet controller: Intel Corporation I350 Gigabit Network
>> Connection (rev 01)
>>
>> I will check it more in next testing phase.
>> Until then, I will use the system I cannot reproduce the issue.
>>
>> Regards,
>> Tetsuya
>>
>>> Regards,
>>> /Bruce
>>>
> Thanks Tetsuya,
> Declan has managed to find a board here that can reproduce the issue so we 
> are now investigating possible solutions.
> /Bruce

Hi Bruce,

Thanks you so much!

Tetsuya

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [dpdk-dev] [PATCH] eal: mmap uio resources using resourceX files
  2015-02-22  5:30 [dpdk-dev] Testpmd returns error Tetsuya Mukawa
  2015-02-22 14:17 ` Thomas Monjalon
  2015-02-22 20:46 ` Bruce Richardson
@ 2015-02-23 14:57 ` Bruce Richardson
  2015-02-23 15:00   ` Bruce Richardson
                     ` (3 more replies)
  2 siblings, 4 replies; 30+ messages in thread
From: Bruce Richardson @ 2015-02-23 14:57 UTC (permalink / raw)
  To: dev

Instead of distinguishing the BAR mappings via offset within a single
file, originally /dev/uioX, switch to mapping each individual bar via
the appropriately numbered resourceX file.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_eal/common/include/rte_pci.h    |  2 +-
 lib/librte_eal/linuxapp/eal/eal_pci_init.h |  1 +
 lib/librte_eal/linuxapp/eal/eal_pci_uio.c  | 34 ++++++++++++++++--------------
 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c |  1 +
 4 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 4301c16..e34b139 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -117,7 +117,7 @@ struct rte_pci_resource {
 };
 
 /** Maximum number of PCI resources. */
-#define PCI_MAX_RESOURCE 7
+#define PCI_MAX_RESOURCE 6
 
 /**
  * A structure describing an ID for a PCI driver. Each driver provides a
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_init.h b/lib/librte_eal/linuxapp/eal/eal_pci_init.h
index 1070eb8..2125d7b 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_init.h
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_init.h
@@ -38,6 +38,7 @@
 
 struct pci_map {
 	void *addr;
+	char *path;
 	uint64_t offset;
 	uint64_t size;
 	uint64_t phaddr;
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
index 2b16fcb..ecf385a 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
@@ -137,10 +137,10 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
 			/*
 			 * open devname, to mmap it
 			 */
-			fd = open(uio_res->path, O_RDWR);
+			fd = open(uio_res->maps[i].path, O_RDWR);
 			if (fd < 0) {
 				RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
-					uio_res->path, strerror(errno));
+					uio_res->maps[i].path, strerror(errno));
 				return -1;
 			}
 
@@ -149,7 +149,8 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
 					     (size_t)uio_res->maps[i].size)
 			    != uio_res->maps[i].addr) {
 				RTE_LOG(ERR, EAL,
-					"Cannot mmap device resource\n");
+					"Cannot mmap device resource file: %s\n",
+					uio_res->maps[i].path);
 				close(fd);
 				return -1;
 			}
@@ -294,8 +295,6 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 	void *mapaddr;
 	int uio_num;
 	uint64_t phaddr;
-	uint64_t offset;
-	uint64_t pagesz;
 	int nb_maps;
 	struct rte_pci_addr *loc = &dev->addr;
 	struct mapped_pci_resource *uio_res;
@@ -336,11 +335,6 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 		return -1;
 	}
 
-	/* update devname for mmap  */
-	snprintf(devname, sizeof(devname),
-		SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/resource%d",
-		loc->domain, loc->bus, loc->devid, loc->function, 0);
-
 	/* set bus master that is not done by uio_pci_generic */
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
 		if (pci_uio_set_bus_master(dev->intr_handle.uio_cfg_fd)) {
@@ -370,8 +364,6 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 	uio_res->nb_maps = nb_maps;
 
 	/* Map all BARs */
-	pagesz = sysconf(_SC_PAGESIZE);
-
 	maps = uio_res->maps;
 	for (i = 0; i != PCI_MAX_RESOURCE; i++) {
 		int fd;
@@ -389,10 +381,15 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 		/* if matching map is found, then use it */
 		if (j != nb_maps) {
 			int fail = 0;
-			offset = j * pagesz;
+
+			/* update devname for mmap  */
+			snprintf(devname, sizeof(devname),
+				SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/resource%d",
+				loc->domain, loc->bus, loc->devid, loc->function,
+				i);
 
 			/*
-			 * open devname, to mmap it
+			 * open resource file, to mmap it
 			 */
 			fd = open(devname, O_RDWR);
 			if (fd < 0) {
@@ -408,7 +405,7 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 				if (pci_map_addr == NULL)
 					pci_map_addr = pci_find_max_end_va();
 
-				mapaddr = pci_map_resource(pci_map_addr, fd, (off_t)offset,
+				mapaddr = pci_map_resource(pci_map_addr, fd, 0,
 						(size_t)maps[j].size);
 				if (mapaddr == MAP_FAILED)
 					fail = 1;
@@ -416,6 +413,10 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 				pci_map_addr = RTE_PTR_ADD(mapaddr, (size_t) maps[j].size);
 			}
 
+			maps[j].path = rte_malloc(NULL, strlen(devname) + 1, 0);
+			if (maps[j].path == NULL)
+				fail = 1;
+
 			if (fail) {
 				rte_free(uio_res);
 				close(fd);
@@ -424,7 +425,8 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 			close(fd);
 
 			maps[j].addr = mapaddr;
-			maps[j].offset = offset;
+			maps[j].offset = 0;
+			strcpy(maps[j].path, devname);
 			dev->mem_resource[i].addr = mapaddr;
 		}
 	}
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
index 20e0977..7a57d0f 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
@@ -751,6 +751,7 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
 		maps[i].addr = bar_addr;
 		maps[i].offset = reg.offset;
 		maps[i].size = reg.size;
+		maps[i].path = NULL; /* vfio doesn't have per-resource paths */
 		dev->mem_resource[i].addr = bar_addr;
 	}
 
-- 
2.1.0

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [dpdk-dev] [PATCH] eal: mmap uio resources using resourceX files
  2015-02-23 14:57 ` [dpdk-dev] [PATCH] eal: mmap uio resources using resourceX files Bruce Richardson
@ 2015-02-23 15:00   ` Bruce Richardson
  2015-02-23 15:30     ` Iremonger, Bernard
  2015-02-24  9:21     ` Tetsuya Mukawa
  2015-02-23 17:02   ` [dpdk-dev] [PATCH v2 0/2] fix and improve uio_pci_generic support Bruce Richardson
                     ` (2 subsequent siblings)
  3 siblings, 2 replies; 30+ messages in thread
From: Bruce Richardson @ 2015-02-23 15:00 UTC (permalink / raw)
  To: dev

On Mon, Feb 23, 2015 at 02:57:24PM +0000, Bruce Richardson wrote:
> Instead of distinguishing the BAR mappings via offset within a single
> file, originally /dev/uioX, switch to mapping each individual bar via
> the appropriately numbered resourceX file.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
Hi Tetsuya,

in our tests here, this patch seems to fix the immediate problem you were
experiencing on your system. Can you perhaps verify?

Thanks,
/Bruce

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [dpdk-dev] [PATCH] eal: mmap uio resources using resourceX files
  2015-02-23 15:00   ` Bruce Richardson
@ 2015-02-23 15:30     ` Iremonger, Bernard
  2015-02-24  9:21     ` Tetsuya Mukawa
  1 sibling, 0 replies; 30+ messages in thread
From: Iremonger, Bernard @ 2015-02-23 15:30 UTC (permalink / raw)
  To: Richardson, Bruce, dev



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> Sent: Monday, February 23, 2015 3:00 PM
> To: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] eal: mmap uio resources using resourceX files
> 
> On Mon, Feb 23, 2015 at 02:57:24PM +0000, Bruce Richardson wrote:
> > Instead of distinguishing the BAR mappings via offset within a single
> > file, originally /dev/uioX, switch to mapping each individual bar via
> > the appropriately numbered resourceX file.
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> Hi Tetsuya,
> 
> in our tests here, this patch seems to fix the immediate problem you were experiencing on your
> system. Can you perhaps verify?
> 
> Thanks,
> /Bruce

Hi Bruce,

I was seeing a similar problem on my system when attaching a virtual function port.

testpmd> port attach 0000:06:10.0 
Attaching a new port...
EAL: PCI device 0000:06:10.0 on NUMA socket -1
EAL:   probe driver: 8086:10ed rte_ixgbevf_pmd
EAL:   PCI memory mapped at 0x10000
EAL: pci_map_resource(): cannot mmap(27, 0x14000, 0x4000, 0x1000): Invalid argument (0xffffffffffffffff)
EAL: Requested device 0000:06:10.0 cannot be used
EAL: Driver, cannot attach the device

This patch seems to solve the problem.

Regards,

Bernard.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [dpdk-dev] [PATCH v2 0/2] fix and improve uio_pci_generic support
  2015-02-23 14:57 ` [dpdk-dev] [PATCH] eal: mmap uio resources using resourceX files Bruce Richardson
  2015-02-23 15:00   ` Bruce Richardson
@ 2015-02-23 17:02   ` Bruce Richardson
  2015-02-23 17:02     ` [dpdk-dev] [PATCH v2 1/2] eal: mmap uio resources using resourceX files Bruce Richardson
                       ` (2 more replies)
  2015-02-24 12:20   ` [dpdk-dev] [PATCH v3 " Bruce Richardson
  2015-02-24 13:30   ` [dpdk-dev] [PATCH v4 0/3] fix and improve uio_pci_generic support Bruce Richardson
  3 siblings, 3 replies; 30+ messages in thread
From: Bruce Richardson @ 2015-02-23 17:02 UTC (permalink / raw)
  To: dev

This patch does some cleanup of the uio mapping code to 
a) fix issue with mmap of PCI bars reported by Tetsuya and confirmed
by others.
b) eliminate redundant code and reduce scans of /sys


Bruce Richardson (2):
  eal: mmap uio resources using resourceX files
  eal: populate uio_maps from pci mem_resources array

 lib/librte_eal/common/include/rte_pci.h    |   2 +-
 lib/librte_eal/linuxapp/eal/eal_pci_init.h |   1 +
 lib/librte_eal/linuxapp/eal/eal_pci_uio.c  | 173 +++++++++++------------------
 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c |   1 +
 4 files changed, 66 insertions(+), 111 deletions(-)

-- 
2.1.0

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [dpdk-dev] [PATCH v2 1/2] eal: mmap uio resources using resourceX files
  2015-02-23 17:02   ` [dpdk-dev] [PATCH v2 0/2] fix and improve uio_pci_generic support Bruce Richardson
@ 2015-02-23 17:02     ` Bruce Richardson
  2015-02-23 17:02     ` [dpdk-dev] [PATCH v2 2/2] eal: populate uio_maps from pci mem_resources array Bruce Richardson
  2015-02-24 10:53     ` [dpdk-dev] [PATCH v2 0/2] fix and improve uio_pci_generic support Bruce Richardson
  2 siblings, 0 replies; 30+ messages in thread
From: Bruce Richardson @ 2015-02-23 17:02 UTC (permalink / raw)
  To: dev

Instead of distinguishing the BAR mappings via offset within a single
file, originally /dev/uioX, switch to mapping each individual bar via
the appropriately numbered resourceX file.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_eal/common/include/rte_pci.h    |  2 +-
 lib/librte_eal/linuxapp/eal/eal_pci_init.h |  1 +
 lib/librte_eal/linuxapp/eal/eal_pci_uio.c  | 34 ++++++++++++++++--------------
 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c |  1 +
 4 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 4301c16..e34b139 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -117,7 +117,7 @@ struct rte_pci_resource {
 };
 
 /** Maximum number of PCI resources. */
-#define PCI_MAX_RESOURCE 7
+#define PCI_MAX_RESOURCE 6
 
 /**
  * A structure describing an ID for a PCI driver. Each driver provides a
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_init.h b/lib/librte_eal/linuxapp/eal/eal_pci_init.h
index 1070eb8..2125d7b 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_init.h
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_init.h
@@ -38,6 +38,7 @@
 
 struct pci_map {
 	void *addr;
+	char *path;
 	uint64_t offset;
 	uint64_t size;
 	uint64_t phaddr;
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
index 2b16fcb..ecf385a 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
@@ -137,10 +137,10 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
 			/*
 			 * open devname, to mmap it
 			 */
-			fd = open(uio_res->path, O_RDWR);
+			fd = open(uio_res->maps[i].path, O_RDWR);
 			if (fd < 0) {
 				RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
-					uio_res->path, strerror(errno));
+					uio_res->maps[i].path, strerror(errno));
 				return -1;
 			}
 
@@ -149,7 +149,8 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
 					     (size_t)uio_res->maps[i].size)
 			    != uio_res->maps[i].addr) {
 				RTE_LOG(ERR, EAL,
-					"Cannot mmap device resource\n");
+					"Cannot mmap device resource file: %s\n",
+					uio_res->maps[i].path);
 				close(fd);
 				return -1;
 			}
@@ -294,8 +295,6 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 	void *mapaddr;
 	int uio_num;
 	uint64_t phaddr;
-	uint64_t offset;
-	uint64_t pagesz;
 	int nb_maps;
 	struct rte_pci_addr *loc = &dev->addr;
 	struct mapped_pci_resource *uio_res;
@@ -336,11 +335,6 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 		return -1;
 	}
 
-	/* update devname for mmap  */
-	snprintf(devname, sizeof(devname),
-		SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/resource%d",
-		loc->domain, loc->bus, loc->devid, loc->function, 0);
-
 	/* set bus master that is not done by uio_pci_generic */
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
 		if (pci_uio_set_bus_master(dev->intr_handle.uio_cfg_fd)) {
@@ -370,8 +364,6 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 	uio_res->nb_maps = nb_maps;
 
 	/* Map all BARs */
-	pagesz = sysconf(_SC_PAGESIZE);
-
 	maps = uio_res->maps;
 	for (i = 0; i != PCI_MAX_RESOURCE; i++) {
 		int fd;
@@ -389,10 +381,15 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 		/* if matching map is found, then use it */
 		if (j != nb_maps) {
 			int fail = 0;
-			offset = j * pagesz;
+
+			/* update devname for mmap  */
+			snprintf(devname, sizeof(devname),
+				SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/resource%d",
+				loc->domain, loc->bus, loc->devid, loc->function,
+				i);
 
 			/*
-			 * open devname, to mmap it
+			 * open resource file, to mmap it
 			 */
 			fd = open(devname, O_RDWR);
 			if (fd < 0) {
@@ -408,7 +405,7 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 				if (pci_map_addr == NULL)
 					pci_map_addr = pci_find_max_end_va();
 
-				mapaddr = pci_map_resource(pci_map_addr, fd, (off_t)offset,
+				mapaddr = pci_map_resource(pci_map_addr, fd, 0,
 						(size_t)maps[j].size);
 				if (mapaddr == MAP_FAILED)
 					fail = 1;
@@ -416,6 +413,10 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 				pci_map_addr = RTE_PTR_ADD(mapaddr, (size_t) maps[j].size);
 			}
 
+			maps[j].path = rte_malloc(NULL, strlen(devname) + 1, 0);
+			if (maps[j].path == NULL)
+				fail = 1;
+
 			if (fail) {
 				rte_free(uio_res);
 				close(fd);
@@ -424,7 +425,8 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 			close(fd);
 
 			maps[j].addr = mapaddr;
-			maps[j].offset = offset;
+			maps[j].offset = 0;
+			strcpy(maps[j].path, devname);
 			dev->mem_resource[i].addr = mapaddr;
 		}
 	}
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
index 20e0977..7a57d0f 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
@@ -751,6 +751,7 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
 		maps[i].addr = bar_addr;
 		maps[i].offset = reg.offset;
 		maps[i].size = reg.size;
+		maps[i].path = NULL; /* vfio doesn't have per-resource paths */
 		dev->mem_resource[i].addr = bar_addr;
 	}
 
-- 
2.1.0

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [dpdk-dev] [PATCH v2 2/2] eal: populate uio_maps from pci mem_resources array
  2015-02-23 17:02   ` [dpdk-dev] [PATCH v2 0/2] fix and improve uio_pci_generic support Bruce Richardson
  2015-02-23 17:02     ` [dpdk-dev] [PATCH v2 1/2] eal: mmap uio resources using resourceX files Bruce Richardson
@ 2015-02-23 17:02     ` Bruce Richardson
  2015-02-24 10:53     ` [dpdk-dev] [PATCH v2 0/2] fix and improve uio_pci_generic support Bruce Richardson
  2 siblings, 0 replies; 30+ messages in thread
From: Bruce Richardson @ 2015-02-23 17:02 UTC (permalink / raw)
  To: dev

Rather than scanning the resource file in sysfs a second time, we
can pull the information on physical addresses of BARs from the
pci resource information already present in the dev structure.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 163 +++++++++++-------------------
 1 file changed, 57 insertions(+), 106 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
index ecf385a..6aa2599 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
@@ -52,41 +52,6 @@ void *pci_map_addr = NULL;
 
 
 #define OFF_MAX              ((uint64_t)(off_t)-1)
-static int
-pci_uio_get_mappings(struct rte_pci_device *dev,
-			struct pci_map maps[], int nb_maps)
-{
-	struct rte_pci_addr *loc = &dev->addr;
-	int i = 0;
-	char filename[PATH_MAX];
-	unsigned long long start_addr, end_addr, flags;
-	FILE *f;
-
-	snprintf(filename, sizeof(filename),
-		SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/resource",
-		loc->domain, loc->bus, loc->devid, loc->function);
-
-	f = fopen(filename, "r");
-	if (f == NULL) {
-		RTE_LOG(ERR, EAL,
-		"%s(): cannot open sysfs %s\n",
-		__func__, filename);
-		return -1;
-	}
-
-	while (fscanf(f, "%llx %llx %llx", &start_addr,
-			&end_addr, &flags) == 3 && i < nb_maps) {
-		if (flags & IORESOURCE_MEM) {
-			maps[i].offset = 0x0;
-			maps[i].size = end_addr - start_addr + 1;
-			maps[i].phaddr = start_addr;
-			i++;
-		}
-	}
-	fclose(f);
-
-	return i;
-}
 
 static int
 pci_uio_set_bus_master(int dev_fd)
@@ -130,10 +95,6 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
 			continue;
 
 		for (i = 0; i != uio_res->nb_maps; i++) {
-			/* ignore mappings unused in primary process */
-			if (uio_res->maps[i].addr == NULL)
-				continue;
-
 			/*
 			 * open devname, to mmap it
 			 */
@@ -144,13 +105,21 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
 				return -1;
 			}
 
-			if (pci_map_resource(uio_res->maps[i].addr, fd,
-					     (off_t)uio_res->maps[i].offset,
-					     (size_t)uio_res->maps[i].size)
-			    != uio_res->maps[i].addr) {
-				RTE_LOG(ERR, EAL,
-					"Cannot mmap device resource file: %s\n",
-					uio_res->maps[i].path);
+			void *mapaddr = pci_map_resource(uio_res->maps[i].addr,
+					fd, (off_t)uio_res->maps[i].offset,
+					(size_t)uio_res->maps[i].size);
+			if (mapaddr != uio_res->maps[i].addr) {
+				if (mapaddr == MAP_FAILED)
+					RTE_LOG(ERR, EAL,
+							"Cannot mmap device resource file %s: %s\n",
+							uio_res->maps[i].path,
+							strerror(errno));
+				else
+					RTE_LOG(ERR, EAL,
+							"Cannot mmap device resource file %s to address: %p\n",
+							uio_res->maps[i].path,
+							uio_res->maps[i].addr);
+
 				close(fd);
 				return -1;
 			}
@@ -288,14 +257,13 @@ pci_get_uio_dev(struct rte_pci_device *dev, char *dstbuf,
 int
 pci_uio_map_resource(struct rte_pci_device *dev)
 {
-	int i, j;
+	int i, map_idx;
 	char dirname[PATH_MAX];
 	char cfgname[PATH_MAX];
 	char devname[PATH_MAX]; /* contains the /dev/uioX */
 	void *mapaddr;
 	int uio_num;
 	uint64_t phaddr;
-	int nb_maps;
 	struct rte_pci_addr *loc = &dev->addr;
 	struct mapped_pci_resource *uio_res;
 	struct pci_map *maps;
@@ -336,11 +304,9 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 	}
 
 	/* set bus master that is not done by uio_pci_generic */
-	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
-		if (pci_uio_set_bus_master(dev->intr_handle.uio_cfg_fd)) {
-			RTE_LOG(ERR, EAL, "Cannot set up bus mastering!\n");
-			return -1;
-		}
+	if (pci_uio_set_bus_master(dev->intr_handle.uio_cfg_fd)) {
+		RTE_LOG(ERR, EAL, "Cannot set up bus mastering!\n");
+		return -1;
 	}
 
 	/* allocate the mapping details for secondary processes*/
@@ -354,83 +320,68 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 	snprintf(uio_res->path, sizeof(uio_res->path), "%s", devname);
 	memcpy(&uio_res->pci_addr, &dev->addr, sizeof(uio_res->pci_addr));
 
-	/* collect info about device mappings */
-	nb_maps = pci_uio_get_mappings(dev, uio_res->maps,
-					RTE_DIM(uio_res->maps));
-	if (nb_maps < 0) {
-		rte_free(uio_res);
-		return nb_maps;
-	}
-	uio_res->nb_maps = nb_maps;
-
 	/* Map all BARs */
 	maps = uio_res->maps;
-	for (i = 0; i != PCI_MAX_RESOURCE; i++) {
+	for (i = 0, map_idx = 0; i != PCI_MAX_RESOURCE; i++) {
 		int fd;
+		int fail = 0;
 
 		/* skip empty BAR */
 		phaddr = dev->mem_resource[i].phys_addr;
 		if (phaddr == 0)
 			continue;
 
-		for (j = 0; j != nb_maps && (phaddr != maps[j].phaddr ||
-				dev->mem_resource[i].len != maps[j].size);
-				j++)
-			;
-
-		/* if matching map is found, then use it */
-		if (j != nb_maps) {
-			int fail = 0;
 
-			/* update devname for mmap  */
-			snprintf(devname, sizeof(devname),
+		/* update devname for mmap  */
+		snprintf(devname, sizeof(devname),
 				SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/resource%d",
 				loc->domain, loc->bus, loc->devid, loc->function,
 				i);
 
-			/*
-			 * open resource file, to mmap it
-			 */
-			fd = open(devname, O_RDWR);
-			if (fd < 0) {
-				RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
+		/*
+		 * open resource file, to mmap it
+		 */
+		fd = open(devname, O_RDWR);
+		if (fd < 0) {
+			RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
 					devname, strerror(errno));
-				return -1;
-			}
+			return -1;
+		}
 
-			if (maps[j].addr != NULL)
-				fail = 1;
-			else {
-				/* try mapping somewhere close to the end of hugepages */
-				if (pci_map_addr == NULL)
-					pci_map_addr = pci_find_max_end_va();
+		/* try mapping somewhere close to the end of hugepages */
+		if (pci_map_addr == NULL)
+			pci_map_addr = pci_find_max_end_va();
 
-				mapaddr = pci_map_resource(pci_map_addr, fd, 0,
-						(size_t)maps[j].size);
-				if (mapaddr == MAP_FAILED)
-					fail = 1;
+		mapaddr = pci_map_resource(pci_map_addr, fd, 0,
+				(size_t)dev->mem_resource[i].len);
+		if (mapaddr == MAP_FAILED)
+			fail = 1;
 
-				pci_map_addr = RTE_PTR_ADD(mapaddr, (size_t) maps[j].size);
-			}
+		pci_map_addr = RTE_PTR_ADD(mapaddr,
+				(size_t)dev->mem_resource[i].len);
 
-			maps[j].path = rte_malloc(NULL, strlen(devname) + 1, 0);
-			if (maps[j].path == NULL)
-				fail = 1;
+		maps[map_idx].path = rte_malloc(NULL, strlen(devname) + 1, 0);
+		if (maps[map_idx].path == NULL)
+			fail = 1;
 
-			if (fail) {
-				rte_free(uio_res);
-				close(fd);
-				return -1;
-			}
+		if (fail) {
+			rte_free(uio_res);
 			close(fd);
-
-			maps[j].addr = mapaddr;
-			maps[j].offset = 0;
-			strcpy(maps[j].path, devname);
-			dev->mem_resource[i].addr = mapaddr;
+			return -1;
 		}
+		close(fd);
+
+		maps[map_idx].phaddr = dev->mem_resource[i].phys_addr;
+		maps[map_idx].size = dev->mem_resource[i].len;
+		maps[map_idx].addr = mapaddr;
+		maps[map_idx].offset = 0;
+		strcpy(maps[map_idx].path, devname);
+		map_idx++;
+		dev->mem_resource[i].addr = mapaddr;
 	}
 
+	uio_res->nb_maps = map_idx;
+
 	TAILQ_INSERT_TAIL(pci_res_list, uio_res, next);
 
 	return 0;
-- 
2.1.0

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [dpdk-dev] [PATCH] eal: mmap uio resources using resourceX files
  2015-02-23 15:00   ` Bruce Richardson
  2015-02-23 15:30     ` Iremonger, Bernard
@ 2015-02-24  9:21     ` Tetsuya Mukawa
  1 sibling, 0 replies; 30+ messages in thread
From: Tetsuya Mukawa @ 2015-02-24  9:21 UTC (permalink / raw)
  To: Bruce Richardson, dev

On 2015/02/24 0:00, Bruce Richardson wrote:
> On Mon, Feb 23, 2015 at 02:57:24PM +0000, Bruce Richardson wrote:
>> Instead of distinguishing the BAR mappings via offset within a single
>> file, originally /dev/uioX, switch to mapping each individual bar via
>> the appropriately numbered resourceX file.
>>
>> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
>> ---
> Hi Tetsuya,
>
> in our tests here, this patch seems to fix the immediate problem you were
> experiencing on your system. Can you perhaps verify?
>
> Thanks,
> /Bruce
>
Hi Bruce,

I've checked it, and the patch solves my issue.
Thank you so much.

Thanks,
Tetsuya

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [dpdk-dev] [PATCH v2 0/2] fix and improve uio_pci_generic support
  2015-02-23 17:02   ` [dpdk-dev] [PATCH v2 0/2] fix and improve uio_pci_generic support Bruce Richardson
  2015-02-23 17:02     ` [dpdk-dev] [PATCH v2 1/2] eal: mmap uio resources using resourceX files Bruce Richardson
  2015-02-23 17:02     ` [dpdk-dev] [PATCH v2 2/2] eal: populate uio_maps from pci mem_resources array Bruce Richardson
@ 2015-02-24 10:53     ` Bruce Richardson
  2015-02-24 11:23       ` David Marchand
  2 siblings, 1 reply; 30+ messages in thread
From: Bruce Richardson @ 2015-02-24 10:53 UTC (permalink / raw)
  To: david.marchand; +Cc: dev

On Mon, Feb 23, 2015 at 05:02:33PM +0000, Bruce Richardson wrote:
> This patch does some cleanup of the uio mapping code to 
> a) fix issue with mmap of PCI bars reported by Tetsuya and confirmed
> by others.
> b) eliminate redundant code and reduce scans of /sys
> 
> 
> Bruce Richardson (2):
>   eal: mmap uio resources using resourceX files
>   eal: populate uio_maps from pci mem_resources array
> 
>  lib/librte_eal/common/include/rte_pci.h    |   2 +-
>  lib/librte_eal/linuxapp/eal/eal_pci_init.h |   1 +
>  lib/librte_eal/linuxapp/eal/eal_pci_uio.c  | 173 +++++++++++------------------
>  lib/librte_eal/linuxapp/eal/eal_pci_vfio.c |   1 +
>  4 files changed, 66 insertions(+), 111 deletions(-)
> 
> -- 
> 2.1.0
> 
Hi David,

Given your previous suggestions on the uio_pic_generic code, I'd appreciate
any feedback you could provide on this patchset.

Regards,
/Bruce

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [dpdk-dev] [PATCH v2 0/2] fix and improve uio_pci_generic support
  2015-02-24 10:53     ` [dpdk-dev] [PATCH v2 0/2] fix and improve uio_pci_generic support Bruce Richardson
@ 2015-02-24 11:23       ` David Marchand
  2015-02-24 11:32         ` Bruce Richardson
  0 siblings, 1 reply; 30+ messages in thread
From: David Marchand @ 2015-02-24 11:23 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev

Hello Bruce,

On Tue, Feb 24, 2015 at 11:53 AM, Bruce Richardson <
bruce.richardson@intel.com> wrote:

> On Mon, Feb 23, 2015 at 05:02:33PM +0000, Bruce Richardson wrote:
> > This patch does some cleanup of the uio mapping code to
> > a) fix issue with mmap of PCI bars reported by Tetsuya and confirmed
> > by others.
> > b) eliminate redundant code and reduce scans of /sys
> >
> >
> > Bruce Richardson (2):
> >   eal: mmap uio resources using resourceX files
> >   eal: populate uio_maps from pci mem_resources array
> >
> >  lib/librte_eal/common/include/rte_pci.h    |   2 +-
> >  lib/librte_eal/linuxapp/eal/eal_pci_init.h |   1 +
> >  lib/librte_eal/linuxapp/eal/eal_pci_uio.c  | 173
> +++++++++++------------------
> >  lib/librte_eal/linuxapp/eal/eal_pci_vfio.c |   1 +
> >  4 files changed, 66 insertions(+), 111 deletions(-)
> >
> > --
> > 2.1.0
> >
> Given your previous suggestions on the uio_pic_generic code, I'd appreciate
> any feedback you could provide on this patchset.


Well, I only have one pending question on the use of resourceX files
instead of /dev/uioX.
You rely on sysfs mmap code for pci resources.
Is this really equivalent to uio mmap operations ?
If you can ensure me this won't break igb_uio setups, then these patches
are ok for me.

Thanks for the cleanup in eal_pci_uio.c.


-- 
David Marchand

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [dpdk-dev] [PATCH v2 0/2] fix and improve uio_pci_generic support
  2015-02-24 11:23       ` David Marchand
@ 2015-02-24 11:32         ` Bruce Richardson
  2015-02-24 12:35           ` David Marchand
  0 siblings, 1 reply; 30+ messages in thread
From: Bruce Richardson @ 2015-02-24 11:32 UTC (permalink / raw)
  To: David Marchand; +Cc: dev

On Tue, Feb 24, 2015 at 12:23:15PM +0100, David Marchand wrote:
> Hello Bruce,
> 
> On Tue, Feb 24, 2015 at 11:53 AM, Bruce Richardson <
> bruce.richardson@intel.com> wrote:
> 
> > On Mon, Feb 23, 2015 at 05:02:33PM +0000, Bruce Richardson wrote:
> > > This patch does some cleanup of the uio mapping code to
> > > a) fix issue with mmap of PCI bars reported by Tetsuya and confirmed
> > > by others.
> > > b) eliminate redundant code and reduce scans of /sys
> > >
> > >
> > > Bruce Richardson (2):
> > >   eal: mmap uio resources using resourceX files
> > >   eal: populate uio_maps from pci mem_resources array
> > >
> > >  lib/librte_eal/common/include/rte_pci.h    |   2 +-
> > >  lib/librte_eal/linuxapp/eal/eal_pci_init.h |   1 +
> > >  lib/librte_eal/linuxapp/eal/eal_pci_uio.c  | 173
> > +++++++++++------------------
> > >  lib/librte_eal/linuxapp/eal/eal_pci_vfio.c |   1 +
> > >  4 files changed, 66 insertions(+), 111 deletions(-)
> > >
> > > --
> > > 2.1.0
> > >
> > Given your previous suggestions on the uio_pic_generic code, I'd appreciate
> > any feedback you could provide on this patchset.
> 
> 
> Well, I only have one pending question on the use of resourceX files
> instead of /dev/uioX.
> You rely on sysfs mmap code for pci resources.
> Is this really equivalent to uio mmap operations ?

uio_pci_generic provides no mappings via /dev/uioX, so the may to mmap the 
bars using uio_pci_generic is via the sysfs. [1]

> If you can ensure me this won't break igb_uio setups, then these patches
> are ok for me.

Since igb_uio is based on the same uio_pci_generic framework, what works for
uio_pci_generic should work for igb_uio also - and testing indicates that this
works. Danny could perhaps provide better insights than I can into any guarantees
as to not breaking things, but I've tested this on a couple of platforms with
1G and 10G NICs using both igb_uio and uio_pci_generic, both individually and
in combination. I've also verified multiprocess support and done a quick sanity
test with 32-bit. Everything seems ok in testing thus far.

> 
> Thanks for the cleanup in eal_pci_uio.c.
> 
No problem.

> 
> -- 
> David Marchand

/Bruce

[1]: https://lkml.org/lkml/2012/3/20/375

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [dpdk-dev] [PATCH v3 0/2] fix and improve uio_pci_generic support
  2015-02-23 14:57 ` [dpdk-dev] [PATCH] eal: mmap uio resources using resourceX files Bruce Richardson
  2015-02-23 15:00   ` Bruce Richardson
  2015-02-23 17:02   ` [dpdk-dev] [PATCH v2 0/2] fix and improve uio_pci_generic support Bruce Richardson
@ 2015-02-24 12:20   ` Bruce Richardson
  2015-02-24 12:20     ` [dpdk-dev] [PATCH v3 1/2] eal: mmap uio resources using resourceX files Bruce Richardson
  2015-02-24 12:20     ` [dpdk-dev] [PATCH v3 2/2] eal: populate uio_maps from pci mem_resources array Bruce Richardson
  2015-02-24 13:30   ` [dpdk-dev] [PATCH v4 0/3] fix and improve uio_pci_generic support Bruce Richardson
  3 siblings, 2 replies; 30+ messages in thread
From: Bruce Richardson @ 2015-02-24 12:20 UTC (permalink / raw)
  To: dev

V3 changes: Rebase to take account of commit 90a1633b 
	"eal/linux: allow to map BARs with MSI-X tables"

This patch does some cleanup of the uio mapping code to 
a) fix issue with mmap of PCI bars reported by Tetsuya and confirmed
by others.
b) eliminate redundant code and reduce scans of /sys

Bruce Richardson (2):
  eal: mmap uio resources using resourceX files
  eal: populate uio_maps from pci mem_resources array

 lib/librte_eal/common/include/rte_pci.h    |   2 +-
 lib/librte_eal/linuxapp/eal/eal_pci_init.h |   1 +
 lib/librte_eal/linuxapp/eal/eal_pci_uio.c  | 173 +++++++++++------------------
 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c |   1 +
 4 files changed, 66 insertions(+), 111 deletions(-)

-- 
2.1.0

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [dpdk-dev] [PATCH v3 1/2] eal: mmap uio resources using resourceX files
  2015-02-24 12:20   ` [dpdk-dev] [PATCH v3 " Bruce Richardson
@ 2015-02-24 12:20     ` Bruce Richardson
  2015-02-24 12:39       ` David Marchand
  2015-02-24 12:20     ` [dpdk-dev] [PATCH v3 2/2] eal: populate uio_maps from pci mem_resources array Bruce Richardson
  1 sibling, 1 reply; 30+ messages in thread
From: Bruce Richardson @ 2015-02-24 12:20 UTC (permalink / raw)
  To: dev

Instead of distinguishing the BAR mappings via offset within a single
file, originally /dev/uioX, switch to mapping each individual bar via
the appropriately numbered resourceX file.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_eal/common/include/rte_pci.h    |  2 +-
 lib/librte_eal/linuxapp/eal/eal_pci_init.h |  1 +
 lib/librte_eal/linuxapp/eal/eal_pci_uio.c  | 34 ++++++++++++++++--------------
 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c |  1 +
 4 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 9836446..3df07e8 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -117,7 +117,7 @@ struct rte_pci_resource {
 };
 
 /** Maximum number of PCI resources. */
-#define PCI_MAX_RESOURCE 7
+#define PCI_MAX_RESOURCE 6
 
 /**
  * A structure describing an ID for a PCI driver. Each driver provides a
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_init.h b/lib/librte_eal/linuxapp/eal/eal_pci_init.h
index 0a0853d..03d2b52 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_init.h
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_init.h
@@ -38,6 +38,7 @@
 
 struct pci_map {
 	void *addr;
+	char *path;
 	uint64_t offset;
 	uint64_t size;
 	uint64_t phaddr;
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
index ff903d2..7dd9ce1 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
@@ -137,10 +137,10 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
 			/*
 			 * open devname, to mmap it
 			 */
-			fd = open(uio_res->path, O_RDWR);
+			fd = open(uio_res->maps[i].path, O_RDWR);
 			if (fd < 0) {
 				RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
-					uio_res->path, strerror(errno));
+					uio_res->maps[i].path, strerror(errno));
 				return -1;
 			}
 
@@ -149,7 +149,8 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
 					     (size_t)uio_res->maps[i].size, 0)
 			    != uio_res->maps[i].addr) {
 				RTE_LOG(ERR, EAL,
-					"Cannot mmap device resource\n");
+					"Cannot mmap device resource file: %s\n",
+					uio_res->maps[i].path);
 				close(fd);
 				return -1;
 			}
@@ -294,8 +295,6 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 	void *mapaddr;
 	int uio_num;
 	uint64_t phaddr;
-	uint64_t offset;
-	uint64_t pagesz;
 	int nb_maps;
 	struct rte_pci_addr *loc = &dev->addr;
 	struct mapped_pci_resource *uio_res;
@@ -336,11 +335,6 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 		return -1;
 	}
 
-	/* update devname for mmap  */
-	snprintf(devname, sizeof(devname),
-		SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/resource%d",
-		loc->domain, loc->bus, loc->devid, loc->function, 0);
-
 	/* set bus master that is not done by uio_pci_generic */
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
 		if (pci_uio_set_bus_master(dev->intr_handle.uio_cfg_fd)) {
@@ -370,8 +364,6 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 	uio_res->nb_maps = nb_maps;
 
 	/* Map all BARs */
-	pagesz = sysconf(_SC_PAGESIZE);
-
 	maps = uio_res->maps;
 	for (i = 0; i != PCI_MAX_RESOURCE; i++) {
 		int fd;
@@ -389,10 +381,15 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 		/* if matching map is found, then use it */
 		if (j != nb_maps) {
 			int fail = 0;
-			offset = j * pagesz;
+
+			/* update devname for mmap  */
+			snprintf(devname, sizeof(devname),
+				SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/resource%d",
+				loc->domain, loc->bus, loc->devid, loc->function,
+				i);
 
 			/*
-			 * open devname, to mmap it
+			 * open resource file, to mmap it
 			 */
 			fd = open(devname, O_RDWR);
 			if (fd < 0) {
@@ -408,7 +405,7 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 				if (pci_map_addr == NULL)
 					pci_map_addr = pci_find_max_end_va();
 
-				mapaddr = pci_map_resource(pci_map_addr, fd, (off_t)offset,
+				mapaddr = pci_map_resource(pci_map_addr, fd, 0,
 						(size_t)maps[j].size, 0);
 				if (mapaddr == MAP_FAILED)
 					fail = 1;
@@ -416,6 +413,10 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 				pci_map_addr = RTE_PTR_ADD(mapaddr, (size_t) maps[j].size);
 			}
 
+			maps[j].path = rte_malloc(NULL, strlen(devname) + 1, 0);
+			if (maps[j].path == NULL)
+				fail = 1;
+
 			if (fail) {
 				rte_free(uio_res);
 				close(fd);
@@ -424,7 +425,8 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 			close(fd);
 
 			maps[j].addr = mapaddr;
-			maps[j].offset = offset;
+			maps[j].offset = 0;
+			strcpy(maps[j].path, devname);
 			dev->mem_resource[i].addr = mapaddr;
 		}
 	}
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
index 0419172..ee9660f 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
@@ -829,6 +829,7 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
 		maps[i].addr = bar_addr;
 		maps[i].offset = reg.offset;
 		maps[i].size = reg.size;
+		maps[i].path = NULL; /* vfio doesn't have per-resource paths */
 		dev->mem_resource[i].addr = bar_addr;
 	}
 
-- 
2.1.0

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [dpdk-dev] [PATCH v3 2/2] eal: populate uio_maps from pci mem_resources array
  2015-02-24 12:20   ` [dpdk-dev] [PATCH v3 " Bruce Richardson
  2015-02-24 12:20     ` [dpdk-dev] [PATCH v3 1/2] eal: mmap uio resources using resourceX files Bruce Richardson
@ 2015-02-24 12:20     ` Bruce Richardson
  2015-02-24 12:38       ` David Marchand
  1 sibling, 1 reply; 30+ messages in thread
From: Bruce Richardson @ 2015-02-24 12:20 UTC (permalink / raw)
  To: dev

Rather than scanning the resource file in sysfs a second time, we
can pull the information on physical addresses of BARs from the
pci resource information already present in the dev structure.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 163 +++++++++++-------------------
 1 file changed, 57 insertions(+), 106 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
index 7dd9ce1..0a95376 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
@@ -52,41 +52,6 @@ void *pci_map_addr = NULL;
 
 
 #define OFF_MAX              ((uint64_t)(off_t)-1)
-static int
-pci_uio_get_mappings(struct rte_pci_device *dev,
-			struct pci_map maps[], int nb_maps)
-{
-	struct rte_pci_addr *loc = &dev->addr;
-	int i = 0;
-	char filename[PATH_MAX];
-	unsigned long long start_addr, end_addr, flags;
-	FILE *f;
-
-	snprintf(filename, sizeof(filename),
-		SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/resource",
-		loc->domain, loc->bus, loc->devid, loc->function);
-
-	f = fopen(filename, "r");
-	if (f == NULL) {
-		RTE_LOG(ERR, EAL,
-		"%s(): cannot open sysfs %s\n",
-		__func__, filename);
-		return -1;
-	}
-
-	while (fscanf(f, "%llx %llx %llx", &start_addr,
-			&end_addr, &flags) == 3 && i < nb_maps) {
-		if (flags & IORESOURCE_MEM) {
-			maps[i].offset = 0x0;
-			maps[i].size = end_addr - start_addr + 1;
-			maps[i].phaddr = start_addr;
-			i++;
-		}
-	}
-	fclose(f);
-
-	return i;
-}
 
 static int
 pci_uio_set_bus_master(int dev_fd)
@@ -130,10 +95,6 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
 			continue;
 
 		for (i = 0; i != uio_res->nb_maps; i++) {
-			/* ignore mappings unused in primary process */
-			if (uio_res->maps[i].addr == NULL)
-				continue;
-
 			/*
 			 * open devname, to mmap it
 			 */
@@ -144,13 +105,21 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
 				return -1;
 			}
 
-			if (pci_map_resource(uio_res->maps[i].addr, fd,
-					     (off_t)uio_res->maps[i].offset,
-					     (size_t)uio_res->maps[i].size, 0)
-			    != uio_res->maps[i].addr) {
-				RTE_LOG(ERR, EAL,
-					"Cannot mmap device resource file: %s\n",
-					uio_res->maps[i].path);
+			void *mapaddr = pci_map_resource(uio_res->maps[i].addr,
+					fd, (off_t)uio_res->maps[i].offset,
+					(size_t)uio_res->maps[i].size, 0);
+			if (mapaddr != uio_res->maps[i].addr) {
+				if (mapaddr == MAP_FAILED)
+					RTE_LOG(ERR, EAL,
+							"Cannot mmap device resource file %s: %s\n",
+							uio_res->maps[i].path,
+							strerror(errno));
+				else
+					RTE_LOG(ERR, EAL,
+							"Cannot mmap device resource file %s to address: %p\n",
+							uio_res->maps[i].path,
+							uio_res->maps[i].addr);
+
 				close(fd);
 				return -1;
 			}
@@ -288,14 +257,13 @@ pci_get_uio_dev(struct rte_pci_device *dev, char *dstbuf,
 int
 pci_uio_map_resource(struct rte_pci_device *dev)
 {
-	int i, j;
+	int i, map_idx;
 	char dirname[PATH_MAX];
 	char cfgname[PATH_MAX];
 	char devname[PATH_MAX]; /* contains the /dev/uioX */
 	void *mapaddr;
 	int uio_num;
 	uint64_t phaddr;
-	int nb_maps;
 	struct rte_pci_addr *loc = &dev->addr;
 	struct mapped_pci_resource *uio_res;
 	struct pci_map *maps;
@@ -336,11 +304,9 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 	}
 
 	/* set bus master that is not done by uio_pci_generic */
-	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
-		if (pci_uio_set_bus_master(dev->intr_handle.uio_cfg_fd)) {
-			RTE_LOG(ERR, EAL, "Cannot set up bus mastering!\n");
-			return -1;
-		}
+	if (pci_uio_set_bus_master(dev->intr_handle.uio_cfg_fd)) {
+		RTE_LOG(ERR, EAL, "Cannot set up bus mastering!\n");
+		return -1;
 	}
 
 	/* allocate the mapping details for secondary processes*/
@@ -354,83 +320,68 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 	snprintf(uio_res->path, sizeof(uio_res->path), "%s", devname);
 	memcpy(&uio_res->pci_addr, &dev->addr, sizeof(uio_res->pci_addr));
 
-	/* collect info about device mappings */
-	nb_maps = pci_uio_get_mappings(dev, uio_res->maps,
-					RTE_DIM(uio_res->maps));
-	if (nb_maps < 0) {
-		rte_free(uio_res);
-		return nb_maps;
-	}
-	uio_res->nb_maps = nb_maps;
-
 	/* Map all BARs */
 	maps = uio_res->maps;
-	for (i = 0; i != PCI_MAX_RESOURCE; i++) {
+	for (i = 0, map_idx = 0; i != PCI_MAX_RESOURCE; i++) {
 		int fd;
+		int fail = 0;
 
 		/* skip empty BAR */
 		phaddr = dev->mem_resource[i].phys_addr;
 		if (phaddr == 0)
 			continue;
 
-		for (j = 0; j != nb_maps && (phaddr != maps[j].phaddr ||
-				dev->mem_resource[i].len != maps[j].size);
-				j++)
-			;
-
-		/* if matching map is found, then use it */
-		if (j != nb_maps) {
-			int fail = 0;
 
-			/* update devname for mmap  */
-			snprintf(devname, sizeof(devname),
+		/* update devname for mmap  */
+		snprintf(devname, sizeof(devname),
 				SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/resource%d",
 				loc->domain, loc->bus, loc->devid, loc->function,
 				i);
 
-			/*
-			 * open resource file, to mmap it
-			 */
-			fd = open(devname, O_RDWR);
-			if (fd < 0) {
-				RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
+		/*
+		 * open resource file, to mmap it
+		 */
+		fd = open(devname, O_RDWR);
+		if (fd < 0) {
+			RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
 					devname, strerror(errno));
-				return -1;
-			}
+			return -1;
+		}
 
-			if (maps[j].addr != NULL)
-				fail = 1;
-			else {
-				/* try mapping somewhere close to the end of hugepages */
-				if (pci_map_addr == NULL)
-					pci_map_addr = pci_find_max_end_va();
+		/* try mapping somewhere close to the end of hugepages */
+		if (pci_map_addr == NULL)
+			pci_map_addr = pci_find_max_end_va();
 
-				mapaddr = pci_map_resource(pci_map_addr, fd, 0,
-						(size_t)maps[j].size, 0);
-				if (mapaddr == MAP_FAILED)
-					fail = 1;
+		mapaddr = pci_map_resource(pci_map_addr, fd, 0,
+				(size_t)dev->mem_resource[i].len, 0);
+		if (mapaddr == MAP_FAILED)
+			fail = 1;
 
-				pci_map_addr = RTE_PTR_ADD(mapaddr, (size_t) maps[j].size);
-			}
+		pci_map_addr = RTE_PTR_ADD(mapaddr,
+				(size_t)dev->mem_resource[i].len);
 
-			maps[j].path = rte_malloc(NULL, strlen(devname) + 1, 0);
-			if (maps[j].path == NULL)
-				fail = 1;
+		maps[map_idx].path = rte_malloc(NULL, strlen(devname) + 1, 0);
+		if (maps[map_idx].path == NULL)
+			fail = 1;
 
-			if (fail) {
-				rte_free(uio_res);
-				close(fd);
-				return -1;
-			}
+		if (fail) {
+			rte_free(uio_res);
 			close(fd);
-
-			maps[j].addr = mapaddr;
-			maps[j].offset = 0;
-			strcpy(maps[j].path, devname);
-			dev->mem_resource[i].addr = mapaddr;
+			return -1;
 		}
+		close(fd);
+
+		maps[map_idx].phaddr = dev->mem_resource[i].phys_addr;
+		maps[map_idx].size = dev->mem_resource[i].len;
+		maps[map_idx].addr = mapaddr;
+		maps[map_idx].offset = 0;
+		strcpy(maps[map_idx].path, devname);
+		map_idx++;
+		dev->mem_resource[i].addr = mapaddr;
 	}
 
+	uio_res->nb_maps = map_idx;
+
 	TAILQ_INSERT_TAIL(pci_res_list, uio_res, next);
 
 	return 0;
-- 
2.1.0

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [dpdk-dev] [PATCH v2 0/2] fix and improve uio_pci_generic support
  2015-02-24 11:32         ` Bruce Richardson
@ 2015-02-24 12:35           ` David Marchand
  0 siblings, 0 replies; 30+ messages in thread
From: David Marchand @ 2015-02-24 12:35 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev

On Tue, Feb 24, 2015 at 12:32 PM, Bruce Richardson <
bruce.richardson@intel.com> wrote:

> On Tue, Feb 24, 2015 at 12:23:15PM +0100, David Marchand wrote:
> > Hello Bruce,
> >
> > On Tue, Feb 24, 2015 at 11:53 AM, Bruce Richardson <
> > bruce.richardson@intel.com> wrote:
> >
> > > On Mon, Feb 23, 2015 at 05:02:33PM +0000, Bruce Richardson wrote:
> > > > This patch does some cleanup of the uio mapping code to
> > > > a) fix issue with mmap of PCI bars reported by Tetsuya and confirmed
> > > > by others.
> > > > b) eliminate redundant code and reduce scans of /sys
> > > >
> > > >
> > > > Bruce Richardson (2):
> > > >   eal: mmap uio resources using resourceX files
> > > >   eal: populate uio_maps from pci mem_resources array
> > > >
> > > >  lib/librte_eal/common/include/rte_pci.h    |   2 +-
> > > >  lib/librte_eal/linuxapp/eal/eal_pci_init.h |   1 +
> > > >  lib/librte_eal/linuxapp/eal/eal_pci_uio.c  | 173
> > > +++++++++++------------------
> > > >  lib/librte_eal/linuxapp/eal/eal_pci_vfio.c |   1 +
> > > >  4 files changed, 66 insertions(+), 111 deletions(-)
> > > >
> > > > --
> > > > 2.1.0
> > > >
> > > Given your previous suggestions on the uio_pic_generic code, I'd
> appreciate
> > > any feedback you could provide on this patchset.
> >
> >
> > Well, I only have one pending question on the use of resourceX files
> > instead of /dev/uioX.
> > You rely on sysfs mmap code for pci resources.
> > Is this really equivalent to uio mmap operations ?
>
> uio_pci_generic provides no mappings via /dev/uioX, so the may to mmap the
> bars using uio_pci_generic is via the sysfs. [1]
>
> > If you can ensure me this won't break igb_uio setups, then these patches
> > are ok for me.
>
> Since igb_uio is based on the same uio_pci_generic framework, what works
> for
> uio_pci_generic should work for igb_uio also - and testing indicates that
> this
> works. Danny could perhaps provide better insights than I can into any
> guarantees
> as to not breaking things, but I've tested this on a couple of platforms
> with
> 1G and 10G NICs using both igb_uio and uio_pci_generic, both individually
> and
> in combination. I've also verified multiprocess support and done a quick
> sanity
> test with 32-bit. Everything seems ok in testing thus far.
>

Ok, noted.
I am looking at your v3.

-- 
David Marchand

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [dpdk-dev] [PATCH v3 2/2] eal: populate uio_maps from pci mem_resources array
  2015-02-24 12:20     ` [dpdk-dev] [PATCH v3 2/2] eal: populate uio_maps from pci mem_resources array Bruce Richardson
@ 2015-02-24 12:38       ` David Marchand
  0 siblings, 0 replies; 30+ messages in thread
From: David Marchand @ 2015-02-24 12:38 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev

On Tue, Feb 24, 2015 at 1:20 PM, Bruce Richardson <
bruce.richardson@intel.com> wrote:

> Rather than scanning the resource file in sysfs a second time, we
> can pull the information on physical addresses of BARs from the
> pci resource information already present in the dev structure.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 163
> +++++++++++-------------------
>  1 file changed, 57 insertions(+), 106 deletions(-)
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> index 7dd9ce1..0a95376 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
>
>
[snip]

@@ -336,11 +304,9 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>         }
>
>         /* set bus master that is not done by uio_pci_generic */
> -       if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> -               if (pci_uio_set_bus_master(dev->intr_handle.uio_cfg_fd)) {
> -                       RTE_LOG(ERR, EAL, "Cannot set up bus
> mastering!\n");
> -                       return -1;
> -               }
> +       if (pci_uio_set_bus_master(dev->intr_handle.uio_cfg_fd)) {
> +               RTE_LOG(ERR, EAL, "Cannot set up bus mastering!\n");
> +               return -1;
>         }
>
>         /* allocate the mapping details for secondary processes*/
>

Would be better in a different patch (at least the commit log does not tell
anything about this removed check).

Then ack.

-- 
David Marchand

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [dpdk-dev] [PATCH v3 1/2] eal: mmap uio resources using resourceX files
  2015-02-24 12:20     ` [dpdk-dev] [PATCH v3 1/2] eal: mmap uio resources using resourceX files Bruce Richardson
@ 2015-02-24 12:39       ` David Marchand
  0 siblings, 0 replies; 30+ messages in thread
From: David Marchand @ 2015-02-24 12:39 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev

On Tue, Feb 24, 2015 at 1:20 PM, Bruce Richardson <
bruce.richardson@intel.com> wrote:

> Instead of distinguishing the BAR mappings via offset within a single
> file, originally /dev/uioX, switch to mapping each individual bar via
> the appropriately numbered resourceX file.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  lib/librte_eal/common/include/rte_pci.h    |  2 +-
>  lib/librte_eal/linuxapp/eal/eal_pci_init.h |  1 +
>  lib/librte_eal/linuxapp/eal/eal_pci_uio.c  | 34
> ++++++++++++++++--------------
>  lib/librte_eal/linuxapp/eal/eal_pci_vfio.c |  1 +
>  4 files changed, 21 insertions(+), 17 deletions(-)
>

As long as igb_uio / uio_pci_generic both work, this is fine for me.

ack.

-- 
David Marchand

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [dpdk-dev] [PATCH v4 0/3] fix and improve uio_pci_generic support
  2015-02-23 14:57 ` [dpdk-dev] [PATCH] eal: mmap uio resources using resourceX files Bruce Richardson
                     ` (2 preceding siblings ...)
  2015-02-24 12:20   ` [dpdk-dev] [PATCH v3 " Bruce Richardson
@ 2015-02-24 13:30   ` Bruce Richardson
  2015-02-24 13:30     ` [dpdk-dev] [PATCH v4 1/3] eal: mmap uio resources using resourceX files Bruce Richardson
                       ` (3 more replies)
  3 siblings, 4 replies; 30+ messages in thread
From: Bruce Richardson @ 2015-02-24 13:30 UTC (permalink / raw)
  To: dev

V4 changes: Split second patch into two for more readable history

V3 changes: Rebase to take account of commit 90a1633b 
	"eal/linux: allow to map BARs with MSI-X tables"

This patch does some cleanup of the uio mapping code to 
a) fix issue with mmap of PCI bars reported by Tetsuya and confirmed
by others.
b) eliminate redundant code and reduce scans of /sys

Bruce Richardson (3):
  eal: mmap uio resources using resourceX files
  eal: populate uio_maps from pci mem_resources array
  eal: remove unnecessary check for primary instance

 lib/librte_eal/common/include/rte_pci.h    |   2 +-
 lib/librte_eal/linuxapp/eal/eal_pci_init.h |   1 +
 lib/librte_eal/linuxapp/eal/eal_pci_uio.c  | 173 +++++++++++------------------
 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c |   1 +
 4 files changed, 66 insertions(+), 111 deletions(-)

-- 
2.1.0

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [dpdk-dev] [PATCH v4 1/3] eal: mmap uio resources using resourceX files
  2015-02-24 13:30   ` [dpdk-dev] [PATCH v4 0/3] fix and improve uio_pci_generic support Bruce Richardson
@ 2015-02-24 13:30     ` Bruce Richardson
  2015-02-24 13:30     ` [dpdk-dev] [PATCH v4 2/3] eal: populate uio_maps from pci mem_resources array Bruce Richardson
                       ` (2 subsequent siblings)
  3 siblings, 0 replies; 30+ messages in thread
From: Bruce Richardson @ 2015-02-24 13:30 UTC (permalink / raw)
  To: dev

Instead of distinguishing the BAR mappings via offset within a single
file, originally /dev/uioX, switch to mapping each individual bar via
the appropriately numbered resourceX file.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_eal/common/include/rte_pci.h    |  2 +-
 lib/librte_eal/linuxapp/eal/eal_pci_init.h |  1 +
 lib/librte_eal/linuxapp/eal/eal_pci_uio.c  | 34 ++++++++++++++++--------------
 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c |  1 +
 4 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 9836446..3df07e8 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -117,7 +117,7 @@ struct rte_pci_resource {
 };
 
 /** Maximum number of PCI resources. */
-#define PCI_MAX_RESOURCE 7
+#define PCI_MAX_RESOURCE 6
 
 /**
  * A structure describing an ID for a PCI driver. Each driver provides a
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_init.h b/lib/librte_eal/linuxapp/eal/eal_pci_init.h
index 0a0853d..03d2b52 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_init.h
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_init.h
@@ -38,6 +38,7 @@
 
 struct pci_map {
 	void *addr;
+	char *path;
 	uint64_t offset;
 	uint64_t size;
 	uint64_t phaddr;
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
index ff903d2..7dd9ce1 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
@@ -137,10 +137,10 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
 			/*
 			 * open devname, to mmap it
 			 */
-			fd = open(uio_res->path, O_RDWR);
+			fd = open(uio_res->maps[i].path, O_RDWR);
 			if (fd < 0) {
 				RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
-					uio_res->path, strerror(errno));
+					uio_res->maps[i].path, strerror(errno));
 				return -1;
 			}
 
@@ -149,7 +149,8 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
 					     (size_t)uio_res->maps[i].size, 0)
 			    != uio_res->maps[i].addr) {
 				RTE_LOG(ERR, EAL,
-					"Cannot mmap device resource\n");
+					"Cannot mmap device resource file: %s\n",
+					uio_res->maps[i].path);
 				close(fd);
 				return -1;
 			}
@@ -294,8 +295,6 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 	void *mapaddr;
 	int uio_num;
 	uint64_t phaddr;
-	uint64_t offset;
-	uint64_t pagesz;
 	int nb_maps;
 	struct rte_pci_addr *loc = &dev->addr;
 	struct mapped_pci_resource *uio_res;
@@ -336,11 +335,6 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 		return -1;
 	}
 
-	/* update devname for mmap  */
-	snprintf(devname, sizeof(devname),
-		SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/resource%d",
-		loc->domain, loc->bus, loc->devid, loc->function, 0);
-
 	/* set bus master that is not done by uio_pci_generic */
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
 		if (pci_uio_set_bus_master(dev->intr_handle.uio_cfg_fd)) {
@@ -370,8 +364,6 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 	uio_res->nb_maps = nb_maps;
 
 	/* Map all BARs */
-	pagesz = sysconf(_SC_PAGESIZE);
-
 	maps = uio_res->maps;
 	for (i = 0; i != PCI_MAX_RESOURCE; i++) {
 		int fd;
@@ -389,10 +381,15 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 		/* if matching map is found, then use it */
 		if (j != nb_maps) {
 			int fail = 0;
-			offset = j * pagesz;
+
+			/* update devname for mmap  */
+			snprintf(devname, sizeof(devname),
+				SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/resource%d",
+				loc->domain, loc->bus, loc->devid, loc->function,
+				i);
 
 			/*
-			 * open devname, to mmap it
+			 * open resource file, to mmap it
 			 */
 			fd = open(devname, O_RDWR);
 			if (fd < 0) {
@@ -408,7 +405,7 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 				if (pci_map_addr == NULL)
 					pci_map_addr = pci_find_max_end_va();
 
-				mapaddr = pci_map_resource(pci_map_addr, fd, (off_t)offset,
+				mapaddr = pci_map_resource(pci_map_addr, fd, 0,
 						(size_t)maps[j].size, 0);
 				if (mapaddr == MAP_FAILED)
 					fail = 1;
@@ -416,6 +413,10 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 				pci_map_addr = RTE_PTR_ADD(mapaddr, (size_t) maps[j].size);
 			}
 
+			maps[j].path = rte_malloc(NULL, strlen(devname) + 1, 0);
+			if (maps[j].path == NULL)
+				fail = 1;
+
 			if (fail) {
 				rte_free(uio_res);
 				close(fd);
@@ -424,7 +425,8 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 			close(fd);
 
 			maps[j].addr = mapaddr;
-			maps[j].offset = offset;
+			maps[j].offset = 0;
+			strcpy(maps[j].path, devname);
 			dev->mem_resource[i].addr = mapaddr;
 		}
 	}
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
index 0419172..ee9660f 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
@@ -829,6 +829,7 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
 		maps[i].addr = bar_addr;
 		maps[i].offset = reg.offset;
 		maps[i].size = reg.size;
+		maps[i].path = NULL; /* vfio doesn't have per-resource paths */
 		dev->mem_resource[i].addr = bar_addr;
 	}
 
-- 
2.1.0

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [dpdk-dev] [PATCH v4 2/3] eal: populate uio_maps from pci mem_resources array
  2015-02-24 13:30   ` [dpdk-dev] [PATCH v4 0/3] fix and improve uio_pci_generic support Bruce Richardson
  2015-02-24 13:30     ` [dpdk-dev] [PATCH v4 1/3] eal: mmap uio resources using resourceX files Bruce Richardson
@ 2015-02-24 13:30     ` Bruce Richardson
  2015-02-24 13:30     ` [dpdk-dev] [PATCH v4 3/3] eal: remove unnecessary check for primary instance Bruce Richardson
  2015-02-24 21:33     ` [dpdk-dev] [PATCH v4 0/3] fix and improve uio_pci_generic support Thomas Monjalon
  3 siblings, 0 replies; 30+ messages in thread
From: Bruce Richardson @ 2015-02-24 13:30 UTC (permalink / raw)
  To: dev

Rather than scanning the resource file in sysfs a second time, we
can pull the information on physical addresses of BARs from the
pci resource information already present in the dev structure.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 155 +++++++++++-------------------
 1 file changed, 54 insertions(+), 101 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
index 7dd9ce1..c12794e 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
@@ -52,41 +52,6 @@ void *pci_map_addr = NULL;
 
 
 #define OFF_MAX              ((uint64_t)(off_t)-1)
-static int
-pci_uio_get_mappings(struct rte_pci_device *dev,
-			struct pci_map maps[], int nb_maps)
-{
-	struct rte_pci_addr *loc = &dev->addr;
-	int i = 0;
-	char filename[PATH_MAX];
-	unsigned long long start_addr, end_addr, flags;
-	FILE *f;
-
-	snprintf(filename, sizeof(filename),
-		SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/resource",
-		loc->domain, loc->bus, loc->devid, loc->function);
-
-	f = fopen(filename, "r");
-	if (f == NULL) {
-		RTE_LOG(ERR, EAL,
-		"%s(): cannot open sysfs %s\n",
-		__func__, filename);
-		return -1;
-	}
-
-	while (fscanf(f, "%llx %llx %llx", &start_addr,
-			&end_addr, &flags) == 3 && i < nb_maps) {
-		if (flags & IORESOURCE_MEM) {
-			maps[i].offset = 0x0;
-			maps[i].size = end_addr - start_addr + 1;
-			maps[i].phaddr = start_addr;
-			i++;
-		}
-	}
-	fclose(f);
-
-	return i;
-}
 
 static int
 pci_uio_set_bus_master(int dev_fd)
@@ -130,10 +95,6 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
 			continue;
 
 		for (i = 0; i != uio_res->nb_maps; i++) {
-			/* ignore mappings unused in primary process */
-			if (uio_res->maps[i].addr == NULL)
-				continue;
-
 			/*
 			 * open devname, to mmap it
 			 */
@@ -144,13 +105,21 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
 				return -1;
 			}
 
-			if (pci_map_resource(uio_res->maps[i].addr, fd,
-					     (off_t)uio_res->maps[i].offset,
-					     (size_t)uio_res->maps[i].size, 0)
-			    != uio_res->maps[i].addr) {
-				RTE_LOG(ERR, EAL,
-					"Cannot mmap device resource file: %s\n",
-					uio_res->maps[i].path);
+			void *mapaddr = pci_map_resource(uio_res->maps[i].addr,
+					fd, (off_t)uio_res->maps[i].offset,
+					(size_t)uio_res->maps[i].size, 0);
+			if (mapaddr != uio_res->maps[i].addr) {
+				if (mapaddr == MAP_FAILED)
+					RTE_LOG(ERR, EAL,
+							"Cannot mmap device resource file %s: %s\n",
+							uio_res->maps[i].path,
+							strerror(errno));
+				else
+					RTE_LOG(ERR, EAL,
+							"Cannot mmap device resource file %s to address: %p\n",
+							uio_res->maps[i].path,
+							uio_res->maps[i].addr);
+
 				close(fd);
 				return -1;
 			}
@@ -288,14 +257,13 @@ pci_get_uio_dev(struct rte_pci_device *dev, char *dstbuf,
 int
 pci_uio_map_resource(struct rte_pci_device *dev)
 {
-	int i, j;
+	int i, map_idx;
 	char dirname[PATH_MAX];
 	char cfgname[PATH_MAX];
 	char devname[PATH_MAX]; /* contains the /dev/uioX */
 	void *mapaddr;
 	int uio_num;
 	uint64_t phaddr;
-	int nb_maps;
 	struct rte_pci_addr *loc = &dev->addr;
 	struct mapped_pci_resource *uio_res;
 	struct pci_map *maps;
@@ -354,83 +322,68 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 	snprintf(uio_res->path, sizeof(uio_res->path), "%s", devname);
 	memcpy(&uio_res->pci_addr, &dev->addr, sizeof(uio_res->pci_addr));
 
-	/* collect info about device mappings */
-	nb_maps = pci_uio_get_mappings(dev, uio_res->maps,
-					RTE_DIM(uio_res->maps));
-	if (nb_maps < 0) {
-		rte_free(uio_res);
-		return nb_maps;
-	}
-	uio_res->nb_maps = nb_maps;
-
 	/* Map all BARs */
 	maps = uio_res->maps;
-	for (i = 0; i != PCI_MAX_RESOURCE; i++) {
+	for (i = 0, map_idx = 0; i != PCI_MAX_RESOURCE; i++) {
 		int fd;
+		int fail = 0;
 
 		/* skip empty BAR */
 		phaddr = dev->mem_resource[i].phys_addr;
 		if (phaddr == 0)
 			continue;
 
-		for (j = 0; j != nb_maps && (phaddr != maps[j].phaddr ||
-				dev->mem_resource[i].len != maps[j].size);
-				j++)
-			;
 
-		/* if matching map is found, then use it */
-		if (j != nb_maps) {
-			int fail = 0;
-
-			/* update devname for mmap  */
-			snprintf(devname, sizeof(devname),
+		/* update devname for mmap  */
+		snprintf(devname, sizeof(devname),
 				SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/resource%d",
 				loc->domain, loc->bus, loc->devid, loc->function,
 				i);
 
-			/*
-			 * open resource file, to mmap it
-			 */
-			fd = open(devname, O_RDWR);
-			if (fd < 0) {
-				RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
+		/*
+		 * open resource file, to mmap it
+		 */
+		fd = open(devname, O_RDWR);
+		if (fd < 0) {
+			RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
 					devname, strerror(errno));
-				return -1;
-			}
+			return -1;
+		}
 
-			if (maps[j].addr != NULL)
-				fail = 1;
-			else {
-				/* try mapping somewhere close to the end of hugepages */
-				if (pci_map_addr == NULL)
-					pci_map_addr = pci_find_max_end_va();
+		/* try mapping somewhere close to the end of hugepages */
+		if (pci_map_addr == NULL)
+			pci_map_addr = pci_find_max_end_va();
 
-				mapaddr = pci_map_resource(pci_map_addr, fd, 0,
-						(size_t)maps[j].size, 0);
-				if (mapaddr == MAP_FAILED)
-					fail = 1;
+		mapaddr = pci_map_resource(pci_map_addr, fd, 0,
+				(size_t)dev->mem_resource[i].len, 0);
+		if (mapaddr == MAP_FAILED)
+			fail = 1;
 
-				pci_map_addr = RTE_PTR_ADD(mapaddr, (size_t) maps[j].size);
-			}
+		pci_map_addr = RTE_PTR_ADD(mapaddr,
+				(size_t)dev->mem_resource[i].len);
 
-			maps[j].path = rte_malloc(NULL, strlen(devname) + 1, 0);
-			if (maps[j].path == NULL)
-				fail = 1;
+		maps[map_idx].path = rte_malloc(NULL, strlen(devname) + 1, 0);
+		if (maps[map_idx].path == NULL)
+			fail = 1;
 
-			if (fail) {
-				rte_free(uio_res);
-				close(fd);
-				return -1;
-			}
+		if (fail) {
+			rte_free(uio_res);
 			close(fd);
-
-			maps[j].addr = mapaddr;
-			maps[j].offset = 0;
-			strcpy(maps[j].path, devname);
-			dev->mem_resource[i].addr = mapaddr;
+			return -1;
 		}
+		close(fd);
+
+		maps[map_idx].phaddr = dev->mem_resource[i].phys_addr;
+		maps[map_idx].size = dev->mem_resource[i].len;
+		maps[map_idx].addr = mapaddr;
+		maps[map_idx].offset = 0;
+		strcpy(maps[map_idx].path, devname);
+		map_idx++;
+		dev->mem_resource[i].addr = mapaddr;
 	}
 
+	uio_res->nb_maps = map_idx;
+
 	TAILQ_INSERT_TAIL(pci_res_list, uio_res, next);
 
 	return 0;
-- 
2.1.0

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [dpdk-dev] [PATCH v4 3/3] eal: remove unnecessary check for primary instance
  2015-02-24 13:30   ` [dpdk-dev] [PATCH v4 0/3] fix and improve uio_pci_generic support Bruce Richardson
  2015-02-24 13:30     ` [dpdk-dev] [PATCH v4 1/3] eal: mmap uio resources using resourceX files Bruce Richardson
  2015-02-24 13:30     ` [dpdk-dev] [PATCH v4 2/3] eal: populate uio_maps from pci mem_resources array Bruce Richardson
@ 2015-02-24 13:30     ` Bruce Richardson
  2015-02-24 21:33     ` [dpdk-dev] [PATCH v4 0/3] fix and improve uio_pci_generic support Thomas Monjalon
  3 siblings, 0 replies; 30+ messages in thread
From: Bruce Richardson @ 2015-02-24 13:30 UTC (permalink / raw)
  To: dev

In pci_uio_map_resource we check that we are in a primary process
before calling pci_uio_set_bus_master. However, there is already
an earlier check which means that we are always in a primary instance
at this point in the code, so the check can be removed.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
index c12794e..0a95376 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
@@ -304,11 +304,9 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 	}
 
 	/* set bus master that is not done by uio_pci_generic */
-	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
-		if (pci_uio_set_bus_master(dev->intr_handle.uio_cfg_fd)) {
-			RTE_LOG(ERR, EAL, "Cannot set up bus mastering!\n");
-			return -1;
-		}
+	if (pci_uio_set_bus_master(dev->intr_handle.uio_cfg_fd)) {
+		RTE_LOG(ERR, EAL, "Cannot set up bus mastering!\n");
+		return -1;
 	}
 
 	/* allocate the mapping details for secondary processes*/
-- 
2.1.0

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [dpdk-dev] [PATCH v4 0/3] fix and improve uio_pci_generic support
  2015-02-24 13:30   ` [dpdk-dev] [PATCH v4 0/3] fix and improve uio_pci_generic support Bruce Richardson
                       ` (2 preceding siblings ...)
  2015-02-24 13:30     ` [dpdk-dev] [PATCH v4 3/3] eal: remove unnecessary check for primary instance Bruce Richardson
@ 2015-02-24 21:33     ` Thomas Monjalon
  3 siblings, 0 replies; 30+ messages in thread
From: Thomas Monjalon @ 2015-02-24 21:33 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev

> V4 changes: Split second patch into two for more readable history
> 
> V3 changes: Rebase to take account of commit 90a1633b 
> 	"eal/linux: allow to map BARs with MSI-X tables"
> 
> This patch does some cleanup of the uio mapping code to 
> a) fix issue with mmap of PCI bars reported by Tetsuya and confirmed
> by others.
> b) eliminate redundant code and reduce scans of /sys
> 
> Bruce Richardson (3):
>   eal: mmap uio resources using resourceX files
>   eal: populate uio_maps from pci mem_resources array
>   eal: remove unnecessary check for primary instance

Applied, thanks

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2015-02-24 21:33 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-22  5:30 [dpdk-dev] Testpmd returns error Tetsuya Mukawa
2015-02-22 14:17 ` Thomas Monjalon
2015-02-22 16:19   ` David Marchand
2015-02-22 18:51     ` Richardson, Bruce
2015-02-22 20:31       ` Bruce Richardson
2015-02-22 20:46 ` Bruce Richardson
2015-02-23  2:33   ` Tetsuya Mukawa
2015-02-23 11:12     ` Bruce Richardson
2015-02-23 11:36       ` Tetsuya Mukawa
2015-02-23 14:57 ` [dpdk-dev] [PATCH] eal: mmap uio resources using resourceX files Bruce Richardson
2015-02-23 15:00   ` Bruce Richardson
2015-02-23 15:30     ` Iremonger, Bernard
2015-02-24  9:21     ` Tetsuya Mukawa
2015-02-23 17:02   ` [dpdk-dev] [PATCH v2 0/2] fix and improve uio_pci_generic support Bruce Richardson
2015-02-23 17:02     ` [dpdk-dev] [PATCH v2 1/2] eal: mmap uio resources using resourceX files Bruce Richardson
2015-02-23 17:02     ` [dpdk-dev] [PATCH v2 2/2] eal: populate uio_maps from pci mem_resources array Bruce Richardson
2015-02-24 10:53     ` [dpdk-dev] [PATCH v2 0/2] fix and improve uio_pci_generic support Bruce Richardson
2015-02-24 11:23       ` David Marchand
2015-02-24 11:32         ` Bruce Richardson
2015-02-24 12:35           ` David Marchand
2015-02-24 12:20   ` [dpdk-dev] [PATCH v3 " Bruce Richardson
2015-02-24 12:20     ` [dpdk-dev] [PATCH v3 1/2] eal: mmap uio resources using resourceX files Bruce Richardson
2015-02-24 12:39       ` David Marchand
2015-02-24 12:20     ` [dpdk-dev] [PATCH v3 2/2] eal: populate uio_maps from pci mem_resources array Bruce Richardson
2015-02-24 12:38       ` David Marchand
2015-02-24 13:30   ` [dpdk-dev] [PATCH v4 0/3] fix and improve uio_pci_generic support Bruce Richardson
2015-02-24 13:30     ` [dpdk-dev] [PATCH v4 1/3] eal: mmap uio resources using resourceX files Bruce Richardson
2015-02-24 13:30     ` [dpdk-dev] [PATCH v4 2/3] eal: populate uio_maps from pci mem_resources array Bruce Richardson
2015-02-24 13:30     ` [dpdk-dev] [PATCH v4 3/3] eal: remove unnecessary check for primary instance Bruce Richardson
2015-02-24 21:33     ` [dpdk-dev] [PATCH v4 0/3] fix and improve uio_pci_generic support Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).