Bug 212039

Created attachment 295605 [details]
dmesg

Most relevant part:
[13425.286713] xhci_hcd 0000:00:14.0: __pci_set_master 0 0x402
[13425.286829] xhci_hcd 0000:00:14.0: __pci_set_master 1 0x402
[13425.287198] xhci_hcd 0000:00:14.0: __pci_set_master 2 0x0
[13425.287413] xhci_hcd 0000:00:14.0: Controller not ready at resume -19
[13425.287415] xhci_hcd 0000:00:14.0: PCI post-resume error -19!
[13425.287417] xhci_hcd 0000:00:14.0: HC died; cleaning up
[13425.287422] PM: dpm_run_callback(): pci_pm_resume+0x0/0xf0 returns -19
[13425.287431] xhci_hcd 0000:00:14.0: PM: failed to resume async: error -19

Here's the patch to illustrate the bug:
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 16a17215f633..a53b8a965d87 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4249,6 +4249,11 @@ static void __pci_set_master(struct pci_dev *dev, bool enable)
        u16 old_cmd, cmd;
 
        pci_read_config_word(dev, PCI_COMMAND, &old_cmd);
+       pci_info(dev, "%s 0 0x%0x", __func__, old_cmd);
+       pci_read_config_word(dev, PCI_COMMAND, &old_cmd);
+       pci_info(dev, "%s 1 0x%0x", __func__, old_cmd);
+       pci_read_config_word(dev, PCI_COMMAND, &old_cmd);
+       pci_info(dev, "%s 2 0x%0x", __func__, old_cmd);
        if (enable)
                cmd = old_cmd | PCI_COMMAND_MASTER;
        else

Created attachment 295607 [details]
lspci -vvnnxxx

Also, the issue is not reproducible on Intel RVP.

Is the sudden 0 return for PCI_COMMAND word read related to a change in the
PCI_STATUS word?

As these two are both part of the same 32bit register could it be possible
that somthing setting the status accidentally overwrites the command?

PCI is not really my area, wanted to give my two cents anyway

I don't think so, because there are cases that the first read is 0, but it flips back to its original value on next read.

Also, there doesn't seem to be anything touches PCI_STATUS in PCI resume path.

As of now it seems like disabling Intel ME completely can make the issue go away.

