xl pci-detach vs xm pci-detach in Xen 4.3 (one works, the other does not)

xl -f pci-detach latest 0000:01:00.0

any thoughts? I don't know if this is a regression or not, but the libxl errors are pointing me to the recent XSA issue - which I thought was fixed?

create ^

title it xl pci-detach failure

thanks

create ^
title it xl pci-detach failure
thanks

Quoted 3 lines · Konrad Rzeszutek Wilk

any thoughts? I don't know if this is a regression or not, but the libxl errors are pointing me to the recent XSA issue - which I thought was fixed?

I think the recent issue was with xend not working. Did we fix send and break xl?

In any case, this is a pretty important feature; I think we need to sort it out before release, so I'm giving it a bug id to track.

Is "pci permissive" set in the global xl.conf file?

Have you tried this with say, a stock Debian Wheezy kernel?

-George

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Quoted 11 lines · George Dunlap

create ^
title it xl pci-detach failure
thanks

Quoted 3 lines · Konrad Rzeszutek Wilk

any thoughts? I don't know if this is a regression or not, but the libxl errors are pointing me to the recent XSA issue - which I thought was fixed?

I think the recent issue was with xend not working. Did we fix send and break xl?

Could be. It might also be that that 'xl' never did it the same way as 'xend' (meaning this might be a XenBus teardown change).

Note also that this is PV guests - and I think most of the testing had been with the HVM guests with PCI passthrough. So it might be a seperate issue altogether. Or that nobody tried doing PCI plug/unplug in the past :-(

Quoted 5 lines

In any case, this is a pretty important feature; I think we need to sort it out before release, so I'm giving it a bug id to track.

Is "pci permissive" set in the global xl.conf file?

No. Let me of course try that.

Quoted 1 line

Have you tried this with say, a stock Debian Wheezy kernel?

No. A v3.10-rc4 with Xen 4.3 latest.

Is Debian Wheezy a 2.6.32 kernel?

Quoted 12 lines · Konrad Rzeszutek Wilk

Quoted 11 lines · George Dunlap

create ^
title it xl pci-detach failure
thanks

Quoted 3 lines · Konrad Rzeszutek Wilk

any thoughts? I don't know if this is a regression or not, but the libxl errors are pointing me to the recent XSA issue - which I thought was fixed?

I think the recent issue was with xend not working. Did we fix send and break xl?

Quoted 7 lines

Could be. It might also be that that 'xl' never did it the same way as 'xend' (meaning this might be a XenBus teardown change).

Note also that this is PV guests - and I think most of the testing had been with the HVM guests with PCI passthrough. So it might be a seperate issue altogether. Or that nobody tried doing PCI plug/unplug in the past :-(

I certainly did before my February FOSDEM talk that included driver domains.

Quoted 6 lines

Quoted 5 lines

In any case, this is a pretty important feature; I think we need to sort it out before release, so I'm giving it a bug id to track.

Is "pci permissive" set in the global xl.conf file?

No. Let me of course try that.

That really should have to do with getting it working in the first place, not detaching it; but still...

Quoted 4 lines

Quoted 1 line

Have you tried this with say, a stock Debian Wheezy kernel?

No. A v3.10-rc4 with Xen 4.3 latest.

Is Debian Wheezy a 2.6.32 kernel?

No, 3.2.

Quoted 1 line

Quoted 1 line · Konrad Rzeszutek Wilk

There are two tool-stack that can instruct the Xen PCI frontend and backend to change states: 'xm' (Python code with a daemon), and 'xl' (C library - does not keep state changes).

With the 'xm', the path to disconnect a PCI device (xm pci-detach <guest> <BDF>)is:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)->5(Closing*).

The * is for states that the tool-stack sets. For 'xl', it is similar:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)

Both of them also tear down the XenBus structure, so the backend state ends up going in the 3(Initialised) and calls pcifront_xenbus_remove.

