mbox series

[v2,0/3] SEV-SNP: Add KVM support for attestation

Message ID 20240628185244.3615928-1-michael.roth@amd.com (mailing list archive)
Headers show
Series SEV-SNP: Add KVM support for attestation | expand

Message

Michael Roth June 28, 2024, 6:52 p.m. UTC
This patchset is also available at:

  https://github.com/amdese/linux/commits/snp-guest-req-v2

and is based on top of kvm-coco-queue (ace0c64d8975):

  https://git.kernel.org/pub/scm/virt/kvm/kvm.git/log/?h=kvm-coco-queue

As discussed on the PUCK call a few weeks backs, I'm re-submitting as a
separate patchset the SNP guest request support that was originally part of
the SNP KVM base support patchset that's now in kvm/next and will be in
kernel 6.11. This support is needed to ensure fully compliance with GHCB
2.0 specification and to support attestation in general, so I'm hoping it
can also make it into 6.11.

I've dropped patches 4-5 from v1 of this series that implemented
KVM_EXIT_COCO and handling for userspace-provided certificate data so that
there's more time to get the API ironed out.


Overview
--------

The GHCB 2.0 specification defines 2 GHCB request types to allow SNP guests
to send encrypted messages/requests to firmware: SNP Guest Requests and SNP
Extended Guest Requests. These encrypted messages are used for things like
servicing attestation requests issued by the guest. Implementing support for
these is required to be fully GHCB-compliant.

For the most part, KVM only needs to handle forwarding these requests to
firmware (to be issued via the SNP_GUEST_REQUEST firmware command defined
in the SEV-SNP Firmware ABI), and then forwarding the encrypted response to
the guest.

However, in the case of SNP Extended Guest Requests, the host is also
able to provide the certificate data corresponding to the endorsement key
used by firmware to sign attestation report requests. This certificate data
is provided by userspace because:

  1) It allows for different keys/key types to be used for each particular
     guest with requiring any sort of KVM API to configure the certificate
     table in advance on a per-guest basis.

  2) It provides additional flexibility with how attestation requests might
     be handled during live migration where the certificate data for
     source/dest might be different.

  3) It allows all synchronization between certificates and firmware/signing
     key updates to be handled purely by userspace rather than requiring
     some in-kernel mechanism to facilitate it. [1]

To support fetching certificate data from userspace, a new KVM exit type will
be needed to handle fetching the certificate from userspace. An attempt to
define a new KVM_EXIT_COCO/KVM_EXIT_COCO_REQ_CERTS exit type to handle this
was introduced in v1 of this patchset, but is still being discussed by
community, so for now this patchset only implements a stub version of SNP
Extended Guest Requests that does not provide certificate data, but is still
enough to provide compliance with the GHCB 2.0 spec.

[1] https://lore.kernel.org/kvm/ZS614OSoritrE1d2@google.com/

Any feedback/review is appreciated.

Thanks!

-Mike

Changes since v1:

 * Fix cleanup path when handling firmware error (Liam, Sean)
 * Use bounce-pages for interacting with firmware rather than passing in the
   guest-provided pages directly. (Sean)
 * Drop SNP_GUEST_VMM_ERR_GENERIC and rely solely on firmware-provided error
   code to report any firmware error to the guest. (Sean)
 * Use kvm_clear_guest() to handle writing empty certificate table instead 
   of kvm_write_guest() (Sean)
 * Add additional comments in commit messages and throughout code to better
   explain the interactions with firmware/guest. (Sean)
 * Drop 4K-alignment restrictions on the guest-provided req/resp buffers,
   since the GHCB-spec only specifically requires they fit within 4K,
   not necessarily that they be 4K-aligned. Additionally, the bounce
   pages passed to firmware will be 4K-aligned regardless.

Changes since splitting this off from v15 SNP KVM patchset:

 * Address clang-reported warnings regarding uninitialized variables 
 * Address a memory leak of the request/response buffer pages, and refactor
   the code based on Sean's suggestions:
   https://lore.kernel.org/kvm/ZktbBRLXeOp9X6aH@google.com/
 * Fix SNP Extended Guest Request handling to only attempt to fetch
   certificates if handling MSG_REQ_REPORT (attestation) message types
 * Drop KVM_EXIT_VMGEXIT and introduce KVM_EXIT_COCO events instead
 * Refactor patch layout for easier handling/review

