All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
@ 2017-04-20 16:35 ` Haiyang Zhang
  0 siblings, 0 replies; 36+ messages in thread
From: Haiyang Zhang @ 2017-04-20 16:35 UTC (permalink / raw)
  To: bhelgaas, linux-pci
  Cc: haiyangz, kys, sthemmin, olaf, vkuznets, driverdev-devel, linux-kernel

From: Haiyang Zhang <haiyangz@microsoft.com>

This patch uses the lower 16 bits of the serial number as PCI
domain, otherwise some drivers may not be able to handle it.

Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/pci/host/pci-hyperv.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
index e73880c..b18dff3 100644
--- a/drivers/pci/host/pci-hyperv.c
+++ b/drivers/pci/host/pci-hyperv.c
@@ -1334,9 +1334,11 @@ static void put_pcichild(struct hv_pci_dev *hpdev,
 	 * can have shorter names than based on the bus instance UUID.
 	 * Only the first device serial number is used for domain, so the
 	 * domain number will not change after the first device is added.
+	 * The lower 16 bits of the serial number is used, otherwise some
+	 * drivers may not be able to handle it.
 	 */
 	if (list_empty(&hbus->children))
-		hbus->sysdata.domain = desc->ser;
+		hbus->sysdata.domain = desc->ser & 0xFFFF;
 	list_add_tail(&hpdev->list_entry, &hbus->children);
 	spin_unlock_irqrestore(&hbus->device_list_lock, flags);
 	return hpdev;
-- 
1.7.1

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

* [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
@ 2017-04-20 16:35 ` Haiyang Zhang
  0 siblings, 0 replies; 36+ messages in thread
From: Haiyang Zhang @ 2017-04-20 16:35 UTC (permalink / raw)
  To: bhelgaas, linux-pci
  Cc: olaf, sthemmin, haiyangz, driverdev-devel, linux-kernel

From: Haiyang Zhang <haiyangz@microsoft.com>

