[PATCH v1 00/18] arm64/nmi: Support for FEAT_NMI

[Mark Brown]

On Sun, Nov 06, 2022 at 11:38:51AM +0000, Marc Zyngier wrote:
> Mark Brown <broonie at kernel.org> wrote:

> > interrupts of any kind to add masking for NMIs.  This masking is not
> > added to our standard interrupt masking operations since that would
> > result in widespread masking of NMIs which would undermine their value.
> > Given that there's a large amount of kernel code a good proportion of
> > which I'm not terribly familar with it is likely that this area of the
> > series needs attention in review as there may be be be areas that have
> > been missed or misunderstood.

> Can you at least clarify why the no-quite-NMI cannot follow the
> pattern established by the pseudo-NMI support? I would have expected
> the critical sections to strictly map between the two so-called-NMI
> implementations.

That's fair, I'll expand on this topic in the cover for next version.
It is basically similar, there's a few differences where I couldn't
convince myself that we were intentionally masking NMIs like
el0_svc_common() and it did seem like we wanted to control things
separately during entry given the separate report path we have with the
architected version.  It's possible I've completely misunderstood this
one way or another.

I did look at a couple of alernatives that had more overlap.  I looked
at pulling the pNMI code out of the DAIF handling but given the
implementation the benefits seemed unclear for the complexity given that
it is actually working with DAIF.  I did also strongly consider putting
the NMI handling into the DAIF handling but that was making it difficult
to distinguish the handling of architected NMIs and the implicit
coverage didn't seem ideal, I could have another spin through that
approach though - I was on the edge with the approach there.  It sounds
like you'd really prefer that one.

> > Using this feature in KVM guests will require the implementation of vGIC
> > support which is not present in this series, and there is also no usage
> > of the feature at EL2.  While FEAT_NMI does add a new writable register
> > ALLINT the value is already context switched for EL1 via SPSR_EL2.ALLINT 
> > and we can't trap read access to the register so we don't manage the
> > write trap that is available in HCRX_EL2.TALLINT.  Guests can read from
> > the register anyway and should only be able to affect their own state.

> ... and create some architectural state that is impossible to manage
> and migrate. Great.

We can't already manage and migrate SPSR_EL2 (or SPSR_EL1 for that
matter)?  I might have missed part of how that's handled or used,
especially given that I've not actively worked through this yet, but if
we're not doing so then we've got other problems with for example
PSTATE.BTYPE or PSTATE.SSBS.  It looks like it's available and I'm not
seeing any filtering of the values or anything.

Note that ALLINT.ALLINT is similar to DAIF or SBSS, it's directly
mirroring the state of PSTATE.ALLINT.  The only access EL2 has to the
EL1 state for ALLINT is via SPSR, on entry to EL2 ALLINT is reset as per
the configuration in SCTLR_EL2.{SPINTMASK,NMI} and then the EL1 value is
restored as part of exception return.

> Frankly, we are accumulating a bunch of half implemented features
> (SME, SPE) for which there is no KVM support, and I don't think this
> is the correct direction of travel.

Yeah, it's not great.  For NMIs Lorenzo is looking at the GIC side, I'll
let him clarify his schedule.  Here it's more the case that implementing
the architecture side without an interrupt controller isn't useful (and
certainly not usefully testable) than anything else - my expectation is
that it should work fine, though from your comments above it sounds like
there will be issues with the handling of PSTATE that I've not realised.

For SME you should start seeing at least preparatory bits very soon
after the SME2 support lands, it's pretty much at the top of my list but
it didn't seem great to have yet another large, interdependent series in
flight at once so.  I had initially been hoping to get the KVM support
on the list in the gap between SME being merged and SME2 being public
but what happened instead is that it took so long to get review of SME
(with all the new ABI going on it's not 100% surprising it was slow but
it was much slower than I expected) that it ran into the SME2 release.
SME2 is only adding a single register of architectural state plus
instructions so I'm hoping it will go a bit quicker but OTOH we're
coming up to Christmas.  This backlog is the main reason I'm reluctant
to do any non-essential cleanups with the FP stuff at the minute - it
just increases the number of things that could cause conflicts or
dependency issues and those are likely to loose a release cycle.

I'm not aware of the situation with SPE.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20221109/b1c23bf3/attachment.sig>