[svsm-devel] RFC: Alternate Injection SVSM support

[Kaplan, David]

[AMD Official Use Only - AMD Internal Distribution Only]

Thanks Jon for writing and sharing this!  I have a few questions/comments on the spec:

p.1: "SVSM involvement is required to present interrupts to the guest OS, so when a guest OS interrupt
becomes deliverable, it is necessary to cause the SVSM to run"

I was a bit confused by this wording (and worry others may be too).  With the Alternate Injection feature, the SVSM may "queue" interrupts for the guest OS, but the HW is actually what "delivers" the interrupt (meaning vectoring to the handler through the IDT).  Additionally, the term "becomes deliverable" here may not be clear.  I believe your intention is that when the HV decides to deliver a guest OS interrupt, it must cause the SVSM to run.  However when a previously sent interrupt that was pending (say because RFLAGS.IF=0 in the guest) becomes deliverable because the guest did STI, that does *not* cause the SVSM to run.  So I'd suggest perhaps rephrasing this to clarify this a bit more.

p.2: "The hypervisor is expected to cause the SVSM to run any time delivery of an
interrupt would result in injection of #HV to the SVSM."

This is just basic Restricted Injection behavior I believe...in order to send an interrupt to the SVSM the HV must inject a #HV into the SVSM and then run it.  I don't think any of that changes with this Alternate Injection spec (right?)

p.2: "Whenever the hypervisor delivers an interrupt for a lower VMPL (or an event, such as a request to
revert to host-managed APIC emulation), it will set the appropriate interrupt bit(s) in the extended
interrupt descriptor corresponding to the lower VMPL, and it will also set bit 1, 2, or 3 in the second
16-bit word of the SVSM's extended interrupt descriptor to indicate which VMPL has pending
interrupt evaluation work."

Shouldn't this be bits 8, 9, or 10?

p.3 extended interrupt descriptor

I would suggest considering defining names for each of these fields, and including a table showing the overall layout.  The spec currently refers to fields exclusively by bit position, which I personally found a bit hard to parse.  For instance, if bits 7:0 were named "SINGLE_VEC" (or whatever you'd like), then later in the section you could refer to that instead of referring to bits 7:0.

p.4: If the SVSM does know that the first component to execute will support the SVSM APIC Protocol, then
it may enable Alternate Injection prior to the first guest entry;

The statement about the SVSM having to know if the first component supports Alternate Injection is for security, correct?  That is, if the guest wants interrupt security then it'll want Alternate Injection enabled from the beginning.  If that's the case, then the SVSM *must* enable Alternate Injection prior to the first guest entry (not "may") true?

p.5 (Enabled/Locked/Disabled) state machine
I found this a bit confusing (at a minimum I'd politely request a diagram showing state transitions).  Usually "Locked" means you can't change, but in this case, the Disabled state seems to be more locked than the Locked state.

I had to read the part about the flows several times to understand it.  I think I get it now, but I am concerned about the fact that once Disabled, Alternate Injection can't be re-enabled.  Is that really required?  For instance, what if we're running a guest OS that doesn't understand alternate injection and then we kexec into a kernel that does.

Another idea to consider: What if you had a counter that increments when someone requested Alternate Injection and decremented when they asked to disable it.  And the feature is enabled when the counter is > 0.  I think that would also solve your problem of guest FW not knowing if the guest OS will want the feature or not.

For example: SVSM knows guest FW supports AlternateInjection so it starts the counter at 1.  The guest OS also supports it so it asks for it to be enabled raising the counter to 2.  Then guest FW disables Alternate Injection in ExitBootServices, decrementing the counter back to 1 so it stays enabled.

p. 5: "Consequently, the SVSM must be designed so that once it passes the point where
it commits to entering the lower VMPL, any delivery of #HV that indicates interrupt delivery to the
lower VMPL will cause it to cancel the VMGEXIT that would cause a return to the lower VMPL, so
that it can process the interrupt that has been signaled."

This probably a bit unrelated to the spec itself as this is really about the internal implementation of the SVSM.  But I think the important point is that the SVSM may be running (for whatever reason), the HV can send it a #HV, and then when the SVSM requests a VMPL change the HV should assume that the SVSM has processed the information from the #HV.

p.6 Disable Alternate Injection

Minor clarification but I would say that this simply disables Alternate Injection but doesn't necessarily require the HV to deliver events to the guest using direct event injection.  There are other interrupt delivery mechanisms too (including Secure AVIC as described in APM vol2).  So I'd perhaps suggest just saying that the HV has to deliver any events through other means, not necessarily direct event injection.

I think that's all I saw for now, the actual protocol itself seemed to make sense, but I'm guessing others may have feedback there.  And again, really appreciate the work on putting this together!

Thanks -David Kaplan

From: AMD-SEV-SNP <amd-sev-snp-bounces+david.kaplan=amd.com at lists.suse.com> On Behalf Of Jon Lange
Sent: Thursday, May 16, 2024 11:40 PM
To: amd-sev-snp at lists.suse.com; svsm-devel at coconut-svsm.dev
Subject: RFC: Alternate Injection SVSM support

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.

Attached is a specification I am proposing to handle support for Alternate Injection in SNP guests through the use of an SVSM.  This document proposes changes to the AMD GHCB specification and to the AMD SVSM specification to enable the negotiation required to make Alternate Injection work correctly.

Support for this protocol will not be possible in KVM until KVM is able to include native support for multiple VMPLs with separate interrupt sources for each.  That work is underway elsewhere, and my expectation is that this proposed specification will align with the capabilities that KVM will be introducing as part of the multi-VMPL support.

I will be submitting changes to COCONUT-SVSM to implement this protocol.  I welcome all feedback to the spec, and will update the code to match whatever spec changes are agreed upon.

-Jon


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.8bytes.org/pipermail/svsm-devel/attachments/20240523/e2115560/attachment-0001.htm>