This patch uses the lower 16 bits of the serial number as PCI
domain, otherwise some drivers may not be able to handle it.

Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/pci/host/pci-hyperv.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
index e73880c..b18dff3 100644
--- a/drivers/pci/host/pci-hyperv.c
+++ b/drivers/pci/host/pci-hyperv.c
@@ -1334,9 +1334,11 @@ static void put_pcichild(struct hv_pci_dev *hpdev,
 	 * can have shorter names than based on the bus instance UUID.
 	 * Only the first device serial number is used for domain, so the
 	 * domain number will not change after the first device is added.
+	 * The lower 16 bits of the serial number is used, otherwise some
+	 * drivers may not be able to handle it.
 	 */
 	if (list_empty(&hbus->children))
-		hbus->sysdata.domain = desc->ser;
+		hbus->sysdata.domain = desc->ser & 0xFFFF;
 	list_add_tail(&hpdev->list_entry, &hbus->children);
 	spin_unlock_irqrestore(&hbus->device_list_lock, flags);
 	return hpdev;
-- 
1.7.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
  2017-04-20 16:35 ` Haiyang Zhang
@ 2017-04-20 18:33   ` Bjorn Helgaas
  -1 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2017-04-20 18:33 UTC (permalink / raw)
  To: Haiyang Zhang
  Cc: linux-pci, KY Srinivasan, Stephen Hemminger, olaf, vkuznets,
	driverdev-devel, linux-kernel

On Thu, Apr 20, 2017 at 11:35 AM, Haiyang Zhang
<haiyangz@exchange.microsoft.com> wrote:
> From: Haiyang Zhang <haiyangz@microsoft.com>
>
> This patch uses the lower 16 bits of the serial number as PCI
> domain, otherwise some drivers may not be able to handle it.

Can you give any more details about this?  Which drivers, for
instance?  Why do drivers care about the domain at all?  Can we or
should we make this more explicit and consistent in the PCI core,
e.g., pci_domain_nr() is currently defined to return "int"; maybe it
should be u32?  (Although I think "int" is the same size as "u32" on
all arches anyway).

> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---
>  drivers/pci/host/pci-hyperv.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
> index e73880c..b18dff3 100644
> --- a/drivers/pci/host/pci-hyperv.c
> +++ b/drivers/pci/host/pci-hyperv.c
> @@ -1334,9 +1334,11 @@ static void put_pcichild(struct hv_pci_dev *hpdev,
>          * can have shorter names than based on the bus instance UUID.
>          * Only the first device serial number is used for domain, so the
>          * domain number will not change after the first device is added.
> +        * The lower 16 bits of the serial number is used, otherwise some
> +        * drivers may not be able to handle it.
>          */
>         if (list_empty(&hbus->children))
> -               hbus->sysdata.domain = desc->ser;
> +               hbus->sysdata.domain = desc->ser & 0xFFFF;
>         list_add_tail(&hpdev->list_entry, &hbus->children);
>         spin_unlock_irqrestore(&hbus->device_list_lock, flags);
>         return hpdev;
> --
> 1.7.1
>

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

* Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
@ 2017-04-20 18:33   ` Bjorn Helgaas
  0 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2017-04-20 18:33 UTC (permalink / raw)
  To: Haiyang Zhang
  Cc: olaf, Stephen Hemminger, linux-pci, driverdev-devel, linux-kernel

On Thu, Apr 20, 2017 at 11:35 AM, Haiyang Zhang
<haiyangz@exchange.microsoft.com> wrote:
> From: Haiyang Zhang <haiyangz@microsoft.com>
>
> This patch uses the lower 16 bits of the serial number as PCI
> domain, otherwise some drivers may not be able to handle it.

Can you give any more details about this?  Which drivers, for
instance?  Why do drivers care about the domain at all?  Can we or
should we make this more explicit and consistent in the PCI core,
e.g., pci_domain_nr() is currently defined to return "int"; maybe it
should be u32?  (Although I think "int" is the same size as "u32" on
all arches anyway).

> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---
>  drivers/pci/host/pci-hyperv.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
> index e73880c..b18dff3 100644
> --- a/drivers/pci/host/pci-hyperv.c
> +++ b/drivers/pci/host/pci-hyperv.c
> @@ -1334,9 +1334,11 @@ static void put_pcichild(struct hv_pci_dev *hpdev,
>          * can have shorter names than based on the bus instance UUID.
>          * Only the first device serial number is used for domain, so the
>          * domain number will not change after the first device is added.
> +        * The lower 16 bits of the serial number is used, otherwise some
> +        * drivers may not be able to handle it.
>          */
>         if (list_empty(&hbus->children))
> -               hbus->sysdata.domain = desc->ser;
> +               hbus->sysdata.domain = desc->ser & 0xFFFF;
>         list_add_tail(&hpdev->list_entry, &hbus->children);
>         spin_unlock_irqrestore(&hbus->device_list_lock, flags);
>         return hpdev;
> --
> 1.7.1
>
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* RE: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
  2017-04-20 18:33   ` Bjorn Helgaas
  (?)
@ 2017-04-20 18:37     ` Haiyang Zhang
  -1 siblings, 0 replies; 36+ messages in thread
From: Haiyang Zhang @ 2017-04-20 18:37 UTC (permalink / raw)
  To: Bjorn Helgaas, Piotr Jaroszynski
  Cc: linux-pci, KY Srinivasan, Stephen Hemminger, olaf, vkuznets,
	driverdev-devel, linux-kernel

> -----Original Message-----
> From: Bjorn Helgaas [mailto:bhelgaas@google.com]
> Sent: Thursday, April 20, 2017 2:33 PM
> To: Haiyang Zhang <haiyangz@microsoft.com>
> Cc: linux-pci@vger.kernel.org; KY Srinivasan <kys@microsoft.com>;
> Stephen Hemminger <sthemmin@microsoft.com>; olaf@aepfle.de;
> vkuznets@redhat.com; driverdev-devel@linuxdriverproject.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
> 
> On Thu, Apr 20, 2017 at 11:35 AM, Haiyang Zhang
> <haiyangz@exchange.microsoft.com> wrote:
> > From: Haiyang Zhang <haiyangz@microsoft.com>
> >
> > This patch uses the lower 16 bits of the serial number as PCI
> > domain, otherwise some drivers may not be able to handle it.
> 
> Can you give any more details about this?  Which drivers, for
> instance?  Why do drivers care about the domain at all?  Can we or
> should we make this more explicit and consistent in the PCI core,
> e.g., pci_domain_nr() is currently defined to return "int"; maybe it
> should be u32?  (Although I think "int" is the same size as "u32" on
> all arches anyway).

It's Nvidia driver.

Piotr, could you explain why the driver expects 16 bit domain number?

Thanks,
- Haiyang

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

* RE: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
@ 2017-04-20 18:37     ` Haiyang Zhang
  0 siblings, 0 replies; 36+ messages in thread
From: Haiyang Zhang @ 2017-04-20 18:37 UTC (permalink / raw)
  To: Bjorn Helgaas, Piotr Jaroszynski
  Cc: linux-pci, KY Srinivasan, Stephen Hemminger, olaf, vkuznets,
	driverdev-devel, linux-kernel

PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBCam9ybiBIZWxnYWFzIFttYWls
dG86YmhlbGdhYXNAZ29vZ2xlLmNvbV0NCj4gU2VudDogVGh1cnNkYXksIEFwcmlsIDIwLCAyMDE3
IDI6MzMgUE0NCj4gVG86IEhhaXlhbmcgWmhhbmcgPGhhaXlhbmd6QG1pY3Jvc29mdC5jb20+DQo+
IENjOiBsaW51eC1wY2lAdmdlci5rZXJuZWwub3JnOyBLWSBTcmluaXZhc2FuIDxreXNAbWljcm9z
b2Z0LmNvbT47DQo+IFN0ZXBoZW4gSGVtbWluZ2VyIDxzdGhlbW1pbkBtaWNyb3NvZnQuY29tPjsg
b2xhZkBhZXBmbGUuZGU7DQo+IHZrdXpuZXRzQHJlZGhhdC5jb207IGRyaXZlcmRldi1kZXZlbEBs
aW51eGRyaXZlcnByb2plY3Qub3JnOyBsaW51eC0NCj4ga2VybmVsQHZnZXIua2VybmVsLm9yZw0K
PiBTdWJqZWN0OiBSZTogW1BBVENIXSBwY2ktaHlwZXJ2OiBVc2Ugb25seSAxNiBiaXQgaW50ZWdl
ciBmb3IgUENJIGRvbWFpbg0KPiANCj4gT24gVGh1LCBBcHIgMjAsIDIwMTcgYXQgMTE6MzUgQU0s
IEhhaXlhbmcgWmhhbmcNCj4gPGhhaXlhbmd6QGV4Y2hhbmdlLm1pY3Jvc29mdC5jb20+IHdyb3Rl
Og0KPiA+IEZyb206IEhhaXlhbmcgWmhhbmcgPGhhaXlhbmd6QG1pY3Jvc29mdC5jb20+DQo+ID4N
Cj4gPiBUaGlzIHBhdGNoIHVzZXMgdGhlIGxvd2VyIDE2IGJpdHMgb2YgdGhlIHNlcmlhbCBudW1i
ZXIgYXMgUENJDQo+ID4gZG9tYWluLCBvdGhlcndpc2Ugc29tZSBkcml2ZXJzIG1heSBub3QgYmUg
YWJsZSB0byBoYW5kbGUgaXQuDQo+IA0KPiBDYW4geW91IGdpdmUgYW55IG1vcmUgZGV0YWlscyBh
Ym91dCB0aGlzPyAgV2hpY2ggZHJpdmVycywgZm9yDQo+IGluc3RhbmNlPyAgV2h5IGRvIGRyaXZl
cnMgY2FyZSBhYm91dCB0aGUgZG9tYWluIGF0IGFsbD8gIENhbiB3ZSBvcg0KPiBzaG91bGQgd2Ug
bWFrZSB0aGlzIG1vcmUgZXhwbGljaXQgYW5kIGNvbnNpc3RlbnQgaW4gdGhlIFBDSSBjb3JlLA0K
PiBlLmcuLCBwY2lfZG9tYWluX25yKCkgaXMgY3VycmVudGx5IGRlZmluZWQgdG8gcmV0dXJuICJp
bnQiOyBtYXliZSBpdA0KPiBzaG91bGQgYmUgdTMyPyAgKEFsdGhvdWdoIEkgdGhpbmsgImludCIg
aXMgdGhlIHNhbWUgc2l6ZSBhcyAidTMyIiBvbg0KPiBhbGwgYXJjaGVzIGFueXdheSkuDQoNCkl0
J3MgTnZpZGlhIGRyaXZlci4NCg0KUGlvdHIsIGNvdWxkIHlvdSBleHBsYWluIHdoeSB0aGUgZHJp
dmVyIGV4cGVjdHMgMTYgYml0IGRvbWFpbiBudW1iZXI/DQoNClRoYW5rcywNCi0gSGFpeWFuZw0K

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

* RE: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
@ 2017-04-20 18:37     ` Haiyang Zhang
  0 siblings, 0 replies; 36+ messages in thread
From: Haiyang Zhang @ 2017-04-20 18:37 UTC (permalink / raw)
  To: Bjorn Helgaas, Piotr Jaroszynski
  Cc: olaf, Stephen Hemminger, linux-pci, driverdev-devel, linux-kernel

> -----Original Message-----
> From: Bjorn Helgaas [mailto:bhelgaas@google.com]
> Sent: Thursday, April 20, 2017 2:33 PM
> To: Haiyang Zhang <haiyangz@microsoft.com>
> Cc: linux-pci@vger.kernel.org; KY Srinivasan <kys@microsoft.com>;
> Stephen Hemminger <sthemmin@microsoft.com>; olaf@aepfle.de;
> vkuznets@redhat.com; driverdev-devel@linuxdriverproject.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
> 
> On Thu, Apr 20, 2017 at 11:35 AM, Haiyang Zhang
> <haiyangz@exchange.microsoft.com> wrote:
> > From: Haiyang Zhang <haiyangz@microsoft.com>
> >
> > This patch uses the lower 16 bits of the serial number as PCI
> > domain, otherwise some drivers may not be able to handle it.
> 
> Can you give any more details about this?  Which drivers, for
> instance?  Why do drivers care about the domain at all?  Can we or
> should we make this more explicit and consistent in the PCI core,
> e.g., pci_domain_nr() is currently defined to return "int"; maybe it
> should be u32?  (Although I think "int" is the same size as "u32" on
> all arches anyway).

It's Nvidia driver.

Piotr, could you explain why the driver expects 16 bit domain number?

Thanks,
- Haiyang
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
  2017-04-20 18:37     ` Haiyang Zhang
  (?)
@ 2017-04-20 19:12       ` Christoph Hellwig
  -1 siblings, 0 replies; 36+ messages in thread
From: Christoph Hellwig @ 2017-04-20 19:12 UTC (permalink / raw)
  To: Haiyang Zhang
  Cc: Bjorn Helgaas, Piotr Jaroszynski, linux-pci, KY Srinivasan,
	Stephen Hemminger, olaf, vkuznets, driverdev-devel, linux-kernel

On Thu, Apr 20, 2017 at 06:37:35PM +0000, Haiyang Zhang wrote:
> It's Nvidia driver.

Which of the many nvidia drivers in the tree?  Just fix it instead of
coming up with stupid workarounds like this.

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

* Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
@ 2017-04-20 19:12       ` Christoph Hellwig
  0 siblings, 0 replies; 36+ messages in thread
From: Christoph Hellwig @ 2017-04-20 19:12 UTC (permalink / raw)
  To: Haiyang Zhang
  Cc: Bjorn Helgaas, Piotr Jaroszynski, linux-pci, KY Srinivasan,
	Stephen Hemminger, olaf, vkuznets, driverdev-devel, linux-kernel

On Thu, Apr 20, 2017 at 06:37:35PM +0000, Haiyang Zhang wrote:
> It's Nvidia driver.

Which of the many nvidia drivers in the tree?  Just fix it instead of
coming up with stupid workarounds like this.

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

* Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
@ 2017-04-20 19:12       ` Christoph Hellwig
  0 siblings, 0 replies; 36+ messages in thread
From: Christoph Hellwig @ 2017-04-20 19:12 UTC (permalink / raw)
  To: Haiyang Zhang
  Cc: olaf, Stephen Hemminger, linux-pci, driverdev-devel,
	linux-kernel, Piotr Jaroszynski, Bjorn Helgaas

On Thu, Apr 20, 2017 at 06:37:35PM +0000, Haiyang Zhang wrote:
> It's Nvidia driver.

Which of the many nvidia drivers in the tree?  Just fix it instead of
coming up with stupid workarounds like this.
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
  2017-04-20 18:37     ` Haiyang Zhang
  (?)
@ 2017-04-24 23:06       ` John Hubbard
  -1 siblings, 0 replies; 36+ messages in thread
From: John Hubbard @ 2017-04-24 23:06 UTC (permalink / raw)
  To: Haiyang Zhang, Bjorn Helgaas, Piotr Jaroszynski, Christoph Hellwig
  Cc: linux-pci, KY Srinivasan, Stephen Hemminger, olaf, vkuznets,
	driverdev-devel, linux-kernel

On 04/20/2017 11:37 AM, Haiyang Zhang wrote:
>> -----Original Message-----
>> From: Bjorn Helgaas [mailto:bhelgaas@google.com]
>> Sent: Thursday, April 20, 2017 2:33 PM
>> To: Haiyang Zhang <haiyangz@microsoft.com>
>> Cc: linux-pci@vger.kernel.org; KY Srinivasan <kys@microsoft.com>;
>> Stephen Hemminger <sthemmin@microsoft.com>; olaf@aepfle.de;
>> vkuznets@redhat.com; driverdev-devel@linuxdriverproject.org; linux-
>> kernel@vger.kernel.org
>> Subject: Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
>>
>> On Thu, Apr 20, 2017 at 11:35 AM, Haiyang Zhang
>> <haiyangz@exchange.microsoft.com> wrote:
>>> From: Haiyang Zhang <haiyangz@microsoft.com>
>>>
>>> This patch uses the lower 16 bits of the serial number as PCI
>>> domain, otherwise some drivers may not be able to handle it.
>>
>> Can you give any more details about this?  Which drivers, for
>> instance?  Why do drivers care about the domain at all?  Can we or
>> should we make this more explicit and consistent in the PCI core,
>> e.g., pci_domain_nr() is currently defined to return "int"; maybe it
>> should be u32?  (Although I think "int" is the same size as "u32" on
>> all arches anyway).
> 
> It's Nvidia driver.
> 
> Piotr, could you explain why the driver expects 16 bit domain number?

Hi Haiyang and all,

First, a tiny nit about the patch: it would be good to add "Fixing a problem that was introduced 
with commit <4a9b0933bdfc>", in the patch commit message.

Piotr and I just now worked through both the driver and the ACPI/PCI history a little bit, and it 
brings up an interesting question: would it be better for the kernel, long-term, if we changed 
pci_domain_nr() and its callers to use 16 bit values (it's a mini-project, but not too hard)? I ask, 
because:

    a) the ACPI specification[1] says that PCI domains ("PCI Segment Groups") are 16 bits. The other 
16 bits are reserved. I'm concerned that if we don't clamp these to 16 bits in the kernel, virtual 
machines and other experimenters may continue to do things that cause problems--especially if 
ACPI/PCI ever tries to use those reserved 16 bits.

    b) A whirlwind survey of a few non-x86 arches shows that they are casting or truncating the PCI 
domain to 16 bits (here, if other, real linux-pci experts have some input, that would help!)

    c) Looking back at the original commit that added PCI domain support, Linux has specified the 
storage size as 32 bits, right from the start...but it looks like merely a convenience, rather than 
an exact match for a specification.

Please, let me emphasize that the driver can be changed to use 32 bits as well, no problem. But I 
really do want the kernel to have the most accurate and correct code, too, and it really looks (so 
far) like it wants to be 16 bits.

Also...it would be nice if we could use Haiyang's patch as at least a temporary fix, because distros 
are just today releasing the previous code, and HyperV will start breaking "occasionally", depending 
on whether the 32-bit virtual (fake) PCI domain fits within 16 bits. (If not, then we can rush out a 
driver update to fix it, but there will be a window of time with some breakage there.)


[1] http://www.uefi.org/sites/default/files/resources/ACPI_6_1.pdf , seciton 6.5.6, page 397

thanks,

--
John Hubbard
NVIDIA

> 
> Thanks,
> - Haiyang
> 

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

* Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
@ 2017-04-24 23:06       ` John Hubbard
  0 siblings, 0 replies; 36+ messages in thread
From: John Hubbard @ 2017-04-24 23:06 UTC (permalink / raw)
  To: Haiyang Zhang, Bjorn Helgaas, Piotr Jaroszynski, Christoph Hellwig
  Cc: linux-pci, KY Srinivasan, Stephen Hemminger, olaf, vkuznets,
	driverdev-devel, linux-kernel

On 04/20/2017 11:37 AM, Haiyang Zhang wrote:
>> -----Original Message-----
>> From: Bjorn Helgaas [mailto:bhelgaas@google.com]
>> Sent: Thursday, April 20, 2017 2:33 PM
>> To: Haiyang Zhang <haiyangz@microsoft.com>
>> Cc: linux-pci@vger.kernel.org; KY Srinivasan <kys@microsoft.com>;
>> Stephen Hemminger <sthemmin@microsoft.com>; olaf@aepfle.de;
>> vkuznets@redhat.com; driverdev-devel@linuxdriverproject.org; linux-
>> kernel@vger.kernel.org
>> Subject: Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
>>
>> On Thu, Apr 20, 2017 at 11:35 AM, Haiyang Zhang
>> <haiyangz@exchange.microsoft.com> wrote:
>>> From: Haiyang Zhang <haiyangz@microsoft.com>
>>>
>>> This patch uses the lower 16 bits of the serial number as PCI
>>> domain, otherwise some drivers may not be able to handle it.
>>
>> Can you give any more details about this?  Which drivers, for
>> instance?  Why do drivers care about the domain at all?  Can we or
>> should we make this more explicit and consistent in the PCI core,
>> e.g., pci_domain_nr() is currently defined to return "int"; maybe it
>> should be u32?  (Although I think "int" is the same size as "u32" on
>> all arches anyway).
> 
> It's Nvidia driver.
> 
> Piotr, could you explain why the driver expects 16 bit domain number?

Hi Haiyang and all,

First, a tiny nit about the patch: it would be good to add "Fixing a problem that was introduced 
with commit <4a9b0933bdfc>", in the patch commit message.

Piotr and I just now worked through both the driver and the ACPI/PCI history a little bit, and it 
brings up an interesting question: would it be better for the kernel, long-term, if we changed 
pci_domain_nr() and its callers to use 16 bit values (it's a mini-project, but not too hard)? I ask, 
because:

    a) the ACPI specification[1] says that PCI domains ("PCI Segment Groups") are 16 bits. The other 
16 bits are reserved. I'm concerned that if we don't clamp these to 16 bits in the kernel, virtual 
machines and other experimenters may continue to do things that cause problems--especially if 
ACPI/PCI ever tries to use those reserved 16 bits.

    b) A whirlwind survey of a few non-x86 arches shows that they are casting or truncating the PCI 
domain to 16 bits (here, if other, real linux-pci experts have some input, that would help!)

    c) Looking back at the original commit that added PCI domain support, Linux has specified the 
