lkml.org 
[lkml]   [2023]   [Jan]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v5] x86/sev: Add SEV-SNP guest feature negotiation support
    On Thu, 12 Jan 2023 14:11:39 +0530
    Nikunj A Dadhania <nikunj@amd.com> wrote:

    > The hypervisor can enable various new features (SEV_FEATURES[1:63])
    > and start the SNP guest. Some of these features need guest side
    > implementation. If any of these features are enabled without guest
    > side implementation, the behavior of the SNP guest will be undefined.
    > The SNP guest boot may fail in a non-obvious way making it difficult
    > to debug.
    >
    > Instead of allowing the guest to continue and have it fail randomly
    > later, detect this early and fail gracefully.
    >
    > SEV_STATUS MSR indicates features which the hypervisor has enabled.
    > While booting, SNP guests should ascertain that all the enabled
    > features have guest side implementation. In case any feature is not
    > implemented in the guest, the guest terminates booting with GHCB
    > protocol Non-Automatic Exit(NAE) termination request event[1]. Populate
    > SW_EXITINFO2 with mask of unsupported features that the hypervisor
    > can easily report to the user.
    >
    > More details in AMD64 APM[2] Vol 2: 15.34.10 SEV_STATUS MSR
    >
    > [1] https://developer.amd.com/wp-content/resources/56421.pdf
    > 4.1.13 Termination Request
    >
    > [2] https://www.amd.com/system/files/TechDocs/40332_4.05.pdf
    >

    The link of [2] is broken. Better update it.

    > Fixes: cbd3d4f7c4e5 ("x86/sev: Check SEV-SNP features support")
    > CC: Borislav Petkov <bp@alien8.de>
    > CC: David Rientjes <rientjes@google.com>
    > CC: Michael Roth <michael.roth@amd.com>
    > CC: Tom Lendacky <thomas.lendacky@amd.com>
    > CC: <stable@kernel.org>
    > Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
    >
    > ---
    >
    > Changes:
    > v4:
    > * Update comments and indentation
    > * Reuse GHCB MSR Protocol reason set
    > * Invalidate ghcb page before using
    > * GHCB protocol NAE termination event is available after version 2,
    > verify ghcb version before using the termination event.
    >
    > v3:
    > * Use GHCB protocol NAE termination event SEV-SNP feature(s)
    > not supported along with SW_EXITINFO2 containing mask of the
    > unsupported features. Need handling of this event on the HV.
    > * Add the SNP features check initialize_identity_maps() when the
    > boot GHCB page can be initialized and used.
    > * Fixed sphinx warnings in documentation
    >
    > v2:
    > * Updated Documentation/x86/amd-memory-encryption.rst
    > * Address review feedback from Boris/Tom
    >
    > v1:
    > * Dropped _ENABLED from the feature bits
    > * Use approprate macro/function names and move closer to the function
    > where it is used.
    > * More details added to the commit message and comments
    > * Fixed compilation issue
    > ---
    > Documentation/x86/amd-memory-encryption.rst | 36 ++++++++++++
    > arch/x86/boot/compressed/head_64.S | 9 +++
    > arch/x86/boot/compressed/misc.h | 1 +
    > arch/x86/boot/compressed/sev.c | 62 +++++++++++++++++++++
    > arch/x86/include/asm/msr-index.h | 20 +++++++
    > arch/x86/include/uapi/asm/svm.h | 6 ++
    > arch/x86/kernel/sev-shared.c | 5 ++
    > 7 files changed, 139 insertions(+)
    >
    > diff --git a/Documentation/x86/amd-memory-encryption.rst
    > b/Documentation/x86/amd-memory-encryption.rst index
    > a1940ebe7be5..b3adc39d7735 100644 ---
    > a/Documentation/x86/amd-memory-encryption.rst +++
    > b/Documentation/x86/amd-memory-encryption.rst @@ -95,3 +95,39 @@ by
    > supplying mem_encrypt=on on the kernel command line. However, if BIOS
    > does not enable SME, then Linux will not be able to activate memory
    > encryption, even if configured to do so by default or the mem_encrypt=on
    > command line parameter is specified. +
    > +Secure Nested Paging (SNP)
    > +==========================
    > +
    > +SEV-SNP introduces new features (SEV_FEATURES[1:63]) which can be
    > enabled +by the hypervisor for security enhancements. Some of these
    > features need +guest side implementation to function correctly. The
    > below table lists the +expected guest behavior with various possible
    > scenarios of guest/hypervisor +SNP feature support.
    > +
    "guest needs implementation" seems a little bit confusing. I suppose it
    means the feature is mandatory for the guest. If so, on the second row
    guest can boot without it. Some explanation?
    > ++-----------------+---------------+---------------+------------------+
    > +| Feature Enabled | Guest needs | Guest has | Guest boot |
    > +| by the HV | implementation| implementation| behaviour |
    > ++=================+===============+===============+==================+
    > +| No | No | No | Boot |
    > +| | | | |
    > ++-----------------+---------------+---------------+------------------+
    > +| No | Yes | No | Boot |
    > +| | | | |
    > ++-----------------+---------------+---------------+------------------+
    > +| No | Yes | Yes | Boot |
    > +| | | | |
    > ++-----------------+---------------+---------------+------------------+
    > +| Yes | No | No | Boot with |
    > +| | | | feature enabled |
    > ++-----------------+---------------+---------------+------------------+
    > +| Yes | Yes | No | Graceful boot |
    > +| | | | failure |
    > ++-----------------+---------------+---------------+------------------+
    > +| Yes | Yes | Yes | Boot with |
    > +| | | | feature enabled |
    > ++-----------------+---------------+---------------+------------------+
    > +
    > +More details in AMD64 APM[1] Vol 2: 15.34.10 SEV_STATUS MSR
    > +
    > +[1] https://www.amd.com/system/files/TechDocs/40332_4.05.pdf

    Probably update the link here as well.

    > diff --git a/arch/x86/boot/compressed/head_64.S
    > b/arch/x86/boot/compressed/head_64.S index a75712991df3..551d583fac9c
    > 100644 --- a/arch/x86/boot/compressed/head_64.S
    > +++ b/arch/x86/boot/compressed/head_64.S
    > @@ -557,6 +557,15 @@ SYM_FUNC_START_LOCAL_NOALIGN(.Lrelocated)
    > /* Pass boot_params to initialize_identity_maps() */
    > movq (%rsp), %rdi
    > call initialize_identity_maps
    > +
    > +#ifdef CONFIG_AMD_MEM_ENCRYPT
    > + /*
    > + * Now that the required page table mappings are established
    > and a
    > + * GHCB can be used, check for SNP guest/HV feature
    > compatibility.
    > + */
    > + call snp_check_features
    > +#endif
    > +
    > popq %rsi
    >
    > /*
    > diff --git a/arch/x86/boot/compressed/misc.h
    > b/arch/x86/boot/compressed/misc.h index 62208ec04ca4..593415e22614 100644
    > --- a/arch/x86/boot/compressed/misc.h
    > +++ b/arch/x86/boot/compressed/misc.h
    > @@ -126,6 +126,7 @@ static inline void console_init(void)
    >
    > #ifdef CONFIG_AMD_MEM_ENCRYPT
    > void sev_enable(struct boot_params *bp);
    > +void snp_check_features(void);
    > void sev_es_shutdown_ghcb(void);
    > extern bool sev_es_check_ghcb_fault(unsigned long address);
    > void snp_set_page_private(unsigned long paddr);
    > diff --git a/arch/x86/boot/compressed/sev.c
    > b/arch/x86/boot/compressed/sev.c index c93930d5ccbd..40c5f8bc733d 100644
    > --- a/arch/x86/boot/compressed/sev.c
    > +++ b/arch/x86/boot/compressed/sev.c
    > @@ -270,6 +270,68 @@ static void enforce_vmpl0(void)
    > sev_es_terminate(SEV_TERM_SET_LINUX,
    > GHCB_TERM_NOT_VMPL0); }
    >
    > +/*
    > + * SNP_FEATURES_IMPL_REQ is the mask of SNP features that will need
    > + * guest side implementation for proper functioning of the guest. If any
    > + * of these features are enabled in the hypervisor but are lacking guest
    > + * side implementation, the behavior of the guest will be undefined. The
    > + * guest could fail in non-obvious way making it difficult to debug.
    > + *
    > + * As the behavior of reserved feature bits is unknown to be on the
    > + * safe side add them to the required features mask.
    > + */
    > +#define SNP_FEATURES_IMPL_REQ (MSR_AMD64_SNP_VTOM |
    > \
    > + MSR_AMD64_SNP_REFLECT_VC |
    > \
    > + MSR_AMD64_SNP_RESTRICTED_INJ |
    > \
    > + MSR_AMD64_SNP_ALT_INJ |
    > \
    > + MSR_AMD64_SNP_DEBUG_SWAP |
    > \
    > + MSR_AMD64_SNP_VMPL_SSS |
    > \
    > + MSR_AMD64_SNP_SECURE_TSC |
    > \
    > + MSR_AMD64_SNP_VMGEXIT_PARAM |
    > \
    > + MSR_AMD64_SNP_VMSA_REG_PROTECTION
    > | \
    > + MSR_AMD64_SNP_RESERVED_BIT13 |
    > \
    > + MSR_AMD64_SNP_RESERVED_BIT15 |
    > \
    > + MSR_AMD64_SNP_RESERVED_MASK)
    > + > +/*
    > + * SNP_FEATURES_PRESENT is the mask of SNP features that are implemented
    > + * by the guest kernel. As and when a new feature is implemented in the
    > + * guest kernel, a corresponding bit should be added to the mask.
    > + */
    > +#define SNP_FEATURES_PRESENT (0)
    > +
    > +void snp_check_features(void)
    > +{
    > + u64 unsupported_features;
    > +
    > + if (!(sev_status & MSR_AMD64_SEV_SNP_ENABLED))
    > + return;
    > +
    > + /*
    > + * Terminate the boot if hypervisor has enabled any feature
    > + * lacking guest side implementation.
    > + */
    > + unsupported_features = sev_status & SNP_FEATURES_IMPL_REQ &
    > ~SNP_FEATURES_PRESENT;
    > + if (unsupported_features) {
    > + if (sev_es_get_ghcb_version() < 2 ||
    > + (!boot_ghcb && !early_setup_ghcb()))
    > + sev_es_terminate(SEV_TERM_SET_GEN,
    > GHCB_SNP_UNSUPPORTED); +

    ===
    > + u64 exit_info_1 =
    > SVM_VMGEXIT_TERM_REASON(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED); +
    > + vc_ghcb_invalidate(boot_ghcb);
    > + ghcb_set_sw_exit_code(boot_ghcb,
    > SVM_VMGEXIT_TERM_REQUEST);
    > + ghcb_set_sw_exit_info_1(boot_ghcb, exit_info_1);
    > + ghcb_set_sw_exit_info_2(boot_ghcb,
    > unsupported_features); +
    > + sev_es_wr_ghcb_msr(__pa(boot_ghcb));
    > + VMGEXIT();
    > +
    > + while (true)
    > + asm volatile("hlt\n" : : : "memory");
    ===

    This seems another approach to terminate the guest which can bring extra
    reason info compared to sev_es_terminate(). It would be better to wrap
    the above snippet into a function and call it here.

    > + }
    > +}
    > +
    > void sev_enable(struct boot_params *bp)
    > {
    > unsigned int eax, ebx, ecx, edx;
    > diff --git a/arch/x86/include/asm/msr-index.h
    > b/arch/x86/include/asm/msr-index.h index 37ff47552bcb..d3fe82c5d6b6
    > 100644 --- a/arch/x86/include/asm/msr-index.h
    > +++ b/arch/x86/include/asm/msr-index.h
    > @@ -566,6 +566,26 @@
    > #define MSR_AMD64_SEV_ES_ENABLED
    > BIT_ULL(MSR_AMD64_SEV_ES_ENABLED_BIT) #define
    > MSR_AMD64_SEV_SNP_ENABLED BIT_ULL(MSR_AMD64_SEV_SNP_ENABLED_BIT)
    > +/* SNP feature bits enabled by the hypervisor */
    > +#define MSR_AMD64_SNP_VTOM BIT_ULL(3)
    > +#define MSR_AMD64_SNP_REFLECT_VC BIT_ULL(4)
    > +#define MSR_AMD64_SNP_RESTRICTED_INJ BIT_ULL(5)
    > +#define MSR_AMD64_SNP_ALT_INJ BIT_ULL(6)
    > +#define MSR_AMD64_SNP_DEBUG_SWAP BIT_ULL(7)
    > +#define MSR_AMD64_SNP_PREVENT_HOST_IBS BIT_ULL(8)
    > +#define MSR_AMD64_SNP_BTB_ISOLATION BIT_ULL(9)
    > +#define MSR_AMD64_SNP_VMPL_SSS BIT_ULL(10)
    > +#define MSR_AMD64_SNP_SECURE_TSC BIT_ULL(11)
    > +#define MSR_AMD64_SNP_VMGEXIT_PARAM BIT_ULL(12)
    > +#define MSR_AMD64_SNP_IBS_VIRT BIT_ULL(14)
    > +#define MSR_AMD64_SNP_VMSA_REG_PROTECTION BIT_ULL(16)
    > +#define MSR_AMD64_SNP_SMT_PROTECTION BIT_ULL(17)
    > +
    > +/* SNP feature bits reserved for future use. */
    > +#define MSR_AMD64_SNP_RESERVED_BIT13 BIT_ULL(13)
    > +#define MSR_AMD64_SNP_RESERVED_BIT15 BIT_ULL(15)
    > +#define MSR_AMD64_SNP_RESERVED_MASK GENMASK_ULL(63, 18)
    > +
    > #define MSR_AMD64_VIRT_SPEC_CTRL 0xc001011f
    >
    > /* AMD Collaborative Processor Performance Control MSRs */
    > diff --git a/arch/x86/include/uapi/asm/svm.h
    > b/arch/x86/include/uapi/asm/svm.h index f69c168391aa..a04fe07eb9a8 100644
    > --- a/arch/x86/include/uapi/asm/svm.h
    > +++ b/arch/x86/include/uapi/asm/svm.h
    > @@ -116,6 +116,12 @@
    > #define SVM_VMGEXIT_AP_CREATE 1
    > #define SVM_VMGEXIT_AP_DESTROY 2
    > #define SVM_VMGEXIT_HV_FEATURES 0x8000fffd
    > +#define SVM_VMGEXIT_TERM_REQUEST 0x8000fffe
    > +#define SVM_VMGEXIT_TERM_REASON(reason_set, reason_code) \
    > + /* SW_EXITINFO1[3:0] */ \
    > + (((((u64)reason_set) & 0xf)) | \
    ^
    One extra space before 0xf should be removed.

    > + /* SW_EXITINFO1[11:4] */ \
    > + ((((u64)reason_code) & 0xff) << 4))
    > #define SVM_VMGEXIT_UNSUPPORTED_EVENT 0x8000ffff
    >
    > /* Exit code reserved for hypervisor/software use */
    > diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
    > index 3a5b0c9c4fcc..38ec3386984a 100644
    > --- a/arch/x86/kernel/sev-shared.c
    > +++ b/arch/x86/kernel/sev-shared.c
    > @@ -158,6 +158,11 @@ static bool sev_es_negotiate_protocol(void)
    > return true;
    > }
    >
    > +static u16 sev_es_get_ghcb_version(void)
    > +{
    > + return ghcb_version;
    > +}
    > +
    > static __always_inline void vc_ghcb_invalidate(struct ghcb *ghcb)
    > {
    > ghcb->save.sw_exit_code = 0;

    \
     
     \ /
      Last update: 2023-03-26 23:39    [W:5.203 / U:0.028 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site