(In reply to Kai-Heng Feng from comment #0)
> After system resume, the read on PCI_COMMAND may randomly be 0 in
> pci_read_config_word(), which is called by __pci_set_master().

In the attached dmesg this is suspend-to-idle, so I wonder if you have any insights in what in particular can cause this failure to happen.

There's a little difference between the handling of suspend-to-idle and PM-runtime, so I'm wondering why this is not visible in the working-state PM but just on system resume.

> So it only enables Bus Master in next pci_read_config_word(), disabling
> "Memory Space", "I/O Space" at the same time.
> 
> This makes xHCI driver fail to access MMIO register, render USB unusable.

It's caused by ME. But I have zero clue on what really happened.

This is the workaround [1] I found that can magically make the issue go away. It makes ME the last to suspend and first to resume, also no async suspend/resume by imposing device links to other PCH devices.

[1] https://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux/+git/unstable/commit/?id=6518ab6907040003c96107ebd247c58eab579f9e

Sasha, do you know if the workaround is something should be done? If this is a proper quirk we should do, I hope we can put it to e1000e driver...

(In reply to Kai-Heng Feng from comment #7)
> It's caused by ME. But I have zero clue on what really happened.

What exactly does ME stand for in this context?

> This is the workaround [1] I found that can magically make the issue go
> away. It makes ME the last to suspend and first to resume, also no async
> suspend/resume by imposing device links to other PCH devices.

So this would be a NIC.

Does reverting commit 6cecf02e77ab make the issue go away or is the workaround still needed after reverting it?

> [1]
> https://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux/+git/unstable/
> commit/?id=6518ab6907040003c96107ebd247c58eab579f9e

(In reply to Rafael J. Wysocki from comment #9)
> (In reply to Kai-Heng Feng from comment #7)
> > It's caused by ME. But I have zero clue on what really happened.
> 
> What exactly does ME stand for in this context?

Intel Management Engine.

> 
> > This is the workaround [1] I found that can magically make the issue go
> > away. It makes ME the last to suspend and first to resume, also no async
> > suspend/resume by imposing device links to other PCH devices.
> 
> So this would be a NIC.
> 
> Does reverting commit 6cecf02e77ab make the issue go away or is the
> workaround still needed after reverting it?

The workaround is still needed. The commit just makes the fail rate increased from 1% to 10%.

> 
> > [1]
> >
> https://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux/+git/unstable/
> > commit/?id=6518ab6907040003c96107ebd247c58eab579f9e

(In reply to Kai-Heng Feng from comment #10)
> (In reply to Rafael J. Wysocki from comment #9)
> > (In reply to Kai-Heng Feng from comment #7)
> > > It's caused by ME. But I have zero clue on what really happened.
> > 
> > What exactly does ME stand for in this context?
> 
> Intel Management Engine.

OK, thanks! [But it would help to resolve the TLA at least once in the report - or git commit for that matter.]

If it is caused by the ME, I wonder why the issue is reported against xHCI.  Surely other PCH devices are affected too?

> > > This is the workaround [1] I found that can magically make the issue go
> > > away. It makes ME the last to suspend and first to resume, also no async
> > > suspend/resume by imposing device links to other PCH devices.
> > 
> > So this would be a NIC.
> > 
> > Does reverting commit 6cecf02e77ab make the issue go away or is the
> > workaround still needed after reverting it?
> 
> The workaround is still needed. The commit just makes the fail rate
> increased from 1% to 10%.

I wonder why PM-runtime isn't affected then.  Or maybe it is, but failures are less likely to occur in that context.

Anyway, it looks like the suspend and resume of ME cannot be carried out in parallel with the suspend and resume of any other PCH devices, but does it have to be the last one to suspend and the first one to resume for the issue to go away?

(In reply to Rafael J. Wysocki from comment #11)
> (In reply to Kai-Heng Feng from comment #10)
> > (In reply to Rafael J. Wysocki from comment #9)
> > > (In reply to Kai-Heng Feng from comment #7)
> > > > It's caused by ME. But I have zero clue on what really happened.
> > > 
> > > What exactly does ME stand for in this context?
> > 
> > Intel Management Engine.
> 
> OK, thanks! [But it would help to resolve the TLA at least once in the
> report - or git commit for that matter.]
> 
> If it is caused by the ME, I wonder why the issue is reported against xHCI. 
> Surely other PCH devices are affected too?

Yes other PCI devices are also affected, xHCI is the most affected one.

> 
> > > > This is the workaround [1] I found that can magically make the issue go
> > > > away. It makes ME the last to suspend and first to resume, also no
> async
> > > > suspend/resume by imposing device links to other PCH devices.
> > > 
> > > So this would be a NIC.
> > > 
> > > Does reverting commit 6cecf02e77ab make the issue go away or is the
> > > workaround still needed after reverting it?
> > 
> > The workaround is still needed. The commit just makes the fail rate
> > increased from 1% to 10%.
> 
> I wonder why PM-runtime isn't affected then.  Or maybe it is, but failures
> are less likely to occur in that context.
> 
> Anyway, it looks like the suspend and resume of ME cannot be carried out in
> parallel with the suspend and resume of any other PCH devices, but does it
> have to be the last one to suspend and the first one to resume for the issue
> to go away?

Logically I think it's the case, because ME is at the center of the PCH and has even higher privilege than OS, so it should be the last one to suspend and the first one to resume.

In reality, I didn't test that because disabling async suspend for just one device is hard, as device_suspend() on non-async suspend doesn't wait for already dispatched asysc suspends to complete, so there are still parallel suspend operations happened at the same time.

(In reply to Kai-Heng Feng from comment #8)
> Sasha, do you know if the workaround is something should be done? If this is
> a proper quirk we should do, I hope we can put it to e1000e driver...

Hello,
How it related to e1000e? Do you have an option to disable ME via BIOS?
In case xHCI device affected - why do you suggest add w/a to e1000e. We agreed to increase polling time for ULP (you know we much duscussed) - I would suggest going by another way.

(In reply to Sasha Neftin from comment #13)
> (In reply to Kai-Heng Feng from comment #8)
> > Sasha, do you know if the workaround is something should be done? If this
> is
> > a proper quirk we should do, I hope we can put it to e1000e driver...
> 
> Hello,
> How it related to e1000e?

Because ME is more or less controlled by e1000e?

> Do you have an option to disable ME via BIOS?

According to ODM, ME can only be disabled at hardware level.
Disabling AMT/vPRO in BIOS doesn't disable ME.

> In case xHCI device affected - why do you suggest add w/a to e1000e. We
> agreed to increase polling time for ULP (you know we much duscussed) - I
> would suggest going by another way.

It's to ensure e1000e suspend/resume routine (and also ME related routines) doesn't overlap with other device driver's suspend/resume routine.

Of course we can add a giant lock on PCI core to wait until ME is fully functional on resume.

(In reply to Sasha Neftin from comment #13)
> (In reply to Kai-Heng Feng from comment #8)
> > Sasha, do you know if the workaround is something that should be done? If
> this is
> > a proper quirk we should do, I hope we can put it to e1000e driver...
> 
> Hello,
> How it related to e1000e? Do you have an option to disable ME via BIOS?
> In case xHCI device affected - why do you suggest add w/a to e1000e. We
> agreed to increase polling time for ULP (you know we much discussed) - I
> would suggest going by another way.

I would suggest to try to mask MEIvia  .config:
Device Drivers -> Misc devices -> Intel Managment Engine Interface remove <m>

(In reply to Sasha Neftin from comment #15)
> (In reply to Sasha Neftin from comment #13)
> > (In reply to Kai-Heng Feng from comment #8)
> > > Sasha, do you know if the workaround is something that should be done? If
> > this is
> > > a proper quirk we should do, I hope we can put it to e1000e driver...
> > 
> > Hello,
> > How it related to e1000e? Do you have an option to disable ME via BIOS?
> > In case xHCI device affected - why do you suggest add w/a to e1000e. We
> > agreed to increase polling time for ULP (you know we much discussed) - I
> > would suggest going by another way.
> 
> I would suggest to try to mask MEIvia  .config:
> Device Drivers -> Misc devices -> Intel Managment Engine Interface remove <m>

It doesn't help. Probably because mei_me is just an "Interface"?

Created attachment 295909 [details]
dmesg with mei_me disabled

(In reply to Kai-Heng Feng from comment #14)
> (In reply to Sasha Neftin from comment #13)
> > (In reply to Kai-Heng Feng from comment #8)
> > > Sasha, do you know if the workaround is something should be done? If this
> > is
> > > a proper quirk we should do, I hope we can put it to e1000e driver...
> > 
> > Hello,
> > How it related to e1000e?
> 
> Because ME is more or less controlled by e1000e?
> 
e1000e do not control ME. We need to find ME/AMT contact and work with them to disable the ME. I do not see another way.

> > Do you have an option to disable ME via BIOS?
> 
> According to ODM, ME can only be disabled at hardware level.
> Disabling AMT/vPRO in BIOS doesn't disable ME.
> 
> > In case xHCI device affected - why do you suggest add w/a to e1000e. We
> > agreed to increase polling time for ULP (you know we much duscussed) - I
> > would suggest going by another way.
> 
> It's to ensure e1000e suspend/resume routine (and also ME related routines)
> doesn't overlap with other device driver's suspend/resume routine.
> 
> Of course we can add a giant lock on PCI core to wait until ME is fully
> functional on resume.

> e1000e do not control ME. We need to find ME/AMT contact and work with them
> to disable the ME. I do not see another way.

Ok, at least ME ULP is more or less related e1000e...

CC Tomas.

Can you please collect additional information 
1. dmidcode
2. MEI firmware version (cat /sys/class/mei/mei0/fw_ver)

Created attachment 296033 [details]
dmidecode

$ cat /sys/class/mei/mei0/fw_ver
0:15.0.22.1622
0:15.0.22.1622
0:15.0.22.1622

I was told that currently [1] is best approach to tackle the issue. Can I get some ACKs from Intel if I send the patch to mailing list?

[1] https://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux/+git/unstable/commit/?id=6518ab6907040003c96107ebd247c58eab579f9e

(In reply to Kai-Heng Feng from comment #24)
> I was told that currently [1] is best approach to tackle the issue. Can I
> get some ACKs from Intel if I send the patch to mailing list?
> 
> [1]
> https://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux/+git/unstable/
> commit/?id=6518ab6907040003c96107ebd247c58eab579f9e

We will need to investigate it. This doesn't look like a device driver issue, I will need to propagate the findings internally.

Created attachment 296101 [details]
attachment-15001-0.html

Out of office. Expected delayed response

Hi, Tomas,

Do you happen to get any update on this?

ping...

We are investigating it internally together with team now

(In reply to Kai-Heng Feng from comment #3)
> Also, the issue is not reproducible on Intel RVP.

What is the system that you reproduce it on?

(In reply to Alexander Usyskin from comment #30)
> (In reply to Kai-Heng Feng from comment #3)
> > Also, the issue is not reproducible on Intel RVP.
> 
> What is the system that you reproduce it on?

It's OptiPlex 7490 AIO.

May I know the latest status of this issue?

Created attachment 301238 [details]
attachment-20658-0.html

Out of office. Expected delayed response

(In reply to Zhang Rui from comment #32)
> May I know the latest status of this issue?
Looks like it's a hardware defect in B0 stepping.

B1 stepping doesn't seem to have this issue, so I'll drop the workaround we applied in Ubuntu kernel.