storage size as 32 bits, right from the start...but it looks like merely a convenience, rather than 
an exact match for a specification.

Please, let me emphasize that the driver can be changed to use 32 bits as well, no problem. But I 
really do want the kernel to have the most accurate and correct code, too, and it really looks (so 
far) like it wants to be 16 bits.

Also...it would be nice if we could use Haiyang's patch as at least a temporary fix, because distros 
are just today releasing the previous code, and HyperV will start breaking "occasionally", depending 
on whether the 32-bit virtual (fake) PCI domain fits within 16 bits. (If not, then we can rush out a 
driver update to fix it, but there will be a window of time with some breakage there.)


[1] http://www.uefi.org/sites/default/files/resources/ACPI_6_1.pdf , seciton 6.5.6, page 397

thanks,

--
John Hubbard
NVIDIA

> 
> Thanks,
> - Haiyang
> 

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

* Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
@ 2017-04-24 23:06       ` John Hubbard
  0 siblings, 0 replies; 36+ messages in thread
From: John Hubbard @ 2017-04-24 23:06 UTC (permalink / raw)
  To: Haiyang Zhang, Bjorn Helgaas, Piotr Jaroszynski, Christoph Hellwig
  Cc: olaf, Stephen Hemminger, linux-pci, driverdev-devel, linux-kernel

On 04/20/2017 11:37 AM, Haiyang Zhang wrote:
>> -----Original Message-----
>> From: Bjorn Helgaas [mailto:bhelgaas@google.com]
>> Sent: Thursday, April 20, 2017 2:33 PM
>> To: Haiyang Zhang <haiyangz@microsoft.com>
>> Cc: linux-pci@vger.kernel.org; KY Srinivasan <kys@microsoft.com>;
>> Stephen Hemminger <sthemmin@microsoft.com>; olaf@aepfle.de;
>> vkuznets@redhat.com; driverdev-devel@linuxdriverproject.org; linux-
>> kernel@vger.kernel.org
>> Subject: Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
>>
>> On Thu, Apr 20, 2017 at 11:35 AM, Haiyang Zhang
>> <haiyangz@exchange.microsoft.com> wrote:
>>> From: Haiyang Zhang <haiyangz@microsoft.com>
>>>
>>> This patch uses the lower 16 bits of the serial number as PCI
>>> domain, otherwise some drivers may not be able to handle it.
>>
>> Can you give any more details about this?  Which drivers, for
>> instance?  Why do drivers care about the domain at all?  Can we or
>> should we make this more explicit and consistent in the PCI core,
>> e.g., pci_domain_nr() is currently defined to return "int"; maybe it
>> should be u32?  (Although I think "int" is the same size as "u32" on
>> all arches anyway).
> 
> It's Nvidia driver.
> 
> Piotr, could you explain why the driver expects 16 bit domain number?

Hi Haiyang and all,

First, a tiny nit about the patch: it would be good to add "Fixing a problem that was introduced 
with commit <4a9b0933bdfc>", in the patch commit message.

Piotr and I just now worked through both the driver and the ACPI/PCI history a little bit, and it 
brings up an interesting question: would it be better for the kernel, long-term, if we changed 
pci_domain_nr() and its callers to use 16 bit values (it's a mini-project, but not too hard)? I ask, 
because:

    a) the ACPI specification[1] says that PCI domains ("PCI Segment Groups") are 16 bits. The other 
16 bits are reserved. I'm concerned that if we don't clamp these to 16 bits in the kernel, virtual 
machines and other experimenters may continue to do things that cause problems--especially if 
ACPI/PCI ever tries to use those reserved 16 bits.

    b) A whirlwind survey of a few non-x86 arches shows that they are casting or truncating the PCI 
domain to 16 bits (here, if other, real linux-pci experts have some input, that would help!)

    c) Looking back at the original commit that added PCI domain support, Linux has specified the 
storage size as 32 bits, right from the start...but it looks like merely a convenience, rather than 
an exact match for a specification.

Please, let me emphasize that the driver can be changed to use 32 bits as well, no problem. But I 
really do want the kernel to have the most accurate and correct code, too, and it really looks (so 
far) like it wants to be 16 bits.

Also...it would be nice if we could use Haiyang's patch as at least a temporary fix, because distros 
are just today releasing the previous code, and HyperV will start breaking "occasionally", depending 
on whether the 32-bit virtual (fake) PCI domain fits within 16 bits. (If not, then we can rush out a 
driver update to fix it, but there will be a window of time with some breakage there.)


[1] http://www.uefi.org/sites/default/files/resources/ACPI_6_1.pdf , seciton 6.5.6, page 397

thanks,

--
John Hubbard
NVIDIA

> 
> Thanks,
> - Haiyang
> 
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
  2017-04-24 23:06       ` John Hubbard
  (?)