When a PCI device is plugged in (xm pci-attach <guest> <BDF>) both of them follow the same pattern:
2(InitWait*), 3(Initialized*), 4(Connected*)->4(Connected).

[xen-pcifront ignores the 2,3 state changes and only acts when 4 (Connected) has been reached]

The problem is that git commit 3d925320e9e2de162bd138bf97816bda8c3f71be ("xen/pcifront: Use Xen-SWIOTLB when initting if required") introduced a mechanism to initialize the SWIOTLB when the Xen PCI front moves to Connected state. It also had some aggressive seatbelt code check that would warn the user if one tried to change to Connected state without hitting first the Closing state:

pcifront pci-0: PCI frontend already installed!

However, that code can be relaxed and we can continue on working even if the frontend is instructed to be the 'Connected' state with no devices and then gets tickled to be in 'Connected' state again.

In other words, this 4(Connected)->5(Closing)->4(Connected) state was expected, while 4(Connected)->.... anything but 5(Closing)->4(Connected) was not. This patch removes that aggressive check and allows
Xen pcifront to work with the 'xl' toolstack.

Cc Bjorn Helgaas <[email protected]>

Cc [email protected]

Cc [email protected]

Signed-off-by Konrad Rzeszutek Wilk <[email protected]>

---
 drivers/pci/xen-pcifront.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

drivers/pci/xen-pcifront.c +2 -3

index ac99515..cc46e253 100644
@@ -675,10 +675,9 @@ static int pcifront_connect_and_init_dma(struct 
 pcifront_device *pdev)
 	if (!pcifront_dev) {
 		dev_info(&pdev->xdev->dev, "Installing PCI frontend\n");
 		pcifront_dev = pdev;
-	} else {
-		dev_err(&pdev->xdev->dev, "PCI frontend already installed!\n");
+	} else
 		err = -EEXIST;
-	}
+
 	spin_unlock(&pcifront_dev_lock);
 

drivers/pci/xen-pcifront.c +1 -1

 	if (!err && !swiotlb_nr_tbl()) {

@@ -846,7 +845,7 @@ static int pcifront_try_connect(struct pcifront_device 
 *pdev)
 		goto out;
 
 	err = pcifront_connect_and_init_dma(pdev);
-	if (err) {
+	if (err && err != -EEXIST) {
 		xenbus_dev_fatal(pdev->xdev, err,
 				 "Error setting up PCI Frontend");

goto out;

Signature · 7 lines

1.8.1.4


_______________________________________________
Xen-devel mailing list
[email protected] 
http://lists.xen.org/xen-devel 

Quoted 1 line

Quoted 1 line · Konrad Rzeszutek Wilk

There are two tool-stack that can instruct the Xen PCI frontend and backend to change states: 'xm' (Python code with a daemon), and 'xl' (C library - does not keep state changes).

With the 'xm', the path to disconnect a PCI device (xm pci-detach <guest> <BDF>)is:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)->5(Closing*).

The * is for states that the tool-stack sets. For 'xl', it is similar:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)

Both of them also tear down the XenBus structure, so the backend state ends up going in the 3(Initialised) and calls pcifront_xenbus_remove.

When a PCI device is plugged in (xm pci-attach <guest> <BDF>) both of them follow the same pattern:
2(InitWait*), 3(Initialized*), 4(Connected*)->4(Connected).

[xen-pcifront ignores the 2,3 state changes and only acts when 4 (Connected) has been reached]

The problem is that git commit 3d925320e9e2de162bd138bf97816bda8c3f71be ("xen/pcifront: Use Xen-SWIOTLB when initting if required") introduced a mechanism to initialize the SWIOTLB when the Xen PCI front moves to Connected state. It also had some aggressive seatbelt code check that would warn the user if one tried to change to Connected state without hitting first the Closing state:

pcifront pci-0: PCI frontend already installed!

However, that code can be relaxed and we can continue on working even if the frontend is instructed to be the 'Connected' state with no devices and then gets tickled to be in 'Connected' state again.

In other words, this 4(Connected)->5(Closing)->4(Connected) state was expected, while 4(Connected)->.... anything but 5(Closing)->4(Connected) was not. This patch removes that aggressive check and allows
Xen pcifront to work with the 'xl' toolstack.

Cc Bjorn Helgaas <[email protected]>

Cc [email protected]

Cc [email protected]

Signed-off-by Konrad Rzeszutek Wilk <[email protected]>

---
 drivers/pci/xen-pcifront.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

drivers/pci/xen-pcifront.c +2 -3

index ac99515..cc46e253 100644
@@ -675,10 +675,9 @@ static int pcifront_connect_and_init_dma(struct 
 pcifront_device *pdev)
 	if (!pcifront_dev) {
 		dev_info(&pdev->xdev->dev, "Installing PCI frontend\n");
 		pcifront_dev = pdev;
-	} else {
-		dev_err(&pdev->xdev->dev, "PCI frontend already installed!\n");
+	} else
 		err = -EEXIST;
-	}
+
 	spin_unlock(&pcifront_dev_lock);
 

