Messages in this thread | | | Date | Thu, 10 Jun 2021 17:26:28 +0100 | From | Marc Zyngier <> | Subject | Re: [PATCH] irqchip/gic-v3: Workaround inconsistent PMR setting on NMI entry |
| |
On Thu, 10 Jun 2021 16:59:30 +0100, Mark Rutland <mark.rutland@arm.com> wrote: > > Hi Marc, > > On Thu, Jun 10, 2021 at 03:57:31PM +0100, Marc Zyngier wrote: > > The arm64 entry code suffers from an annoying issue on taking > > a NMI, as it sets PMR to a value that actually allows IRQs > > to be acknowledged. This is done for consistency with other parts > > of the code, and is in the process of being fixed. This shouldn't > > be a problem, as we are not enabling interrupts whilst in NMI > > context. > > > > However, in the infortunate scenario that we took a spurious NMI > > (retired before the read of IAR) *and* that there is an IRQ pending > > at the same time, we'll ack the IRQ in NMI context. Too bad. > > > > In order to avoid deadlocks while running something like perf, > > teach the GICv3 driver about this situation: if we were in > > a context where no interrupt should have fired, transiently > > set PMR to a value that only allows NMIs before acking the pending > > interrupt, and restore the original value after that. > > > > This papers over the core issue for the time being, and makes > > NMIs great again. Sort of. > > > > Co-developed-by: Mark Rutland <mark.rutland@arm.com> > > According to the kernel documentation, a Co-developed-by should be > immediately followed by that developer's Signed-off-by, so FWIW: > > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > > ... unless you want to downgrade that to a Suggested-by, which is also > fine by me!
Nah, we both wasted too many grey bits on this one, and I want shared responsibility for it!
> > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > Having played about with a few options, I think this is the > simplest/cleanest thing we can do for now, and given it's all in one > place and "obviously correct", I think there's little risk that this > will break something else. So: > > Reviewed-by: Mark Rutland <mark.rutland@arm.com> > > We should probably also give this: > > Fixes: 4d6a38da8e79e94c ("arm64: entry: always set GIC_PRIO_PSR_I_SET during entry") > > ... since prior to that commit the `gic_prio_irq_setup` gunk would > prevent this specific problem (though other bits like > local_daif_{save,restore}()) would be broken in NMI paths.
Yup. I'll add that too and send it as a fix for -rc6.
Thanks,
M.
-- Without deviation from the norm, progress is not possible.
| |