* [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
@ 2018-03-15 0:03 ` Sridhar Pitchai
0 siblings, 0 replies; 26+ messages in thread
From: Sridhar Pitchai @ 2018-03-15 0:03 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Stephen Hemminger, linux-pci, Haiyang Zhang, linux-kernel,
Michael Kelley (EOSG),
Jake Oshins, Bjorn Helgaas, devel
Whenever PCI bus is added, HyperV guarantees the BUS id is unique. Even
with that when a first device is added to the bus, it overrides bus domain
ID with the device serial number. Sometime this can result in BUS ID not
being unique. In this case, when PCI_BUS and a device to bus is added, the
first device overwrites the bus domain ID to the device serial number,
which is 0. Since there exsist a PCI bus with domain ID 0 already the PCI
bus addition fails. This patch make sure when a device is added to a bus,
it never updated the bus domain ID. Since we have the transparent SRIOV
mode now, the short VF device name is no longer needed.
Fixes: 4a9b0933bdfc("PCI:hv:Use device serial number as PCI domain")
Cc: stable@vger.kernel.org
Signed-off-by: Sridhar Pitchai <srpitcha@microsoft.com>
---
Changes in v3:
* fix the commit comment. [KY Srinivasan, Michael Kelley]
---
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 2faf38e..ac67e56 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.7.4
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
@ 2018-03-15 0:03 ` Sridhar Pitchai
0 siblings, 0 replies; 26+ messages in thread
From: Sridhar Pitchai @ 2018-03-15 0:03 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Bjorn Helgaas, Jake Oshins, Haiyang Zhang, Stephen Hemminger,
Dexuan Cui, KY Srinivasan, Michael Kelley (EOSG),
devel, linux-pci, linux-kernel
V2hlbmV2ZXIgUENJIGJ1cyBpcyBhZGRlZCwgSHlwZXJWIGd1YXJhbnRlZXMgdGhlIEJVUyBpZCBp
cyB1bmlxdWUuIEV2ZW4NCndpdGggdGhhdCB3aGVuIGEgZmlyc3QgZGV2aWNlIGlzIGFkZGVkIHRv
IHRoZSBidXMsIGl0IG92ZXJyaWRlcyBidXMgZG9tYWluDQpJRCB3aXRoIHRoZSBkZXZpY2Ugc2Vy
aWFsIG51bWJlci4gU29tZXRpbWUgdGhpcyBjYW4gcmVzdWx0IGluIEJVUyBJRCBub3QNCmJlaW5n
IHVuaXF1ZS4gSW4gdGhpcyBjYXNlLCB3aGVuIFBDSV9CVVMgYW5kIGEgZGV2aWNlIHRvIGJ1cyBp
cyBhZGRlZCwgdGhlDQpmaXJzdCBkZXZpY2Ugb3ZlcndyaXRlcyB0aGUgYnVzIGRvbWFpbiBJRCB0
byB0aGUgZGV2aWNlIHNlcmlhbCBudW1iZXIsDQp3aGljaCBpcyAwLiBTaW5jZSB0aGVyZSBleHNp
c3QgYSBQQ0kgYnVzIHdpdGggZG9tYWluIElEIDAgYWxyZWFkeSB0aGUgUENJDQpidXMgYWRkaXRp
b24gZmFpbHMuIFRoaXMgcGF0Y2ggbWFrZSBzdXJlIHdoZW4gYSBkZXZpY2UgaXMgYWRkZWQgdG8g
YSBidXMsDQppdCBuZXZlciB1cGRhdGVkIHRoZSBidXMgZG9tYWluIElELiBTaW5jZSB3ZSBoYXZl
IHRoZSB0cmFuc3BhcmVudCBTUklPVg0KbW9kZSBub3csIHRoZSBzaG9ydCBWRiBkZXZpY2UgbmFt
ZSBpcyBubyBsb25nZXIgbmVlZGVkLg0KDQpGaXhlczogNGE5YjA5MzNiZGZjKCJQQ0k6aHY6VXNl
IGRldmljZSBzZXJpYWwgbnVtYmVyIGFzIFBDSSBkb21haW4iKQ0KQ2M6IHN0YWJsZUB2Z2VyLmtl
cm5lbC5vcmcNClNpZ25lZC1vZmYtYnk6IFNyaWRoYXIgUGl0Y2hhaSA8c3JwaXRjaGFAbWljcm9z
b2Z0LmNvbT4NCi0tLQ0KDQpDaGFuZ2VzIGluIHYzOg0KKiBmaXggdGhlIGNvbW1pdCBjb21tZW50
LiBbS1kgU3Jpbml2YXNhbiwgTWljaGFlbCBLZWxsZXldDQotLS0NCiBkcml2ZXJzL3BjaS9ob3N0
L3BjaS1oeXBlcnYuYyB8IDExIC0tLS0tLS0tLS0tDQogMSBmaWxlIGNoYW5nZWQsIDExIGRlbGV0
aW9ucygtKQ0KDQpkaWZmIC0tZ2l0IGEvZHJpdmVycy9wY2kvaG9zdC9wY2ktaHlwZXJ2LmMgYi9k
cml2ZXJzL3BjaS9ob3N0L3BjaS1oeXBlcnYuYw0KaW5kZXggMmZhZjM4ZS4uYWM2N2U1NiAxMDA2
NDQNCi0tLSBhL2RyaXZlcnMvcGNpL2hvc3QvcGNpLWh5cGVydi5jDQorKysgYi9kcml2ZXJzL3Bj
aS9ob3N0L3BjaS1oeXBlcnYuYw0KQEAgLTE1MTgsMTcgKzE1MTgsNiBAQCBzdGF0aWMgc3RydWN0
IGh2X3BjaV9kZXYgKm5ld19wY2ljaGlsZF9kZXZpY2Uoc3RydWN0IGh2X3BjaWJ1c19kZXZpY2Ug
KmhidXMsDQogCWdldF9wY2ljaGlsZChocGRldiwgaHZfcGNpZGV2X3JlZl9jaGlsZGxpc3QpOw0K
IAlzcGluX2xvY2tfaXJxc2F2ZSgmaGJ1cy0+ZGV2aWNlX2xpc3RfbG9jaywgZmxhZ3MpOw0KIA0K
LQkvKg0KLQkgKiBXaGVuIGEgZGV2aWNlIGlzIGJlaW5nIGFkZGVkIHRvIHRoZSBidXMsIHdlIHNl
dCB0aGUgUENJIGRvbWFpbg0KLQkgKiBudW1iZXIgdG8gYmUgdGhlIGRldmljZSBzZXJpYWwgbnVt
YmVyLCB3aGljaCBpcyBub24temVybyBhbmQNCi0JICogdW5pcXVlIG9uIHRoZSBzYW1lIFZNLiAg
VGhlIHNlcmlhbCBudW1iZXJzIHN0YXJ0IHdpdGggMSwgYW5kDQotCSAqIGluY3JlYXNlIGJ5IDEg
Zm9yIGVhY2ggZGV2aWNlLiAgU28gZGV2aWNlIG5hbWVzIGluY2x1ZGluZyB0aGlzDQotCSAqIGNh
biBoYXZlIHNob3J0ZXIgbmFtZXMgdGhhbiBiYXNlZCBvbiB0aGUgYnVzIGluc3RhbmNlIFVVSUQu
DQotCSAqIE9ubHkgdGhlIGZpcnN0IGRldmljZSBzZXJpYWwgbnVtYmVyIGlzIHVzZWQgZm9yIGRv
bWFpbiwgc28gdGhlDQotCSAqIGRvbWFpbiBudW1iZXIgd2lsbCBub3QgY2hhbmdlIGFmdGVyIHRo
ZSBmaXJzdCBkZXZpY2UgaXMgYWRkZWQuDQotCSAqLw0KLQlpZiAobGlzdF9lbXB0eSgmaGJ1cy0+
Y2hpbGRyZW4pKQ0KLQkJaGJ1cy0+c3lzZGF0YS5kb21haW4gPSBkZXNjLT5zZXI7DQogCWxpc3Rf
YWRkX3RhaWwoJmhwZGV2LT5saXN0X2VudHJ5LCAmaGJ1cy0+Y2hpbGRyZW4pOw0KIAlzcGluX3Vu
bG9ja19pcnFyZXN0b3JlKCZoYnVzLT5kZXZpY2VfbGlzdF9sb2NrLCBmbGFncyk7DQogCXJldHVy
biBocGRldjsNCi0tIA0KMi43LjQgDQoNCg==
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
2018-03-15 0:03 ` Sridhar Pitchai
@ 2018-03-15 12:05 ` Lorenzo Pieralisi
-1 siblings, 0 replies; 26+ messages in thread
From: Lorenzo Pieralisi @ 2018-03-15 12:05 UTC (permalink / raw)
To: Sridhar Pitchai
Cc: Stephen Hemminger, linux-pci, Haiyang Zhang, linux-kernel,
Michael Kelley (EOSG),
Jake Oshins, Bjorn Helgaas, devel
On Thu, Mar 15, 2018 at 12:03:07AM +0000, Sridhar Pitchai wrote:
> Whenever PCI bus is added, HyperV guarantees the BUS id is unique. Even
"Whenever a PCI bus is added"
> with that when a first device is added to the bus, it overrides bus domain
> ID with the device serial number. Sometime this can result in BUS ID not
Define "Sometime".
> being unique. In this case, when PCI_BUS and a device to bus is added, the
> first device overwrites the bus domain ID to the device serial number,
> which is 0. Since there exsist a PCI bus with domain ID 0 already the PCI
s/exsist/exist
> bus addition fails. This patch make sure when a device is added to a bus,
> it never updated the bus domain ID.
s/updated/updates
> Since we have the transparent SRIOV mode now, the short VF device name
> is no longer needed.
I still do not understand what this means and how it is related to the
patch below, it may be clear to you, it is not to me, at all.
> Fixes: 4a9b0933bdfc("PCI:hv:Use device serial number as PCI domain")
Fixes: 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI domain")
I asked you an explicit question. Commit above was added for a reason
I assume. This patch implies that kernel has been broken since v4.11
which is almost a year ago and nobody every noticed ? Or there are
systems where commit above is _necessary_ and this patch would break
them ?
I want a detailed explanation that highlights *why* it is safe to apply
this patch and send it to stable kernels, commit log above won't do.
Thanks,
Lorenzo
> Cc: stable@vger.kernel.org
> Signed-off-by: Sridhar Pitchai <srpitcha@microsoft.com>
> ---
>
> Changes in v3:
> * fix the commit comment. [KY Srinivasan, Michael Kelley]
> ---
> 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 2faf38e..ac67e56 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.7.4
>
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
@ 2018-03-15 12:05 ` Lorenzo Pieralisi
0 siblings, 0 replies; 26+ messages in thread
From: Lorenzo Pieralisi @ 2018-03-15 12:05 UTC (permalink / raw)
To: Sridhar Pitchai
Cc: Bjorn Helgaas, Jake Oshins, Haiyang Zhang, Stephen Hemminger,
Dexuan Cui, KY Srinivasan, Michael Kelley (EOSG),
devel, linux-pci, linux-kernel
On Thu, Mar 15, 2018 at 12:03:07AM +0000, Sridhar Pitchai wrote:
> Whenever PCI bus is added, HyperV guarantees the BUS id is unique. Even
"Whenever a PCI bus is added"
> with that when a first device is added to the bus, it overrides bus domain
> ID with the device serial number. Sometime this can result in BUS ID not
Define "Sometime".
> being unique. In this case, when PCI_BUS and a device to bus is added, the
> first device overwrites the bus domain ID to the device serial number,
> which is 0. Since there exsist a PCI bus with domain ID 0 already the PCI
s/exsist/exist
> bus addition fails. This patch make sure when a device is added to a bus,
> it never updated the bus domain ID.
s/updated/updates
> Since we have the transparent SRIOV mode now, the short VF device name
> is no longer needed.
I still do not understand what this means and how it is related to the
patch below, it may be clear to you, it is not to me, at all.
> Fixes: 4a9b0933bdfc("PCI:hv:Use device serial number as PCI domain")
Fixes: 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI domain")
I asked you an explicit question. Commit above was added for a reason
I assume. This patch implies that kernel has been broken since v4.11
which is almost a year ago and nobody every noticed ? Or there are
systems where commit above is _necessary_ and this patch would break
them ?
I want a detailed explanation that highlights *why* it is safe to apply
this patch and send it to stable kernels, commit log above won't do.
Thanks,
Lorenzo
> Cc: stable@vger.kernel.org
> Signed-off-by: Sridhar Pitchai <srpitcha@microsoft.com>
> ---
>
> Changes in v3:
> * fix the commit comment. [KY Srinivasan, Michael Kelley]
> ---
> 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 2faf38e..ac67e56 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.7.4
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
2018-03-15 12:05 ` Lorenzo Pieralisi
@ 2018-03-15 17:56 ` Sridhar Pitchai
-1 siblings, 0 replies; 26+ messages in thread
From: Sridhar Pitchai @ 2018-03-15 17:56 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Stephen Hemminger, linux-pci, Haiyang Zhang, linux-kernel,
Michael Kelley (EOSG),
Jake Oshins, Bjorn Helgaas, devel
Hi Lorenzo,
Answering the question inline.
Kindly let me know if it clarifies. I will send out another patch after we agree on the clarification.
Thanks
Sridhar Pitchai
-----Original Message-----
From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Sent: Thursday, March 15, 2018 5:05 AM
To: Sridhar Pitchai <Sridhar.Pitchai@microsoft.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>; Jake Oshins <jakeo@microsoft.com>; Haiyang Zhang <haiyangz@microsoft.com>; Stephen Hemminger <sthemmin@microsoft.com>; Dexuan Cui <decui@microsoft.com>; KY Srinivasan <kys@microsoft.com>; Michael Kelley (EOSG) <Michael.H.Kelley@microsoft.com>; devel@linuxdriverproject.org; linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
On Thu, Mar 15, 2018 at 12:03:07AM +0000, Sridhar Pitchai wrote:
> Whenever PCI bus is added, HyperV guarantees the BUS id is unique. Even
"Whenever a PCI bus is added"
Sridhar>> yes
> with that when a first device is added to the bus, it overrides bus domain
> ID with the device serial number. Sometime this can result in BUS ID not
Define "Sometime".
Sridhar>> HyperV when it creates a PCI bus it guarantees it provide a unique ID for it. But, that unique BUS ID is replaced with device serial number. 0 is a valid device serial number, and if there exists a PCI bus with domain ID 0 (Gen 1 version of hyperV VM have this for para virtual devices), this will result in PCI bus id not being unique.
> being unique. In this case, when PCI_BUS and a device to bus is added, the
> first device overwrites the bus domain ID to the device serial number,
> which is 0. Since there exsist a PCI bus with domain ID 0 already the PCI
s/exsist/exist
Sridhar>> yes
> bus addition fails. This patch make sure when a device is added to a bus,
> it never updated the bus domain ID.
s/updated/updates
Sridhar >> yes
> Since we have the transparent SRIOV mode now, the short VF device name
> is no longer needed.
I still do not understand what this means and how it is related to the
patch below, it may be clear to you, it is not to me, at all.
Sridhar >> the patch below, was introduced to make the device name small, by taking only 16bits of the serial number. Since we are not going to have the serial number updated to the BUS id, this has to be removed.
> Fixes: 4a9b0933bdfc("PCI:hv:Use device serial number as PCI domain")
Fixes: 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI domain")
Sridhr >> yes
I asked you an explicit question. Commit above was added for a reason
I assume. This patch implies that kernel has been broken since v4.11
which is almost a year ago and nobody every noticed ? Or there are
systems where commit above is _necessary_ and this patch would break
them ?
I want a detailed explanation that highlights *why* it is safe to apply
this patch and send it to stable kernels, commit log above won't do.
Sridhar>> HyperV provides a unique domain ID for PCI BUS. But it is modified by the child device when it is added. This cannot produce a unique domain ID all the time. Here in the bug, we see the collision between the serial number and already existing PCI bus. The cleaner way is never touch the domain ID provided by hyperV during the PCI bus creation. As long as hyperV make sure it provides a unique domain ID for the PCI for a VM it will not break, and HyperV will guarantees that the domain for the PCI bus for a given VM will be always unique.
The original patch was also intending to have a unique domain ID for the PCI bus, by taking the serial number of the device, but it is not sufficient, when the device serial number is number which is the domain ID of the existing PCI bus.
With the current kernel we can repro this issue by adding a device with a serial number matching the existing PCI bus domain id. (in this case that happens to be zero).
Thanks,
Lorenzo
> Cc: stable@vger.kernel.org
> Signed-off-by: Sridhar Pitchai <srpitcha@microsoft.com>
> ---
>
> Changes in v3:
> * fix the commit comment. [KY Srinivasan, Michael Kelley]
> ---
> 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 2faf38e..ac67e56 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.7.4
>
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
@ 2018-03-15 17:56 ` Sridhar Pitchai
0 siblings, 0 replies; 26+ messages in thread
From: Sridhar Pitchai @ 2018-03-15 17:56 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Bjorn Helgaas, Jake Oshins, Haiyang Zhang, Stephen Hemminger,
Dexuan Cui, KY Srinivasan, Michael Kelley (EOSG),
devel, linux-pci, linux-kernel
Hi Lorenzo,
Answering the question inline.
Kindly let me know if it clarifies. I will send out another patch after we =
agree on the clarification.
Thanks
Sridhar Pitchai
-----Original Message-----
From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>=20
Sent: Thursday, March 15, 2018 5:05 AM
To: Sridhar Pitchai <Sridhar.Pitchai@microsoft.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>; Jake Oshins <jakeo@microsoft.com>;=
Haiyang Zhang <haiyangz@microsoft.com>; Stephen Hemminger <sthemmin@micros=
oft.com>; Dexuan Cui <decui@microsoft.com>; KY Srinivasan <kys@microsoft.co=
m>; Michael Kelley (EOSG) <Michael.H.Kelley@microsoft.com>; devel@linuxdriv=
erproject.org; linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
On Thu, Mar 15, 2018 at 12:03:07AM +0000, Sridhar Pitchai wrote:
> Whenever PCI bus is added, HyperV guarantees the BUS id is unique. Even
"Whenever a PCI bus is added"
Sridhar>> yes
> with that when a first device is added to the bus, it overrides bus domai=
n
> ID with the device serial number. Sometime this can result in BUS ID not
Define "Sometime".
Sridhar>> HyperV when it creates a PCI bus it guarantees it provide a uniqu=
e ID for it. But, that unique BUS ID is replaced with device serial number.=
0 is a valid device serial number, and if there exists a PCI bus with doma=
in ID 0 (Gen 1 version of hyperV VM have this for para virtual devices), th=
is will result in PCI bus id not being unique.=20
> being unique. In this case, when PCI_BUS and a device to bus is added, th=
e
> first device overwrites the bus domain ID to the device serial number,
> which is 0. Since there exsist a PCI bus with domain ID 0 already the PCI
s/exsist/exist
Sridhar>> yes
> bus addition fails. This patch make sure when a device is added to a bus,
> it never updated the bus domain ID.=20
s/updated/updates
Sridhar >> yes
> Since we have the transparent SRIOV mode now, the short VF device name
> is no longer needed.
I still do not understand what this means and how it is related to the
patch below, it may be clear to you, it is not to me, at all.
Sridhar >> the patch below, was introduced to make the device name small, b=
y taking only 16bits of the serial number. Since we are not going to have t=
he serial number updated to the BUS id, this has to be removed.
> Fixes: 4a9b0933bdfc("PCI:hv:Use device serial number as PCI domain")
Fixes: 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI domain")
Sridhr >> yes
I asked you an explicit question. Commit above was added for a reason
I assume. This patch implies that kernel has been broken since v4.11
which is almost a year ago and nobody every noticed ? Or there are
systems where commit above is _necessary_ and this patch would break
them ?
I want a detailed explanation that highlights *why* it is safe to apply
this patch and send it to stable kernels, commit log above won't do.
Sridhar>> HyperV provides a unique domain ID for PCI BUS. But it is modifie=
d by the child device when it is added. This cannot produce a unique domain=
ID all the time. Here in the bug, we see the collision between the serial =
number and already existing PCI bus. The cleaner way is never touch the dom=
ain ID provided by hyperV during the PCI bus creation. As long as hyperV ma=
ke sure it provides a unique domain ID for the PCI for a VM it will not bre=
ak, and HyperV will guarantees that the domain for the PCI bus for a given =
VM will be always unique.
The original patch was also intending to have a unique domain ID for the PC=
I bus, by taking the serial number of the device, but it is not sufficient,=
when the device serial number is number which is the domain ID of the exis=
ting PCI bus.
With the current kernel we can repro this issue by adding a device with a s=
erial number matching the existing PCI bus domain id. (in this case that ha=
ppens to be zero).
Thanks,
Lorenzo
> Cc: stable@vger.kernel.org
> Signed-off-by: Sridhar Pitchai <srpitcha@microsoft.com>
> ---
>=20
> Changes in v3:
> * fix the commit comment. [KY Srinivasan, Michael Kelley]
> ---
> drivers/pci/host/pci-hyperv.c | 11 -----------
> 1 file changed, 11 deletions(-)
>=20
> diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.=
c
> index 2faf38e..ac67e56 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(stru=
ct hv_pcibus_device *hbus,
> get_pcichild(hpdev, hv_pcidev_ref_childlist);
> spin_lock_irqsave(&hbus->device_list_lock, flags);
> =20
> - /*
> - * 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 =3D desc->ser;
> list_add_tail(&hpdev->list_entry, &hbus->children);
> spin_unlock_irqrestore(&hbus->device_list_lock, flags);
> return hpdev;
> --=20
> 2.7.4=20
>=20
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
2018-03-15 17:56 ` Sridhar Pitchai
@ 2018-03-15 18:24 ` Sridhar Pitchai
-1 siblings, 0 replies; 26+ messages in thread
From: Sridhar Pitchai @ 2018-03-15 18:24 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Stephen Hemminger, linux-pci, Haiyang Zhang, linux-kernel,
Michael Kelley (EOSG),
Jake Oshins, Bjorn Helgaas, devel
Apologies for not aligning my reply, I just realized after looking into
the mail archive in LKML. I have aligned the replay now.
On 3/15/18, 10:56 AM, "Sridhar Pitchai" <Sridhar.Pitchai@microsoft.com> wrote:
Hi Lorenzo,
Answering the question inline.
Kindly let me know if it clarifies. I will send out another patch after we agree on the clarification.
Thanks
Sridhar Pitchai
-----Original Message-----
From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Sent: Thursday, March 15, 2018 5:05 AM
To: Sridhar Pitchai <Sridhar.Pitchai@microsoft.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>; Jake Oshins <jakeo@microsoft.com>; Haiyang Zhang <haiyangz@microsoft.com>; Stephen Hemminger <sthemmin@microsoft.com>; Dexuan Cui <decui@microsoft.com>; KY Srinivasan <kys@microsoft.com>; Michael Kelley (EOSG) <Michael.H.Kelley@microsoft.com>; devel@linuxdriverproject.org; linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
On Thu, Mar 15, 2018 at 12:03:07AM +0000, Sridhar Pitchai wrote:
Whenever PCI bus is added, HyperV guarantees the BUS id is unique. Even
"Whenever a PCI bus is added"
Sridhar>> yes
with that when a first device is added to the bus, it overrides bus domain
ID with the device serial number. Sometime this can result in BUS ID not
Define "Sometime".
Sridhar>> HyperV when it creates a PCI bus it guarantees it provide a unique ID for it.
But, that unique BUS ID is replaced with device serial number. 0 is a valid
device serial number, and if there exists a PCI bus with domain ID 0 (Gen 1
version of hyperV VM have this for para virtual devices), this will result in
PCI bus id not being unique.
being unique. In this case, when PCI_BUS and a device to bus is added, the
first device overwrites the bus domain ID to the device serial number,
which is 0. Since there exsist a PCI bus with domain ID 0 already the PCI
s/exsist/exist
Sridhar>> yes
bus addition fails. This patch make sure when a device is added to a bus,
it never updated the bus domain ID.
s/updated/updates
Sridhar >> yes
Since we have the transparent SRIOV mode now, the short VF device name
is no longer needed.
I still do not understand what this means and how it is related to the
patch below, it may be clear to you, it is not to me, at all.
Sridhar >> the patch below, was introduced to make the device name small, by taking only
16bits of the serial number. Since we are not going to have the serial number
updated to the BUS id, this has to be removed.
Fixes: 4a9b0933bdfc("PCI:hv:Use device serial number as PCI domain")
Fixes: 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI domain")
Sridhr >> yes
I asked you an explicit question. Commit above was added for a reason
I assume. This patch implies that kernel has been broken since v4.11
which is almost a year ago and nobody every noticed ? Or there are
systems where commit above is _necessary_ and this patch would break
them ?
I want a detailed explanation that highlights *why* it is safe to apply
this patch and send it to stable kernels, commit log above won't do.
Sridhar>> HyperV provides a unique domain ID for PCI BUS. But it is modified by the child
device when it is added. This cannot produce a unique domain ID all the time.
Here in the bug, we see the collision between the serial number and already
existing PCI bus. The cleaner way is never touch the domain ID provided by
hyperV during the PCI bus creation. As long as hyperV make sure it provides a
unique domain ID for the PCI for a VM it will not break, and HyperV will
guarantees that the domain for the PCI bus for a given VM will be always unique.
The original patch was also intending to have a unique domain ID for the PCI
bus, by taking the serial number of the device, but it is not sufficient, when
the device serial number is number which is the domain ID of the existing PCI
bus. With the current kernel we can repro this issue by adding a device with a
serial number matching the existing PCI bus domain id. (in this case that
happens to be zero).
Thanks,
Lorenzo
Cc: stable@vger.kernel.org
Signed-off-by: Sridhar Pitchai <srpitcha@microsoft.com>
---
Changes in v3:
* fix the commit comment. [KY Srinivasan, Michael Kelley]
---
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 2faf38e..ac67e56 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.7.4
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
@ 2018-03-15 18:24 ` Sridhar Pitchai
0 siblings, 0 replies; 26+ messages in thread
From: Sridhar Pitchai @ 2018-03-15 18:24 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Bjorn Helgaas, Jake Oshins, Haiyang Zhang, Stephen Hemminger,
Dexuan Cui, KY Srinivasan, Michael Kelley (EOSG),
devel, linux-pci, linux-kernel
QXBvbG9naWVzIGZvciBub3QgYWxpZ25pbmcgbXkgcmVwbHksIEkganVzdCByZWFsaXplZCBhZnRl
ciBsb29raW5nIGludG8NCnRoZSBtYWlsIGFyY2hpdmUgaW4gTEtNTC4gIEkgaGF2ZSBhbGlnbmVk
IHRoZSByZXBsYXkgbm93Lg0KDQoNCk9uIDMvMTUvMTgsIDEwOjU2IEFNLCAiU3JpZGhhciBQaXRj
aGFpIiA8U3JpZGhhci5QaXRjaGFpQG1pY3Jvc29mdC5jb20+IHdyb3RlOg0KDQpIaSBMb3Jlbnpv
LA0KQW5zd2VyaW5nIHRoZSBxdWVzdGlvbiBpbmxpbmUuDQpLaW5kbHkgbGV0IG1lIGtub3cgaWYg
aXQgY2xhcmlmaWVzLiBJIHdpbGwgc2VuZCBvdXQgYW5vdGhlciBwYXRjaCBhZnRlciB3ZSBhZ3Jl
ZSBvbiB0aGUgY2xhcmlmaWNhdGlvbi4NCg0KVGhhbmtzDQpTcmlkaGFyIFBpdGNoYWkNCg0KLS0t
LS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCkZyb206IExvcmVuem8gUGllcmFsaXNpIDxsb3Jlbnpv
LnBpZXJhbGlzaUBhcm0uY29tPg0KU2VudDogVGh1cnNkYXksIE1hcmNoIDE1LCAyMDE4IDU6MDUg
QU0NClRvOiBTcmlkaGFyIFBpdGNoYWkgPFNyaWRoYXIuUGl0Y2hhaUBtaWNyb3NvZnQuY29tPg0K
Q2M6IEJqb3JuIEhlbGdhYXMgPGJoZWxnYWFzQGdvb2dsZS5jb20+OyBKYWtlIE9zaGlucyA8amFr
ZW9AbWljcm9zb2Z0LmNvbT47IEhhaXlhbmcgWmhhbmcgPGhhaXlhbmd6QG1pY3Jvc29mdC5jb20+
OyBTdGVwaGVuIEhlbW1pbmdlciA8c3RoZW1taW5AbWljcm9zb2Z0LmNvbT47IERleHVhbiBDdWkg
PGRlY3VpQG1pY3Jvc29mdC5jb20+OyBLWSBTcmluaXZhc2FuIDxreXNAbWljcm9zb2Z0LmNvbT47
IE1pY2hhZWwgS2VsbGV5IChFT1NHKSA8TWljaGFlbC5ILktlbGxleUBtaWNyb3NvZnQuY29tPjsg
ZGV2ZWxAbGludXhkcml2ZXJwcm9qZWN0Lm9yZzsgbGludXgtcGNpQHZnZXIua2VybmVsLm9yZzsg
bGludXgta2VybmVsQHZnZXIua2VybmVsLm9yZw0KU3ViamVjdDogUmU6IFtQQVRDSCB2M11QQ0k6
IGh2OiBmaXggUENJLUJVUyBkb21haW5JRCBjb3JydXB0aW9uDQoNCk9uIFRodSwgTWFyIDE1LCAy
MDE4IGF0IDEyOjAzOjA3QU0gKzAwMDAsIFNyaWRoYXIgUGl0Y2hhaSB3cm90ZToNCldoZW5ldmVy
IFBDSSBidXMgaXMgYWRkZWQsIEh5cGVyViBndWFyYW50ZWVzIHRoZSBCVVMgaWQgaXMgdW5pcXVl
LiBFdmVuDQoNCiJXaGVuZXZlciBhIFBDSSBidXMgaXMgYWRkZWQiDQpTcmlkaGFyPj4geWVzDQoN
CndpdGggdGhhdCB3aGVuIGEgZmlyc3QgZGV2aWNlIGlzIGFkZGVkIHRvIHRoZSBidXMsIGl0IG92
ZXJyaWRlcyBidXMgZG9tYWluDQpJRCB3aXRoIHRoZSBkZXZpY2Ugc2VyaWFsIG51bWJlci4gU29t
ZXRpbWUgdGhpcyBjYW4gcmVzdWx0IGluIEJVUyBJRCBub3QNCg0KRGVmaW5lICJTb21ldGltZSIu
DQoNClNyaWRoYXI+PiBIeXBlclYgd2hlbiBpdCBjcmVhdGVzIGEgUENJIGJ1cyBpdCBndWFyYW50
ZWVzIGl0IHByb3ZpZGUgYSB1bmlxdWUgSUQgZm9yIGl0Lg0KQnV0LCB0aGF0IHVuaXF1ZSBCVVMg
SUQgaXMgcmVwbGFjZWQgd2l0aCBkZXZpY2Ugc2VyaWFsIG51bWJlci4gMCBpcyBhIHZhbGlkDQpk
ZXZpY2Ugc2VyaWFsIG51bWJlciwgYW5kIGlmIHRoZXJlIGV4aXN0cyBhIFBDSSBidXMgd2l0aCBk
b21haW4gSUQgMCAoR2VuIDENCnZlcnNpb24gb2YgaHlwZXJWIFZNIGhhdmUgdGhpcyBmb3IgcGFy
YSB2aXJ0dWFsIGRldmljZXMpLCB0aGlzIHdpbGwgcmVzdWx0IGluDQpQQ0kgYnVzIGlkIG5vdCBi
ZWluZyB1bmlxdWUuDQoNCg0KYmVpbmcgdW5pcXVlLiBJbiB0aGlzIGNhc2UsIHdoZW4gUENJX0JV
UyBhbmQgYSBkZXZpY2UgdG8gYnVzIGlzIGFkZGVkLCB0aGUNCmZpcnN0IGRldmljZSBvdmVyd3Jp
dGVzIHRoZSBidXMgZG9tYWluIElEIHRvIHRoZSBkZXZpY2Ugc2VyaWFsIG51bWJlciwNCndoaWNo
IGlzIDAuIFNpbmNlIHRoZXJlIGV4c2lzdCBhIFBDSSBidXMgd2l0aCBkb21haW4gSUQgMCBhbHJl
YWR5IHRoZSBQQ0kNCg0Kcy9leHNpc3QvZXhpc3QNCg0KU3JpZGhhcj4+IHllcw0KDQpidXMgYWRk
aXRpb24gZmFpbHMuIFRoaXMgcGF0Y2ggbWFrZSBzdXJlIHdoZW4gYSBkZXZpY2UgaXMgYWRkZWQg
dG8gYSBidXMsDQppdCBuZXZlciB1cGRhdGVkIHRoZSBidXMgZG9tYWluIElELg0KDQpzL3VwZGF0
ZWQvdXBkYXRlcw0KU3JpZGhhciA+PiB5ZXMNCg0KU2luY2Ugd2UgaGF2ZSB0aGUgdHJhbnNwYXJl
bnQgU1JJT1YgbW9kZSBub3csIHRoZSBzaG9ydCBWRiBkZXZpY2UgbmFtZQ0KaXMgbm8gbG9uZ2Vy
IG5lZWRlZC4NCg0KSSBzdGlsbCBkbyBub3QgdW5kZXJzdGFuZCB3aGF0IHRoaXMgbWVhbnMgYW5k
IGhvdyBpdCBpcyByZWxhdGVkIHRvIHRoZQ0KcGF0Y2ggYmVsb3csIGl0IG1heSBiZSBjbGVhciB0
byB5b3UsIGl0IGlzIG5vdCB0byBtZSwgYXQgYWxsLg0KDQpTcmlkaGFyID4+IHRoZSBwYXRjaCBi
ZWxvdywgd2FzIGludHJvZHVjZWQgdG8gbWFrZSB0aGUgZGV2aWNlIG5hbWUgc21hbGwsIGJ5IHRh
a2luZyBvbmx5DQoxNmJpdHMgb2YgdGhlIHNlcmlhbCBudW1iZXIuIFNpbmNlIHdlIGFyZSBub3Qg
Z29pbmcgdG8gaGF2ZSB0aGUgc2VyaWFsIG51bWJlcg0KdXBkYXRlZCB0byB0aGUgQlVTIGlkLCB0
aGlzIGhhcyB0byBiZSByZW1vdmVkLg0KDQpGaXhlczogNGE5YjA5MzNiZGZjKCJQQ0k6aHY6VXNl
IGRldmljZSBzZXJpYWwgbnVtYmVyIGFzIFBDSSBkb21haW4iKQ0KDQpGaXhlczogNGE5YjA5MzNi
ZGZjICgiUENJOiBodjogVXNlIGRldmljZSBzZXJpYWwgbnVtYmVyIGFzIFBDSSBkb21haW4iKQ0K
U3JpZGhyID4+IHllcw0KDQpJIGFza2VkIHlvdSBhbiBleHBsaWNpdCBxdWVzdGlvbi4gQ29tbWl0
IGFib3ZlIHdhcyBhZGRlZCBmb3IgYSByZWFzb24NCkkgYXNzdW1lLiBUaGlzIHBhdGNoIGltcGxp
ZXMgdGhhdCBrZXJuZWwgaGFzIGJlZW4gYnJva2VuIHNpbmNlIHY0LjExDQp3aGljaCBpcyBhbG1v
c3QgYSB5ZWFyIGFnbyBhbmQgbm9ib2R5IGV2ZXJ5IG5vdGljZWQgPyBPciB0aGVyZSBhcmUNCnN5
c3RlbXMgd2hlcmUgY29tbWl0IGFib3ZlIGlzIF9uZWNlc3NhcnlfIGFuZCB0aGlzIHBhdGNoIHdv
dWxkIGJyZWFrDQp0aGVtID8NCg0KSSB3YW50IGEgZGV0YWlsZWQgZXhwbGFuYXRpb24gdGhhdCBo
aWdobGlnaHRzICp3aHkqIGl0IGlzIHNhZmUgdG8gYXBwbHkNCnRoaXMgcGF0Y2ggYW5kIHNlbmQg
aXQgdG8gc3RhYmxlIGtlcm5lbHMsIGNvbW1pdCBsb2cgYWJvdmUgd29uJ3QgZG8uDQoNClNyaWRo
YXI+PiBIeXBlclYgcHJvdmlkZXMgYSB1bmlxdWUgZG9tYWluIElEIGZvciBQQ0kgQlVTLiBCdXQg
aXQgaXMgbW9kaWZpZWQgYnkgdGhlIGNoaWxkDQpkZXZpY2Ugd2hlbiBpdCBpcyBhZGRlZC4gVGhp
cyBjYW5ub3QgcHJvZHVjZSBhIHVuaXF1ZSBkb21haW4gSUQgYWxsIHRoZSB0aW1lLg0KSGVyZSBp
biB0aGUgYnVnLCB3ZSBzZWUgdGhlIGNvbGxpc2lvbiBiZXR3ZWVuIHRoZSBzZXJpYWwgbnVtYmVy
IGFuZCBhbHJlYWR5DQpleGlzdGluZyBQQ0kgYnVzLiBUaGUgY2xlYW5lciB3YXkgaXMgbmV2ZXIg
dG91Y2ggdGhlIGRvbWFpbiBJRCBwcm92aWRlZCBieQ0KaHlwZXJWIGR1cmluZyB0aGUgUENJIGJ1
cyBjcmVhdGlvbi4gQXMgbG9uZyBhcyBoeXBlclYgbWFrZSBzdXJlIGl0IHByb3ZpZGVzIGENCnVu
aXF1ZSBkb21haW4gSUQgZm9yIHRoZSBQQ0kgZm9yIGEgVk0gaXQgd2lsbCBub3QgYnJlYWssIGFu
ZCBIeXBlclYgd2lsbA0KZ3VhcmFudGVlcyB0aGF0IHRoZSBkb21haW4gZm9yIHRoZSBQQ0kgYnVz
IGZvciBhIGdpdmVuIFZNIHdpbGwgYmUgYWx3YXlzIHVuaXF1ZS4NClRoZSBvcmlnaW5hbCBwYXRj
aCB3YXMgYWxzbyBpbnRlbmRpbmcgdG8gaGF2ZSBhIHVuaXF1ZSBkb21haW4gSUQgZm9yIHRoZSBQ
Q0kNCmJ1cywgYnkgdGFraW5nIHRoZSBzZXJpYWwgbnVtYmVyIG9mIHRoZSBkZXZpY2UsIGJ1dCBp
dCBpcyBub3Qgc3VmZmljaWVudCwgd2hlbg0KdGhlIGRldmljZSBzZXJpYWwgbnVtYmVyIGlzIG51
bWJlciB3aGljaCBpcyB0aGUgZG9tYWluIElEIG9mIHRoZSBleGlzdGluZyBQQ0kNCmJ1cy4gIFdp
dGggdGhlIGN1cnJlbnQga2VybmVsIHdlIGNhbiByZXBybyB0aGlzIGlzc3VlIGJ5IGFkZGluZyBh
IGRldmljZSB3aXRoIGENCnNlcmlhbCBudW1iZXIgbWF0Y2hpbmcgdGhlIGV4aXN0aW5nIFBDSSBi
dXMgZG9tYWluIGlkLiAoaW4gdGhpcyBjYXNlIHRoYXQNCmhhcHBlbnMgdG8gYmUgemVybykuDQoN
Cg0KVGhhbmtzLA0KTG9yZW56bw0KDQpDYzogc3RhYmxlQHZnZXIua2VybmVsLm9yZw0KU2lnbmVk
LW9mZi1ieTogU3JpZGhhciBQaXRjaGFpIDxzcnBpdGNoYUBtaWNyb3NvZnQuY29tPg0KLS0tDQpD
aGFuZ2VzIGluIHYzOg0KKiBmaXggdGhlIGNvbW1pdCBjb21tZW50LiBbS1kgU3Jpbml2YXNhbiwg
TWljaGFlbCBLZWxsZXldDQotLS0NCiAgZHJpdmVycy9wY2kvaG9zdC9wY2ktaHlwZXJ2LmMgfCAx
MSAtLS0tLS0tLS0tLQ0KICAxIGZpbGUgY2hhbmdlZCwgMTEgZGVsZXRpb25zKC0pDQpkaWZmIC0t
Z2l0IGEvZHJpdmVycy9wY2kvaG9zdC9wY2ktaHlwZXJ2LmMgYi9kcml2ZXJzL3BjaS9ob3N0L3Bj
aS1oeXBlcnYuYw0KaW5kZXggMmZhZjM4ZS4uYWM2N2U1NiAxMDA2NDQNCi0tLSBhL2RyaXZlcnMv
cGNpL2hvc3QvcGNpLWh5cGVydi5jDQorKysgYi9kcml2ZXJzL3BjaS9ob3N0L3BjaS1oeXBlcnYu
Yw0KQEAgLTE1MTgsMTcgKzE1MTgsNiBAQCBzdGF0aWMgc3RydWN0IGh2X3BjaV9kZXYgKm5ld19w
Y2ljaGlsZF9kZXZpY2Uoc3RydWN0IGh2X3BjaWJ1c19kZXZpY2UgKmhidXMsDQogIAlnZXRfcGNp
Y2hpbGQoaHBkZXYsIGh2X3BjaWRldl9yZWZfY2hpbGRsaXN0KTsNCiAgCXNwaW5fbG9ja19pcnFz
YXZlKCZoYnVzLT5kZXZpY2VfbGlzdF9sb2NrLCBmbGFncyk7DQogIA0KLQkvKg0KLQkgKiBXaGVu
IGEgZGV2aWNlIGlzIGJlaW5nIGFkZGVkIHRvIHRoZSBidXMsIHdlIHNldCB0aGUgUENJIGRvbWFp
bg0KLQkgKiBudW1iZXIgdG8gYmUgdGhlIGRldmljZSBzZXJpYWwgbnVtYmVyLCB3aGljaCBpcyBu
b24temVybyBhbmQNCi0JICogdW5pcXVlIG9uIHRoZSBzYW1lIFZNLiAgVGhlIHNlcmlhbCBudW1i
ZXJzIHN0YXJ0IHdpdGggMSwgYW5kDQotCSAqIGluY3JlYXNlIGJ5IDEgZm9yIGVhY2ggZGV2aWNl
LiAgU28gZGV2aWNlIG5hbWVzIGluY2x1ZGluZyB0aGlzDQotCSAqIGNhbiBoYXZlIHNob3J0ZXIg
bmFtZXMgdGhhbiBiYXNlZCBvbiB0aGUgYnVzIGluc3RhbmNlIFVVSUQuDQotCSAqIE9ubHkgdGhl
IGZpcnN0IGRldmljZSBzZXJpYWwgbnVtYmVyIGlzIHVzZWQgZm9yIGRvbWFpbiwgc28gdGhlDQot
CSAqIGRvbWFpbiBudW1iZXIgd2lsbCBub3QgY2hhbmdlIGFmdGVyIHRoZSBmaXJzdCBkZXZpY2Ug
aXMgYWRkZWQuDQotCSAqLw0KLQlpZiAobGlzdF9lbXB0eSgmaGJ1cy0+Y2hpbGRyZW4pKQ0KLQkJ
aGJ1cy0+c3lzZGF0YS5kb21haW4gPSBkZXNjLT5zZXI7DQogIAlsaXN0X2FkZF90YWlsKCZocGRl
di0+bGlzdF9lbnRyeSwgJmhidXMtPmNoaWxkcmVuKTsNCiAgCXNwaW5fdW5sb2NrX2lycXJlc3Rv
cmUoJmhidXMtPmRldmljZV9saXN0X2xvY2ssIGZsYWdzKTsNCiAgCXJldHVybiBocGRldjsNCi0t
DQoyLjcuNCANCg0K
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
2018-03-15 18:24 ` Sridhar Pitchai
@ 2018-03-20 17:56 ` Sridhar Pitchai
-1 siblings, 0 replies; 26+ messages in thread
From: Sridhar Pitchai @ 2018-03-20 17:56 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Stephen Hemminger, linux-pci, Haiyang Zhang, linux-kernel,
Michael Kelley (EOSG),
Jake Oshins, Bjorn Helgaas, devel
Hi Lorenzo,
Are we good with the explanation? Can I send the patch with the updated commit comments?
Thanks
Sridhar
On 3/15/18, 11:24 AM, "Sridhar Pitchai" <Sridhar.Pitchai@microsoft.com> wrote:
Apologies for not aligning my reply, I just realized after looking into
the mail archive in LKML. I have aligned the replay now.
On 3/15/18, 10:56 AM, "Sridhar Pitchai" <Sridhar.Pitchai@microsoft.com> wrote:
Hi Lorenzo,
Answering the question inline.
Kindly let me know if it clarifies. I will send out another patch after we agree on the clarification.
Thanks
Sridhar Pitchai
-----Original Message-----
From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Sent: Thursday, March 15, 2018 5:05 AM
To: Sridhar Pitchai <Sridhar.Pitchai@microsoft.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>; Jake Oshins <jakeo@microsoft.com>; Haiyang Zhang <haiyangz@microsoft.com>; Stephen Hemminger <sthemmin@microsoft.com>; Dexuan Cui <decui@microsoft.com>; KY Srinivasan <kys@microsoft.com>; Michael Kelley (EOSG) <Michael.H.Kelley@microsoft.com>; devel@linuxdriverproject.org; linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
On Thu, Mar 15, 2018 at 12:03:07AM +0000, Sridhar Pitchai wrote:
Whenever PCI bus is added, HyperV guarantees the BUS id is unique. Even
"Whenever a PCI bus is added"
Sridhar>> yes
with that when a first device is added to the bus, it overrides bus domain
ID with the device serial number. Sometime this can result in BUS ID not
Define "Sometime".
Sridhar>> HyperV when it creates a PCI bus it guarantees it provide a unique ID for it.
But, that unique BUS ID is replaced with device serial number. 0 is a valid
device serial number, and if there exists a PCI bus with domain ID 0 (Gen 1
version of hyperV VM have this for para virtual devices), this will result in
PCI bus id not being unique.
being unique. In this case, when PCI_BUS and a device to bus is added, the
first device overwrites the bus domain ID to the device serial number,
which is 0. Since there exsist a PCI bus with domain ID 0 already the PCI
s/exsist/exist
Sridhar>> yes
bus addition fails. This patch make sure when a device is added to a bus,
it never updated the bus domain ID.
s/updated/updates
Sridhar >> yes
Since we have the transparent SRIOV mode now, the short VF device name
is no longer needed.
I still do not understand what this means and how it is related to the
patch below, it may be clear to you, it is not to me, at all.
Sridhar >> the patch below, was introduced to make the device name small, by taking only
16bits of the serial number. Since we are not going to have the serial number
updated to the BUS id, this has to be removed.
Fixes: 4a9b0933bdfc("PCI:hv:Use device serial number as PCI domain")
Fixes: 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI domain")
Sridhr >> yes
I asked you an explicit question. Commit above was added for a reason
I assume. This patch implies that kernel has been broken since v4.11
which is almost a year ago and nobody every noticed ? Or there are
systems where commit above is _necessary_ and this patch would break
them ?
I want a detailed explanation that highlights *why* it is safe to apply
this patch and send it to stable kernels, commit log above won't do.
Sridhar>> HyperV provides a unique domain ID for PCI BUS. But it is modified by the child
device when it is added. This cannot produce a unique domain ID all the time.
Here in the bug, we see the collision between the serial number and already
existing PCI bus. The cleaner way is never touch the domain ID provided by
hyperV during the PCI bus creation. As long as hyperV make sure it provides a
unique domain ID for the PCI for a VM it will not break, and HyperV will
guarantees that the domain for the PCI bus for a given VM will be always unique.
The original patch was also intending to have a unique domain ID for the PCI
bus, by taking the serial number of the device, but it is not sufficient, when
the device serial number is number which is the domain ID of the existing PCI
bus. With the current kernel we can repro this issue by adding a device with a
serial number matching the existing PCI bus domain id. (in this case that
happens to be zero).
Thanks,
Lorenzo
Cc: stable@vger.kernel.org
Signed-off-by: Sridhar Pitchai <srpitcha@microsoft.com>
---
Changes in v3:
* fix the commit comment. [KY Srinivasan, Michael Kelley]
---
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 2faf38e..ac67e56 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.7.4
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
@ 2018-03-20 17:56 ` Sridhar Pitchai
0 siblings, 0 replies; 26+ messages in thread
From: Sridhar Pitchai @ 2018-03-20 17:56 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Bjorn Helgaas, Jake Oshins, Haiyang Zhang, Stephen Hemminger,
Dexuan Cui, KY Srinivasan, Michael Kelley (EOSG),
devel, linux-pci, linux-kernel
SGkgTG9yZW56bywNCkFyZSB3ZSBnb29kIHdpdGggdGhlIGV4cGxhbmF0aW9uPyBDYW4gSSBzZW5k
IHRoZSBwYXRjaCB3aXRoIHRoZSB1cGRhdGVkIGNvbW1pdCBjb21tZW50cz8NCg0KVGhhbmtzDQpT
cmlkaGFyDQoNCu+7v09uIDMvMTUvMTgsIDExOjI0IEFNLCAiU3JpZGhhciBQaXRjaGFpIiA8U3Jp
ZGhhci5QaXRjaGFpQG1pY3Jvc29mdC5jb20+IHdyb3RlOg0KDQogICAgQXBvbG9naWVzIGZvciBu
b3QgYWxpZ25pbmcgbXkgcmVwbHksIEkganVzdCByZWFsaXplZCBhZnRlciBsb29raW5nIGludG8N
CiAgICB0aGUgbWFpbCBhcmNoaXZlIGluIExLTUwuICBJIGhhdmUgYWxpZ25lZCB0aGUgcmVwbGF5
IG5vdy4NCiAgICANCiAgICANCiAgICBPbiAzLzE1LzE4LCAxMDo1NiBBTSwgIlNyaWRoYXIgUGl0
Y2hhaSIgPFNyaWRoYXIuUGl0Y2hhaUBtaWNyb3NvZnQuY29tPiB3cm90ZToNCiAgICANCiAgICBI
aSBMb3JlbnpvLA0KICAgIEFuc3dlcmluZyB0aGUgcXVlc3Rpb24gaW5saW5lLg0KICAgIEtpbmRs
eSBsZXQgbWUga25vdyBpZiBpdCBjbGFyaWZpZXMuIEkgd2lsbCBzZW5kIG91dCBhbm90aGVyIHBh
dGNoIGFmdGVyIHdlIGFncmVlIG9uIHRoZSBjbGFyaWZpY2F0aW9uLg0KICAgIA0KICAgIFRoYW5r
cw0KICAgIFNyaWRoYXIgUGl0Y2hhaQ0KICAgIA0KICAgIC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0t
LS0tDQogICAgRnJvbTogTG9yZW56byBQaWVyYWxpc2kgPGxvcmVuem8ucGllcmFsaXNpQGFybS5j
b20+DQogICAgU2VudDogVGh1cnNkYXksIE1hcmNoIDE1LCAyMDE4IDU6MDUgQU0NCiAgICBUbzog
U3JpZGhhciBQaXRjaGFpIDxTcmlkaGFyLlBpdGNoYWlAbWljcm9zb2Z0LmNvbT4NCiAgICBDYzog
Qmpvcm4gSGVsZ2FhcyA8YmhlbGdhYXNAZ29vZ2xlLmNvbT47IEpha2UgT3NoaW5zIDxqYWtlb0Bt
aWNyb3NvZnQuY29tPjsgSGFpeWFuZyBaaGFuZyA8aGFpeWFuZ3pAbWljcm9zb2Z0LmNvbT47IFN0
ZXBoZW4gSGVtbWluZ2VyIDxzdGhlbW1pbkBtaWNyb3NvZnQuY29tPjsgRGV4dWFuIEN1aSA8ZGVj
dWlAbWljcm9zb2Z0LmNvbT47IEtZIFNyaW5pdmFzYW4gPGt5c0BtaWNyb3NvZnQuY29tPjsgTWlj
aGFlbCBLZWxsZXkgKEVPU0cpIDxNaWNoYWVsLkguS2VsbGV5QG1pY3Jvc29mdC5jb20+OyBkZXZl
bEBsaW51eGRyaXZlcnByb2plY3Qub3JnOyBsaW51eC1wY2lAdmdlci5rZXJuZWwub3JnOyBsaW51
eC1rZXJuZWxAdmdlci5rZXJuZWwub3JnDQogICAgU3ViamVjdDogUmU6IFtQQVRDSCB2M11QQ0k6
IGh2OiBmaXggUENJLUJVUyBkb21haW5JRCBjb3JydXB0aW9uDQogICAgDQogICAgT24gVGh1LCBN
YXIgMTUsIDIwMTggYXQgMTI6MDM6MDdBTSArMDAwMCwgU3JpZGhhciBQaXRjaGFpIHdyb3RlOg0K
ICAgIFdoZW5ldmVyIFBDSSBidXMgaXMgYWRkZWQsIEh5cGVyViBndWFyYW50ZWVzIHRoZSBCVVMg
aWQgaXMgdW5pcXVlLiBFdmVuDQogICAgDQogICAgIldoZW5ldmVyIGEgUENJIGJ1cyBpcyBhZGRl
ZCINCiAgICBTcmlkaGFyPj4geWVzDQogICAgDQogICAgd2l0aCB0aGF0IHdoZW4gYSBmaXJzdCBk
ZXZpY2UgaXMgYWRkZWQgdG8gdGhlIGJ1cywgaXQgb3ZlcnJpZGVzIGJ1cyBkb21haW4NCiAgICBJ
RCB3aXRoIHRoZSBkZXZpY2Ugc2VyaWFsIG51bWJlci4gU29tZXRpbWUgdGhpcyBjYW4gcmVzdWx0
IGluIEJVUyBJRCBub3QNCiAgICANCiAgICBEZWZpbmUgIlNvbWV0aW1lIi4NCiAgICANCiAgICBT
cmlkaGFyPj4gSHlwZXJWIHdoZW4gaXQgY3JlYXRlcyBhIFBDSSBidXMgaXQgZ3VhcmFudGVlcyBp
dCBwcm92aWRlIGEgdW5pcXVlIElEIGZvciBpdC4NCiAgICBCdXQsIHRoYXQgdW5pcXVlIEJVUyBJ
RCBpcyByZXBsYWNlZCB3aXRoIGRldmljZSBzZXJpYWwgbnVtYmVyLiAwIGlzIGEgdmFsaWQNCiAg
ICBkZXZpY2Ugc2VyaWFsIG51bWJlciwgYW5kIGlmIHRoZXJlIGV4aXN0cyBhIFBDSSBidXMgd2l0
aCBkb21haW4gSUQgMCAoR2VuIDENCiAgICB2ZXJzaW9uIG9mIGh5cGVyViBWTSBoYXZlIHRoaXMg
Zm9yIHBhcmEgdmlydHVhbCBkZXZpY2VzKSwgdGhpcyB3aWxsIHJlc3VsdCBpbg0KICAgIFBDSSBi
dXMgaWQgbm90IGJlaW5nIHVuaXF1ZS4NCiAgICANCiAgICANCiAgICBiZWluZyB1bmlxdWUuIElu
IHRoaXMgY2FzZSwgd2hlbiBQQ0lfQlVTIGFuZCBhIGRldmljZSB0byBidXMgaXMgYWRkZWQsIHRo
ZQ0KICAgIGZpcnN0IGRldmljZSBvdmVyd3JpdGVzIHRoZSBidXMgZG9tYWluIElEIHRvIHRoZSBk
ZXZpY2Ugc2VyaWFsIG51bWJlciwNCiAgICB3aGljaCBpcyAwLiBTaW5jZSB0aGVyZSBleHNpc3Qg
YSBQQ0kgYnVzIHdpdGggZG9tYWluIElEIDAgYWxyZWFkeSB0aGUgUENJDQogICAgDQogICAgcy9l
eHNpc3QvZXhpc3QNCiAgICANCiAgICBTcmlkaGFyPj4geWVzDQogICAgDQogICAgYnVzIGFkZGl0
aW9uIGZhaWxzLiBUaGlzIHBhdGNoIG1ha2Ugc3VyZSB3aGVuIGEgZGV2aWNlIGlzIGFkZGVkIHRv
IGEgYnVzLA0KICAgIGl0IG5ldmVyIHVwZGF0ZWQgdGhlIGJ1cyBkb21haW4gSUQuDQogICAgDQog
ICAgcy91cGRhdGVkL3VwZGF0ZXMNCiAgICBTcmlkaGFyID4+IHllcw0KICAgIA0KICAgIFNpbmNl
IHdlIGhhdmUgdGhlIHRyYW5zcGFyZW50IFNSSU9WIG1vZGUgbm93LCB0aGUgc2hvcnQgVkYgZGV2
aWNlIG5hbWUNCiAgICBpcyBubyBsb25nZXIgbmVlZGVkLg0KICAgIA0KICAgIEkgc3RpbGwgZG8g
bm90IHVuZGVyc3RhbmQgd2hhdCB0aGlzIG1lYW5zIGFuZCBob3cgaXQgaXMgcmVsYXRlZCB0byB0
aGUNCiAgICBwYXRjaCBiZWxvdywgaXQgbWF5IGJlIGNsZWFyIHRvIHlvdSwgaXQgaXMgbm90IHRv
IG1lLCBhdCBhbGwuDQogICAgDQogICAgU3JpZGhhciA+PiB0aGUgcGF0Y2ggYmVsb3csIHdhcyBp
bnRyb2R1Y2VkIHRvIG1ha2UgdGhlIGRldmljZSBuYW1lIHNtYWxsLCBieSB0YWtpbmcgb25seQ0K
ICAgIDE2Yml0cyBvZiB0aGUgc2VyaWFsIG51bWJlci4gU2luY2Ugd2UgYXJlIG5vdCBnb2luZyB0
byBoYXZlIHRoZSBzZXJpYWwgbnVtYmVyDQogICAgdXBkYXRlZCB0byB0aGUgQlVTIGlkLCB0aGlz
IGhhcyB0byBiZSByZW1vdmVkLg0KICAgIA0KICAgIEZpeGVzOiA0YTliMDkzM2JkZmMoIlBDSTpo
djpVc2UgZGV2aWNlIHNlcmlhbCBudW1iZXIgYXMgUENJIGRvbWFpbiIpDQogICAgDQogICAgRml4
ZXM6IDRhOWIwOTMzYmRmYyAoIlBDSTogaHY6IFVzZSBkZXZpY2Ugc2VyaWFsIG51bWJlciBhcyBQ
Q0kgZG9tYWluIikNCiAgICBTcmlkaHIgPj4geWVzDQogICAgDQogICAgSSBhc2tlZCB5b3UgYW4g
ZXhwbGljaXQgcXVlc3Rpb24uIENvbW1pdCBhYm92ZSB3YXMgYWRkZWQgZm9yIGEgcmVhc29uDQog
ICAgSSBhc3N1bWUuIFRoaXMgcGF0Y2ggaW1wbGllcyB0aGF0IGtlcm5lbCBoYXMgYmVlbiBicm9r
ZW4gc2luY2UgdjQuMTENCiAgICB3aGljaCBpcyBhbG1vc3QgYSB5ZWFyIGFnbyBhbmQgbm9ib2R5
IGV2ZXJ5IG5vdGljZWQgPyBPciB0aGVyZSBhcmUNCiAgICBzeXN0ZW1zIHdoZXJlIGNvbW1pdCBh
Ym92ZSBpcyBfbmVjZXNzYXJ5XyBhbmQgdGhpcyBwYXRjaCB3b3VsZCBicmVhaw0KICAgIHRoZW0g
Pw0KICAgIA0KICAgIEkgd2FudCBhIGRldGFpbGVkIGV4cGxhbmF0aW9uIHRoYXQgaGlnaGxpZ2h0
cyAqd2h5KiBpdCBpcyBzYWZlIHRvIGFwcGx5DQogICAgdGhpcyBwYXRjaCBhbmQgc2VuZCBpdCB0
byBzdGFibGUga2VybmVscywgY29tbWl0IGxvZyBhYm92ZSB3b24ndCBkby4NCiAgICANCiAgICBT
cmlkaGFyPj4gSHlwZXJWIHByb3ZpZGVzIGEgdW5pcXVlIGRvbWFpbiBJRCBmb3IgUENJIEJVUy4g
QnV0IGl0IGlzIG1vZGlmaWVkIGJ5IHRoZSBjaGlsZA0KICAgIGRldmljZSB3aGVuIGl0IGlzIGFk
ZGVkLiBUaGlzIGNhbm5vdCBwcm9kdWNlIGEgdW5pcXVlIGRvbWFpbiBJRCBhbGwgdGhlIHRpbWUu
DQogICAgSGVyZSBpbiB0aGUgYnVnLCB3ZSBzZWUgdGhlIGNvbGxpc2lvbiBiZXR3ZWVuIHRoZSBz
ZXJpYWwgbnVtYmVyIGFuZCBhbHJlYWR5DQogICAgZXhpc3RpbmcgUENJIGJ1cy4gVGhlIGNsZWFu
ZXIgd2F5IGlzIG5ldmVyIHRvdWNoIHRoZSBkb21haW4gSUQgcHJvdmlkZWQgYnkNCiAgICBoeXBl
clYgZHVyaW5nIHRoZSBQQ0kgYnVzIGNyZWF0aW9uLiBBcyBsb25nIGFzIGh5cGVyViBtYWtlIHN1
cmUgaXQgcHJvdmlkZXMgYQ0KICAgIHVuaXF1ZSBkb21haW4gSUQgZm9yIHRoZSBQQ0kgZm9yIGEg
Vk0gaXQgd2lsbCBub3QgYnJlYWssIGFuZCBIeXBlclYgd2lsbA0KICAgIGd1YXJhbnRlZXMgdGhh
dCB0aGUgZG9tYWluIGZvciB0aGUgUENJIGJ1cyBmb3IgYSBnaXZlbiBWTSB3aWxsIGJlIGFsd2F5
cyB1bmlxdWUuDQogICAgVGhlIG9yaWdpbmFsIHBhdGNoIHdhcyBhbHNvIGludGVuZGluZyB0byBo
YXZlIGEgdW5pcXVlIGRvbWFpbiBJRCBmb3IgdGhlIFBDSQ0KICAgIGJ1cywgYnkgdGFraW5nIHRo
ZSBzZXJpYWwgbnVtYmVyIG9mIHRoZSBkZXZpY2UsIGJ1dCBpdCBpcyBub3Qgc3VmZmljaWVudCwg
d2hlbg0KICAgIHRoZSBkZXZpY2Ugc2VyaWFsIG51bWJlciBpcyBudW1iZXIgd2hpY2ggaXMgdGhl
IGRvbWFpbiBJRCBvZiB0aGUgZXhpc3RpbmcgUENJDQogICAgYnVzLiAgV2l0aCB0aGUgY3VycmVu
dCBrZXJuZWwgd2UgY2FuIHJlcHJvIHRoaXMgaXNzdWUgYnkgYWRkaW5nIGEgZGV2aWNlIHdpdGgg
YQ0KICAgIHNlcmlhbCBudW1iZXIgbWF0Y2hpbmcgdGhlIGV4aXN0aW5nIFBDSSBidXMgZG9tYWlu
IGlkLiAoaW4gdGhpcyBjYXNlIHRoYXQNCiAgICBoYXBwZW5zIHRvIGJlIHplcm8pLg0KICAgIA0K
ICAgIA0KICAgIFRoYW5rcywNCiAgICBMb3JlbnpvDQogICAgDQogICAgQ2M6IHN0YWJsZUB2Z2Vy
Lmtlcm5lbC5vcmcNCiAgICBTaWduZWQtb2ZmLWJ5OiBTcmlkaGFyIFBpdGNoYWkgPHNycGl0Y2hh
QG1pY3Jvc29mdC5jb20+DQogICAgLS0tDQogICAgQ2hhbmdlcyBpbiB2MzoNCiAgICAqIGZpeCB0
aGUgY29tbWl0IGNvbW1lbnQuIFtLWSBTcmluaXZhc2FuLCBNaWNoYWVsIEtlbGxleV0NCiAgICAt
LS0NCiAgICAgIGRyaXZlcnMvcGNpL2hvc3QvcGNpLWh5cGVydi5jIHwgMTEgLS0tLS0tLS0tLS0N
CiAgICAgIDEgZmlsZSBjaGFuZ2VkLCAxMSBkZWxldGlvbnMoLSkNCiAgICBkaWZmIC0tZ2l0IGEv
ZHJpdmVycy9wY2kvaG9zdC9wY2ktaHlwZXJ2LmMgYi9kcml2ZXJzL3BjaS9ob3N0L3BjaS1oeXBl
cnYuYw0KICAgIGluZGV4IDJmYWYzOGUuLmFjNjdlNTYgMTAwNjQ0DQogICAgLS0tIGEvZHJpdmVy
cy9wY2kvaG9zdC9wY2ktaHlwZXJ2LmMNCiAgICArKysgYi9kcml2ZXJzL3BjaS9ob3N0L3BjaS1o
eXBlcnYuYw0KICAgIEBAIC0xNTE4LDE3ICsxNTE4LDYgQEAgc3RhdGljIHN0cnVjdCBodl9wY2lf
ZGV2ICpuZXdfcGNpY2hpbGRfZGV2aWNlKHN0cnVjdCBodl9wY2lidXNfZGV2aWNlICpoYnVzLA0K
ICAgICAgCWdldF9wY2ljaGlsZChocGRldiwgaHZfcGNpZGV2X3JlZl9jaGlsZGxpc3QpOw0KICAg
ICAgCXNwaW5fbG9ja19pcnFzYXZlKCZoYnVzLT5kZXZpY2VfbGlzdF9sb2NrLCBmbGFncyk7DQog
ICAgICANCiAgICAtCS8qDQogICAgLQkgKiBXaGVuIGEgZGV2aWNlIGlzIGJlaW5nIGFkZGVkIHRv
IHRoZSBidXMsIHdlIHNldCB0aGUgUENJIGRvbWFpbg0KICAgIC0JICogbnVtYmVyIHRvIGJlIHRo
ZSBkZXZpY2Ugc2VyaWFsIG51bWJlciwgd2hpY2ggaXMgbm9uLXplcm8gYW5kDQogICAgLQkgKiB1
bmlxdWUgb24gdGhlIHNhbWUgVk0uICBUaGUgc2VyaWFsIG51bWJlcnMgc3RhcnQgd2l0aCAxLCBh
bmQNCiAgICAtCSAqIGluY3JlYXNlIGJ5IDEgZm9yIGVhY2ggZGV2aWNlLiAgU28gZGV2aWNlIG5h
bWVzIGluY2x1ZGluZyB0aGlzDQogICAgLQkgKiBjYW4gaGF2ZSBzaG9ydGVyIG5hbWVzIHRoYW4g
YmFzZWQgb24gdGhlIGJ1cyBpbnN0YW5jZSBVVUlELg0KICAgIC0JICogT25seSB0aGUgZmlyc3Qg
ZGV2aWNlIHNlcmlhbCBudW1iZXIgaXMgdXNlZCBmb3IgZG9tYWluLCBzbyB0aGUNCiAgICAtCSAq
IGRvbWFpbiBudW1iZXIgd2lsbCBub3QgY2hhbmdlIGFmdGVyIHRoZSBmaXJzdCBkZXZpY2UgaXMg
YWRkZWQuDQogICAgLQkgKi8NCiAgICAtCWlmIChsaXN0X2VtcHR5KCZoYnVzLT5jaGlsZHJlbikp
DQogICAgLQkJaGJ1cy0+c3lzZGF0YS5kb21haW4gPSBkZXNjLT5zZXI7DQogICAgICAJbGlzdF9h
ZGRfdGFpbCgmaHBkZXYtPmxpc3RfZW50cnksICZoYnVzLT5jaGlsZHJlbik7DQogICAgICAJc3Bp
bl91bmxvY2tfaXJxcmVzdG9yZSgmaGJ1cy0+ZGV2aWNlX2xpc3RfbG9jaywgZmxhZ3MpOw0KICAg
ICAgCXJldHVybiBocGRldjsNCiAgICAtLQ0KICAgIDIuNy40IA0KICAgIA0KICAgIA0KDQo=
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
2018-03-20 17:56 ` Sridhar Pitchai
@ 2018-03-20 18:32 ` Lorenzo Pieralisi
-1 siblings, 0 replies; 26+ messages in thread
From: Lorenzo Pieralisi @ 2018-03-20 18:32 UTC (permalink / raw)
To: Sridhar Pitchai
Cc: Stephen Hemminger, linux-pci, Haiyang Zhang, linux-kernel,
Michael Kelley (EOSG),
Jake Oshins, Bjorn Helgaas, devel
On Tue, Mar 20, 2018 at 05:56:15PM +0000, Sridhar Pitchai wrote:
> Hi Lorenzo,
> Are we good with the explanation? Can I send the patch with the
> updated commit comments?
Almost.
[...]
> Since we have the transparent SRIOV mode now, the short VF device name
> is no longer needed.
Can you correlate transparent SRIOV mode to the point you are making
below ? Please explain what transparent SRIOV mode allows you to remove
and why. The rest of the explanation seems OK.
Please follow this email format:
http://vger.kernel.org/lkml/#s3-9
Thanks,
Lorenzo
> I still do not understand what this means and how it is related to the
> patch below, it may be clear to you, it is not to me, at all.
>
> Sridhar >> the patch below, was introduced to make the device name small, by taking only
> 16bits of the serial number. Since we are not going to have the serial number
> updated to the BUS id, this has to be removed.
>
> Fixes: 4a9b0933bdfc("PCI:hv:Use device serial number as PCI domain")
>
> Fixes: 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI domain")
> Sridhr >> yes
>
> I asked you an explicit question. Commit above was added for a reason
> I assume. This patch implies that kernel has been broken since v4.11
> which is almost a year ago and nobody every noticed ? Or there are
> systems where commit above is _necessary_ and this patch would break
> them ?
>
> I want a detailed explanation that highlights *why* it is safe to apply
> this patch and send it to stable kernels, commit log above won't do.
>
> Sridhar>> HyperV provides a unique domain ID for PCI BUS. But it is modified by the child
> device when it is added. This cannot produce a unique domain ID all the time.
> Here in the bug, we see the collision between the serial number and already
> existing PCI bus. The cleaner way is never touch the domain ID provided by
> hyperV during the PCI bus creation. As long as hyperV make sure it provides a
> unique domain ID for the PCI for a VM it will not break, and HyperV will
> guarantees that the domain for the PCI bus for a given VM will be always unique.
> The original patch was also intending to have a unique domain ID for the PCI
> bus, by taking the serial number of the device, but it is not sufficient, when
> the device serial number is number which is the domain ID of the existing PCI
> bus. With the current kernel we can repro this issue by adding a device with a
> serial number matching the existing PCI bus domain id. (in this case that
> happens to be zero).
>
>
> Thanks,
> Lorenzo
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Sridhar Pitchai <srpitcha@microsoft.com>
> ---
> Changes in v3:
> * fix the commit comment. [KY Srinivasan, Michael Kelley]
> ---
> 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 2faf38e..ac67e56 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.7.4
>
>
>
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
@ 2018-03-20 18:32 ` Lorenzo Pieralisi
0 siblings, 0 replies; 26+ messages in thread
From: Lorenzo Pieralisi @ 2018-03-20 18:32 UTC (permalink / raw)
To: Sridhar Pitchai
Cc: Bjorn Helgaas, Jake Oshins, Haiyang Zhang, Stephen Hemminger,
Dexuan Cui, KY Srinivasan, Michael Kelley (EOSG),
devel, linux-pci, linux-kernel
On Tue, Mar 20, 2018 at 05:56:15PM +0000, Sridhar Pitchai wrote:
> Hi Lorenzo,
> Are we good with the explanation? Can I send the patch with the
> updated commit comments?
Almost.
[...]
> Since we have the transparent SRIOV mode now, the short VF device name
> is no longer needed.
Can you correlate transparent SRIOV mode to the point you are making
below ? Please explain what transparent SRIOV mode allows you to remove
and why. The rest of the explanation seems OK.
Please follow this email format:
http://vger.kernel.org/lkml/#s3-9
Thanks,
Lorenzo
> I still do not understand what this means and how it is related to the
> patch below, it may be clear to you, it is not to me, at all.
>
> Sridhar >> the patch below, was introduced to make the device name small, by taking only
> 16bits of the serial number. Since we are not going to have the serial number
> updated to the BUS id, this has to be removed.
>
> Fixes: 4a9b0933bdfc("PCI:hv:Use device serial number as PCI domain")
>
> Fixes: 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI domain")
> Sridhr >> yes
>
> I asked you an explicit question. Commit above was added for a reason
> I assume. This patch implies that kernel has been broken since v4.11
> which is almost a year ago and nobody every noticed ? Or there are
> systems where commit above is _necessary_ and this patch would break
> them ?
>
> I want a detailed explanation that highlights *why* it is safe to apply
> this patch and send it to stable kernels, commit log above won't do.
>
> Sridhar>> HyperV provides a unique domain ID for PCI BUS. But it is modified by the child
> device when it is added. This cannot produce a unique domain ID all the time.
> Here in the bug, we see the collision between the serial number and already
> existing PCI bus. The cleaner way is never touch the domain ID provided by
> hyperV during the PCI bus creation. As long as hyperV make sure it provides a
> unique domain ID for the PCI for a VM it will not break, and HyperV will
> guarantees that the domain for the PCI bus for a given VM will be always unique.
> The original patch was also intending to have a unique domain ID for the PCI
> bus, by taking the serial number of the device, but it is not sufficient, when
> the device serial number is number which is the domain ID of the existing PCI
> bus. With the current kernel we can repro this issue by adding a device with a
> serial number matching the existing PCI bus domain id. (in this case that
> happens to be zero).
>
>
> Thanks,
> Lorenzo
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Sridhar Pitchai <srpitcha@microsoft.com>
> ---
> Changes in v3:
> * fix the commit comment. [KY Srinivasan, Michael Kelley]
> ---
> 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 2faf38e..ac67e56 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.7.4
>
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
2018-03-20 18:32 ` Lorenzo Pieralisi
@ 2018-03-20 23:00 ` Sridhar Pitchai
-1 siblings, 0 replies; 26+ messages in thread
From: Sridhar Pitchai @ 2018-03-20 23:00 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Stephen Hemminger, linux-pci, Haiyang Zhang, linux-kernel,
Michael Kelley (EOSG),
Jake Oshins, Bjorn Helgaas, devel
Hi Lorenzo,
Transparent SRIOV is exposing the NIC directly to the kernel via
para-virtual device, unlike creating a netdev and associating it with the bond
driver. Further descriptions here,
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=0c195567a8f6e82ea5535cd9f1d54a1626dd233e
Previously, when using the bond driver, unique and persistent VF NIC name
was required, so we used serial number as PCI domain which is included as
part of the VF NIC name. Transparent SRIOV mode puts VF NIC based on MAC
match as a slave of synthetic NIC, so VF NIC’s name is no longer important.
Thanks,
Sridhar
On 3/20/18, 11:32 AM, "Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com> wrote:
On Tue, Mar 20, 2018 at 05:56:15PM +0000, Sridhar Pitchai wrote:
> Hi Lorenzo,
> Are we good with the explanation? Can I send the patch with the
> updated commit comments?
Almost.
[...]
> Since we have the transparent SRIOV mode now, the short VF device name
> is no longer needed.
Can you correlate transparent SRIOV mode to the point you are making
below ? Please explain what transparent SRIOV mode allows you to remove
and why. The rest of the explanation seems OK.
Please follow this email format:
https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fvger.kernel.org%2Flkml%2F%23s3-9&data=04%7C01%7CSridhar.Pitchai%40microsoft.com%7Cc5cdcb7951f64318e52708d58e90e6f2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636571675366181738%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwifQ%3D%3D%7C-1&sdata=yBdqc4NQZsO7O9vfgJsr5olU8GfLNjF5e9EAaCb7vq4%3D&reserved=0
Thanks,
Lorenzo
> I still do not understand what this means and how it is related to the
> patch below, it may be clear to you, it is not to me, at all.
>
> Sridhar >> the patch below, was introduced to make the device name small, by taking only
> 16bits of the serial number. Since we are not going to have the serial number
> updated to the BUS id, this has to be removed.
>
> Fixes: 4a9b0933bdfc("PCI:hv:Use device serial number as PCI domain")
>
> Fixes: 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI domain")
> Sridhr >> yes
>
> I asked you an explicit question. Commit above was added for a reason
> I assume. This patch implies that kernel has been broken since v4.11
> which is almost a year ago and nobody every noticed ? Or there are
> systems where commit above is _necessary_ and this patch would break
> them ?
>
> I want a detailed explanation that highlights *why* it is safe to apply
> this patch and send it to stable kernels, commit log above won't do.
>
> Sridhar>> HyperV provides a unique domain ID for PCI BUS. But it is modified by the child
> device when it is added. This cannot produce a unique domain ID all the time.
> Here in the bug, we see the collision between the serial number and already
> existing PCI bus. The cleaner way is never touch the domain ID provided by
> hyperV during the PCI bus creation. As long as hyperV make sure it provides a
> unique domain ID for the PCI for a VM it will not break, and HyperV will
> guarantees that the domain for the PCI bus for a given VM will be always unique.
> The original patch was also intending to have a unique domain ID for the PCI
> bus, by taking the serial number of the device, but it is not sufficient, when
> the device serial number is number which is the domain ID of the existing PCI
> bus. With the current kernel we can repro this issue by adding a device with a
> serial number matching the existing PCI bus domain id. (in this case that
> happens to be zero).
>
>
> Thanks,
> Lorenzo
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Sridhar Pitchai <srpitcha@microsoft.com>
> ---
> Changes in v3:
> * fix the commit comment. [KY Srinivasan, Michael Kelley]
> ---
> 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 2faf38e..ac67e56 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.7.4
>
>
>
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
@ 2018-03-20 23:00 ` Sridhar Pitchai
0 siblings, 0 replies; 26+ messages in thread
From: Sridhar Pitchai @ 2018-03-20 23:00 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Bjorn Helgaas, Jake Oshins, Haiyang Zhang, Stephen Hemminger,
Dexuan Cui, KY Srinivasan, Michael Kelley (EOSG),
devel, linux-pci, linux-kernel
SGkgTG9yZW56bywNCiAgIFRyYW5zcGFyZW50IFNSSU9WIGlzIGV4cG9zaW5nIHRoZSBOSUMgZGly
ZWN0bHkgdG8gdGhlIGtlcm5lbCB2aWENCnBhcmEtdmlydHVhbCBkZXZpY2UsIHVubGlrZSBjcmVh
dGluZyBhIG5ldGRldiBhbmQgYXNzb2NpYXRpbmcgaXQgd2l0aCB0aGUgYm9uZA0KZHJpdmVyLiBG
dXJ0aGVyIGRlc2NyaXB0aW9ucyBoZXJlLA0KICAgIGh0dHBzOi8vZ2l0Lmtlcm5lbC5vcmcvcHVi
L3NjbS9saW51eC9rZXJuZWwvZ2l0L25leHQvbGludXgtbmV4dC5naXQvY29tbWl0Lz9pZD0wYzE5
NTU2N2E4ZjZlODJlYTU1MzVjZDlmMWQ1NGExNjI2ZGQyMzNlDQoNClByZXZpb3VzbHksIHdoZW4g
dXNpbmcgdGhlIGJvbmQgZHJpdmVyLCB1bmlxdWUgYW5kIHBlcnNpc3RlbnQgVkYgTklDIG5hbWUN
CndhcyByZXF1aXJlZCwgc28gd2UgdXNlZCBzZXJpYWwgbnVtYmVyIGFzIFBDSSBkb21haW4gd2hp
Y2ggaXMgaW5jbHVkZWQgYXMNCnBhcnQgb2YgdGhlIFZGIE5JQyBuYW1lLiAgVHJhbnNwYXJlbnQg
U1JJT1YgbW9kZSBwdXRzIFZGIE5JQyBiYXNlZCBvbiBNQUMNCm1hdGNoIGFzIGEgc2xhdmUgb2Yg
c3ludGhldGljIE5JQywgc28gVkYgTklD4oCZcyBuYW1lIGlzIG5vIGxvbmdlciBpbXBvcnRhbnQu
DQoNClRoYW5rcywNClNyaWRoYXINCg0K77u/T24gMy8yMC8xOCwgMTE6MzIgQU0sICJMb3Jlbnpv
IFBpZXJhbGlzaSIgPGxvcmVuem8ucGllcmFsaXNpQGFybS5jb20+IHdyb3RlOg0KDQogICAgT24g
VHVlLCBNYXIgMjAsIDIwMTggYXQgMDU6NTY6MTVQTSArMDAwMCwgU3JpZGhhciBQaXRjaGFpIHdy
b3RlOg0KICAgID4gSGkgTG9yZW56bywNCiAgICANCiAgICA+IEFyZSB3ZSBnb29kIHdpdGggdGhl
IGV4cGxhbmF0aW9uPyBDYW4gSSBzZW5kIHRoZSBwYXRjaCB3aXRoIHRoZQ0KICAgID4gdXBkYXRl
ZCBjb21taXQgY29tbWVudHM/DQogICAgDQogICAgQWxtb3N0Lg0KICAgIA0KICAgIFsuLi5dDQog
ICAgDQogICAgPiAgICAgU2luY2Ugd2UgaGF2ZSB0aGUgdHJhbnNwYXJlbnQgU1JJT1YgbW9kZSBu
b3csIHRoZSBzaG9ydCBWRiBkZXZpY2UgbmFtZQ0KICAgID4gICAgIGlzIG5vIGxvbmdlciBuZWVk
ZWQuDQogICAgDQogICAgQ2FuIHlvdSBjb3JyZWxhdGUgdHJhbnNwYXJlbnQgU1JJT1YgbW9kZSB0
byB0aGUgcG9pbnQgeW91IGFyZSBtYWtpbmcNCiAgICBiZWxvdyA/IFBsZWFzZSBleHBsYWluIHdo
YXQgdHJhbnNwYXJlbnQgU1JJT1YgbW9kZSBhbGxvd3MgeW91IHRvIHJlbW92ZQ0KICAgIGFuZCB3
aHkuIFRoZSByZXN0IG9mIHRoZSBleHBsYW5hdGlvbiBzZWVtcyBPSy4NCiAgICANCiAgICBQbGVh
c2UgZm9sbG93IHRoaXMgZW1haWwgZm9ybWF0Og0KICAgIA0KICAgIGh0dHBzOi8vbmEwMS5zYWZl
bGlua3MucHJvdGVjdGlvbi5vdXRsb29rLmNvbS8/dXJsPWh0dHAlM0ElMkYlMkZ2Z2VyLmtlcm5l
bC5vcmclMkZsa21sJTJGJTIzczMtOSZkYXRhPTA0JTdDMDElN0NTcmlkaGFyLlBpdGNoYWklNDBt
aWNyb3NvZnQuY29tJTdDYzVjZGNiNzk1MWY2NDMxOGU1MjcwOGQ1OGU5MGU2ZjIlN0M3MmY5ODhi
Zjg2ZjE0MWFmOTFhYjJkN2NkMDExZGI0NyU3QzElN0MwJTdDNjM2NTcxNjc1MzY2MTgxNzM4JTdD
VW5rbm93biU3Q1RXRnBiR1pzYjNkOGV5SldJam9pTUM0d0xqQXdNREFpTENKUUlqb2lWMmx1TXpJ
aUxDSkJUaUk2SWsxaGFXd2lmUSUzRCUzRCU3Qy0xJnNkYXRhPXlCZHFjNE5RWnNPN085dmZnSnNy
NW9sVThHZkxOakY1ZTlFQWFDYjd2cTQlM0QmcmVzZXJ2ZWQ9MA0KICAgIA0KICAgIFRoYW5rcywN
CiAgICBMb3JlbnpvDQogICAgDQogICAgPiAgICAgSSBzdGlsbCBkbyBub3QgdW5kZXJzdGFuZCB3
aGF0IHRoaXMgbWVhbnMgYW5kIGhvdyBpdCBpcyByZWxhdGVkIHRvIHRoZQ0KICAgID4gICAgIHBh
dGNoIGJlbG93LCBpdCBtYXkgYmUgY2xlYXIgdG8geW91LCBpdCBpcyBub3QgdG8gbWUsIGF0IGFs
bC4NCiAgICA+ICAgICANCiAgICA+ICAgICBTcmlkaGFyID4+IHRoZSBwYXRjaCBiZWxvdywgd2Fz
IGludHJvZHVjZWQgdG8gbWFrZSB0aGUgZGV2aWNlIG5hbWUgc21hbGwsIGJ5IHRha2luZyBvbmx5
DQogICAgPiAgICAgMTZiaXRzIG9mIHRoZSBzZXJpYWwgbnVtYmVyLiBTaW5jZSB3ZSBhcmUgbm90
IGdvaW5nIHRvIGhhdmUgdGhlIHNlcmlhbCBudW1iZXINCiAgICA+ICAgICB1cGRhdGVkIHRvIHRo
ZSBCVVMgaWQsIHRoaXMgaGFzIHRvIGJlIHJlbW92ZWQuDQogICAgPiAgICAgDQogICAgPiAgICAg
Rml4ZXM6IDRhOWIwOTMzYmRmYygiUENJOmh2OlVzZSBkZXZpY2Ugc2VyaWFsIG51bWJlciBhcyBQ
Q0kgZG9tYWluIikNCiAgICA+ICAgICANCiAgICA+ICAgICBGaXhlczogNGE5YjA5MzNiZGZjICgi
UENJOiBodjogVXNlIGRldmljZSBzZXJpYWwgbnVtYmVyIGFzIFBDSSBkb21haW4iKQ0KICAgID4g
ICAgIFNyaWRociA+PiB5ZXMNCiAgICA+ICAgICANCiAgICA+ICAgICBJIGFza2VkIHlvdSBhbiBl
eHBsaWNpdCBxdWVzdGlvbi4gQ29tbWl0IGFib3ZlIHdhcyBhZGRlZCBmb3IgYSByZWFzb24NCiAg
ICA+ICAgICBJIGFzc3VtZS4gVGhpcyBwYXRjaCBpbXBsaWVzIHRoYXQga2VybmVsIGhhcyBiZWVu
IGJyb2tlbiBzaW5jZSB2NC4xMQ0KICAgID4gICAgIHdoaWNoIGlzIGFsbW9zdCBhIHllYXIgYWdv
IGFuZCBub2JvZHkgZXZlcnkgbm90aWNlZCA/IE9yIHRoZXJlIGFyZQ0KICAgID4gICAgIHN5c3Rl
bXMgd2hlcmUgY29tbWl0IGFib3ZlIGlzIF9uZWNlc3NhcnlfIGFuZCB0aGlzIHBhdGNoIHdvdWxk
IGJyZWFrDQogICAgPiAgICAgdGhlbSA/DQogICAgPiAgICAgDQogICAgPiAgICAgSSB3YW50IGEg
ZGV0YWlsZWQgZXhwbGFuYXRpb24gdGhhdCBoaWdobGlnaHRzICp3aHkqIGl0IGlzIHNhZmUgdG8g
YXBwbHkNCiAgICA+ICAgICB0aGlzIHBhdGNoIGFuZCBzZW5kIGl0IHRvIHN0YWJsZSBrZXJuZWxz
LCBjb21taXQgbG9nIGFib3ZlIHdvbid0IGRvLg0KICAgID4gICAgIA0KICAgID4gICAgIFNyaWRo
YXI+PiBIeXBlclYgcHJvdmlkZXMgYSB1bmlxdWUgZG9tYWluIElEIGZvciBQQ0kgQlVTLiBCdXQg
aXQgaXMgbW9kaWZpZWQgYnkgdGhlIGNoaWxkDQogICAgPiAgICAgZGV2aWNlIHdoZW4gaXQgaXMg
YWRkZWQuIFRoaXMgY2Fubm90IHByb2R1Y2UgYSB1bmlxdWUgZG9tYWluIElEIGFsbCB0aGUgdGlt
ZS4NCiAgICA+ICAgICBIZXJlIGluIHRoZSBidWcsIHdlIHNlZSB0aGUgY29sbGlzaW9uIGJldHdl
ZW4gdGhlIHNlcmlhbCBudW1iZXIgYW5kIGFscmVhZHkNCiAgICA+ICAgICBleGlzdGluZyBQQ0kg
YnVzLiBUaGUgY2xlYW5lciB3YXkgaXMgbmV2ZXIgdG91Y2ggdGhlIGRvbWFpbiBJRCBwcm92aWRl
ZCBieQ0KICAgID4gICAgIGh5cGVyViBkdXJpbmcgdGhlIFBDSSBidXMgY3JlYXRpb24uIEFzIGxv
bmcgYXMgaHlwZXJWIG1ha2Ugc3VyZSBpdCBwcm92aWRlcyBhDQogICAgPiAgICAgdW5pcXVlIGRv
bWFpbiBJRCBmb3IgdGhlIFBDSSBmb3IgYSBWTSBpdCB3aWxsIG5vdCBicmVhaywgYW5kIEh5cGVy
ViB3aWxsDQogICAgPiAgICAgZ3VhcmFudGVlcyB0aGF0IHRoZSBkb21haW4gZm9yIHRoZSBQQ0kg
YnVzIGZvciBhIGdpdmVuIFZNIHdpbGwgYmUgYWx3YXlzIHVuaXF1ZS4NCiAgICA+ICAgICBUaGUg
b3JpZ2luYWwgcGF0Y2ggd2FzIGFsc28gaW50ZW5kaW5nIHRvIGhhdmUgYSB1bmlxdWUgZG9tYWlu
IElEIGZvciB0aGUgUENJDQogICAgPiAgICAgYnVzLCBieSB0YWtpbmcgdGhlIHNlcmlhbCBudW1i
ZXIgb2YgdGhlIGRldmljZSwgYnV0IGl0IGlzIG5vdCBzdWZmaWNpZW50LCB3aGVuDQogICAgPiAg
ICAgdGhlIGRldmljZSBzZXJpYWwgbnVtYmVyIGlzIG51bWJlciB3aGljaCBpcyB0aGUgZG9tYWlu
IElEIG9mIHRoZSBleGlzdGluZyBQQ0kNCiAgICA+ICAgICBidXMuICBXaXRoIHRoZSBjdXJyZW50
IGtlcm5lbCB3ZSBjYW4gcmVwcm8gdGhpcyBpc3N1ZSBieSBhZGRpbmcgYSBkZXZpY2Ugd2l0aCBh
DQogICAgPiAgICAgc2VyaWFsIG51bWJlciBtYXRjaGluZyB0aGUgZXhpc3RpbmcgUENJIGJ1cyBk
b21haW4gaWQuIChpbiB0aGlzIGNhc2UgdGhhdA0KICAgID4gICAgIGhhcHBlbnMgdG8gYmUgemVy
bykuDQogICAgPiAgICAgDQogICAgPiAgICAgDQogICAgPiAgICAgVGhhbmtzLA0KICAgID4gICAg
IExvcmVuem8NCiAgICA+ICAgICANCiAgICA+ICAgICBDYzogc3RhYmxlQHZnZXIua2VybmVsLm9y
Zw0KICAgID4gICAgIFNpZ25lZC1vZmYtYnk6IFNyaWRoYXIgUGl0Y2hhaSA8c3JwaXRjaGFAbWlj
cm9zb2Z0LmNvbT4NCiAgICA+ICAgICAtLS0NCiAgICA+ICAgICBDaGFuZ2VzIGluIHYzOg0KICAg
ID4gICAgICogZml4IHRoZSBjb21taXQgY29tbWVudC4gW0tZIFNyaW5pdmFzYW4sIE1pY2hhZWwg
S2VsbGV5XQ0KICAgID4gICAgIC0tLQ0KICAgID4gICAgICAgZHJpdmVycy9wY2kvaG9zdC9wY2kt
aHlwZXJ2LmMgfCAxMSAtLS0tLS0tLS0tLQ0KICAgID4gICAgICAgMSBmaWxlIGNoYW5nZWQsIDEx
IGRlbGV0aW9ucygtKQ0KICAgID4gICAgIGRpZmYgLS1naXQgYS9kcml2ZXJzL3BjaS9ob3N0L3Bj
aS1oeXBlcnYuYyBiL2RyaXZlcnMvcGNpL2hvc3QvcGNpLWh5cGVydi5jDQogICAgPiAgICAgaW5k
ZXggMmZhZjM4ZS4uYWM2N2U1NiAxMDA2NDQNCiAgICA+ICAgICAtLS0gYS9kcml2ZXJzL3BjaS9o
b3N0L3BjaS1oeXBlcnYuYw0KICAgID4gICAgICsrKyBiL2RyaXZlcnMvcGNpL2hvc3QvcGNpLWh5
cGVydi5jDQogICAgPiAgICAgQEAgLTE1MTgsMTcgKzE1MTgsNiBAQCBzdGF0aWMgc3RydWN0IGh2
X3BjaV9kZXYgKm5ld19wY2ljaGlsZF9kZXZpY2Uoc3RydWN0IGh2X3BjaWJ1c19kZXZpY2UgKmhi
dXMsDQogICAgPiAgICAgICAJZ2V0X3BjaWNoaWxkKGhwZGV2LCBodl9wY2lkZXZfcmVmX2NoaWxk
bGlzdCk7DQogICAgPiAgICAgICAJc3Bpbl9sb2NrX2lycXNhdmUoJmhidXMtPmRldmljZV9saXN0
X2xvY2ssIGZsYWdzKTsNCiAgICA+ICAgICAgIA0KICAgID4gICAgIC0JLyoNCiAgICA+ICAgICAt
CSAqIFdoZW4gYSBkZXZpY2UgaXMgYmVpbmcgYWRkZWQgdG8gdGhlIGJ1cywgd2Ugc2V0IHRoZSBQ
Q0kgZG9tYWluDQogICAgPiAgICAgLQkgKiBudW1iZXIgdG8gYmUgdGhlIGRldmljZSBzZXJpYWwg
bnVtYmVyLCB3aGljaCBpcyBub24temVybyBhbmQNCiAgICA+ICAgICAtCSAqIHVuaXF1ZSBvbiB0
aGUgc2FtZSBWTS4gIFRoZSBzZXJpYWwgbnVtYmVycyBzdGFydCB3aXRoIDEsIGFuZA0KICAgID4g
ICAgIC0JICogaW5jcmVhc2UgYnkgMSBmb3IgZWFjaCBkZXZpY2UuICBTbyBkZXZpY2UgbmFtZXMg
aW5jbHVkaW5nIHRoaXMNCiAgICA+ICAgICAtCSAqIGNhbiBoYXZlIHNob3J0ZXIgbmFtZXMgdGhh
biBiYXNlZCBvbiB0aGUgYnVzIGluc3RhbmNlIFVVSUQuDQogICAgPiAgICAgLQkgKiBPbmx5IHRo
ZSBmaXJzdCBkZXZpY2Ugc2VyaWFsIG51bWJlciBpcyB1c2VkIGZvciBkb21haW4sIHNvIHRoZQ0K
ICAgID4gICAgIC0JICogZG9tYWluIG51bWJlciB3aWxsIG5vdCBjaGFuZ2UgYWZ0ZXIgdGhlIGZp
cnN0IGRldmljZSBpcyBhZGRlZC4NCiAgICA+ICAgICAtCSAqLw0KICAgID4gICAgIC0JaWYgKGxp
c3RfZW1wdHkoJmhidXMtPmNoaWxkcmVuKSkNCiAgICA+ICAgICAtCQloYnVzLT5zeXNkYXRhLmRv
bWFpbiA9IGRlc2MtPnNlcjsNCiAgICA+ICAgICAgIAlsaXN0X2FkZF90YWlsKCZocGRldi0+bGlz
dF9lbnRyeSwgJmhidXMtPmNoaWxkcmVuKTsNCiAgICA+ICAgICAgIAlzcGluX3VubG9ja19pcnFy
ZXN0b3JlKCZoYnVzLT5kZXZpY2VfbGlzdF9sb2NrLCBmbGFncyk7DQogICAgPiAgICAgICAJcmV0
dXJuIGhwZGV2Ow0KICAgID4gICAgIC0tDQogICAgPiAgICAgMi43LjQgDQogICAgPiAgICAgDQog
ICAgPiAgICAgDQogICAgPiANCiAgICANCg0K
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
2018-03-20 23:00 ` Sridhar Pitchai
@ 2018-03-21 16:26 ` Lorenzo Pieralisi
-1 siblings, 0 replies; 26+ messages in thread
From: Lorenzo Pieralisi @ 2018-03-21 16:26 UTC (permalink / raw)
To: Sridhar Pitchai
Cc: Stephen Hemminger, linux-pci, Haiyang Zhang, linux-kernel,
Michael Kelley (EOSG),
Jake Oshins, Bjorn Helgaas, devel
On Tue, Mar 20, 2018 at 11:00:36PM +0000, Sridhar Pitchai wrote:
> Hi Lorenzo,
> Transparent SRIOV is exposing the NIC directly to the kernel via
> para-virtual device, unlike creating a netdev and associating it
> with the bond driver. Further descriptions here,
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=0c195567a8f6e82ea5535cd9f1d54a1626dd233e
>
> Previously, when using the bond driver, unique and persistent VF NIC
> name was required, so we used serial number as PCI domain which is
> included as part of the VF NIC name. Transparent SRIOV mode puts VF
> NIC based on MAC match as a slave of synthetic NIC, so VF NIC’s name
> is no longer important.
Please read the link I sent you in relation to email formatting.
Then add your description above in a way that anyone not 100% familiar
with hyperv can understand it - that's what the commit log is for.
You are sending this patch to stable kernels, patch above has been in
the kernel from v4.14. The patch you are fixing since v4.11, you ought
to be careful since you do not want to have broken kernel versions owing
to stable patches mismatches, that's why I asked and I will ask again,
are you sure you won't trigger a regression by sending this fix to
stable ?
I assume the bond driver mechanism is now done and dusted.
Thanks,
Lorenzo
> Thanks,
> Sridhar
>
> On 3/20/18, 11:32 AM, "Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com> wrote:
>
> On Tue, Mar 20, 2018 at 05:56:15PM +0000, Sridhar Pitchai wrote:
> > Hi Lorenzo,
>
> > Are we good with the explanation? Can I send the patch with the
> > updated commit comments?
>
> Almost.
>
> [...]
>
> > Since we have the transparent SRIOV mode now, the short VF device name
> > is no longer needed.
>
> Can you correlate transparent SRIOV mode to the point you are making
> below ? Please explain what transparent SRIOV mode allows you to remove
> and why. The rest of the explanation seems OK.
>
> Please follow this email format:
>
> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fvger.kernel.org%2Flkml%2F%23s3-9&data=04%7C01%7CSridhar.Pitchai%40microsoft.com%7Cc5cdcb7951f64318e52708d58e90e6f2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636571675366181738%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwifQ%3D%3D%7C-1&sdata=yBdqc4NQZsO7O9vfgJsr5olU8GfLNjF5e9EAaCb7vq4%3D&reserved=0
>
> Thanks,
> Lorenzo
>
> > I still do not understand what this means and how it is related to the
> > patch below, it may be clear to you, it is not to me, at all.
> >
> > Sridhar >> the patch below, was introduced to make the device name small, by taking only
> > 16bits of the serial number. Since we are not going to have the serial number
> > updated to the BUS id, this has to be removed.
> >
> > Fixes: 4a9b0933bdfc("PCI:hv:Use device serial number as PCI domain")
> >
> > Fixes: 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI domain")
> > Sridhr >> yes
> >
> > I asked you an explicit question. Commit above was added for a reason
> > I assume. This patch implies that kernel has been broken since v4.11
> > which is almost a year ago and nobody every noticed ? Or there are
> > systems where commit above is _necessary_ and this patch would break
> > them ?
> >
> > I want a detailed explanation that highlights *why* it is safe to apply
> > this patch and send it to stable kernels, commit log above won't do.
> >
> > Sridhar>> HyperV provides a unique domain ID for PCI BUS. But it is modified by the child
> > device when it is added. This cannot produce a unique domain ID all the time.
> > Here in the bug, we see the collision between the serial number and already
> > existing PCI bus. The cleaner way is never touch the domain ID provided by
> > hyperV during the PCI bus creation. As long as hyperV make sure it provides a
> > unique domain ID for the PCI for a VM it will not break, and HyperV will
> > guarantees that the domain for the PCI bus for a given VM will be always unique.
> > The original patch was also intending to have a unique domain ID for the PCI
> > bus, by taking the serial number of the device, but it is not sufficient, when
> > the device serial number is number which is the domain ID of the existing PCI
> > bus. With the current kernel we can repro this issue by adding a device with a
> > serial number matching the existing PCI bus domain id. (in this case that
> > happens to be zero).
> >
> >
> > Thanks,
> > Lorenzo
> >
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Sridhar Pitchai <srpitcha@microsoft.com>
> > ---
> > Changes in v3:
> > * fix the commit comment. [KY Srinivasan, Michael Kelley]
> > ---
> > 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 2faf38e..ac67e56 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.7.4
> >
> >
> >
>
>
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
@ 2018-03-21 16:26 ` Lorenzo Pieralisi
0 siblings, 0 replies; 26+ messages in thread
From: Lorenzo Pieralisi @ 2018-03-21 16:26 UTC (permalink / raw)
To: Sridhar Pitchai
Cc: Bjorn Helgaas, Jake Oshins, Haiyang Zhang, Stephen Hemminger,
Dexuan Cui, KY Srinivasan, Michael Kelley (EOSG),
devel, linux-pci, linux-kernel
On Tue, Mar 20, 2018 at 11:00:36PM +0000, Sridhar Pitchai wrote:
> Hi Lorenzo,
> Transparent SRIOV is exposing the NIC directly to the kernel via
> para-virtual device, unlike creating a netdev and associating it
> with the bond driver. Further descriptions here,
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=0c195567a8f6e82ea5535cd9f1d54a1626dd233e
>
> Previously, when using the bond driver, unique and persistent VF NIC
> name was required, so we used serial number as PCI domain which is
> included as part of the VF NIC name. Transparent SRIOV mode puts VF
> NIC based on MAC match as a slave of synthetic NIC, so VF NIC’s name
> is no longer important.
Please read the link I sent you in relation to email formatting.
Then add your description above in a way that anyone not 100% familiar
with hyperv can understand it - that's what the commit log is for.
You are sending this patch to stable kernels, patch above has been in
the kernel from v4.14. The patch you are fixing since v4.11, you ought
to be careful since you do not want to have broken kernel versions owing
to stable patches mismatches, that's why I asked and I will ask again,
are you sure you won't trigger a regression by sending this fix to
stable ?
I assume the bond driver mechanism is now done and dusted.
Thanks,
Lorenzo
> Thanks,
> Sridhar
>
> On 3/20/18, 11:32 AM, "Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com> wrote:
>
> On Tue, Mar 20, 2018 at 05:56:15PM +0000, Sridhar Pitchai wrote:
> > Hi Lorenzo,
>
> > Are we good with the explanation? Can I send the patch with the
> > updated commit comments?
>
> Almost.
>
> [...]
>
> > Since we have the transparent SRIOV mode now, the short VF device name
> > is no longer needed.
>
> Can you correlate transparent SRIOV mode to the point you are making
> below ? Please explain what transparent SRIOV mode allows you to remove
> and why. The rest of the explanation seems OK.
>
> Please follow this email format:
>
> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fvger.kernel.org%2Flkml%2F%23s3-9&data=04%7C01%7CSridhar.Pitchai%40microsoft.com%7Cc5cdcb7951f64318e52708d58e90e6f2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636571675366181738%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwifQ%3D%3D%7C-1&sdata=yBdqc4NQZsO7O9vfgJsr5olU8GfLNjF5e9EAaCb7vq4%3D&reserved=0
>
> Thanks,
> Lorenzo
>
> > I still do not understand what this means and how it is related to the
> > patch below, it may be clear to you, it is not to me, at all.
> >
> > Sridhar >> the patch below, was introduced to make the device name small, by taking only
> > 16bits of the serial number. Since we are not going to have the serial number
> > updated to the BUS id, this has to be removed.
> >
> > Fixes: 4a9b0933bdfc("PCI:hv:Use device serial number as PCI domain")
> >
> > Fixes: 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI domain")
> > Sridhr >> yes
> >
> > I asked you an explicit question. Commit above was added for a reason
> > I assume. This patch implies that kernel has been broken since v4.11
> > which is almost a year ago and nobody every noticed ? Or there are
> > systems where commit above is _necessary_ and this patch would break
> > them ?
> >
> > I want a detailed explanation that highlights *why* it is safe to apply
> > this patch and send it to stable kernels, commit log above won't do.
> >
> > Sridhar>> HyperV provides a unique domain ID for PCI BUS. But it is modified by the child
> > device when it is added. This cannot produce a unique domain ID all the time.
> > Here in the bug, we see the collision between the serial number and already
> > existing PCI bus. The cleaner way is never touch the domain ID provided by
> > hyperV during the PCI bus creation. As long as hyperV make sure it provides a
> > unique domain ID for the PCI for a VM it will not break, and HyperV will
> > guarantees that the domain for the PCI bus for a given VM will be always unique.
> > The original patch was also intending to have a unique domain ID for the PCI
> > bus, by taking the serial number of the device, but it is not sufficient, when
> > the device serial number is number which is the domain ID of the existing PCI
> > bus. With the current kernel we can repro this issue by adding a device with a
> > serial number matching the existing PCI bus domain id. (in this case that
> > happens to be zero).
> >
> >
> > Thanks,
> > Lorenzo
> >
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Sridhar Pitchai <srpitcha@microsoft.com>
> > ---
> > Changes in v3:
> > * fix the commit comment. [KY Srinivasan, Michael Kelley]
> > ---
> > 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 2faf38e..ac67e56 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.7.4
> >
> >
> >
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
2018-03-20 23:00 ` Sridhar Pitchai
@ 2018-03-21 17:30 ` Bjorn Helgaas
-1 siblings, 0 replies; 26+ messages in thread
From: Bjorn Helgaas @ 2018-03-21 17:30 UTC (permalink / raw)
To: Sridhar Pitchai
Cc: Lorenzo Pieralisi, Stephen Hemminger, linux-pci, Haiyang Zhang,
linux-kernel, Michael Kelley (EOSG),
Jake Oshins, Bjorn Helgaas, devel
[I composed most of this before seeing Lorenzo's response, so sorry
about the duplication. Maybe seeing things stated a different way
will help :)]
On Tue, Mar 20, 2018 at 11:00:36PM +0000, Sridhar Pitchai wrote:
> Hi Lorenzo,
> Transparent SRIOV is exposing the NIC directly to the kernel via
> para-virtual device, unlike creating a netdev and associating it with the bond
> driver. Further descriptions here,
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=0c195567a8f6e82ea5535cd9f1d54a1626dd233e
>
> Previously, when using the bond driver, unique and persistent VF NIC name
> was required, so we used serial number as PCI domain which is included as
> part of the VF NIC name. Transparent SRIOV mode puts VF NIC based on MAC
> match as a slave of synthetic NIC, so VF NIC’s name is no longer important.
Hi Sridhar,
A few hints about submitting patches more efficiently:
1) You never have to ask "Are we OK with the explanation? If so, I'll
send a patch with updated changelog." That forces an extra
round-trip. Simply post a new version with your proposed update. If
Lorenzo has more questions, he'll say so and you can do another
version.
2) When Lorenzo is asking for clarification, he's not really asking
for the clarification in an email response, because the email thread
will soon be forgotten and lost in the archives. What we really want
is for the permanent git changelog to make sense to someone in the
future. The easiest way is to post a new patch version with a
revised changelog that answers the questions.
3) Please capitalize and punctuate consistently with previous history,
e.g., "PCI: hv: Fix domain ID corruption" for your title, "SR-IOV"
(not "SRIOV") and "bus" (not "BUS") in changelog. Both "para-virtual"
and "paravirtual" are used in the kernel, but "paravirtual" is much
more common. Run "git log" and "git log --oneline" on your file and
follow the same style.
4) When you reference a previous commit, please use this style:
0c195567a8f6 ("netvsc: transparent VF management"), i.e., 12-char SHA1
followed by title. You seem to have removed some spaces from the
commit you mention in the "Fixes" tag.
And a few content questions/observations of my own:
1) "Fix domain ID corruption" isn't a very good title because it
suggests you're fixing a memory corruption or similar defect. But in
fact, I think you're removing something that used to be a feature
(added by 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI
domain")) but is now no longer needed and in fact now causes a
problem.
2) Your changelog does mention 4a9b0933bdfc, which is good, but
there must be some other <commit X> that makes it safe to remove
4a9b0933bdfc, i.e., <commit X> removes the need for using the device
serial number as the PCI domain. <Commit X> *must* be mentioned in
the changelog. Otherwise, people may backport this patch to a kernel
that doesn't include <commit X>, and things will break.
3) I don't understand what you mean by "transparent SR-IOV mode". Is
that something different than regular SR-IOV? If so, what exactly is
the difference? I don't think the PCIe specs mention a "transparent
mode", so is it a Hyper-V thing? It seems important, but I don't see
any pci-hyperv.c commits that mention it.
Here's a stab at the sort of changelog I would be looking for.
Obviously I don't understand much about Hyper-V and pci-hyperv.c, so
please correct the things I got wrong:
When Linux runs as a guest in a Hyper-V VM, pci-hyperv.c
paravirtualizes access to PCI devices assigned to the guest. For
each of those devices, hv_pci_probe() creates a virtual PCI bus in
its own unique PCI domain.
4a9b0933bdfc ("PCI: hv: Use device serial number as PCI domain")
overrode that unique PCI domain to be the Hyper-V device serial
number to make device names more convenient <or whatever the real
reason is; I don't quite understand this part>.
One problem with 4a9b0933bdfc is that the Hyper-V device serial
number is not necessarily unique, so we may end up with two buses
with the same domain and bus number, and adding the second bus
fails.
We no longer need to override the PCI domain numbers because <commit
X> removed the need for that.
Revert 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 X>.
Bjorn
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
@ 2018-03-21 17:30 ` Bjorn Helgaas
0 siblings, 0 replies; 26+ messages in thread
From: Bjorn Helgaas @ 2018-03-21 17:30 UTC (permalink / raw)
To: Sridhar Pitchai
Cc: Lorenzo Pieralisi, Bjorn Helgaas, Jake Oshins, Haiyang Zhang,
Stephen Hemminger, Dexuan Cui, KY Srinivasan,
Michael Kelley (EOSG),
devel, linux-pci, linux-kernel
[I composed most of this before seeing Lorenzo's response, so sorry
about the duplication. Maybe seeing things stated a different way
will help :)]
On Tue, Mar 20, 2018 at 11:00:36PM +0000, Sridhar Pitchai wrote:
> Hi Lorenzo,
> Transparent SRIOV is exposing the NIC directly to the kernel via
> para-virtual device, unlike creating a netdev and associating it with the bond
> driver. Further descriptions here,
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=0c195567a8f6e82ea5535cd9f1d54a1626dd233e
>
> Previously, when using the bond driver, unique and persistent VF NIC name
> was required, so we used serial number as PCI domain which is included as
> part of the VF NIC name. Transparent SRIOV mode puts VF NIC based on MAC
> match as a slave of synthetic NIC, so VF NIC’s name is no longer important.
Hi Sridhar,
A few hints about submitting patches more efficiently:
1) You never have to ask "Are we OK with the explanation? If so, I'll
send a patch with updated changelog." That forces an extra
round-trip. Simply post a new version with your proposed update. If
Lorenzo has more questions, he'll say so and you can do another
version.
2) When Lorenzo is asking for clarification, he's not really asking
for the clarification in an email response, because the email thread
will soon be forgotten and lost in the archives. What we really want
is for the permanent git changelog to make sense to someone in the
future. The easiest way is to post a new patch version with a
revised changelog that answers the questions.
3) Please capitalize and punctuate consistently with previous history,
e.g., "PCI: hv: Fix domain ID corruption" for your title, "SR-IOV"
(not "SRIOV") and "bus" (not "BUS") in changelog. Both "para-virtual"
and "paravirtual" are used in the kernel, but "paravirtual" is much
more common. Run "git log" and "git log --oneline" on your file and
follow the same style.
4) When you reference a previous commit, please use this style:
0c195567a8f6 ("netvsc: transparent VF management"), i.e., 12-char SHA1
followed by title. You seem to have removed some spaces from the
commit you mention in the "Fixes" tag.
And a few content questions/observations of my own:
1) "Fix domain ID corruption" isn't a very good title because it
suggests you're fixing a memory corruption or similar defect. But in
fact, I think you're removing something that used to be a feature
(added by 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI
domain")) but is now no longer needed and in fact now causes a
problem.
2) Your changelog does mention 4a9b0933bdfc, which is good, but
there must be some other <commit X> that makes it safe to remove
4a9b0933bdfc, i.e., <commit X> removes the need for using the device
serial number as the PCI domain. <Commit X> *must* be mentioned in
the changelog. Otherwise, people may backport this patch to a kernel
that doesn't include <commit X>, and things will break.
3) I don't understand what you mean by "transparent SR-IOV mode". Is
that something different than regular SR-IOV? If so, what exactly is
the difference? I don't think the PCIe specs mention a "transparent
mode", so is it a Hyper-V thing? It seems important, but I don't see
any pci-hyperv.c commits that mention it.
Here's a stab at the sort of changelog I would be looking for.
Obviously I don't understand much about Hyper-V and pci-hyperv.c, so
please correct the things I got wrong:
When Linux runs as a guest in a Hyper-V VM, pci-hyperv.c
paravirtualizes access to PCI devices assigned to the guest. For
each of those devices, hv_pci_probe() creates a virtual PCI bus in
its own unique PCI domain.
4a9b0933bdfc ("PCI: hv: Use device serial number as PCI domain")
overrode that unique PCI domain to be the Hyper-V device serial
number to make device names more convenient <or whatever the real
reason is; I don't quite understand this part>.
One problem with 4a9b0933bdfc is that the Hyper-V device serial
number is not necessarily unique, so we may end up with two buses
with the same domain and bus number, and adding the second bus
fails.
We no longer need to override the PCI domain numbers because <commit
X> removed the need for that.
Revert 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 X>.
Bjorn
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
2018-03-21 17:30 ` Bjorn Helgaas
@ 2018-03-23 16:09 ` Sridhar Pitchai
-1 siblings, 0 replies; 26+ messages in thread
From: Sridhar Pitchai @ 2018-03-23 16:09 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Lorenzo Pieralisi, Stephen Hemminger, linux-pci, Haiyang Zhang,
linux-kernel, Michael Kelley (EOSG),
Jake Oshins, Bjorn Helgaas, devel
> Previously, when using the bond driver, unique and persistent VF NIC name
> was required, so we used serial number as PCI domain which is included as
> part of the VF NIC name. Transparent SRIOV mode puts VF NIC based on MAC
> match as a slave of synthetic NIC, so VF NIC’s name is no longer important.
Hi Sridhar,
A few hints about submitting patches more efficiently:
1) You never have to ask "Are we OK with the explanation? If so, I'll
send a patch with updated changelog." That forces an extra
round-trip. Simply post a new version with your proposed update. If
Lorenzo has more questions, he'll say so and you can do another
version.
2) When Lorenzo is asking for clarification, he's not really asking
for the clarification in an email response, because the email thread
will soon be forgotten and lost in the archives. What we really want
is for the permanent git changelog to make sense to someone in the
future. The easiest way is to post a new patch version with a
revised changelog that answers the questions.
3) Please capitalize and punctuate consistently with previous history,
e.g., "PCI: hv: Fix domain ID corruption" for your title, "SR-IOV"
(not "SRIOV") and "bus" (not "BUS") in changelog. Both "para-virtual"
and "paravirtual" are used in the kernel, but "paravirtual" is much
more common. Run "git log" and "git log --oneline" on your file and
follow the same style.
4) When you reference a previous commit, please use this style:
0c195567a8f6 ("netvsc: transparent VF management"), i.e., 12-char SHA1
followed by title. You seem to have removed some spaces from the
commit you mention in the "Fixes" tag.
And a few content questions/observations of my own:
1) "Fix domain ID corruption" isn't a very good title because it
suggests you're fixing a memory corruption or similar defect. But in
fact, I think you're removing something that used to be a feature
(added by 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI
domain")) but is now no longer needed and in fact now causes a
problem.
2) Your changelog does mention 4a9b0933bdfc, which is good, but
there must be some other <commit X> that makes it safe to remove
4a9b0933bdfc, i.e., <commit X> removes the need for using the device
serial number as the PCI domain. <Commit X> *must* be mentioned in
the changelog. Otherwise, people may backport this patch to a kernel
that doesn't include <commit X>, and things will break.
3) I don't understand what you mean by "transparent SR-IOV mode". Is
that something different than regular SR-IOV? If so, what exactly is
the difference? I don't think the PCIe specs mention a "transparent
mode", so is it a Hyper-V thing? It seems important, but I don't see
any pci-hyperv.c commits that mention it.
Here's a stab at the sort of changelog I would be looking for.
Obviously I don't understand much about Hyper-V and pci-hyperv.c, so
please correct the things I got wrong:
When Linux runs as a guest in a Hyper-V VM, pci-hyperv.c
paravirtualizes access to PCI devices assigned to the guest. For
each of those devices, hv_pci_probe() creates a virtual PCI bus in
its own unique PCI domain.
4a9b0933bdfc ("PCI: hv: Use device serial number as PCI domain")
overrode that unique PCI domain to be the Hyper-V device serial
number to make device names more convenient <or whatever the real
reason is; I don't quite understand this part>.
One problem with 4a9b0933bdfc is that the Hyper-V device serial
number is not necessarily unique, so we may end up with two buses
with the same domain and bus number, and adding the second bus
fails.
We no longer need to override the PCI domain numbers because <commit
X> removed the need for that.
Revert 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 X>.
Bjorn
Thanks for your comments Bjorn. I took some time to go over the all the
comments and sending a new version of the patch with all the comments
incorporated.
Thanks
Sridhar
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
@ 2018-03-23 16:09 ` Sridhar Pitchai
0 siblings, 0 replies; 26+ messages in thread
From: Sridhar Pitchai @ 2018-03-23 16:09 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Lorenzo Pieralisi, Bjorn Helgaas, Jake Oshins, Haiyang Zhang,
Stephen Hemminger, Dexuan Cui, KY Srinivasan,
Michael Kelley (EOSG),
devel, linux-pci, linux-kernel
ICAgID4gUHJldmlvdXNseSwgd2hlbiB1c2luZyB0aGUgYm9uZCBkcml2ZXIsIHVuaXF1ZSBhbmQg
cGVyc2lzdGVudCBWRiBOSUMgbmFtZQ0KICAgID4gd2FzIHJlcXVpcmVkLCBzbyB3ZSB1c2VkIHNl
cmlhbCBudW1iZXIgYXMgUENJIGRvbWFpbiB3aGljaCBpcyBpbmNsdWRlZCBhcw0KICAgID4gcGFy
dCBvZiB0aGUgVkYgTklDIG5hbWUuICBUcmFuc3BhcmVudCBTUklPViBtb2RlIHB1dHMgVkYgTklD
IGJhc2VkIG9uIE1BQw0KICAgID4gbWF0Y2ggYXMgYSBzbGF2ZSBvZiBzeW50aGV0aWMgTklDLCBz
byBWRiBOSUPigJlzIG5hbWUgaXMgbm8gbG9uZ2VyIGltcG9ydGFudC4NCiAgICANCiAgICBIaSBT
cmlkaGFyLA0KICAgIA0KICAgIEEgZmV3IGhpbnRzIGFib3V0IHN1Ym1pdHRpbmcgcGF0Y2hlcyBt
b3JlIGVmZmljaWVudGx5Og0KICAgIA0KICAgIDEpIFlvdSBuZXZlciBoYXZlIHRvIGFzayAiQXJl
IHdlIE9LIHdpdGggdGhlIGV4cGxhbmF0aW9uPyAgSWYgc28sIEknbGwNCiAgICBzZW5kIGEgcGF0
Y2ggd2l0aCB1cGRhdGVkIGNoYW5nZWxvZy4iICBUaGF0IGZvcmNlcyBhbiBleHRyYQ0KICAgIHJv
dW5kLXRyaXAuICBTaW1wbHkgcG9zdCBhIG5ldyB2ZXJzaW9uIHdpdGggeW91ciBwcm9wb3NlZCB1
cGRhdGUuICBJZg0KICAgIExvcmVuem8gaGFzIG1vcmUgcXVlc3Rpb25zLCBoZSdsbCBzYXkgc28g
YW5kIHlvdSBjYW4gZG8gYW5vdGhlcg0KICAgIHZlcnNpb24uDQogICAgDQogICAgMikgV2hlbiBM
b3JlbnpvIGlzIGFza2luZyBmb3IgY2xhcmlmaWNhdGlvbiwgaGUncyBub3QgcmVhbGx5IGFza2lu
Zw0KICAgIGZvciB0aGUgY2xhcmlmaWNhdGlvbiBpbiBhbiBlbWFpbCByZXNwb25zZSwgYmVjYXVz
ZSB0aGUgZW1haWwgdGhyZWFkDQogICAgd2lsbCBzb29uIGJlIGZvcmdvdHRlbiBhbmQgbG9zdCBp
biB0aGUgYXJjaGl2ZXMuICBXaGF0IHdlIHJlYWxseSB3YW50DQogICAgaXMgZm9yIHRoZSBwZXJt
YW5lbnQgZ2l0IGNoYW5nZWxvZyB0byBtYWtlIHNlbnNlIHRvIHNvbWVvbmUgaW4gdGhlDQogICAg
ZnV0dXJlLiAgVGhlIGVhc2llc3Qgd2F5IGlzIHRvIHBvc3QgYSBuZXcgcGF0Y2ggdmVyc2lvbiB3
aXRoIGENCiAgICByZXZpc2VkIGNoYW5nZWxvZyB0aGF0IGFuc3dlcnMgdGhlIHF1ZXN0aW9ucy4N
CiAgICANCiAgICAzKSBQbGVhc2UgY2FwaXRhbGl6ZSBhbmQgcHVuY3R1YXRlIGNvbnNpc3RlbnRs
eSB3aXRoIHByZXZpb3VzIGhpc3RvcnksDQogICAgZS5nLiwgIlBDSTogaHY6IEZpeCBkb21haW4g
SUQgY29ycnVwdGlvbiIgZm9yIHlvdXIgdGl0bGUsICJTUi1JT1YiDQogICAgKG5vdCAiU1JJT1Yi
KSBhbmQgImJ1cyIgKG5vdCAiQlVTIikgaW4gY2hhbmdlbG9nLiAgQm90aCAicGFyYS12aXJ0dWFs
Ig0KICAgIGFuZCAicGFyYXZpcnR1YWwiIGFyZSB1c2VkIGluIHRoZSBrZXJuZWwsIGJ1dCAicGFy
YXZpcnR1YWwiIGlzIG11Y2gNCiAgICBtb3JlIGNvbW1vbi4gIFJ1biAiZ2l0IGxvZyIgYW5kICJn
aXQgbG9nIC0tb25lbGluZSIgb24geW91ciBmaWxlIGFuZA0KICAgIGZvbGxvdyB0aGUgc2FtZSBz
dHlsZS4NCiAgICANCiAgICA0KSBXaGVuIHlvdSByZWZlcmVuY2UgYSBwcmV2aW91cyBjb21taXQs
IHBsZWFzZSB1c2UgdGhpcyBzdHlsZToNCiAgICAwYzE5NTU2N2E4ZjYgKCJuZXR2c2M6IHRyYW5z
cGFyZW50IFZGIG1hbmFnZW1lbnQiKSwgaS5lLiwgMTItY2hhciBTSEExDQogICAgZm9sbG93ZWQg
YnkgdGl0bGUuICBZb3Ugc2VlbSB0byBoYXZlIHJlbW92ZWQgc29tZSBzcGFjZXMgZnJvbSB0aGUN
CiAgICBjb21taXQgeW91IG1lbnRpb24gaW4gdGhlICJGaXhlcyIgdGFnLg0KICAgIA0KICAgIEFu
ZCBhIGZldyBjb250ZW50IHF1ZXN0aW9ucy9vYnNlcnZhdGlvbnMgb2YgbXkgb3duOg0KICAgIA0K
ICAgIDEpICJGaXggZG9tYWluIElEIGNvcnJ1cHRpb24iIGlzbid0IGEgdmVyeSBnb29kIHRpdGxl
IGJlY2F1c2UgaXQNCiAgICBzdWdnZXN0cyB5b3UncmUgZml4aW5nIGEgbWVtb3J5IGNvcnJ1cHRp
b24gb3Igc2ltaWxhciBkZWZlY3QuICBCdXQgaW4NCiAgICBmYWN0LCBJIHRoaW5rIHlvdSdyZSBy
ZW1vdmluZyBzb21ldGhpbmcgdGhhdCB1c2VkIHRvIGJlIGEgZmVhdHVyZQ0KICAgIChhZGRlZCBi
eSA0YTliMDkzM2JkZmMgKCJQQ0k6IGh2OiBVc2UgZGV2aWNlIHNlcmlhbCBudW1iZXIgYXMgUENJ
DQogICAgZG9tYWluIikpIGJ1dCBpcyBub3cgbm8gbG9uZ2VyIG5lZWRlZCBhbmQgaW4gZmFjdCBu
b3cgY2F1c2VzIGENCiAgICBwcm9ibGVtLg0KICAgIA0KICAgIDIpIFlvdXIgY2hhbmdlbG9nIGRv
ZXMgbWVudGlvbiA0YTliMDkzM2JkZmMsIHdoaWNoIGlzIGdvb2QsIGJ1dA0KICAgIHRoZXJlIG11
c3QgYmUgc29tZSBvdGhlciA8Y29tbWl0IFg+IHRoYXQgbWFrZXMgaXQgc2FmZSB0byByZW1vdmUN
CiAgICA0YTliMDkzM2JkZmMsIGkuZS4sIDxjb21taXQgWD4gcmVtb3ZlcyB0aGUgbmVlZCBmb3Ig
dXNpbmcgdGhlIGRldmljZQ0KICAgIHNlcmlhbCBudW1iZXIgYXMgdGhlIFBDSSBkb21haW4uICA8
Q29tbWl0IFg+ICptdXN0KiBiZSBtZW50aW9uZWQgaW4NCiAgICB0aGUgY2hhbmdlbG9nLiAgT3Ro
ZXJ3aXNlLCBwZW9wbGUgbWF5IGJhY2twb3J0IHRoaXMgcGF0Y2ggdG8gYSBrZXJuZWwNCiAgICB0
aGF0IGRvZXNuJ3QgaW5jbHVkZSA8Y29tbWl0IFg+LCBhbmQgdGhpbmdzIHdpbGwgYnJlYWsuDQog
ICAgDQogICAgMykgSSBkb24ndCB1bmRlcnN0YW5kIHdoYXQgeW91IG1lYW4gYnkgInRyYW5zcGFy
ZW50IFNSLUlPViBtb2RlIi4gIElzDQogICAgdGhhdCBzb21ldGhpbmcgZGlmZmVyZW50IHRoYW4g
cmVndWxhciBTUi1JT1Y/ICBJZiBzbywgd2hhdCBleGFjdGx5IGlzDQogICAgdGhlIGRpZmZlcmVu
Y2U/ICBJIGRvbid0IHRoaW5rIHRoZSBQQ0llIHNwZWNzIG1lbnRpb24gYSAidHJhbnNwYXJlbnQN
CiAgICBtb2RlIiwgc28gaXMgaXQgYSBIeXBlci1WIHRoaW5nPyAgSXQgc2VlbXMgaW1wb3J0YW50
LCBidXQgSSBkb24ndCBzZWUNCiAgICBhbnkgcGNpLWh5cGVydi5jIGNvbW1pdHMgdGhhdCBtZW50
aW9uIGl0Lg0KICAgIA0KICAgIEhlcmUncyBhIHN0YWIgYXQgdGhlIHNvcnQgb2YgY2hhbmdlbG9n
IEkgd291bGQgYmUgbG9va2luZyBmb3IuDQogICAgT2J2aW91c2x5IEkgZG9uJ3QgdW5kZXJzdGFu
ZCBtdWNoIGFib3V0IEh5cGVyLVYgYW5kIHBjaS1oeXBlcnYuYywgc28NCiAgICBwbGVhc2UgY29y
cmVjdCB0aGUgdGhpbmdzIEkgZ290IHdyb25nOg0KICAgIA0KICAgICAgV2hlbiBMaW51eCBydW5z
IGFzIGEgZ3Vlc3QgaW4gYSBIeXBlci1WIFZNLCBwY2ktaHlwZXJ2LmMNCiAgICAgIHBhcmF2aXJ0
dWFsaXplcyBhY2Nlc3MgdG8gUENJIGRldmljZXMgYXNzaWduZWQgdG8gdGhlIGd1ZXN0LiAgRm9y
DQogICAgICBlYWNoIG9mIHRob3NlIGRldmljZXMsIGh2X3BjaV9wcm9iZSgpIGNyZWF0ZXMgYSB2
aXJ0dWFsIFBDSSBidXMgaW4NCiAgICAgIGl0cyBvd24gdW5pcXVlIFBDSSBkb21haW4uDQogICAg
DQogICAgICA0YTliMDkzM2JkZmMgKCJQQ0k6IGh2OiBVc2UgZGV2aWNlIHNlcmlhbCBudW1iZXIg
YXMgUENJIGRvbWFpbiIpDQogICAgICBvdmVycm9kZSB0aGF0IHVuaXF1ZSBQQ0kgZG9tYWluIHRv
IGJlIHRoZSBIeXBlci1WIGRldmljZSBzZXJpYWwNCiAgICAgIG51bWJlciB0byBtYWtlIGRldmlj
ZSBuYW1lcyBtb3JlIGNvbnZlbmllbnQgPG9yIHdoYXRldmVyIHRoZSByZWFsDQogICAgICByZWFz
b24gaXM7IEkgZG9uJ3QgcXVpdGUgdW5kZXJzdGFuZCB0aGlzIHBhcnQ+Lg0KICAgIA0KICAgICAg
T25lIHByb2JsZW0gd2l0aCA0YTliMDkzM2JkZmMgaXMgdGhhdCB0aGUgSHlwZXItViBkZXZpY2Ug
c2VyaWFsDQogICAgICBudW1iZXIgaXMgbm90IG5lY2Vzc2FyaWx5IHVuaXF1ZSwgc28gd2UgbWF5
IGVuZCB1cCB3aXRoIHR3byBidXNlcw0KICAgICAgd2l0aCB0aGUgc2FtZSBkb21haW4gYW5kIGJ1
cyBudW1iZXIsIGFuZCBhZGRpbmcgdGhlIHNlY29uZCBidXMNCiAgICAgIGZhaWxzLg0KICAgIA0K
ICAgICAgV2Ugbm8gbG9uZ2VyIG5lZWQgdG8gb3ZlcnJpZGUgdGhlIFBDSSBkb21haW4gbnVtYmVy
cyBiZWNhdXNlIDxjb21taXQNCiAgICAgIFg+IHJlbW92ZWQgdGhlIG5lZWQgZm9yIHRoYXQuDQog
ICAgDQogICAgICBSZXZlcnQgNGE5YjA5MzNiZGZjICgiUENJOiBodjogVXNlIGRldmljZSBzZXJp
YWwgbnVtYmVyIGFzIFBDSQ0KICAgICAgZG9tYWluIikgc28gd2UgY2FuIHJlbGlhYmx5IHN1cHBv
cnQgbXVsdGlwbGUgZGV2aWNlcyBiZWluZyBhc3NpZ25lZA0KICAgICAgdG8gYSBndWVzdC4NCiAg
ICANCiAgICAgIFRoaXMgcmV2ZXJ0IHNob3VsZCBvbmx5IGJlIGJhY2twb3J0ZWQgdG8ga2VybmVs
cyB0aGF0IGNvbnRhaW4NCiAgICAgIDxjb21taXQgWD4uDQogICAgDQogICAgQmpvcm4NCg0KVGhh
bmtzIGZvciB5b3VyIGNvbW1lbnRzIEJqb3JuLiBJIHRvb2sgc29tZSB0aW1lIHRvIGdvIG92ZXIg
dGhlIGFsbCB0aGUNCmNvbW1lbnRzIGFuZCBzZW5kaW5nIGEgbmV3IHZlcnNpb24gb2YgdGhlIHBh
dGNoIHdpdGggYWxsIHRoZSBjb21tZW50cyANCmluY29ycG9yYXRlZC4NCg0KVGhhbmtzDQpTcmlk
aGFyICAgIA0KDQo=
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
2018-03-21 16:26 ` Lorenzo Pieralisi
@ 2018-03-23 16:41 ` Sridhar Pitchai
-1 siblings, 0 replies; 26+ messages in thread
From: Sridhar Pitchai @ 2018-03-23 16:41 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Stephen Hemminger, linux-pci, Haiyang Zhang, linux-kernel,
Michael Kelley (EOSG),
Jake Oshins, Bjorn Helgaas, devel
Please read the link I sent you in relation to email formatting.
Then add your description above in a way that anyone not 100% familiar
with hyperv can understand it - that's what the commit log is for.
You are sending this patch to stable kernels, patch above has been in
the kernel from v4.14. The patch you are fixing since v4.11, you ought
to be careful since you do not want to have broken kernel versions owing
to stable patches mismatches, that's why I asked and I will ask again,
are you sure you won't trigger a regression by sending this fix to
stable ?
I assume the bond driver mechanism is now done and dusted.
That is correct. I have sent a v4 version of the patch. I am sending this
patch for stable kernel. We have tested and I am sure this should not trigger
regression by sending this fix to stable.
Thanks
Sridhar Pitchai
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
@ 2018-03-23 16:41 ` Sridhar Pitchai
0 siblings, 0 replies; 26+ messages in thread
From: Sridhar Pitchai @ 2018-03-23 16:41 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Bjorn Helgaas, Jake Oshins, Haiyang Zhang, Stephen Hemminger,
Dexuan Cui, KY Srinivasan, Michael Kelley (EOSG),
devel, linux-pci, linux-kernel
ICAgIFBsZWFzZSByZWFkIHRoZSBsaW5rIEkgc2VudCB5b3UgaW4gcmVsYXRpb24gdG8gZW1haWwg
Zm9ybWF0dGluZy4NCiAgICANCiAgICBUaGVuIGFkZCB5b3VyIGRlc2NyaXB0aW9uIGFib3ZlIGlu
IGEgd2F5IHRoYXQgYW55b25lIG5vdCAxMDAlIGZhbWlsaWFyDQogICAgd2l0aCBoeXBlcnYgY2Fu
IHVuZGVyc3RhbmQgaXQgLSB0aGF0J3Mgd2hhdCB0aGUgY29tbWl0IGxvZyBpcyBmb3IuDQogICAg
DQogICAgWW91IGFyZSBzZW5kaW5nIHRoaXMgcGF0Y2ggdG8gc3RhYmxlIGtlcm5lbHMsIHBhdGNo
IGFib3ZlIGhhcyBiZWVuIGluDQogICAgdGhlIGtlcm5lbCBmcm9tIHY0LjE0LiBUaGUgcGF0Y2gg
eW91IGFyZSBmaXhpbmcgc2luY2UgdjQuMTEsIHlvdSBvdWdodA0KICAgIHRvIGJlIGNhcmVmdWwg
c2luY2UgeW91IGRvIG5vdCB3YW50IHRvIGhhdmUgYnJva2VuIGtlcm5lbCB2ZXJzaW9ucyBvd2lu
Zw0KICAgIHRvIHN0YWJsZSBwYXRjaGVzIG1pc21hdGNoZXMsIHRoYXQncyB3aHkgSSBhc2tlZCBh
bmQgSSB3aWxsIGFzayBhZ2FpbiwNCiAgICBhcmUgeW91IHN1cmUgeW91IHdvbid0IHRyaWdnZXIg
YSByZWdyZXNzaW9uIGJ5IHNlbmRpbmcgdGhpcyBmaXggdG8NCiAgICBzdGFibGUgPw0KICAgIA0K
ICAgIEkgYXNzdW1lIHRoZSBib25kIGRyaXZlciBtZWNoYW5pc20gaXMgbm93IGRvbmUgYW5kIGR1
c3RlZC4NCiAgICANClRoYXQgaXMgY29ycmVjdC4gSSBoYXZlIHNlbnQgYSB2NCB2ZXJzaW9uIG9m
IHRoZSBwYXRjaC4gSSBhbSBzZW5kaW5nIHRoaXMNCnBhdGNoIGZvciBzdGFibGUga2VybmVsLiBX
ZSBoYXZlIHRlc3RlZCBhbmQgSSBhbSBzdXJlIHRoaXMgc2hvdWxkIG5vdCB0cmlnZ2VyDQpyZWdy
ZXNzaW9uIGJ5IHNlbmRpbmcgdGhpcyBmaXggdG8gc3RhYmxlLg0KDQpUaGFua3MNClNyaWRoYXIg
UGl0Y2hhaSAgICAgDQogDQoNCg==
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
2018-03-23 16:41 ` Sridhar Pitchai
@ 2018-03-23 16:47 ` Greg KH
-1 siblings, 0 replies; 26+ messages in thread
From: Greg KH @ 2018-03-23 16:47 UTC (permalink / raw)
To: Sridhar Pitchai
Cc: Lorenzo Pieralisi, Stephen Hemminger, linux-pci, Haiyang Zhang,
linux-kernel, Michael Kelley (EOSG),
Bjorn Helgaas, Jake Oshins, devel
On Fri, Mar 23, 2018 at 04:41:02PM +0000, Sridhar Pitchai wrote:
> Please read the link I sent you in relation to email formatting.
>
> Then add your description above in a way that anyone not 100% familiar
> with hyperv can understand it - that's what the commit log is for.
>
> You are sending this patch to stable kernels, patch above has been in
> the kernel from v4.14. The patch you are fixing since v4.11, you ought
> to be careful since you do not want to have broken kernel versions owing
> to stable patches mismatches, that's why I asked and I will ask again,
> are you sure you won't trigger a regression by sending this fix to
> stable ?
>
> I assume the bond driver mechanism is now done and dusted.
>
> That is correct. I have sent a v4 version of the patch. I am sending this
> patch for stable kernel. We have tested and I am sure this should not trigger
> regression by sending this fix to stable.
<formletter>
This is not the correct way to submit patches for inclusion in the
stable kernel tree. Please read:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.
</formletter>
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
@ 2018-03-23 16:47 ` Greg KH
0 siblings, 0 replies; 26+ messages in thread
From: Greg KH @ 2018-03-23 16:47 UTC (permalink / raw)
To: Sridhar Pitchai
Cc: Lorenzo Pieralisi, Stephen Hemminger, linux-pci, Haiyang Zhang,
linux-kernel, Michael Kelley (EOSG),
Jake Oshins, Bjorn Helgaas, devel
On Fri, Mar 23, 2018 at 04:41:02PM +0000, Sridhar Pitchai wrote:
> Please read the link I sent you in relation to email formatting.
>
> Then add your description above in a way that anyone not 100% familiar
> with hyperv can understand it - that's what the commit log is for.
>
> You are sending this patch to stable kernels, patch above has been in
> the kernel from v4.14. The patch you are fixing since v4.11, you ought
> to be careful since you do not want to have broken kernel versions owing
> to stable patches mismatches, that's why I asked and I will ask again,
> are you sure you won't trigger a regression by sending this fix to
> stable ?
>
> I assume the bond driver mechanism is now done and dusted.
>
> That is correct. I have sent a v4 version of the patch. I am sending this
> patch for stable kernel. We have tested and I am sure this should not trigger
> regression by sending this fix to stable.
<formletter>
This is not the correct way to submit patches for inclusion in the
stable kernel tree. Please read:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.
</formletter>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
2018-03-23 16:47 ` Greg KH
@ 2018-03-26 17:32 ` Sridhar Pitchai
-1 siblings, 0 replies; 26+ messages in thread
From: Sridhar Pitchai @ 2018-03-26 17:32 UTC (permalink / raw)
To: Greg KH
Cc: Lorenzo Pieralisi, Stephen Hemminger, linux-pci, Haiyang Zhang,
linux-kernel, Michael Kelley (EOSG),
Jake Oshins, Bjorn Helgaas, devel
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree. Please read:
> <url>
> for how to do this properly.
My bad. I am sending the patch to the mainline kernel with a Cc: tag for stable
and hoping the patch will be automatically merged to the stable kernels.
Thanks,
Sridhar Pitchai
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
@ 2018-03-26 17:32 ` Sridhar Pitchai
0 siblings, 0 replies; 26+ messages in thread
From: Sridhar Pitchai @ 2018-03-26 17:32 UTC (permalink / raw)
To: Greg KH
Cc: Lorenzo Pieralisi, Stephen Hemminger, linux-pci, Haiyang Zhang,
linux-kernel, Michael Kelley (EOSG),
Jake Oshins, Bjorn Helgaas, devel
ICANCiAgICA+IFRoaXMgaXMgbm90IHRoZSBjb3JyZWN0IHdheSB0byBzdWJtaXQgcGF0Y2hlcyBm
b3IgaW5jbHVzaW9uIGluIHRoZQ0KICAgID4gc3RhYmxlIGtlcm5lbCB0cmVlLiAgUGxlYXNlIHJl
YWQ6DQogICAgPiAgPHVybD4NCiAgICA+IGZvciBob3cgdG8gZG8gdGhpcyBwcm9wZXJseS4NCg0K
TXkgYmFkLiBJIGFtIHNlbmRpbmcgdGhlIHBhdGNoIHRvIHRoZSBtYWlubGluZSBrZXJuZWwgd2l0
aCBhIENjOiB0YWcgZm9yIHN0YWJsZQ0KYW5kIGhvcGluZyB0aGUgcGF0Y2ggd2lsbCBiZSBhdXRv
bWF0aWNhbGx5IG1lcmdlZCB0byB0aGUgc3RhYmxlIGtlcm5lbHMuDQoNClRoYW5rcywNClNyaWRo
YXIgUGl0Y2hhaQ0KDQo=
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2018-03-26 17:32 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-15 0:03 [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption Sridhar Pitchai
2018-03-15 0:03 ` Sridhar Pitchai
2018-03-15 12:05 ` Lorenzo Pieralisi
2018-03-15 12:05 ` Lorenzo Pieralisi
2018-03-15 17:56 ` Sridhar Pitchai
2018-03-15 17:56 ` Sridhar Pitchai
2018-03-15 18:24 ` Sridhar Pitchai
2018-03-15 18:24 ` Sridhar Pitchai
2018-03-20 17:56 ` Sridhar Pitchai
2018-03-20 17:56 ` Sridhar Pitchai
2018-03-20 18:32 ` Lorenzo Pieralisi
2018-03-20 18:32 ` Lorenzo Pieralisi
2018-03-20 23:00 ` Sridhar Pitchai
2018-03-20 23:00 ` Sridhar Pitchai
2018-03-21 16:26 ` Lorenzo Pieralisi
2018-03-21 16:26 ` Lorenzo Pieralisi
2018-03-23 16:41 ` Sridhar Pitchai
2018-03-23 16:41 ` Sridhar Pitchai
2018-03-23 16:47 ` Greg KH
2018-03-23 16:47 ` Greg KH
2018-03-26 17:32 ` Sridhar Pitchai
2018-03-26 17:32 ` Sridhar Pitchai
2018-03-21 17:30 ` Bjorn Helgaas
2018-03-21 17:30 ` Bjorn Helgaas
2018-03-23 16:09 ` Sridhar Pitchai
2018-03-23 16:09 ` Sridhar Pitchai
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.