drivers/pci/xen-pcifront.c +1 -1

 	if (!err && !swiotlb_nr_tbl()) {

@@ -846,7 +845,7 @@ static int pcifront_try_connect(struct pcifront_device 
 *pdev)
 		goto out;
 
 	err = pcifront_connect_and_init_dma(pdev);
-	if (err) {
+	if (err && err != -EEXIST) {
 		xenbus_dev_fatal(pdev->xdev, err,
 				 "Error setting up PCI Frontend");

goto out;

Signature · 7 lines

1.8.1.4


_______________________________________________
Xen-devel mailing list
[email protected] 
http://lists.xen.org/xen-devel 

Quoted 37 lines

Quoted 1 line

Quoted 1 line · Konrad Rzeszutek Wilk

There are two tool-stack that can instruct the Xen PCI frontend and backend to change states: 'xm' (Python code with a daemon), and 'xl' (C library - does not keep state changes).

With the 'xm', the path to disconnect a PCI device (xm pci-detach <guest> <BDF>)is:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)->5(Closing*).

The * is for states that the tool-stack sets. For 'xl', it is similar:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)

Both of them also tear down the XenBus structure, so the backend state ends up going in the 3(Initialised) and calls pcifront_xenbus_remove.

When a PCI device is plugged in (xm pci-attach <guest> <BDF>) both of them follow the same pattern:
2(InitWait*), 3(Initialized*), 4(Connected*)->4(Connected).

[xen-pcifront ignores the 2,3 state changes and only acts when 4 (Connected) has been reached]

The problem is that git commit 3d925320e9e2de162bd138bf97816bda8c3f71be ("xen/pcifront: Use Xen-SWIOTLB when initting if required") introduced a mechanism to initialize the SWIOTLB when the Xen PCI front moves to Connected state. It also had some aggressive seatbelt code check that would warn the user if one tried to change to Connected state without hitting first the Closing state:

pcifront pci-0: PCI frontend already installed!

However, that code can be relaxed and we can continue on working even if the frontend is instructed to be the 'Connected' state with no devices and then gets tickled to be in 'Connected' state again.

Quoted 4 lines

In other words, this 4(Connected)->5(Closing)->4(Connected) state was expected, while 4(Connected)->.... anything but 5(Closing)->4(Connected) was not. This patch removes that aggressive check and allows
Xen pcifront to work with the 'xl' toolstack.

I actually think this shouldn't be worked around here, but fixed in xl. Any device removed from a guest should be driven towards the "Closed" state.

Quoted 37 lines

Quoted 1 line

Quoted 1 line · Konrad Rzeszutek Wilk

There are two tool-stack that can instruct the Xen PCI frontend and backend to change states: 'xm' (Python code with a daemon), and 'xl' (C library - does not keep state changes).

With the 'xm', the path to disconnect a PCI device (xm pci-detach <guest> <BDF>)is:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)->5(Closing*).

The * is for states that the tool-stack sets. For 'xl', it is similar:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)

