On Thu, Aug 20, 2020 at 11:21:24AM -0700, Evan Green wrote: > > > > > > 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(). > > I understand the distinction. I was mostly musing on the difference in > behavior your change causes if this function is entered with > interrupts enabled at the core (ie sti()). But I think it never is, so > that thought is moot. > > I could never repro the issue reliably on comet lake after Thomas' > original fix. But I can still repro it easily on jasper lake. And this > patch fixes the issue for me on that platform. Thanks for the fix. > > Reviewed-by: Evan Green <evgreen@xxxxxxxxxxxx> > Tested-by: Evan Green <evgreen@xxxxxxxxxxxx> Thanks Evan for testing. I'll wait for thomas if he finds anything else that needs to be fixed and send a final v2 after fixing the typos and others identified by Randy. Cheers, Ashok
