Message-ID: <YMeOnO4PBnvxEQEv@work-vm>
Date:   Mon, 14 Jun 2021 18:15:08 +0100
From:   "Dr. David Alan Gilbert" <dgilbert@...hat.com>
To:     Tom Lendacky <thomas.lendacky@....com>
Cc:     Brijesh Singh <brijesh.singh@....com>, x86@...nel.org,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        linux-efi@...r.kernel.org, platform-driver-x86@...r.kernel.org,
        linux-coco@...ts.linux.dev, linux-mm@...ck.org,
        linux-crypto@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Joerg Roedel <jroedel@...e.de>,
        "H. Peter Anvin" <hpa@...or.com>, Ard Biesheuvel <ardb@...nel.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Sean Christopherson <seanjc@...gle.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Andy Lutomirski <luto@...nel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Sergio Lopez <slp@...hat.com>, Peter Gonda <pgonda@...gle.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
        David Rientjes <rientjes@...gle.com>, tony.luck@...el.com,
        npmccallum@...hat.com
Subject: Re: [PATCH Part1 RFC v3 21/22] x86/sev: Register SNP guest request
 platform device

* Tom Lendacky (thomas.lendacky@....com) wrote:
> On 6/9/21 2:24 PM, Dr. David Alan Gilbert wrote:
> > * Brijesh Singh (brijesh.singh@....com) wrote:
> >> Version 2 of GHCB specification provides NAEs that can be used by the SNP
> >> guest to communicate with the PSP without risk from a malicious hypervisor
> >> who wishes to read, alter, drop or replay the messages sent.
> >>
> >> The hypervisor uses the SNP_GUEST_REQUEST command interface provided by
> >> the SEV-SNP firmware to forward the guest messages to the PSP.
> >>
> >> In order to communicate with the PSP, the guest need to locate the secrets
> >> page inserted by the hypervisor during the SEV-SNP guest launch. The
> >> secrets page contains the communication keys used to send and receive the
> >> encrypted messages between the guest and the PSP.
> >>
> >> The secrets page is located either through the setup_data cc_blob_address
> >> or EFI configuration table.
> >>
> >> Create a platform device that the SNP guest driver can bind to get the
> >> platform resources. The SNP guest driver can provide userspace interface
> >> to get the attestation report, key derivation etc.
> >>
> >> The helper snp_issue_guest_request() will be used by the drivers to
> >> send the guest message request to the hypervisor. The guest message header
> >> contains a message count. The message count is used in the IV. The
> >> firmware increments the message count by 1, and expects that next message
> >> will be using the incremented count.
> >>
> >> The helper snp_msg_seqno() will be used by driver to get and message
> >> sequence counter, and it will be automatically incremented by the
> >> snp_issue_guest_request(). The incremented value is be saved in the
> >> secrets page so that the kexec'ed kernel knows from where to begin.
> >>
> >> See SEV-SNP and GHCB spec for more details.
> >>
> >> Signed-off-by: Brijesh Singh <brijesh.singh@....com>
> >> ---
> >>  arch/x86/include/asm/sev.h      |  12 +++
> >>  arch/x86/include/uapi/asm/svm.h |   2 +
> >>  arch/x86/kernel/sev.c           | 176 ++++++++++++++++++++++++++++++++
> >>  arch/x86/platform/efi/efi.c     |   2 +
> >>  include/linux/efi.h             |   1 +
> >>  include/linux/sev-guest.h       |  76 ++++++++++++++
> >>  6 files changed, 269 insertions(+)
> >>  create mode 100644 include/linux/sev-guest.h
> >>
> 
> >> +u64 snp_msg_seqno(void)
> >> +{
> >> +	struct snp_secrets_page_layout *layout;
> >> +	u64 count;
> >> +
> >> +	layout = snp_map_secrets_page();
> >> +	if (layout == NULL)
> >> +		return 0;
> >> +
> >> +	/* Read the current message sequence counter from secrets pages */
> >> +	count = readl(&layout->os_area.msg_seqno_0);
> > 
> > Why is this seqno_0 - is that because it's the count of talking to the
> > PSP?
> 
> Yes, the sequence number is an ever increasing value that is used in
> communicating with the PSP. The PSP maintains the next expected sequence
> number and will reject messages which have a sequence number that is not
> in sync with the PSP. The 0 refers to the VMPL level. Each VMPL level has
> its own sequence number.

Can you just clarify; is that the VMPL of the caller or the destination?
What I'm partially asking here is whether it matters which VMPL the
kernel is running at (which I'm assuming could well be non-0)

> > 
> >> +	iounmap(layout);
> >> +
> >> +	/*
> >> +	 * The message sequence counter for the SNP guest request is a 64-bit value
> >> +	 * but the version 2 of GHCB specification defines the 32-bit storage for the
> >> +	 * it.
> >> +	 */
> >> +	if ((count + 1) >= INT_MAX)
> >> +		return 0;
> > 
> > Is that UINT_MAX?
> > 
> >> +
> >> +	return count + 1;
> >> +}
> >> +EXPORT_SYMBOL_GPL(snp_msg_seqno);
> >> +
> >> +static void snp_gen_msg_seqno(void)
> >> +{
> >> +	struct snp_secrets_page_layout *layout;
> >> +	u64 count;
> >> +
> >> +	layout = snp_map_secrets_page();
> >> +	if (layout == NULL)
> >> +		return;
> >> +
> >> +	/* Increment the sequence counter by 2 and save in secrets page. */
> >> +	count = readl(&layout->os_area.msg_seqno_0);
> >> +	count += 2;
> > 
> > Why 2 not 1 ?
> 
> The return message by the PSP also increments the sequence number, hence
> the increment by 2 instead of 1 for the next message to be submitted.

OK

Dave

> I'll let Brijesh address the other questions.
> 
> Thanks,
> Tom
> 
-- 
Dr. David Alan Gilbert / dgilbert@...hat.com / Manchester, UK

Powered by blists - more mailing lists