Both of them also tear down the XenBus structure, so the backend state ends up going in the 3(Initialised) and calls pcifront_xenbus_remove.

When a PCI device is plugged in (xm pci-attach <guest> <BDF>) both of them follow the same pattern:
2(InitWait*), 3(Initialized*), 4(Connected*)->4(Connected).

[xen-pcifront ignores the 2,3 state changes and only acts when 4 (Connected) has been reached]

The problem is that git commit 3d925320e9e2de162bd138bf97816bda8c3f71be ("xen/pcifront: Use Xen-SWIOTLB when initting if required") introduced a mechanism to initialize the SWIOTLB when the Xen PCI front moves to Connected state. It also had some aggressive seatbelt code check that would warn the user if one tried to change to Connected state without hitting first the Closing state:

pcifront pci-0: PCI frontend already installed!

However, that code can be relaxed and we can continue on working even if the frontend is instructed to be the 'Connected' state with no devices and then gets tickled to be in 'Connected' state again.

Quoted 4 lines

In other words, this 4(Connected)->5(Closing)->4(Connected) state was expected, while 4(Connected)->.... anything but 5(Closing)->4(Connected) was not. This patch removes that aggressive check and allows
Xen pcifront to work with the 'xl' toolstack.

I actually think this shouldn't be worked around here, but fixed in xl. Any device removed from a guest should be driven towards the "Closed" state.

Quoted 37 lines · Jan Beulich

Quoted 37 lines

Quoted 1 line

Quoted 1 line · Konrad Rzeszutek Wilk

There are two tool-stack that can instruct the Xen PCI frontend and backend to change states: 'xm' (Python code with a daemon), and 'xl' (C library - does not keep state changes).

With the 'xm', the path to disconnect a PCI device (xm pci-detach <guest> <BDF>)is:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)->5(Closing*).

The * is for states that the tool-stack sets. For 'xl', it is similar:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)

Both of them also tear down the XenBus structure, so the backend state ends up going in the 3(Initialised) and calls pcifront_xenbus_remove.

When a PCI device is plugged in (xm pci-attach <guest> <BDF>) both of them follow the same pattern:
2(InitWait*), 3(Initialized*), 4(Connected*)->4(Connected).

[xen-pcifront ignores the 2,3 state changes and only acts when 4 (Connected) has been reached]

The problem is that git commit 3d925320e9e2de162bd138bf97816bda8c3f71be ("xen/pcifront: Use Xen-SWIOTLB when initting if required") introduced a mechanism to initialize the SWIOTLB when the Xen PCI front moves to Connected state. It also had some aggressive seatbelt code check that would warn the user if one tried to change to Connected state without hitting first the Closing state:

pcifront pci-0: PCI frontend already installed!

However, that code can be relaxed and we can continue on working even if the frontend is instructed to be the 'Connected' state with no devices and then gets tickled to be in 'Connected' state again.

Quoted 8 lines

Quoted 4 lines

In other words, this 4(Connected)->5(Closing)->4(Connected) state was expected, while 4(Connected)->.... anything but 5(Closing)->4(Connected) was not. This patch removes that aggressive check and allows
Xen pcifront to work with the 'xl' toolstack.

I actually think this shouldn't be worked around here, but fixed in xl. Any device removed from a guest should be driven towards the "Closed" state.

Yeah, that seems pretty obvious to me. The weird thing is that this wasn't noticed before -- does this work in 4.2? Have you been doing this test all along, or has it only broken recently?

I've reproduced it on one of my test boxes; let me see if I can sort it out.

-George

Quoted 37 lines · Jan Beulich

Quoted 37 lines

Quoted 1 line

Quoted 1 line · Konrad Rzeszutek Wilk

There are two tool-stack that can instruct the Xen PCI frontend and backend to change states: 'xm' (Python code with a daemon), and 'xl' (C library - does not keep state changes).

With the 'xm', the path to disconnect a PCI device (xm pci-detach <guest> <BDF>)is:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)->5(Closing*).

