Date: Thu, 27 Jun 2024 10:23:12 -0600
From: Peter Gonda <pgonda@...gle.com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Tom Lendacky <thomas.lendacky@....com>, Michael Roth <michael.roth@....com>, kvm@...r.kernel.org, 
	linux-coco@...ts.linux.dev, linux-kernel@...r.kernel.org, x86@...nel.org, 
	pbonzini@...hat.com, jroedel@...e.de, ashish.kalra@....com, bp@...en8.de, 
	pankaj.gupta@....com, liam.merwick@...cle.com, 
	Brijesh Singh <brijesh.singh@....com>, Alexey Kardashevskiy <aik@....com>
Subject: Re: [PATCH v1 1/5] KVM: SEV: Provide support for SNP_GUEST_REQUEST
 NAE event

On Thu, Jun 27, 2024 at 9:35 AM Sean Christopherson <seanjc@...gle.com> 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.
>
> 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
> 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.

The statement is fairly vague so its a bit confusing that the
encryption scheme on the Context page doesn't include integrity. In
reality the encryption is AES-XTS with a physical address tweak so no
integrity. The integrity comes purely from the RMP with SNP, but it's
still integrity protected right? So I don't think that part is wrong.

Powered by blists - more mailing lists