Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <CAFv23QmH4ooXJzX9A0CBObrETZgb9sT4dfh03NZA--fKfZnHDA@mail.gmail.com>
 <87fsixz8cb.wl-maz@kernel.org> <CAFv23Q=O29J4K_bdTi_SuThsEZN_SFNNz+bBPQSA+RGCANm6=w@mail.gmail.com>
In-Reply-To: <CAFv23Q=O29J4K_bdTi_SuThsEZN_SFNNz+bBPQSA+RGCANm6=w@mail.gmail.com>
From:   Kai-Heng Feng <kai.heng.feng@canonical.com>
Date:   Wed, 20 Jul 2022 11:16:39 +0800
Message-ID: <CAAd53p7GGjqpo97VP=uXnJQstKzxCf9Si+ZCD98UsbdJ7oS=SA@mail.gmail.com>
Subject: Re: There are not enough CPU0 APIC IRQs while doing IRQ migration
 during S3
To:     AceLan Kao <acelan.kao@canonical.com>
Cc:     Marc Zyngier <maz@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        "Linux-Kernel@Vger. Kernel. Org" <linux-kernel@vger.kernel.org>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Linux PM <linux-pm@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

[+Cc Rafael, linux-pm]

On Wed, Jul 20, 2022 at 10:53 AM AceLan Kao <acelan.kao@canonical.com> wrot=
e:
>
> Marc Zyngier <maz@kernel.org> =E6=96=BC 2022=E5=B9=B47=E6=9C=8819=E6=97=
=A5 =E9=80=B1=E4=BA=8C =E4=B8=8B=E5=8D=886:48=E5=AF=AB=E9=81=93=EF=BC=9A
> >
> > [- Jason]
> >
> > On Tue, 19 Jul 2022 06:55:21 +0100,
> > AceLan Kao <acelan.kao@canonical.com> wrote:
> > >
> > > HI all,
> > >
> > > I encountered an issue while doing S3, it shows below message and the=
n
> > > failed to enter S3
> > > [  106.731140] CPU 31 has 116 vectors, 85 available. Cannot disable C=
PU
> > > [  106.731551] ACPI: \_PR_.C01F: Found 2 idle states
> > > [  106.732610] Error taking CPU31 down: -28
> > > [  106.732612] Non-boot CPUs are not disabled
> > >
> > > CPU: AMD Ryzen Threadripper PRO 3955WX 16-Cores
> > > Kernel: v5.19-rc7
> > > There are 5 PCI to 4 type-c ports USB cards on the machine, and It
> > > wouldn't lead to the issue if only 4 cards are plugged. So, it looks
> > > like it can't handle 5 cards, and failed on the IRQ migration.
> > >
> > > The workaround provided by kaiheng is to release the irq while
> > > suspending and request irq while resuming.
> > > I'm wondering do we have a better solution for this kind of issue?
> > > Thanks.
> > >
> > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> > > index edc6881c8a1b..91c79b21cb57 100644
> > > --- a/drivers/usb/host/xhci.c
> > > +++ b/drivers/usb/host/xhci.c
> > > @@ -17,6 +17,7 @@
> > > #include <linux/slab.h>
> > > #include <linux/dmi.h>
> > > #include <linux/dma-mapping.h>
> > > +#include <linux/suspend.h>
> > >
> > > #include "xhci.h"
> > > #include "xhci-trace.h"
> > > @@ -1079,6 +1080,9 @@ int xhci_suspend(struct xhci_hcd *xhci, bool do=
_wakeup)
> > >                                __func__);
> > >        }
> > >
> > > +       if (pm_suspend_via_firmware())
> > > +               xhci_cleanup_msix(xhci);
> >
> > I'm a bit clueless when it comes to the combination of x86 and xhci,
> > but doesn't this prevent resuming on a xhci interrupt?
> The PCI cards provide 4 type-c USB ports, and in the beginning we
> found that removing one PCI card fixed the issue, so we were trying to
> fix the issue in xhci driver.
> The USB ports on the PCI cards can't resume the system from S3 even
> without the workaround,
> but the USB ports on the rear panel of the motherboard still work with
> the workaround.

The isn't xHCI specific. The issue here is that CPU0 APIC doesn't have
enough IRQ vector for ACPI S3 suspend.
Ideally we don't want to tear down IRQs during suspend, but for this
case minimizing IRQ numbers means successful S3.

So maybe we can have a suspend flow like this:
- At the beginning of suspend, check if there's enough free IRQ for
CPU0 migration.
- If there isn't enough free slots, hint drivers to tear down non-wake
IRQs. Maybe use a global variable if we don't want to add a new
parameter to current PM ops.
- If it's still not enough, abort suspend.

For suspend that doesn't unplug CPU like suspend-to-idle, no
modification is needed.
I wonder if that makes sense?

Kai-Heng