@ 2017-04-25 13:00         ` Dan Carpenter
  -1 siblings, 0 replies; 36+ messages in thread
From: Dan Carpenter @ 2017-04-25 13:00 UTC (permalink / raw)
  To: John Hubbard
  Cc: Haiyang Zhang, Bjorn Helgaas, Piotr Jaroszynski,
	Christoph Hellwig, olaf, Stephen Hemminger, linux-pci,
	driverdev-devel, linux-kernel

On Mon, Apr 24, 2017 at 04:06:37PM -0700, John Hubbard wrote:
> First, a tiny nit about the patch: it would be good to add "Fixing a problem
> that was introduced with commit <4a9b0933bdfc>", in the patch commit
> message.
> 

Please use the Fixes tag.

Fixes: 123456789012 ("blah blah blah")

regards,
dan carpenter

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

* Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
@ 2017-04-25 13:00         ` Dan Carpenter
  0 siblings, 0 replies; 36+ messages in thread
From: Dan Carpenter @ 2017-04-25 13:00 UTC (permalink / raw)
  To: John Hubbard
  Cc: Haiyang Zhang, Bjorn Helgaas, Piotr Jaroszynski,
	Christoph Hellwig, olaf, Stephen Hemminger, linux-pci,
	driverdev-devel, linux-kernel

On Mon, Apr 24, 2017 at 04:06:37PM -0700, John Hubbard wrote:
> First, a tiny nit about the patch: it would be good to add "Fixing a problem
> that was introduced with commit <4a9b0933bdfc>", in the patch commit
> message.
> 

Please use the Fixes tag.

Fixes: 123456789012 ("blah blah blah")

regards,
dan carpenter

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

* Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
@ 2017-04-25 13:00         ` Dan Carpenter
  0 siblings, 0 replies; 36+ messages in thread
From: Dan Carpenter @ 2017-04-25 13:00 UTC (permalink / raw)
  To: John Hubbard
  Cc: olaf, Stephen Hemminger, linux-pci, Haiyang Zhang,
	driverdev-devel, linux-kernel, Christoph Hellwig,
	Piotr Jaroszynski, Bjorn Helgaas

On Mon, Apr 24, 2017 at 04:06:37PM -0700, John Hubbard wrote:
> First, a tiny nit about the patch: it would be good to add "Fixing a problem
> that was introduced with commit <4a9b0933bdfc>", in the patch commit
> message.
> 

Please use the Fixes tag.

Fixes: 123456789012 ("blah blah blah")

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
  2017-04-24 23:06       ` John Hubbard
  (?)
