Re: [PATCH] Extending kernel option pci=resource_alignment to be able to specify PCI device/vendor IDs

Re: [PATCH] Extending kernel option pci=resource_alignment to be able to specify PCI device/vendor IDs

[Koehrer Mathias (ETAS/ESW5)]

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

Re: [PATCH] Extending kernel option pci=resource_alignment to be able to specify PCI device/vendor IDs

[Bjorn Helgaas]

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

[PATCH] Extending kernel option pci=resource_alignment to be able to specify PCI device/vendor IDs - Documentation

[Mathias Koehrer]

Extend the documentation for kernel option pci=resource_alignment

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>]

Examples: 
  pci=resource_alignment=4096@pci:1234:abcd:1234:bcde
  pci=resource_alignment=pci:1234:abcd

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

---
 pci/Documentation/kernel-parameters.txt |    3 +++
 1 file changed, 3 insertions(+)

Index: linux-pci-git/pci/Documentation/kernel-parameters.txt
===================================================================
--- linux-pci-git.orig/pci/Documentation/kernel-parameters.txt
+++ linux-pci-git/pci/Documentation/kernel-parameters.txt
@@ -3032,6 +3032,9 @@ bytes respectively. Such letter suffixes
 				PAGE_SIZE is used as alignment.
 				PCI-PCI bridge can be specified, if resource
 				windows need to be expanded.
+				To specify the alignment for certain types of devices, the
+				PCI vendor/device (and subvendor/subdevice) may be
+				specified. E.g. 4096@pci:1234:abcd:1234:bcde
 		ecrc=		Enable/disable PCIe ECRC (transaction layer
 				end-to-end CRC checking).
 				bios: Use BIOS/firmware settings. This is the

Re: [PATCH] Extending kernel option pci=resource_alignment to be able to specify PCI device/vendor IDs

[Koehrer Mathias (ETAS/ESW5)]

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

Re: [PATCH] Extending kernel option pci=resource_alignment to be able to specify PCI device/vendor IDs

[gregkh]

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...

Re: [PATCH] Extending kernel option pci=resource_alignment to be able to specify PCI device/vendor IDs

[Koehrer Mathias (ETAS/ESW5)]

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

[PATCH] Extending kernel option pci=resource_alignment to be able to specify PCI device/vendor IDs

[Koehrer Mathias (ETAS/ESW5)]

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 */

Re: [PATCH] Extending kernel option pci=resource_alignment to be able to specify PCI device/vendor IDs

[gregkh]

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

Re: [PATCH] Extending kernel option pci=resource_alignment to be able to specify PCI device/vendor IDs

[Bjorn Helgaas]

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

Re: [PATCH] Extending kernel option pci=resource_alignment to be able to specify PCI device/vendor IDs

[Bjorn Helgaas]

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