----------------------------------------------------------------
Brijesh Singh (1):
      KVM: SEV: Provide support for SNP_GUEST_REQUEST NAE event

Michael Roth (2):
      x86/sev: Move sev_guest.h into common SEV header
      KVM: SEV: Provide support for SNP_EXTENDED_GUEST_REQUEST NAE event

 arch/x86/include/asm/sev.h              |  48 ++++++++
 arch/x86/kvm/svm/sev.c                  | 187 ++++++++++++++++++++++++++++++++
 arch/x86/kvm/svm/svm.h                  |   3 +
 drivers/virt/coco/sev-guest/sev-guest.c |   2 -
 drivers/virt/coco/sev-guest/sev-guest.h |  63 -----------
 include/uapi/linux/sev-guest.h          |   3 +
 6 files changed, 241 insertions(+), 65 deletions(-)
 delete mode 100644 drivers/virt/coco/sev-guest/sev-guest.h

Comments

Michael Roth June 29, 2024, 12:48 a.m. UTC | #1
On Fri, Jun 28, 2024 at 01:52:41PM -0500, Michael Roth wrote:
> Changes since v1:
> 
>  * Fix cleanup path when handling firmware error (Liam, Sean)
>  * Use bounce-pages for interacting with firmware rather than passing in the
>    guest-provided pages directly. (Sean)
>  * Drop SNP_GUEST_VMM_ERR_GENERIC and rely solely on firmware-provided error
>    code to report any firmware error to the guest. (Sean)
>  * Use kvm_clear_guest() to handle writing empty certificate table instead 
>    of kvm_write_guest() (Sean)
>  * Add additional comments in commit messages and throughout code to better
>    explain the interactions with firmware/guest. (Sean)
>  * Drop 4K-alignment restrictions on the guest-provided req/resp buffers,
>    since the GHCB-spec only specifically requires they fit within 4K,

It turns out my eyeballs were not functional when I reached that
conclusion and it's clearly documented that the pages needed to be
4K-aligned in the GHCB spec.

With the current implementation, KVM can actually handle unaligned
req/resp GPAs thanks to the bounce buffers, but it should still be
enforced. So I will resend a v3 with this change, but leave a bit more
time in case there are other review comments for v2.

Thanks,

Mike

>    not necessarily that they be 4K-aligned. Additionally, the bounce
>    pages passed to firmware will be 4K-aligned regardless.
> 
> Changes since splitting this off from v15 SNP KVM patchset:
> 
>  * Address clang-reported warnings regarding uninitialized variables 
>  * Address a memory leak of the request/response buffer pages, and refactor
>    the code based on Sean's suggestions:
>    https://lore.kernel.org/kvm/ZktbBRLXeOp9X6aH@google.com/
>  * Fix SNP Extended Guest Request handling to only attempt to fetch
>    certificates if handling MSG_REQ_REPORT (attestation) message types
>  * Drop KVM_EXIT_VMGEXIT and introduce KVM_EXIT_COCO events instead
>  * Refactor patch layout for easier handling/review
> 
> ----------------------------------------------------------------
> Brijesh Singh (1):
>       KVM: SEV: Provide support for SNP_GUEST_REQUEST NAE event
> 
> Michael Roth (2):
>       x86/sev: Move sev_guest.h into common SEV header
>       KVM: SEV: Provide support for SNP_EXTENDED_GUEST_REQUEST NAE event
> 
>  arch/x86/include/asm/sev.h              |  48 ++++++++
>  arch/x86/kvm/svm/sev.c                  | 187 ++++++++++++++++++++++++++++++++
>  arch/x86/kvm/svm/svm.h                  |   3 +
>  drivers/virt/coco/sev-guest/sev-guest.c |   2 -
>  drivers/virt/coco/sev-guest/sev-guest.h |  63 -----------
>  include/uapi/linux/sev-guest.h          |   3 +
>  6 files changed, 241 insertions(+), 65 deletions(-)
>  delete mode 100644 drivers/virt/coco/sev-guest/sev-guest.h
> 
>