This patchset is also available at:
https://github.com/amdese/linux/commits/snp-host-init-v1
and is based on top of linux-next tag next-20231222
These patches were originally included in v10 of the SNP KVM/hypervisor
patches[1], but have been split off from the general KVM support for easier
review and eventual merging into the x86 tree. They are based on linux-next
to help stay in sync with both tip and kvm-next.
There is 1 KVM-specific patch here since it is needed to avoid regressions
when running legacy SEV guests while the RMP table is enabled.
== OVERVIEW ==
AMD EPYC systems utilizing Zen 3 and newer microarchitectures add support
for a new feature called SEV-SNP, which adds Secure Nested Paging support
on top of the SEV/SEV-ES support already present on existing EPYC systems.
One of the main features of SNP is the addition of an RMP (Reverse Map)
table to enforce additional security protections for private guest memory.
This series primarily focuses on the various host initialization
requirements for enabling SNP on the system, while the actual KVM support
for running SNP guests is added as a separate series based on top of these
patches.
The basic requirements to initialize SNP support on a host when the feature
has been enabled in the BIOS are:
- Discovering and initializing the RMP table
- Initializing various MSRs to enable the capability across CPUs
- Various tasks to maintain legacy functionality on the system, such as:
- Setting up hooks for handling RMP-related changes for IOMMU pages
- Initializing SNP in the firmware via the CCP driver, and implement
additional requirements needed for continued operation of legacy
SEV/SEV-ES guests
Additionally some basic SEV ioctl interfaces are added to configure various
aspects of SNP-enabled firmwares via the CCP driver.
More details are available in the SEV-SNP Firmware ABI[2].
== KNOWN ISSUES ==
- When SNP is enabled, it has been observed that TMR allocation may fail when
reloading CCP driver after kexec. This is being investigated.
Feedback/review is very much appreciated!
-Mike
[1] https://lore.kernel.org/kvm/[email protected]/
[2] https://www.amd.com/en/developer/sev.html
Changes since being split off from v10 hypervisor patches:
* Move all host initialization patches to beginning of overall series and
post as a separate patchset. Include only KVM patches that are necessary
for maintaining legacy SVM/SEV functionality with SNP enabled.
(Paolo, Boris)
* Don't enable X86_FEATURE_SEV_SNP until all patches are in place to maintain
legacy SVM/SEV/SEV-ES functionality when RMP table and SNP firmware support
are enabled. (Paolo)
* Re-write how firmware-owned buffers are handled when dealing with legacy
SEV commands. Allocate on-demand rather than relying on pre-allocated pool,
use a descriptor format for handling nested/pointer params instead of
relying on macros. (Boris)
* Don't introduce sev-host.h, re-use sev.h (Boris)
* Various renames, cleanups, refactorings throughout the tree (Boris)
* Rework leaked pages handling (Vlastimil)
* Fix kernel-doc errors introduced by series (Boris)
* Fix warnings when onlining/offlining CPUs (Jeremi, Ashish)
* Ensure X86_FEATURE_SEV_SNP is cleared early enough that AutoIBRS will still
be enabled if RMP table support is not available/configured. (Tom)
* Only read the RMP base/end MSR values once via BSP (Boris)
* Handle IOMMU SNP setup automatically based on state machine rather than via
external caller (Boris)
----------------------------------------------------------------
Ashish Kalra (5):
iommu/amd: Don't rely on external callers to enable IOMMU SNP support
x86/mtrr: Don't print errors if MtrrFixDramModEn is set when SNP enabled
x86/sev: Introduce snp leaked pages list
iommu/amd: Clean up RMP entries for IOMMU pages during SNP shutdown
crypto: ccp: Add panic notifier for SEV/SNP firmware shutdown on kdump
Brijesh Singh (16):
x86/cpufeatures: Add SEV-SNP CPU feature
x86/sev: Add the host SEV-SNP initialization support
x86/sev: Add RMP entry lookup helpers
x86/fault: Add helper for dumping RMP entries
x86/traps: Define RMP violation #PF error code
x86/sev: Add helper functions for RMPUPDATE and PSMASH instruction
x86/sev: Invalidate pages from the direct map when adding them to the RMP table
crypto: ccp: Define the SEV-SNP commands
crypto: ccp: Add support to initialize the AMD-SP for SEV-SNP
crypto: ccp: Provide API to issue SEV and SNP commands
crypto: ccp: Handle the legacy TMR allocation when SNP is enabled
crypto: ccp: Handle legacy SEV commands when SNP is enabled
crypto: ccp: Add debug support for decrypting pages
KVM: SEV: Make AVIC backing, VMSA and VMCB memory allocation SNP safe
crypto: ccp: Add the SNP_PLATFORM_STATUS command
crypto: ccp: Add the SNP_SET_CONFIG command
Kim Phillips (1):
x86/speculation: Do not enable Automatic IBRS if SEV SNP is enabled
Michael Roth (2):
x86/fault: Dump RMP table information when RMP page faults occur
x86/cpufeatures: Enable/unmask SEV-SNP CPU feature
Tom Lendacky (2):
crypto: ccp: Handle non-volatile INIT_EX data when SNP is enabled
crypto: ccp: Add the SNP_COMMIT command
Documentation/virt/coco/sev-guest.rst | 51 ++
arch/x86/Kbuild | 2 +
arch/x86/include/asm/cpufeatures.h | 1 +
arch/x86/include/asm/disabled-features.h | 8 +-
arch/x86/include/asm/iommu.h | 1 +
arch/x86/include/asm/kvm-x86-ops.h | 1 +
arch/x86/include/asm/kvm_host.h | 1 +
arch/x86/include/asm/msr-index.h | 11 +-
arch/x86/include/asm/sev.h | 36 +
arch/x86/include/asm/trap_pf.h | 20 +-
arch/x86/kernel/cpu/amd.c | 20 +-
arch/x86/kernel/cpu/common.c | 7 +-
arch/x86/kernel/cpu/mtrr/generic.c | 3 +
arch/x86/kernel/crash.c | 7 +
arch/x86/kvm/lapic.c | 5 +-
arch/x86/kvm/svm/nested.c | 2 +-
arch/x86/kvm/svm/sev.c | 37 +-
arch/x86/kvm/svm/svm.c | 17 +-
arch/x86/kvm/svm/svm.h | 1 +
arch/x86/mm/fault.c | 5 +
arch/x86/virt/svm/Makefile | 3 +
arch/x86/virt/svm/sev.c | 513 +++++++++++++
drivers/crypto/ccp/sev-dev.c | 1216 +++++++++++++++++++++++++++---
drivers/crypto/ccp/sev-dev.h | 5 +
drivers/iommu/amd/amd_iommu.h | 1 -
drivers/iommu/amd/init.c | 120 ++-
include/linux/amd-iommu.h | 6 +-
include/linux/psp-sev.h | 337 ++++++++-
include/uapi/linux/psp-sev.h | 59 ++
tools/arch/x86/include/asm/cpufeatures.h | 1 +
30 files changed, 2359 insertions(+), 138 deletions(-)
create mode 100644 arch/x86/virt/svm/Makefile
create mode 100644 arch/x86/virt/svm/sev.c
From: Brijesh Singh <[email protected]>
Add CPU feature detection for Secure Encrypted Virtualization with
Secure Nested Paging. This feature adds a strong memory integrity
protection to help prevent malicious hypervisor-based attacks like
data replay, memory re-mapping, and more.
Since enabling the SNP CPU feature imposes a number of additional
requirements on host initialization and handling legacy firmware APIs
for SEV/SEV-ES guests, only introduce the CPU feature bit so that the
relevant handling can be added, but leave it disabled via a
disabled-features mask.
Once all the necessary changes needed to maintain legacy SEV/SEV-ES
support are introduced in subsequent patches, the SNP feature bit will
be unmasked/enabled.
Signed-off-by: Brijesh Singh <[email protected]>
Signed-off-by: Jarkko Sakkinen <[email protected]>
Signed-off-by: Ashish Kalra <[email protected]>
Signed-off-by: Michael Roth <[email protected]>
---
arch/x86/include/asm/cpufeatures.h | 1 +
arch/x86/include/asm/disabled-features.h | 4 +++-
arch/x86/kernel/cpu/amd.c | 5 +++--
tools/arch/x86/include/asm/cpufeatures.h | 1 +
4 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 29cb275a219d..9492dcad560d 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -442,6 +442,7 @@
#define X86_FEATURE_SEV (19*32+ 1) /* AMD Secure Encrypted Virtualization */
#define X86_FEATURE_VM_PAGE_FLUSH (19*32+ 2) /* "" VM Page Flush MSR is supported */
#define X86_FEATURE_SEV_ES (19*32+ 3) /* AMD Secure Encrypted Virtualization - Encrypted State */
+#define X86_FEATURE_SEV_SNP (19*32+ 4) /* AMD Secure Encrypted Virtualization - Secure Nested Paging */
#define X86_FEATURE_V_TSC_AUX (19*32+ 9) /* "" Virtual TSC_AUX */
#define X86_FEATURE_SME_COHERENT (19*32+10) /* "" AMD hardware-enforced cache coherency */
#define X86_FEATURE_DEBUG_SWAP (19*32+14) /* AMD SEV-ES full debug state swap support */
diff --git a/arch/x86/include/asm/disabled-features.h b/arch/x86/include/asm/disabled-features.h
index 702d93fdd10e..a864a5b208fa 100644
--- a/arch/x86/include/asm/disabled-features.h
+++ b/arch/x86/include/asm/disabled-features.h
@@ -117,6 +117,8 @@
#define DISABLE_IBT (1 << (X86_FEATURE_IBT & 31))
#endif
+#define DISABLE_SEV_SNP 0
+
/*
* Make sure to add features to the correct mask
*/
@@ -141,7 +143,7 @@
DISABLE_ENQCMD)
#define DISABLED_MASK17 0
#define DISABLED_MASK18 (DISABLE_IBT)
-#define DISABLED_MASK19 0
+#define DISABLED_MASK19 (DISABLE_SEV_SNP)
#define DISABLED_MASK20 0
#define DISABLED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 21)
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 9f42d1c59e09..9a17165dfe84 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -592,8 +592,8 @@ static void early_detect_mem_encrypt(struct cpuinfo_x86 *c)
* SME feature (set in scattered.c).
* If the kernel has not enabled SME via any means then
* don't advertise the SME feature.
- * For SEV: If BIOS has not enabled SEV then don't advertise the
- * SEV and SEV_ES feature (set in scattered.c).
+ * For SEV: If BIOS has not enabled SEV then don't advertise SEV and
+ * any additional functionality based on it.
*
* In all cases, since support for SME and SEV requires long mode,
* don't advertise the feature under CONFIG_X86_32.
@@ -628,6 +628,7 @@ static void early_detect_mem_encrypt(struct cpuinfo_x86 *c)
clear_sev:
setup_clear_cpu_cap(X86_FEATURE_SEV);
setup_clear_cpu_cap(X86_FEATURE_SEV_ES);
+ setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
}
}
diff --git a/tools/arch/x86/include/asm/cpufeatures.h b/tools/arch/x86/include/asm/cpufeatures.h
index f4542d2718f4..e58bd69356ee 100644
--- a/tools/arch/x86/include/asm/cpufeatures.h
+++ b/tools/arch/x86/include/asm/cpufeatures.h
@@ -437,6 +437,7 @@
#define X86_FEATURE_SEV (19*32+ 1) /* AMD Secure Encrypted Virtualization */
#define X86_FEATURE_VM_PAGE_FLUSH (19*32+ 2) /* "" VM Page Flush MSR is supported */
#define X86_FEATURE_SEV_ES (19*32+ 3) /* AMD Secure Encrypted Virtualization - Encrypted State */
+#define X86_FEATURE_SEV_SNP (19*32+ 4) /* AMD Secure Encrypted Virtualization - Secure Nested Paging */
#define X86_FEATURE_V_TSC_AUX (19*32+ 9) /* "" Virtual TSC_AUX */
#define X86_FEATURE_SME_COHERENT (19*32+10) /* "" AMD hardware-enforced cache coherency */
#define X86_FEATURE_DEBUG_SWAP (19*32+14) /* AMD SEV-ES full debug state swap support */
--
2.25.1
From: Kim Phillips <[email protected]>
Without SEV-SNP, Automatic IBRS protects only the kernel. But when
SEV-SNP is enabled, the Automatic IBRS protection umbrella widens to all
host-side code, including userspace. This protection comes at a cost:
reduced userspace indirect branch performance.
To avoid this performance loss, don't use Automatic IBRS on SEV-SNP
hosts. Fall back to retpolines instead.
Signed-off-by: Kim Phillips <[email protected]>
Acked-by: Dave Hansen <[email protected]>
[mdr: squash in changes from review discussion]
Signed-off-by: Michael Roth <[email protected]>
---
arch/x86/kernel/cpu/common.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 8f367d376520..6b253440ea72 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1355,8 +1355,13 @@ static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c)
/*
* AMD's AutoIBRS is equivalent to Intel's eIBRS - use the Intel feature
* flag and protect from vendor-specific bugs via the whitelist.
+ *
+ * Don't use AutoIBRS when SNP is enabled because it degrades host
+ * userspace indirect branch performance.
*/
- if ((ia32_cap & ARCH_CAP_IBRS_ALL) || cpu_has(c, X86_FEATURE_AUTOIBRS)) {
+ if ((ia32_cap & ARCH_CAP_IBRS_ALL) ||
+ (cpu_has(c, X86_FEATURE_AUTOIBRS) &&
+ !cpu_feature_enabled(X86_FEATURE_SEV_SNP))) {
setup_force_cpu_cap(X86_FEATURE_IBRS_ENHANCED);
if (!cpu_matches(cpu_vuln_whitelist, NO_EIBRS_PBRSB) &&
!(ia32_cap & ARCH_CAP_PBRSB_NO))
--
2.25.1
From: Ashish Kalra <[email protected]>
Currently the expectation is that the kernel will call
amd_iommu_snp_enable() to perform various checks and set the
amd_iommu_snp_en flag that the IOMMU uses to adjust its setup routines
to account for additional requirements on hosts where SNP is enabled.
This is somewhat fragile as it relies on this call being done prior to
IOMMU setup. It is more robust to just do this automatically as part of
IOMMU initialization, so rework the code accordingly.
There is still a need to export information about whether or not the
IOMMU is configured in a manner compatible with SNP, so relocate the
existing amd_iommu_snp_en flag so it can be used to convey that
information in place of the return code that was previously provided by
calls to amd_iommu_snp_enable().
While here, also adjust the kernel messages related to IOMMU SNP
enablement for consistency/grammar/clarity.
Suggested-by: Borislav Petkov (AMD) <[email protected]>
Signed-off-by: Ashish Kalra <[email protected]>
Co-developed-by: Michael Roth <[email protected]>
Signed-off-by: Michael Roth <[email protected]>
---
arch/x86/include/asm/iommu.h | 1 +
drivers/iommu/amd/amd_iommu.h | 1 -
drivers/iommu/amd/init.c | 69 ++++++++++++++++-------------------
include/linux/amd-iommu.h | 4 --
4 files changed, 32 insertions(+), 43 deletions(-)
diff --git a/arch/x86/include/asm/iommu.h b/arch/x86/include/asm/iommu.h
index 2fd52b65deac..3be2451e7bc8 100644
--- a/arch/x86/include/asm/iommu.h
+++ b/arch/x86/include/asm/iommu.h
@@ -10,6 +10,7 @@ extern int force_iommu, no_iommu;
extern int iommu_detected;
extern int iommu_merge;
extern int panic_on_overflow;
+extern bool amd_iommu_snp_en;
#ifdef CONFIG_SWIOTLB
extern bool x86_swiotlb_enable;
diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
index 8b3601f285fd..c970eae2313d 100644
--- a/drivers/iommu/amd/amd_iommu.h
+++ b/drivers/iommu/amd/amd_iommu.h
@@ -164,5 +164,4 @@ void amd_iommu_domain_set_pgtable(struct protection_domain *domain,
u64 *root, int mode);
struct dev_table_entry *get_dev_table(struct amd_iommu *iommu);
-extern bool amd_iommu_snp_en;
#endif
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index c83bd0c2a1c9..96a1a7fed470 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -3221,6 +3221,36 @@ static bool __init detect_ivrs(void)
return true;
}
+static void iommu_snp_enable(void)
+{
+#ifdef CONFIG_KVM_AMD_SEV
+ if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
+ return;
+ /*
+ * The SNP support requires that IOMMU must be enabled, and is
+ * not configured in the passthrough mode.
+ */
+ if (no_iommu || iommu_default_passthrough()) {
+ pr_err("SNP: IOMMU is disabled or configured in passthrough mode, SNP cannot be supported.\n");
+ return;
+ }
+
+ amd_iommu_snp_en = check_feature(FEATURE_SNP);
+ if (!amd_iommu_snp_en) {
+ pr_err("SNP: IOMMU SNP feature is not enabled, SNP cannot be supported.\n");
+ return;
+ }
+
+ pr_info("IOMMU SNP support is enabled.\n");
+
+ /* Enforce IOMMU v1 pagetable when SNP is enabled. */
+ if (amd_iommu_pgtable != AMD_IOMMU_V1) {
+ pr_warn("Forcing use of AMD IOMMU v1 page table due to SNP.\n");
+ amd_iommu_pgtable = AMD_IOMMU_V1;
+ }
+#endif
+}
+
/****************************************************************************
*
* AMD IOMMU Initialization State Machine
@@ -3256,6 +3286,7 @@ static int __init state_next(void)
break;
case IOMMU_ENABLED:
register_syscore_ops(&amd_iommu_syscore_ops);
+ iommu_snp_enable();
ret = amd_iommu_init_pci();
init_state = ret ? IOMMU_INIT_ERROR : IOMMU_PCI_INIT;
break;
@@ -3766,41 +3797,3 @@ int amd_iommu_pc_set_reg(struct amd_iommu *iommu, u8 bank, u8 cntr, u8 fxn, u64
return iommu_pc_get_set_reg(iommu, bank, cntr, fxn, value, true);
}
-
-#ifdef CONFIG_AMD_MEM_ENCRYPT
-int amd_iommu_snp_enable(void)
-{
- /*
- * The SNP support requires that IOMMU must be enabled, and is
- * not configured in the passthrough mode.
- */
- if (no_iommu || iommu_default_passthrough()) {
- pr_err("SNP: IOMMU is disabled or configured in passthrough mode, SNP cannot be supported");
- return -EINVAL;
- }
-
- /*
- * Prevent enabling SNP after IOMMU_ENABLED state because this process
- * affect how IOMMU driver sets up data structures and configures
- * IOMMU hardware.
- */
- if (init_state > IOMMU_ENABLED) {
- pr_err("SNP: Too late to enable SNP for IOMMU.\n");
- return -EINVAL;
- }
-
- amd_iommu_snp_en = check_feature(FEATURE_SNP);
- if (!amd_iommu_snp_en)
- return -EINVAL;
-
- pr_info("SNP enabled\n");
-
- /* Enforce IOMMU v1 pagetable when SNP is enabled. */
- if (amd_iommu_pgtable != AMD_IOMMU_V1) {
- pr_warn("Force to using AMD IOMMU v1 page table due to SNP\n");
- amd_iommu_pgtable = AMD_IOMMU_V1;
- }
-
- return 0;
-}
-#endif
diff --git a/include/linux/amd-iommu.h b/include/linux/amd-iommu.h
index dc7ed2f46886..7365be00a795 100644
--- a/include/linux/amd-iommu.h
+++ b/include/linux/amd-iommu.h
@@ -85,8 +85,4 @@ int amd_iommu_pc_get_reg(struct amd_iommu *iommu, u8 bank, u8 cntr, u8 fxn,
u64 *value);
struct amd_iommu *get_amd_iommu(unsigned int idx);
-#ifdef CONFIG_AMD_MEM_ENCRYPT
-int amd_iommu_snp_enable(void);
-#endif
-
#endif /* _ASM_X86_AMD_IOMMU_H */
--
2.25.1
From: Brijesh Singh <[email protected]>
The memory integrity guarantees of SEV-SNP are enforced through a new
structure called the Reverse Map Table (RMP). The RMP is a single data
structure shared across the system that contains one entry for every 4K
page of DRAM that may be used by SEV-SNP VMs. APM2 section 15.36 details
a number of steps needed to detect/enable SEV-SNP and RMP table support
on the host:
- Detect SEV-SNP support based on CPUID bit
- Initialize the RMP table memory reported by the RMP base/end MSR
registers and configure IOMMU to be compatible with RMP access
restrictions
- Set the MtrrFixDramModEn bit in SYSCFG MSR
- Set the SecureNestedPagingEn and VMPLEn bits in the SYSCFG MSR
- Configure IOMMU
RMP table entry format is non-architectural and it can vary by
processor. It is defined by the PPR. Restrict SNP support to CPU
models/families which are compatible with the current RMP table entry
format to guard against any undefined behavior when running on other
system types. Future models/support will handle this through an
architectural mechanism to allow for broader compatibility.
SNP host code depends on CONFIG_KVM_AMD_SEV config flag, which may be
enabled even when CONFIG_AMD_MEM_ENCRYPT isn't set, so update the
SNP-specific IOMMU helpers used here to rely on CONFIG_KVM_AMD_SEV
instead of CONFIG_AMD_MEM_ENCRYPT.
Signed-off-by: Brijesh Singh <[email protected]>
Co-developed-by: Ashish Kalra <[email protected]>
Signed-off-by: Ashish Kalra <[email protected]>
Co-developed-by: Tom Lendacky <[email protected]>
Signed-off-by: Tom Lendacky <[email protected]>
Co-developed-by: Michael Roth <[email protected]>
Signed-off-by: Michael Roth <[email protected]>
---
arch/x86/Kbuild | 2 +
arch/x86/include/asm/msr-index.h | 11 +-
arch/x86/include/asm/sev.h | 6 +
arch/x86/kernel/cpu/amd.c | 15 +++
arch/x86/virt/svm/Makefile | 3 +
arch/x86/virt/svm/sev.c | 219 +++++++++++++++++++++++++++++++
6 files changed, 255 insertions(+), 1 deletion(-)
create mode 100644 arch/x86/virt/svm/Makefile
create mode 100644 arch/x86/virt/svm/sev.c
diff --git a/arch/x86/Kbuild b/arch/x86/Kbuild
index 5a83da703e87..6a1f36df6a18 100644
--- a/arch/x86/Kbuild
+++ b/arch/x86/Kbuild
@@ -28,5 +28,7 @@ obj-y += net/
obj-$(CONFIG_KEXEC_FILE) += purgatory/
+obj-y += virt/svm/
+
# for cleaning
subdir- += boot tools
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index f1bd7b91b3c6..15ce1269f270 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -599,6 +599,8 @@
#define MSR_AMD64_SEV_ENABLED BIT_ULL(MSR_AMD64_SEV_ENABLED_BIT)
#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)
+#define MSR_AMD64_RMP_BASE 0xc0010132
+#define MSR_AMD64_RMP_END 0xc0010133
/* SNP feature bits enabled by the hypervisor */
#define MSR_AMD64_SNP_VTOM BIT_ULL(3)
@@ -709,7 +711,14 @@
#define MSR_K8_TOP_MEM2 0xc001001d
#define MSR_AMD64_SYSCFG 0xc0010010
#define MSR_AMD64_SYSCFG_MEM_ENCRYPT_BIT 23
-#define MSR_AMD64_SYSCFG_MEM_ENCRYPT BIT_ULL(MSR_AMD64_SYSCFG_MEM_ENCRYPT_BIT)
+#define MSR_AMD64_SYSCFG_MEM_ENCRYPT BIT_ULL(MSR_AMD64_SYSCFG_MEM_ENCRYPT_BIT)
+#define MSR_AMD64_SYSCFG_SNP_EN_BIT 24
+#define MSR_AMD64_SYSCFG_SNP_EN BIT_ULL(MSR_AMD64_SYSCFG_SNP_EN_BIT)
+#define MSR_AMD64_SYSCFG_SNP_VMPL_EN_BIT 25
+#define MSR_AMD64_SYSCFG_SNP_VMPL_EN BIT_ULL(MSR_AMD64_SYSCFG_SNP_VMPL_EN_BIT)
+#define MSR_AMD64_SYSCFG_MFDM_BIT 19
+#define MSR_AMD64_SYSCFG_MFDM BIT_ULL(MSR_AMD64_SYSCFG_MFDM_BIT)
+
#define MSR_K8_INT_PENDING_MSG 0xc0010055
/* C1E active bits in int pending message */
#define K8_INTP_C1E_ACTIVE_MASK 0x18000000
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 5b4a1ce3d368..1f59d8ba9776 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -243,4 +243,10 @@ static inline u64 snp_get_unsupported_features(u64 status) { return 0; }
static inline u64 sev_get_status(void) { return 0; }
#endif
+#ifdef CONFIG_KVM_AMD_SEV
+bool snp_probe_rmptable_info(void);
+#else
+static inline bool snp_probe_rmptable_info(void) { return false; }
+#endif
+
#endif
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 9a17165dfe84..0f0d425f0440 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -20,6 +20,7 @@
#include <asm/delay.h>
#include <asm/debugreg.h>
#include <asm/resctrl.h>
+#include <asm/sev.h>
#ifdef CONFIG_X86_64
# include <asm/mmconfig.h>
@@ -574,6 +575,20 @@ static void bsp_init_amd(struct cpuinfo_x86 *c)
break;
}
+ if (cpu_has(c, X86_FEATURE_SEV_SNP)) {
+ /*
+ * RMP table entry format is not architectural and it can vary by processor
+ * and is defined by the per-processor PPR. Restrict SNP support on the
+ * known CPU model and family for which the RMP table entry format is
+ * currently defined for.
+ */
+ if (!(c->x86 == 0x19 && c->x86_model <= 0xaf) &&
+ !(c->x86 == 0x1a && c->x86_model <= 0xf))
+ setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
+ else if (!snp_probe_rmptable_info())
+ setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
+ }
+
return;
warn:
diff --git a/arch/x86/virt/svm/Makefile b/arch/x86/virt/svm/Makefile
new file mode 100644
index 000000000000..ef2a31bdcc70
--- /dev/null
+++ b/arch/x86/virt/svm/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_KVM_AMD_SEV) += sev.o
diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
new file mode 100644
index 000000000000..ce7ede9065ed
--- /dev/null
+++ b/arch/x86/virt/svm/sev.c
@@ -0,0 +1,219 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * AMD SVM-SEV Host Support.
+ *
+ * Copyright (C) 2023 Advanced Micro Devices, Inc.
+ *
+ * Author: Ashish Kalra <[email protected]>
+ *
+ */
+
+#include <linux/cc_platform.h>
+#include <linux/printk.h>
+#include <linux/mm_types.h>
+#include <linux/set_memory.h>
+#include <linux/memblock.h>
+#include <linux/kernel.h>
+#include <linux/mm.h>
+#include <linux/cpumask.h>
+#include <linux/iommu.h>
+#include <linux/amd-iommu.h>
+
+#include <asm/sev.h>
+#include <asm/processor.h>
+#include <asm/setup.h>
+#include <asm/svm.h>
+#include <asm/smp.h>
+#include <asm/cpu.h>
+#include <asm/apic.h>
+#include <asm/cpuid.h>
+#include <asm/cmdline.h>
+#include <asm/iommu.h>
+
+/*
+ * The RMP entry format is not architectural. The format is defined in PPR
+ * Family 19h Model 01h, Rev B1 processor.
+ */
+struct rmpentry {
+ u64 assigned : 1,
+ pagesize : 1,
+ immutable : 1,
+ rsvd1 : 9,
+ gpa : 39,
+ asid : 10,
+ vmsa : 1,
+ validated : 1,
+ rsvd2 : 1;
+ u64 rsvd3;
+} __packed;
+
+/*
+ * The first 16KB from the RMP_BASE is used by the processor for the
+ * bookkeeping, the range needs to be added during the RMP entry lookup.
+ */
+#define RMPTABLE_CPU_BOOKKEEPING_SZ 0x4000
+
+static u64 probed_rmp_base, probed_rmp_size;
+static struct rmpentry *rmptable __ro_after_init;
+static u64 rmptable_max_pfn __ro_after_init;
+
+#undef pr_fmt
+#define pr_fmt(fmt) "SEV-SNP: " fmt
+
+static int __mfd_enable(unsigned int cpu)
+{
+ u64 val;
+
+ if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
+ return 0;
+
+ rdmsrl(MSR_AMD64_SYSCFG, val);
+
+ val |= MSR_AMD64_SYSCFG_MFDM;
+
+ wrmsrl(MSR_AMD64_SYSCFG, val);
+
+ return 0;
+}
+
+static __init void mfd_enable(void *arg)
+{
+ __mfd_enable(smp_processor_id());
+}
+
+static int __snp_enable(unsigned int cpu)
+{
+ u64 val;
+
+ if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
+ return 0;
+
+ rdmsrl(MSR_AMD64_SYSCFG, val);
+
+ val |= MSR_AMD64_SYSCFG_SNP_EN;
+ val |= MSR_AMD64_SYSCFG_SNP_VMPL_EN;
+
+ wrmsrl(MSR_AMD64_SYSCFG, val);
+
+ return 0;
+}
+
+static __init void snp_enable(void *arg)
+{
+ __snp_enable(smp_processor_id());
+}
+
+#define RMP_ADDR_MASK GENMASK_ULL(51, 13)
+
+bool snp_probe_rmptable_info(void)
+{
+ u64 max_rmp_pfn, calc_rmp_sz, rmp_sz, rmp_base, rmp_end;
+
+ rdmsrl(MSR_AMD64_RMP_BASE, rmp_base);
+ rdmsrl(MSR_AMD64_RMP_END, rmp_end);
+
+ if (!(rmp_base & RMP_ADDR_MASK) || !(rmp_end & RMP_ADDR_MASK)) {
+ pr_err("Memory for the RMP table has not been reserved by BIOS\n");
+ return false;
+ }
+
+ if (rmp_base > rmp_end) {
+ pr_err("RMP configuration not valid: base=%#llx, end=%#llx\n", rmp_base, rmp_end);
+ return false;
+ }
+
+ rmp_sz = rmp_end - rmp_base + 1;
+
+ /*
+ * Calculate the amount the memory that must be reserved by the BIOS to
+ * address the whole RAM, including the bookkeeping area. The RMP itself
+ * must also be covered.
+ */
+ max_rmp_pfn = max_pfn;
+ if (PHYS_PFN(rmp_end) > max_pfn)
+ max_rmp_pfn = PHYS_PFN(rmp_end);
+
+ calc_rmp_sz = (max_rmp_pfn << 4) + RMPTABLE_CPU_BOOKKEEPING_SZ;
+
+ if (calc_rmp_sz > rmp_sz) {
+ pr_err("Memory reserved for the RMP table does not cover full system RAM (expected 0x%llx got 0x%llx)\n",
+ calc_rmp_sz, rmp_sz);
+ return false;
+ }
+
+ probed_rmp_base = rmp_base;
+ probed_rmp_size = rmp_sz;
+
+ pr_info("RMP table physical range [0x%016llx - 0x%016llx]\n",
+ probed_rmp_base, probed_rmp_base + probed_rmp_size - 1);
+
+ return true;
+}
+
+static int __init __snp_rmptable_init(void)
+{
+ u64 rmptable_size;
+ void *rmptable_start;
+ u64 val;
+
+ if (!probed_rmp_size)
+ return 1;
+
+ rmptable_start = memremap(probed_rmp_base, probed_rmp_size, MEMREMAP_WB);
+ if (!rmptable_start) {
+ pr_err("Failed to map RMP table\n");
+ return 1;
+ }
+
+ /*
+ * Check if SEV-SNP is already enabled, this can happen in case of
+ * kexec boot.
+ */
+ rdmsrl(MSR_AMD64_SYSCFG, val);
+ if (val & MSR_AMD64_SYSCFG_SNP_EN)
+ goto skip_enable;
+
+ memset(rmptable_start, 0, probed_rmp_size);
+
+ /* Flush the caches to ensure that data is written before SNP is enabled. */
+ wbinvd_on_all_cpus();
+
+ /* MtrrFixDramModEn must be enabled on all the CPUs prior to enabling SNP. */
+ on_each_cpu(mfd_enable, NULL, 1);
+
+ on_each_cpu(snp_enable, NULL, 1);
+
+skip_enable:
+ rmptable_start += RMPTABLE_CPU_BOOKKEEPING_SZ;
+ rmptable_size = probed_rmp_size - RMPTABLE_CPU_BOOKKEEPING_SZ;
+
+ rmptable = (struct rmpentry *)rmptable_start;
+ rmptable_max_pfn = rmptable_size / sizeof(struct rmpentry) - 1;
+
+ return 0;
+}
+
+static int __init snp_rmptable_init(void)
+{
+ if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
+ return 0;
+
+ if (!amd_iommu_snp_en)
+ return 0;
+
+ if (__snp_rmptable_init())
+ goto nosnp;
+
+ cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "x86/rmptable_init:online", __snp_enable, NULL);
+
+ return 0;
+
+nosnp:
+ setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
+ return -ENOSYS;
+}
+
+/*
+ * This must be called after the IOMMU has been initialized.
+ */
+device_initcall(snp_rmptable_init);
--
2.25.1
From: Ashish Kalra <[email protected]>
SNP enabled platforms require the MtrrFixDramModeEn bit to be set across
all CPUs when SNP is enabled. Therefore, don't print error messages when
MtrrFixDramModeEn is set when bringing CPUs online.
Reported-by: Jeremi Piotrowski <[email protected]>
Closes: https://lore.kernel.org/kvm/[email protected]/
Signed-off-by: Ashish Kalra <[email protected]>
Signed-off-by: Michael Roth <[email protected]>
---
arch/x86/kernel/cpu/mtrr/generic.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c
index d3524778a545..422a4ddc2ab7 100644
--- a/arch/x86/kernel/cpu/mtrr/generic.c
+++ b/arch/x86/kernel/cpu/mtrr/generic.c
@@ -108,6 +108,9 @@ static inline void k8_check_syscfg_dram_mod_en(void)
(boot_cpu_data.x86 >= 0x0f)))
return;
+ if (cpu_feature_enabled(X86_FEATURE_SEV_SNP))
+ return;
+
rdmsr(MSR_AMD64_SYSCFG, lo, hi);
if (lo & K8_MTRRFIXRANGE_DRAM_MODIFY) {
pr_err(FW_WARN "MTRR: CPU %u: SYSCFG[MtrrFixDramModEn]"
--
2.25.1
On Sat, Dec 30, 2023 at 10:19:29AM -0600, Michael Roth wrote:
> From: Brijesh Singh <[email protected]>
>
> Add CPU feature detection for Secure Encrypted Virtualization with
> Secure Nested Paging. This feature adds a strong memory integrity
> protection to help prevent malicious hypervisor-based attacks like
> data replay, memory re-mapping, and more.
>
> Since enabling the SNP CPU feature imposes a number of additional
> requirements on host initialization and handling legacy firmware APIs
> for SEV/SEV-ES guests, only introduce the CPU feature bit so that the
> relevant handling can be added, but leave it disabled via a
> disabled-features mask.
>
> Once all the necessary changes needed to maintain legacy SEV/SEV-ES
> support are introduced in subsequent patches, the SNP feature bit will
> be unmasked/enabled.
>
> Signed-off-by: Brijesh Singh <[email protected]>
> Signed-off-by: Jarkko Sakkinen <[email protected]>
> Signed-off-by: Ashish Kalra <[email protected]>
> Signed-off-by: Michael Roth <[email protected]>
> ---
> arch/x86/include/asm/cpufeatures.h | 1 +
> arch/x86/include/asm/disabled-features.h | 4 +++-
> arch/x86/kernel/cpu/amd.c | 5 +++--
> tools/arch/x86/include/asm/cpufeatures.h | 1 +
> 4 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index 29cb275a219d..9492dcad560d 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -442,6 +442,7 @@
> #define X86_FEATURE_SEV (19*32+ 1) /* AMD Secure Encrypted Virtualization */
> #define X86_FEATURE_VM_PAGE_FLUSH (19*32+ 2) /* "" VM Page Flush MSR is supported */
> #define X86_FEATURE_SEV_ES (19*32+ 3) /* AMD Secure Encrypted Virtualization - Encrypted State */
> +#define X86_FEATURE_SEV_SNP (19*32+ 4) /* AMD Secure Encrypted Virtualization - Secure Nested Paging */
> #define X86_FEATURE_V_TSC_AUX (19*32+ 9) /* "" Virtual TSC_AUX */
> #define X86_FEATURE_SME_COHERENT (19*32+10) /* "" AMD hardware-enforced cache coherency */
> #define X86_FEATURE_DEBUG_SWAP (19*32+14) /* AMD SEV-ES full debug state swap support */
> diff --git a/arch/x86/include/asm/disabled-features.h b/arch/x86/include/asm/disabled-features.h
> index 702d93fdd10e..a864a5b208fa 100644
> --- a/arch/x86/include/asm/disabled-features.h
> +++ b/arch/x86/include/asm/disabled-features.h
> @@ -117,6 +117,8 @@
> #define DISABLE_IBT (1 << (X86_FEATURE_IBT & 31))
> #endif
>
> +#define DISABLE_SEV_SNP 0
I think you want this here if SEV_SNP should be initially disabled:
diff --git a/arch/x86/include/asm/disabled-features.h b/arch/x86/include/asm/disabled-features.h
index a864a5b208fa..5b2fab8ad262 100644
--- a/arch/x86/include/asm/disabled-features.h
+++ b/arch/x86/include/asm/disabled-features.h
@@ -117,7 +117,7 @@
#define DISABLE_IBT (1 << (X86_FEATURE_IBT & 31))
#endif
-#define DISABLE_SEV_SNP 0
+#define DISABLE_SEV_SNP (1 << (X86_FEATURE_SEV_SNP & 31))
/*
* Make sure to add features to the correct mask
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Sun, Dec 31, 2023 at 12:50:12PM +0100, Borislav Petkov wrote:
> On Sat, Dec 30, 2023 at 10:19:29AM -0600, Michael Roth wrote:
> > From: Brijesh Singh <[email protected]>
> >
> > Add CPU feature detection for Secure Encrypted Virtualization with
> > Secure Nested Paging. This feature adds a strong memory integrity
> > protection to help prevent malicious hypervisor-based attacks like
> > data replay, memory re-mapping, and more.
> >
> > Since enabling the SNP CPU feature imposes a number of additional
> > requirements on host initialization and handling legacy firmware APIs
> > for SEV/SEV-ES guests, only introduce the CPU feature bit so that the
> > relevant handling can be added, but leave it disabled via a
> > disabled-features mask.
> >
> > Once all the necessary changes needed to maintain legacy SEV/SEV-ES
> > support are introduced in subsequent patches, the SNP feature bit will
> > be unmasked/enabled.
> >
> > Signed-off-by: Brijesh Singh <[email protected]>
> > Signed-off-by: Jarkko Sakkinen <[email protected]>
> > Signed-off-by: Ashish Kalra <[email protected]>
> > Signed-off-by: Michael Roth <[email protected]>
> > ---
> > arch/x86/include/asm/cpufeatures.h | 1 +
> > arch/x86/include/asm/disabled-features.h | 4 +++-
> > arch/x86/kernel/cpu/amd.c | 5 +++--
> > tools/arch/x86/include/asm/cpufeatures.h | 1 +
> > 4 files changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> > index 29cb275a219d..9492dcad560d 100644
> > --- a/arch/x86/include/asm/cpufeatures.h
> > +++ b/arch/x86/include/asm/cpufeatures.h
> > @@ -442,6 +442,7 @@
> > #define X86_FEATURE_SEV (19*32+ 1) /* AMD Secure Encrypted Virtualization */
> > #define X86_FEATURE_VM_PAGE_FLUSH (19*32+ 2) /* "" VM Page Flush MSR is supported */
> > #define X86_FEATURE_SEV_ES (19*32+ 3) /* AMD Secure Encrypted Virtualization - Encrypted State */
> > +#define X86_FEATURE_SEV_SNP (19*32+ 4) /* AMD Secure Encrypted Virtualization - Secure Nested Paging */
> > #define X86_FEATURE_V_TSC_AUX (19*32+ 9) /* "" Virtual TSC_AUX */
> > #define X86_FEATURE_SME_COHERENT (19*32+10) /* "" AMD hardware-enforced cache coherency */
> > #define X86_FEATURE_DEBUG_SWAP (19*32+14) /* AMD SEV-ES full debug state swap support */
> > diff --git a/arch/x86/include/asm/disabled-features.h b/arch/x86/include/asm/disabled-features.h
> > index 702d93fdd10e..a864a5b208fa 100644
> > --- a/arch/x86/include/asm/disabled-features.h
> > +++ b/arch/x86/include/asm/disabled-features.h
> > @@ -117,6 +117,8 @@
> > #define DISABLE_IBT (1 << (X86_FEATURE_IBT & 31))
> > #endif
> >
> > +#define DISABLE_SEV_SNP 0
>
> I think you want this here if SEV_SNP should be initially disabled:
>
> diff --git a/arch/x86/include/asm/disabled-features.h b/arch/x86/include/asm/disabled-features.h
> index a864a5b208fa..5b2fab8ad262 100644
> --- a/arch/x86/include/asm/disabled-features.h
> +++ b/arch/x86/include/asm/disabled-features.h
> @@ -117,7 +117,7 @@
> #define DISABLE_IBT (1 << (X86_FEATURE_IBT & 31))
> #endif
>
> -#define DISABLE_SEV_SNP 0
> +#define DISABLE_SEV_SNP (1 << (X86_FEATURE_SEV_SNP & 31))
>
> /*
> * Make sure to add features to the correct mask
Sorry, I must have inverted things when I was squashing in the changes =\
I've gone ahead and force-pushed your fixup to the snp-host-init-v1
branch.
Thanks,
Mike
>
> --
> Regards/Gruss,
> Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette
On Sat, Dec 30, 2023 at 10:19:31AM -0600, Michael Roth wrote:
> +static void iommu_snp_enable(void)
> +{
> +#ifdef CONFIG_KVM_AMD_SEV
> + if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
> + return;
> + /*
> + * The SNP support requires that IOMMU must be enabled, and is
> + * not configured in the passthrough mode.
> + */
> + if (no_iommu || iommu_default_passthrough()) {
> + pr_err("SNP: IOMMU is disabled or configured in passthrough mode, SNP cannot be supported.\n");
> + return;
> + }
> +
> + amd_iommu_snp_en = check_feature(FEATURE_SNP);
> + if (!amd_iommu_snp_en) {
> + pr_err("SNP: IOMMU SNP feature is not enabled, SNP cannot be supported.\n");
> + return;
> + }
> +
> + pr_info("IOMMU SNP support is enabled.\n");
> +
> + /* Enforce IOMMU v1 pagetable when SNP is enabled. */
> + if (amd_iommu_pgtable != AMD_IOMMU_V1) {
> + pr_warn("Forcing use of AMD IOMMU v1 page table due to SNP.\n");
> + amd_iommu_pgtable = AMD_IOMMU_V1;
> + }
Kernel code usually says simple "<bla> enabled" not "<bla> is enabled".
Other than that, LGTM.
---
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 1ed2ef22a0fb..2f1517acaba0 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -3231,17 +3231,17 @@ static void iommu_snp_enable(void)
* not configured in the passthrough mode.
*/
if (no_iommu || iommu_default_passthrough()) {
- pr_err("SNP: IOMMU is disabled or configured in passthrough mode, SNP cannot be supported.\n");
+ pr_err("SNP: IOMMU disabled or configured in passthrough mode, SNP cannot be supported.\n");
return;
}
amd_iommu_snp_en = check_feature(FEATURE_SNP);
if (!amd_iommu_snp_en) {
- pr_err("SNP: IOMMU SNP feature is not enabled, SNP cannot be supported.\n");
+ pr_err("SNP: IOMMU SNP feature not enabled, SNP cannot be supported.\n");
return;
}
- pr_info("IOMMU SNP support is enabled.\n");
+ pr_info("IOMMU SNP support enabled.\n");
/* Enforce IOMMU v1 pagetable when SNP is enabled. */
if (amd_iommu_pgtable != AMD_IOMMU_V1) {
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Sat, Dec 30, 2023 at 10:19:31AM -0600, Michael Roth wrote:
> From: Ashish Kalra <[email protected]>
>
> Currently the expectation is that the kernel will call
> amd_iommu_snp_enable() to perform various checks and set the
> amd_iommu_snp_en flag that the IOMMU uses to adjust its setup routines
> to account for additional requirements on hosts where SNP is enabled.
>
> This is somewhat fragile as it relies on this call being done prior to
> IOMMU setup. It is more robust to just do this automatically as part of
> IOMMU initialization, so rework the code accordingly.
>
> There is still a need to export information about whether or not the
> IOMMU is configured in a manner compatible with SNP, so relocate the
> existing amd_iommu_snp_en flag so it can be used to convey that
> information in place of the return code that was previously provided by
> calls to amd_iommu_snp_enable().
>
> While here, also adjust the kernel messages related to IOMMU SNP
> enablement for consistency/grammar/clarity.
>
> Suggested-by: Borislav Petkov (AMD) <[email protected]>
> Signed-off-by: Ashish Kalra <[email protected]>
> Co-developed-by: Michael Roth <[email protected]>
> Signed-off-by: Michael Roth <[email protected]>
Acked-by: Joerg Roedel <[email protected]>
--
J?rg R?del
[email protected]
SUSE Software Solutions Germany GmbH
Frankenstra?e 146
90461 N?rnberg
Germany
(HRB 36809, AG N?rnberg)
Gesch?ftsf?hrer: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
On 30/12/2023 17:19, Michael Roth wrote:
> From: Brijesh Singh <[email protected]>
>
> The memory integrity guarantees of SEV-SNP are enforced through a new
> structure called the Reverse Map Table (RMP). The RMP is a single data
> structure shared across the system that contains one entry for every 4K
> page of DRAM that may be used by SEV-SNP VMs. APM2 section 15.36 details
> a number of steps needed to detect/enable SEV-SNP and RMP table support
> on the host:
>
> - Detect SEV-SNP support based on CPUID bit
> - Initialize the RMP table memory reported by the RMP base/end MSR
> registers and configure IOMMU to be compatible with RMP access
> restrictions
> - Set the MtrrFixDramModEn bit in SYSCFG MSR
> - Set the SecureNestedPagingEn and VMPLEn bits in the SYSCFG MSR
> - Configure IOMMU
>
> RMP table entry format is non-architectural and it can vary by
> processor. It is defined by the PPR. Restrict SNP support to CPU
> models/families which are compatible with the current RMP table entry
> format to guard against any undefined behavior when running on other
> system types. Future models/support will handle this through an
> architectural mechanism to allow for broader compatibility.
>
> SNP host code depends on CONFIG_KVM_AMD_SEV config flag, which may be
> enabled even when CONFIG_AMD_MEM_ENCRYPT isn't set, so update the
> SNP-specific IOMMU helpers used here to rely on CONFIG_KVM_AMD_SEV
> instead of CONFIG_AMD_MEM_ENCRYPT.
>
> Signed-off-by: Brijesh Singh <[email protected]>
> Co-developed-by: Ashish Kalra <[email protected]>
> Signed-off-by: Ashish Kalra <[email protected]>
> Co-developed-by: Tom Lendacky <[email protected]>
> Signed-off-by: Tom Lendacky <[email protected]>
> Co-developed-by: Michael Roth <[email protected]>
> Signed-off-by: Michael Roth <[email protected]>
> ---
> arch/x86/Kbuild | 2 +
> arch/x86/include/asm/msr-index.h | 11 +-
> arch/x86/include/asm/sev.h | 6 +
> arch/x86/kernel/cpu/amd.c | 15 +++
> arch/x86/virt/svm/Makefile | 3 +
> arch/x86/virt/svm/sev.c | 219 +++++++++++++++++++++++++++++++
> 6 files changed, 255 insertions(+), 1 deletion(-)
> create mode 100644 arch/x86/virt/svm/Makefile
> create mode 100644 arch/x86/virt/svm/sev.c
>
> diff --git a/arch/x86/Kbuild b/arch/x86/Kbuild
> index 5a83da703e87..6a1f36df6a18 100644
> --- a/arch/x86/Kbuild
> +++ b/arch/x86/Kbuild
> @@ -28,5 +28,7 @@ obj-y += net/
>
> obj-$(CONFIG_KEXEC_FILE) += purgatory/
>
> +obj-y += virt/svm/
> +
> # for cleaning
> subdir- += boot tools
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index f1bd7b91b3c6..15ce1269f270 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -599,6 +599,8 @@
> #define MSR_AMD64_SEV_ENABLED BIT_ULL(MSR_AMD64_SEV_ENABLED_BIT)
> #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)
> +#define MSR_AMD64_RMP_BASE 0xc0010132
> +#define MSR_AMD64_RMP_END 0xc0010133
>
> /* SNP feature bits enabled by the hypervisor */
> #define MSR_AMD64_SNP_VTOM BIT_ULL(3)
> @@ -709,7 +711,14 @@
> #define MSR_K8_TOP_MEM2 0xc001001d
> #define MSR_AMD64_SYSCFG 0xc0010010
> #define MSR_AMD64_SYSCFG_MEM_ENCRYPT_BIT 23
> -#define MSR_AMD64_SYSCFG_MEM_ENCRYPT BIT_ULL(MSR_AMD64_SYSCFG_MEM_ENCRYPT_BIT)
> +#define MSR_AMD64_SYSCFG_MEM_ENCRYPT BIT_ULL(MSR_AMD64_SYSCFG_MEM_ENCRYPT_BIT)
> +#define MSR_AMD64_SYSCFG_SNP_EN_BIT 24
> +#define MSR_AMD64_SYSCFG_SNP_EN BIT_ULL(MSR_AMD64_SYSCFG_SNP_EN_BIT)
> +#define MSR_AMD64_SYSCFG_SNP_VMPL_EN_BIT 25
> +#define MSR_AMD64_SYSCFG_SNP_VMPL_EN BIT_ULL(MSR_AMD64_SYSCFG_SNP_VMPL_EN_BIT)
> +#define MSR_AMD64_SYSCFG_MFDM_BIT 19
> +#define MSR_AMD64_SYSCFG_MFDM BIT_ULL(MSR_AMD64_SYSCFG_MFDM_BIT)
> +
> #define MSR_K8_INT_PENDING_MSG 0xc0010055
> /* C1E active bits in int pending message */
> #define K8_INTP_C1E_ACTIVE_MASK 0x18000000
> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
> index 5b4a1ce3d368..1f59d8ba9776 100644
> --- a/arch/x86/include/asm/sev.h
> +++ b/arch/x86/include/asm/sev.h
> @@ -243,4 +243,10 @@ static inline u64 snp_get_unsupported_features(u64 status) { return 0; }
> static inline u64 sev_get_status(void) { return 0; }
> #endif
>
> +#ifdef CONFIG_KVM_AMD_SEV
> +bool snp_probe_rmptable_info(void);
> +#else
> +static inline bool snp_probe_rmptable_info(void) { return false; }
> +#endif
> +
> #endif
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index 9a17165dfe84..0f0d425f0440 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -20,6 +20,7 @@
> #include <asm/delay.h>
> #include <asm/debugreg.h>
> #include <asm/resctrl.h>
> +#include <asm/sev.h>
>
> #ifdef CONFIG_X86_64
> # include <asm/mmconfig.h>
> @@ -574,6 +575,20 @@ static void bsp_init_amd(struct cpuinfo_x86 *c)
> break;
> }
>
> + if (cpu_has(c, X86_FEATURE_SEV_SNP)) {
> + /*
> + * RMP table entry format is not architectural and it can vary by processor
> + * and is defined by the per-processor PPR. Restrict SNP support on the
> + * known CPU model and family for which the RMP table entry format is
> + * currently defined for.
> + */
> + if (!(c->x86 == 0x19 && c->x86_model <= 0xaf) &&
> + !(c->x86 == 0x1a && c->x86_model <= 0xf))
> + setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
> + else if (!snp_probe_rmptable_info())
> + setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
Is there a really good reason to perform the snp_probe_smptable_info() check at this
point (instead of in snp_rmptable_init). snp_rmptable_init will also clear the cap
on failure, and bsp_init_amd() runs too early to allow for the kernel to allocate the
rmptable itself. I pointed out in the previous review that kernel allocation of rmptable
is necessary in SNP-host capable VMs in Azure.
> + }
> +
> return;
>
> warn:
> diff --git a/arch/x86/virt/svm/Makefile b/arch/x86/virt/svm/Makefile
> new file mode 100644
> index 000000000000..ef2a31bdcc70
> --- /dev/null
> +++ b/arch/x86/virt/svm/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +obj-$(CONFIG_KVM_AMD_SEV) += sev.o
> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> new file mode 100644
> index 000000000000..ce7ede9065ed
> --- /dev/null
> +++ b/arch/x86/virt/svm/sev.c
> @@ -0,0 +1,219 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * AMD SVM-SEV Host Support.
> + *
> + * Copyright (C) 2023 Advanced Micro Devices, Inc.
> + *
> + * Author: Ashish Kalra <[email protected]>
> + *
> + */
> +
> +#include <linux/cc_platform.h>
> +#include <linux/printk.h>
> +#include <linux/mm_types.h>
> +#include <linux/set_memory.h>
> +#include <linux/memblock.h>
> +#include <linux/kernel.h>
> +#include <linux/mm.h>
> +#include <linux/cpumask.h>
> +#include <linux/iommu.h>
> +#include <linux/amd-iommu.h>
> +
> +#include <asm/sev.h>
> +#include <asm/processor.h>
> +#include <asm/setup.h>
> +#include <asm/svm.h>
> +#include <asm/smp.h>
> +#include <asm/cpu.h>
> +#include <asm/apic.h>
> +#include <asm/cpuid.h>
> +#include <asm/cmdline.h>
> +#include <asm/iommu.h>
> +
> +/*
> + * The RMP entry format is not architectural. The format is defined in PPR
> + * Family 19h Model 01h, Rev B1 processor.
> + */
> +struct rmpentry {
> + u64 assigned : 1,
> + pagesize : 1,
> + immutable : 1,
> + rsvd1 : 9,
> + gpa : 39,
> + asid : 10,
> + vmsa : 1,
> + validated : 1,
> + rsvd2 : 1;
> + u64 rsvd3;
> +} __packed;
> +
> +/*
> + * The first 16KB from the RMP_BASE is used by the processor for the
> + * bookkeeping, the range needs to be added during the RMP entry lookup.
> + */
> +#define RMPTABLE_CPU_BOOKKEEPING_SZ 0x4000
> +
> +static u64 probed_rmp_base, probed_rmp_size;
> +static struct rmpentry *rmptable __ro_after_init;
> +static u64 rmptable_max_pfn __ro_after_init;
> +
> +#undef pr_fmt
> +#define pr_fmt(fmt) "SEV-SNP: " fmt
> +
> +static int __mfd_enable(unsigned int cpu)
> +{
> + u64 val;
> +
> + if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
> + return 0;
> +
> + rdmsrl(MSR_AMD64_SYSCFG, val);
> +
> + val |= MSR_AMD64_SYSCFG_MFDM;
> +
> + wrmsrl(MSR_AMD64_SYSCFG, val);
> +
> + return 0;
> +}
> +
> +static __init void mfd_enable(void *arg)
> +{
> + __mfd_enable(smp_processor_id());
> +}
> +
> +static int __snp_enable(unsigned int cpu)
> +{
> + u64 val;
> +
> + if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
> + return 0;
> +
> + rdmsrl(MSR_AMD64_SYSCFG, val);
> +
> + val |= MSR_AMD64_SYSCFG_SNP_EN;
> + val |= MSR_AMD64_SYSCFG_SNP_VMPL_EN;
> +
> + wrmsrl(MSR_AMD64_SYSCFG, val);
> +
> + return 0;
> +}
> +
> +static __init void snp_enable(void *arg)
> +{
> + __snp_enable(smp_processor_id());
> +}
> +
> +#define RMP_ADDR_MASK GENMASK_ULL(51, 13)
> +
> +bool snp_probe_rmptable_info(void)
> +{
> + u64 max_rmp_pfn, calc_rmp_sz, rmp_sz, rmp_base, rmp_end;
> +
> + rdmsrl(MSR_AMD64_RMP_BASE, rmp_base);
> + rdmsrl(MSR_AMD64_RMP_END, rmp_end);
> +
> + if (!(rmp_base & RMP_ADDR_MASK) || !(rmp_end & RMP_ADDR_MASK)) {
> + pr_err("Memory for the RMP table has not been reserved by BIOS\n");
> + return false;
> + }
> +
> + if (rmp_base > rmp_end) {
> + pr_err("RMP configuration not valid: base=%#llx, end=%#llx\n", rmp_base, rmp_end);
> + return false;
> + }
> +
> + rmp_sz = rmp_end - rmp_base + 1;
> +
> + /*
> + * Calculate the amount the memory that must be reserved by the BIOS to
> + * address the whole RAM, including the bookkeeping area. The RMP itself
> + * must also be covered.
> + */
> + max_rmp_pfn = max_pfn;
> + if (PHYS_PFN(rmp_end) > max_pfn)
> + max_rmp_pfn = PHYS_PFN(rmp_end);
> +
> + calc_rmp_sz = (max_rmp_pfn << 4) + RMPTABLE_CPU_BOOKKEEPING_SZ;
> +
> + if (calc_rmp_sz > rmp_sz) {
> + pr_err("Memory reserved for the RMP table does not cover full system RAM (expected 0x%llx got 0x%llx)\n",
> + calc_rmp_sz, rmp_sz);
> + return false;
> + }
> +
> + probed_rmp_base = rmp_base;
> + probed_rmp_size = rmp_sz;
> +
> + pr_info("RMP table physical range [0x%016llx - 0x%016llx]\n",
> + probed_rmp_base, probed_rmp_base + probed_rmp_size - 1);
> +
> + return true;
> +}
> +
> +static int __init __snp_rmptable_init(void)
> +{
> + u64 rmptable_size;
> + void *rmptable_start;
> + u64 val;
> +
> + if (!probed_rmp_size)
> + return 1;
> +
> + rmptable_start = memremap(probed_rmp_base, probed_rmp_size, MEMREMAP_WB);
> + if (!rmptable_start) {
> + pr_err("Failed to map RMP table\n");
> + return 1;
> + }
> +
> + /*
> + * Check if SEV-SNP is already enabled, this can happen in case of
> + * kexec boot.
> + */
> + rdmsrl(MSR_AMD64_SYSCFG, val);
> + if (val & MSR_AMD64_SYSCFG_SNP_EN)
> + goto skip_enable;
> +
> + memset(rmptable_start, 0, probed_rmp_size);
> +
> + /* Flush the caches to ensure that data is written before SNP is enabled. */
> + wbinvd_on_all_cpus();
> +
> + /* MtrrFixDramModEn must be enabled on all the CPUs prior to enabling SNP. */
> + on_each_cpu(mfd_enable, NULL, 1);
> +
> + on_each_cpu(snp_enable, NULL, 1);
> +
> +skip_enable:
> + rmptable_start += RMPTABLE_CPU_BOOKKEEPING_SZ;
> + rmptable_size = probed_rmp_size - RMPTABLE_CPU_BOOKKEEPING_SZ;
> +
> + rmptable = (struct rmpentry *)rmptable_start;
> + rmptable_max_pfn = rmptable_size / sizeof(struct rmpentry) - 1;
> +
> + return 0;
> +}
> +
> +static int __init snp_rmptable_init(void)
> +{
> + if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
> + return 0;
> +
> + if (!amd_iommu_snp_en)
> + return 0;
Looks better - do you think it'll be OK to add a X86_FEATURE_HYPERVISOR check at this point
later to account for SNP-host capable VMs with no access to an iommu?
Jeremi
> +
> + if (__snp_rmptable_init())
> + goto nosnp;
> +
> + cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "x86/rmptable_init:online", __snp_enable, NULL);
> +
> + return 0;
> +
> +nosnp:
> + setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
> + return -ENOSYS;
> +}
> +
> +/*
> + * This must be called after the IOMMU has been initialized.
> + */
> +device_initcall(snp_rmptable_init);
On Sat, Dec 30, 2023 at 10:19:32AM -0600, Michael Roth wrote:
> From: Brijesh Singh <[email protected]>
>
> The memory integrity guarantees of SEV-SNP are enforced through a new
> structure called the Reverse Map Table (RMP). The RMP is a single data
> structure shared across the system that contains one entry for every 4K
> page of DRAM that may be used by SEV-SNP VMs. APM2 section 15.36 details
> a number of steps needed to detect/enable SEV-SNP and RMP table support
> on the host:
>
> - Detect SEV-SNP support based on CPUID bit
> - Initialize the RMP table memory reported by the RMP base/end MSR
> registers and configure IOMMU to be compatible with RMP access
> restrictions
> - Set the MtrrFixDramModEn bit in SYSCFG MSR
> - Set the SecureNestedPagingEn and VMPLEn bits in the SYSCFG MSR
> - Configure IOMMU
>
> RMP table entry format is non-architectural and it can vary by
> processor. It is defined by the PPR. Restrict SNP support to CPU
> models/families which are compatible with the current RMP table entry
> format to guard against any undefined behavior when running on other
> system types. Future models/support will handle this through an
> architectural mechanism to allow for broader compatibility.
>
> SNP host code depends on CONFIG_KVM_AMD_SEV config flag, which may be
> enabled even when CONFIG_AMD_MEM_ENCRYPT isn't set, so update the
> SNP-specific IOMMU helpers used here to rely on CONFIG_KVM_AMD_SEV
> instead of CONFIG_AMD_MEM_ENCRYPT.
Small fixups to the commit message:
The memory integrity guarantees of SEV-SNP are enforced through a new
structure called the Reverse Map Table (RMP). The RMP is a single data
structure shared across the system that contains one entry for every 4K
page of DRAM that may be used by SEV-SNP VMs. The APM v2 section on
Secure Nested Paging (SEV-SNP) details a number of steps needed to
detect/enable SEV-SNP and RMP table support on the host:
- Detect SEV-SNP support based on CPUID bit
- Initialize the RMP table memory reported by the RMP base/end MSR
registers and configure IOMMU to be compatible with RMP access
restrictions
- Set the MtrrFixDramModEn bit in SYSCFG MSR
- Set the SecureNestedPagingEn and VMPLEn bits in the SYSCFG MSR
- Configure IOMMU
The RMP table entry format is non-architectural and it can vary by
processor. It is defined by the PPR document for each respective CPU
family. Restrict SNP support to CPU models/families which are compatible
with the current RMP table entry format to guard against any undefined
behavior when running on other system types. Future models/support will
handle this through an architectural mechanism to allow for broader
compatibility.
The SNP host code depends on CONFIG_KVM_AMD_SEV config flag which may
be enabled even when CONFIG_AMD_MEM_ENCRYPT isn't set, so update the
SNP-specific IOMMU helpers used here to rely on CONFIG_KVM_AMD_SEV
instead of CONFIG_AMD_MEM_ENCRYPT.
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index f1bd7b91b3c6..15ce1269f270 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -599,6 +599,8 @@
> #define MSR_AMD64_SEV_ENABLED BIT_ULL(MSR_AMD64_SEV_ENABLED_BIT)
> #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)
> +#define MSR_AMD64_RMP_BASE 0xc0010132
> +#define MSR_AMD64_RMP_END 0xc0010133
>
> /* SNP feature bits enabled by the hypervisor */
> #define MSR_AMD64_SNP_VTOM BIT_ULL(3)
> @@ -709,7 +711,14 @@
> #define MSR_K8_TOP_MEM2 0xc001001d
> #define MSR_AMD64_SYSCFG 0xc0010010
> #define MSR_AMD64_SYSCFG_MEM_ENCRYPT_BIT 23
> -#define MSR_AMD64_SYSCFG_MEM_ENCRYPT BIT_ULL(MSR_AMD64_SYSCFG_MEM_ENCRYPT_BIT)
> +#define MSR_AMD64_SYSCFG_MEM_ENCRYPT BIT_ULL(MSR_AMD64_SYSCFG_MEM_ENCRYPT_BIT)
> +#define MSR_AMD64_SYSCFG_SNP_EN_BIT 24
> +#define MSR_AMD64_SYSCFG_SNP_EN BIT_ULL(MSR_AMD64_SYSCFG_SNP_EN_BIT)
> +#define MSR_AMD64_SYSCFG_SNP_VMPL_EN_BIT 25
> +#define MSR_AMD64_SYSCFG_SNP_VMPL_EN BIT_ULL(MSR_AMD64_SYSCFG_SNP_VMPL_EN_BIT)
> +#define MSR_AMD64_SYSCFG_MFDM_BIT 19
> +#define MSR_AMD64_SYSCFG_MFDM BIT_ULL(MSR_AMD64_SYSCFG_MFDM_BIT)
> +
Fix the vertical alignment:
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 15ce1269f270..f482bc6a5ae7 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -710,14 +710,14 @@
#define MSR_K8_TOP_MEM1 0xc001001a
#define MSR_K8_TOP_MEM2 0xc001001d
#define MSR_AMD64_SYSCFG 0xc0010010
-#define MSR_AMD64_SYSCFG_MEM_ENCRYPT_BIT 23
-#define MSR_AMD64_SYSCFG_MEM_ENCRYPT BIT_ULL(MSR_AMD64_SYSCFG_MEM_ENCRYPT_BIT)
-#define MSR_AMD64_SYSCFG_SNP_EN_BIT 24
+#define MSR_AMD64_SYSCFG_MEM_ENCRYPT_BIT 23
+#define MSR_AMD64_SYSCFG_MEM_ENCRYPT BIT_ULL(MSR_AMD64_SYSCFG_MEM_ENCRYPT_BIT)
+#define MSR_AMD64_SYSCFG_SNP_EN_BIT 24
#define MSR_AMD64_SYSCFG_SNP_EN BIT_ULL(MSR_AMD64_SYSCFG_SNP_EN_BIT)
-#define MSR_AMD64_SYSCFG_SNP_VMPL_EN_BIT 25
-#define MSR_AMD64_SYSCFG_SNP_VMPL_EN BIT_ULL(MSR_AMD64_SYSCFG_SNP_VMPL_EN_BIT)
-#define MSR_AMD64_SYSCFG_MFDM_BIT 19
-#define MSR_AMD64_SYSCFG_MFDM BIT_ULL(MSR_AMD64_SYSCFG_MFDM_BIT)
+#define MSR_AMD64_SYSCFG_SNP_VMPL_EN_BIT 25
+#define MSR_AMD64_SYSCFG_SNP_VMPL_EN BIT_ULL(MSR_AMD64_SYSCFG_SNP_VMPL_EN_BIT)
+#define MSR_AMD64_SYSCFG_MFDM_BIT 19
+#define MSR_AMD64_SYSCFG_MFDM BIT_ULL(MSR_AMD64_SYSCFG_MFDM_BIT)
#define MSR_K8_INT_PENDING_MSG 0xc0010055
/* C1E active bits in int pending message */
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Sat, Dec 30, 2023 at 10:19:32AM -0600, Michael Roth wrote:
> + if (cpu_has(c, X86_FEATURE_SEV_SNP)) {
> + /*
> + * RMP table entry format is not architectural and it can vary by processor
> + * and is defined by the per-processor PPR. Restrict SNP support on the
> + * known CPU model and family for which the RMP table entry format is
> + * currently defined for.
> + */
> + if (!(c->x86 == 0x19 && c->x86_model <= 0xaf) &&
> + !(c->x86 == 0x1a && c->x86_model <= 0xf))
> + setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
> + else if (!snp_probe_rmptable_info())
> + setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
> + }
IOW, this below.
Lemme send the ZEN5 thing as a separate patch.
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 9492dcad560d..0fa702673e73 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -81,10 +81,8 @@
#define X86_FEATURE_K6_MTRR ( 3*32+ 1) /* AMD K6 nonstandard MTRRs */
#define X86_FEATURE_CYRIX_ARR ( 3*32+ 2) /* Cyrix ARRs (= MTRRs) */
#define X86_FEATURE_CENTAUR_MCR ( 3*32+ 3) /* Centaur MCRs (= MTRRs) */
-
-/* CPU types for specific tunings: */
#define X86_FEATURE_K8 ( 3*32+ 4) /* "" Opteron, Athlon64 */
-/* FREE, was #define X86_FEATURE_K7 ( 3*32+ 5) "" Athlon */
+#define X86_FEATURE_ZEN5 ( 3*32+ 5) /* "" CPU based on Zen5 microarchitecture */
#define X86_FEATURE_P3 ( 3*32+ 6) /* "" P3 */
#define X86_FEATURE_P4 ( 3*32+ 7) /* "" P4 */
#define X86_FEATURE_CONSTANT_TSC ( 3*32+ 8) /* TSC ticks at a constant rate */
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 0f0d425f0440..46335c2df083 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -539,7 +539,7 @@ static void bsp_init_amd(struct cpuinfo_x86 *c)
/* Figure out Zen generations: */
switch (c->x86) {
- case 0x17: {
+ case 0x17:
switch (c->x86_model) {
case 0x00 ... 0x2f:
case 0x50 ... 0x5f:
@@ -555,8 +555,8 @@ static void bsp_init_amd(struct cpuinfo_x86 *c)
goto warn;
}
break;
- }
- case 0x19: {
+
+ case 0x19:
switch (c->x86_model) {
case 0x00 ... 0x0f:
case 0x20 ... 0x5f:
@@ -570,20 +570,31 @@ static void bsp_init_amd(struct cpuinfo_x86 *c)
goto warn;
}
break;
- }
+
+ case 0x1a:
+ switch (c->x86_model) {
+ case 0x00 ... 0x0f:
+ setup_force_cpu_cap(X86_FEATURE_ZEN5);
+ break;
+ default:
+ goto warn;
+ }
+ break;
+
default:
break;
}
if (cpu_has(c, X86_FEATURE_SEV_SNP)) {
/*
- * RMP table entry format is not architectural and it can vary by processor
+ * RMP table entry format is not architectural, can vary by processor
* and is defined by the per-processor PPR. Restrict SNP support on the
* known CPU model and family for which the RMP table entry format is
* currently defined for.
*/
- if (!(c->x86 == 0x19 && c->x86_model <= 0xaf) &&
- !(c->x86 == 0x1a && c->x86_model <= 0xf))
+ if (!boot_cpu_has(X86_FEATURE_ZEN3) &&
+ !boot_cpu_has(X86_FEATURE_ZEN4) &&
+ !boot_cpu_has(X86_FEATURE_ZEN5))
setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
else if (!snp_probe_rmptable_info())
setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
@@ -1055,6 +1066,11 @@ static void init_amd_zen4(struct cpuinfo_x86 *c)
msr_set_bit(MSR_ZEN4_BP_CFG, MSR_ZEN4_BP_CFG_SHARED_BTB_FIX_BIT);
}
+static void init_amd_zen5(struct cpuinfo_x86 *c)
+{
+ init_amd_zen_common();
+}
+
static void init_amd(struct cpuinfo_x86 *c)
{
u64 vm_cr;
@@ -1100,6 +1116,8 @@ static void init_amd(struct cpuinfo_x86 *c)
init_amd_zen3(c);
else if (boot_cpu_has(X86_FEATURE_ZEN4))
init_amd_zen4(c);
+ else if (boot_cpu_has(X86_FEATURE_ZEN5))
+ init_amd_zen5(c);
/*
* Enable workaround for FXSAVE leak on CPUs
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Thu, Jan 04, 2024 at 12:05:27PM +0100, Jeremi Piotrowski wrote:
> Is there a really good reason to perform the snp_probe_smptable_info() check at this
> point (instead of in snp_rmptable_init). snp_rmptable_init will also clear the cap
> on failure, and bsp_init_amd() runs too early to allow for the kernel to allocate the
> rmptable itself. I pointed out in the previous review that kernel allocation of rmptable
> is necessary in SNP-host capable VMs in Azure.
What does that even mean?
That function is doing some calculations after reading two MSRs. What
can possibly go wrong?!
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Fri, Jan 05, 2024 at 05:09:16PM +0100, Borislav Petkov wrote:
> On Thu, Jan 04, 2024 at 12:05:27PM +0100, Jeremi Piotrowski wrote:
> > Is there a really good reason to perform the snp_probe_smptable_info() check at this
> > point (instead of in snp_rmptable_init). snp_rmptable_init will also clear the cap
> > on failure, and bsp_init_amd() runs too early to allow for the kernel to allocate the
> > rmptable itself. I pointed out in the previous review that kernel allocation of rmptable
> > is necessary in SNP-host capable VMs in Azure.
>
> What does that even mean?
>
> That function is doing some calculations after reading two MSRs. What
> can possibly go wrong?!
That could be one reason perhaps:
"It needs to be called early enough to allow for AutoIBRS to not be disabled
just because SNP is supported. By calling it where it is currently called, the
SNP feature can be cleared if, even though supported, SNP can't be used,
allowing AutoIBRS to be used as a more performant Spectre mitigation."
https://lore.kernel.org/r/[email protected]
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Sat, Dec 30, 2023 at 10:19:32AM -0600, Michael Roth wrote:
> +static int __init __snp_rmptable_init(void)
> +{
> + u64 rmptable_size;
> + void *rmptable_start;
> + u64 val;
...
Ontop:
diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
index ce7ede9065ed..566bb6f39665 100644
--- a/arch/x86/virt/svm/sev.c
+++ b/arch/x86/virt/svm/sev.c
@@ -150,6 +150,11 @@ bool snp_probe_rmptable_info(void)
return true;
}
+/*
+ * Do the necessary preparations which are verified by the firmware as
+ * described in the SNP_INIT_EX firmware command description in the SNP
+ * firmware ABI spec.
+ */
static int __init __snp_rmptable_init(void)
{
u64 rmptable_size;
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Sat, Dec 30, 2023 at 10:19:32AM -0600, Michael Roth wrote:
> +static int __init __snp_rmptable_init(void)
I already asked a year ago:
https://lore.kernel.org/all/[email protected]/
why is the __ version - __snp_rmptable_init - carved out but crickets.
It simply gets ignored. :-\
So let me do it myself, diff below.
Please add to the next version:
Co-developed-by: Borislav Petkov (AMD) <[email protected]>
Signed-off-by: Borislav Petkov (AMD) <[email protected]>
after incorporating all the changes.
Thx.
---
diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
index 566bb6f39665..feed65f80776 100644
--- a/arch/x86/virt/svm/sev.c
+++ b/arch/x86/virt/svm/sev.c
@@ -155,19 +155,25 @@ bool snp_probe_rmptable_info(void)
* described in the SNP_INIT_EX firmware command description in the SNP
* firmware ABI spec.
*/
-static int __init __snp_rmptable_init(void)
+static int __init snp_rmptable_init(void)
{
- u64 rmptable_size;
void *rmptable_start;
+ u64 rmptable_size;
u64 val;
+ if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
+ return 0;
+
+ if (!amd_iommu_snp_en)
+ return 0;
+
if (!probed_rmp_size)
- return 1;
+ goto nosnp;
rmptable_start = memremap(probed_rmp_base, probed_rmp_size, MEMREMAP_WB);
if (!rmptable_start) {
pr_err("Failed to map RMP table\n");
- return 1;
+ goto nosnp;
}
/*
@@ -195,20 +201,6 @@ static int __init __snp_rmptable_init(void)
rmptable = (struct rmpentry *)rmptable_start;
rmptable_max_pfn = rmptable_size / sizeof(struct rmpentry) - 1;
- return 0;
-}
-
-static int __init snp_rmptable_init(void)
-{
- if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
- return 0;
-
- if (!amd_iommu_snp_en)
- return 0;
-
- if (__snp_rmptable_init())
- goto nosnp;
-
cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "x86/rmptable_init:online", __snp_enable, NULL);
return 0;
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On 05/01/2024 17:21, Borislav Petkov wrote:
> On Fri, Jan 05, 2024 at 05:09:16PM +0100, Borislav Petkov wrote:
>> On Thu, Jan 04, 2024 at 12:05:27PM +0100, Jeremi Piotrowski wrote:
>>> Is there a really good reason to perform the snp_probe_smptable_info() check at this
>>> point (instead of in snp_rmptable_init). snp_rmptable_init will also clear the cap
>>> on failure, and bsp_init_amd() runs too early to allow for the kernel to allocate the
>>> rmptable itself. I pointed out in the previous review that kernel allocation of rmptable
>>> is necessary in SNP-host capable VMs in Azure.
>>
>> What does that even mean?>>
>> That function is doing some calculations after reading two MSRs. What
>> can possibly go wrong?!
>
What I wrote: "allow for the kernel to allocate the rmptable". Until the kernel allocates a
rmptable the two MSRs are not initialized in a VM. This is specific to SNP-host VMs because
they don't have access to the system-wide rmptable (or a virtualized version of it), and the
rmptable is only useful for kernel internal tracking in this case. So we don't strictly need
one and could save the overhead but not having one would complicate the KVM SNP code so I'd
rather allocate one for now.
It makes most sense to perform the rmptable allocation later in kernel init, after platform
detection and e820 setup. It isn't really used until device_initcall.
https://lore.kernel.org/lkml/[email protected]/
(I'll be posting updated patches soon).
> That could be one reason perhaps:
>
> "It needs to be called early enough to allow for AutoIBRS to not be disabled
> just because SNP is supported. By calling it where it is currently called, the
> SNP feature can be cleared if, even though supported, SNP can't be used,
> allowing AutoIBRS to be used as a more performant Spectre mitigation."
>
> https://lore.kernel.org/r/[email protected]
>
This logic seems twisted. Why use firmware rmptable allocation as a proxy for SEV-SNP
enablement if BIOS provides an explicit flag to enable/disable SEV-SNP support. That
would be a better signal to use to control AutoIBRS enablement.
On 08/01/2024 18:04, Borislav Petkov wrote:
> On Mon, Jan 08, 2024 at 05:49:01PM +0100, Jeremi Piotrowski wrote:
>> What I wrote: "allow for the kernel to allocate the rmptable".
>
> What?!
>
> "15.36.5 Hypervisor RMP Management
>
> ...
>
> Because the RMP is initialized by the AMD-SP to prevent direct access to
> the RMP, the hypervisor must use the RMPUPDATE instruction to alter the
> entries of the RMP. RMPUPDATE allows the hypervisor to alter the
> Guest_Physical_Address, Assigned, Page_Size, Immutable, and ASID fields
> of an RMP entry."
>> What you want is something that you should keep far and away from the
> upstream kernel.
>
Can we please not assume I am acting in bad faith. I am explicitly trying to
integrate nicely with AMD's KVM SNP host patches to cover an additional usecase
and get something upstreamable.
Let's separate RMP allocation from who (and how) maintains the entries.
"""
15.36.4 Initializing the RMP
...
Software must program RMP_BASE and RMP_END identically for each core in the
system and before enabling SEV-SNP globally.
"""
KVM expects UEFI to do this, Hyper-V does the allocation itself (on bare-metal).
Both are valid. Afaik it is the SNP_INIT command that hands over control of the
RMP from software to AMD-SP.
When it comes to "who and how maintains the rmp" - that is of course the AMD-SP
and hypervisor issues RMPUPDATE instructions. The paragraph you cite talks about
the physical RMP and AMD-SP - not virtualized SNP (aka "SNP-host VM"/nested SNP).
AMD specified an MSR-based RMPUPDATE for us for that usecase (15.36.19 SEV-SNP
Instruction Virtualization). The RMP inside the SNP-host VM is not related to
the physical RMP and is an entirely software based construct.
The RMP in nested SNP is only used for kernel bookkeeping and so its allocation
is optional. KVM could do without reading the RMP directly altogether (by tracking
the assigned bit somewhere) but that would be a design change and I'd rather see
the KVM SNP host patches merged in their current shape. Which is why the patch
I linked allocates a (shadow) RMP from the kernel.
I would very much appreciate if we would not prevent that usecase from working -
that's why I've been reviewing and testing multiple revisions of these patches
and providing feedback all along.
On Tue, Jan 09, 2024 at 12:56:17PM +0100, Jeremi Piotrowski wrote:
> Can we please not assume I am acting in bad faith.
No you're not acting with bad faith.
What you're doing, in my experience so far is, you come with some weird
HV + guest models which has been invented somewhere, behind some closed
doors, then you come with some desire that the upstream kernel should
support it and you're not even documenting it properly and I'm left with
asking questions all the time, what is this, what's the use case,
blabla.
Don't take this personally - I guess this is all due to NDAs,
development schedules, and whatever else and yes, I've heard it all.
But just because you want this, we're not going to jump on it and
support it unconditionally. It needs to integrate properly with the rest
of the kernel and if it doesn't, it is not going upstream. That simple.
> I am explicitly trying to integrate nicely with AMD's KVM SNP host
> patches to cover an additional usecase and get something upstreamable.
And yet I still have no clue what your use case is. I always have to go
ask behind the scenes and get some half-answers about *maybe* this is
what they support.
Looking at the patch you pointed at I see there a proper explanation of
your nested SNP stuff. Finally!
From now on, please make sure your use case is properly explained
before you come with patches.
> The RMP in nested SNP is only used for kernel bookkeeping and so its
> allocation is optional. KVM could do without reading the RMP directly
> altogether (by tracking the assigned bit somewhere) but that would be
> a design change and I'd rather see the KVM SNP host patches merged in
> their current shape. Which is why the patch I linked allocates
> a (shadow) RMP from the kernel.
At least three issues I see with that:
- the allocation can fail so it is a lot more convenient when the
firmware prepares it
- the RMP_BASE and RMP_END writes need to be verified they actially did
set up the RMP range because if they haven't, you might as well
throw SNP security out of the window. In general, letting the kernel
do the RMP allocation needs to be verified very very thoroughly.
- a future feature might make this more complicated
> I would very much appreciate if we would not prevent that usecase from
> working - that's why I've been reviewing and testing multiple
> revisions of these patches and providing feedback all along.
I very much appreciate the help but we need to get the main SNP host
stuff in first and then we can talk about modifications.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Tue, Jan 09, 2024 at 01:29:06PM +0100, Borislav Petkov wrote:
> At least three issues I see with that:
>
> - the allocation can fail so it is a lot more convenient when the
> firmware prepares it
>
> - the RMP_BASE and RMP_END writes need to be verified they actially did
> set up the RMP range because if they haven't, you might as well
> throw SNP security out of the window. In general, letting the kernel
> do the RMP allocation needs to be verified very very thoroughly.
>
> - a future feature might make this more complicated
- What do you do if you boot on a system which has the RMP already
allocated in the BIOS?
- How do you detect that it is the L1 kernel that must allocate the RMP?
- Why can't you use the BIOS allocated RMP in your scenario too instead
of the L1 kernel allocating it?
- ...
I might think of more.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On 09/01/2024 13:44, Borislav Petkov wrote:
> On Tue, Jan 09, 2024 at 01:29:06PM +0100, Borislav Petkov wrote:
>> At least three issues I see with that:
>>
>> - the allocation can fail so it is a lot more convenient when the
>> firmware prepares it
>>
>> - the RMP_BASE and RMP_END writes need to be verified they actially did
>> set up the RMP range because if they haven't, you might as well
>> throw SNP security out of the window. In general, letting the kernel
>> do the RMP allocation needs to be verified very very thoroughly.
>>
>> - a future feature might make this more complicated
>
> - What do you do if you boot on a system which has the RMP already
> allocated in the BIOS?
>
> - How do you detect that it is the L1 kernel that must allocate the RMP?
>
> - Why can't you use the BIOS allocated RMP in your scenario too instead
> of the L1 kernel allocating it?
>
> - ...
>
> I might think of more.
>
Sorry for not replying back sooner.
I agree, lets get the base SNP stuff in and then talk about extensions.
I want to sync up with Michael to make sure he's onboard with what I'm
proposing. I'll add more design/documentation/usecase descriptions with the
next submission and will make sure to address all the issues you brought up.
Jeremi