[dpdk-dev] eal: mmap uio resources using resourceX files
Commit Message
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(-)
Comments
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
> -----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.
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(-)
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
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
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.
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
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(-)
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.
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(-)
> 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
@@ -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
@@ -38,6 +38,7 @@
struct pci_map {
void *addr;
+ char *path;
uint64_t offset;
uint64_t size;
uint64_t phaddr;
@@ -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;
}
}
@@ -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;
}