[PATCH v8] PCI: hv: Make sure the bus domain is really unique

[PATCH v8] PCI: hv: Make sure the bus domain is really unique

[Sridhar Pitchai]

When Linux runs as a guest VM in Hyper-V and Hyper-V adds the virtual PCI
bus to the guest, Hyper-V always provides unique PCI domain.

commit 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI domain")
overrode unique domain with the serial number of the first device added to
the virtual PCI bus.

The reason for that patch was to have a consistent and short name for the
device, but Hyper-V doesn't provide unique serial numbers. Using non-unique
serial numbers as domain IDs leads to duplicate device addresses, which
causes PCI bus registration to fail.

commit 0c195567a8f6 ("netvsc: transparent VF management") avoids the need
for commit 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI
domain").  When scripts were used to configure VF devices, the name of
the VF needed to be consistent and short, but with commit 0c195567a8f6
("netvsc: transparent VF management") all the setup is done in the kernel,
and we do not need to maintain consistent name.

Revert commit 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI
domain") so we can reliably support multiple devices being assigned to
a guest.

This revert should only be backported to kernels that contain commit
0c195567a8f6 ("netvsc: transparent VF management").

Fixes: 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI domain")
Signed-off-by: Sridhar Pitchai <sridhar.pitchai@microsoft.com>
Cc: stable@vger.kernel.org # v4.14+
Reviewed-by: Bjorn Helgaas <bhelgaas@google.com>

---
Changes in v8:
* fix the commit comment. [Lorenzo Pieralisi, Bjorn Helgaas]
---
 drivers/pci/host/pci-hyperv.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