The * is for states that the tool-stack sets. For 'xl', it is similar:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)

Both of them also tear down the XenBus structure, so the backend state ends up going in the 3(Initialised) and calls pcifront_xenbus_remove.

When a PCI device is plugged in (xm pci-attach <guest> <BDF>) both of them follow the same pattern:
2(InitWait*), 3(Initialized*), 4(Connected*)->4(Connected).

[xen-pcifront ignores the 2,3 state changes and only acts when 4 (Connected) has been reached]

The problem is that git commit 3d925320e9e2de162bd138bf97816bda8c3f71be ("xen/pcifront: Use Xen-SWIOTLB when initting if required") introduced a mechanism to initialize the SWIOTLB when the Xen PCI front moves to Connected state. It also had some aggressive seatbelt code check that would warn the user if one tried to change to Connected state without hitting first the Closing state:

pcifront pci-0: PCI frontend already installed!

However, that code can be relaxed and we can continue on working even if the frontend is instructed to be the 'Connected' state with no devices and then gets tickled to be in 'Connected' state again.

Quoted 8 lines

Quoted 4 lines

In other words, this 4(Connected)->5(Closing)->4(Connected) state was expected, while 4(Connected)->.... anything but 5(Closing)->4(Connected) was not. This patch removes that aggressive check and allows
Xen pcifront to work with the 'xl' toolstack.

I actually think this shouldn't be worked around here, but fixed in xl. Any device removed from a guest should be driven towards the "Closed" state.

Yeah, that seems pretty obvious to me. The weird thing is that this wasn't noticed before -- does this work in 4.2? Have you been doing this test all along, or has it only broken recently?

I've reproduced it on one of my test boxes; let me see if I can sort it out.

-George

There are two tool-stack that can instruct the Xen PCI frontend and backend to change states: 'xm' (Python code with a daemon), and 'xl' (C library - does not keep state changes).

With the 'xm', the path to disconnect a PCI device (xm pci-detach <guest> <BDF>)is:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)->5(Closing*).

The * is for states that the tool-stack sets. For 'xl', it is similar:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)

Both of them also tear down the XenBus structure, so the backend state ends up going in the 3(Initialised) and calls pcifront_xenbus_remove.

Quoted 7 lines · Konrad Rzeszutek Wilk

There are two tool-stack that can instruct the Xen PCI frontend and backend to change states: 'xm' (Python code with a daemon), and 'xl' (C library - does not keep state changes).

With the 'xm', the path to disconnect a PCI device (xm pci-detach <guest> <BDF>)is:

Quoted 8 lines

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)->5(Closing*).

The * is for states that the tool-stack sets. For 'xl', it is similar:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)

Both of them also tear down the XenBus structure, so the backend state ends up going in the 3(Initialised) and calls pcifront_xenbus_remove.

So I looked a little bit into this; there are actually two different states that happen as part of this handshake. In order to disonnect a *device*, xl signals using the *bus* state, like this:
* Wait for the *bus* to be in state 4(Connected)
* Set the *device* state to 5(Closing)
* Set the *bus* state to 7(Reconfiguring)
* Wait for the *bus* state to return to 4(Connected)

So are all of these states you see the *bus* state? And why would you disconnect the whole pci bus if you're only removing one device?

-George

Quoted 27 lines · George Dunlap

Quoted 10 lines · Konrad Rzeszutek Wilk

There are two tool-stack that can instruct the Xen PCI frontend and backend to change states: 'xm' (Python code with a daemon), and 'xl' (C library - does not keep state changes).

With the 'xm', the path to disconnect a PCI device (xm pci-detach <guest> <BDF>)is:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)->5(Closing*).

Quoted 7 lines

The * is for states that the tool-stack sets. For 'xl', it is similar:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)

Both of them also tear down the XenBus structure, so the backend state ends up going in the 3(Initialised) and calls pcifront_xenbus_remove.

