Hi Randy

For some unknown reason my previous response said its taiting to be delivered.

On Fri, Aug 14, 2020 at 04:25:32PM -0700, Randy Dunlap wrote:

On 8/14/20 2:38 PM, Ashok Raj wrote:

...

When offlining CPU's, fixup_irqs() migrates all interrupts away from the

             CPUs,

I'll fix all these in the next rev.

Just waiting to hear back from Thomas if he has additional ones I can fix and resend v2.

Cheers, Ashok

...

outgoing CPU to an online CPU. Its always possible the device sent an

                             It's

...

interrupt to the previous CPU destination. Pending interrupt bit in IRR in lapic identifies such interrupts. apic_soft_disable() will not capture any

LAPIC

...

new interrupts in IRR. This causes interrupts from device to be lost during cpu offline. The issue was found when explicitly setting MSI affinity to a

CPU

...

CPU and immediately offlining it. It was simple to recreate with a USB ethernet device and doing I/O to it while the CPU is offlined. Lost interrupts happen even when Interrupt Remapping is enabled.

Current code does apic_soft_disable() before migrating interrupts.

native_cpu_disable() { ... apic_soft_disable(); cpu_disable_common(); --> fixup_irqs(); // Too late to capture anything in IRR. }

Just fliping the above call sequence seems to hit the IRR checks

   flipping

...

and the lost interrupt is fixed for both legacy MSI and when interrupt remapping is enabled.

Fixes: 60dcaad5736f ("x86/hotplug: Silence APIC and NMI when CPU is dead") Link: https://lore.kernel.org/lkml/875zdarr4h.fsf@nanos.tec.linutronix.de/ Signed-off-by: Ashok Raj ashok.raj@intel.com

To: linux-kernel@vger.kernel.org To: Thomas Gleixner tglx@linutronix.de Cc: Sukumar Ghorai sukumar.ghorai@intel.com Cc: Srikanth Nandamuri srikanth.nandamuri@intel.com Cc: Evan Green evgreen@chromium.org Cc: Mathias Nyman mathias.nyman@linux.intel.com Cc: Bjorn Helgaas bhelgaas@google.com Cc: stable@vger.kernel.org

---

arch/x86/kernel/smpboot.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c index ffbd9a3d78d8..278cc9f92f2f 100644 --- a/arch/x86/kernel/smpboot.c +++ b/arch/x86/kernel/smpboot.c @@ -1603,13 +1603,20 @@ int native_cpu_disable(void) if (ret) return ret;

• cpu_disable_common(); /*
  • Disable the local APIC. Otherwise IPI broadcasts will reach
  • it. It still responds normally to INIT, NMI, SMI, and SIPI

• * messages.
  

• * messages. Its important to do apic_soft_disable() after
  

            It's

...

• * fixup_irqs(), because fixup_irqs() called from cpu_disable_common()
  

• * depends on IRR being set. After apic_soft_disable() CPU preserves
  

• * currently set IRR/ISR but new interrupts will not set IRR.
  

• * This causes interrupts sent to outgoing cpu before completion
  

                                          CPU

...

• * of irq migration to be lost. Check SDM Vol 3 "10.4.7.2 Local
  

     IRQ

...

• * APIC State after It Has been Software Disabled" section for more
  

• * details.
  

  */ apic_soft_disable();

• cpu_disable_common();

return 0; }

thanks.

~Randy

Hi Evan

Some details below,

On Mon, Aug 17, 2020 at 11:12:17AM -0700, Evan Green wrote:

Hi Ashok, Thank you, Srikanth, and Sukumar for some very impressive debugging here.

On Fri, Aug 14, 2020 at 2:38 PM Ashok Raj ashok.raj@intel.com wrote:

...

When offlining CPU's, fixup_irqs() migrates all interrupts away from the outgoing CPU to an online CPU. Its always possible the device sent an interrupt to the previous CPU destination. Pending interrupt bit in IRR in lapic identifies such interrupts. apic_soft_disable() will not capture any new interrupts in IRR. This causes interrupts from device to be lost during cpu offline. The issue was found when explicitly setting MSI affinity to a CPU and immediately offlining it. It was simple to recreate with a USB ethernet device and doing I/O to it while the CPU is offlined. Lost interrupts happen even when Interrupt Remapping is enabled.

Current code does apic_soft_disable() before migrating interrupts.

native_cpu_disable() { ... apic_soft_disable(); cpu_disable_common(); --> fixup_irqs(); // Too late to capture anything in IRR. }

Just fliping the above call sequence seems to hit the IRR checks and the lost interrupt is fixed for both legacy MSI and when interrupt remapping is enabled.

Fixes: 60dcaad5736f ("x86/hotplug: Silence APIC and NMI when CPU is dead") Link: https://lore.kernel.org/lkml/875zdarr4h.fsf@nanos.tec.linutronix.de/ Signed-off-by: Ashok Raj ashok.raj@intel.com

To: linux-kernel@vger.kernel.org To: Thomas Gleixner tglx@linutronix.de Cc: Sukumar Ghorai sukumar.ghorai@intel.com Cc: Srikanth Nandamuri srikanth.nandamuri@intel.com Cc: Evan Green evgreen@chromium.org Cc: Mathias Nyman mathias.nyman@linux.intel.com Cc: Bjorn Helgaas bhelgaas@google.com Cc: stable@vger.kernel.org

---

arch/x86/kernel/smpboot.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c index ffbd9a3d78d8..278cc9f92f2f 100644 --- a/arch/x86/kernel/smpboot.c +++ b/arch/x86/kernel/smpboot.c @@ -1603,13 +1603,20 @@ int native_cpu_disable(void) if (ret) return ret;

•   cpu_disable_common();
    /*
     * Disable the local APIC. Otherwise IPI broadcasts will reach
     * it. It still responds normally to INIT, NMI, SMI, and SIPI
  

•    * messages.
  

I'm slightly unclear about whether interrupts are disabled at the core by this point or not. I followed native_cpu_disable() up to __cpu_disable(), up to take_cpu_down(). This is passed into a call to stop_machine_cpuslocked(), where interrupts get disabled at the core. So unless there's another path, it seems like interrupts are always disabled at the core by this point.

local_irq_disable() just does cli() which allows interrupts to trickle in to the IRR bits, and once you do sti() things would flow back for normal interrupt processing.

If interrupts are always disabled, then the comment above is a little

Disable interrupts is different from disabling LAPIC. Once you do the apic_soft_disable(), there is nothing flowing into the LAPIC except for INIT, NMI, SMI and SIPI messages.

This turns off the pipe for all other interrupts to enter LAPIC. Which is different from doing a cli().

obsolete, since we're not expecting to receive broadcast IPIs from here on out anyway. We could clean up that comment in this change.

If there is a path to here where interrupts are still enabled at the core, then we'd need to watch out, because this change now allows broadcast IPIs to come in during cpu_disable_common(). That could be bad. But I think that's not this case, so this should be ok.

Section SDM Vol3.b 10.4.7.2 says.

* The reception of any interrupt or transmission of any IPIs that are in progress when the local APIC is disabled are completed before the local APIC enters the software-disabled state.

It doesn't actually say much about broadcast IPI's, except broadcast NMI for instance, which is still permitted when cli() is set.

Hope this helps.

Cheers, Ashok