[dpdk-dev,1/5] pci: fix access to PCI config space in bsd

Message ID 99991ceaae3c8ead727fb43d78dd62670214e573.1462519635.git.rahul.lakkireddy@chelsio.com (mailing list archive)
State Accepted, archived
Delegated to: Bruce Richardson
Headers

Commit Message

Rahul Lakkireddy May 6, 2016, 7:43 a.m. UTC
  PCIOCREAD and PCIOCWRITE ioctls to read/write PCI config space fail
with EPERM due to missing write permission.  Fix by opening /dev/pci/
with O_RDWR instead.

Fixes: 632b2d1deeed ("eal: provide functions to access PCI config")

Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
Signed-off-by: Kumar Sanghvi <kumaras@chelsio.com>
---
 lib/librte_eal/bsdapp/eal/eal_pci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
  

Comments

Bruce Richardson May 31, 2016, 4:20 p.m. UTC | #1
On Fri, May 06, 2016 at 01:13:15PM +0530, Rahul Lakkireddy wrote:
> PCIOCREAD and PCIOCWRITE ioctls to read/write PCI config space fail
> with EPERM due to missing write permission.  Fix by opening /dev/pci/
> with O_RDWR instead.
> 
> Fixes: 632b2d1deeed ("eal: provide functions to access PCI config")
> 
> Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
> Signed-off-by: Kumar Sanghvi <kumaras@chelsio.com>
> ---
>  lib/librte_eal/bsdapp/eal/eal_pci.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
> index 2d16d78..82330be 100644
> --- a/lib/librte_eal/bsdapp/eal/eal_pci.c
> +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
> @@ -422,7 +422,7 @@ int rte_eal_pci_read_config(const struct rte_pci_device *dev,
>  		goto error;
>  	}
>  
> -	fd = open("/dev/pci", O_RDONLY);
> +	fd = open("/dev/pci", O_RDWR);
>  	if (fd < 0) {
>  		RTE_LOG(ERR, EAL, "%s(): error opening /dev/pci\n", __func__);
>  		goto error;
> @@ -466,7 +466,7 @@ int rte_eal_pci_write_config(const struct rte_pci_device *dev,
>  
>  	memcpy(&pi.pi_data, buf, len);
>  
> -	fd = open("/dev/pci", O_RDONLY);
> +	fd = open("/dev/pci", O_RDWR);
>  	if (fd < 0) {
>  		RTE_LOG(ERR, EAL, "%s(): error opening /dev/pci\n", __func__);
>  		goto error;
> -- 
Does the read function as well as the write one need O_RDWR permissions? There
is also an ioctl in rte_eal_pci_scan which operates on a RDONLY file descriptor.
Does that need to be modified also?

/Bruce
  
Rahul Lakkireddy June 1, 2016, 8:34 a.m. UTC | #2
Hi Bruce,

On Tuesday, May 05/31/16, 2016 at 09:20:13 -0700, Bruce Richardson wrote:
> On Fri, May 06, 2016 at 01:13:15PM +0530, Rahul Lakkireddy wrote:
> > PCIOCREAD and PCIOCWRITE ioctls to read/write PCI config space fail
> > with EPERM due to missing write permission.  Fix by opening /dev/pci/
> > with O_RDWR instead.
> > 
> > Fixes: 632b2d1deeed ("eal: provide functions to access PCI config")
> > 
> > Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
> > Signed-off-by: Kumar Sanghvi <kumaras@chelsio.com>
> > ---
> >  lib/librte_eal/bsdapp/eal/eal_pci.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
> > index 2d16d78..82330be 100644
> > --- a/lib/librte_eal/bsdapp/eal/eal_pci.c
> > +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
> > @@ -422,7 +422,7 @@ int rte_eal_pci_read_config(const struct rte_pci_device *dev,
> >  		goto error;
> >  	}
> >  
> > -	fd = open("/dev/pci", O_RDONLY);
> > +	fd = open("/dev/pci", O_RDWR);
> >  	if (fd < 0) {
> >  		RTE_LOG(ERR, EAL, "%s(): error opening /dev/pci\n", __func__);
> >  		goto error;
> > @@ -466,7 +466,7 @@ int rte_eal_pci_write_config(const struct rte_pci_device *dev,
> >  
> >  	memcpy(&pi.pi_data, buf, len);
> >  
> > -	fd = open("/dev/pci", O_RDONLY);
> > +	fd = open("/dev/pci", O_RDWR);
> >  	if (fd < 0) {
> >  		RTE_LOG(ERR, EAL, "%s(): error opening /dev/pci\n", __func__);
> >  		goto error;
> > -- 
> Does the read function as well as the write one need O_RDWR permissions? There
> is also an ioctl in rte_eal_pci_scan which operates on a RDONLY file descriptor.
> Does that need to be modified also?

Yes, both PCIOCREAD and PCIOCWRITE ioctls seem to require write
permission.  Otherwise, pci_ioctl [1] seems to return EPERM.  On the
other hand, the PCIOCGETCONF ioctl used in rte_eal_pci_scan doesn't
seem to require a write permission.  So, it should be fine to leave it
as RDONLY.

[1] https://svnweb.freebsd.org/base/release/10.3.0/sys/dev/pci/pci_user.c?revision=297553&view=markup#l493

Thanks,
Rahul
  

Patch

diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
index 2d16d78..82330be 100644
--- a/lib/librte_eal/bsdapp/eal/eal_pci.c
+++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
@@ -422,7 +422,7 @@  int rte_eal_pci_read_config(const struct rte_pci_device *dev,
 		goto error;
 	}
 
-	fd = open("/dev/pci", O_RDONLY);
+	fd = open("/dev/pci", O_RDWR);
 	if (fd < 0) {
 		RTE_LOG(ERR, EAL, "%s(): error opening /dev/pci\n", __func__);
 		goto error;
@@ -466,7 +466,7 @@  int rte_eal_pci_write_config(const struct rte_pci_device *dev,
 
 	memcpy(&pi.pi_data, buf, len);
 
-	fd = open("/dev/pci", O_RDONLY);
+	fd = open("/dev/pci", O_RDWR);
 	if (fd < 0) {
 		RTE_LOG(ERR, EAL, "%s(): error opening /dev/pci\n", __func__);
 		goto error;