So I looked a little bit into this; there are actually two different states that happen as part of this handshake. In order to disonnect a *device*, xl signals using the *bus* state, like this:
* Wait for the *bus* to be in state 4(Connected)
* Set the *device* state to 5(Closing)
* Set the *bus* state to 7(Reconfiguring)
* Wait for the *bus* state to return to 4(Connected)

Quoted 2 lines

So are all of these states you see the *bus* state? And why would you disconnect the whole pci bus if you're only removing one device?

Correct. The stats I enumerated are *bus* states. Not per-device states. I presume (and I hadn't checked xm) that Xend has some logic to only disconnect the bus if all of the PCI devices have been disconnected. In 'xl' it does not do that.

The testing I did was just with one PCI device.

Quoted 37 lines · konrad wilk

Quoted 27 lines · George Dunlap

Quoted 10 lines · Konrad Rzeszutek Wilk

There are two tool-stack that can instruct the Xen PCI frontend and backend to change states: 'xm' (Python code with a daemon), and 'xl' (C library - does not keep state changes).

With the 'xm', the path to disconnect a PCI device (xm pci-detach <guest> <BDF>)is:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)->5(Closing*).

Quoted 7 lines

The * is for states that the tool-stack sets. For 'xl', it is similar:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)

Both of them also tear down the XenBus structure, so the backend state ends up going in the 3(Initialised) and calls pcifront_xenbus_remove.

So I looked a little bit into this; there are actually two different states that happen as part of this handshake. In order to disonnect a *device*, xl signals using the *bus* state, like this:
* Wait for the *bus* to be in state 4(Connected)
* Set the *device* state to 5(Closing)
* Set the *bus* state to 7(Reconfiguring)
* Wait for the *bus* state to return to 4(Connected)

Quoted 2 lines

So are all of these states you see the *bus* state? And why would you disconnect the whole pci bus if you're only removing one device?

Correct. The stats I enumerated are *bus* states. Not per-device states. I presume (and I hadn't checked xm) that Xend has some logic to only disconnect the bus if all of the PCI devices have been disconnected. In 'xl' it does not do that.

Quoted 1 line

The testing I did was just with one PCI device.

Ah, OK -- I see now. The problem is that the code in the Linux side didn't know about the whole "4->7->8->4" thing to unplug a device. In all likelihood, if you had used xm with two devices (so that the bus didn't get disconnected), then you would have run across the same error.

So at least part of the problem *is* a bug in Linux.

That doesn't explain why I have problems doing this on Debian's version of 3.2 -- unless the "fix" you mentoned above was backported to the stable kernel, perhaps?

-George

Quoted 32 lines · konrad wilk

Quoted 27 lines · George Dunlap

Quoted 10 lines · Konrad Rzeszutek Wilk

There are two tool-stack that can instruct the Xen PCI frontend and backend to change states: 'xm' (Python code with a daemon), and 'xl' (C library - does not keep state changes).

With the 'xm', the path to disconnect a PCI device (xm pci-detach <guest> <BDF>)is:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)->5(Closing*).

Quoted 7 lines

The * is for states that the tool-stack sets. For 'xl', it is similar:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)

Both of them also tear down the XenBus structure, so the backend state ends up going in the 3(Initialised) and calls pcifront_xenbus_remove.

So I looked a little bit into this; there are actually two different states that happen as part of this handshake. In order to disonnect a *device*, xl signals using the *bus* state, like this:
* Wait for the *bus* to be in state 4(Connected)
* Set the *device* state to 5(Closing)
* Set the *bus* state to 7(Reconfiguring)
* Wait for the *bus* state to return to 4(Connected)

Quoted 2 lines

So are all of these states you see the *bus* state? And why would you disconnect the whole pci bus if you're only removing one device?

Quoted 6 lines

