Re: [PATCH v1 1/5] KVM: SEV: Provide support for SNP_GUEST_REQUEST NAE event

[Tom Lendacky <thomas.lendacky@amd.com>]

On 6/27/24 10:35, Sean Christopherson wrote:
> On Thu, Jun 27, 2024, Tom Lendacky wrote:
>> On 6/26/24 14:54, Sean Christopherson wrote:
>>> On Wed, Jun 26, 2024, Michael Roth wrote:
>>>>> What about the host kernel though?  I don't see anything here that ensures resp_pfn
>>>>> isn't "regular" memory, i.e. that ensure the page isn't being concurrently accessed
>>>>> by the host kernel (or some other userspace process).
>>>>>
>>>>> Or is the "private" memory still accessible by the host?
>>>>
>>>> It's accessible, but it is immutable according to RMP table, so so it would
>>>> require KVM to be elsewhere doing a write to the page,
>>>
>>> I take it "immutable" means "read-only"?  If so, it would be super helpful to
>>> document that in the APM.  I assumed "immutable" only meant that the RMP entry
>>> itself is immutable, and that Assigned=AMD-SP is what prevented host accesses.
>>
>> Not quite. It depends on the page state associated with the page. For
>> example, Hypervisor-Fixed pages have the immutable bit set, but can be
>> read and written.
>>
>> The page states are documented in the SNP API (Chapter 5, Page
>> Management):
> 
> Heh, but then that section says:
> 
>   Pages in the Firmware state are owned by the firmware. Because the RMP.Immutable
>                                                          ^^^^^^^^^^^^^^^^^^^^^^^^^
>   bit is set, the hypervisor cannot write to Firmware pages nor alter the RMP entry
>   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>   with the RMPUPDATE instruction.
> 
> which to me very clearly suggests that the RMP.Immutable bit is what makes the
> page read-only.
> 
> Can you ask^Wbribe someone to add a "Table 11. Page State Properties" or something?
> E.g. to explicitly list out the read vs. write protections and the state of the
> page's data (encrypted, integrity-protected, zeroed?, etc).  I've read through
> all of "5.2 Page States" and genuinely have no clue as to what protections most
> of the states have.

I'll get with the document owner and provide that feedback and see what we
can do to remove some of the ambiguity and improve upon it.

> 
> Ah, never mind, I found "Table 15-39. RMP Memory Access Checks" in the APM.  FWIW,
> that somewhat contradicts this blurb from the SNP ABI spec:
> 
>   The content of a Context page is encrypted and integrity protected so that the
>   hypervisor cannot read or write to it.
> 
> I also find that statement confusing.  IIUC, the fact that the Context page is
> encrypted and integrity protected doesn't actually have anything to do with the
> host's ability to access the data.  The host _can_ read the data, but it will get
> ciphertext.  But the host can't write the data because the page isn't HV-owned.
> 
> Actually, isn't the intregrity protected part wrong?  I thought one of the benefits

The RMP protection is what helps provide the integrity protection. So if a
hypervisor tries to write to a non-hypervisor owned page, it will generate
an RMP #PF. If the page can't be RMPUPDATEd (the immutable bit is set for
the page in the RMP), then the page can't be written to by the hypervisor.

If the page can be RMPUPDATEd and made hypervisor-owned and was previously
PVALIDATEd, then a guest access (as private) will generate a #NPF. If the
hypervisor then assigns the page to the guest (after having possibly
written to the page), the guest will then get a #VC and detect that the
integrity of the page may have been compromised and it should take action.

> of SNP vs. ES is that the RMP means the VMSA doesn't have to be integrity protected,
> and so VMRUN and #VMEXIT transitions are faster because the CPU doesn't need to do
> as much work.

That is one of the benefits. A page can only be turned into a VMSA page
via an SNP_LAUNCH_UPDATE (becoming part of the guest measurement) or via
RMPADJUST (which can only be executed in the guest), making it a guest
private page. If an RMPUPDATE is done by the hypervisor, the VMSA bit will
be cleared and the VMSA won't be runnable anymore.

Thanks,
Tom