index 2faf38eab785..ac67e56e451a 100644
--- a/drivers/pci/host/pci-hyperv.c
+++ b/drivers/pci/host/pci-hyperv.c
@@ -1518,17 +1518,6 @@ static struct hv_pci_dev *new_pcichild_device(struct hv_pcibus_device *hbus,
 	get_pcichild(hpdev, hv_pcidev_ref_childlist);
 	spin_lock_irqsave(&hbus->device_list_lock, flags);
 
-	/*
-	 * When a device is being added to the bus, we set the PCI domain
-	 * number to be the device serial number, which is non-zero and
-	 * unique on the same VM.  The serial numbers start with 1, and
-	 * increase by 1 for each device.  So device names including this
-	 * 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.
-	 */
-	if (list_empty(&hbus->children))
-		hbus->sysdata.domain = desc->ser;
 	list_add_tail(&hpdev->list_entry, &hbus->children);
 	spin_unlock_irqrestore(&hbus->device_list_lock, flags);
 	return hpdev;
-- 
2.14.1


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

[PATCH v8] PCI: hv: Make sure the bus domain is really unique

[Sridhar Pitchai]

V2hlbiBMaW51eCBydW5zIGFzIGEgZ3Vlc3QgVk0gaW4gSHlwZXItViBhbmQgSHlwZXItViBhZGRz
IHRoZSB2aXJ0dWFsIFBDSQ0KYnVzIHRvIHRoZSBndWVzdCwgSHlwZXItViBhbHdheXMgcHJvdmlk
ZXMgdW5pcXVlIFBDSSBkb21haW4uDQoNCmNvbW1pdCA0YTliMDkzM2JkZmMgKCJQQ0k6IGh2OiBV
c2UgZGV2aWNlIHNlcmlhbCBudW1iZXIgYXMgUENJIGRvbWFpbiIpDQpvdmVycm9kZSB1bmlxdWUg
ZG9tYWluIHdpdGggdGhlIHNlcmlhbCBudW1iZXIgb2YgdGhlIGZpcnN0IGRldmljZSBhZGRlZCB0
bw0KdGhlIHZpcnR1YWwgUENJIGJ1cy4NCg0KVGhlIHJlYXNvbiBmb3IgdGhhdCBwYXRjaCB3YXMg
dG8gaGF2ZSBhIGNvbnNpc3RlbnQgYW5kIHNob3J0IG5hbWUgZm9yIHRoZQ0KZGV2aWNlLCBidXQg
SHlwZXItViBkb2Vzbid0IHByb3ZpZGUgdW5pcXVlIHNlcmlhbCBudW1iZXJzLiBVc2luZyBub24t
dW5pcXVlDQpzZXJpYWwgbnVtYmVycyBhcyBkb21haW4gSURzIGxlYWRzIHRvIGR1cGxpY2F0ZSBk
ZXZpY2UgYWRkcmVzc2VzLCB3aGljaA0KY2F1c2VzIFBDSSBidXMgcmVnaXN0cmF0aW9uIHRvIGZh
aWwuDQoNCmNvbW1pdCAwYzE5NTU2N2E4ZjYgKCJuZXR2c2M6IHRyYW5zcGFyZW50IFZGIG1hbmFn
ZW1lbnQiKSBhdm9pZHMgdGhlIG5lZWQNCmZvciBjb21taXQgNGE5YjA5MzNiZGZjICgiUENJOiBo
djogVXNlIGRldmljZSBzZXJpYWwgbnVtYmVyIGFzIFBDSQ0KZG9tYWluIikuICBXaGVuIHNjcmlw
dHMgd2VyZSB1c2VkIHRvIGNvbmZpZ3VyZSBWRiBkZXZpY2VzLCB0aGUgbmFtZSBvZg0KdGhlIFZG
IG5lZWRlZCB0byBiZSBjb25zaXN0ZW50IGFuZCBzaG9ydCwgYnV0IHdpdGggY29tbWl0IDBjMTk1
NTY3YThmNg0KKCJuZXR2c2M6IHRyYW5zcGFyZW50IFZGIG1hbmFnZW1lbnQiKSBhbGwgdGhlIHNl
dHVwIGlzIGRvbmUgaW4gdGhlIGtlcm5lbCwNCmFuZCB3ZSBkbyBub3QgbmVlZCB0byBtYWludGFp
biBjb25zaXN0ZW50IG5hbWUuDQoNClJldmVydCBjb21taXQgNGE5YjA5MzNiZGZjICgiUENJOiBo
djogVXNlIGRldmljZSBzZXJpYWwgbnVtYmVyIGFzIFBDSQ0KZG9tYWluIikgc28gd2UgY2FuIHJl
bGlhYmx5IHN1cHBvcnQgbXVsdGlwbGUgZGV2aWNlcyBiZWluZyBhc3NpZ25lZCB0bw0KYSBndWVz
dC4NCg0KVGhpcyByZXZlcnQgc2hvdWxkIG9ubHkgYmUgYmFja3BvcnRlZCB0byBrZXJuZWxzIHRo
YXQgY29udGFpbiBjb21taXQNCjBjMTk1NTY3YThmNiAoIm5ldHZzYzogdHJhbnNwYXJlbnQgVkYg
bWFuYWdlbWVudCIpLg0KDQpGaXhlczogNGE5YjA5MzNiZGZjICgiUENJOiBodjogVXNlIGRldmlj
ZSBzZXJpYWwgbnVtYmVyIGFzIFBDSSBkb21haW4iKQ0KU2lnbmVkLW9mZi1ieTogU3JpZGhhciBQ
aXRjaGFpIDxzcmlkaGFyLnBpdGNoYWlAbWljcm9zb2Z0LmNvbT4NCkNjOiBzdGFibGVAdmdlci5r
ZXJuZWwub3JnICMgdjQuMTQrDQpSZXZpZXdlZC1ieTogQmpvcm4gSGVsZ2FhcyA8YmhlbGdhYXNA
Z29vZ2xlLmNvbT4NCg0KLS0tDQpDaGFuZ2VzIGluIHY4Og0KKiBmaXggdGhlIGNvbW1pdCBjb21t
ZW50LiBbTG9yZW56byBQaWVyYWxpc2ksIEJqb3JuIEhlbGdhYXNdDQotLS0NCiBkcml2ZXJzL3Bj
aS9ob3N0L3BjaS1oeXBlcnYuYyB8IDExIC0tLS0tLS0tLS0tDQogMSBmaWxlIGNoYW5nZWQsIDEx
IGRlbGV0aW9ucygtKQ0KDQpkaWZmIC0tZ2l0IGEvZHJpdmVycy9wY2kvaG9zdC9wY2ktaHlwZXJ2
LmMgYi9kcml2ZXJzL3BjaS9ob3N0L3BjaS1oeXBlcnYuYw0KaW5kZXggMmZhZjM4ZWFiNzg1Li5h
YzY3ZTU2ZTQ1MWEgMTAwNjQ0DQotLS0gYS9kcml2ZXJzL3BjaS9ob3N0L3BjaS1oeXBlcnYuYw0K
KysrIGIvZHJpdmVycy9wY2kvaG9zdC9wY2ktaHlwZXJ2LmMNCkBAIC0xNTE4LDE3ICsxNTE4LDYg
QEAgc3RhdGljIHN0cnVjdCBodl9wY2lfZGV2ICpuZXdfcGNpY2hpbGRfZGV2aWNlKHN0cnVjdCBo
dl9wY2lidXNfZGV2aWNlICpoYnVzLA0KIAlnZXRfcGNpY2hpbGQoaHBkZXYsIGh2X3BjaWRldl9y
ZWZfY2hpbGRsaXN0KTsNCiAJc3Bpbl9sb2NrX2lycXNhdmUoJmhidXMtPmRldmljZV9saXN0X2xv
Y2ssIGZsYWdzKTsNCiANCi0JLyoNCi0JICogV2hlbiBhIGRldmljZSBpcyBiZWluZyBhZGRlZCB0
byB0aGUgYnVzLCB3ZSBzZXQgdGhlIFBDSSBkb21haW4NCi0JICogbnVtYmVyIHRvIGJlIHRoZSBk
ZXZpY2Ugc2VyaWFsIG51bWJlciwgd2hpY2ggaXMgbm9uLXplcm8gYW5kDQotCSAqIHVuaXF1ZSBv
biB0aGUgc2FtZSBWTS4gIFRoZSBzZXJpYWwgbnVtYmVycyBzdGFydCB3aXRoIDEsIGFuZA0KLQkg
KiBpbmNyZWFzZSBieSAxIGZvciBlYWNoIGRldmljZS4gIFNvIGRldmljZSBuYW1lcyBpbmNsdWRp
bmcgdGhpcw0KLQkgKiBjYW4gaGF2ZSBzaG9ydGVyIG5hbWVzIHRoYW4gYmFzZWQgb24gdGhlIGJ1
cyBpbnN0YW5jZSBVVUlELg0KLQkgKiBPbmx5IHRoZSBmaXJzdCBkZXZpY2Ugc2VyaWFsIG51bWJl
ciBpcyB1c2VkIGZvciBkb21haW4sIHNvIHRoZQ0KLQkgKiBkb21haW4gbnVtYmVyIHdpbGwgbm90
IGNoYW5nZSBhZnRlciB0aGUgZmlyc3QgZGV2aWNlIGlzIGFkZGVkLg0KLQkgKi8NCi0JaWYgKGxp
c3RfZW1wdHkoJmhidXMtPmNoaWxkcmVuKSkNCi0JCWhidXMtPnN5c2RhdGEuZG9tYWluID0gZGVz
Yy0+c2VyOw0KIAlsaXN0X2FkZF90YWlsKCZocGRldi0+bGlzdF9lbnRyeSwgJmhidXMtPmNoaWxk
cmVuKTsNCiAJc3Bpbl91bmxvY2tfaXJxcmVzdG9yZSgmaGJ1cy0+ZGV2aWNlX2xpc3RfbG9jaywg
ZmxhZ3MpOw0KIAlyZXR1cm4gaHBkZXY7DQotLSANCjIuMTQuMQ0KDQoNCg==

Re: [PATCH v8] PCI: hv: Make sure the bus domain is really unique

[Lorenzo Pieralisi]

On Tue, May 01, 2018 at 05:56:32PM +0000, Sridhar Pitchai wrote:
> When Linux runs as a guest VM in Hyper-V and Hyper-V adds the virtual PCI
> bus to the guest, Hyper-V always provides unique PCI domain.
> 
> commit 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI domain")
> overrode unique domain with the serial number of the first device added to
> the virtual PCI bus.
> 
> The reason for that patch was to have a consistent and short name for the
> device, but Hyper-V doesn't provide unique serial numbers. Using non-unique
> serial numbers as domain IDs leads to duplicate device addresses, which
> causes PCI bus registration to fail.
> 
> commit 0c195567a8f6 ("netvsc: transparent VF management") avoids the need
> for commit 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI
> domain").  When scripts were used to configure VF devices, the name of
> the VF needed to be consistent and short, but with commit 0c195567a8f6
> ("netvsc: transparent VF management") all the setup is done in the kernel,
> and we do not need to maintain consistent name.
> 
> Revert commit 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI
> domain") so we can reliably support multiple devices being assigned to
> a guest.
> 
> This revert should only be backported to kernels that contain commit
> 0c195567a8f6 ("netvsc: transparent VF management").

I removed this last paragraph, the information is in the stable tag
below.

> Fixes: 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI domain")
> Signed-off-by: Sridhar Pitchai <sridhar.pitchai@microsoft.com>
> Cc: stable@vger.kernel.org # v4.14+
> Reviewed-by: Bjorn Helgaas <bhelgaas@google.com>
> 
> ---
> Changes in v8:
> * fix the commit comment. [Lorenzo Pieralisi, Bjorn Helgaas]
> ---
>  drivers/pci/host/pci-hyperv.c | 11 -----------
>  1 file changed, 11 deletions(-)

Applied to pci/hv for v4.18, thanks.

Lorenzo

> diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
> index 2faf38eab785..ac67e56e451a 100644
> --- a/drivers/pci/host/pci-hyperv.c
> +++ b/drivers/pci/host/pci-hyperv.c
> @@ -1518,17 +1518,6 @@ static struct hv_pci_dev *new_pcichild_device(struct hv_pcibus_device *hbus,
>  	get_pcichild(hpdev, hv_pcidev_ref_childlist);
>  	spin_lock_irqsave(&hbus->device_list_lock, flags);
>  
> -	/*
> -	 * When a device is being added to the bus, we set the PCI domain
> -	 * number to be the device serial number, which is non-zero and
> -	 * unique on the same VM.  The serial numbers start with 1, and
> -	 * increase by 1 for each device.  So device names including this
> -	 * 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.
> -	 */
> -	if (list_empty(&hbus->children))
> -		hbus->sysdata.domain = desc->ser;
>  	list_add_tail(&hpdev->list_entry, &hbus->children);
>  	spin_unlock_irqrestore(&hbus->device_list_lock, flags);
>  	return hpdev;
> -- 
> 2.14.1
> 
> 
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v8] PCI: hv: Make sure the bus domain is really unique

[Lorenzo Pieralisi]

On Tue, May 01, 2018 at 05:56:32PM +0000, Sridhar Pitchai wrote:
> When Linux runs as a guest VM in Hyper-V and Hyper-V adds the virtual PCI
> bus to the guest, Hyper-V always provides unique PCI domain.
> 
> commit 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI domain")
> overrode unique domain with the serial number of the first device added to
> the virtual PCI bus.
> 
> The reason for that patch was to have a consistent and short name for the
> device, but Hyper-V doesn't provide unique serial numbers. Using non-unique
> serial numbers as domain IDs leads to duplicate device addresses, which
> causes PCI bus registration to fail.
> 
> commit 0c195567a8f6 ("netvsc: transparent VF management") avoids the need
> for commit 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI
> domain").  When scripts were used to configure VF devices, the name of
> the VF needed to be consistent and short, but with commit 0c195567a8f6
> ("netvsc: transparent VF management") all the setup is done in the kernel,
> and we do not need to maintain consistent name.
> 
> Revert commit 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI
> domain") so we can reliably support multiple devices being assigned to
> a guest.
> 
> This revert should only be backported to kernels that contain commit
> 0c195567a8f6 ("netvsc: transparent VF management").

I removed this last paragraph, the information is in the stable tag
below.

> Fixes: 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI domain")
> Signed-off-by: Sridhar Pitchai <sridhar.pitchai@microsoft.com>
> Cc: stable@vger.kernel.org # v4.14+
> Reviewed-by: Bjorn Helgaas <bhelgaas@google.com>
> 
> ---
> Changes in v8:
> * fix the commit comment. [Lorenzo Pieralisi, Bjorn Helgaas]
> ---
>  drivers/pci/host/pci-hyperv.c | 11 -----------
>  1 file changed, 11 deletions(-)

Applied to pci/hv for v4.18, thanks.

Lorenzo

> diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
> index 2faf38eab785..ac67e56e451a 100644
> --- a/drivers/pci/host/pci-hyperv.c
> +++ b/drivers/pci/host/pci-hyperv.c
> @@ -1518,17 +1518,6 @@ static struct hv_pci_dev *new_pcichild_device(struct hv_pcibus_device *hbus,
>  	get_pcichild(hpdev, hv_pcidev_ref_childlist);
>  	spin_lock_irqsave(&hbus->device_list_lock, flags);
>  
> -	/*
> -	 * When a device is being added to the bus, we set the PCI domain
> -	 * number to be the device serial number, which is non-zero and
> -	 * unique on the same VM.  The serial numbers start with 1, and
> -	 * increase by 1 for each device.  So device names including this
> -	 * 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.
> -	 */
> -	if (list_empty(&hbus->children))
> -		hbus->sysdata.domain = desc->ser;
>  	list_add_tail(&hpdev->list_entry, &hbus->children);
>  	spin_unlock_irqrestore(&hbus->device_list_lock, flags);
>  	return hpdev;
> -- 
> 2.14.1
> 
>