Correct. The stats I enumerated are *bus* states. Not per-device states. I presume (and I hadn't checked xm) that Xend has some logic to only disconnect the bus if all of the PCI devices have been disconnected. In 'xl' it does not do that.

The testing I did was just with one PCI device.

Ah, OK -- I see now. The problem is that the code in the Linux side didn't know about the whole "4->7->8->4" thing to unplug a device. In all likelihood, if you had used xm with two devices (so that the bus didn't get disconnected), then you would have run across the same error.

So at least part of the problem *is* a bug in Linux.

From efdfbd66b4f0ff6f005f9d30891adb8bd3f3eefa Mon Sep 17 00:00:00 2001

Quoted 42 lines · George Dunlap

Quoted 27 lines · konrad wilk

Quoted 26 lines · George Dunlap

Quoted 10 lines · Konrad Rzeszutek Wilk

There are two tool-stack that can instruct the Xen PCI frontend and backend to change states: 'xm' (Python code with a daemon), and 'xl' (C library - does not keep state changes).

With the 'xm', the path to disconnect a PCI device (xm pci-detach <guest> <BDF>)is:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)->5(Closing*).

Quoted 7 lines

The * is for states that the tool-stack sets. For 'xl', it is similar:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)

Both of them also tear down the XenBus structure, so the backend state ends up going in the 3(Initialised) and calls pcifront_xenbus_remove.

So I looked a little bit into this; there are actually two different states that happen as part of this handshake. In order to disonnect a *device*, xl signals using the *bus* state, like this:
* Wait for the *bus* to be in state 4(Connected)
* Set the *device* state to 5(Closing)
* Set the *bus* state to 7(Reconfiguring)
* Wait for the *bus* state to return to 4(Connected)

Quoted 8 lines

Quoted 2 lines

So are all of these states you see the *bus* state? And why would you disconnect the whole pci bus if you're only removing one device?

Correct. The stats I enumerated are *bus* states. Not per-device states. I presume (and I hadn't checked xm) that Xend has some logic to only disconnect the bus if all of the PCI devices have been disconnected. In 'xl' it does not do that.

The testing I did was just with one PCI device.

Ah, OK -- I see now. The problem is that the code in the Linux side didn't know about the whole "4->7->8->4" thing to unplug a device. In all likelihood, if you had used xm with two devices (so that the bus didn't get disconnected), then you would have run across the same error.

Quoted 1 line

So at least part of the problem *is* a bug in Linux.

Good! Bjorn, would you be OK Ack-ing the patch I sent (attached here for reference) or putting it in your queue for Linus?

My plan would be to send it to Linus in the 3.11 merge window.

Quoted 44 lines · Konrad Rzeszutek Wilk

Quoted 43 lines · George Dunlap

Quoted 27 lines · konrad wilk

Quoted 26 lines · George Dunlap

Quoted 10 lines · Konrad Rzeszutek Wilk

There are two tool-stack that can instruct the Xen PCI frontend and backend to change states: 'xm' (Python code with a daemon), and 'xl' (C library - does not keep state changes).

With the 'xm', the path to disconnect a PCI device (xm pci-detach <guest> <BDF>)is:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)->5(Closing*).

Quoted 7 lines

The * is for states that the tool-stack sets. For 'xl', it is similar:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)

Both of them also tear down the XenBus structure, so the backend state ends up going in the 3(Initialised) and calls pcifront_xenbus_remove.

So I looked a little bit into this; there are actually two different states that happen as part of this handshake. In order to disonnect a *device*, xl signals using the *bus* state, like this:
* Wait for the *bus* to be in state 4(Connected)
* Set the *device* state to 5(Closing)
* Set the *bus* state to 7(Reconfiguring)
* Wait for the *bus* state to return to 4(Connected)

Quoted 8 lines

Quoted 2 lines

So are all of these states you see the *bus* state? And why would you disconnect the whole pci bus if you're only removing one device?

Correct. The stats I enumerated are *bus* states. Not per-device states. I presume (and I hadn't checked xm) that Xend has some logic to only disconnect the bus if all of the PCI devices have been disconnected. In 'xl' it does not do that.

The testing I did was just with one PCI device.

Ah, OK -- I see now. The problem is that the code in the Linux side didn't know about the whole "4->7->8->4" thing to unplug a device. In all likelihood, if you had used xm with two devices (so that the bus didn't get disconnected), then you would have run across the same error.

