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
Fixes: cbd3d4f7c4e5 ("x86/sev: Check SEV-SNP features support")
CC: Borislav Petkov <[email protected]>
CC: David Rientjes <[email protected]>
CC: Michael Roth <[email protected]>
CC: Tom Lendacky <[email protected]>
CC: <[email protected]>
Signed-off-by: Nikunj A Dadhania <[email protected]>
---
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.
+
++-----------------+---------------+---------------+------------------+
+| 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
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) {
+ u64 exit_info_1 = SVM_VMGEXIT_TERM_REASON(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED);
+
+ if (sev_es_get_ghcb_version() < 2 ||
+ (!boot_ghcb && !early_setup_ghcb()))
+ sev_es_terminate(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");
+ }
+}
+
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)) | \
+ /* 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;
--
2.32.0
On 1/12/23 02:41, Nikunj A Dadhania 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
>
> Fixes: cbd3d4f7c4e5 ("x86/sev: Check SEV-SNP features support")
> CC: Borislav Petkov <[email protected]>
> CC: David Rientjes <[email protected]>
> CC: Michael Roth <[email protected]>
> CC: Tom Lendacky <[email protected]>
> CC: <[email protected]>
> Signed-off-by: Nikunj A Dadhania <[email protected]>
>
> ---
>
> 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.
> +
> ++-----------------+---------------+---------------+------------------+
> +| 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
> 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) {
> + u64 exit_info_1 = SVM_VMGEXIT_TERM_REASON(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED);
> +
> + if (sev_es_get_ghcb_version() < 2 ||
I would just use ghcb_version directly here.
But if you really want a function, that should be a pre-patch that also
updates the other areas that directly use ghcb_version.
Thanks,
Tom
> + (!boot_ghcb && !early_setup_ghcb()))
> + sev_es_terminate(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");
> + }
> +}
> +
> 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)) | \
> + /* 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;
On 12/01/23 20:12, Tom Lendacky wrote:
> On 1/12/23 02:41, Nikunj A Dadhania wrote:
>> +
>> + if (sev_es_get_ghcb_version() < 2 ||
>
> I would just use ghcb_version directly here.
Right, that will work.
> But if you really want a function, that should be a pre-patch that also updates the other areas that directly use ghcb_version.
There is only one such reference other than this one, will use ghcb_version directly.
Regards
Nikunj
On Thu, 12 Jan 2023 14:11:39 +0530
Nikunj A Dadhania <[email protected]> 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 <[email protected]>
> CC: David Rientjes <[email protected]>
> CC: Michael Roth <[email protected]>
> CC: Tom Lendacky <[email protected]>
> CC: <[email protected]>
> Signed-off-by: Nikunj A Dadhania <[email protected]>
>
> ---
>
> 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;
On 13/01/23 17:23, Zhi Wang wrote:
> On Thu, 12 Jan 2023 14:11:39 +0530
> Nikunj A Dadhania <[email protected]> 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.
That is strange, I will fix that.
>> +
>> +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.
Right, I will add it as part of the new function in my next revision:
static void __noreturn sev_es_ghcb_terminate(struct ghcb *ghcb, u64 exit_info_1, u64 exit_info_2)
Regards
Nikunj
On 13/01/23 17:23, Zhi Wang wrote:
> On Thu, 12 Jan 2023 14:11:39 +0530
> Nikunj A Dadhania <[email protected]> wrote:
>
>> 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.
That is not correct. None of these features are mandatory for the guest.
The hypervisor can enable this feature without the knowledge of guest
kernel support. So there should be a mechanism in the guest to detect this
and fail the boot if needed.
> If so, on the second row
> guest can boot without it. Some explanation?
In the first and second row, HV has not enabled the feature, so the
guest should boot fine irrespective of "Guest needs implementation".
>> +| No | No | No | Boot |
>> +| No | Yes | No | Boot |
>> ++-----------------+---------------+---------------+------------------+
>> +| 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.
Sure.
>> 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.
Sure.
Thanks for the review.
Nikunj
On 16/01/23 17:09, Zhi Wang wrote:
> On Mon, 16 Jan 2023 13:53:56 +0530
> "Nikunj A. Dadhania" <[email protected]> wrote:
>
>> On 13/01/23 17:23, Zhi Wang wrote:
>>> On Thu, 12 Jan 2023 14:11:39 +0530
>>> Nikunj A Dadhania <[email protected]> wrote:
>>>
>>
>>>> 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.
>>
>> That is not correct. None of these features are mandatory for the guest.
>> The hypervisor can enable this feature without the knowledge of guest
>> kernel support. So there should be a mechanism in the guest to detect this
>> and fail the boot if needed.
>>
>>> If so, on the second row
>>> guest can boot without it. Some explanation?
>>
>> In the first and second row, HV has not enabled the feature, so the
>> guest should boot fine irrespective of "Guest needs implementation".
>>
>
> Feel free to educate me if I understand correctly or not:
>
> There are two kinds of features in SEV_FEATURES:
>
> 1. Features that HV can freely enable/disable and they won't distrub the guest.
>
> HV | Guest needs impl | Guest has impl | Result
> Y/N N X (not necessary) Boot
>
> 2. Features that a guest has to be aware of and handle when HV enables them.
>
> HV | Guest needs impl | Guest has impl | Result
> N Y X (Dont care) Boot
> Y Y N Fail
> Y Y Y Boot
Yes, that is correct understanding.
Regards
Nikunj
On Mon, 16 Jan 2023 13:53:56 +0530
"Nikunj A. Dadhania" <[email protected]> wrote:
> On 13/01/23 17:23, Zhi Wang wrote:
> > On Thu, 12 Jan 2023 14:11:39 +0530
> > Nikunj A Dadhania <[email protected]> wrote:
> >
>
> >> 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.
>
> That is not correct. None of these features are mandatory for the guest.
> The hypervisor can enable this feature without the knowledge of guest
> kernel support. So there should be a mechanism in the guest to detect this
> and fail the boot if needed.
>
> > If so, on the second row
> > guest can boot without it. Some explanation?
>
> In the first and second row, HV has not enabled the feature, so the
> guest should boot fine irrespective of "Guest needs implementation".
>
Feel free to educate me if I understand correctly or not:
There are two kinds of features in SEV_FEATURES:
1. Features that HV can freely enable/disable and they won't distrub the guest.
HV | Guest needs impl | Guest has impl | Result
Y/N N X (not necessary) Boot
2. Features that a guest has to be aware of and handle when HV enables them.
HV | Guest needs impl | Guest has impl | Result
N Y X (Dont care) Boot
Y Y N Fail
Y Y Y Boot
> >> +| No | No | No | Boot |
>
> >> +| No | Yes | No | Boot |
>
>
> >> ++-----------------+---------------+---------------+------------------+
> >> +| 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.
>
> Sure.
>
> >> 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.
>
> Sure.
>
> Thanks for the review.
>
> Nikunj
>