Many users report rather sluggish RX speed on TGP I219. Since
"intel_idle.max_cstate=1" doesn't help, so it's not caused by deeper
package C-state.
A workaround that always works is to make sure mei_me is runtime active
when e1000e is in use.
The root cause is still unknown, but since many users are affected by
the issue, implment the quirk in the driver as a temporary workaround.
Also adds mei_me as soft dependency to ensure the device link can be
created if e1000e is in initramfs.
BugLink: https://bugs.launchpad.net/bugs/1927925
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=213377
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=213651
Signed-off-by: Kai-Heng Feng <redacted>
---
drivers/net/ethernet/intel/e1000e/netdev.c | 26 ++++++++++++++++++++++
1 file changed, 26 insertions(+)
@@ -7317,6 +7317,27 @@ static const struct net_device_ops e1000e_netdev_ops = {.ndo_features_check=passthru_features_check,};+staticvoide1000e_create_device_links(structpci_dev*pdev)+{+structpci_dev*tgp_mei_me;++/* Find TGP mei_me devices and make e1000e power depend on mei_me */+tgp_mei_me=pci_get_device(PCI_VENDOR_ID_INTEL,0xa0e0,NULL);+if(!tgp_mei_me){+tgp_mei_me=pci_get_device(PCI_VENDOR_ID_INTEL,0x43e0,NULL);+if(!tgp_mei_me)+return;+}++if(device_link_add(&pdev->dev,&tgp_mei_me->dev,+DL_FLAG_PM_RUNTIME|DL_FLAG_RPM_ACTIVE|+DL_FLAG_AUTOREMOVE_CONSUMER))+pci_info(pdev,"System and runtime PM depends on %s\n",+pci_name(tgp_mei_me));++pci_dev_put(tgp_mei_me);+}+/***e1000_probe-DeviceInitializationRoutine*@pdev:PCIdeviceinformationstruct
@@ -7917,6 +7941,8 @@ static void __exit e1000_exit_module(void)}module_exit(e1000_exit_module);+/* Ensure device link can be created if e1000e is in the initramfs. */+MODULE_SOFTDEP("pre: mei_me");MODULE_AUTHOR("Intel Corporation, <linux.nics@intel.com>");MODULE_DESCRIPTION("Intel(R) PRO/1000 Network Driver");MODULE_LICENSE("GPL v2");
On TGL systems, PCI_COMMAND may randomly flip to 0 on system resume.
This is devastating to drivers that use pci_set_master(), like NVMe and
xHCI, to enable DMA in their resume routine, as pci_set_master() can
inadvertently disable PCI_COMMAND_IO and PCI_COMMAND_MEMORY, making
resources inaccessible.
The issue is reproducible on all kernel releases, but the situation is
exacerbated by commit 6cecf02e77ab ("Revert "e1000e: disable s0ix entry
and exit flows for ME systems"").
Seems like ME can do many things to other PCI devices until it's finally out of
ULP polling. So ensure e1000e PM ops are serialized by enforcing suspend/resume
order to workaround the issue.
Of course this will make system suspend and resume a bit slower, but we
probably need to settle on this workaround until ME is fully supported.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=212039
Signed-off-by: Kai-Heng Feng <redacted>
---
drivers/net/ethernet/intel/e1000e/netdev.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
@@ -7319,7 +7319,8 @@ static const struct net_device_ops e1000e_netdev_ops = {staticvoide1000e_create_device_links(structpci_dev*pdev){-structpci_dev*tgp_mei_me;+structpci_bus*bus=pdev->bus;+structpci_dev*tgp_mei_me,*p;/* Find TGP mei_me devices and make e1000e power depend on mei_me */tgp_mei_me=pci_get_device(PCI_VENDOR_ID_INTEL,0xa0e0,NULL);
@@ -7335,6 +7336,17 @@ static void e1000e_create_device_links(struct pci_dev *pdev)pci_info(pdev,"System and runtime PM depends on %s\n",pci_name(tgp_mei_me));+/* Find other devices in the SoC and make them depend on e1000e */+list_for_each_entry(p,&bus->devices,bus_list){+if(&p->dev==&pdev->dev||&p->dev==&tgp_mei_me->dev)+continue;++if(device_link_add(&p->dev,&pdev->dev,+DL_FLAG_AUTOREMOVE_SUPPLIER))+pci_info(p,"System PM depends on %s\n",+pci_name(pdev));+}+pci_dev_put(tgp_mei_me);}
Hello Kai-Heng,
I would agree with you here. I would suggest extending it also for other
PCH (at least ADP and MTP). The same controller on a different PCH.
We will be able to differentiate between boards via MAC type and submit
quirks if need.
Hello Kai-Heng,
I would agree with you here. I would suggest extending it also for other
PCH (at least ADP and MTP). The same controller on a different PCH.
We will be able to differentiate between boards via MAC type and submit
quirks if need.
Sure, will do in v2.
The issue patch [3/3] addresses may be fixed by [1], but I'll need to
dig the affected system out and do some testing.
Meanwhile, many users are affected by the RX issue patch [2/3]
addresses, so it'll be great if someone can review it.
[1] https://patchwork.ozlabs.org/project/intel-wired-lan/list/?series=250480
Kai-Heng
Many users report rather sluggish RX speed on TGP I219. Since
"intel_idle.max_cstate=1" doesn't help, so it's not caused by deeper
package C-state.
A workaround that always works is to make sure mei_me is runtime active
when e1000e is in use.
The root cause is still unknown, but since many users are affected by
the issue, implment the quirk in the driver as a temporary workaround.
Hello Kai-Heng,
First - thanks for the investigation of this problem. As I know CSME/AMT
not POR on Linux and not supported. Recently we started add support for
CSME/AMT none provisioned version (handshake with CSME in s0ix flow -
only CSME with s0ix will support). It is not related to rx bandwidth
problem.
I do not know how MEI driver affect 1Gbe driver - so, I would suggest to
involve our CSME engineer (alexander.usyskin@intel.com) and try to
investigate this problem.
Does this problem observed on Dell systems? As I heard no reproduction
on Intel's RVP platform.
Another question: does disable mei_me runpm solve your problem?
@@ -7317,6 +7317,27 @@ static const struct net_device_ops e1000e_netdev_ops = {.ndo_features_check=passthru_features_check,};+staticvoide1000e_create_device_links(structpci_dev*pdev)+{+structpci_dev*tgp_mei_me;++/* Find TGP mei_me devices and make e1000e power depend on mei_me */+tgp_mei_me=pci_get_device(PCI_VENDOR_ID_INTEL,0xa0e0,NULL);+if(!tgp_mei_me){+tgp_mei_me=pci_get_device(PCI_VENDOR_ID_INTEL,0x43e0,NULL);+if(!tgp_mei_me)+return;+}++if(device_link_add(&pdev->dev,&tgp_mei_me->dev,+DL_FLAG_PM_RUNTIME|DL_FLAG_RPM_ACTIVE|+DL_FLAG_AUTOREMOVE_CONSUMER))+pci_info(pdev,"System and runtime PM depends on %s\n",+pci_name(tgp_mei_me));++pci_dev_put(tgp_mei_me);+}+/***e1000_probe-DeviceInitializationRoutine*@pdev:PCIdeviceinformationstruct
@@ -7917,6 +7941,8 @@ static void __exit e1000_exit_module(void)}module_exit(e1000_exit_module);+/* Ensure device link can be created if e1000e is in the initramfs. */+MODULE_SOFTDEP("pre: mei_me");MODULE_AUTHOR("Intel Corporation, <linux.nics@intel.com>");MODULE_DESCRIPTION("Intel(R) PRO/1000 Network Driver");MODULE_LICENSE("GPL v2");
Hi Sasha,
On Wed, Jul 14, 2021 at 1:39 PM Sasha Neftin [off-list ref] wrote:
On 7/12/2021 16:34, Kai-Heng Feng wrote:
quoted
Many users report rather sluggish RX speed on TGP I219. Since
"intel_idle.max_cstate=1" doesn't help, so it's not caused by deeper
package C-state.
A workaround that always works is to make sure mei_me is runtime active
when e1000e is in use.
The root cause is still unknown, but since many users are affected by
the issue, implment the quirk in the driver as a temporary workaround.
Hello Kai-Heng,
First - thanks for the investigation of this problem. As I know CSME/AMT
not POR on Linux and not supported. Recently we started add support for
CSME/AMT none provisioned version (handshake with CSME in s0ix flow -
only CSME with s0ix will support). It is not related to rx bandwidth
problem.
I am aware that ME is not POR under Linux, so the commit message
states clearly that the patch is just a "temporary workaround".
Not every laptop can disable ME/AMT, and I don't think asking user to
fiddle with BIOS is a good thing, hence the patch.
I do not know how MEI driver affect 1Gbe driver - so, I would suggest to
involve our CSME engineer (alexander.usyskin@intel.com) and try to
investigate this problem.
Does this problem observed on Dell systems? As I heard no reproduction
on Intel's RVP platform.
Another question: does disable mei_me runpm solve your problem?
Yes, disabling runpm on mei_me can workaround the issue, and that's
essentially what this patch does by adding DL_FLAG_PM_RUNTIME |
DL_FLAG_RPM_ACTIVE flag.
Kai-Heng
@@ -7317,6 +7317,27 @@ static const struct net_device_ops e1000e_netdev_ops = {.ndo_features_check=passthru_features_check,};+staticvoide1000e_create_device_links(structpci_dev*pdev)+{+structpci_dev*tgp_mei_me;++/* Find TGP mei_me devices and make e1000e power depend on mei_me */+tgp_mei_me=pci_get_device(PCI_VENDOR_ID_INTEL,0xa0e0,NULL);+if(!tgp_mei_me){+tgp_mei_me=pci_get_device(PCI_VENDOR_ID_INTEL,0x43e0,NULL);+if(!tgp_mei_me)+return;+}++if(device_link_add(&pdev->dev,&tgp_mei_me->dev,+DL_FLAG_PM_RUNTIME|DL_FLAG_RPM_ACTIVE|+DL_FLAG_AUTOREMOVE_CONSUMER))+pci_info(pdev,"System and runtime PM depends on %s\n",+pci_name(tgp_mei_me));++pci_dev_put(tgp_mei_me);+}+/***e1000_probe-DeviceInitializationRoutine*@pdev:PCIdeviceinformationstruct
@@ -7917,6 +7941,8 @@ static void __exit e1000_exit_module(void)}module_exit(e1000_exit_module);+/* Ensure device link can be created if e1000e is in the initramfs. */+MODULE_SOFTDEP("pre: mei_me");MODULE_AUTHOR("Intel Corporation, <linux.nics@intel.com>");MODULE_DESCRIPTION("Intel(R) PRO/1000 Network Driver");MODULE_LICENSE("GPL v2");
Hello Kai-Heng,
I would agree with you here. I would suggest extending it also for other
PCH (at least ADP and MTP). The same controller on a different PCH.
We will be able to differentiate between boards via MAC type and submit
quirks if need.
Sure, will do in v2.
The issue patch [3/3] addresses may be fixed by [1], but I'll need to
dig the affected system out and do some testing.
Meanwhile, many users are affected by the RX issue patch [2/3]
addresses, so it'll be great if someone can review it.
[1] https://patchwork.ozlabs.org/project/intel-wired-lan/list/?series=250480
regards patches 2/3 and 3/3: looks it is not right place for temporary
w.a. Let's work with alexander.usyskin@intel.com to understand to root
cause and right place.
I do not know how MEI driver affect 1Gbe driver - so, I would suggest to
involve our CSME engineer (alexander.usyskin@intel.com) and try to
investigate this problem.
Does this problem observed on Dell systems? As I heard no reproduction
on Intel's RVP platform.
Another question: does disable mei_me runpm solve your problem?
Yes, disabling runpm on mei_me can workaround the issue, and that's
essentially what this patch does by adding DL_FLAG_PM_RUNTIME |
DL_FLAG_RPM_ACTIVE flag.
Kai-Heng
Hi, Kai-Heng,
If the goal of the patch is to essentially disable runpm on mei_me, then
why is the patch touching code in the e1000e driver?
I agree with Sasha Neftin; it seems like the wrong location, and the
wrong way to do it, even if it currently works. We need to understand
what causes runpm of mei_me to adversely affect LAN Rx, and for this we
need the involvement of mei_me owners.
--Dima
---------------------------------------------------------------------
Intel Israel (74) Limited
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
On Wed, Jul 14, 2021 at 5:06 PM Ruinskiy, Dima [off-list ref] wrote:
On 14/07/2021 9:28, Kai-Heng Feng wrote:
quoted
quoted
I do not know how MEI driver affect 1Gbe driver - so, I would suggest to
involve our CSME engineer (alexander.usyskin@intel.com) and try to
investigate this problem.
Does this problem observed on Dell systems? As I heard no reproduction
on Intel's RVP platform.
Another question: does disable mei_me runpm solve your problem?
Yes, disabling runpm on mei_me can workaround the issue, and that's
essentially what this patch does by adding DL_FLAG_PM_RUNTIME |
DL_FLAG_RPM_ACTIVE flag.
Kai-Heng
Hi, Kai-Heng,
If the goal of the patch is to essentially disable runpm on mei_me, then
why is the patch touching code in the e1000e driver?
We can put the workaround in e1000e, mei_me or as PCI quirk.
But since the bug itself manifests in e1000e, I think it's more
appropriate to put it here.
To be more specific, it doesn't disable runtime suspend on mei_me, it
makes mei_me the power supplier of e1000e.
So when e1000e can be runtime suspended (i.e. no link partner), mei_me
can also get runtime suspended too.
I agree with Sasha Neftin; it seems like the wrong location, and the
wrong way to do it, even if it currently works. We need to understand
what causes runpm of mei_me to adversely affect LAN Rx, and for this we
need the involvement of mei_me owners.
I think it's the right location, however I totally agree with your
other arguments.
There are many users already affected by this bug, so if a proper fix
isn't available for now, the temporary workaround can help here.
Kai-Heng
--Dima
---------------------------------------------------------------------
Intel Israel (74) Limited
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
On Wed, Jul 14, 2021 at 5:06 PM Ruinskiy, Dima [off-list ref] wrote:
quoted
On 14/07/2021 9:28, Kai-Heng Feng wrote:
quoted
quoted
I do not know how MEI driver affect 1Gbe driver - so, I would suggest to
involve our CSME engineer (alexander.usyskin@intel.com) and try to
investigate this problem.
Does this problem observed on Dell systems? As I heard no reproduction
on Intel's RVP platform.
Another question: does disable mei_me runpm solve your problem?
Yes, disabling runpm on mei_me can workaround the issue, and that's
essentially what this patch does by adding DL_FLAG_PM_RUNTIME |
DL_FLAG_RPM_ACTIVE flag.
Kai-Heng
Hi, Kai-Heng,
If the goal of the patch is to essentially disable runpm on mei_me, then
why is the patch touching code in the e1000e driver?
We can put the workaround in e1000e, mei_me or as PCI quirk.
But since the bug itself manifests in e1000e, I think it's more
appropriate to put it here.
To be more specific, it doesn't disable runtime suspend on mei_me, it
makes mei_me the power supplier of e1000e.
So when e1000e can be runtime suspended (i.e. no link partner), mei_me
can also get runtime suspended too.
quoted
I agree with Sasha Neftin; it seems like the wrong location, and the
wrong way to do it, even if it currently works. We need to understand
what causes runpm of mei_me to adversely affect LAN Rx, and for this we
need the involvement of mei_me owners.
I think it's the right location, however I totally agree with your
other arguments.
There are many users already affected by this bug, so if a proper fix
isn't available for now, the temporary workaround can help here.
Hello Kai-Heng,
The temporary workaround without root cause is vague. Please, let's work
with the manageability FW engineer (alexander.usyskin@intel.com) and
understand the root cause.
Also, here is interfere with runpm flow of CSME - we must their inputs.
sasha
Kai-Heng
quoted
--Dima
---------------------------------------------------------------------
Intel Israel (74) Limited
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
Hi Sasha,
On Mon, Jul 12, 2021 at 9:35 PM Kai-Heng Feng
[off-list ref] wrote:
On TGL systems, PCI_COMMAND may randomly flip to 0 on system resume.
This is devastating to drivers that use pci_set_master(), like NVMe and
xHCI, to enable DMA in their resume routine, as pci_set_master() can
inadvertently disable PCI_COMMAND_IO and PCI_COMMAND_MEMORY, making
resources inaccessible.
The issue is reproducible on all kernel releases, but the situation is
exacerbated by commit 6cecf02e77ab ("Revert "e1000e: disable s0ix entry
and exit flows for ME systems"").
Seems like ME can do many things to other PCI devices until it's finally out of
ULP polling. So ensure e1000e PM ops are serialized by enforcing suspend/resume
order to workaround the issue.
Of course this will make system suspend and resume a bit slower, but we
probably need to settle on this workaround until ME is fully supported.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=212039
Signed-off-by: Kai-Heng Feng <redacted>
Series "e1000e: Add handshake with the CSME to support s0ix" doesn't
fix the issue, so this patch is still needed.
Kai-Heng
@@ -7319,7 +7319,8 @@ static const struct net_device_ops e1000e_netdev_ops = {staticvoide1000e_create_device_links(structpci_dev*pdev){-structpci_dev*tgp_mei_me;+structpci_bus*bus=pdev->bus;+structpci_dev*tgp_mei_me,*p;/* Find TGP mei_me devices and make e1000e power depend on mei_me */tgp_mei_me=pci_get_device(PCI_VENDOR_ID_INTEL,0xa0e0,NULL);
@@ -7335,6 +7336,17 @@ static void e1000e_create_device_links(struct pci_dev *pdev)pci_info(pdev,"System and runtime PM depends on %s\n",pci_name(tgp_mei_me));+/* Find other devices in the SoC and make them depend on e1000e */+list_for_each_entry(p,&bus->devices,bus_list){+if(&p->dev==&pdev->dev||&p->dev==&tgp_mei_me->dev)+continue;++if(device_link_add(&p->dev,&pdev->dev,+DL_FLAG_AUTOREMOVE_SUPPLIER))+pci_info(p,"System PM depends on %s\n",+pci_name(pdev));+}+pci_dev_put(tgp_mei_me);}--
Hi Sasha,
On Mon, Jul 12, 2021 at 9:35 PM Kai-Heng Feng
[off-list ref] wrote:
quoted
On TGL systems, PCI_COMMAND may randomly flip to 0 on system resume.
This is devastating to drivers that use pci_set_master(), like NVMe and
xHCI, to enable DMA in their resume routine, as pci_set_master() can
inadvertently disable PCI_COMMAND_IO and PCI_COMMAND_MEMORY, making
resources inaccessible.
The issue is reproducible on all kernel releases, but the situation is
exacerbated by commit 6cecf02e77ab ("Revert "e1000e: disable s0ix entry
and exit flows for ME systems"").
Seems like ME can do many things to other PCI devices until it's finally out of
ULP polling. So ensure e1000e PM ops are serialized by enforcing suspend/resume
order to workaround the issue.
Of course this will make system suspend and resume a bit slower, but we
probably need to settle on this workaround until ME is fully supported.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=212039
Signed-off-by: Kai-Heng Feng <redacted>
Series "e1000e: Add handshake with the CSME to support s0ix" doesn't
fix the issue, so this patch is still needed.
Hello Kai-Heng,
This problem is still under investigation by the ME team. Let's wait for
their response.
Series "e1000e: Add handshake with the CSME to support s0ix" - support
only s0ix flow on AMT/CSME none provisioned systems and not related to
this problem.
@@ -7319,7 +7319,8 @@ static const struct net_device_ops e1000e_netdev_ops = {staticvoide1000e_create_device_links(structpci_dev*pdev){-structpci_dev*tgp_mei_me;+structpci_bus*bus=pdev->bus;+structpci_dev*tgp_mei_me,*p;/* Find TGP mei_me devices and make e1000e power depend on mei_me */tgp_mei_me=pci_get_device(PCI_VENDOR_ID_INTEL,0xa0e0,NULL);
@@ -7335,6 +7336,17 @@ static void e1000e_create_device_links(struct pci_dev *pdev)pci_info(pdev,"System and runtime PM depends on %s\n",pci_name(tgp_mei_me));+/* Find other devices in the SoC and make them depend on e1000e */+list_for_each_entry(p,&bus->devices,bus_list){+if(&p->dev==&pdev->dev||&p->dev==&tgp_mei_me->dev)+continue;++if(device_link_add(&p->dev,&pdev->dev,+DL_FLAG_AUTOREMOVE_SUPPLIER))+pci_info(p,"System PM depends on %s\n",+pci_name(pdev));+}+pci_dev_put(tgp_mei_me);}--