All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] Extending kernel option pci=resource_alignment to be able to specify PCI device/vendor IDs
@ 2016-06-08  6:14 Koehrer Mathias (ETAS/ESW5)
  0 siblings, 0 replies; 9+ messages in thread
From: Koehrer Mathias (ETAS/ESW5) @ 2016-06-08  6:14 UTC (permalink / raw)
  To: gregkh, bhelgaas; +Cc: linux-pci, bhelgaas, hjk

Hi Greg,

> > Signed-off-by: Mathias Koehrer <mathias.koehrer@etas.com>
> >
> > ---
> >  Documentation/kernel-parameters.txt |    2 +
> >  drivers/pci/pci.c                   |   66 +++++++++++++++++++++++++-----------
> >  2 files changed, 49 insertions(+), 19 deletions(-)
> 
> This looks better, but wow, messy.  I'll defer to the PCI maintainer and
> developers now, this is in their camp, not mine :)
It looks messy as I had to change the indentation of existing code...
A "diff -u -w" on pci.c delivers a much simpler diff:

--- pci.c.orig	2016-05-29 16:29:24.000000000 +0000
+++ pci.c	2016-06-08 06:08:05.000000000 +0000
@@ -4755,6 +4755,7 @@
 static resource_size_t pci_specified_resource_alignment(struct pci_dev *dev)
 {
 	int seg, bus, slot, func, align_order, count;
+	unsigned short vendor, device, subsystem_vendor, subsystem_device;
 	resource_size_t align = 0;
 	char *p;
 
@@ -4768,6 +4769,32 @@
 		} else {
 			align_order = -1;
 		}