@ 2017-04-25 13:14         ` Christoph Hellwig
  -1 siblings, 0 replies; 36+ messages in thread
From: Christoph Hellwig @ 2017-04-25 13:14 UTC (permalink / raw)
  To: John Hubbard
  Cc: Haiyang Zhang, Bjorn Helgaas, Piotr Jaroszynski,
	Christoph Hellwig, linux-pci, KY Srinivasan, Stephen Hemminger,
	olaf, vkuznets, driverdev-devel, linux-kernel

Hi John,

please fix your quoting of the previous mails, thanks!


What ACPI defines does not matter at all.  Linux uses 32-bit domains
IDs, and on x86 specifily uses those for non-ACPI enumarated domains
(e.g. VMD).

You've also not demontrated any issue with any Linux driver yet.

> Also...it would be nice if we could use Haiyang's patch as at least a
> temporary fix, because distros are just today releasing the previous code,
> and HyperV will start breaking "occasionally", depending on whether the
> 32-bit virtual (fake) PCI domain fits within 16 bits. (If not, then we can
> rush out a driver update to fix it, but there will be a window of time with
> some breakage there.)

Just send the fix to whatever driver is broken to the driver maintainer.
But I can't find a single broken driver in the tree, and as you know
nothing else matters for Linux anyway.

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

* Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
@ 2017-04-25 13:14         ` Christoph Hellwig
  0 siblings, 0 replies; 36+ messages in thread
From: Christoph Hellwig @ 2017-04-25 13:14 UTC (permalink / raw)
  To: John Hubbard
  Cc: Haiyang Zhang, Bjorn Helgaas, Piotr Jaroszynski,
	Christoph Hellwig, linux-pci, KY Srinivasan, Stephen Hemminger,
	olaf, vkuznets, driverdev-devel, linux-kernel

Hi John,

please fix your quoting of the previous mails, thanks!


What ACPI defines does not matter at all.  Linux uses 32-bit domains
IDs, and on x86 specifily uses those for non-ACPI enumarated domains
(e.g. VMD).

You've also not demontrated any issue with any Linux driver yet.

> Also...it would be nice if we could use Haiyang's patch as at least a
> temporary fix, because distros are just today releasing the previous code,
> and HyperV will start breaking "occasionally", depending on whether the
> 32-bit virtual (fake) PCI domain fits within 16 bits. (If not, then we can
> rush out a driver update to fix it, but there will be a window of time with
> some breakage there.)

Just send the fix to whatever driver is broken to the driver maintainer.
But I can't find a single broken driver in the tree, and as you know
nothing else matters for Linux anyway.

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

* Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
@ 2017-04-25 13:14         ` Christoph Hellwig
  0 siblings, 0 replies; 36+ messages in thread
From: Christoph Hellwig @ 2017-04-25 13:14 UTC (permalink / raw)
  To: John Hubbard
  Cc: olaf, Stephen Hemminger, linux-pci, Haiyang Zhang,
	driverdev-devel, linux-kernel, Christoph Hellwig,
	Piotr Jaroszynski, Bjorn Helgaas

Hi John,

please fix your quoting of the previous mails, thanks!


What ACPI defines does not matter at all.  Linux uses 32-bit domains
IDs, and on x86 specifily uses those for non-ACPI enumarated domains
(e.g. VMD).

You've also not demontrated any issue with any Linux driver yet.

> Also...it would be nice if we could use Haiyang's patch as at least a
> temporary fix, because distros are just today releasing the previous code,
> and HyperV will start breaking "occasionally", depending on whether the
> 32-bit virtual (fake) PCI domain fits within 16 bits. (If not, then we can
> rush out a driver update to fix it, but there will be a window of time with
> some breakage there.)

Just send the fix to whatever driver is broken to the driver maintainer.
But I can't find a single broken driver in the tree, and as you know
nothing else matters for Linux anyway.
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
  2017-04-25 13:14         ` Christoph Hellwig
  (?)
@ 2017-04-25 18:19           ` John Hubbard
  -1 siblings, 0 replies; 36+ messages in thread
From: John Hubbard @ 2017-04-25 18:19 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Haiyang Zhang, Bjorn Helgaas, Piotr Jaroszynski, linux-pci,
	KY Srinivasan, Stephen Hemminger, olaf, vkuznets,
	driverdev-devel, linux-kernel

On Tue, 25 Apr 2017, Christoph Hellwig wrote:

> Hi John,
> 
> please fix your quoting of the previous mails, thanks!

Shoot, sorry about any quoting issues. I'm sufficiently new to conversing 
on these lists that I'm not even sure which mistake I made.

> 
> 
> What ACPI defines does not matter at all.  Linux uses 32-bit domains
> IDs, and on x86 specifily uses those for non-ACPI enumarated domains
> (e.g. VMD).
> 
> You've also not demontrated any issue with any Linux driver yet.

The NVIDIA out-of-tree driver has historically treated domains as 16-bit. 
So this showed up when people tried to run that driver in a hyper-v VM.

> 
> > Also...it would be nice if we could use Haiyang's patch as at least a
> > temporary fix, because distros are just today releasing the previous code,
> > and HyperV will start breaking "occasionally", depending on whether the
> > 32-bit virtual (fake) PCI domain fits within 16 bits. (If not, then we can
> > rush out a driver update to fix it, but there will be a window of time with
> > some breakage there.)
> 
> Just send the fix to whatever driver is broken to the driver maintainer.

Done: that would be us. :)

> But I can't find a single broken driver in the tree, and as you know
> nothing else matters for Linux anyway.
> 

Yes, I looked at Nouveau, and I see that they allow for a 32-bit domain, 
so I agree that we haven't found any in-tree drivers that have a problem.

Anyway, thanks for the answers and explanations.

--
thanks,
john h

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

* Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
@ 2017-04-25 18:19           ` John Hubbard
  0 siblings, 0 replies; 36+ messages in thread
From: John Hubbard @ 2017-04-25 18:19 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Haiyang Zhang, Bjorn Helgaas, Piotr Jaroszynski, linux-pci,
	KY Srinivasan, Stephen Hemminger, olaf, vkuznets,
	driverdev-devel, linux-kernel

On Tue, 25 Apr 2017, Christoph Hellwig wrote:

> Hi John,
> 
> please fix your quoting of the previous mails, thanks!

Shoot, sorry about any quoting issues. I'm sufficiently new to conversing 
on these lists that I'm not even sure which mistake I made.

> 
> 
> What ACPI defines does not matter at all.  Linux uses 32-bit domains
> IDs, and on x86 specifily uses those for non-ACPI enumarated domains
> (e.g. VMD).
> 
> You've also not demontrated any issue with any Linux driver yet.

The NVIDIA out-of-tree driver has historically treated domains as 16-bit. 
So this showed up when people tried to run that driver in a hyper-v VM.

> 
> > Also...it would be nice if we could use Haiyang's patch as at least a
> > temporary fix, because distros are just today releasing the previous code,
> > and HyperV will start breaking "occasionally", depending on whether the
> > 32-bit virtual (fake) PCI domain fits within 16 bits. (If not, then we can
> > rush out a driver update to fix it, but there will be a window of time with
> > some breakage there.)
> 
> Just send the fix to whatever driver is broken to the driver maintainer.

Done: that would be us. :)

> But I can't find a single broken driver in the tree, and as you know
> nothing else matters for Linux anyway.
> 

Yes, I looked at Nouveau, and I see that they allow for a 32-bit domain, 
so I agree that we haven't found any in-tree drivers that have a problem.