So at least part of the problem *is* a bug in Linux.

Quoted 4 lines

Good! Bjorn, would you be OK Ack-ing the patch I sent (attached here for reference) or putting it in your queue for Linus?

My plan would be to send it to Linus in the 3.11 merge window.

One nit -- "to work with the 'xl' toolstack" -- didn't we theorize this would also be broken with xm if you had two devices passed through?

Quoted 46 lines · George Dunlap

Quoted 32 lines · konrad wilk

Quoted 27 lines · George Dunlap

Quoted 10 lines · Konrad Rzeszutek Wilk

There are two tool-stack that can instruct the Xen PCI frontend and backend to change states: 'xm' (Python code with a daemon), and 'xl' (C library - does not keep state changes).

With the 'xm', the path to disconnect a PCI device (xm pci-detach <guest> <BDF>)is:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)->5(Closing*).

Quoted 7 lines

The * is for states that the tool-stack sets. For 'xl', it is similar:

4(Connected)->7(Reconfiguring*)-> 8(Reconfigured)-> 4(Connected)

Both of them also tear down the XenBus structure, so the backend state ends up going in the 3(Initialised) and calls pcifront_xenbus_remove.

So I looked a little bit into this; there are actually two different states that happen as part of this handshake. In order to disonnect a *device*, xl signals using the *bus* state, like this:
* Wait for the *bus* to be in state 4(Connected)
* Set the *device* state to 5(Closing)
* Set the *bus* state to 7(Reconfiguring)
* Wait for the *bus* state to return to 4(Connected)

Quoted 2 lines

So are all of these states you see the *bus* state? And why would you disconnect the whole pci bus if you're only removing one device?

Quoted 6 lines

Correct. The stats I enumerated are *bus* states. Not per-device states. I presume (and I hadn't checked xm) that Xend has some logic to only disconnect the bus if all of the PCI devices have been disconnected. In 'xl' it does not do that.

The testing I did was just with one PCI device.

Ah, OK -- I see now. The problem is that the code in the Linux side didn't know about the whole "4->7->8->4" thing to unplug a device. In all likelihood, if you had used xm with two devices (so that the bus didn't get disconnected), then you would have run across the same error.

Quoted 1 line

So at least part of the problem *is* a bug in Linux.

Good! Bjorn, would you be OK Ack-ing the patch I sent (attached here for reference) or putting it in your queue for Linus?

My plan would be to send it to Linus in the 3.11 merge window.

Quoted 6 lines

Quoted 1 line

So at least part of the problem *is* a bug in Linux.

Good! Bjorn, would you be OK Ack-ing the patch I sent (attached here for reference) or putting it in your queue for Linus?

My plan would be to send it to Linus in the 3.11 merge window.

Sure; this is your baby :) Why don't you handle it via your tree, since it's more related to xen than any PCI core stuff.

help

Processing commands for [email protected]:

Quoted 1 line

help

Command failed: Unknown command `help'. at /srv/xen-devel-bugs/lib/emesinae/control.pl line 437, <M> line 43. Stop processing here.

---
Xen Hypervisor Bug Tracker
See http://wiki.xen.org/wiki/Reporting_Bugs_against_Xen for information on reporting bugs Contact [email protected] with any infrastructure issues

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Sure; this is your baby :) Why don't you handle it via your tree, since it's more related to xen than any PCI core stuff.

Acked-by Bjorn Helgaas <[email protected]>

Quoted 5 lines

Sure; this is your baby :) Why don't you handle it via your tree, since it's more related to xen than any PCI core stuff.

Acked-by Bjorn Helgaas <[email protected]>

Definitly fixed in v3.12. Just tested it and it works.

George, Ian, how do I "close" a bug in
http://bugs.xenproject.org/xen/bug/12 ?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

close 12