+		if (strncmp(p, "pci:", 4) == 0) {
+			/* PCI vendor/device (subvendor/subdevice) ids are specified */
+			p += 4;
+			if (sscanf(p, "%hx:%hx:%hx:%hx%n",
+				&vendor, &device, &subsystem_vendor, &subsystem_device, &count) != 4) {
+				if (sscanf(p, "%hx:%hx%n", &vendor, &device, &count) != 2) {
+					printk(KERN_ERR "PCI: Can't parse resource_alignment parameter: pci:%s\n",
+						p);
+					break;
+				}
+				subsystem_vendor = subsystem_device = 0;
+			}
+			p += count;
+			if ((!vendor || (vendor == dev->vendor)) &&
+				(!device || (device == dev->device)) &&
+				(!subsystem_vendor || (subsystem_vendor == dev->subsystem_vendor)) &&
+				(!subsystem_device || (subsystem_device == dev->subsystem_device))) {
+				if (align_order == -1)
+					align = PAGE_SIZE;
+				else
+					align = 1 << align_order;
+				/* Found */
+				break;
+			}
+		}
+		else {
 		if (sscanf(p, "%x:%x:%x.%x%n",
 			&seg, &bus, &slot, &func, &count) != 4) {
 			seg = 0;
@@ -4791,6 +4818,7 @@
 			/* Found */
 			break;
 		}
+		}
 		if (*p != ';' && *p != ',') {
 			/* End of param or invalid format */
 			break;


Best regards

Mathias

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

* Re: [PATCH] Extending kernel option pci=resource_alignment to be able to specify PCI device/vendor IDs
  2016-08-08  7:39 Koehrer Mathias (ETAS/ESW5)
@ 2016-08-08 14:01 ` Bjorn Helgaas
  0 siblings, 0 replies; 9+ messages in thread
From: Bjorn Helgaas @ 2016-08-08 14:01 UTC (permalink / raw)
  To: Koehrer Mathias (ETAS/ESW5); +Cc: gregkh, linux-pci, bhelgaas, hjk

On Mon, Aug 08, 2016 at 07:39:01AM +0000, Koehrer Mathias (ETAS/ESW5) wrote:
> Hi Bjorn,
> 
> > On Tue, Jun 07, 2016 at 02:24:17PM +0000, Koehrer Mathias (ETAS/ESW5) wrote:
> > > Some uio based PCI drivers (e.g. uio_cif) do not work if the assigned
> > > PCI memory resources are not page aligned.
> > > By using the kernel option "pci=resource_alignment" it is possible to
> > > force single PCI boards to use page alignment for their memory resources.
> > > However, this is fairly cumbersome if multiple of these boards are in
> > > use as the specification of the cards has to be done via PCI
> > > bus/slot/function number which might change e.g. by adding another board.
> > > This patch extends the kernel option "pci=resource_alignment" to allow
> > > to specify the relevant boards via PCI device/vendor (and subdevice/subvendor)
> > ids.
> > > The specification of the devices via device/vendor is indicated by a
> > > leading string "pci:" as argument to "pci=resource_alignment".
> > > The format of the specification is
> > >   pci:<vendor>:<device>[:<subvendor>:<subdevice>]
> > >
> > > Signed-off-by: Mathias Koehrer <mathias.koehrer@etas.com>
> > >
> > > ---
> > >  Documentation/kernel-parameters.txt |    2 +
> > >  drivers/pci/pci.c                   |   66 +++++++++++++++++++++++++-----------
> > >  2 files changed, 49 insertions(+), 19 deletions(-)
> > >
> > > Index: linux-4.7-rc1/Documentation/kernel-parameters.txt
> > >
> > ============================================================
> > =======
> > > --- linux-4.7-rc1.orig/Documentation/kernel-parameters.txt
> > > +++ linux-4.7-rc1/Documentation/kernel-parameters.txt
> > > @@ -2998,6 +2998,8 @@ bytes respectively. Such letter suffixes
> > >  		resource_alignment=
> > >  				Format:
> > >  				[<order of
> > align>@][<domain>:]<bus>:<slot>.<func>[; ...]
> > > +				[<order of align>@]pci:<vendor>:<device>\
> > > +						[:<subvendor>:<subdevice>][; ...]
> > 
> > Can you include a little example here so we know whether to use "pci:8086:1234" or
> > "pci:0x8086:0x1234"?
> > 
> > Bjorn
> 
> I have provided an example and extended the docu (sent in  http://marc.info/?l=linux-pci&m=146657769505684&w=2 and http://marc.info/?l=linux-pci&m=146918412704107&w=2 ).
> It would be great if you could comment on the modified patch...

It looks like I applied the patch, but I forgot to include the updated
documentation.  Can you confirm that?  If you send a documentation
patch to add the example, I can add that.

Bjorn

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

* Re: [PATCH] Extending kernel option pci=resource_alignment to be able to specify PCI device/vendor IDs
@ 2016-08-08  7:39 Koehrer Mathias (ETAS/ESW5)
  2016-08-08 14:01 ` Bjorn Helgaas
  0 siblings, 1 reply; 9+ messages in thread
From: Koehrer Mathias (ETAS/ESW5) @ 2016-08-08  7:39 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: gregkh, linux-pci, bhelgaas, hjk

Hi Bjorn,

> On Tue, Jun 07, 2016 at 02:24:17PM +0000, Koehrer Mathias (ETAS/ESW5) wro=
te:
> > Some uio based PCI drivers (e.g. uio_cif) do not work if the assigned
> > PCI memory resources are not page aligned.
> > By using the kernel option "pci=3Dresource_alignment" it is possible to
> > force single PCI boards to use page alignment for their memory resource=
s.
> > However, this is fairly cumbersome if multiple of these boards are in
> > use as the specification of the cards has to be done via PCI
> > bus/slot/function number which might change e.g. by adding another boar=
d.
> > This patch extends the kernel option "pci=3Dresource_alignment" to allo=
w
> > to specify the relevant boards via PCI device/vendor (and subdevice/sub=
vendor)
> ids.
> > The specification of the devices via device/vendor is indicated by a
> > leading string "pci:" as argument to "pci=3Dresource_alignment".
> > The format of the specification is
> >   pci:<vendor>:<device>[:<subvendor>:<subdevice>]
> >
> > Signed-off-by: Mathias Koehrer <mathias.koehrer@etas.com>
> >
> > ---
> >  Documentation/kernel-parameters.txt |    2 +
> >  drivers/pci/pci.c                   |   66 +++++++++++++++++++++++++--=
---------
> >  2 files changed, 49 insertions(+), 19 deletions(-)
> >
> > Index: linux-4.7-rc1/Documentation/kernel-parameters.txt
> >
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> =3D=3D=3D=3D=3D=3D=3D
> > --- linux-4.7-rc1.orig/Documentation/kernel-parameters.txt
> > +++ linux-4.7-rc1/Documentation/kernel-parameters.txt
> > @@ -2998,6 +2998,8 @@ bytes respectively. Such letter suffixes
> >  		resource_alignment=3D
> >  				Format:
> >  				[<order of
> align>@][<domain>:]<bus>:<slot>.<func>[; ...]
> > +				[<order of align>@]pci:<vendor>:<device>\
> > +						[:<subvendor>:<subdevice>][; ...]
>=20
> Can you include a little example here so we know whether to use "pci:8086=
:1234" or
> "pci:0x8086:0x1234"?
>=20
> Bjorn

I have provided an example and extended the docu (sent in  http://marc.info=
/?l=3Dlinux-pci&m=3D146657769505684&w=3D2 and http://marc.info/?l=3Dlinux-p=
ci&m=3D146918412704107&w=3D2 ).
It would be great if you could comment on the modified patch...

Thanks=20

Best regards

Mathias

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

* Re: [PATCH] Extending kernel option pci=resource_alignment to be able to specify PCI device/vendor IDs
  2016-06-07 14:24 Koehrer Mathias (ETAS/ESW5)
  2016-06-07 14:32 ` gregkh
  2016-06-21 22:00 ` Bjorn Helgaas
@ 2016-06-21 22:04 ` Bjorn Helgaas
  2 siblings, 0 replies; 9+ messages in thread
From: Bjorn Helgaas @ 2016-06-21 22:04 UTC (permalink / raw)
  To: Koehrer Mathias (ETAS/ESW5); +Cc: gregkh, linux-pci, bhelgaas, hjk

On Tue, Jun 07, 2016 at 02:24:17PM +0000, Koehrer Mathias (ETAS/ESW5) wrote:
> Some uio based PCI drivers (e.g. uio_cif) do not work if the assigned 
> PCI memory resources are not page aligned.
> By using the kernel option "pci=resource_alignment" it is possible to force
> single PCI boards to use page alignment for their memory resources.
> However, this is fairly cumbersome if multiple of these boards are in use as 
> the specification of the cards has to be done via PCI bus/slot/function number
> which might change e.g. by adding another board.
> This patch extends the kernel option "pci=resource_alignment" to allow to
> specify the relevant boards via PCI device/vendor (and subdevice/subvendor) ids.
> The specification of the devices via device/vendor is indicated by a leading
> string "pci:" as argument to "pci=resource_alignment".
> The format of the specification is
>   pci:<vendor>:<device>[:<subvendor>:<subdevice>]
> 
> Signed-off-by: Mathias Koehrer <mathias.koehrer@etas.com>
> 
> ---
>  Documentation/kernel-parameters.txt |    2 +
>  drivers/pci/pci.c                   |   66 +++++++++++++++++++++++++-----------
>  2 files changed, 49 insertions(+), 19 deletions(-)
> 
> Index: linux-4.7-rc1/Documentation/kernel-parameters.txt
> ===================================================================
> --- linux-4.7-rc1.orig/Documentation/kernel-parameters.txt
> +++ linux-4.7-rc1/Documentation/kernel-parameters.txt
> @@ -2998,6 +2998,8 @@ bytes respectively. Such letter suffixes
>  		resource_alignment=
>  				Format:
>  				[<order of align>@][<domain>:]<bus>:<slot>.<func>[; ...]
> +				[<order of align>@]pci:<vendor>:<device>\
> +						[:<subvendor>:<subdevice>][; ...] 

Can you include a little example here so we know whether to use
"pci:8086:1234" or "pci:0x8086:0x1234"?

Bjorn

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

* Re: [PATCH] Extending kernel option pci=resource_alignment to be able to specify PCI device/vendor IDs
  2016-06-07 14:24 Koehrer Mathias (ETAS/ESW5)
  2016-06-07 14:32 ` gregkh
@ 2016-06-21 22:00 ` Bjorn Helgaas
  2016-06-21 22:04 ` Bjorn Helgaas
  2 siblings, 0 replies; 9+ messages in thread
From: Bjorn Helgaas @ 2016-06-21 22:00 UTC (permalink / raw)
  To: Koehrer Mathias (ETAS/ESW5); +Cc: gregkh, linux-pci, bhelgaas, hjk

On Tue, Jun 07, 2016 at 02:24:17PM +0000, Koehrer Mathias (ETAS/ESW5) wrote:
> Some uio based PCI drivers (e.g. uio_cif) do not work if the assigned 
> PCI memory resources are not page aligned.

Right.  PCI BARs must be aligned on their size.  BARs that are smaller
than a page are not required by PCI to be page-aligned.  Therefore, a
single page may contain several small BARs, or it may contain a small
BAR and some unallocated space.

Both are potential issues because mmap works on a page granularity.
If a user can mmap a page that contains several small BARs, he may be
able to access to more devices than he should.  If a user can mmap a
page that contains unallocated space, she may be able to cause PCI
errors.

You have a device with a 128-byte BAR at 0xeae4f400.  That doesn't
work with uio_cif because b65502879556 ("uio: we cannot mmap unaligned
page contents"), which appeared in v3.13, makes the mmap fail if the
starting address is not page-aligned.

Your patch works around that by moving BARs so they are page-aligned.
I think this is OK, but of course, there's nothing you can do about
the fact that the BAR is smaller than a page, and there might be other
things after the BAR in the same page.  I think that's a problem, and
I wouldn't be surprised if we eventually disallow mmap of any BAR
smaller than a page.

I don't know the history of UIO mmap, and I do see the comment in
vm_iomap_memory() about how we've historically allowed non
page-aligned mmap because I/O memory often has smaller alignment, but
it seems like a safety issue to me, so I'm kind of surprised that we
allow it.

In any case, I think I'll apply this patch for v4.8 because it seems
like a reasonable extension of the existing resource_alignment
support.

> By using the kernel option "pci=resource_alignment" it is possible to force
> single PCI boards to use page alignment for their memory resources.
> However, this is fairly cumbersome if multiple of these boards are in use as 
> the specification of the cards has to be done via PCI bus/slot/function number
> which might change e.g. by adding another board.
> This patch extends the kernel option "pci=resource_alignment" to allow to
> specify the relevant boards via PCI device/vendor (and subdevice/subvendor) ids.
> The specification of the devices via device/vendor is indicated by a leading
> string "pci:" as argument to "pci=resource_alignment".
> The format of the specification is
>   pci:<vendor>:<device>[:<subvendor>:<subdevice>]
> 
> Signed-off-by: Mathias Koehrer <mathias.koehrer@etas.com>
> 
> ---
>  Documentation/kernel-parameters.txt |    2 +
>  drivers/pci/pci.c                   |   66 +++++++++++++++++++++++++-----------
>  2 files changed, 49 insertions(+), 19 deletions(-)
> 
> Index: linux-4.7-rc1/Documentation/kernel-parameters.txt
> ===================================================================
> --- linux-4.7-rc1.orig/Documentation/kernel-parameters.txt
> +++ linux-4.7-rc1/Documentation/kernel-parameters.txt
> @@ -2998,6 +2998,8 @@ bytes respectively. Such letter suffixes
>  		resource_alignment=
>  				Format:
>  				[<order of align>@][<domain>:]<bus>:<slot>.<func>[; ...]
> +				[<order of align>@]pci:<vendor>:<device>\
> +						[:<subvendor>:<subdevice>][; ...] 
>  				Specifies alignment and device to reassign
>  				aligned memory resources.
>  				If <order of align> is not specified,
> Index: linux-4.7-rc1/drivers/pci/pci.c
> ===================================================================
> --- linux-4.7-rc1.orig/drivers/pci/pci.c
> +++ linux-4.7-rc1/drivers/pci/pci.c
> @@ -4755,6 +4755,7 @@ static DEFINE_SPINLOCK(resource_alignmen
>  static resource_size_t pci_specified_resource_alignment(struct pci_dev *dev)
>  {
>  	int seg, bus, slot, func, align_order, count;
> +	unsigned short vendor, device, subsystem_vendor, subsystem_device;
>  	resource_size_t align = 0;
>  	char *p;
>  
> @@ -4768,28 +4769,55 @@ static resource_size_t pci_specified_res
>  		} else {
>  			align_order = -1;
>  		}
> -		if (sscanf(p, "%x:%x:%x.%x%n",
> -			&seg, &bus, &slot, &func, &count) != 4) {
> -			seg = 0;
> -			if (sscanf(p, "%x:%x.%x%n",
> -					&bus, &slot, &func, &count) != 3) {
> -				/* Invalid format */
> -				printk(KERN_ERR "PCI: Can't parse resource_alignment parameter: %s\n",
> -					p);
> +		if (strncmp(p, "pci:", 4) == 0) {
> +			/* PCI vendor/device (subvendor/subdevice) ids are specified */
> +			p += 4;
> +			if (sscanf(p, "%hx:%hx:%hx:%hx%n",
> +				&vendor, &device, &subsystem_vendor, &subsystem_device, &count) != 4) {
> +				if (sscanf(p, "%hx:%hx%n", &vendor, &device, &count) != 2) {
> +					printk(KERN_ERR "PCI: Can't parse resource_alignment parameter: pci:%s\n",
> +						p);
> +					break;
> +				}
> +				subsystem_vendor = subsystem_device = 0;
> +			}
> +			p += count;
> +			if ((!vendor || (vendor == dev->vendor)) &&
> +				(!device || (device == dev->device)) &&
> +				(!subsystem_vendor || (subsystem_vendor == dev->subsystem_vendor)) &&
> +				(!subsystem_device || (subsystem_device == dev->subsystem_device))) {
> +				if (align_order == -1)
> +					align = PAGE_SIZE;
> +				else
> +					align = 1 << align_order;
> +				/* Found */
>  				break;
>  			}
>  		}
> -		p += count;
> -		if (seg == pci_domain_nr(dev->bus) &&
> -			bus == dev->bus->number &&
> -			slot == PCI_SLOT(dev->devfn) &&
> -			func == PCI_FUNC(dev->devfn)) {
> -			if (align_order == -1)
> -				align = PAGE_SIZE;
> -			else
> -				align = 1 << align_order;
> -			/* Found */
> -			break;
> +		else {
> +			if (sscanf(p, "%x:%x:%x.%x%n",
> +				&seg, &bus, &slot, &func, &count) != 4) {
> +				seg = 0;
> +				if (sscanf(p, "%x:%x.%x%n",
> +						&bus, &slot, &func, &count) != 3) {
> +					/* Invalid format */
> +					printk(KERN_ERR "PCI: Can't parse resource_alignment parameter: %s\n",
> +						p);
> +					break;
> +				}
> +			}
> +			p += count;
> +			if (seg == pci_domain_nr(dev->bus) &&
> +				bus == dev->bus->number &&
> +				slot == PCI_SLOT(dev->devfn) &&
> +				func == PCI_FUNC(dev->devfn)) {
> +				if (align_order == -1)
> +					align = PAGE_SIZE;
> +				else
> +					align = 1 << align_order;
> +				/* Found */
> +				break;
> +			}
>  		}
>  		if (*p != ';' && *p != ',') {
>  			/* End of param or invalid format */
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH] Extending kernel option pci=resource_alignment to be able to specify PCI device/vendor IDs
  2016-06-09 12:40 Koehrer Mathias (ETAS/ESW5)
@ 2016-06-09 16:12 ` gregkh
  0 siblings, 0 replies; 9+ messages in thread
From: gregkh @ 2016-06-09 16:12 UTC (permalink / raw)
  To: Koehrer Mathias (ETAS/ESW5); +Cc: helgaas, linux-pci, bhelgaas

On Thu, Jun 09, 2016 at 12:40:30PM +0000, Koehrer Mathias (ETAS/ESW5) wrote:
> Hi Bjorn,
> 
> can you please comment on that?

Relax, maintainers are busy, give them a chance to catch up on
patches...

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

* Re: [PATCH] Extending kernel option pci=resource_alignment to be able to specify PCI device/vendor IDs
@ 2016-06-09 12:40 Koehrer Mathias (ETAS/ESW5)
  2016-06-09 16:12 ` gregkh
  0 siblings, 1 reply; 9+ messages in thread
From: Koehrer Mathias (ETAS/ESW5) @ 2016-06-09 12:40 UTC (permalink / raw)
  To: Koehrer Mathias (ETAS/ESW5), gregkh, helgaas; +Cc: linux-pci, bhelgaas

Hi Bjorn,

can you please comment on that?
> > > Signed-off-by: Mathias Koehrer <mathias.koehrer@etas.com>
> > >
> > > ---
> > >  Documentation/kernel-parameters.txt |    2 +
> > >  drivers/pci/pci.c                   |   66 +++++++++++++++++++++++++=
-----------
> > >  2 files changed, 49 insertions(+), 19 deletions(-)
> >
> > This looks better, but wow, messy.  I'll defer to the PCI maintainer
> > and developers now, this is in their camp, not mine :)
> It looks messy as I had to change the indentation of existing code...
> A "diff -u -w" on pci.c delivers a much simpler diff:
>=20
> --- pci.c.orig	2016-05-29 16:29:24.000000000 +0000
> +++ pci.c	2016-06-08 06:08:05.000000000 +0000
> @@ -4755,6 +4755,7 @@
>  static resource_size_t pci_specified_resource_alignment(struct pci_dev *=
dev)
> {
>  	int seg, bus, slot, func, align_order, count;
> +	unsigned short vendor, device, subsystem_vendor, subsystem_device;
>  	resource_size_t align =3D 0;
>  	char *p;
>=20
> @@ -4768,6 +4769,32 @@
>  		} else {
>  			align_order =3D -1;
>  		}
> +		if (strncmp(p, "pci:", 4) =3D=3D 0) {
> +			/* PCI vendor/device (subvendor/subdevice) ids are
> specified */
> +			p +=3D 4;
> +			if (sscanf(p, "%hx:%hx:%hx:%hx%n",
> +				&vendor, &device, &subsystem_vendor,
> &subsystem_device, &count) !=3D 4) {
> +				if (sscanf(p, "%hx:%hx%n", &vendor, &device,
> &count) !=3D 2) {
> +					printk(KERN_ERR "PCI: Can't parse
> resource_alignment parameter: pci:%s\n",
> +						p);
> +					break;
> +				}
> +				subsystem_vendor =3D subsystem_device =3D 0;
> +			}
> +			p +=3D count;
> +			if ((!vendor || (vendor =3D=3D dev->vendor)) &&
> +				(!device || (device =3D=3D dev->device)) &&
> +				(!subsystem_vendor || (subsystem_vendor =3D=3D
> dev->subsystem_vendor)) &&
> +				(!subsystem_device || (subsystem_device =3D=3D
> dev->subsystem_device))) {
> +				if (align_order =3D=3D -1)
> +					align =3D PAGE_SIZE;
> +				else
> +					align =3D 1 << align_order;
> +				/* Found */
> +				break;
> +			}
> +		}
> +		else {
>  		if (sscanf(p, "%x:%x:%x.%x%n",
>  			&seg, &bus, &slot, &func, &count) !=3D 4) {
>  			seg =3D 0;
> @@ -4791,6 +4818,7 @@
>  			/* Found */
>  			break;
>  		}
> +		}
>  		if (*p !=3D ';' && *p !=3D ',') {
>  			/* End of param or invalid format */
>  			break;
>=20
>=20
Thanks
Mathias

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

* Re: [PATCH] Extending kernel option pci=resource_alignment to be able to specify PCI device/vendor IDs
  2016-06-07 14:24 Koehrer Mathias (ETAS/ESW5)
@ 2016-06-07 14:32 ` gregkh
  2016-06-21 22:00 ` Bjorn Helgaas
  2016-06-21 22:04 ` Bjorn Helgaas
  2 siblings, 0 replies; 9+ messages in thread
From: gregkh @ 2016-06-07 14:32 UTC (permalink / raw)
  To: Koehrer Mathias (ETAS/ESW5); +Cc: linux-pci, bhelgaas, hjk

On Tue, Jun 07, 2016 at 02:24:17PM +0000, Koehrer Mathias (ETAS/ESW5) wrote:
> Some uio based PCI drivers (e.g. uio_cif) do not work if the assigned 
> PCI memory resources are not page aligned.
> By using the kernel option "pci=resource_alignment" it is possible to force
> single PCI boards to use page alignment for their memory resources.
> However, this is fairly cumbersome if multiple of these boards are in use as 
> the specification of the cards has to be done via PCI bus/slot/function number
> which might change e.g. by adding another board.
> This patch extends the kernel option "pci=resource_alignment" to allow to
> specify the relevant boards via PCI device/vendor (and subdevice/subvendor) ids.
> The specification of the devices via device/vendor is indicated by a leading
> string "pci:" as argument to "pci=resource_alignment".
> The format of the specification is
>   pci:<vendor>:<device>[:<subvendor>:<subdevice>]
> 
> Signed-off-by: Mathias Koehrer <mathias.koehrer@etas.com>
> 
> ---
>  Documentation/kernel-parameters.txt |    2 +
>  drivers/pci/pci.c                   |   66 +++++++++++++++++++++++++-----------
>  2 files changed, 49 insertions(+), 19 deletions(-)

This looks better, but wow, messy.  I'll defer to the PCI maintainer and
developers now, this is in their camp, not mine :)

thanks,

greg k-h

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

* [PATCH] Extending kernel option pci=resource_alignment to be able to specify PCI device/vendor IDs
@ 2016-06-07 14:24 Koehrer Mathias (ETAS/ESW5)
  2016-06-07 14:32 ` gregkh
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Koehrer Mathias (ETAS/ESW5) @ 2016-06-07 14:24 UTC (permalink / raw)
  To: gregkh; +Cc: linux-pci, bhelgaas, hjk

Some uio based PCI drivers (e.g. uio_cif) do not work if the assigned 
PCI memory resources are not page aligned.
By using the kernel option "pci=resource_alignment" it is possible to force
single PCI boards to use page alignment for their memory resources.
However, this is fairly cumbersome if multiple of these boards are in use as 
the specification of the cards has to be done via PCI bus/slot/function number
which might change e.g. by adding another board.
This patch extends the kernel option "pci=resource_alignment" to allow to
specify the relevant boards via PCI device/vendor (and subdevice/subvendor) ids.
The specification of the devices via device/vendor is indicated by a leading
string "pci:" as argument to "pci=resource_alignment".
The format of the specification is
  pci:<vendor>:<device>[:<subvendor>:<subdevice>]

Signed-off-by: Mathias Koehrer <mathias.koehrer@etas.com>

---
 Documentation/kernel-parameters.txt |    2 +
 drivers/pci/pci.c                   |   66 +++++++++++++++++++++++++-----------
 2 files changed, 49 insertions(+), 19 deletions(-)

Index: linux-4.7-rc1/Documentation/kernel-parameters.txt
===================================================================
--- linux-4.7-rc1.orig/Documentation/kernel-parameters.txt
+++ linux-4.7-rc1/Documentation/kernel-parameters.txt
@@ -2998,6 +2998,8 @@ bytes respectively. Such letter suffixes
 		resource_alignment=
 				Format:
 				[<order of align>@][<domain>:]<bus>:<slot>.<func>[; ...]
+				[<order of align>@]pci:<vendor>:<device>\
+						[:<subvendor>:<subdevice>][; ...] 
 				Specifies alignment and device to reassign
 				aligned memory resources.
 				If <order of align> is not specified,
Index: linux-4.7-rc1/drivers/pci/pci.c
===================================================================
--- linux-4.7-rc1.orig/drivers/pci/pci.c
+++ linux-4.7-rc1/drivers/pci/pci.c
@@ -4755,6 +4755,7 @@ static DEFINE_SPINLOCK(resource_alignmen
 static resource_size_t pci_specified_resource_alignment(struct pci_dev *dev)
 {
 	int seg, bus, slot, func, align_order, count;
+	unsigned short vendor, device, subsystem_vendor, subsystem_device;
 	resource_size_t align = 0;
 	char *p;
 
@@ -4768,28 +4769,55 @@ static resource_size_t pci_specified_res
 		} else {
 			align_order = -1;
 		}
-		if (sscanf(p, "%x:%x:%x.%x%n",
-			&seg, &bus, &slot, &func, &count) != 4) {
-			seg = 0;
-			if (sscanf(p, "%x:%x.%x%n",
-					&bus, &slot, &func, &count) != 3) {
-				/* Invalid format */
-				printk(KERN_ERR "PCI: Can't parse resource_alignment parameter: %s\n",
-					p);
+		if (strncmp(p, "pci:", 4) == 0) {
+			/* PCI vendor/device (subvendor/subdevice) ids are specified */
+			p += 4;
+			if (sscanf(p, "%hx:%hx:%hx:%hx%n",
+				&vendor, &device, &subsystem_vendor, &subsystem_device, &count) != 4) {
+				if (sscanf(p, "%hx:%hx%n", &vendor, &device, &count) != 2) {
+					printk(KERN_ERR "PCI: Can't parse resource_alignment parameter: pci:%s\n",
+						p);
+					break;
+				}
+				subsystem_vendor = subsystem_device = 0;
+			}
+			p += count;
+			if ((!vendor || (vendor == dev->vendor)) &&
+				(!device || (device == dev->device)) &&
+				(!subsystem_vendor || (subsystem_vendor == dev->subsystem_vendor)) &&
+				(!subsystem_device || (subsystem_device == dev->subsystem_device))) {
+				if (align_order == -1)
+					align = PAGE_SIZE;
+				else
+					align = 1 << align_order;
+				/* Found */
 				break;
 			}
 		}
-		p += count;
-		if (seg == pci_domain_nr(dev->bus) &&
-			bus == dev->bus->number &&
-			slot == PCI_SLOT(dev->devfn) &&
-			func == PCI_FUNC(dev->devfn)) {
-			if (align_order == -1)
-				align = PAGE_SIZE;
-			else
-				align = 1 << align_order;
-			/* Found */
-			break;
+		else {
+			if (sscanf(p, "%x:%x:%x.%x%n",
+				&seg, &bus, &slot, &func, &count) != 4) {
+				seg = 0;
+				if (sscanf(p, "%x:%x.%x%n",
+						&bus, &slot, &func, &count) != 3) {
+					/* Invalid format */
+					printk(KERN_ERR "PCI: Can't parse resource_alignment parameter: %s\n",
+						p);
+					break;
+				}
+			}
+			p += count;
+			if (seg == pci_domain_nr(dev->bus) &&
+				bus == dev->bus->number &&
+				slot == PCI_SLOT(dev->devfn) &&
+				func == PCI_FUNC(dev->devfn)) {
+				if (align_order == -1)
+					align = PAGE_SIZE;
+				else
+					align = 1 << align_order;
+				/* Found */
+				break;
+			}
 		}
 		if (*p != ';' && *p != ',') {
 			/* End of param or invalid format */

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

end of thread, other threads:[~2016-08-08 14:01 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-08  6:14 [PATCH] Extending kernel option pci=resource_alignment to be able to specify PCI device/vendor IDs Koehrer Mathias (ETAS/ESW5)
  -- strict thread matches above, loose matches on Subject: below --
2016-08-08  7:39 Koehrer Mathias (ETAS/ESW5)
2016-08-08 14:01 ` Bjorn Helgaas
2016-06-09 12:40 Koehrer Mathias (ETAS/ESW5)
2016-06-09 16:12 ` gregkh
2016-06-07 14:24 Koehrer Mathias (ETAS/ESW5)
2016-06-07 14:32 ` gregkh
2016-06-21 22:00 ` Bjorn Helgaas
2016-06-21 22:04 ` Bjorn Helgaas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.