Anyway, thanks for the answers and explanations.

--
thanks,
john h

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

* Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
@ 2017-04-25 18:19           ` John Hubbard
  0 siblings, 0 replies; 36+ messages in thread
From: John Hubbard @ 2017-04-25 18:19 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: olaf, Stephen Hemminger, linux-pci, Haiyang Zhang,
	driverdev-devel, linux-kernel, Piotr Jaroszynski, Bjorn Helgaas

On Tue, 25 Apr 2017, Christoph Hellwig wrote:

> Hi John,
> 
> please fix your quoting of the previous mails, thanks!

Shoot, sorry about any quoting issues. I'm sufficiently new to conversing 
on these lists that I'm not even sure which mistake I made.

> 
> 
> What ACPI defines does not matter at all.  Linux uses 32-bit domains
> IDs, and on x86 specifily uses those for non-ACPI enumarated domains
> (e.g. VMD).
> 
> You've also not demontrated any issue with any Linux driver yet.

The NVIDIA out-of-tree driver has historically treated domains as 16-bit. 
So this showed up when people tried to run that driver in a hyper-v VM.

> 
> > Also...it would be nice if we could use Haiyang's patch as at least a
> > temporary fix, because distros are just today releasing the previous code,
> > and HyperV will start breaking "occasionally", depending on whether the
> > 32-bit virtual (fake) PCI domain fits within 16 bits. (If not, then we can
> > rush out a driver update to fix it, but there will be a window of time with
> > some breakage there.)
> 
> Just send the fix to whatever driver is broken to the driver maintainer.

Done: that would be us. :)

> But I can't find a single broken driver in the tree, and as you know
> nothing else matters for Linux anyway.
> 

Yes, I looked at Nouveau, and I see that they allow for a 32-bit domain, 
so I agree that we haven't found any in-tree drivers that have a problem.

Anyway, thanks for the answers and explanations.

--
thanks,
john h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
  2017-06-19 21:07   ` Bjorn Helgaas
@ 2017-06-19 21:10     ` Christoph Hellwig
  -1 siblings, 0 replies; 36+ messages in thread
From: Christoph Hellwig @ 2017-06-19 21:10 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: haiyangz, bhelgaas, linux-pci, kys, sthemmin, olaf, vkuznets,
	driverdev-devel, linux-kernel, Christoph Hellwig

FYI, I've also got another driver in progress that will need domains
assigned outside the ACPI range, so it's not just limited to VMD.

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

* Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
@ 2017-06-19 21:10     ` Christoph Hellwig
  0 siblings, 0 replies; 36+ messages in thread
From: Christoph Hellwig @ 2017-06-19 21:10 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: olaf, sthemmin, linux-pci, haiyangz, driverdev-devel,
	linux-kernel, Christoph Hellwig, bhelgaas, vkuznets

FYI, I've also got another driver in progress that will need domains
assigned outside the ACPI range, so it's not just limited to VMD.
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
  2017-05-24 20:39 ` Haiyang Zhang
@ 2017-06-19 21:07   ` Bjorn Helgaas
  -1 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2017-06-19 21:07 UTC (permalink / raw)
  To: haiyangz
  Cc: bhelgaas, linux-pci, kys, sthemmin, olaf, vkuznets,
	driverdev-devel, linux-kernel, Christoph Hellwig

[+cc Christoph]

On Wed, May 24, 2017 at 01:39:15PM -0700, Haiyang Zhang wrote:
> From: Haiyang Zhang <haiyangz@microsoft.com>
> 
> This patch uses the lower 16 bits of the serial number as PCI
> domain, otherwise some drivers may not be able to handle it.
> 
> Besides Nvidia drivers, we also found X.org, and DPDK handle
> only 16 bit PCI domain.

If you've sent patches to X.org and DPDK, please includes URLs to
them.

Christoph pointed out the conflict with VMD: vmd_find_free_domain()
allocates domains starting at 0x10000 to avoid the 16-bit domains
returned by ACPI _SEG.

I think we need a solution that works for both Nvidia/Hyper-V and VMD.
As it is, it looks like this will fix one place but break things
elsewhere.

If you believe that this will not break VMD, please explain.

Bjorn

> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---
>  drivers/pci/host/pci-hyperv.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
> index 8493638..51a815d 100644
> --- a/drivers/pci/host/pci-hyperv.c
> +++ b/drivers/pci/host/pci-hyperv.c
> @@ -1335,9 +1335,11 @@ static void put_pcichild(struct hv_pci_dev *hpdev,
>  	 * can have shorter names than based on the bus instance UUID.
>  	 * Only the first device serial number is used for domain, so the
>  	 * domain number will not change after the first device is added.
> +	 * The lower 16 bits of the serial number is used, otherwise some
> +	 * drivers may not be able to handle it.
>  	 */
>  	if (list_empty(&hbus->children))
> -		hbus->sysdata.domain = desc->ser;
> +		hbus->sysdata.domain = desc->ser & 0xFFFF;
>  	list_add_tail(&hpdev->list_entry, &hbus->children);
>  	spin_unlock_irqrestore(&hbus->device_list_lock, flags);
>  	return hpdev;
> -- 
> 1.7.1
> 

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

* Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
@ 2017-06-19 21:07   ` Bjorn Helgaas
  0 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2017-06-19 21:07 UTC (permalink / raw)
  To: haiyangz
  Cc: olaf, sthemmin, linux-pci, driverdev-devel, linux-kernel,
	Christoph Hellwig, bhelgaas, vkuznets

[+cc Christoph]

On Wed, May 24, 2017 at 01:39:15PM -0700, Haiyang Zhang wrote:
> From: Haiyang Zhang <haiyangz@microsoft.com>
> 
> This patch uses the lower 16 bits of the serial number as PCI
> domain, otherwise some drivers may not be able to handle it.
> 
> Besides Nvidia drivers, we also found X.org, and DPDK handle
> only 16 bit PCI domain.

If you've sent patches to X.org and DPDK, please includes URLs to
them.

Christoph pointed out the conflict with VMD: vmd_find_free_domain()
allocates domains starting at 0x10000 to avoid the 16-bit domains
returned by ACPI _SEG.

I think we need a solution that works for both Nvidia/Hyper-V and VMD.
As it is, it looks like this will fix one place but break things
elsewhere.

If you believe that this will not break VMD, please explain.

Bjorn

> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---
>  drivers/pci/host/pci-hyperv.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
> index 8493638..51a815d 100644
> --- a/drivers/pci/host/pci-hyperv.c
> +++ b/drivers/pci/host/pci-hyperv.c
> @@ -1335,9 +1335,11 @@ static void put_pcichild(struct hv_pci_dev *hpdev,
>  	 * can have shorter names than based on the bus instance UUID.
>  	 * Only the first device serial number is used for domain, so the
>  	 * domain number will not change after the first device is added.
> +	 * The lower 16 bits of the serial number is used, otherwise some
> +	 * drivers may not be able to handle it.
>  	 */
>  	if (list_empty(&hbus->children))
> -		hbus->sysdata.domain = desc->ser;
> +		hbus->sysdata.domain = desc->ser & 0xFFFF;
>  	list_add_tail(&hpdev->list_entry, &hbus->children);
>  	spin_unlock_irqrestore(&hbus->device_list_lock, flags);
>  	return hpdev;
> -- 
> 1.7.1
> 
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
  2017-05-25 13:19   ` Alan Cox
@ 2017-05-25 15:52       ` Stephen Hemminger
  0 siblings, 0 replies; 36+ messages in thread
From: Stephen Hemminger @ 2017-05-25 15:52 UTC (permalink / raw)
  To: Alan Cox; +Cc: Haiyang Zhang, olaf, linux-pci, driverdev-devel, linux-kernel

On Thu, 25 May 2017 14:19:55 +0100
Alan Cox <gnomes@lxorguk.ukuu.org.uk> wrote:

> > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > ---    
> > 
> > According to Stephen Hemminger <sthemmin@microsoft.com>, there are
> > additional programs, like X.org, DPDK, are also using 16-bit only
> > PCI domain numbers. So, I'm submitting this patch for re-consideration.  
> 
> The correct way to handle this is to send the needed patches to DPDK and
> to X.org both of whom will I am sure be delighted to get it fixed in
> their codebase.

Both projects have stable ABI requirements. And the lead time to get
the change propagated out to applications is long (>5yrs) even longer
with the Enterprise distro's. As developers we can all just pass the
buck but this doesn't help users in any reasonable time fram. It is not
as simple as just making a patch or pull request for their upstream
code bases.

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

* Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
@ 2017-05-25 15:52       ` Stephen Hemminger
  0 siblings, 0 replies; 36+ messages in thread
From: Stephen Hemminger @ 2017-05-25 15:52 UTC (permalink / raw)
  To: Alan Cox; +Cc: linux-pci, Haiyang Zhang, driverdev-devel, olaf, linux-kernel

On Thu, 25 May 2017 14:19:55 +0100
Alan Cox <gnomes@lxorguk.ukuu.org.uk> wrote:

> > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > ---    
> > 
> > According to Stephen Hemminger <sthemmin@microsoft.com>, there are
> > additional programs, like X.org, DPDK, are also using 16-bit only
> > PCI domain numbers. So, I'm submitting this patch for re-consideration.  
> 
> The correct way to handle this is to send the needed patches to DPDK and
> to X.org both of whom will I am sure be delighted to get it fixed in
> their codebase.

Both projects have stable ABI requirements. And the lead time to get
the change propagated out to applications is long (>5yrs) even longer
with the Enterprise distro's. As developers we can all just pass the
buck but this doesn't help users in any reasonable time fram. It is not
as simple as just making a patch or pull request for their upstream
code bases.
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
  2017-05-24 20:43   ` Haiyang Zhang
  (?)
  (?)
@ 2017-05-25 13:19   ` Alan Cox
  2017-05-25 15:52       ` Stephen Hemminger
  -1 siblings, 1 reply; 36+ messages in thread
From: Alan Cox @ 2017-05-25 13:19 UTC (permalink / raw)
  To: Haiyang Zhang
  Cc: olaf,   <linux-kernel@vger.kernel.org>,
	Stephen Hemminger, linux-pci,
	driverdev-devel@linuxdriverproject.org"
	<driverdev-devel@linuxdriverproject.org>,

> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > ---  
> 
> According to Stephen Hemminger <sthemmin@microsoft.com>, there are
> additional programs, like X.org, DPDK, are also using 16-bit only
> PCI domain numbers. So, I'm submitting this patch for re-consideration.

The correct way to handle this is to send the needed patches to DPDK and
to X.org both of whom will I am sure be delighted to get it fixed in
their codebase.

Alan
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
  2017-05-24 20:39 ` Haiyang Zhang
@ 2017-05-25  7:56   ` Christoph Hellwig
  -1 siblings, 0 replies; 36+ messages in thread
From: Christoph Hellwig @ 2017-05-25  7:56 UTC (permalink / raw)
  To: haiyangz
  Cc: bhelgaas, linux-pci, kys, sthemmin, olaf, vkuznets,
	driverdev-devel, linux-kernel

On Wed, May 24, 2017 at 01:39:15PM -0700, Haiyang Zhang wrote:
> From: Haiyang Zhang <haiyangz@microsoft.com>
> 
> This patch uses the lower 16 bits of the serial number as PCI
> domain, otherwise some drivers may not be able to handle it.
> 
> Besides Nvidia drivers, we also found X.org, and DPDK handle
> only 16 bit PCI domain.

And they will all break behind VMD for example, so NAK.  Linux domains
are 32 bits.

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

* Re: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
@ 2017-05-25  7:56   ` Christoph Hellwig
  0 siblings, 0 replies; 36+ messages in thread
From: Christoph Hellwig @ 2017-05-25  7:56 UTC (permalink / raw)
  To: haiyangz
  Cc: olaf, sthemmin, linux-pci, driverdev-devel, linux-kernel,
	bhelgaas, vkuznets

On Wed, May 24, 2017 at 01:39:15PM -0700, Haiyang Zhang wrote:
> From: Haiyang Zhang <haiyangz@microsoft.com>
> 
> This patch uses the lower 16 bits of the serial number as PCI
> domain, otherwise some drivers may not be able to handle it.
> 
> Besides Nvidia drivers, we also found X.org, and DPDK handle
> only 16 bit PCI domain.

And they will all break behind VMD for example, so NAK.  Linux domains
are 32 bits.
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* RE: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
  2017-05-24 20:39 ` Haiyang Zhang
  (?)
@ 2017-05-24 20:43   ` Haiyang Zhang
  -1 siblings, 0 replies; 36+ messages in thread
From: Haiyang Zhang @ 2017-05-24 20:43 UTC (permalink / raw)
  To: Haiyang Zhang, bhelgaas, linux-pci
  Cc: KY Srinivasan, Stephen Hemminger, olaf, vkuznets,
	driverdev-devel, linux-kernel



> -----Original Message-----
> From: Haiyang Zhang [mailto:haiyangz@exchange.microsoft.com]
> Sent: Wednesday, May 24, 2017 4:39 PM
> To: bhelgaas@google.com; linux-pci@vger.kernel.org
> Cc: Haiyang Zhang <haiyangz@microsoft.com>; KY Srinivasan
> <kys@microsoft.com>; Stephen Hemminger <sthemmin@microsoft.com>;
> olaf@aepfle.de; vkuznets@redhat.com; driverdev-
> devel@linuxdriverproject.org; linux-kernel@vger.kernel.org
> Subject: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
> 
> [This sender failed our fraud detection checks and may not be who they
> appear to be. Learn about spoofing at http://aka.ms/LearnAboutSpoofing]
> 
> From: Haiyang Zhang <haiyangz@microsoft.com>
> 
> This patch uses the lower 16 bits of the serial number as PCI
> domain, otherwise some drivers may not be able to handle it.
> 
> Besides Nvidia drivers, we also found X.org, and DPDK handle
> only 16 bit PCI domain.
> 
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---

According to Stephen Hemminger <sthemmin@microsoft.com>, there are
additional programs, like X.org, DPDK, are also using 16-bit only
PCI domain numbers. So, I'm submitting this patch for re-consideration.

Thanks,
- Haiyang

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

* RE: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
@ 2017-05-24 20:43   ` Haiyang Zhang
  0 siblings, 0 replies; 36+ messages in thread
From: Haiyang Zhang @ 2017-05-24 20:43 UTC (permalink / raw)
  To: Haiyang Zhang, bhelgaas, linux-pci
  Cc: KY Srinivasan, Stephen Hemminger, olaf, vkuznets,
	driverdev-devel, linux-kernel



> -----Original Message-----
> From: Haiyang Zhang [mailto:haiyangz@exchange.microsoft.com]
> Sent: Wednesday, May 24, 2017 4:39 PM
> To: bhelgaas@google.com; linux-pci@vger.kernel.org
> Cc: Haiyang Zhang <haiyangz@microsoft.com>; KY Srinivasan
> <kys@microsoft.com>; Stephen Hemminger <sthemmin@microsoft.com>;
> olaf@aepfle.de; vkuznets@redhat.com; driverdev-
> devel@linuxdriverproject.org; linux-kernel@vger.kernel.org
> Subject: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
>=20
> [This sender failed our fraud detection checks and may not be who they
> appear to be. Learn about spoofing at http://aka.ms/LearnAboutSpoofing]
>=20
> From: Haiyang Zhang <haiyangz@microsoft.com>
>=20
> This patch uses the lower 16 bits of the serial number as PCI
> domain, otherwise some drivers may not be able to handle it.
>=20
> Besides Nvidia drivers, we also found X.org, and DPDK handle
> only 16 bit PCI domain.
>=20
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---

According to Stephen Hemminger <sthemmin@microsoft.com>, there are
additional programs, like X.org, DPDK, are also using 16-bit only
PCI domain numbers. So, I'm submitting this patch for re-consideration.

Thanks,
- Haiyang

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

* RE: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
@ 2017-05-24 20:43   ` Haiyang Zhang
  0 siblings, 0 replies; 36+ messages in thread
From: Haiyang Zhang @ 2017-05-24 20:43 UTC (permalink / raw)
  To: Haiyang Zhang, bhelgaas, linux-pci
  Cc: olaf, Stephen Hemminger, driverdev-devel, linux-kernel, vkuznets



> -----Original Message-----
> From: Haiyang Zhang [mailto:haiyangz@exchange.microsoft.com]
> Sent: Wednesday, May 24, 2017 4:39 PM
> To: bhelgaas@google.com; linux-pci@vger.kernel.org
> Cc: Haiyang Zhang <haiyangz@microsoft.com>; KY Srinivasan
> <kys@microsoft.com>; Stephen Hemminger <sthemmin@microsoft.com>;
> olaf@aepfle.de; vkuznets@redhat.com; driverdev-
> devel@linuxdriverproject.org; linux-kernel@vger.kernel.org
> Subject: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
> 
> [This sender failed our fraud detection checks and may not be who they
> appear to be. Learn about spoofing at http://aka.ms/LearnAboutSpoofing]
> 
> From: Haiyang Zhang <haiyangz@microsoft.com>
> 
> This patch uses the lower 16 bits of the serial number as PCI
> domain, otherwise some drivers may not be able to handle it.
> 
> Besides Nvidia drivers, we also found X.org, and DPDK handle
> only 16 bit PCI domain.
> 
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---

According to Stephen Hemminger <sthemmin@microsoft.com>, there are
additional programs, like X.org, DPDK, are also using 16-bit only
PCI domain numbers. So, I'm submitting this patch for re-consideration.

Thanks,
- Haiyang

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
@ 2017-05-24 20:39 ` Haiyang Zhang
  0 siblings, 0 replies; 36+ messages in thread
From: Haiyang Zhang @ 2017-05-24 20:39 UTC (permalink / raw)
  To: bhelgaas, linux-pci
  Cc: haiyangz, kys, sthemmin, olaf, vkuznets, driverdev-devel, linux-kernel

From: Haiyang Zhang <haiyangz@microsoft.com>

This patch uses the lower 16 bits of the serial number as PCI
domain, otherwise some drivers may not be able to handle it.

Besides Nvidia drivers, we also found X.org, and DPDK handle
only 16 bit PCI domain.

Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/pci/host/pci-hyperv.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
index 8493638..51a815d 100644
--- a/drivers/pci/host/pci-hyperv.c
+++ b/drivers/pci/host/pci-hyperv.c
@@ -1335,9 +1335,11 @@ static void put_pcichild(struct hv_pci_dev *hpdev,
 	 * can have shorter names than based on the bus instance UUID.
 	 * Only the first device serial number is used for domain, so the
 	 * domain number will not change after the first device is added.
+	 * The lower 16 bits of the serial number is used, otherwise some
+	 * drivers may not be able to handle it.
 	 */
 	if (list_empty(&hbus->children))
-		hbus->sysdata.domain = desc->ser;
+		hbus->sysdata.domain = desc->ser & 0xFFFF;
 	list_add_tail(&hpdev->list_entry, &hbus->children);
 	spin_unlock_irqrestore(&hbus->device_list_lock, flags);
 	return hpdev;
-- 
1.7.1

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

* [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain
@ 2017-05-24 20:39 ` Haiyang Zhang
  0 siblings, 0 replies; 36+ messages in thread
From: Haiyang Zhang @ 2017-05-24 20:39 UTC (permalink / raw)
  To: bhelgaas, linux-pci
  Cc: olaf, sthemmin, haiyangz, driverdev-devel, linux-kernel, vkuznets

From: Haiyang Zhang <haiyangz@microsoft.com>

This patch uses the lower 16 bits of the serial number as PCI
domain, otherwise some drivers may not be able to handle it.

Besides Nvidia drivers, we also found X.org, and DPDK handle
only 16 bit PCI domain.

Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/pci/host/pci-hyperv.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
index 8493638..51a815d 100644
--- a/drivers/pci/host/pci-hyperv.c
+++ b/drivers/pci/host/pci-hyperv.c
@@ -1335,9 +1335,11 @@ static void put_pcichild(struct hv_pci_dev *hpdev,
 	 * can have shorter names than based on the bus instance UUID.
 	 * Only the first device serial number is used for domain, so the
 	 * domain number will not change after the first device is added.
+	 * The lower 16 bits of the serial number is used, otherwise some
+	 * drivers may not be able to handle it.
 	 */
 	if (list_empty(&hbus->children))
-		hbus->sysdata.domain = desc->ser;
+		hbus->sysdata.domain = desc->ser & 0xFFFF;
 	list_add_tail(&hpdev->list_entry, &hbus->children);
 	spin_unlock_irqrestore(&hbus->device_list_lock, flags);
 	return hpdev;
-- 
1.7.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

end of thread, other threads:[~2017-06-19 21:10 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-20 16:35 [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain Haiyang Zhang
2017-04-20 16:35 ` Haiyang Zhang
2017-04-20 18:33 ` Bjorn Helgaas
2017-04-20 18:33   ` Bjorn Helgaas
2017-04-20 18:37   ` Haiyang Zhang
2017-04-20 18:37     ` Haiyang Zhang
2017-04-20 18:37     ` Haiyang Zhang
2017-04-20 19:12     ` Christoph Hellwig
2017-04-20 19:12       ` Christoph Hellwig
2017-04-20 19:12       ` Christoph Hellwig
2017-04-24 23:06     ` John Hubbard
2017-04-24 23:06       ` John Hubbard
2017-04-24 23:06       ` John Hubbard
2017-04-25 13:00       ` Dan Carpenter
2017-04-25 13:00         ` Dan Carpenter
2017-04-25 13:00         ` Dan Carpenter
2017-04-25 13:14       ` Christoph Hellwig
2017-04-25 13:14         ` Christoph Hellwig
2017-04-25 13:14         ` Christoph Hellwig
2017-04-25 18:19         ` John Hubbard
2017-04-25 18:19           ` John Hubbard
2017-04-25 18:19           ` John Hubbard
2017-05-24 20:39 Haiyang Zhang
2017-05-24 20:39 ` Haiyang Zhang
2017-05-24 20:43 ` Haiyang Zhang
2017-05-24 20:43   ` Haiyang Zhang
2017-05-24 20:43   ` Haiyang Zhang
2017-05-25 13:19   ` Alan Cox
2017-05-25 15:52     ` Stephen Hemminger
2017-05-25 15:52       ` Stephen Hemminger
2017-05-25  7:56 ` Christoph Hellwig
2017-05-25  7:56   ` Christoph Hellwig
2017-06-19 21:07 ` Bjorn Helgaas
2017-06-19 21:07   ` Bjorn Helgaas
2017-06-19 21:10   ` Christoph Hellwig
2017-06-19 21:10     ` Christoph Hellwig

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.