Add AMD Secure Nested Paging (SEV-SNP) Initialization Support
Subsystems (11)
v2 cover Add AMD Secure Nested Paging (SEV-SNP) Initialization Support
This patchset is also available at:
https://github.com/amdese/linux/commits/snp-host-init-v2
and is based on top of linux-next tag next-20240125
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].
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 v1:
* rebased on linux-next tag next-202401125
* patch 26: crypto: ccp: Add the SNP_SET_CONFIG command
- documentation updates (Boris)
* patch 25: crypto: ccp: Add the SNP_COMMIT command
- s/len/length/ in SNP_COMMIT kernel-doc (Boris)
- s/length/len/ in struct for consistency with other cmds/structs
* patch 24: crypto: ccp: Add the SNP_PLATFORM_STATUS command
- add comments regarding the need for page reclaim of status page (Boris)
* patch 21: crypto: ccp: Add panic notifier for SEV/SNP firmware shutdown on kdump
- enable crash_kexec_post_notifiers by default for SNP (Ashish)
- squash in refactorings from Boris, but keep wbinvd_on_all_cpus() so that non-SNP/non-panic case is still handled as it was prior to SNP.
* patch 20: crypto: ccp: Add debug support for decrypting pages
- not utilized in current code, dropped it for now (Sean)
* patch 18: crypto: ccp: Handle legacy SEV commands when SNP is enabled
- drop uneeded index variables used when rolling back descriptor mappings when failure occurs (Boris)
- error message fixups (Boris)
- fix indentation to align each argument description line to same column as recommended by kernel-doc documentation
- attempt to unmap/reclaim all descriptors even if a previous unmap/reclaim failed
- s/restored to/restored to hypervisor-owned/ in commit message
- fix reclaim clobbering return value when a command failure occurs
- reclaim cmd buffers and descriptors separately so that active cmd buffers can be marked !inuse even when reclaim for a descriptor fails
* patch 17: crypto: ccp: Handle non-volatile INIT_EX data when SNP is enabled
- drop usage of vmap() to access INIT_EX non-volatile data buffer, no longer needed now that rmpupdate() splits directmap rather than removes mappings
* patch 15: x86/sev: Introduce snp leaked pages list
- add Suggested-by: Vlastimil Babka <[email protected]>
- avoid the need to allocate memory when leaking unreclaimable pages (Vlastimil, Ashish)
- use pr_warn() instead of pr_debug() when leaking (Sean)
* patch 14: crypto: ccp: Provide API to issue SEV and SNP commands
- s/SEV/SEV device/ in kernel-doc for sev_do_cmd() (Boris)
* patch 13: crypto: ccp: Add support to initialize the AMD-SP for SEV-SNP
- account for non-page-aligned HV-Fixed ranges when calling SNP_INIT (Ashish)
- various comment/commit fixups (Boris)
* patch 12: crypto: ccp: Define the SEV-SNP commands
- fix alignment of SEV_CMD_SNP_DOWNLOAD_FIRMWARE_EX enum
- add "Incoming Migration Image" in all places that mention of IMI for purposes of documentation (Boris)
- fix indentation to align each argument description line to same column in accordance with Documentation/doc-guide/kernel-doc.rst
- rename sev_data_snp_addr.paddr_gctx to more appropriate sev_data_snp_addr.address, fix up affected patches
- len/length in kernel-doc descriptions
* patch 11: x86/sev: Invalidate pages from the direct map when adding them to the RMP table
- replace old patch/handling with an implementation based on set_memory_4k() that splits directmap as-needed rather than adding/removing mappings in response to shared/private conversions.
- lookup/confirm the mappings in advance of splitting, and avoid unecessary splits for 2M private ranges to reduce unecessary contention / TLB flushing (Dave, Boris, Tom, Mike, Vlastimil)
* patch 10: x86/sev: Add helper functions for RMPUPDATE and PSMASH instruction
- add additional comments clarifying RMPUPDATE retry logic (Boris)
* patch 07: x86/fault: Add helper for dumping RMP entries
- fix dump logic for 2MB+ pages (Tom)
- dump full 2MB range if particular 4K RMP entry is not populated (Boris)
- drop EXPORT_SYMBOL_GPL() until actually needed by a module (Boris)
* patch 04: x86/sev: Add the host SEV-SNP initialization support
- macro alignment fixups (Boris)
- commit message cleanups (Boris)
- work in Zen5 patch (Boris)
- squash __snp_init_rmptable in helper back in (Boris)
* patch 03: iommu/amd: Don't rely on external callers to enable IOMMU SNP support
- add Joerg's acked-by
- error message cleanups (Boris)
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 (14):
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
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
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 (3):
x86/fault: Dump RMP table information when RMP page faults occur x86/sev: Adjust directmap to avoid inadvertant RMP faults 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 | 38 + arch/x86/include/asm/trap_pf.h | 20 +- arch/x86/kernel/cpu/amd.c | 21 +- arch/x86/kernel/cpu/common.c | 7 +- arch/x86/kernel/cpu/mtrr/generic.c | 3 + arch/x86/kernel/crash.c | 3 + arch/x86/kernel/sev.c | 10 + 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 | 547 +++++++++++++++ drivers/crypto/ccp/sev-dev.c | 1122 ++++++++++++++++++++++++++++-- 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 | 319 ++++++++- include/uapi/linux/psp-sev.h | 59 ++ tools/arch/x86/include/asm/cpufeatures.h | 1 + 31 files changed, 2303 insertions(+), 125 deletions(-) create mode 100644 arch/x86/virt/svm/Makefile create mode 100644 arch/x86/virt/svm/sev.c
PATCH v2 1/25 x86/cpufeatures: Add SEV-SNP CPU feature
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.
--- 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(-)
arch/x86/include/asm/cpufeatures.h +1 -0
@@ -440,6 +440,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 */
arch/x86/include/asm/disabled-features.h +3 -1
@@ -117,6 +117,8 @@#define DISABLE_IBT (1 << (X86_FEATURE_IBT & 31))#endif +#define DISABLE_SEV_SNP (1 << (X86_FEATURE_SEV_SNP & 31)) +/* * 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)
arch/x86/kernel/cpu/amd.c +3 -2
@@ -602,8 +602,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.
@@ -638,6 +638,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 ); } }
tools/arch/x86/include/asm/cpufeatures.h +1 -0
@@ -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 */
Signature · 1 line
2.25.1
[tip: x86/sev] x86/cpufeatures: Add SEV-SNP CPU feature
The following commit has been merged into the x86/sev branch of tip:
The following commit has been merged into the x86/sev branch of tip:
Commit-ID: b6e0f6666f74f0794530e3557f5b0a4ce37bd556 Gitweb: https://git.kernel.org/tip/b6e0f6666f74f0794530e3557f5b0a4ce37bd556 Author: Brijesh Singh <[email protected]> AuthorDate: Thu, 25 Jan 2024 22:11:01 -06:00 Committer: Borislav Petkov (AMD) <[email protected]> CommitterDate: Mon, 29 Jan 2024 17:13:16 +01:00
x86/cpufeatures: Add SEV-SNP CPU feature
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.
--- 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(-)
arch/x86/include/asm/cpufeatures.h +1 -0
@@ -440,6 +440,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 */
arch/x86/include/asm/disabled-features.h +3 -1
@@ -117,6 +117,8 @@#define DISABLE_IBT (1 << (X86_FEATURE_IBT & 31))#endif +#define DISABLE_SEV_SNP (1 << (X86_FEATURE_SEV_SNP & 31)) +/* * 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)
arch/x86/kernel/cpu/amd.c +3 -2
@@ -605,8 +605,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.
@@ -641,6 +641,7 @@ clear_all: 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 ); } }
tools/arch/x86/include/asm/cpufeatures.h +1 -0
@@ -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 */
PATCH v2 2/25 x86/speculation: Do not enable Automatic IBRS if SEV SNP is enabled
In reply to Michael RothFrom: 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.
--- arch/x86/kernel/cpu/common.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
arch/x86/kernel/cpu/common.c +6 -1
@@ -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 ))
Signature · 1 line
2.25.1
[tip: x86/sev] x86/speculation: Do not enable Automatic IBRS if SEV-SNP is enabled
The following commit has been merged into the x86/sev branch of tip:
The following commit has been merged into the x86/sev branch of tip:
Commit-ID: acaa4b5c4c854b5009f4d4a5395b2609ad0f4937 Gitweb: https://git.kernel.org/tip/acaa4b5c4c854b5009f4d4a5395b2609ad0f4937 Author: Kim Phillips <[email protected]> AuthorDate: Thu, 25 Jan 2024 22:11:02 -06:00 Committer: Borislav Petkov (AMD) <[email protected]> CommitterDate: Mon, 29 Jan 2024 17:19:01 +01:00
x86/speculation: Do not enable Automatic IBRS if SEV-SNP is enabled
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 and all back to retpolines instead.
[ mdr: squash in changes from review discussion. ]
--- arch/x86/kernel/cpu/common.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
arch/x86/kernel/cpu/common.c +6 -1
@@ -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 ))
PATCH v2 3/25 iommu/amd: Don't rely on external callers to enable IOMMU SNP support
In reply to Michael RothFrom: 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.
--- 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(-)
arch/x86/include/asm/iommu.h +1 -0
@@ -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;
drivers/iommu/amd/amd_iommu.h +0 -1
@@ -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
drivers/iommu/amd/init.c +31 -38
84 lines not on this page · raw message
include/linux/amd-iommu.h +0 -4
@@ -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 */
Signature · 1 line
2.25.1
[tip: x86/sev] iommu/amd: Don't rely on external callers to enable IOMMU SNP support
The following commit has been merged into the x86/sev branch of tip:
The following commit has been merged into the x86/sev branch of tip:
Commit-ID: 04d65a9dbb33e20500005e151d720acead78c539 Gitweb: https://git.kernel.org/tip/04d65a9dbb33e20500005e151d720acead78c539 Author: Ashish Kalra <[email protected]> AuthorDate: Thu, 25 Jan 2024 22:11:03 -06:00 Committer: Borislav Petkov (AMD) <[email protected]> CommitterDate: Mon, 29 Jan 2024 17:19:28 +01:00
iommu/amd: Don't rely on external callers to enable IOMMU SNP support
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.
--- 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(-)
arch/x86/include/asm/iommu.h +1 -0
@@ -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;
drivers/iommu/amd/amd_iommu.h +0 -1
@@ -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
drivers/iommu/amd/init.c +31 -38
84 lines not on this page · raw message
include/linux/amd-iommu.h +0 -4
@@ -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 */
PATCH v2 4/25 x86/sev: Add the host SEV-SNP initialization support
In reply to Michael RothFrom: 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. The APM Volume 2 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
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.
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.
--- 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 | 16 +++ arch/x86/virt/svm/Makefile | 3 + arch/x86/virt/svm/sev.c | 216 +++++++++++++++++++++++++++++++ 6 files changed, 253 insertions(+), 1 deletion(-) create mode 100644 arch/x86/virt/svm/Makefile create mode 100644 arch/x86/virt/svm/sev.c
arch/x86/Kbuild +2 -0
@@ -28,5 +28,7 @@ obj-y += net/ obj-$(CONFIG_KEXEC_FILE) += purgatory/ +obj-y += virt/svm/ + # for cleaning subdir- += boot tools
arch/x86/include/asm/msr-index.h +10 -1
@@ -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)
@@ -708,8 +710,15 @@#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 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_K8_INT_PENDING_MSG 0xc0010055/* C1E active bits in int pending message */ #define K8_INTP_C1E_ACTIVE_MASK 0x18000000
arch/x86/include/asm/sev.h +6 -0
@@ -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
arch/x86/kernel/cpu/amd.c +16 -0
@@ -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>
@@ -584,6 +585,21 @@ 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 (!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 ); + } +return ; warn:
arch/x86/virt/svm/Makefile new file +3 -0
@@ -0,0 +1,3 @@ +# SPDX-License-Identifier: GPL-2.0 + +obj-$(CONFIG_KVM_AMD_SEV) += sev.o
arch/x86/virt/svm/sev.c new file +216 -0
216 lines not on this page · raw message
Signature · 1 line
2.25.1
[tip: x86/sev] x86/sev: Add SEV-SNP host initialization support
The following commit has been merged into the x86/sev branch of tip:
The following commit has been merged into the x86/sev branch of tip:
Commit-ID: 216d106c7ff7b3dcabacf2b5dc6c9c40eba7495c Gitweb: https://git.kernel.org/tip/216d106c7ff7b3dcabacf2b5dc6c9c40eba7495c Author: Brijesh Singh <[email protected]> AuthorDate: Thu, 25 Jan 2024 22:11:04 -06:00 Committer: Borislav Petkov (AMD) <[email protected]> CommitterDate: Mon, 29 Jan 2024 17:20:23 +01:00
x86/sev: Add SEV-SNP host initialization support
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 Volume 2 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
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.
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.
--- 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 | 16 ++- arch/x86/virt/svm/Makefile | 3 +- arch/x86/virt/svm/sev.c | 216 ++++++++++++++++++++++++++++++- 6 files changed, 253 insertions(+), 1 deletion(-) create mode 100644 arch/x86/virt/svm/Makefile create mode 100644 arch/x86/virt/svm/sev.c
arch/x86/Kbuild +2 -0
@@ -28,5 +28,7 @@ obj-y += net/ obj-$(CONFIG_KEXEC_FILE) += purgatory/ +obj-y += virt/svm/ + # for cleaning subdir- += boot tools
arch/x86/include/asm/msr-index.h +10 -1
@@ -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)
@@ -708,8 +710,15 @@#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 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_K8_INT_PENDING_MSG 0xc0010055/* C1E active bits in int pending message */ #define K8_INTP_C1E_ACTIVE_MASK 0x18000000
arch/x86/include/asm/sev.h +6 -0
@@ -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
arch/x86/kernel/cpu/amd.c +16 -0
@@ -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>
@@ -587,6 +588,21 @@ 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 (!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 ); + } +return ; warn:
arch/x86/virt/svm/Makefile new file +3 -0
@@ -0,0 +1,3 @@ +# SPDX-License-Identifier: GPL-2.0 + +obj-$(CONFIG_KVM_AMD_SEV) += sev.o
arch/x86/virt/svm/sev.c new file +216 -0
216 lines not on this page · raw message
PATCH v2 5/25 x86/mtrr: Don't print errors if MtrrFixDramModEn is set when SNP enabled
In reply to Michael RothFrom: 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.
--- arch/x86/kernel/cpu/mtrr/generic.c | 3 +++ 1 file changed, 3 insertions(+)
arch/x86/kernel/cpu/mtrr/generic.c +3 -0
@@ -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]"
Signature · 1 line
2.25.1
[tip: x86/sev] x86/mtrr: Don't print errors if MtrrFixDramModEn is set when SNP enabled
The following commit has been merged into the x86/sev branch of tip:
The following commit has been merged into the x86/sev branch of tip:
Commit-ID: e3fd08afb7c350d5612f113eadfb4ebb6ed08deb Gitweb: https://git.kernel.org/tip/e3fd08afb7c350d5612f113eadfb4ebb6ed08deb Author: Ashish Kalra <[email protected]> AuthorDate: Thu, 25 Jan 2024 22:11:05 -06:00 Committer: Borislav Petkov (AMD) <[email protected]> CommitterDate: Mon, 29 Jan 2024 17:24:27 +01:00
x86/mtrr: Don't print errors if MtrrFixDramModEn is set when SNP enabled
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.
--- arch/x86/kernel/cpu/mtrr/generic.c | 3 +++ 1 file changed, 3 insertions(+)
arch/x86/kernel/cpu/mtrr/generic.c +3 -0
@@ -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]"
PATCH v2 6/25 x86/sev: Add RMP entry lookup helpers
In reply to Michael RothFrom: Brijesh Singh <[email protected]>
Add a helper that can be used to access information contained in the RMP entry corresponding to a particular PFN. This will be needed to make decisions on how to handle setting up mappings in the NPT in response to guest page-faults and handling things like cleaning up pages and setting them back to the default hypervisor-owned state when they are no longer being used for private data.
[mdr: separate 'assigned' indicator from return code, and simplify
function signatures for various helpers]
--- arch/x86/include/asm/sev.h | 3 +++ arch/x86/virt/svm/sev.c | 49 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+)
arch/x86/include/asm/sev.h +3 -0
@@ -90,6 +90,7 @@ extern bool handle_vc_boot_ghcb(struct pt_regs *regs);/* RMP page size */ #define RMP_PG_SIZE_4K 0#define RMP_PG_SIZE_2M 1 +#define RMP_TO_PG_LEVEL (level) (((level) == RMP_PG_SIZE_4K) ? PG_LEVEL_4K : PG_LEVEL_2M)#define RMPADJUST_VMSA_PAGE_BIT BIT(16)
@@ -245,8 +246,10 @@ static inline u64 sev_get_status(void) { return 0; }#ifdef CONFIG_KVM_AMD_SEV bool snp_probe_rmptable_info (void ); +int snp_lookup_rmpentry (u64 pfn,bool *assigned,int *level);#else static inline bool snp_probe_rmptable_info (void ) {return false; } +static inline int snp_lookup_rmpentry (u64 pfn,bool *assigned,int *level) {return -ENODEV ; }#endif #endif
arch/x86/virt/svm/sev.c +49 -0
58 lines not on this page · raw message
Signature · 1 line
2.25.1
[tip: x86/sev] x86/sev: Add RMP entry lookup helpers
The following commit has been merged into the x86/sev branch of tip:
The following commit has been merged into the x86/sev branch of tip:
Commit-ID: 94b36bc244bb134ec616dd3f2d37343cd8c1be54 Gitweb: https://git.kernel.org/tip/94b36bc244bb134ec616dd3f2d37343cd8c1be54 Author: Brijesh Singh <[email protected]> AuthorDate: Thu, 25 Jan 2024 22:11:06 -06:00 Committer: Borislav Petkov (AMD) <[email protected]> CommitterDate: Mon, 29 Jan 2024 17:25:55 +01:00
x86/sev: Add RMP entry lookup helpers
Add a helper that can be used to access information contained in the RMP entry corresponding to a particular PFN. This will be needed to make decisions on how to handle setting up mappings in the NPT in response to guest page-faults and handling things like cleaning up pages and setting them back to the default hypervisor-owned state when they are no longer being used for private data.
[ mdr: separate 'assigned' indicator from return code, and simplify
function signatures for various helpers. ]
--- arch/x86/include/asm/sev.h | 3 ++- arch/x86/virt/svm/sev.c | 49 +++++++++++++++++++++++++++++++++++++- 2 files changed, 52 insertions(+)
arch/x86/include/asm/sev.h +3 -0
@@ -90,6 +90,7 @@ extern bool handle_vc_boot_ghcb(struct pt_regs *regs);/* RMP page size */ #define RMP_PG_SIZE_4K 0#define RMP_PG_SIZE_2M 1 +#define RMP_TO_PG_LEVEL (level) (((level) == RMP_PG_SIZE_4K) ? PG_LEVEL_4K : PG_LEVEL_2M)#define RMPADJUST_VMSA_PAGE_BIT BIT(16)
@@ -245,8 +246,10 @@ static inline u64 sev_get_status(void) { return 0; }#ifdef CONFIG_KVM_AMD_SEV bool snp_probe_rmptable_info (void ); +int snp_lookup_rmpentry (u64 pfn,bool *assigned,int *level);#else static inline bool snp_probe_rmptable_info (void ) {return false; } +static inline int snp_lookup_rmpentry (u64 pfn,bool *assigned,int *level) {return -ENODEV ; }#endif #endif
arch/x86/virt/svm/sev.c +49 -0
58 lines not on this page · raw message
PATCH v2 7/25 x86/fault: Add helper for dumping RMP entries
In reply to Michael RothFrom: Brijesh Singh <[email protected]>
This information will be useful for debugging things like page faults due to RMP access violations and RMPUPDATE failures.
[mdr: move helper to standalone patch, rework dump logic as suggested by
Boris ]
--- arch/x86/include/asm/sev.h | 2 + arch/x86/virt/svm/sev.c | 99 ++++++++++++++++++++++++++++++++++---- 2 files changed, 91 insertions(+), 10 deletions(-)
arch/x86/include/asm/sev.h +2 -0
@@ -247,9 +247,11 @@ static inline u64 sev_get_status(void) { return 0; }#ifdef CONFIG_KVM_AMD_SEV bool snp_probe_rmptable_info (void );int snp_lookup_rmpentry (u64 pfn,bool *assigned,int *level); +void snp_dump_hva_rmpentry (unsigned long address);#else static inline bool snp_probe_rmptable_info (void ) {return false; }static inline int snp_lookup_rmpentry (u64 pfn,bool *assigned,int *level) {return -ENODEV ; } +static inline void snp_dump_hva_rmpentry (unsigned long address) {}#endif #endif
arch/x86/virt/svm/sev.c +89 -10
108 lines not on this page · raw message
Signature · 1 line
2.25.1
[tip: x86/sev] x86/fault: Add helper for dumping RMP entries
The following commit has been merged into the x86/sev branch of tip:
The following commit has been merged into the x86/sev branch of tip:
Commit-ID: 1f568d36361b4891696280b719ca4b142db872ba Gitweb: https://git.kernel.org/tip/1f568d36361b4891696280b719ca4b142db872ba Author: Brijesh Singh <[email protected]> AuthorDate: Thu, 25 Jan 2024 22:11:07 -06:00 Committer: Borislav Petkov (AMD) <[email protected]> CommitterDate: Mon, 29 Jan 2024 17:26:30 +01:00
x86/fault: Add helper for dumping RMP entries
This information will be useful for debugging things like page faults due to RMP access violations and RMPUPDATE failures.
[ mdr: move helper to standalone patch, rework dump logic as suggested
by Boris. ]
--- arch/x86/include/asm/sev.h | 2 +- arch/x86/virt/svm/sev.c | 99 ++++++++++++++++++++++++++++++++---- 2 files changed, 91 insertions(+), 10 deletions(-)
arch/x86/include/asm/sev.h +2 -0
@@ -247,9 +247,11 @@ static inline u64 sev_get_status(void) { return 0; }#ifdef CONFIG_KVM_AMD_SEV bool snp_probe_rmptable_info (void );int snp_lookup_rmpentry (u64 pfn,bool *assigned,int *level); +void snp_dump_hva_rmpentry (unsigned long address);#else static inline bool snp_probe_rmptable_info (void ) {return false; }static inline int snp_lookup_rmpentry (u64 pfn,bool *assigned,int *level) {return -ENODEV ; } +static inline void snp_dump_hva_rmpentry (unsigned long address) {}#endif #endif
arch/x86/virt/svm/sev.c +89 -10
108 lines not on this page · raw message
PATCH v2 8/25 x86/traps: Define RMP violation #PF error code
In reply to Michael RothFrom: Brijesh Singh <[email protected]>
Bit 31 in the page fault-error bit will be set when processor encounters an RMP violation.
While at it, use the BIT() macro.
Signed-off by: Ashish Kalra <[email protected]>
--- arch/x86/include/asm/trap_pf.h | 20 ++++++++++++-------- arch/x86/mm/fault.c | 1 + 2 files changed, 13 insertions(+), 8 deletions(-)
arch/x86/include/asm/trap_pf.h +12 -8
@@ -2,6 +2,8 @@#ifndef _ASM_X86_TRAP_PF_H#define _ASM_X86_TRAP_PF_H +#include <linux/bits.h> /* BIT() macro */ +/* * Page fault error code bits: *
@@ -13,16 +15,18 @@* bit 5 == 1: protection keys block access * bit 6 == 1: shadow stack access fault * bit 15 == 1: SGX MMU page-fault +* bit 31 == 1: fault was due to RMP violation */ enum x86_pf_error_code { -X86_PF_PROT =1 <<0 , -X86_PF_WRITE =1 <<1 , -X86_PF_USER =1 <<2 , -X86_PF_RSVD =1 <<3 , -X86_PF_INSTR =1 <<4 , -X86_PF_PK =1 <<5 , -X86_PF_SHSTK =1 <<6 , -X86_PF_SGX =1 <<15 , +X86_PF_PROT =BIT (0 ), +X86_PF_WRITE =BIT (1 ), +X86_PF_USER =BIT (2 ), +X86_PF_RSVD =BIT (3 ), +X86_PF_INSTR =BIT (4 ), +X86_PF_PK =BIT (5 ), +X86_PF_SHSTK =BIT (6 ), +X86_PF_SGX =BIT (15 ), +X86_PF_RMP =BIT (31 ), };#endif /* _ASM_X86_TRAP_PF_H */
arch/x86/mm/fault.c +1 -0
@@ -547,6 +547,7 @@ show_fault_oops(struct pt_regs *regs, unsigned long error_code, unsigned long ad !(error_code &X86_PF_PROT ) ?"not-present page" : (error_code &X86_PF_RSVD ) ?"reserved bit violation" : (error_code &X86_PF_PK ) ?"protection keys violation" : + (error_code &X86_PF_RMP ) ?"RMP violation" : "permissions violation");if (!(error_code &X86_PF_USER ) &&user_mode (regs)) {
Signature · 1 line
2.25.1
[tip: x86/sev] x86/traps: Define RMP violation #PF error code
The following commit has been merged into the x86/sev branch of tip:
The following commit has been merged into the x86/sev branch of tip:
Commit-ID: 54055344b232c917a9e492a8bf5864fed99ad6b8 Gitweb: https://git.kernel.org/tip/54055344b232c917a9e492a8bf5864fed99ad6b8 Author: Brijesh Singh <[email protected]> AuthorDate: Thu, 25 Jan 2024 22:11:08 -06:00 Committer: Borislav Petkov (AMD) <[email protected]> CommitterDate: Mon, 29 Jan 2024 17:26:56 +01:00
x86/traps: Define RMP violation #PF error code
Bit 31 in the page fault-error bit will be set when processor encounters an RMP violation.
While at it, use the BIT() macro.
--- arch/x86/include/asm/trap_pf.h | 20 ++++++++++++-------- arch/x86/mm/fault.c | 1 + 2 files changed, 13 insertions(+), 8 deletions(-)
arch/x86/include/asm/trap_pf.h +12 -8
@@ -2,6 +2,8 @@#ifndef _ASM_X86_TRAP_PF_H#define _ASM_X86_TRAP_PF_H +#include <linux/bits.h> +/* * Page fault error code bits: *
@@ -13,16 +15,18 @@* bit 5 == 1: protection keys block access * bit 6 == 1: shadow stack access fault * bit 15 == 1: SGX MMU page-fault +* bit 31 == 1: fault was due to RMP violation */ enum x86_pf_error_code { -X86_PF_PROT =1 <<0 , -X86_PF_WRITE =1 <<1 , -X86_PF_USER =1 <<2 , -X86_PF_RSVD =1 <<3 , -X86_PF_INSTR =1 <<4 , -X86_PF_PK =1 <<5 , -X86_PF_SHSTK =1 <<6 , -X86_PF_SGX =1 <<15 , +X86_PF_PROT =BIT (0 ), +X86_PF_WRITE =BIT (1 ), +X86_PF_USER =BIT (2 ), +X86_PF_RSVD =BIT (3 ), +X86_PF_INSTR =BIT (4 ), +X86_PF_PK =BIT (5 ), +X86_PF_SHSTK =BIT (6 ), +X86_PF_SGX =BIT (15 ), +X86_PF_RMP =BIT (31 ), };#endif /* _ASM_X86_TRAP_PF_H */
arch/x86/mm/fault.c +1 -0
@@ -547,6 +547,7 @@ show_fault_oops(struct pt_regs *regs, unsigned long error_code, unsigned long ad !(error_code &X86_PF_PROT ) ?"not-present page" : (error_code &X86_PF_RSVD ) ?"reserved bit violation" : (error_code &X86_PF_PK ) ?"protection keys violation" : + (error_code &X86_PF_RMP ) ?"RMP violation" : "permissions violation");if (!(error_code &X86_PF_USER ) &&user_mode (regs)) {
PATCH v2 9/25 x86/fault: Dump RMP table information when RMP page faults occur
In reply to Michael RothRMP faults on kernel addresses are fatal and should never happen in practice. They indicate a bug in the host kernel somewhere. Userspace RMP faults shouldn't occur either, since even for VMs the memory used for private pages is handled by guest_memfd and by design is not mappable by userspace.
Dump RMP table information about the PFN corresponding to the faulting HVA to help diagnose any issues of this sort when show_fault_oops() is triggered by an RMP fault.
--- arch/x86/mm/fault.c | 4 ++++ 1 file changed, 4 insertions(+)
arch/x86/mm/fault.c +4 -0
@@ -34,6 +34,7 @@#include <asm/kvm_para.h> /* kvm_handle_async_pf */ #include <asm/vdso.h> /* fixup_vdso_exception() */ #include <asm/irq_stack.h> +#include <asm/sev.h> /* snp_dump_hva_rmpentry() */ #define CREATE_TRACE_POINTS #include <asm/trace/exceptions.h>
@@ -580,6 +581,9 @@ show_fault_oops(struct pt_regs *regs, unsigned long error_code, unsigned long ad }dump_pagetable (address); + +if (error_code &X86_PF_RMP ) +snp_dump_hva_rmpentry (address); }static noinline void
Signature · 1 line
2.25.1
[tip: x86/sev] x86/fault: Dump RMP table information when RMP page faults occur
The following commit has been merged into the x86/sev branch of tip:
The following commit has been merged into the x86/sev branch of tip:
Commit-ID: e8bbd303d7de3fb32be1434a5d5ce3e1cb182018 Gitweb: https://git.kernel.org/tip/e8bbd303d7de3fb32be1434a5d5ce3e1cb182018 Author: Michael Roth <[email protected]> AuthorDate: Thu, 25 Jan 2024 22:11:09 -06:00 Committer: Borislav Petkov (AMD) <[email protected]> CommitterDate: Mon, 29 Jan 2024 17:27:25 +01:00
x86/fault: Dump RMP table information when RMP page faults occur
RMP faults on kernel addresses are fatal and should never happen in practice. They indicate a bug in the host kernel somewhere. Userspace RMP faults shouldn't occur either, since even for VMs the memory used for private pages is handled by guest_memfd and by design is not mappable by userspace.
Dump RMP table information about the PFN corresponding to the faulting HVA to help diagnose any issues of this sort when show_fault_oops() is triggered by an RMP fault.
--- arch/x86/mm/fault.c | 4 ++++ 1 file changed, 4 insertions(+)
arch/x86/mm/fault.c +4 -0
@@ -34,6 +34,7 @@#include <asm/kvm_para.h> /* kvm_handle_async_pf */ #include <asm/vdso.h> /* fixup_vdso_exception() */ #include <asm/irq_stack.h> +#include <asm/sev.h> /* snp_dump_hva_rmpentry() */ #define CREATE_TRACE_POINTS #include <asm/trace/exceptions.h>
@@ -580,6 +581,9 @@ show_fault_oops(struct pt_regs *regs, unsigned long error_code, unsigned long ad }dump_pagetable (address); + +if (error_code &X86_PF_RMP ) +snp_dump_hva_rmpentry (address); }static noinline void
PATCH v2 10/25 x86/sev: Add helper functions for RMPUPDATE and PSMASH instruction
In reply to Michael RothFrom: Brijesh Singh <[email protected]>
The RMPUPDATE instruction updates the access restrictions for a page via its corresponding entry in the RMP Table. The hypervisor will use the instruction to enforce various access restrictions on pages used for confidential guests and other specialized functionality. See APM3 for details on the instruction operations.
The PSMASH instruction expands a 2MB RMP entry in the RMP table into a corresponding set of contiguous 4KB RMP entries while retaining the state of the validated bit from the original 2MB RMP entry. The hypervisor will use this instruction in cases where it needs to re-map a page as 4K rather than 2MB in a guest's nested page table.
Add helpers to make use of these instructions.
--- arch/x86/include/asm/sev.h | 23 ++++++++++ arch/x86/virt/svm/sev.c | 92 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+)
arch/x86/include/asm/sev.h +23 -0
@@ -87,10 +87,23 @@ extern bool handle_vc_boot_ghcb(struct pt_regs *regs);/* Software defined (when rFlags.CF = 1) */ #define PVALIDATE_FAIL_NOUPDATE 255 +/* RMUPDATE detected 4K page and 2MB page overlap. */ +#define RMPUPDATE_FAIL_OVERLAP 4 +/* RMP page size */ #define RMP_PG_SIZE_4K 0#define RMP_PG_SIZE_2M 1#define RMP_TO_PG_LEVEL (level) (((level) == RMP_PG_SIZE_4K) ? PG_LEVEL_4K : PG_LEVEL_2M) +#define PG_LEVEL_TO_RMP (level) (((level) == PG_LEVEL_4K) ? RMP_PG_SIZE_4K : RMP_PG_SIZE_2M) + +struct rmp_state { +u64 gpa; +u8 assigned; +u8 pagesize; +u8 immutable; +u8 rsvd; +u32 asid; +} __packed;#define RMPADJUST_VMSA_PAGE_BIT BIT(16)
@@ -248,10 +261,20 @@ static inline u64 sev_get_status(void) { return 0; }bool snp_probe_rmptable_info (void );int snp_lookup_rmpentry (u64 pfn,bool *assigned,int *level);void snp_dump_hva_rmpentry (unsigned long address); +int psmash (u64 pfn); +int rmp_make_private (u64 pfn,u64 gpa,enum pg_level level,int asid,bool immutable); +int rmp_make_shared (u64 pfn,enum pg_level level);#else static inline bool snp_probe_rmptable_info (void ) {return false; }static inline int snp_lookup_rmpentry (u64 pfn,bool *assigned,int *level) {return -ENODEV ; }static inline void snp_dump_hva_rmpentry (unsigned long address) {} +static inline int psmash (u64 pfn) {return -ENODEV ; } +static inline int rmp_make_private (u64 pfn,u64 gpa,enum pg_level level,int asid, +bool immutable) +{ +return -ENODEV ; +} +static inline int rmp_make_shared (u64 pfn,enum pg_level level) {return -ENODEV ; }#endif #endif
arch/x86/virt/svm/sev.c +92 -0
95 lines not on this page · raw message
Signature · 1 line
2.25.1
Re: [PATCH v2 10/25] x86/sev: Add helper functions for RMPUPDATE and PSMASH instruction
Quoted 139 lines · Michael Roth
From: Brijesh Singh <[email protected]>
The RMPUPDATE instruction updates the access restrictions for a page via its corresponding entry in the RMP Table. The hypervisor will use the instruction to enforce various access restrictions on pages used for confidential guests and other specialized functionality. See APM3 for details on the instruction operations.
The PSMASH instruction expands a 2MB RMP entry in the RMP table into a corresponding set of contiguous 4KB RMP entries while retaining the state of the validated bit from the original 2MB RMP entry. The hypervisor will use this instruction in cases where it needs to re-map a page as 4K rather than 2MB in a guest's nested page table.
Add helpers to make use of these instructions.
Signed-off-by Brijesh Singh <[email protected]>Signed-off-by Ashish Kalra <[email protected]>[mdr: add RMPUPDATE retry logic for transient FAIL_OVERLAP errors]Signed-off-by Michael Roth <[email protected]>--- arch/x86/include/asm/sev.h | 23 ++++++++++ arch/x86/virt/svm/sev.c | 92 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+)arch/x86/include/asm/sev.h +23
-0@@ -87,10 +87,23 @@ extern bool handle_vc_boot_ghcb(struct pt_regs *regs);/* Software defined (when rFlags.CF = 1) */ #define PVALIDATE_FAIL_NOUPDATE 255 +/* RMUPDATE detected 4K page and 2MB page overlap. */ +#define RMPUPDATE_FAIL_OVERLAP 4 +/* RMP page size */ #define RMP_PG_SIZE_4K 0#define RMP_PG_SIZE_2M 1#define RMP_TO_PG_LEVEL (level) (((level) == RMP_PG_SIZE_4K) ? PG_LEVEL_4K : PG_LEVEL_2M) +#define PG_LEVEL_TO_RMP (level) (((level) == PG_LEVEL_4K) ? RMP_PG_SIZE_4K : RMP_PG_SIZE_2M) + +struct rmp_state { +u64 gpa; +u8 assigned; +u8 pagesize; +u8 immutable; +u8 rsvd; +u32 asid; +} __packed;#define RMPADJUST_VMSA_PAGE_BIT BIT(16)@@ -248,10 +261,20 @@ static inline u64 sev_get_status(void) { return 0; }bool snp_probe_rmptable_info (void );int snp_lookup_rmpentry (u64 pfn,bool *assigned,int *level);void snp_dump_hva_rmpentry (unsigned long address); +int psmash (u64 pfn); +int rmp_make_private (u64 pfn,u64 gpa,enum pg_level level,int asid,bool immutable); +int rmp_make_shared (u64 pfn,enum pg_level level);#else static inline bool snp_probe_rmptable_info (void ) {return false; }static inline int snp_lookup_rmpentry (u64 pfn,bool *assigned,int *level) {return -ENODEV ; }static inline void snp_dump_hva_rmpentry (unsigned long address) {} +static inline int psmash (u64 pfn) {return -ENODEV ; } +static inline int rmp_make_private (u64 pfn,u64 gpa,enum pg_level level,int asid, +bool immutable) +{ +return -ENODEV ; +} +static inline int rmp_make_shared (u64 pfn,enum pg_level level) {return -ENODEV ; }#endif #endif arch/x86/virt/svm/sev.c +57
-0@@ -342,3 +342,95 @@ void snp_dump_hva_rmpentry(unsigned long hva) paddr =PFN_PHYS (pte_pfn (*pte)) | (hva & ~page_level_mask (level));dump_rmpentry (PHYS_PFN (paddr)); } + +/* +* PSMASH a 2MB aligned page into 4K pages in the RMP table while preserving the +* Validated bit. +*/ +int psmash (u64 pfn) +{ +unsigned long paddr = pfn <<PAGE_SHIFT ; +int ret; + +if (!cpu_feature_enabled (X86_FEATURE_SEV_SNP )) +return -ENODEV ; + +if (!pfn_valid (pfn)) +return -EINVAL ; + +/* Binutils version 2.36 supports the PSMASH mnemonic. */ + asmvolatile (".byte 0xF3, 0x0F, 0x01, 0xFF" + :"=a" (ret) + :"a" (paddr) + :"memory" ,"cc" ); + +return ret; +} +EXPORT_SYMBOL_GPL (psmash); + +/* +* It is expected that those operations are seldom enough so that no mutual +* exclusion of updaters is needed and thus the overlap error condition below +* should happen very seldomly and would get resolved relatively quickly by +* the firmware. +* +* If not, one could consider introducing a mutex or so here to sync concurrent +* RMP updates and thus diminish the amount of cases where firmware needs to +* lock 2M ranges to protect against concurrent updates. +* +* The optimal solution would be range locking to avoid locking disjoint +* regions unnecessarily but there's no support for that yet. +*/ +static int rmpupdate (u64 pfn,struct rmp_state *state) +{ +unsigned long paddr = pfn <<PAGE_SHIFT ; +int ret; + +if (!cpu_feature_enabled (X86_FEATURE_SEV_SNP )) +return -ENODEV ; + +do { +/* Binutils version 2.36 supports the RMPUPDATE mnemonic. */ + asmvolatile (".byte 0xF2, 0x0F, 0x01, 0xFE" + :"=a" (ret) + :"a" (paddr),"c" ((unsigned long )state) + :"memory" ,"cc" ); + }while (ret ==RMPUPDATE_FAIL_OVERLAP ); + +if (ret) { +pr_err ("RMPUPDATE failed for PFN %llx, ret: %d\n" , pfn, ret);
arch/x86/virt/svm/sev.c void snp_dump_hva_rmpentry(unsigned long hva) · line 411
Quoted 10 lines
arch/x86/virt/svm/sev.c +10
-0+dump_rmpentry (pfn); +dump_stack (); +return -EFAULT ; + } + +return 0 ; +} + +/* Transition a page to guest-owned/private state in the RMP table. */ +int rmp_make_private (u64 pfn,u64 gpa,enum pg_level level,int asid,bool immutable)
asid is typically a u32 (or at least unsigned) - is it better to avoid potential conversion issues with an 'int'?
Otherwise
Quoted 24 lines
arch/x86/virt/svm/sev.c +24
-0+{ +struct rmp_state state; + +memset (&state,0 ,sizeof (state)); + state.assigned =1 ; + state.asid = asid > + state.immutable = immutable; + state.gpa = gpa; + state.pagesize =PG_LEVEL_TO_RMP (level); + +return rmpupdate (pfn, &state); +} +EXPORT_SYMBOL_GPL (rmp_make_private); + +/* Transition a page to hypervisor-owned/shared state in the RMP table. */ +int rmp_make_shared (u64 pfn,enum pg_level level) +{ +struct rmp_state state; + +memset (&state,0 ,sizeof (state)); + state.pagesize =PG_LEVEL_TO_RMP (level); + +return rmpupdate (pfn, &state); +} +EXPORT_SYMBOL_GPL (rmp_make_shared);
Borislav Petkov
maintainerRe: [PATCH v2 10/25] x86/sev: Add helper functions for RMPUPDATE and PSMASH instruction
Quoted 2 lines · Liam Merwick
asid is typically a u32 (or at least unsigned) - is it better to avoid potential conversion issues with an 'int'?
struct rmp_state.asid is already u32 but yeah, lemme fix that.
Quoted 2 lines
Otherwise
Reviewed-by Liam Merwick <[email protected]>
Thx.
Signature · 4 lines
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquetteBorislav Petkov
maintainerRe: [PATCH v2 10/25] x86/sev: Add helper functions for RMPUPDATE and PSMASH instruction
Quoted 5 lines · Borislav Petkov
Quoted 2 lines · Liam Merwick
asid is typically a u32 (or at least unsigned) - is it better to avoid potential conversion issues with an 'int'?
struct rmp_state.asid is already u32 but yeah, lemme fix that.
Btw,
static int sev_get_asid(struct kvm *kvm)
{
struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
return sev->asid;
}whereas
struct kvm_sev_info {
...
unsigned int asid; /* ASID used for this guest */so that function needs fixing too.
Thx.
Signature · 4 lines
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette[tip: x86/sev] x86/sev: Add helper functions for RMPUPDATE and PSMASH instruction
In reply to Michael RothThe following commit has been merged into the x86/sev branch of tip:
The following commit has been merged into the x86/sev branch of tip:
Commit-ID: 2c35819ee00b8893626914b3384cdef2afea7dbd Gitweb: https://git.kernel.org/tip/2c35819ee00b8893626914b3384cdef2afea7dbd Author: Brijesh Singh <[email protected]> AuthorDate: Thu, 25 Jan 2024 22:11:10 -06:00 Committer: Borislav Petkov (AMD) <[email protected]> CommitterDate: Mon, 29 Jan 2024 20:30:59 +01:00
x86/sev: Add helper functions for RMPUPDATE and PSMASH instruction
The RMPUPDATE instruction updates the access restrictions for a page via its corresponding entry in the RMP Table. The hypervisor will use the instruction to enforce various access restrictions on pages used for confidential guests and other specialized functionality. See APM3 for details on the instruction operations.
The PSMASH instruction expands a 2MB RMP entry in the RMP table into a corresponding set of contiguous 4KB RMP entries while retaining the state of the validated bit from the original 2MB RMP entry. The hypervisor will use this instruction in cases where it needs to re-map a page as 4K rather than 2MB in a guest's nested page table.
Add helpers to make use of these instructions.
[ mdr: add RMPUPDATE retry logic for transient FAIL_OVERLAP errors. ]
--- arch/x86/include/asm/sev.h | 23 +++++++++- arch/x86/virt/svm/sev.c | 92 +++++++++++++++++++++++++++++++++++++- 2 files changed, 115 insertions(+)
arch/x86/include/asm/sev.h +23 -0
@@ -87,10 +87,23 @@ extern bool handle_vc_boot_ghcb(struct pt_regs *regs);/* Software defined (when rFlags.CF = 1) */ #define PVALIDATE_FAIL_NOUPDATE 255 +/* RMUPDATE detected 4K page and 2MB page overlap. */ +#define RMPUPDATE_FAIL_OVERLAP 4 +/* RMP page size */ #define RMP_PG_SIZE_4K 0#define RMP_PG_SIZE_2M 1#define RMP_TO_PG_LEVEL (level) (((level) == RMP_PG_SIZE_4K) ? PG_LEVEL_4K : PG_LEVEL_2M) +#define PG_LEVEL_TO_RMP (level) (((level) == PG_LEVEL_4K) ? RMP_PG_SIZE_4K : RMP_PG_SIZE_2M) + +struct rmp_state { +u64 gpa; +u8 assigned; +u8 pagesize; +u8 immutable; +u8 rsvd; +u32 asid; +} __packed;#define RMPADJUST_VMSA_PAGE_BIT BIT(16)
@@ -248,10 +261,20 @@ static inline u64 sev_get_status(void) { return 0; }bool snp_probe_rmptable_info (void );int snp_lookup_rmpentry (u64 pfn,bool *assigned,int *level);void snp_dump_hva_rmpentry (unsigned long address); +int psmash (u64 pfn); +int rmp_make_private (u64 pfn,u64 gpa,enum pg_level level,u32 asid,bool immutable); +int rmp_make_shared (u64 pfn,enum pg_level level);#else static inline bool snp_probe_rmptable_info (void ) {return false; }static inline int snp_lookup_rmpentry (u64 pfn,bool *assigned,int *level) {return -ENODEV ; }static inline void snp_dump_hva_rmpentry (unsigned long address) {} +static inline int psmash (u64 pfn) {return -ENODEV ; } +static inline int rmp_make_private (u64 pfn,u64 gpa,enum pg_level level,u32 asid, +bool immutable) +{ +return -ENODEV ; +} +static inline int rmp_make_shared (u64 pfn,enum pg_level level) {return -ENODEV ; }#endif #endif
arch/x86/virt/svm/sev.c +92 -0
95 lines not on this page · raw message
PATCH v2 11/25 x86/sev: Adjust directmap to avoid inadvertant RMP faults
In reply to Michael RothIf the kernel uses a 2MB or larger directmap mapping to write to an address, and that mapping contains any 4KB pages that are set to private in the RMP table, an RMP #PF will trigger and cause a host crash. SNP-aware code that owns the private PFNs will never attempt such a write, but other kernel tasks writing to other PFNs in the range may trigger these checks inadvertantly due to writing to those other PFNs via a large directmap mapping that happens to also map a private PFN.
Prevent this by splitting any 2MB+ mappings that might end up containing a mix of private/shared PFNs as a result of a subsequent RMPUPDATE for the PFN/rmp_level passed in.
Another way to handle this would be to limit the directmap to 4K mappings in the case of hosts that support SNP, but there is potential risk for performance regressions of certain host workloads. Handling it as-needed results in the directmap being slowly split over time, which lessens the risk of a performance regression since the more the directmap gets split as a result of running SNP guests, the more likely the host is being used primarily to run SNP guests, where a mostly-split directmap is actually beneficial since there is less chance of TLB flushing and cpa_lock contention being needed to perform these splits.
Cases where a host knows in advance it wants to primarily run SNP guests and wishes to pre-split the directmap can be handled by adding a tuneable in the future, but preliminary testing has shown this to not provide a signficant benefit in the common case of guests that are backed primarily by 2MB THPs, so it does not seem to be warranted currently and can be added later if a need arises in the future.
--- arch/x86/virt/svm/sev.c | 75 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 73 insertions(+), 2 deletions(-)
arch/x86/virt/svm/sev.c +73 -2
@@ -368,6 +368,71 @@ int psmash(u64 pfn) }EXPORT_SYMBOL_GPL (psmash); +/* +* If the kernel uses a 2MB or larger directmap mapping to write to an address, +* and that mapping contains any 4KB pages that are set to private in the RMP +* table, an RMP #PF will trigger and cause a host crash. Hypervisor code that +* owns the PFNs being transitioned will never attempt such a write, but other +* kernel tasks writing to other PFNs in the range may trigger these checks +* inadvertantly due a large directmap mapping that happens to overlap such a +* PFN. +* +* Prevent this by splitting any 2MB+ mappings that might end up containing a +* mix of private/shared PFNs as a result of a subsequent RMPUPDATE for the +* PFN/rmp_level passed in. +* +* Note that there is no attempt here to scan all the RMP entries for the 2MB +* physical range, since it would only be worthwhile in determining if a +* subsequent RMPUPDATE for a 4KB PFN would result in all the entries being of +* the same shared/private state, thus avoiding the need to split the mapping. +* But that would mean the entries are currently in a mixed state, and so the +* mapping would have already been split as a result of prior transitions. +* And since the 4K split is only done if the mapping is 2MB+, and there isn't +* currently a mechanism in place to restore 2MB+ mappings, such a check would +* not provide any usable benefit. +* +* More specifics on how these checks are carried out can be found in APM +* Volume 2, "RMP and VMPL Access Checks". +*/ +static int adjust_direct_map (u64 pfn,int rmp_level) +{ +unsigned long vaddr = (unsigned long )pfn_to_kaddr (pfn); +unsigned int level; +int npages, ret; +pte_t *pte; + +/* Only 4KB/2MB RMP entries are supported by current hardware. */ +if (WARN_ON_ONCE (rmp_level >PG_LEVEL_2M )) +return -EINVAL ; + +if (WARN_ON_ONCE (rmp_level ==PG_LEVEL_2M && !IS_ALIGNED (pfn,PTRS_PER_PMD ))) +return -EINVAL ; + +/* +* If an entire 2MB physical range is being transitioned, then there is +* no risk of RMP #PFs due to write accesses from overlapping mappings, +* since even accesses from 1GB mappings will be treated as 2MB accesses +* as far as RMP table checks are concerned. +*/ +if (rmp_level ==PG_LEVEL_2M ) +return 0 ; + + pte =lookup_address (vaddr, &level); +if (!pte ||pte_none (*pte)) +return 0 ; + +if (level ==PG_LEVEL_4K ) +return 0 ; + + npages =page_level_size (rmp_level) /PAGE_SIZE ; + ret =set_memory_4k (vaddr, npages); +if (ret) +pr_warn ("Failed to split direct map for PFN 0x%llx, ret: %d\n" , + pfn, ret); + +return ret; +} +/* * It is expected that those operations are seldom enough so that no mutual * exclusion of updaters is needed and thus the overlap error condition below
@@ -384,11 +449,16 @@ EXPORT_SYMBOL_GPL(psmash);static int rmpupdate (u64 pfn,struct rmp_state *state) {unsigned long paddr = pfn <<PAGE_SHIFT ; -int ret; +int ret, level;if (!cpu_feature_enabled (X86_FEATURE_SEV_SNP ))return -ENODEV ; + level =RMP_TO_PG_LEVEL (state->pagesize); + +if (adjust_direct_map (pfn, level)) +return -EFAULT ; + do { /* Binutils version 2.36 supports the RMPUPDATE mnemonic. */ asm volatile(".byte 0xF2, 0x0F, 0x01, 0xFE"
@@ -398,7 +468,8 @@ static int rmpupdate(u64 pfn, struct rmp_state *state) }while (ret ==RMPUPDATE_FAIL_OVERLAP );if (ret) { -pr_err ("RMPUPDATE failed for PFN %llx, ret: %d\n" , pfn, ret); +pr_err ("RMPUPDATE failed for PFN %llx, pg_level: %d, ret: %d\n" , + pfn, level, ret);dump_rmpentry (pfn);dump_stack ();return -EFAULT ;
Signature · 1 line
2.25.1
Borislav Petkov
maintainerRe: [PATCH v2 11/25] x86/sev: Adjust directmap to avoid inadvertant RMP faults
arch/x86/virt/svm/sev.c int psmash(u64 pfn) · line 402
Quoted 6 lines · Michael Roth
arch/x86/virt/svm/sev.c +6
-0+static int adjust_direct_map (u64 pfn,int rmp_level) +{ +unsigned long vaddr = (unsigned long )pfn_to_kaddr (pfn); +unsigned int level; +int npages, ret; +pte_t *pte;
Again, something I asked the last time but no reply:
Looking at Documentation/arch/x86/x86_64/mm.rst, the direct map starts at page_offset_base so this here should at least check
if (vaddr < __PAGE_OFFSET) return 0;
I'm not sure about the upper end. Right now, the adjusting should not happen only for the direct map but also for the whole kernel address space range because we don't want to cause any mismatch between page mappings anywhere.
Which means, this function should be called adjust_kernel_map() or so...
Hmmm.
Signature · 4 lines
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquetteRe: [PATCH v2 11/25] x86/sev: Adjust directmap to avoid inadvertant RMP faults
Quoted 10 lines · Borislav Petkov
arch/x86/virt/svm/sev.c int psmash(u64 pfn) · line 402
Quoted 6 lines · Michael Roth
arch/x86/virt/svm/sev.c +6
-0+static int adjust_direct_map (u64 pfn,int rmp_level) +{ +unsigned long vaddr = (unsigned long )pfn_to_kaddr (pfn); +unsigned int level; +int npages, ret; +pte_t *pte;Again, something I asked the last time but no reply:
Quoted 5 lines
Looking at
Documentation/arch/x86/x86_64/mm.rst, the direct map starts atpage_offset_baseso this here should at least checkif (vaddr < __PAGE_OFFSET) return 0;
vaddr comes from pfn_to_kaddr(pfn), i.e. __va(paddr), so it will necessarily be a direct-mapped address above __PAGE_OFFSET.
Quoted 2 lines
I'm not sure about the upper end. Right now, the adjusting should not
For upper-end, a pfn_valid(pfn) check might suffice, since only a valid PFN would have a possibly-valid mapping wthin the directmap range.
Quoted 5 lines
happen only for the direct map but also for the whole kernel address space range because we don't want to cause any mismatch between page mappings anywhere.
Which means, this function should be called
adjust_kernel_map()or so...
These are PFNs that are owned/allocated-to the caller. Due to the nature of the directmap it's possible non-owners would write to a mapping that overlaps, but vmalloc()/etc. would not create mappings for any pages that were not specifically part of an allocation that belongs to the caller, so I don't see where there's any chance for an overlap there. And the caller of these functions would not be adjusting directmap for PFNs that might be mapped into other kernel address ranges like kernel-text/etc unless the caller was specifically making SNP-aware adjustments to those ranges, in which case it would be responsible for making those other adjustments, or implementing the necessary helpers/etc.
I'm not aware of such cases in the current code, and I don't think it makes sense to attempt to try to handle them here generically until such a case arises, since it will likely involve more specific requirements than what we can anticipate from a theoretical/generic standpoint.
-Mike
Quoted 8 lines
Hmmm.
Signature · 4 lines
Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Borislav Petkov
maintainerRe: [PATCH v2 11/25] x86/sev: Adjust directmap to avoid inadvertant RMP faults
Quoted 2 lines · Michael Roth
vaddr comes from
pfn_to_kaddr(pfn), i.e.__va(paddr), so it will necessarily be a direct-mapped address above__PAGE_OFFSET.
Ah, true.
Quoted 2 lines
For upper-end, a
pfn_valid(pfn)check might suffice, since only a valid PFN would have a possibly-valid mapping wthin the directmap range.
Looking at it, yap, that could be a sensible thing to check.
Quoted 10 lines
These are PFNs that are owned/allocated-to the caller. Due to the nature of the directmap it's possible non-owners would write to a mapping that overlaps, but vmalloc()/etc. would not create mappings for any pages that were not specifically part of an allocation that belongs to the caller, so I don't see where there's any chance for an overlap there. And the caller of these functions would not be adjusting directmap for PFNs that might be mapped into other kernel address ranges like kernel-text/etc unless the caller was specifically making SNP-aware adjustments to those ranges, in which case it would be responsible for making those other adjustments, or implementing the necessary helpers/etc.
Why does any of that matter?
If you can make this helper as generic as possible now, why don't you?
Quoted 4 lines
I'm not aware of such cases in the current code, and I don't think it makes sense to attempt to try to handle them here generically until such a case arises, since it will likely involve more specific requirements than what we can anticipate from a theoretical/generic standpoint.
Then that's a different story. If it will likely involve more specific handling, then that function should deal with pfns for which it can DTRT and for others warn loudly so that the code gets fixed in time.
IOW, then it should check for the upper pfn of the direct map here and we have two, depending on the page sizes used...
Thx.
Signature · 4 lines
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquetteRe: [PATCH v2 11/25] x86/sev: Adjust directmap to avoid inadvertant RMP faults
Quoted 22 lines · Borislav Petkov
Quoted 2 lines · Michael Roth
vaddr comes from
pfn_to_kaddr(pfn), i.e.__va(paddr), so it will necessarily be a direct-mapped address above__PAGE_OFFSET.Ah, true.
Quoted 2 lines
For upper-end, a
pfn_valid(pfn)check might suffice, since only a valid PFN would have a possibly-valid mapping wthin the directmap range.Looking at it, yap, that could be a sensible thing to check.
Quoted 10 lines
These are PFNs that are owned/allocated-to the caller. Due to the nature of the directmap it's possible non-owners would write to a mapping that overlaps, but vmalloc()/etc. would not create mappings for any pages that were not specifically part of an allocation that belongs to the caller, so I don't see where there's any chance for an overlap there. And the caller of these functions would not be adjusting directmap for PFNs that might be mapped into other kernel address ranges like kernel-text/etc unless the caller was specifically making SNP-aware adjustments to those ranges, in which case it would be responsible for making those other adjustments, or implementing the necessary helpers/etc.
Quoted 3 lines
Why does any of that matter?
If you can make this helper as generic as possible now, why don't you?
In this case, it would make it more difficult to handle things efficiently and implement proper bounds-checking/etc. For instance, if the caller *knows* they are doing something different like splitting a kernel-text mapping, then we could implement proper bounds-checking based on expected ranges, and implement any special handling associated with that use-case, and capture that in a nice/understandable adjust_kernel_text_mapping() helper. Or maybe if these are adjustments for non-static/non-linear mappings, it makes more sense to be given an HVA rather than PFN, etc., since we might not have any sort of reverse-map structure/function than can be used to do the PFN->HVA lookups efficiently.
It's just a lot to guess at. And just the directmap splitting itself has been the source of so much discussion, investigation, re-work, benchmarking, etc., that it hints that implementing similar handling for other use-cases really needs to have a clear and necessary purpose and set of requirements hat can be evaluated/tested before enabling them and reasonably expecting them to work as expected.
Quoted 6 lines
Quoted 5 lines
I'm not aware of such cases in the current code, and I don't think it makes sense to attempt to try to handle them here generically until such a case arises, since it will likely involve more specific requirements than what we can anticipate from a theoretical/generic standpoint.
Quoted 6 lines
Then that's a different story. If it will likely involve more specific handling, then that function should deal with pfns for which it can DTRT and for others warn loudly so that the code gets fixed in time.
IOW, then it should check for the upper pfn of the direct map here and we have two, depending on the page sizes used...
Is something like this close to what you're thinking? I've re-tested with SNP guests and it seems to work as expected.
arch/x86/virt/svm/sev.c +6 -1
@@ -421,7 +421,12 @@ static int adjust_direct_map(u64 pfn, int rmp_level)if (WARN_ON_ONCE (rmp_level >PG_LEVEL_2M ))return -EINVAL ; -if (WARN_ON_ONCE (rmp_level ==PG_LEVEL_2M && !IS_ALIGNED (pfn,PTRS_PER_PMD ))) +if (!pfn_valid (pfn)) +return -EINVAL ; + +if (rmp_level ==PG_LEVEL_2M && + (!IS_ALIGNED (pfn,PTRS_PER_PMD ) || + !pfn_valid (pfn +PTRS_PER_PMD -1 )))return -EINVAL ;
Note that I removed the WARN_ON_ONCE(), which I think was a bit overzealous for this check. The one above it for rmp_level > PG_LEVEL_2M I think is more warranted, since it returns error for a use-case that might in theory become valid on future hardware, but would result in unecessary 1GB->2MB directmap splitting if we were to try to implement handling for that before it's actually needed (which may well be never).
-Mike
Quoted 8 lines
Thx.
Signature · 4 lines
Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Borislav Petkov
maintainerRe: [PATCH v2 11/25] x86/sev: Adjust directmap to avoid inadvertant RMP faults
arch/x86/virt/svm/sev.c static int adjust_direct_map(u64 pfn, int rmp_level) · line 424
Quoted 13 lines · Michael Roth
Is something like this close to what you're thinking? I've re-tested with SNP guests and it seems to work as expected.
arch/x86/virt/svm/sev.c +1
-1@@ -421,7 +421,12 @@ static int adjust_direct_map(u64 pfn, int rmp_level)if (WARN_ON_ONCE (rmp_level >PG_LEVEL_2M ))return -EINVAL ; -if (WARN_ON_ONCE (rmp_level ==PG_LEVEL_2M && !IS_ALIGNED (pfn,PTRS_PER_PMD ))) +if (!pfn_valid (pfn))
_text at VA 0xffffffff81000000 is also a valid pfn so no, this is not enough.
Either this function should not have "direct map" in the name as it converts *any* valid pfn not just the direct map ones or it should check whether the pfn belongs to the direct map range.
Thx.
Signature · 4 lines
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquetteRe: [PATCH v2 11/25] x86/sev: Adjust directmap to avoid inadvertant RMP faults
Quoted 12 lines · Borislav Petkov
Quoted 11 lines · Michael Roth
Is something like this close to what you're thinking? I've re-tested with SNP guests and it seems to work as expected.
arch/x86/virt/svm/sev.c +0
-0@@ -421,7 +421,12 @@ static int adjust_direct_map(u64 pfn, int rmp_level)if (WARN_ON_ONCE (rmp_level >PG_LEVEL_2M ))return -EINVAL ;
Quoted 5 lines
arch/x86/virt/svm/sev.c static int adjust_direct_map(u64 pfn, int rmp_level) · line 424
Quoted 2 lines
arch/x86/virt/svm/sev.c +1
-1-if (WARN_ON_ONCE (rmp_level ==PG_LEVEL_2M && !IS_ALIGNED (pfn,PTRS_PER_PMD ))) +if (!pfn_valid (pfn))
_textat VA0xffffffff81000000is also a valid pfn so no, this is not enough.
directmap maps all physical memory accessible by kernel, including text pages, so those are valid PFNs as far as this function is concerned. We can't generally guard against the caller passing in any random PFN that might also be mapped into additional address ranges, similarly to how we can't guard against something doing a write to some random PFN __va(0x1234) and scribbling over memory that it doesn't own, or just unmapping the entire directmap range and blowing up the kernel.
The expectation is that the caller is aware of what PFNs it is passing in, whether those PFNs have additional mappings, and if those mappings are of concern, implement the necessary handlers if new use-cases are ever introducted, like the adjust_kernel_text_mapping() example I mentioned earlier.
Quoted 4 lines
Either this function should not have "direct map" in the name as it converts *any* valid pfn not just the direct map ones or it should check whether the pfn belongs to the direct map range.
This function only splits mappings in the 0xffff888000000000 directmap range. It would be inaccurate to name it in such a way that suggests that it does anything else. If a use-case ever arises for splitting _text mappings at 0xffffffff81000000, or any other ranges, those too would best served by dedicated helpers adjust_kernel_text_mapping() that *actually* modify mappings for those virtual ranges, and implement bounds-checking appropriate for those physical/virtual ranges. The directmap range maps all kernel-accessible physical memory, so it's appropriate that our bounds-checking for the purpose of adjust_direct_map() is all kernel-accessible physical memory. If that includes PFNs mapped to other virtual ranges as well, the caller needs to consider that and implement additional helpers as necessary, but they'd likely *still* need to call adjust_direct_map() to adjust the directmap range in addition to those other ranges, so even if were possible to do so reliably, we shouldn't try to selectively reject any PFN ranges beyond what mm.rst suggests is valid for 0xffff888000000000.
-Mike
Quoted 9 lines
Thx.
Signature · 4 lines
Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Borislav Petkov
maintainerRe: [PATCH v2 11/25] x86/sev: Adjust directmap to avoid inadvertant RMP faults
Quoted 2 lines · Michael Roth
directmap maps all physical memory accessible by kernel, including text pages, so those are valid PFNs as far as this function is concerned.
Why don't you have a look at
Documentation/arch/x86/x86_64/mm.rst
to sync up on the nomenclature first?
ffff888000000000 | -119.5 TB | ffffc87fffffffff | 64 TB | direct mapping of all physical memory (page_offset_base)
...
ffffffff80000000 | -2 GB | ffffffff9fffffff | 512 MB | kernel text mapping, mapped to physical address 0
and so on.
Quoted 1 line
The expectation is that the caller is aware of what PFNs it is passing in,
There are no expectations. Have you written them down somewhere?
Quoted 2 lines
This function only splits mappings in the
0xffff888000000000directmap range.
This function takes any PFN it gets passed in as it is. I don't care who its users are now or in the future and whether they pay attention what they pass into - it needs to be properly defined.
Mike, please get on with the program. Use the right naming for the function and basta.
IOW, this:
arch/x86/virt/svm/sev.c +8 -3
@@ -394,7 +394,7 @@ EXPORT_SYMBOL_GPL(psmash);* More specifics on how these checks are carried out can be found in APM * Volume 2, "RMP and VMPL Access Checks". */ -static int adjust_direct_map (u64 pfn,int rmp_level) +static int split_pfn (u64 pfn,int rmp_level) {unsigned long vaddr = (unsigned long )pfn_to_kaddr (pfn);unsigned int level;
@@ -405,7 +405,12 @@ static int adjust_direct_map(u64 pfn, int rmp_level)if (WARN_ON_ONCE (rmp_level >PG_LEVEL_2M ))return -EINVAL ; -if (WARN_ON_ONCE (rmp_level ==PG_LEVEL_2M && !IS_ALIGNED (pfn,PTRS_PER_PMD ))) +if (!pfn_valid (pfn)) +return -EINVAL ; + +if (rmp_level ==PG_LEVEL_2M && + (!IS_ALIGNED (pfn,PTRS_PER_PMD ) || + !pfn_valid (pfn +PTRS_PER_PMD -1 )))return -EINVAL ;/*
@@ -456,7 +461,7 @@ static int rmpupdate(u64 pfn, struct rmp_state *state) level =RMP_TO_PG_LEVEL (state->pagesize); -if (adjust_direct_map (pfn, level)) +if (split_pfn (pfn, level))return -EFAULT ; do {
Signature · 4 lines
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquetteBorislav Petkov
maintainerRe: [PATCH v2 11/25] x86/sev: Adjust directmap to avoid inadvertant RMP faults
Quoted 3 lines · Borislav Petkov
This function takes any PFN it gets passed in as it is. I don't care who its users are now or in the future and whether they pay attention what they pass into - it needs to be properly defined.
Ok, we solved it offlist, here's the final version I have. It has a comment explaining what I was asking.
---
From: Michael Roth <[email protected]>
Date: Thu, 25 Jan 2024 22:11:11 -0600
Subject: [PATCH] x86/sev: Adjust the directmap to avoid inadvertent RMP faults
If the kernel uses a 2MB or larger directmap mapping to write to an address, and that mapping contains any 4KB pages that are set to private in the RMP table, an RMP #PF will trigger and cause a host crash.
SNP-aware code that owns the private PFNs will never attempt such a write, but other kernel tasks writing to other PFNs in the range may trigger these checks inadvertently due to writing to those other PFNs via a large directmap mapping that happens to also map a private PFN.
Prevent this by splitting any 2MB+ mappings that might end up containing a mix of private/shared PFNs as a result of a subsequent RMPUPDATE for the PFN/rmp_level passed in.
Another way to handle this would be to limit the directmap to 4K mappings in the case of hosts that support SNP, but there is potential risk for performance regressions of certain host workloads.
Handling it as-needed results in the directmap being slowly split over time, which lessens the risk of a performance regression since the more the directmap gets split as a result of running SNP guests, the more likely the host is being used primarily to run SNP guests, where a mostly-split directmap is actually beneficial since there is less chance of TLB flushing and cpa_lock contention being needed to perform these splits.
Cases where a host knows in advance it wants to primarily run SNP guests and wishes to pre-split the directmap can be handled by adding a tuneable in the future, but preliminary testing has shown this to not provide a signficant benefit in the common case of guests that are backed primarily by 2MB THPs, so it does not seem to be warranted currently and can be added later if a need arises in the future.
--- arch/x86/virt/svm/sev.c | 86 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 84 insertions(+), 2 deletions(-)
arch/x86/virt/svm/sev.c +84 -2
@@ -368,6 +368,82 @@ int psmash(u64 pfn) }EXPORT_SYMBOL_GPL (psmash); +/* +* If the kernel uses a 2MB or larger directmap mapping to write to an address, +* and that mapping contains any 4KB pages that are set to private in the RMP +* table, an RMP #PF will trigger and cause a host crash. Hypervisor code that +* owns the PFNs being transitioned will never attempt such a write, but other +* kernel tasks writing to other PFNs in the range may trigger these checks +* inadvertently due a large directmap mapping that happens to overlap such a +* PFN. +* +* Prevent this by splitting any 2MB+ mappings that might end up containing a +* mix of private/shared PFNs as a result of a subsequent RMPUPDATE for the +* PFN/rmp_level passed in. +* +* Note that there is no attempt here to scan all the RMP entries for the 2MB +* physical range, since it would only be worthwhile in determining if a +* subsequent RMPUPDATE for a 4KB PFN would result in all the entries being of +* the same shared/private state, thus avoiding the need to split the mapping. +* But that would mean the entries are currently in a mixed state, and so the +* mapping would have already been split as a result of prior transitions. +* And since the 4K split is only done if the mapping is 2MB+, and there isn't +* currently a mechanism in place to restore 2MB+ mappings, such a check would +* not provide any usable benefit. +* +* More specifics on how these checks are carried out can be found in APM +* Volume 2, "RMP and VMPL Access Checks". +*/ +static int adjust_direct_map (u64 pfn,int rmp_level) +{ +unsigned long vaddr; +unsigned int level; +int npages, ret; +pte_t *pte; + +/* +* pfn_to_kaddr() will return a vaddr only within the direct +* map range. +*/ + vaddr = (unsigned long )pfn_to_kaddr (pfn); + +/* Only 4KB/2MB RMP entries are supported by current hardware. */ +if (WARN_ON_ONCE (rmp_level >PG_LEVEL_2M )) +return -EINVAL ; + +if (!pfn_valid (pfn)) +return -EINVAL ; + +if (rmp_level ==PG_LEVEL_2M && + (!IS_ALIGNED (pfn,PTRS_PER_PMD ) || + !pfn_valid (pfn +PTRS_PER_PMD -1 ))) +return -EINVAL ; + +/* +* If an entire 2MB physical range is being transitioned, then there is +* no risk of RMP #PFs due to write accesses from overlapping mappings, +* since even accesses from 1GB mappings will be treated as 2MB accesses +* as far as RMP table checks are concerned. +*/ +if (rmp_level ==PG_LEVEL_2M ) +return 0 ; + + pte =lookup_address (vaddr, &level); +if (!pte ||pte_none (*pte)) +return 0 ; + +if (level ==PG_LEVEL_4K ) +return 0 ; + + npages =page_level_size (rmp_level) /PAGE_SIZE ; + ret =set_memory_4k (vaddr, npages); +if (ret) +pr_warn ("Failed to split direct map for PFN 0x%llx, ret: %d\n" , + pfn, ret); + +return ret; +} +/* * It is expected that those operations are seldom enough so that no mutual * exclusion of updaters is needed and thus the overlap error condition below
@@ -384,11 +460,16 @@ EXPORT_SYMBOL_GPL(psmash);static int rmpupdate (u64 pfn,struct rmp_state *state) {unsigned long paddr = pfn <<PAGE_SHIFT ; -int ret; +int ret, level;if (!cpu_feature_enabled (X86_FEATURE_SEV_SNP ))return -ENODEV ; + level =RMP_TO_PG_LEVEL (state->pagesize); + +if (adjust_direct_map (pfn, level)) +return -EFAULT ; + do { /* Binutils version 2.36 supports the RMPUPDATE mnemonic. */ asm volatile(".byte 0xF2, 0x0F, 0x01, 0xFE"
@@ -398,7 +479,8 @@ static int rmpupdate(u64 pfn, struct rmp_state *state) }while (ret ==RMPUPDATE_FAIL_OVERLAP );if (ret) { -pr_err ("RMPUPDATE failed for PFN %llx, ret: %d\n" , pfn, ret); +pr_err ("RMPUPDATE failed for PFN %llx, pg_level: %d, ret: %d\n" , + pfn, level, ret);dump_rmpentry (pfn);dump_stack ();return -EFAULT ;
Signature · 7 lines
2.43.0
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquetteVlastimil Babka
Acked-byRe: [PATCH v2 11/25] x86/sev: Adjust directmap to avoid inadvertant RMP faults
Quoted 45 lines · Borislav Petkov
Quoted 3 lines · Borislav Petkov
This function takes any PFN it gets passed in as it is. I don't care who its users are now or in the future and whether they pay attention what they pass into - it needs to be properly defined.
Ok, we solved it offlist, here's the final version I have. It has a comment explaining what I was asking.
---
From: Michael Roth <[email protected]>
Date: Thu, 25 Jan 2024 22:11:11 -0600
Subject: [PATCH] x86/sev: Adjust the directmap to avoid inadvertent RMP faultsIf the kernel uses a 2MB or larger directmap mapping to write to an address, and that mapping contains any 4KB pages that are set to private in the RMP table, an RMP #PF will trigger and cause a host crash.
SNP-aware code that owns the private PFNs will never attempt such a write, but other kernel tasks writing to other PFNs in the range may trigger these checks inadvertently due to writing to those other PFNs via a large directmap mapping that happens to also map a private PFN.
Prevent this by splitting any 2MB+ mappings that might end up containing a mix of private/shared PFNs as a result of a subsequent RMPUPDATE for the PFN/rmp_level passed in.
Another way to handle this would be to limit the directmap to 4K mappings in the case of hosts that support SNP, but there is potential risk for performance regressions of certain host workloads.
Handling it as-needed results in the directmap being slowly split over time, which lessens the risk of a performance regression since the more the directmap gets split as a result of running SNP guests, the more likely the host is being used primarily to run SNP guests, where a mostly-split directmap is actually beneficial since there is less chance of TLB flushing and
cpa_lockcontention being needed to perform these splits.Cases where a host knows in advance it wants to primarily run SNP guests and wishes to pre-split the directmap can be handled by adding a tuneable in the future, but preliminary testing has shown this to not provide a signficant benefit in the common case of guests that are backed primarily by 2MB THPs, so it does not seem to be warranted currently and can be added later if a need arises in the future.
Quoted 3 lines
Signed-off-by Michael Roth <[email protected]>Signed-off-by Borislav Petkov (AMD) <[email protected]>
Thanks!
[tip: x86/sev] x86/sev: Adjust the directmap to avoid inadvertent RMP faults
In reply to Michael RothThe following commit has been merged into the x86/sev branch of tip:
The following commit has been merged into the x86/sev branch of tip:
Commit-ID: 661b1c6169e2b260753b850823d583d9f2c01be4 Gitweb: https://git.kernel.org/tip/661b1c6169e2b260753b850823d583d9f2c01be4 Author: Michael Roth <[email protected]> AuthorDate: Thu, 25 Jan 2024 22:11:11 -06:00 Committer: Borislav Petkov (AMD) <[email protected]> CommitterDate: Mon, 29 Jan 2024 20:34:18 +01:00
x86/sev: Adjust the directmap to avoid inadvertent RMP faults
If the kernel uses a 2MB or larger directmap mapping to write to an address, and that mapping contains any 4KB pages that are set to private in the RMP table, an RMP #PF will trigger and cause a host crash.
SNP-aware code that owns the private PFNs will never attempt such a write, but other kernel tasks writing to other PFNs in the range may trigger these checks inadvertently due to writing to those other PFNs via a large directmap mapping that happens to also map a private PFN.
Prevent this by splitting any 2MB+ mappings that might end up containing a mix of private/shared PFNs as a result of a subsequent RMPUPDATE for the PFN/rmp_level passed in.
Another way to handle this would be to limit the directmap to 4K mappings in the case of hosts that support SNP, but there is potential risk for performance regressions of certain host workloads.
Handling it as-needed results in the directmap being slowly split over time, which lessens the risk of a performance regression since the more the directmap gets split as a result of running SNP guests, the more likely the host is being used primarily to run SNP guests, where a mostly-split directmap is actually beneficial since there is less chance of TLB flushing and cpa_lock contention being needed to perform these splits.
Cases where a host knows in advance it wants to primarily run SNP guests and wishes to pre-split the directmap can be handled by adding a tuneable in the future, but preliminary testing has shown this to not provide a signficant benefit in the common case of guests that are backed primarily by 2MB THPs, so it does not seem to be warranted currently and can be added later if a need arises in the future.
--- arch/x86/virt/svm/sev.c | 85 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 83 insertions(+), 2 deletions(-)
arch/x86/virt/svm/sev.c +83 -2
@@ -369,6 +369,81 @@ int psmash(u64 pfn)EXPORT_SYMBOL_GPL (psmash);/* +* If the kernel uses a 2MB or larger directmap mapping to write to an address, +* and that mapping contains any 4KB pages that are set to private in the RMP +* table, an RMP #PF will trigger and cause a host crash. Hypervisor code that +* owns the PFNs being transitioned will never attempt such a write, but other +* kernel tasks writing to other PFNs in the range may trigger these checks +* inadvertently due a large directmap mapping that happens to overlap such a +* PFN. +* +* Prevent this by splitting any 2MB+ mappings that might end up containing a +* mix of private/shared PFNs as a result of a subsequent RMPUPDATE for the +* PFN/rmp_level passed in. +* +* Note that there is no attempt here to scan all the RMP entries for the 2MB +* physical range, since it would only be worthwhile in determining if a +* subsequent RMPUPDATE for a 4KB PFN would result in all the entries being of +* the same shared/private state, thus avoiding the need to split the mapping. +* But that would mean the entries are currently in a mixed state, and so the +* mapping would have already been split as a result of prior transitions. +* And since the 4K split is only done if the mapping is 2MB+, and there isn't +* currently a mechanism in place to restore 2MB+ mappings, such a check would +* not provide any usable benefit. +* +* More specifics on how these checks are carried out can be found in APM +* Volume 2, "RMP and VMPL Access Checks". +*/ +static int adjust_direct_map (u64 pfn,int rmp_level) +{ +unsigned long vaddr; +unsigned int level; +int npages, ret; +pte_t *pte; + +/* +* pfn_to_kaddr() will return a vaddr only within the direct +* map range. +*/ + vaddr = (unsigned long )pfn_to_kaddr (pfn); + +/* Only 4KB/2MB RMP entries are supported by current hardware. */ +if (WARN_ON_ONCE (rmp_level >PG_LEVEL_2M )) +return -EINVAL ; + +if (!pfn_valid (pfn)) +return -EINVAL ; + +if (rmp_level ==PG_LEVEL_2M && + (!IS_ALIGNED (pfn,PTRS_PER_PMD ) || !pfn_valid (pfn +PTRS_PER_PMD -1 ))) +return -EINVAL ; + +/* +* If an entire 2MB physical range is being transitioned, then there is +* no risk of RMP #PFs due to write accesses from overlapping mappings, +* since even accesses from 1GB mappings will be treated as 2MB accesses +* as far as RMP table checks are concerned. +*/ +if (rmp_level ==PG_LEVEL_2M ) +return 0 ; + + pte =lookup_address (vaddr, &level); +if (!pte ||pte_none (*pte)) +return 0 ; + +if (level ==PG_LEVEL_4K ) +return 0 ; + + npages =page_level_size (rmp_level) /PAGE_SIZE ; + ret =set_memory_4k (vaddr, npages); +if (ret) +pr_warn ("Failed to split direct map for PFN 0x%llx, ret: %d\n" , + pfn, ret); + +return ret; +} + +/* * It is expected that those operations are seldom enough so that no mutual * exclusion of updaters is needed and thus the overlap error condition below * should happen very rarely and would get resolved relatively quickly by
@@ -384,11 +459,16 @@ EXPORT_SYMBOL_GPL(psmash);static int rmpupdate (u64 pfn,struct rmp_state *state) {unsigned long paddr = pfn <<PAGE_SHIFT ; -int ret; +int ret, level;if (!cpu_feature_enabled (X86_FEATURE_SEV_SNP ))return -ENODEV ; + level =RMP_TO_PG_LEVEL (state->pagesize); + +if (adjust_direct_map (pfn, level)) +return -EFAULT ; + do { /* Binutils version 2.36 supports the RMPUPDATE mnemonic. */ asm volatile(".byte 0xF2, 0x0F, 0x01, 0xFE"
@@ -398,7 +478,8 @@ static int rmpupdate(u64 pfn, struct rmp_state *state) }while (ret ==RMPUPDATE_FAIL_OVERLAP );if (ret) { -pr_err ("RMPUPDATE failed for PFN %llx, ret: %d\n" , pfn, ret); +pr_err ("RMPUPDATE failed for PFN %llx, pg_level: %d, ret: %d\n" , + pfn, level, ret);dump_rmpentry (pfn);dump_stack ();return -EFAULT ;
PATCH v2 12/25 crypto: ccp: Define the SEV-SNP commands
In reply to Michael RothFrom: Brijesh Singh <[email protected]>
AMD introduced the next generation of SEV called SEV-SNP (Secure Nested Paging). SEV-SNP builds upon existing SEV and SEV-ES functionality while adding new hardware security protection.
Define the commands and structures used to communicate with the AMD-SP when creating and managing the SEV-SNP guests. The SEV-SNP firmware spec is available at developer.amd.com/sev.
[mdr: update SNP command list and SNP status struct based on current
spec, use C99 flexible arrays, fix kernel-doc issues]
--- drivers/crypto/ccp/sev-dev.c | 16 +++ include/linux/psp-sev.h | 265 +++++++++++++++++++++++++++++++++++ include/uapi/linux/psp-sev.h | 56 ++++++++ 3 files changed, 337 insertions(+)
drivers/crypto/ccp/sev-dev.c +16 -0
@@ -130,6 +130,8 @@ static int sev_cmd_buffer_len(int cmd)switch (cmd) {case SEV_CMD_INIT :return sizeof (struct sev_data_init );case SEV_CMD_INIT_EX :return sizeof (struct sev_data_init_ex ); +case SEV_CMD_SNP_SHUTDOWN_EX :return sizeof (struct sev_data_snp_shutdown_ex ); +case SEV_CMD_SNP_INIT_EX :return sizeof (struct sev_data_snp_init_ex );case SEV_CMD_PLATFORM_STATUS :return sizeof (struct sev_user_data_status );case SEV_CMD_PEK_CSR :return sizeof (struct sev_data_pek_csr );case SEV_CMD_PEK_CERT_IMPORT :return sizeof (struct sev_data_pek_cert_import );
@@ -158,6 +160,20 @@ static int sev_cmd_buffer_len(int cmd)case SEV_CMD_GET_ID :return sizeof (struct sev_data_get_id );case SEV_CMD_ATTESTATION_REPORT :return sizeof (struct sev_data_attestation_report );case SEV_CMD_SEND_CANCEL :return sizeof (struct sev_data_send_cancel ); +case SEV_CMD_SNP_GCTX_CREATE :return sizeof (struct sev_data_snp_addr ); +case SEV_CMD_SNP_LAUNCH_START :return sizeof (struct sev_data_snp_launch_start ); +case SEV_CMD_SNP_LAUNCH_UPDATE :return sizeof (struct sev_data_snp_launch_update ); +case SEV_CMD_SNP_ACTIVATE :return sizeof (struct sev_data_snp_activate ); +case SEV_CMD_SNP_DECOMMISSION :return sizeof (struct sev_data_snp_addr ); +case SEV_CMD_SNP_PAGE_RECLAIM :return sizeof (struct sev_data_snp_page_reclaim ); +case SEV_CMD_SNP_GUEST_STATUS :return sizeof (struct sev_data_snp_guest_status ); +case SEV_CMD_SNP_LAUNCH_FINISH :return sizeof (struct sev_data_snp_launch_finish ); +case SEV_CMD_SNP_DBG_DECRYPT :return sizeof (struct sev_data_snp_dbg ); +case SEV_CMD_SNP_DBG_ENCRYPT :return sizeof (struct sev_data_snp_dbg ); +case SEV_CMD_SNP_PAGE_UNSMASH :return sizeof (struct sev_data_snp_page_unsmash ); +case SEV_CMD_SNP_PLATFORM_STATUS :return sizeof (struct sev_data_snp_addr ); +case SEV_CMD_SNP_GUEST_REQUEST :return sizeof (struct sev_data_snp_guest_request ); +case SEV_CMD_SNP_CONFIG :return sizeof (struct sev_user_data_snp_config );default :return 0 ; }
include/linux/psp-sev.h +265 -0
277 lines not on this page · raw message
include/uapi/linux/psp-sev.h +56 -0
68 lines not on this page · raw message
Signature · 1 line
2.25.1
[tip: x86/sev] crypto: ccp: Define the SEV-SNP commands
The following commit has been merged into the x86/sev branch of tip:
The following commit has been merged into the x86/sev branch of tip:
Commit-ID: 3a45dc2b419e691f3dd7fb42c2a1b1cc8146be4f Gitweb: https://git.kernel.org/tip/3a45dc2b419e691f3dd7fb42c2a1b1cc8146be4f Author: Brijesh Singh <[email protected]> AuthorDate: Thu, 25 Jan 2024 22:11:12 -06:00 Committer: Borislav Petkov (AMD) <[email protected]> CommitterDate: Mon, 29 Jan 2024 20:34:18 +01:00
crypto: ccp: Define the SEV-SNP commands
AMD introduced the next generation of SEV called SEV-SNP (Secure Nested Paging). SEV-SNP builds upon existing SEV and SEV-ES functionality while adding new hardware security protection.
Define the commands and structures used to communicate with the AMD-SP when creating and managing the SEV-SNP guests. The SEV-SNP firmware spec is available at developer.amd.com/sev.
[ mdr: update SNP command list and SNP status struct based on current
spec, use C99 flexible arrays, fix kernel-doc issues. ]
--- drivers/crypto/ccp/sev-dev.c | 16 ++- include/linux/psp-sev.h | 265 ++++++++++++++++++++++++++++++++++- include/uapi/linux/psp-sev.h | 56 +++++++- 3 files changed, 337 insertions(+)
drivers/crypto/ccp/sev-dev.c +16 -0
@@ -130,6 +130,8 @@ static int sev_cmd_buffer_len(int cmd)switch (cmd) {case SEV_CMD_INIT :return sizeof (struct sev_data_init );case SEV_CMD_INIT_EX :return sizeof (struct sev_data_init_ex ); +case SEV_CMD_SNP_SHUTDOWN_EX :return sizeof (struct sev_data_snp_shutdown_ex ); +case SEV_CMD_SNP_INIT_EX :return sizeof (struct sev_data_snp_init_ex );case SEV_CMD_PLATFORM_STATUS :return sizeof (struct sev_user_data_status );case SEV_CMD_PEK_CSR :return sizeof (struct sev_data_pek_csr );case SEV_CMD_PEK_CERT_IMPORT :return sizeof (struct sev_data_pek_cert_import );
@@ -158,6 +160,20 @@ static int sev_cmd_buffer_len(int cmd)case SEV_CMD_GET_ID :return sizeof (struct sev_data_get_id );case SEV_CMD_ATTESTATION_REPORT :return sizeof (struct sev_data_attestation_report );case SEV_CMD_SEND_CANCEL :return sizeof (struct sev_data_send_cancel ); +case SEV_CMD_SNP_GCTX_CREATE :return sizeof (struct sev_data_snp_addr ); +case SEV_CMD_SNP_LAUNCH_START :return sizeof (struct sev_data_snp_launch_start ); +case SEV_CMD_SNP_LAUNCH_UPDATE :return sizeof (struct sev_data_snp_launch_update ); +case SEV_CMD_SNP_ACTIVATE :return sizeof (struct sev_data_snp_activate ); +case SEV_CMD_SNP_DECOMMISSION :return sizeof (struct sev_data_snp_addr ); +case SEV_CMD_SNP_PAGE_RECLAIM :return sizeof (struct sev_data_snp_page_reclaim ); +case SEV_CMD_SNP_GUEST_STATUS :return sizeof (struct sev_data_snp_guest_status ); +case SEV_CMD_SNP_LAUNCH_FINISH :return sizeof (struct sev_data_snp_launch_finish ); +case SEV_CMD_SNP_DBG_DECRYPT :return sizeof (struct sev_data_snp_dbg ); +case SEV_CMD_SNP_DBG_ENCRYPT :return sizeof (struct sev_data_snp_dbg ); +case SEV_CMD_SNP_PAGE_UNSMASH :return sizeof (struct sev_data_snp_page_unsmash ); +case SEV_CMD_SNP_PLATFORM_STATUS :return sizeof (struct sev_data_snp_addr ); +case SEV_CMD_SNP_GUEST_REQUEST :return sizeof (struct sev_data_snp_guest_request ); +case SEV_CMD_SNP_CONFIG :return sizeof (struct sev_user_data_snp_config );default :return 0 ; }
include/linux/psp-sev.h +265 -0
277 lines not on this page · raw message
include/uapi/linux/psp-sev.h +56 -0
68 lines not on this page · raw message
PATCH v2 13/25 crypto: ccp: Add support to initialize the AMD-SP for SEV-SNP
In reply to Michael RothFrom: Brijesh Singh <[email protected]>
Before SNP VMs can be launched, the platform must be appropriately configured and initialized. Platform initialization is accomplished via the SNP_INIT command.
During the execution of SNP_INIT command, the firmware configures and enables SNP security policy enforcement in many system components. Some system components write to regions of memory reserved by early x86 firmware (e.g. UEFI). Other system components write to regions provided by the operation system, hypervisor, or x86 firmware. Such system components can only write to HV-fixed pages or Default pages. They will error when attempting to write to pages in other page states after SNP_INIT enables their SNP enforcement.
Starting in SNP firmware v1.52, the SNP_INIT_EX command takes a list of system physical address ranges to convert into the HV-fixed page states during the RMP initialization. If INIT_RMP is 1, hypervisors should provide all system physical address ranges that the hypervisor will never assign to a guest until the next RMP re-initialization. For instance, the memory that UEFI reserves should be included in the range list. This allows system components that occasionally write to memory (e.g. logging to UEFI reserved regions) to not fail due to RMP initialization and SNP enablement.
Note that SNP_INIT(_EX) must not be executed while non-SEV guests are executing, otherwise it is possible that the system could reset or hang. The psp_init_on_probe module parameter was added for SEV/SEV-ES support and the init_ex_path module parameter to allow for time for the necessary file system to be mounted/available. SNP_INIT(_EX) does not use the file associated with init_ex_path. So, to avoid running into issues where SNP_INIT(_EX) is called while there are other running guests, issue it during module probe regardless of the psp_init_on_probe setting, but maintain the previous deferrable handling for SEV/SEV-ES initialization.
[mdr: squash in psp_init_on_probe changes from Tom, reduce
proliferation of 'probe' function parameter where possible]
--- arch/x86/kvm/svm/sev.c | 5 +- drivers/crypto/ccp/sev-dev.c | 280 ++++++++++++++++++++++++++++++++--- drivers/crypto/ccp/sev-dev.h | 2 + include/linux/psp-sev.h | 17 ++- 4 files changed, 281 insertions(+), 23 deletions(-)
arch/x86/kvm/svm/sev.c +4 -1
@@ -246,6 +246,7 @@ static void sev_unbind_asid(struct kvm *kvm, unsigned int handle)static int sev_guest_init (struct kvm *kvm,struct kvm_sev_cmd *argp) {struct kvm_sev_info *sev = &to_kvm_svm (kvm)->sev_info; +struct sev_platform_init_args init_args = {0 };int asid, ret;if (kvm->created_vcpus)
@@ -262,7 +263,8 @@ static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp) goto e_no_asid; sev->asid = asid; - ret =sev_platform_init (&argp->error); + init_args.probe = false; + ret =sev_platform_init (&init_args);if (ret) goto e_free;
@@ -274,6 +276,7 @@ static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp)return 0 ; e_free: + argp->error = init_args.error;sev_asid_free (sev); sev->asid =0 ; e_no_asid:
drivers/crypto/ccp/sev-dev.c +260 -20
358 lines not on this page · raw message
drivers/crypto/ccp/sev-dev.h +2 -0
@@ -52,6 +52,8 @@ struct sev_device {u8 build;void *cmd_buf; + +bool snp_initialized; };int sev_dev_init (struct psp_device *psp);
include/linux/psp-sev.h +15 -2
@@ -790,10 +790,23 @@ struct sev_data_snp_shutdown_ex {#ifdef CONFIG_CRYPTO_DEV_SP_PSP +/** +* struct sev_platform_init_args +* +* @error: SEV firmware error code +* @probe: True if this is being called as part of CCP module probe, which +* will defer SEV_INIT/SEV_INIT_EX firmware initialization until needed +* unless psp_init_on_probe module param is set +*/ +struct sev_platform_init_args { +int error; +bool probe; +}; +/** * sev_platform_init - perform SEV INIT command * -* @error: SEV command return code +* @args: struct sev_platform_init_args to pass in arguments * * Returns: * 0 if the SEV successfully processed the command
@@ -802,7 +815,7 @@ struct sev_data_snp_shutdown_ex {* -%ETIMEDOUT if the SEV command timed out * -%EIO if the SEV returned a non-zero return code */ -int sev_platform_init (int *error); +int sev_platform_init (struct sev_platform_init_args *args);/** * sev_platform_status - perform SEV PLATFORM_STATUS command
Signature · 1 line
2.25.1
Borislav Petkov
maintainerRe: [PATCH v2 13/25] crypto: ccp: Add support to initialize the AMD-SP for SEV-SNP
Quoted 10 lines · Michael Roth
include/linux/psp-sev.h +2
-0@@ -790,10 +790,23 @@ struct sev_data_snp_shutdown_ex {#ifdef CONFIG_CRYPTO_DEV_SP_PSP +/** +* struct sev_platform_init_args
include/linux/psp-sev.h struct sev_data_snp_shutdown_ex { · line 804
Quoted 10 lines
include/linux/psp-sev.h +10
-0+* +* @error: SEV firmware error code +* @probe: True if this is being called as part of CCP module probe, which +* will defer SEV_INIT/SEV_INIT_EX firmware initialization until needed +* unless psp_init_on_probe module param is set +*/ +struct sev_platform_init_args { +int error; +bool probe; +};
This struct definition cannot be under the ifdef, otherwise:
arch/x86/kvm/svm/sev.c: In function ‘sev_guest_init’:arch/x86/kvm/svm/sev.c:267:33: error: passing argument 1 of ‘sev_platform_init’ from incompatible pointer type [-Werror=incompatible-pointer-types]
267 | ret = sev_platform_init(&init_args);
| ^~~~~~~~~~
| |
| struct sev_platform_init_args *In file included from arch/x86/kvm/svm/sev.c:16:./include/linux/psp-sev.h:952:42: note: expected ‘int *’ but argument is of type ‘struct sev_platform_init_args *’
952 | static inline int sev_platform_init(int *error) { return -ENODEV; }
| ~~~~~^~~~~
cc1: all warnings being treated as errors
---
on a 32-bit allmodconfig.
Build fix:
---
include/linux/psp-sev.h +3 -3
@@ -797,8 +797,6 @@ struct sev_data_snp_commit {u32 len; } __packed; -#ifdef CONFIG_CRYPTO_DEV_SP_PSP -/** * struct sev_platform_init_args *
@@ -812,6 +810,8 @@ struct sev_platform_init_args {bool probe; }; +#ifdef CONFIG_CRYPTO_DEV_SP_PSP +/** * sev_platform_init - perform SEV INIT command *
@@ -949,7 +949,7 @@ void snp_free_firmware_page(void *addr);static inline int sev_platform_status (struct sev_user_data_status *status,int *error) {return -ENODEV ; } -static inline int sev_platform_init (int *error) {return -ENODEV ; } +static inline int sev_platform_init (struct sev_platform_init_args *args) {return -ENODEV ; }static inline int sev_guest_deactivate (struct sev_data_deactivate *data,int *error) {return -ENODEV ; }
Signature · 4 lines
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette[tip: x86/sev] crypto: ccp: Add support to initialize the AMD-SP for SEV-SNP
In reply to Michael RothThe following commit has been merged into the x86/sev branch of tip:
The following commit has been merged into the x86/sev branch of tip:
Commit-ID: 1ca5614b84eed5904f65f143e0e7aaab0ac4c6b2 Gitweb: https://git.kernel.org/tip/1ca5614b84eed5904f65f143e0e7aaab0ac4c6b2 Author: Brijesh Singh <[email protected]> AuthorDate: Thu, 25 Jan 2024 22:11:13 -06:00 Committer: Borislav Petkov (AMD) <[email protected]> CommitterDate: Mon, 29 Jan 2024 20:34:18 +01:00
crypto: ccp: Add support to initialize the AMD-SP for SEV-SNP
Before SNP VMs can be launched, the platform must be appropriately configured and initialized via the SNP_INIT command.
During the execution of SNP_INIT command, the firmware configures and enables SNP security policy enforcement in many system components. Some system components write to regions of memory reserved by early x86 firmware (e.g. UEFI). Other system components write to regions provided by the operation system, hypervisor, or x86 firmware. Such system components can only write to HV-fixed pages or Default pages. They will error when attempting to write to pages in other page states after SNP_INIT enables their SNP enforcement.
Starting in SNP firmware v1.52, the SNP_INIT_EX command takes a list of system physical address ranges to convert into the HV-fixed page states during the RMP initialization. If INIT_RMP is 1, hypervisors should provide all system physical address ranges that the hypervisor will never assign to a guest until the next RMP re-initialization.
For instance, the memory that UEFI reserves should be included in the range list. This allows system components that occasionally write to memory (e.g. logging to UEFI reserved regions) to not fail due to RMP initialization and SNP enablement.
Note that SNP_INIT(_EX) must not be executed while non-SEV guests are executing, otherwise it is possible that the system could reset or hang. The psp_init_on_probe module parameter was added for SEV/SEV-ES support and the init_ex_path module parameter to allow for time for the necessary file system to be mounted/available.
SNP_INIT(_EX) does not use the file associated with init_ex_path. So, to avoid running into issues where SNP_INIT(_EX) is called while there are other running guests, issue it during module probe regardless of the psp_init_on_probe setting, but maintain the previous deferrable handling for SEV/SEV-ES initialization.
[ mdr: Squash in psp_init_on_probe changes from Tom, reduce
proliferation of 'probe' function parameter where possible.
bp: Fix 32-bit allmodconfig build. ]
--- arch/x86/kvm/svm/sev.c | 5 +- drivers/crypto/ccp/sev-dev.c | 280 +++++++++++++++++++++++++++++++--- drivers/crypto/ccp/sev-dev.h | 2 +- include/linux/psp-sev.h | 19 +- 4 files changed, 282 insertions(+), 24 deletions(-)
arch/x86/kvm/svm/sev.c +4 -1
@@ -246,6 +246,7 @@ static void sev_unbind_asid(struct kvm *kvm, unsigned int handle)static int sev_guest_init (struct kvm *kvm,struct kvm_sev_cmd *argp) {struct kvm_sev_info *sev = &to_kvm_svm (kvm)->sev_info; +struct sev_platform_init_args init_args = {0 };int asid, ret;if (kvm->created_vcpus)
@@ -262,7 +263,8 @@ static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp) goto e_no_asid; sev->asid = asid; - ret =sev_platform_init (&argp->error); + init_args.probe = false; + ret =sev_platform_init (&init_args);if (ret) goto e_free;
@@ -274,6 +276,7 @@ static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp)return 0 ; e_free: + argp->error = init_args.error;sev_asid_free (sev); sev->asid =0 ; e_no_asid:
drivers/crypto/ccp/sev-dev.c +260 -20
358 lines not on this page · raw message
drivers/crypto/ccp/sev-dev.h +2 -0
@@ -52,6 +52,8 @@ struct sev_device {u8 build;void *cmd_buf; + +bool snp_initialized; };int sev_dev_init (struct psp_device *psp);
include/linux/psp-sev.h +16 -3
@@ -788,12 +788,25 @@ struct sev_data_snp_shutdown_ex { u32 rsvd1:31; } __packed; +/** +* struct sev_platform_init_args +* +* @error: SEV firmware error code +* @probe: True if this is being called as part of CCP module probe, which +* will defer SEV_INIT/SEV_INIT_EX firmware initialization until needed +* unless psp_init_on_probe module param is set +*/ +struct sev_platform_init_args { +int error; +bool probe; +}; +#ifdef CONFIG_CRYPTO_DEV_SP_PSP /** * sev_platform_init - perform SEV INIT command * -* @error: SEV command return code +* @args: struct sev_platform_init_args to pass in arguments * * Returns: * 0 if the SEV successfully processed the command
@@ -802,7 +815,7 @@ struct sev_data_snp_shutdown_ex {* -%ETIMEDOUT if the SEV command timed out * -%EIO if the SEV returned a non-zero return code */ -int sev_platform_init (int *error); +int sev_platform_init (struct sev_platform_init_args *args);/** * sev_platform_status - perform SEV PLATFORM_STATUS command
@@ -909,7 +922,7 @@ void *psp_copy_user_blob(u64 uaddr, u32 len);static inline int sev_platform_status (struct sev_user_data_status *status,int *error) {return -ENODEV ; } -static inline int sev_platform_init (int *error) {return -ENODEV ; } +static inline int sev_platform_init (struct sev_platform_init_args *args) {return -ENODEV ; }static inline int sev_guest_deactivate (struct sev_data_deactivate *data,int *error) {return -ENODEV ; }
PATCH v2 14/25 crypto: ccp: Provide API to issue SEV and SNP commands
In reply to Michael RothFrom: Brijesh Singh <[email protected]>
Export sev_do_cmd() as a generic API for the hypervisor to issue commands to manage an SEV and SNP guest. The commands for SEV and SNP are defined in the SEV and SEV-SNP firmware specifications.
--- drivers/crypto/ccp/sev-dev.c | 3 ++- include/linux/psp-sev.h | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-)
drivers/crypto/ccp/sev-dev.c +2 -1
@@ -431,7 +431,7 @@ static int __sev_do_cmd_locked(int cmd, void *data, int *psp_ret)return ret; } -static int sev_do_cmd (int cmd,void *data,int *psp_ret) +int sev_do_cmd (int cmd,void *data,int *psp_ret) {int rc;
@@ -441,6 +441,7 @@ static int sev_do_cmd(int cmd, void *data, int *psp_ret)return rc; } +EXPORT_SYMBOL_GPL (sev_do_cmd);static int __sev_init_locked (int *error) {
include/linux/psp-sev.h +19 -0
@@ -915,6 +915,22 @@ int sev_guest_df_flush(int *error);*/ int sev_guest_decommission (struct sev_data_decommission *data,int *error); +/** +* sev_do_cmd - issue an SEV or an SEV-SNP command +* +* @cmd: SEV or SEV-SNP firmware command to issue +* @data: arguments for firmware command +* @psp_ret: SEV command return code +* +* Returns: +* 0 if the SEV device successfully processed the command +* -%ENODEV if the PSP device is not available +* -%ENOTSUPP if PSP device does not support SEV +* -%ETIMEDOUT if the SEV command timed out +* -%EIO if PSP device returned a non-zero return code +*/ +int sev_do_cmd (int cmd,void *data,int *psp_ret); +void *psp_copy_user_blob (u64 uaddr,u32 len);#else /* !CONFIG_CRYPTO_DEV_SP_PSP */
@@ -930,6 +946,9 @@ sev_guest_deactivate(struct sev_data_deactivate *data, int *error) { return -ENOstatic inline int sev_guest_decommission (struct sev_data_decommission *data,int *error) {return -ENODEV ; } +static inline int +sev_do_cmd (int cmd,void *data,int *psp_ret) {return -ENODEV ; } +static inline int sev_guest_activate (struct sev_data_activate *data,int *error) {return -ENODEV ; }
Signature · 1 line
2.25.1
[tip: x86/sev] crypto: ccp: Provide an API to issue SEV and SNP commands
The following commit has been merged into the x86/sev branch of tip:
The following commit has been merged into the x86/sev branch of tip:
Commit-ID: 18085ac2f2fbf56aee9cbf5846740150e394f4f4 Gitweb: https://git.kernel.org/tip/18085ac2f2fbf56aee9cbf5846740150e394f4f4 Author: Brijesh Singh <[email protected]> AuthorDate: Thu, 25 Jan 2024 22:11:14 -06:00 Committer: Borislav Petkov (AMD) <[email protected]> CommitterDate: Mon, 29 Jan 2024 20:34:18 +01:00
crypto: ccp: Provide an API to issue SEV and SNP commands
Export sev_do_cmd() as a generic API for the hypervisor to issue commands to manage an SEV or an SNP guest. The commands for SEV and SNP are defined in the SEV and SEV-SNP firmware specifications.
--- drivers/crypto/ccp/sev-dev.c | 3 ++- include/linux/psp-sev.h | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-)
drivers/crypto/ccp/sev-dev.c +2 -1
@@ -431,7 +431,7 @@ static int __sev_do_cmd_locked(int cmd, void *data, int *psp_ret)return ret; } -static int sev_do_cmd (int cmd,void *data,int *psp_ret) +int sev_do_cmd (int cmd,void *data,int *psp_ret) {int rc;
@@ -441,6 +441,7 @@ static int sev_do_cmd(int cmd, void *data, int *psp_ret)return rc; } +EXPORT_SYMBOL_GPL (sev_do_cmd);static int __sev_init_locked (int *error) {
include/linux/psp-sev.h +19 -0
@@ -915,6 +915,22 @@ int sev_guest_df_flush(int *error);*/ int sev_guest_decommission (struct sev_data_decommission *data,int *error); +/** +* sev_do_cmd - issue an SEV or an SEV-SNP command +* +* @cmd: SEV or SEV-SNP firmware command to issue +* @data: arguments for firmware command +* @psp_ret: SEV command return code +* +* Returns: +* 0 if the SEV device successfully processed the command +* -%ENODEV if the PSP device is not available +* -%ENOTSUPP if PSP device does not support SEV +* -%ETIMEDOUT if the SEV command timed out +* -%EIO if PSP device returned a non-zero return code +*/ +int sev_do_cmd (int cmd,void *data,int *psp_ret); +void *psp_copy_user_blob (u64 uaddr,u32 len);#else /* !CONFIG_CRYPTO_DEV_SP_PSP */
@@ -931,6 +947,9 @@ static inline int sev_guest_decommission(struct sev_data_decommission *data, int *error) { return -ENODEV; }static inline int +sev_do_cmd (int cmd,void *data,int *psp_ret) {return -ENODEV ; } + +static inline int sev_guest_activate (struct sev_data_activate *data,int *error) {return -ENODEV ; }static inline int sev_guest_df_flush (int *error) {return -ENODEV ; }
PATCH v2 15/25 x86/sev: Introduce snp leaked pages list
In reply to Michael RothFrom: Ashish Kalra <[email protected]>
Pages are unsafe to be released back to the page-allocator, if they have been transitioned to firmware/guest state and can't be reclaimed or transitioned back to hypervisor/shared state. In this case add them to an internal leaked pages list to ensure that they are not freed or touched/accessed to cause fatal page faults.
--- arch/x86/include/asm/sev.h | 2 ++ arch/x86/virt/svm/sev.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+)
arch/x86/include/asm/sev.h +2 -0
@@ -264,6 +264,7 @@ void snp_dump_hva_rmpentry(unsigned long address);int psmash (u64 pfn);int rmp_make_private (u64 pfn,u64 gpa,enum pg_level level,int asid,bool immutable);int rmp_make_shared (u64 pfn,enum pg_level level); +void snp_leak_pages (u64 pfn,unsigned int npages);#else static inline bool snp_probe_rmptable_info (void ) {return false; }static inline int snp_lookup_rmpentry (u64 pfn,bool *assigned,int *level) {return -ENODEV ; }
@@ -275,6 +276,7 @@ static inline int rmp_make_private(u64 pfn, u64 gpa, enum pg_level level, int asreturn -ENODEV ; }static inline int rmp_make_shared (u64 pfn,enum pg_level level) {return -ENODEV ; } +static inline void snp_leak_pages (u64 pfn,unsigned int npages) {}#endif #endif
arch/x86/virt/svm/sev.c +34 -0
@@ -65,6 +65,11 @@ static u64 probed_rmp_base, probed_rmp_size;static struct rmpentry *rmptable__ro_after_init ;static u64 rmptable_max_pfn__ro_after_init ; +static LIST_HEAD (snp_leaked_pages_list); +static DEFINE_SPINLOCK (snp_leaked_pages_list_lock); + +static unsigned long snp_nr_leaked_pages; +#undef pr_fmt#define pr_fmt (fmt) "SEV-SNP: " fmt
@@ -505,3 +510,32 @@ int rmp_make_shared(u64 pfn, enum pg_level level)return rmpupdate (pfn, &state); }EXPORT_SYMBOL_GPL (rmp_make_shared); + +void snp_leak_pages (u64 pfn,unsigned int npages) +{ +struct page *page =pfn_to_page (pfn); + +pr_warn ("Leaking PFN range 0x%llx-0x%llx\n" , pfn, pfn + npages); + +spin_lock (&snp_leaked_pages_list_lock); +while (npages--) { +/* +* Reuse the page's buddy list for chaining into the leaked +* pages list. This page should not be on a free list currently +* and is also unsafe to be added to a free list. +*/ +if (likely (!PageCompound (page)) || + (PageHead (page) &&compound_nr (page) <= npages)) +/* +* Skip inserting tail pages of compound page as +* page->buddy_list of tail pages is not usable. +*/ +list_add_tail (&page->buddy_list, &snp_leaked_pages_list); +dump_rmpentry (pfn); + snp_nr_leaked_pages++; + pfn++; + page++; + } +spin_unlock (&snp_leaked_pages_list_lock); +} +EXPORT_SYMBOL_GPL (snp_leak_pages);
Signature · 1 line
2.25.1
Re: [PATCH v2 15/25] x86/sev: Introduce snp leaked pages list
Quoted 8 lines · Michael Roth
From: Ashish Kalra <[email protected]>
Pages are unsafe to be released back to the page-allocator, if they have been transitioned to
firmware/gueststate and can't be reclaimed or transitioned back to hypervisor/shared state. In this case add them to an internal leaked pages list to ensure that they are not freed or touched/accessed to cause fatal page faults.
Quoted 4 lines
Suggested-by Vlastimil Babka <[email protected]>Signed-off-by Ashish Kalra <[email protected]>[mdr: relocate to arch/x86/virt/svm/sev.c]Signed-off-by Michael Roth <[email protected]>
Some minor nitpicks:
Quoted 56 lines
--- arch/x86/include/asm/sev.h | 2 ++ arch/x86/virt/svm/sev.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+)arch/x86/include/asm/sev.h +2
-0@@ -264,6 +264,7 @@ void snp_dump_hva_rmpentry(unsigned long address);int psmash (u64 pfn);int rmp_make_private (u64 pfn,u64 gpa,enum pg_level level,int asid,bool immutable);int rmp_make_shared (u64 pfn,enum pg_level level); +void snp_leak_pages (u64 pfn,unsigned int npages);#else static inline bool snp_probe_rmptable_info (void ) {return false; }static inline int snp_lookup_rmpentry (u64 pfn,bool *assigned,int *level) {return -ENODEV ; }@@ -275,6 +276,7 @@ static inline int rmp_make_private(u64 pfn, u64 gpa, enum pg_level level, int asreturn -ENODEV ; }static inline int rmp_make_shared (u64 pfn,enum pg_level level) {return -ENODEV ; } +static inline void snp_leak_pages (u64 pfn,unsigned int npages) {}#endif #endif arch/x86/virt/svm/sev.c +16
-0@@ -65,6 +65,11 @@ static u64 probed_rmp_base, probed_rmp_size;static struct rmpentry *rmptable__ro_after_init ;static u64 rmptable_max_pfn__ro_after_init ; +static LIST_HEAD (snp_leaked_pages_list); +static DEFINE_SPINLOCK (snp_leaked_pages_list_lock); + +static unsigned long snp_nr_leaked_pages; +#undef pr_fmt#define pr_fmt (fmt) "SEV-SNP: " fmt@@ -505,3 +510,32 @@ int rmp_make_shared(u64 pfn, enum pg_level level)return rmpupdate (pfn, &state); }EXPORT_SYMBOL_GPL (rmp_make_shared); + +void snp_leak_pages (u64 pfn,unsigned int npages) +{ +struct page *page =pfn_to_page (pfn); + +pr_warn ("Leaking PFN range 0x%llx-0x%llx\n" , pfn, pfn + npages); + +spin_lock (&snp_leaked_pages_list_lock); +while (npages--) { +/* +* Reuse the page's buddy list for chaining into the leaked
arch/x86/virt/svm/sev.c int rmp_make_shared(u64 pfn, enum pg_level level) · line 533
Quoted 10 lines
arch/x86/virt/svm/sev.c +10
-0+* pages list. This page should not be on a free list currently +* and is also unsafe to be added to a free list. +*/ +if (likely (!PageCompound (page)) || + (PageHead (page) &&compound_nr (page) <= npages)) +/* +* Skip inserting tail pages of compound page as +* page->buddy_list of tail pages is not usable. +*/ +list_add_tail (&page->buddy_list, &snp_leaked_pages_list);
Even though it's not necessary for correctness, with the comment I'd put the whole block into { } to make easier to follow. Or just move the comment above the if() itself?
Quoted 8 lines
arch/x86/virt/svm/sev.c +8
-0+dump_rmpentry (pfn); + snp_nr_leaked_pages++; + pfn++; + page++; + } +spin_unlock (&snp_leaked_pages_list_lock); +} +EXPORT_SYMBOL_GPL (snp_leak_pages);
Re: [PATCH v2 15/25] x86/sev: Introduce snp leaked pages list
Quoted 3 lines · Vlastimil Babka
Reviewed-by Vlastimil Babka <[email protected]>Some minor nitpicks:
Thanks, here's what I have applied:
commit c3875aff4e0739a6af385795470da70d675a7635
Author: Ashish Kalra <[email protected]> Date: Thu Jan 25 22:11:15 2024 -0600
x86/sev: Introduce an SNP leaked pages list
Pages are unsafe to be released back to the page-allocator if they have been transitioned to firmware/guest state and can't be reclaimed or transitioned back to hypervisor/shared state. In this case, add them to an internal leaked pages list to ensure that they are not freed or touched/accessed to cause fatal page faults.
[ mdr: Relocate to arch/x86/virt/svm/sev.c ]
arch/x86/include/asm/sev.h +2 -0
@@ -264,6 +264,7 @@ void snp_dump_hva_rmpentry(unsigned long address);int psmash (u64 pfn);int rmp_make_private (u64 pfn,u64 gpa,enum pg_level level,int asid,bool immutable);int rmp_make_shared (u64 pfn,enum pg_level level); +void snp_leak_pages (u64 pfn,unsigned int npages);#else static inline bool snp_probe_rmptable_info (void ) {return false; }static inline int snp_lookup_rmpentry (u64 pfn,bool *assigned,int *level) {return -ENODEV ; }
@@ -275,6 +276,7 @@ static inline int rmp_make_private(u64 pfn, u64 gpa, enum pg_level level, int asreturn -ENODEV ; }static inline int rmp_make_shared (u64 pfn,enum pg_level level) {return -ENODEV ; } +static inline void snp_leak_pages (u64 pfn,unsigned int npages) {}#endif #endif
arch/x86/virt/svm/sev.c +37 -0
@@ -65,6 +65,11 @@ static u64 probed_rmp_base, probed_rmp_size;static struct rmpentry *rmptable__ro_after_init ;static u64 rmptable_max_pfn__ro_after_init ; +static LIST_HEAD (snp_leaked_pages_list); +static DEFINE_SPINLOCK (snp_leaked_pages_list_lock); + +static unsigned long snp_nr_leaked_pages; +#undef pr_fmt#define pr_fmt (fmt) "SEV-SNP: " fmt
@@ -515,3 +520,35 @@ int rmp_make_shared(u64 pfn, enum pg_level level)return rmpupdate (pfn, &state); }EXPORT_SYMBOL_GPL (rmp_make_shared); + +void snp_leak_pages (u64 pfn,unsigned int npages) +{ +struct page *page =pfn_to_page (pfn); + +pr_warn ("Leaking PFN range 0x%llx-0x%llx\n" , pfn, pfn + npages); + +spin_lock (&snp_leaked_pages_list_lock); +while (npages--) { + +/* +* Reuse the page's buddy list for chaining into the leaked +* pages list. This page should not be on a free list currently +* and is also unsafe to be added to a free list. +*/ +if (likely (!PageCompound (page)) || + +/* +* Skip inserting tail pages of compound page as +* page->buddy_list of tail pages is not usable. +*/ + (PageHead (page) &&compound_nr (page) <= npages)) +list_add_tail (&page->buddy_list, &snp_leaked_pages_list); + +dump_rmpentry (pfn); + snp_nr_leaked_pages++; + pfn++; + page++; + } +spin_unlock (&snp_leaked_pages_list_lock); +} +EXPORT_SYMBOL_GPL (snp_leak_pages);
Signature · 4 lines
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette[tip: x86/sev] x86/sev: Introduce an SNP leaked pages list
In reply to Michael RothThe following commit has been merged into the x86/sev branch of tip:
The following commit has been merged into the x86/sev branch of tip:
Commit-ID: 8dac642999b1542e0f0abefba100d8bd11226c83 Gitweb: https://git.kernel.org/tip/8dac642999b1542e0f0abefba100d8bd11226c83 Author: Ashish Kalra <[email protected]> AuthorDate: Thu, 25 Jan 2024 22:11:15 -06:00 Committer: Borislav Petkov (AMD) <[email protected]> CommitterDate: Mon, 29 Jan 2024 20:34:18 +01:00
x86/sev: Introduce an SNP leaked pages list
Pages are unsafe to be released back to the page-allocator if they have been transitioned to firmware/guest state and can't be reclaimed or transitioned back to hypervisor/shared state. In this case, add them to an internal leaked pages list to ensure that they are not freed or touched/accessed to cause fatal page faults.
[ mdr: Relocate to arch/x86/virt/svm/sev.c ]
--- arch/x86/include/asm/sev.h | 2 ++- arch/x86/virt/svm/sev.c | 37 +++++++++++++++++++++++++++++++++++++- 2 files changed, 39 insertions(+)
arch/x86/include/asm/sev.h +2 -0
@@ -264,6 +264,7 @@ void snp_dump_hva_rmpentry(unsigned long address);int psmash (u64 pfn);int rmp_make_private (u64 pfn,u64 gpa,enum pg_level level,u32 asid,bool immutable);int rmp_make_shared (u64 pfn,enum pg_level level); +void snp_leak_pages (u64 pfn,unsigned int npages);#else static inline bool snp_probe_rmptable_info (void ) {return false; }static inline int snp_lookup_rmpentry (u64 pfn,bool *assigned,int *level) {return -ENODEV ; }
@@ -275,6 +276,7 @@ static inline int rmp_make_private(u64 pfn, u64 gpa, enum pg_level level, u32 asreturn -ENODEV ; }static inline int rmp_make_shared (u64 pfn,enum pg_level level) {return -ENODEV ; } +static inline void snp_leak_pages (u64 pfn,unsigned int npages) {}#endif #endif
arch/x86/virt/svm/sev.c +37 -0
@@ -65,6 +65,11 @@ static u64 probed_rmp_base, probed_rmp_size;static struct rmpentry *rmptable__ro_after_init ;static u64 rmptable_max_pfn__ro_after_init ; +static LIST_HEAD (snp_leaked_pages_list); +static DEFINE_SPINLOCK (snp_leaked_pages_list_lock); + +static unsigned long snp_nr_leaked_pages; +#undef pr_fmt#define pr_fmt (fmt) "SEV-SNP: " fmt
@@ -515,3 +520,35 @@ int rmp_make_shared(u64 pfn, enum pg_level level)return rmpupdate (pfn, &state); }EXPORT_SYMBOL_GPL (rmp_make_shared); + +void snp_leak_pages (u64 pfn,unsigned int npages) +{ +struct page *page =pfn_to_page (pfn); + +pr_warn ("Leaking PFN range 0x%llx-0x%llx\n" , pfn, pfn + npages); + +spin_lock (&snp_leaked_pages_list_lock); +while (npages--) { + +/* +* Reuse the page's buddy list for chaining into the leaked +* pages list. This page should not be on a free list currently +* and is also unsafe to be added to a free list. +*/ +if (likely (!PageCompound (page)) || + +/* +* Skip inserting tail pages of compound page as +* page->buddy_list of tail pages is not usable. +*/ + (PageHead (page) &&compound_nr (page) <= npages)) +list_add_tail (&page->buddy_list, &snp_leaked_pages_list); + +dump_rmpentry (pfn); + snp_nr_leaked_pages++; + pfn++; + page++; + } +spin_unlock (&snp_leaked_pages_list_lock); +} +EXPORT_SYMBOL_GPL (snp_leak_pages);
PATCH v2 16/25 crypto: ccp: Handle the legacy TMR allocation when SNP is enabled
In reply to Michael RothFrom: Brijesh Singh <[email protected]>
The behavior and requirement for the SEV-legacy command is altered when the SNP firmware is in the INIT state. See SEV-SNP firmware ABI specification for more details.
Allocate the Trusted Memory Region (TMR) as a 2MB-sized/aligned region when SNP is enabled to satisfy new requirements for SNP. Continue allocating a 1MB-sized region for !SNP configuration.
[mdr: use struct sev_data_snp_page_reclaim instead of passing paddr
directly to SEV_CMD_SNP_PAGE_RECLAIM]
--- drivers/crypto/ccp/sev-dev.c | 176 +++++++++++++++++++++++++++++++---- include/linux/psp-sev.h | 9 ++ 2 files changed, 165 insertions(+), 20 deletions(-)
drivers/crypto/ccp/sev-dev.c +156 -20
@@ -30,6 +30,7 @@#include <asm/smp.h> #include <asm/cacheflush.h> #include <asm/e820/types.h> +#include <asm/sev.h> #include "psp-dev.h" #include "sev-dev.h"
@@ -73,9 +74,14 @@ static int psp_timeout;* The TMR is a 1MB area that must be 1MB aligned. Use the page allocator * to allocate the memory, which will return aligned memory for the specified * allocation order. +* +* When SEV-SNP is enabled the TMR needs to be 2MB aligned and 2MB sized. */ -#define SEV_ES_TMR_SIZE (1024 * 1024) +#define SEV_TMR_SIZE (1024 * 1024) +#define SNP_TMR_SIZE (2 * 1024 * 1024) +static void *sev_es_tmr; +static size_t sev_es_tmr_size =SEV_TMR_SIZE ;/* INIT_EX NV Storage: * The NV Storage is a 32Kb area and must be 4Kb page aligned. Use the page
@@ -192,17 +198,6 @@ static int sev_cmd_buffer_len(int cmd)return 0 ; } -static void *sev_fw_alloc (unsigned long len) -{ -struct page *page; - - page =alloc_pages (GFP_KERNEL ,get_order (len)); -if (!page) -return NULL ; - -return page_address (page); -} -static struct file *open_file_as_root (const char *filename,int flags,umode_t mode) {struct file *fp;
@@ -333,6 +328,142 @@ static int sev_write_init_ex_file_if_required(int cmd_id)return sev_write_init_ex_file (); } +/* +* snp_reclaim_pages() needs __sev_do_cmd_locked(), and __sev_do_cmd_locked() +* needs snp_reclaim_pages(), so a forward declaration is needed. +*/ +static int __sev_do_cmd_locked (int cmd,void *data,int *psp_ret); + +static int snp_reclaim_pages (unsigned long paddr,unsigned int npages,bool locked) +{ +int ret, err, i; + + paddr =__sme_clr (ALIGN_DOWN (paddr,PAGE_SIZE )); + +for (i =0 ; i < npages; i++, paddr +=PAGE_SIZE ) { +struct sev_data_snp_page_reclaim data = {0 }; + + data.paddr = paddr; + +if (locked) + ret =__sev_do_cmd_locked (SEV_CMD_SNP_PAGE_RECLAIM , &data, &err); +else + ret =sev_do_cmd (SEV_CMD_SNP_PAGE_RECLAIM , &data, &err); + +if (ret) + goto cleanup; + + ret =rmp_make_shared (__phys_to_pfn (paddr),PG_LEVEL_4K ); +if (ret) + goto cleanup; + } + +return 0 ; + +cleanup: +/* +* If there was a failure reclaiming the page then it is no longer safe +* to release it back to the system; leak it instead. +*/ +snp_leak_pages (__phys_to_pfn (paddr), npages - i); +return ret; +} + +static int rmp_mark_pages_firmware (unsigned long paddr,unsigned int npages,bool locked) +{ +unsigned long pfn =__sme_clr (paddr) >>PAGE_SHIFT ; +int rc, i; + +for (i =0 ; i < npages; i++, pfn++) { + rc =rmp_make_private (pfn,0 ,PG_LEVEL_4K ,0 , true); +if (rc) + goto cleanup; + } + +return 0 ; + +cleanup: +/* +* Try unrolling the firmware state changes by +* reclaiming the pages which were already changed to the +* firmware state. +*/ +snp_reclaim_pages (paddr, i, locked); + +return rc; +} + +static struct page *__snp_alloc_firmware_pages (gfp_t gfp_mask,int order) +{ +unsigned long npages =1ul << order, paddr; +struct sev_device *sev; +struct page *page; + +if (!psp_master || !psp_master->sev_data) +return NULL ; + + page =alloc_pages (gfp_mask, order); +if (!page) +return NULL ; + +/* If SEV-SNP is initialized then add the page in RMP table. */ + sev = psp_master->sev_data; +if (!sev->snp_initialized) +return page; + + paddr =__pa ((unsigned long )page_address (page)); +if (rmp_mark_pages_firmware (paddr, npages, false)) +return NULL ; + +return page; +} + +void *snp_alloc_firmware_page (gfp_t gfp_mask) +{ +struct page *page; + + page =__snp_alloc_firmware_pages (gfp_mask,0 ); + +return page ?page_address (page) :NULL ; +} +EXPORT_SYMBOL_GPL (snp_alloc_firmware_page); + +static void __snp_free_firmware_pages (struct page *page,int order,bool locked) +{ +struct sev_device *sev = psp_master->sev_data; +unsigned long paddr, npages =1ul << order; + +if (!page) +return ; + + paddr =__pa ((unsigned long )page_address (page)); +if (sev->snp_initialized && +snp_reclaim_pages (paddr, npages, locked)) +return ; + +__free_pages (page, order); +} + +void snp_free_firmware_page (void *addr) +{ +if (!addr) +return ; + +__snp_free_firmware_pages (virt_to_page (addr),0 , false); +} +EXPORT_SYMBOL_GPL (snp_free_firmware_page); + +static void *sev_fw_alloc (unsigned long len) +{ +struct page *page; + + page =__snp_alloc_firmware_pages (GFP_KERNEL ,get_order (len)); +if (!page) +return NULL ; + +return page_address (page); +} +static int __sev_do_cmd_locked (int cmd,void *data,int *psp_ret) {struct psp_device *psp = psp_master;
@@ -456,7 +587,7 @@ static int __sev_init_locked(int *error) data.tmr_address =__pa (sev_es_tmr); data.flags |=SEV_INIT_FLAGS_SEV_ES ; - data.tmr_len =SEV_ES_TMR_SIZE ; + data.tmr_len = sev_es_tmr_size; }return __sev_do_cmd_locked (SEV_CMD_INIT , &data, error);
@@ -479,7 +610,7 @@ static int __sev_init_ex_locked(int *error) data.tmr_address =__pa (sev_es_tmr); data.flags |=SEV_INIT_FLAGS_SEV_ES ; - data.tmr_len =SEV_ES_TMR_SIZE ; + data.tmr_len = sev_es_tmr_size; }return __sev_do_cmd_locked (SEV_CMD_INIT_EX , &data, error);
@@ -623,6 +754,8 @@ static int __sev_snp_init_locked(int *error) sev->snp_initialized = true;dev_dbg (sev->dev,"SEV-SNP firmware initialized\n" ); + sev_es_tmr_size =SNP_TMR_SIZE ; +return rc; }
@@ -641,14 +774,16 @@ static int __sev_platform_init_locked(int *error)if (!sev_es_tmr) {/* Obtain the TMR memory area for SEV-ES use */ - sev_es_tmr =sev_fw_alloc (SEV_ES_TMR_SIZE ); -if (sev_es_tmr) + sev_es_tmr =sev_fw_alloc (sev_es_tmr_size); +if (sev_es_tmr) {/* Must flush the cache before giving it to the firmware */ -clflush_cache_range (sev_es_tmr,SEV_ES_TMR_SIZE ); -else +if (!sev->snp_initialized) +clflush_cache_range (sev_es_tmr, sev_es_tmr_size); + }else {dev_warn (sev->dev,"SEV: TMR allocation failed, SEV-ES support unavailable\n" ); } + }if (sev_init_ex_buffer) { rc =sev_read_init_ex_file ();
@@ -1546,8 +1681,9 @@ static void sev_firmware_shutdown(struct sev_device *sev)/* The TMR area was encrypted, flush it from the cache */ wbinvd_on_all_cpus (); -free_pages ((unsigned long )sev_es_tmr, -get_order (SEV_ES_TMR_SIZE )); +__snp_free_firmware_pages (virt_to_page (sev_es_tmr), +get_order (sev_es_tmr_size), + false); sev_es_tmr =NULL ; }
include/linux/psp-sev.h +9 -0
@@ -932,6 +932,8 @@ int sev_guest_decommission(struct sev_data_decommission *data, int *error);int sev_do_cmd (int cmd,void *data,int *psp_ret);void *psp_copy_user_blob (u64 uaddr,u32 len); +void *snp_alloc_firmware_page (gfp_t mask); +void snp_free_firmware_page (void *addr);#else /* !CONFIG_CRYPTO_DEV_SP_PSP */
@@ -959,6 +961,13 @@ sev_issue_cmd_external_user(struct file *filep, unsigned int id, void *data, intstatic inline void *psp_copy_user_blob (u64 __user uaddr,u32 len) {return ERR_PTR (-EINVAL ); } +static inline void *snp_alloc_firmware_page (gfp_t mask) +{ +return NULL ; +} + +static inline void snp_free_firmware_page (void *addr) { } +#endif /* CONFIG_CRYPTO_DEV_SP_PSP */ #endif /* __PSP_SEV_H__ */
Signature · 1 line
2.25.1
Borislav Petkov
maintainerRe: [PATCH v2 16/25] crypto: ccp: Handle the legacy TMR allocation when SNP is enabled
Review static int __sev_platform_init_locked(int *error)
Quoted 1 line · Michael Roth
(quoted diff) +0
-0@@ -641,14 +774,16 @@ static int __sev_platform_init_locked(int *error)
That function - especially when looking at the next patch - becomes too big and hard to follow.
Let's add subfunctions for each thing, diff ontop:
---
drivers/crypto/ccp/sev-dev.c +17 -12
@@ -759,6 +759,22 @@ static int __sev_snp_init_locked(int *error)return rc; } +static void __sev_platform_init_handle_tmr (struct sev_device *sev) +{ +if (sev_es_tmr) +return ; + +/* Obtain the TMR memory area for SEV-ES use */ + sev_es_tmr =sev_fw_alloc (sev_es_tmr_size); +if (sev_es_tmr) { +/* Must flush the cache before giving it to the firmware */ +if (!sev->snp_initialized) +clflush_cache_range (sev_es_tmr, sev_es_tmr_size); + }else { +dev_warn (sev->dev,"SEV: TMR allocation failed, SEV-ES support unavailable\n" ); + } +} +static int __sev_platform_init_locked (int *error) {int rc, psp_ret =SEV_RET_NO_FW_CALL ;
@@ -772,18 +788,7 @@ static int __sev_platform_init_locked(int *error)if (sev->state ==SEV_STATE_INIT )return 0 ; -if (!sev_es_tmr) { -/* Obtain the TMR memory area for SEV-ES use */ - sev_es_tmr =sev_fw_alloc (sev_es_tmr_size); -if (sev_es_tmr) { -/* Must flush the cache before giving it to the firmware */ -if (!sev->snp_initialized) -clflush_cache_range (sev_es_tmr, sev_es_tmr_size); - }else { -dev_warn (sev->dev, -"SEV: TMR allocation failed, SEV-ES support unavailable\n" ); - } - } +__sev_platform_init_handle_tmr (sev);if (sev_init_ex_buffer) { rc =sev_read_init_ex_file ();
Signature · 4 lines
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette[tip: x86/sev] crypto: ccp: Handle the legacy TMR allocation when SNP is enabled
In reply to Michael RothThe following commit has been merged into the x86/sev branch of tip:
The following commit has been merged into the x86/sev branch of tip:
Commit-ID: 24512afa4336a1c14de750238abe32759cfba4b0 Gitweb: https://git.kernel.org/tip/24512afa4336a1c14de750238abe32759cfba4b0 Author: Brijesh Singh <[email protected]> AuthorDate: Thu, 25 Jan 2024 22:11:16 -06:00 Committer: Borislav Petkov (AMD) <[email protected]> CommitterDate: Mon, 29 Jan 2024 20:34:18 +01:00
crypto: ccp: Handle the legacy TMR allocation when SNP is enabled
The behavior and requirement for the SEV-legacy command is altered when the SNP firmware is in the INIT state. See SEV-SNP firmware ABI specification for more details.
Allocate the Trusted Memory Region (TMR) as a 2MB-sized/aligned region when SNP is enabled to satisfy new requirements for SNP. Continue allocating a 1MB-sized region for !SNP configuration.
[ bp: Carve out TMR allocation into a helper. ]
--- drivers/crypto/ccp/sev-dev.c | 193 +++++++++++++++++++++++++++++----- include/linux/psp-sev.h | 9 ++- 2 files changed, 176 insertions(+), 26 deletions(-)
drivers/crypto/ccp/sev-dev.c +167 -26
@@ -30,6 +30,7 @@#include <asm/smp.h> #include <asm/cacheflush.h> #include <asm/e820/types.h> +#include <asm/sev.h> #include "psp-dev.h" #include "sev-dev.h"
@@ -73,9 +74,14 @@ static int psp_timeout;* The TMR is a 1MB area that must be 1MB aligned. Use the page allocator * to allocate the memory, which will return aligned memory for the specified * allocation order. +* +* When SEV-SNP is enabled the TMR needs to be 2MB aligned and 2MB sized. */ -#define SEV_ES_TMR_SIZE (1024 * 1024) +#define SEV_TMR_SIZE (1024 * 1024) +#define SNP_TMR_SIZE (2 * 1024 * 1024) +static void *sev_es_tmr; +static size_t sev_es_tmr_size =SEV_TMR_SIZE ;/* INIT_EX NV Storage: * The NV Storage is a 32Kb area and must be 4Kb page aligned. Use the page
@@ -192,17 +198,6 @@ static int sev_cmd_buffer_len(int cmd)return 0 ; } -static void *sev_fw_alloc (unsigned long len) -{ -struct page *page; - - page =alloc_pages (GFP_KERNEL ,get_order (len)); -if (!page) -return NULL ; - -return page_address (page); -} -static struct file *open_file_as_root (const char *filename,int flags,umode_t mode) {struct file *fp;
@@ -333,6 +328,142 @@ static int sev_write_init_ex_file_if_required(int cmd_id)return sev_write_init_ex_file (); } +/* +* snp_reclaim_pages() needs __sev_do_cmd_locked(), and __sev_do_cmd_locked() +* needs snp_reclaim_pages(), so a forward declaration is needed. +*/ +static int __sev_do_cmd_locked (int cmd,void *data,int *psp_ret); + +static int snp_reclaim_pages (unsigned long paddr,unsigned int npages,bool locked) +{ +int ret, err, i; + + paddr =__sme_clr (ALIGN_DOWN (paddr,PAGE_SIZE )); + +for (i =0 ; i < npages; i++, paddr +=PAGE_SIZE ) { +struct sev_data_snp_page_reclaim data = {0 }; + + data.paddr = paddr; + +if (locked) + ret =__sev_do_cmd_locked (SEV_CMD_SNP_PAGE_RECLAIM , &data, &err); +else + ret =sev_do_cmd (SEV_CMD_SNP_PAGE_RECLAIM , &data, &err); + +if (ret) + goto cleanup; + + ret =rmp_make_shared (__phys_to_pfn (paddr),PG_LEVEL_4K ); +if (ret) + goto cleanup; + } + +return 0 ; + +cleanup: +/* +* If there was a failure reclaiming the page then it is no longer safe +* to release it back to the system; leak it instead. +*/ +snp_leak_pages (__phys_to_pfn (paddr), npages - i); +return ret; +} + +static int rmp_mark_pages_firmware (unsigned long paddr,unsigned int npages,bool locked) +{ +unsigned long pfn =__sme_clr (paddr) >>PAGE_SHIFT ; +int rc, i; + +for (i =0 ; i < npages; i++, pfn++) { + rc =rmp_make_private (pfn,0 ,PG_LEVEL_4K ,0 , true); +if (rc) + goto cleanup; + } + +return 0 ; + +cleanup: +/* +* Try unrolling the firmware state changes by +* reclaiming the pages which were already changed to the +* firmware state. +*/ +snp_reclaim_pages (paddr, i, locked); + +return rc; +} + +static struct page *__snp_alloc_firmware_pages (gfp_t gfp_mask,int order) +{ +unsigned long npages =1ul << order, paddr; +struct sev_device *sev; +struct page *page; + +if (!psp_master || !psp_master->sev_data) +return NULL ; + + page =alloc_pages (gfp_mask, order); +if (!page) +return NULL ; + +/* If SEV-SNP is initialized then add the page in RMP table. */ + sev = psp_master->sev_data; +if (!sev->snp_initialized) +return page; + + paddr =__pa ((unsigned long )page_address (page)); +if (rmp_mark_pages_firmware (paddr, npages, false)) +return NULL ; + +return page; +} + +void *snp_alloc_firmware_page (gfp_t gfp_mask) +{ +struct page *page; + + page =__snp_alloc_firmware_pages (gfp_mask,0 ); + +return page ?page_address (page) :NULL ; +} +EXPORT_SYMBOL_GPL (snp_alloc_firmware_page); + +static void __snp_free_firmware_pages (struct page *page,int order,bool locked) +{ +struct sev_device *sev = psp_master->sev_data; +unsigned long paddr, npages =1ul << order; + +if (!page) +return ; + + paddr =__pa ((unsigned long )page_address (page)); +if (sev->snp_initialized && +snp_reclaim_pages (paddr, npages, locked)) +return ; + +__free_pages (page, order); +} + +void snp_free_firmware_page (void *addr) +{ +if (!addr) +return ; + +__snp_free_firmware_pages (virt_to_page (addr),0 , false); +} +EXPORT_SYMBOL_GPL (snp_free_firmware_page); + +static void *sev_fw_alloc (unsigned long len) +{ +struct page *page; + + page =__snp_alloc_firmware_pages (GFP_KERNEL ,get_order (len)); +if (!page) +return NULL ; + +return page_address (page); +} +static int __sev_do_cmd_locked (int cmd,void *data,int *psp_ret) {struct psp_device *psp = psp_master;
@@ -456,7 +587,7 @@ static int __sev_init_locked(int *error) data.tmr_address =__pa (sev_es_tmr); data.flags |=SEV_INIT_FLAGS_SEV_ES ; - data.tmr_len =SEV_ES_TMR_SIZE ; + data.tmr_len = sev_es_tmr_size; }return __sev_do_cmd_locked (SEV_CMD_INIT , &data, error);
@@ -479,7 +610,7 @@ static int __sev_init_ex_locked(int *error) data.tmr_address =__pa (sev_es_tmr); data.flags |=SEV_INIT_FLAGS_SEV_ES ; - data.tmr_len =SEV_ES_TMR_SIZE ; + data.tmr_len = sev_es_tmr_size; }return __sev_do_cmd_locked (SEV_CMD_INIT_EX , &data, error);
@@ -623,9 +754,27 @@ static int __sev_snp_init_locked(int *error) sev->snp_initialized = true;dev_dbg (sev->dev,"SEV-SNP firmware initialized\n" ); + sev_es_tmr_size =SNP_TMR_SIZE ; +return rc; } +static void __sev_platform_init_handle_tmr (struct sev_device *sev) +{ +if (sev_es_tmr) +return ; + +/* Obtain the TMR memory area for SEV-ES use */ + sev_es_tmr =sev_fw_alloc (sev_es_tmr_size); +if (sev_es_tmr) { +/* Must flush the cache before giving it to the firmware */ +if (!sev->snp_initialized) +clflush_cache_range (sev_es_tmr, sev_es_tmr_size); + }else { +dev_warn (sev->dev,"SEV: TMR allocation failed, SEV-ES support unavailable\n" ); + } +} +static int __sev_platform_init_locked (int *error) {int rc, psp_ret =SEV_RET_NO_FW_CALL ;
@@ -639,16 +788,7 @@ static int __sev_platform_init_locked(int *error)if (sev->state ==SEV_STATE_INIT )return 0 ; -if (!sev_es_tmr) { -/* Obtain the TMR memory area for SEV-ES use */ - sev_es_tmr =sev_fw_alloc (SEV_ES_TMR_SIZE ); -if (sev_es_tmr) -/* Must flush the cache before giving it to the firmware */ -clflush_cache_range (sev_es_tmr,SEV_ES_TMR_SIZE ); -else -dev_warn (sev->dev, -"SEV: TMR allocation failed, SEV-ES support unavailable\n" ); - } +__sev_platform_init_handle_tmr (sev);if (sev_init_ex_buffer) { rc =sev_read_init_ex_file ();
@@ -1546,8 +1686,9 @@ static void sev_firmware_shutdown(struct sev_device *sev)/* The TMR area was encrypted, flush it from the cache */ wbinvd_on_all_cpus (); -free_pages ((unsigned long )sev_es_tmr, -get_order (SEV_ES_TMR_SIZE )); +__snp_free_firmware_pages (virt_to_page (sev_es_tmr), +get_order (sev_es_tmr_size), + false); sev_es_tmr =NULL ; }
include/linux/psp-sev.h +9 -0
@@ -932,6 +932,8 @@ int sev_guest_decommission(struct sev_data_decommission *data, int *error);int sev_do_cmd (int cmd,void *data,int *psp_ret);void *psp_copy_user_blob (u64 uaddr,u32 len); +void *snp_alloc_firmware_page (gfp_t mask); +void snp_free_firmware_page (void *addr);#else /* !CONFIG_CRYPTO_DEV_SP_PSP */
@@ -959,6 +961,13 @@ sev_issue_cmd_external_user(struct file *filep, unsigned int id, void *data, intstatic inline void *psp_copy_user_blob (u64 __user uaddr,u32 len) {return ERR_PTR (-EINVAL ); } +static inline void *snp_alloc_firmware_page (gfp_t mask) +{ +return NULL ; +} + +static inline void snp_free_firmware_page (void *addr) { } +#endif /* CONFIG_CRYPTO_DEV_SP_PSP */ #endif /* __PSP_SEV_H__ */
PATCH v2 17/25 crypto: ccp: Handle non-volatile INIT_EX data when SNP is enabled
In reply to Michael RothFrom: Tom Lendacky <[email protected]>
For SEV/SEV-ES, a buffer can be used to access non-volatile data so it can be initialized from a file specified by the init_ex_path CCP module parameter instead of relying on the SPI bus for NV storage, and afterward the buffer can be read from to sync new data back to the file.
When SNP is enabled, the pages comprising this buffer need to be set to firmware-owned in the RMP table before they can be accessed by firmware for subsequent updates to the initial contents.
Implement that handling here.
--- drivers/crypto/ccp/sev-dev.c | 47 ++++++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 15 deletions(-)
drivers/crypto/ccp/sev-dev.c +32 -15
@@ -785,10 +785,38 @@ static int __sev_platform_init_locked(int *error) } } -if (sev_init_ex_buffer) { +/* +* If an init_ex_path is provided allocate a buffer for the file and +* read in the contents. Additionally, if SNP is initialized, convert +* the buffer pages to firmware pages. +*/ +if (init_ex_path && !sev_init_ex_buffer) { +struct page *page; + + page =alloc_pages (GFP_KERNEL ,get_order (NV_LENGTH )); +if (!page) { +dev_err (sev->dev,"SEV: INIT_EX NV memory allocation failed\n" ); +return -ENOMEM ; + } + + sev_init_ex_buffer =page_address (page); + rc =sev_read_init_ex_file ();if (rc)return rc; + +/* If SEV-SNP is initialized, transition to firmware page. */ +if (sev->snp_initialized) { +unsigned long npages; + + npages =1UL <<get_order (NV_LENGTH ); +if (rmp_mark_pages_firmware (__pa (sev_init_ex_buffer), + npages, false)) { +dev_err (sev->dev, +"SEV: INIT_EX NV memory page state change failed.\n" ); +return -ENOMEM ; + } + } } rc =__sev_do_init_locked (&psp_ret);
@@ -1688,8 +1716,9 @@ static void sev_firmware_shutdown(struct sev_device *sev) }if (sev_init_ex_buffer) { -free_pages ((unsigned long )sev_init_ex_buffer, -get_order (NV_LENGTH )); +__snp_free_firmware_pages (virt_to_page (sev_init_ex_buffer), +get_order (NV_LENGTH ), + true); sev_init_ex_buffer =NULL ; }
@@ -1743,18 +1772,6 @@ void sev_pci_init(void)if (sev_update_firmware (sev->dev) ==0 )sev_get_api_version (); -/* If an init_ex_path is provided rely on INIT_EX for PSP initialization -* instead of INIT. -*/ -if (init_ex_path) { - sev_init_ex_buffer =sev_fw_alloc (NV_LENGTH ); -if (!sev_init_ex_buffer) { -dev_err (sev->dev, -"SEV: INIT_EX NV memory allocation failed\n" ); - goto err; - } - } -/* Initialize the platform */ args.probe = true; rc =sev_platform_init (&args);
Signature · 1 line
2.25.1
Borislav Petkov
maintainerRe: [PATCH v2 17/25] crypto: ccp: Handle non-volatile INIT_EX data when SNP is enabled
Quoted 24 lines · Michael Roth
(quoted diff) +20
-1- if (sev_init_ex_buffer) { + /* + * If an init_ex_path is provided allocate a buffer for the file and + * read in the contents. Additionally, if SNP is initialized, convert + * the buffer pages to firmware pages. + */ + if (init_ex_path && !sev_init_ex_buffer) { + struct page *page; + + page = alloc_pages(GFP_KERNEL, get_order(NV_LENGTH)); + if (!page) { + dev_err(sev->dev, "SEV: INIT_EX NV memory allocation failed\n"); + return -ENOMEM; + } + + sev_init_ex_buffer = page_address(page); + rc = sev_read_init_ex_file(); if (rc) return rc; + + /* If SEV-SNP is initialized, transition to firmware page. */ + if (sev->snp_initialized) { + unsigned long npages;
Review
Quoted 10 lines
(quoted diff) +9
-0+ + npages = 1UL << get_order(NV_LENGTH); + if (rmp_mark_pages_firmware(__pa(sev_init_ex_buffer), + npages, false)) { + dev_err(sev->dev, + "SEV: INIT_EX NV memory page state change failed.\n"); + return -ENOMEM; + } + } }
Ontop:
---
drivers/crypto/ccp/sev-dev.c +45 -33
@@ -775,6 +775,48 @@ static void __sev_platform_init_handle_tmr(struct sev_device *sev) } } +/* +* If an init_ex_path is provided allocate a buffer for the file and +* read in the contents. Additionally, if SNP is initialized, convert +* the buffer pages to firmware pages. +*/ +static int __sev_platform_init_handle_init_ex_path (struct sev_device *sev) +{ +struct page *page; +int rc; + +if (!init_ex_path) +return 0 ; + +if (sev_init_ex_buffer) +return 0 ; + + page =alloc_pages (GFP_KERNEL ,get_order (NV_LENGTH )); +if (!page) { +dev_err (sev->dev,"SEV: INIT_EX NV memory allocation failed\n" ); +return -ENOMEM ; + } + + sev_init_ex_buffer =page_address (page); + + rc =sev_read_init_ex_file (); +if (rc) +return rc; + +/* If SEV-SNP is initialized, transition to firmware page. */ +if (sev->snp_initialized) { +unsigned long npages; + + npages =1UL <<get_order (NV_LENGTH ); +if (rmp_mark_pages_firmware (__pa (sev_init_ex_buffer), npages, false)) { +dev_err (sev->dev,"SEV: INIT_EX NV memory page state change failed.\n" ); +return -ENOMEM ; + } + } + +return 0 ; +} +static int __sev_platform_init_locked (int *error) {int rc, psp_ret =SEV_RET_NO_FW_CALL ;
@@ -790,39 +832,9 @@ static int __sev_platform_init_locked(int *error)__sev_platform_init_handle_tmr (sev); -/* -* If an init_ex_path is provided allocate a buffer for the file and -* read in the contents. Additionally, if SNP is initialized, convert -* the buffer pages to firmware pages. -*/ -if (init_ex_path && !sev_init_ex_buffer) { -struct page *page; - - page =alloc_pages (GFP_KERNEL ,get_order (NV_LENGTH )); -if (!page) { -dev_err (sev->dev,"SEV: INIT_EX NV memory allocation failed\n" ); -return -ENOMEM ; - } - - sev_init_ex_buffer =page_address (page); - - rc =sev_read_init_ex_file (); -if (rc) -return rc; - -/* If SEV-SNP is initialized, transition to firmware page. */ -if (sev->snp_initialized) { -unsigned long npages; - - npages =1UL <<get_order (NV_LENGTH ); -if (rmp_mark_pages_firmware (__pa (sev_init_ex_buffer), - npages, false)) { -dev_err (sev->dev, -"SEV: INIT_EX NV memory page state change failed.\n" ); -return -ENOMEM ; - } - } - } + rc =__sev_platform_init_handle_init_ex_path (sev); +if (rc) +return rc; rc =__sev_do_init_locked (&psp_ret);if (rc && psp_ret ==SEV_RET_SECURE_DATA_INVALID ) {
Signature · 4 lines
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette[tip: x86/sev] crypto: ccp: Handle non-volatile INIT_EX data when SNP is enabled
In reply to Michael RothThe following commit has been merged into the x86/sev branch of tip:
The following commit has been merged into the x86/sev branch of tip:
Commit-ID: 7364a6fbca45f826952ea932699fb2171d06ee73 Gitweb: https://git.kernel.org/tip/7364a6fbca45f826952ea932699fb2171d06ee73 Author: Tom Lendacky <[email protected]> AuthorDate: Thu, 25 Jan 2024 22:11:17 -06:00 Committer: Borislav Petkov (AMD) <[email protected]> CommitterDate: Mon, 29 Jan 2024 20:34:18 +01:00
crypto: ccp: Handle non-volatile INIT_EX data when SNP is enabled
For SEV/SEV-ES, a buffer can be used to access non-volatile data so it can be initialized from a file specified by the init_ex_path CCP module parameter instead of relying on the SPI bus for NV storage, and afterward the buffer can be read from to sync new data back to the file.
When SNP is enabled, the pages comprising this buffer need to be set to firmware-owned in the RMP table before they can be accessed by firmware for subsequent updates to the initial contents.
Implement that handling here.
[ bp: Carve out allocation into a helper. ]
--- drivers/crypto/ccp/sev-dev.c | 67 +++++++++++++++++++++++++---------- 1 file changed, 48 insertions(+), 19 deletions(-)
drivers/crypto/ccp/sev-dev.c +48 -19
@@ -775,6 +775,48 @@ static void __sev_platform_init_handle_tmr(struct sev_device *sev) } } +/* +* If an init_ex_path is provided allocate a buffer for the file and +* read in the contents. Additionally, if SNP is initialized, convert +* the buffer pages to firmware pages. +*/ +static int __sev_platform_init_handle_init_ex_path (struct sev_device *sev) +{ +struct page *page; +int rc; + +if (!init_ex_path) +return 0 ; + +if (sev_init_ex_buffer) +return 0 ; + + page =alloc_pages (GFP_KERNEL ,get_order (NV_LENGTH )); +if (!page) { +dev_err (sev->dev,"SEV: INIT_EX NV memory allocation failed\n" ); +return -ENOMEM ; + } + + sev_init_ex_buffer =page_address (page); + + rc =sev_read_init_ex_file (); +if (rc) +return rc; + +/* If SEV-SNP is initialized, transition to firmware page. */ +if (sev->snp_initialized) { +unsigned long npages; + + npages =1UL <<get_order (NV_LENGTH ); +if (rmp_mark_pages_firmware (__pa (sev_init_ex_buffer), npages, false)) { +dev_err (sev->dev,"SEV: INIT_EX NV memory page state change failed.\n" ); +return -ENOMEM ; + } + } + +return 0 ; +} +static int __sev_platform_init_locked (int *error) {int rc, psp_ret =SEV_RET_NO_FW_CALL ;
@@ -790,11 +832,9 @@ static int __sev_platform_init_locked(int *error)__sev_platform_init_handle_tmr (sev); -if (sev_init_ex_buffer) { - rc =sev_read_init_ex_file (); -if (rc) -return rc; - } + rc =__sev_platform_init_handle_init_ex_path (sev); +if (rc) +return rc; rc =__sev_do_init_locked (&psp_ret);if (rc && psp_ret ==SEV_RET_SECURE_DATA_INVALID ) {
@@ -1693,8 +1733,9 @@ static void sev_firmware_shutdown(struct sev_device *sev) }if (sev_init_ex_buffer) { -free_pages ((unsigned long )sev_init_ex_buffer, -get_order (NV_LENGTH )); +__snp_free_firmware_pages (virt_to_page (sev_init_ex_buffer), +get_order (NV_LENGTH ), + true); sev_init_ex_buffer =NULL ; }
@@ -1748,18 +1789,6 @@ void sev_pci_init(void)if (sev_update_firmware (sev->dev) ==0 )sev_get_api_version (); -/* If an init_ex_path is provided rely on INIT_EX for PSP initialization -* instead of INIT. -*/ -if (init_ex_path) { - sev_init_ex_buffer =sev_fw_alloc (NV_LENGTH ); -if (!sev_init_ex_buffer) { -dev_err (sev->dev, -"SEV: INIT_EX NV memory allocation failed\n" ); - goto err; - } - } -/* Initialize the platform */ args.probe = true; rc =sev_platform_init (&args);
PATCH v2 18/25 crypto: ccp: Handle legacy SEV commands when SNP is enabled
In reply to Michael RothFrom: Brijesh Singh <[email protected]>
The behavior of legacy SEV commands is altered when the firmware is initialized for SNP support. In that case, all command buffer memory that may get written to by legacy SEV commands must be marked as firmware-owned in the RMP table prior to issuing the command.
Additionally, when a command buffer contains a system physical address that points to additional buffers that firmware may write to, special handling is needed depending on whether:
1) the system physical address points to guest memory
2) the system physical address points to host memory
To handle case #1, the pages of these buffers are changed to firmware-owned in the RMP table before issuing the command, and restored to hypervisor-owned after the command completes.
For case #2, a bounce buffer is used instead of the original address.
--- drivers/crypto/ccp/sev-dev.c | 418 ++++++++++++++++++++++++++++++++++- drivers/crypto/ccp/sev-dev.h | 3 + 2 files changed, 411 insertions(+), 10 deletions(-)
drivers/crypto/ccp/sev-dev.c +408 -10
@@ -43,6 +43,15 @@#define SNP_MIN_API_MAJOR 1#define SNP_MIN_API_MINOR 51 +/* +* Maximum number of firmware-writable buffers that might be specified +* in the parameters of a legacy SEV command buffer. +*/ +#define CMD_BUF_FW_WRITABLE_MAX 2 + +/* Leave room in the descriptor array for an end-of-list indicator. */ +#define CMD_BUF_DESC_MAX (CMD_BUF_FW_WRITABLE_MAX + 1) +static DEFINE_MUTEX (sev_cmd_mutex);static struct sev_misc_dev *misc_dev;
@@ -464,13 +473,344 @@ static void *sev_fw_alloc(unsigned long len)return page_address (page); } +/** +* struct cmd_buf_desc - descriptors for managing legacy SEV command address +* parameters corresponding to buffers that may be written to by firmware. +* +* @paddr_ptr: pointer the address parameter in the command buffer, which may +* need to be saved/restored depending on whether a bounce buffer is +* used. Must be NULL if this descriptor is only an end-of-list +* indicator. +* @paddr_orig: storage for the original address parameter, which can be used to +* restore the original value in @paddr_ptr in cases where it is +* replaced with the address of a bounce buffer. +* @len: length of buffer located at the address originally stored at @paddr_ptr +* @guest_owned: true if the address corresponds to guest-owned pages, in which +* case bounce buffers are not needed. +*/ +struct cmd_buf_desc { +u64 *paddr_ptr; +u64 paddr_orig; +u32 len; +bool guest_owned; +}; + +/* +* If a legacy SEV command parameter is a memory address, those pages in +* turn need to be transitioned to/from firmware-owned before/after +* executing the firmware command. +* +* Additionally, in cases where those pages are not guest-owned, a bounce +* buffer is needed in place of the original memory address parameter. +* +* A set of descriptors are used to keep track of this handling, and +* initialized here based on the specific commands being executed. +*/ +static void snp_populate_cmd_buf_desc_list (int cmd,void *cmd_buf, +struct cmd_buf_desc *desc_list) +{ +switch (cmd) { +case SEV_CMD_PDH_CERT_EXPORT : { +struct sev_data_pdh_cert_export *data = cmd_buf; + + desc_list[0 ].paddr_ptr = &data->pdh_cert_address; + desc_list[0 ].len = data->pdh_cert_len; + desc_list[1 ].paddr_ptr = &data->cert_chain_address; + desc_list[1 ].len = data->cert_chain_len; +break ; + } +case SEV_CMD_GET_ID : { +struct sev_data_get_id *data = cmd_buf; + + desc_list[0 ].paddr_ptr = &data->address; + desc_list[0 ].len = data->len; +break ; + } +case SEV_CMD_PEK_CSR : { +struct sev_data_pek_csr *data = cmd_buf; + + desc_list[0 ].paddr_ptr = &data->address; + desc_list[0 ].len = data->len; +break ; + } +case SEV_CMD_LAUNCH_UPDATE_DATA : { +struct sev_data_launch_update_data *data = cmd_buf; + + desc_list[0 ].paddr_ptr = &data->address; + desc_list[0 ].len = data->len; + desc_list[0 ].guest_owned = true; +break ; + } +case SEV_CMD_LAUNCH_UPDATE_VMSA : { +struct sev_data_launch_update_vmsa *data = cmd_buf; + + desc_list[0 ].paddr_ptr = &data->address; + desc_list[0 ].len = data->len; + desc_list[0 ].guest_owned = true; +break ; + } +case SEV_CMD_LAUNCH_MEASURE : { +struct sev_data_launch_measure *data = cmd_buf; + + desc_list[0 ].paddr_ptr = &data->address; + desc_list[0 ].len = data->len; +break ; + } +case SEV_CMD_LAUNCH_UPDATE_SECRET : { +struct sev_data_launch_secret *data = cmd_buf; + + desc_list[0 ].paddr_ptr = &data->guest_address; + desc_list[0 ].len = data->guest_len; + desc_list[0 ].guest_owned = true; +break ; + } +case SEV_CMD_DBG_DECRYPT : { +struct sev_data_dbg *data = cmd_buf; + + desc_list[0 ].paddr_ptr = &data->dst_addr; + desc_list[0 ].len = data->len; + desc_list[0 ].guest_owned = true; +break ; + } +case SEV_CMD_DBG_ENCRYPT : { +struct sev_data_dbg *data = cmd_buf; + + desc_list[0 ].paddr_ptr = &data->dst_addr; + desc_list[0 ].len = data->len; + desc_list[0 ].guest_owned = true; +break ; + } +case SEV_CMD_ATTESTATION_REPORT : { +struct sev_data_attestation_report *data = cmd_buf; + + desc_list[0 ].paddr_ptr = &data->address; + desc_list[0 ].len = data->len; +break ; + } +case SEV_CMD_SEND_START : { +struct sev_data_send_start *data = cmd_buf; + + desc_list[0 ].paddr_ptr = &data->session_address; + desc_list[0 ].len = data->session_len; +break ; + } +case SEV_CMD_SEND_UPDATE_DATA : { +struct sev_data_send_update_data *data = cmd_buf; + + desc_list[0 ].paddr_ptr = &data->hdr_address; + desc_list[0 ].len = data->hdr_len; + desc_list[1 ].paddr_ptr = &data->trans_address; + desc_list[1 ].len = data->trans_len; +break ; + } +case SEV_CMD_SEND_UPDATE_VMSA : { +struct sev_data_send_update_vmsa *data = cmd_buf; + + desc_list[0 ].paddr_ptr = &data->hdr_address; + desc_list[0 ].len = data->hdr_len; + desc_list[1 ].paddr_ptr = &data->trans_address; + desc_list[1 ].len = data->trans_len; +break ; + } +case SEV_CMD_RECEIVE_UPDATE_DATA : { +struct sev_data_receive_update_data *data = cmd_buf; + + desc_list[0 ].paddr_ptr = &data->guest_address; + desc_list[0 ].len = data->guest_len; + desc_list[0 ].guest_owned = true; +break ; + } +case SEV_CMD_RECEIVE_UPDATE_VMSA : { +struct sev_data_receive_update_vmsa *data = cmd_buf; + + desc_list[0 ].paddr_ptr = &data->guest_address; + desc_list[0 ].len = data->guest_len; + desc_list[0 ].guest_owned = true; +break ; + } +default : +break ; + } +} + +static int snp_map_cmd_buf_desc (struct cmd_buf_desc *desc) +{ +unsigned int npages; + +if (!desc->len) +return 0 ; + +/* Allocate a bounce buffer if this isn't a guest owned page. */ +if (!desc->guest_owned) { +struct page *page; + + page =alloc_pages (GFP_KERNEL_ACCOUNT ,get_order (desc->len)); +if (!page) { +pr_warn ("Failed to allocate bounce buffer for SEV legacy command.\n" ); +return -ENOMEM ; + } + + desc->paddr_orig = *desc->paddr_ptr; + *desc->paddr_ptr =__psp_pa (page_to_virt (page)); + } + + npages =PAGE_ALIGN (desc->len) >>PAGE_SHIFT ; + +/* Transition the buffer to firmware-owned. */ +if (rmp_mark_pages_firmware (*desc->paddr_ptr, npages, true)) { +pr_warn ("Error moving pages to firmware-owned state for SEV legacy command.\n" ); +return -EFAULT ; + } + +return 0 ; +} + +static int snp_unmap_cmd_buf_desc (struct cmd_buf_desc *desc) +{ +unsigned int npages; + +if (!desc->len) +return 0 ; + + npages =PAGE_ALIGN (desc->len) >>PAGE_SHIFT ; + +/* Transition the buffers back to hypervisor-owned. */ +if (snp_reclaim_pages (*desc->paddr_ptr, npages, true)) { +pr_warn ("Failed to reclaim firmware-owned pages while issuing SEV legacy command.\n" ); +return -EFAULT ; + } + +/* Copy data from bounce buffer and then free it. */ +if (!desc->guest_owned) { +void *bounce_buf =__va (__sme_clr (*desc->paddr_ptr)); +void *dst_buf =__va (__sme_clr (desc->paddr_orig)); + +memcpy (dst_buf, bounce_buf, desc->len); +__free_pages (virt_to_page (bounce_buf),get_order (desc->len)); + +/* Restore the original address in the command buffer. */ + *desc->paddr_ptr = desc->paddr_orig; + } + +return 0 ; +} + +static int snp_map_cmd_buf_desc_list (int cmd,void *cmd_buf,struct cmd_buf_desc *desc_list) +{ +int i; + +snp_populate_cmd_buf_desc_list (cmd, cmd_buf, desc_list); + +for (i =0 ; i <CMD_BUF_DESC_MAX ; i++) { +struct cmd_buf_desc *desc = &desc_list[i]; + +if (!desc->paddr_ptr) +break ; + +if (snp_map_cmd_buf_desc (desc)) + goto err_unmap; + } + +return 0 ; + +err_unmap: +for (i--; i >=0 ; i--) +snp_unmap_cmd_buf_desc (&desc_list[i]); + +return -EFAULT ; +} + +static int snp_unmap_cmd_buf_desc_list (struct cmd_buf_desc *desc_list) +{ +int i, ret =0 ; + +for (i =0 ; i <CMD_BUF_DESC_MAX ; i++) { +struct cmd_buf_desc *desc = &desc_list[i]; + +if (!desc->paddr_ptr) +break ; + +if (snp_unmap_cmd_buf_desc (&desc_list[i])) + ret = -EFAULT ; + } + +return ret; +} + +static bool sev_cmd_buf_writable (int cmd) +{ +switch (cmd) { +case SEV_CMD_PLATFORM_STATUS : +case SEV_CMD_GUEST_STATUS : +case SEV_CMD_LAUNCH_START : +case SEV_CMD_RECEIVE_START : +case SEV_CMD_LAUNCH_MEASURE : +case SEV_CMD_SEND_START : +case SEV_CMD_SEND_UPDATE_DATA : +case SEV_CMD_SEND_UPDATE_VMSA : +case SEV_CMD_PEK_CSR : +case SEV_CMD_PDH_CERT_EXPORT : +case SEV_CMD_GET_ID : +case SEV_CMD_ATTESTATION_REPORT : +return true; +default : +return false; + } +} + +/* After SNP is INIT'ed, the behavior of legacy SEV commands is changed. */ +static bool snp_legacy_handling_needed (int cmd) +{ +struct sev_device *sev = psp_master->sev_data; + +return cmd <SEV_CMD_SNP_INIT && sev->snp_initialized; +} + +static int snp_prep_cmd_buf (int cmd,void *cmd_buf,struct cmd_buf_desc *desc_list) +{ +if (!snp_legacy_handling_needed (cmd)) +return 0 ; + +if (snp_map_cmd_buf_desc_list (cmd, cmd_buf, desc_list)) +return -EFAULT ; + +/* +* Before command execution, the command buffer needs to be put into +* the firmware-owned state. +*/ +if (sev_cmd_buf_writable (cmd)) { +if (rmp_mark_pages_firmware (__pa (cmd_buf),1 , true)) +return -EFAULT ; + } + +return 0 ; +} + +static int snp_reclaim_cmd_buf (int cmd,void *cmd_buf) +{ +if (!snp_legacy_handling_needed (cmd)) +return 0 ; + +/* +* After command completion, the command buffer needs to be put back +* into the hypervisor-owned state. +*/ +if (sev_cmd_buf_writable (cmd)) +if (snp_reclaim_pages (__pa (cmd_buf),1 , true)) +return -EFAULT ; + +return 0 ; +} +static int __sev_do_cmd_locked (int cmd,void *data,int *psp_ret) { +struct cmd_buf_desc desc_list[CMD_BUF_DESC_MAX ] = {0 };struct psp_device *psp = psp_master;struct sev_device *sev;unsigned int cmdbuff_hi, cmdbuff_lo;unsigned int phys_lsb, phys_msb;unsigned int reg, ret =0 ; +void *cmd_buf;int buf_len;if (!psp || !psp->sev_data)
@@ -490,12 +830,47 @@ static int __sev_do_cmd_locked(int cmd, void *data, int *psp_ret)* work for some memory, e.g. vmalloc'd addresses, and @data may not be * physically contiguous. */ -if (data) -memcpy (sev->cmd_buf, data, buf_len); +if (data) { +/* +* Commands are generally issued one at a time and require the +* sev_cmd_mutex, but there could be recursive firmware requests +* due to SEV_CMD_SNP_PAGE_RECLAIM needing to be issued while +* preparing buffers for another command. This is the only known +* case of nesting in the current code, so exactly one +* additional command buffer is available for that purpose. +*/ +if (!sev->cmd_buf_active) { + cmd_buf = sev->cmd_buf; + sev->cmd_buf_active = true; + }else if (!sev->cmd_buf_backup_active) { + cmd_buf = sev->cmd_buf_backup; + sev->cmd_buf_backup_active = true; + }else { +dev_err (sev->dev, +"SEV: too many firmware commands in progress, no command buffers available.\n" ); +return -EBUSY ; + } + +memcpy (cmd_buf, data, buf_len); + +/* +* The behavior of the SEV-legacy commands is altered when the +* SNP firmware is in the INIT state. +*/ + ret =snp_prep_cmd_buf (cmd, cmd_buf, desc_list); +if (ret) { +dev_err (sev->dev, +"SEV: failed to prepare buffer for legacy command 0x%x. Error: %d\n" , + cmd, ret); +return ret; + } + }else { + cmd_buf = sev->cmd_buf; + }/* Get the physical address of the command buffer */ - phys_lsb = data ?lower_32_bits (__psp_pa (sev->cmd_buf)) :0 ; - phys_msb = data ?upper_32_bits (__psp_pa (sev->cmd_buf)) :0 ; + phys_lsb = data ?lower_32_bits (__psp_pa (cmd_buf)) :0 ; + phys_msb = data ?upper_32_bits (__psp_pa (cmd_buf)) :0 ;dev_dbg (sev->dev,"sev command id %#x buffer 0x%08x%08x timeout %us\n" , cmd, phys_msb, phys_lsb, psp_timeout);
@@ -549,15 +924,36 @@ static int __sev_do_cmd_locked(int cmd, void *data, int *psp_ret) ret =sev_write_init_ex_file_if_required (cmd); } -print_hex_dump_debug ("(out): " ,DUMP_PREFIX_OFFSET ,16 ,2 , data, - buf_len, false); -/* * Copy potential output from the PSP back to data. Do this even on * failure in case the caller wants to glean something from the error. */ -if (data) -memcpy (data, sev->cmd_buf, buf_len); +if (data) { +int ret_reclaim; +/* +* Restore the page state after the command completes. +*/ + ret_reclaim =snp_reclaim_cmd_buf (cmd, cmd_buf); +if (ret_reclaim) { +dev_err (sev->dev, +"SEV: failed to reclaim buffer for legacy command %#x. Error: %d\n" , + cmd, ret_reclaim); +return ret_reclaim; + } + +memcpy (data, cmd_buf, buf_len); + +if (sev->cmd_buf_backup_active) + sev->cmd_buf_backup_active = false; +else + sev->cmd_buf_active = false; + +if (snp_unmap_cmd_buf_desc_list (desc_list)) +return -EFAULT ; + } + +print_hex_dump_debug ("(out): " ,DUMP_PREFIX_OFFSET ,16 ,2 , data, + buf_len, false);return ret; }
@@ -1657,10 +2053,12 @@ int sev_dev_init(struct psp_device *psp)if (!sev) goto e_err; - sev->cmd_buf = (void *)devm_get_free_pages (dev,GFP_KERNEL ,0 ); + sev->cmd_buf = (void *)devm_get_free_pages (dev,GFP_KERNEL ,1 );if (!sev->cmd_buf) goto e_sev; + sev->cmd_buf_backup = (uint8_t *)sev->cmd_buf +PAGE_SIZE ; + psp->sev_data = sev; sev->dev = dev;
drivers/crypto/ccp/sev-dev.h +3 -0
@@ -52,6 +52,9 @@ struct sev_device {u8 build;void *cmd_buf; +void *cmd_buf_backup; +bool cmd_buf_active; +bool cmd_buf_backup_active;bool snp_initialized; };
Signature · 1 line
2.25.1
[tip: x86/sev] crypto: ccp: Handle legacy SEV commands when SNP is enabled
The following commit has been merged into the x86/sev branch of tip:
The following commit has been merged into the x86/sev branch of tip:
Commit-ID: a867ad6b340f47b7333b80a54b8507fc2cd80aa4 Gitweb: https://git.kernel.org/tip/a867ad6b340f47b7333b80a54b8507fc2cd80aa4 Author: Brijesh Singh <[email protected]> AuthorDate: Thu, 25 Jan 2024 22:11:18 -06:00 Committer: Borislav Petkov (AMD) <[email protected]> CommitterDate: Mon, 29 Jan 2024 20:34:18 +01:00
crypto: ccp: Handle legacy SEV commands when SNP is enabled
The behavior of legacy SEV commands is altered when the firmware is initialized for SNP support. In that case, all command buffer memory that may get written to by legacy SEV commands must be marked as firmware-owned in the RMP table prior to issuing the command.
Additionally, when a command buffer contains a system physical address that points to additional buffers that firmware may write to, special handling is needed depending on whether:
1) the system physical address points to guest memory
2) the system physical address points to host memory
To handle case #1, the pages of these buffers are changed to firmware-owned in the RMP table before issuing the command, and restored to hypervisor-owned after the command completes.
For case #2, a bounce buffer is used instead of the original address.
--- drivers/crypto/ccp/sev-dev.c | 423 +++++++++++++++++++++++++++++++++- drivers/crypto/ccp/sev-dev.h | 3 +- 2 files changed, 416 insertions(+), 10 deletions(-)
drivers/crypto/ccp/sev-dev.c +413 -10
@@ -43,6 +43,15 @@#define SNP_MIN_API_MAJOR 1#define SNP_MIN_API_MINOR 51 +/* +* Maximum number of firmware-writable buffers that might be specified +* in the parameters of a legacy SEV command buffer. +*/ +#define CMD_BUF_FW_WRITABLE_MAX 2 + +/* Leave room in the descriptor array for an end-of-list indicator. */ +#define CMD_BUF_DESC_MAX (CMD_BUF_FW_WRITABLE_MAX + 1) +static DEFINE_MUTEX (sev_cmd_mutex);static struct sev_misc_dev *misc_dev;
@@ -464,13 +473,349 @@ static void *sev_fw_alloc(unsigned long len)return page_address (page); } +/** +* struct cmd_buf_desc - descriptors for managing legacy SEV command address +* parameters corresponding to buffers that may be written to by firmware. +* +* @paddr_ptr: pointer to the address parameter in the command buffer which may +* need to be saved/restored depending on whether a bounce buffer +* is used. In the case of a bounce buffer, the command buffer +* needs to be updated with the address of the new bounce buffer +* snp_map_cmd_buf_desc() has allocated specifically for it. Must +* be NULL if this descriptor is only an end-of-list indicator. +* +* @paddr_orig: storage for the original address parameter, which can be used to +* restore the original value in @paddr_ptr in cases where it is +* replaced with the address of a bounce buffer. +* +* @len: length of buffer located at the address originally stored at @paddr_ptr +* +* @guest_owned: true if the address corresponds to guest-owned pages, in which +* case bounce buffers are not needed. +*/ +struct cmd_buf_desc { +u64 *paddr_ptr; +u64 paddr_orig; +u32 len; +bool guest_owned; +}; + +/* +* If a legacy SEV command parameter is a memory address, those pages in +* turn need to be transitioned to/from firmware-owned before/after +* executing the firmware command. +* +* Additionally, in cases where those pages are not guest-owned, a bounce +* buffer is needed in place of the original memory address parameter. +* +* A set of descriptors are used to keep track of this handling, and +* initialized here based on the specific commands being executed. +*/ +static void snp_populate_cmd_buf_desc_list (int cmd,void *cmd_buf, +struct cmd_buf_desc *desc_list) +{ +switch (cmd) { +case SEV_CMD_PDH_CERT_EXPORT : { +struct sev_data_pdh_cert_export *data = cmd_buf; + + desc_list[0 ].paddr_ptr = &data->pdh_cert_address; + desc_list[0 ].len = data->pdh_cert_len; + desc_list[1 ].paddr_ptr = &data->cert_chain_address; + desc_list[1 ].len = data->cert_chain_len; +break ; + } +case SEV_CMD_GET_ID : { +struct sev_data_get_id *data = cmd_buf; + + desc_list[0 ].paddr_ptr = &data->address; + desc_list[0 ].len = data->len; +break ; + } +case SEV_CMD_PEK_CSR : { +struct sev_data_pek_csr *data = cmd_buf; + + desc_list[0 ].paddr_ptr = &data->address; + desc_list[0 ].len = data->len; +break ; + } +case SEV_CMD_LAUNCH_UPDATE_DATA : { +struct sev_data_launch_update_data *data = cmd_buf; + + desc_list[0 ].paddr_ptr = &data->address; + desc_list[0 ].len = data->len; + desc_list[0 ].guest_owned = true; +break ; + } +case SEV_CMD_LAUNCH_UPDATE_VMSA : { +struct sev_data_launch_update_vmsa *data = cmd_buf; + + desc_list[0 ].paddr_ptr = &data->address; + desc_list[0 ].len = data->len; + desc_list[0 ].guest_owned = true; +break ; + } +case SEV_CMD_LAUNCH_MEASURE : { +struct sev_data_launch_measure *data = cmd_buf; + + desc_list[0 ].paddr_ptr = &data->address; + desc_list[0 ].len = data->len; +break ; + } +case SEV_CMD_LAUNCH_UPDATE_SECRET : { +struct sev_data_launch_secret *data = cmd_buf; + + desc_list[0 ].paddr_ptr = &data->guest_address; + desc_list[0 ].len = data->guest_len; + desc_list[0 ].guest_owned = true; +break ; + } +case SEV_CMD_DBG_DECRYPT : { +struct sev_data_dbg *data = cmd_buf; + + desc_list[0 ].paddr_ptr = &data->dst_addr; + desc_list[0 ].len = data->len; + desc_list[0 ].guest_owned = true; +break ; + } +case SEV_CMD_DBG_ENCRYPT : { +struct sev_data_dbg *data = cmd_buf; + + desc_list[0 ].paddr_ptr = &data->dst_addr; + desc_list[0 ].len = data->len; + desc_list[0 ].guest_owned = true; +break ; + } +case SEV_CMD_ATTESTATION_REPORT : { +struct sev_data_attestation_report *data = cmd_buf; + + desc_list[0 ].paddr_ptr = &data->address; + desc_list[0 ].len = data->len; +break ; + } +case SEV_CMD_SEND_START : { +struct sev_data_send_start *data = cmd_buf; + + desc_list[0 ].paddr_ptr = &data->session_address; + desc_list[0 ].len = data->session_len; +break ; + } +case SEV_CMD_SEND_UPDATE_DATA : { +struct sev_data_send_update_data *data = cmd_buf; + + desc_list[0 ].paddr_ptr = &data->hdr_address; + desc_list[0 ].len = data->hdr_len; + desc_list[1 ].paddr_ptr = &data->trans_address; + desc_list[1 ].len = data->trans_len; +break ; + } +case SEV_CMD_SEND_UPDATE_VMSA : { +struct sev_data_send_update_vmsa *data = cmd_buf; + + desc_list[0 ].paddr_ptr = &data->hdr_address; + desc_list[0 ].len = data->hdr_len; + desc_list[1 ].paddr_ptr = &data->trans_address; + desc_list[1 ].len = data->trans_len; +break ; + } +case SEV_CMD_RECEIVE_UPDATE_DATA : { +struct sev_data_receive_update_data *data = cmd_buf; + + desc_list[0 ].paddr_ptr = &data->guest_address; + desc_list[0 ].len = data->guest_len; + desc_list[0 ].guest_owned = true; +break ; + } +case SEV_CMD_RECEIVE_UPDATE_VMSA : { +struct sev_data_receive_update_vmsa *data = cmd_buf; + + desc_list[0 ].paddr_ptr = &data->guest_address; + desc_list[0 ].len = data->guest_len; + desc_list[0 ].guest_owned = true; +break ; + } +default : +break ; + } +} + +static int snp_map_cmd_buf_desc (struct cmd_buf_desc *desc) +{ +unsigned int npages; + +if (!desc->len) +return 0 ; + +/* Allocate a bounce buffer if this isn't a guest owned page. */ +if (!desc->guest_owned) { +struct page *page; + + page =alloc_pages (GFP_KERNEL_ACCOUNT ,get_order (desc->len)); +if (!page) { +pr_warn ("Failed to allocate bounce buffer for SEV legacy command.\n" ); +return -ENOMEM ; + } + + desc->paddr_orig = *desc->paddr_ptr; + *desc->paddr_ptr =__psp_pa (page_to_virt (page)); + } + + npages =PAGE_ALIGN (desc->len) >>PAGE_SHIFT ; + +/* Transition the buffer to firmware-owned. */ +if (rmp_mark_pages_firmware (*desc->paddr_ptr, npages, true)) { +pr_warn ("Error moving pages to firmware-owned state for SEV legacy command.\n" ); +return -EFAULT ; + } + +return 0 ; +} + +static int snp_unmap_cmd_buf_desc (struct cmd_buf_desc *desc) +{ +unsigned int npages; + +if (!desc->len) +return 0 ; + + npages =PAGE_ALIGN (desc->len) >>PAGE_SHIFT ; + +/* Transition the buffers back to hypervisor-owned. */ +if (snp_reclaim_pages (*desc->paddr_ptr, npages, true)) { +pr_warn ("Failed to reclaim firmware-owned pages while issuing SEV legacy command.\n" ); +return -EFAULT ; + } + +/* Copy data from bounce buffer and then free it. */ +if (!desc->guest_owned) { +void *bounce_buf =__va (__sme_clr (*desc->paddr_ptr)); +void *dst_buf =__va (__sme_clr (desc->paddr_orig)); + +memcpy (dst_buf, bounce_buf, desc->len); +__free_pages (virt_to_page (bounce_buf),get_order (desc->len)); + +/* Restore the original address in the command buffer. */ + *desc->paddr_ptr = desc->paddr_orig; + } + +return 0 ; +} + +static int snp_map_cmd_buf_desc_list (int cmd,void *cmd_buf,struct cmd_buf_desc *desc_list) +{ +int i; + +snp_populate_cmd_buf_desc_list (cmd, cmd_buf, desc_list); + +for (i =0 ; i <CMD_BUF_DESC_MAX ; i++) { +struct cmd_buf_desc *desc = &desc_list[i]; + +if (!desc->paddr_ptr) +break ; + +if (snp_map_cmd_buf_desc (desc)) + goto err_unmap; + } + +return 0 ; + +err_unmap: +for (i--; i >=0 ; i--) +snp_unmap_cmd_buf_desc (&desc_list[i]); + +return -EFAULT ; +} + +static int snp_unmap_cmd_buf_desc_list (struct cmd_buf_desc *desc_list) +{ +int i, ret =0 ; + +for (i =0 ; i <CMD_BUF_DESC_MAX ; i++) { +struct cmd_buf_desc *desc = &desc_list[i]; + +if (!desc->paddr_ptr) +break ; + +if (snp_unmap_cmd_buf_desc (&desc_list[i])) + ret = -EFAULT ; + } + +return ret; +} + +static bool sev_cmd_buf_writable (int cmd) +{ +switch (cmd) { +case SEV_CMD_PLATFORM_STATUS : +case SEV_CMD_GUEST_STATUS : +case SEV_CMD_LAUNCH_START : +case SEV_CMD_RECEIVE_START : +case SEV_CMD_LAUNCH_MEASURE : +case SEV_CMD_SEND_START : +case SEV_CMD_SEND_UPDATE_DATA : +case SEV_CMD_SEND_UPDATE_VMSA : +case SEV_CMD_PEK_CSR : +case SEV_CMD_PDH_CERT_EXPORT : +case SEV_CMD_GET_ID : +case SEV_CMD_ATTESTATION_REPORT : +return true; +default : +return false; + } +} + +/* After SNP is INIT'ed, the behavior of legacy SEV commands is changed. */ +static bool snp_legacy_handling_needed (int cmd) +{ +struct sev_device *sev = psp_master->sev_data; + +return cmd <SEV_CMD_SNP_INIT && sev->snp_initialized; +} + +static int snp_prep_cmd_buf (int cmd,void *cmd_buf,struct cmd_buf_desc *desc_list) +{ +if (!snp_legacy_handling_needed (cmd)) +return 0 ; + +if (snp_map_cmd_buf_desc_list (cmd, cmd_buf, desc_list)) +return -EFAULT ; + +/* +* Before command execution, the command buffer needs to be put into +* the firmware-owned state. +*/ +if (sev_cmd_buf_writable (cmd)) { +if (rmp_mark_pages_firmware (__pa (cmd_buf),1 , true)) +return -EFAULT ; + } + +return 0 ; +} + +static int snp_reclaim_cmd_buf (int cmd,void *cmd_buf) +{ +if (!snp_legacy_handling_needed (cmd)) +return 0 ; + +/* +* After command completion, the command buffer needs to be put back +* into the hypervisor-owned state. +*/ +if (sev_cmd_buf_writable (cmd)) +if (snp_reclaim_pages (__pa (cmd_buf),1 , true)) +return -EFAULT ; + +return 0 ; +} +static int __sev_do_cmd_locked (int cmd,void *data,int *psp_ret) { +struct cmd_buf_desc desc_list[CMD_BUF_DESC_MAX ] = {0 };struct psp_device *psp = psp_master;struct sev_device *sev;unsigned int cmdbuff_hi, cmdbuff_lo;unsigned int phys_lsb, phys_msb;unsigned int reg, ret =0 ; +void *cmd_buf;int buf_len;if (!psp || !psp->sev_data)
@@ -490,12 +835,47 @@ static int __sev_do_cmd_locked(int cmd, void *data, int *psp_ret)* work for some memory, e.g. vmalloc'd addresses, and @data may not be * physically contiguous. */ -if (data) -memcpy (sev->cmd_buf, data, buf_len); +if (data) { +/* +* Commands are generally issued one at a time and require the +* sev_cmd_mutex, but there could be recursive firmware requests +* due to SEV_CMD_SNP_PAGE_RECLAIM needing to be issued while +* preparing buffers for another command. This is the only known +* case of nesting in the current code, so exactly one +* additional command buffer is available for that purpose. +*/ +if (!sev->cmd_buf_active) { + cmd_buf = sev->cmd_buf; + sev->cmd_buf_active = true; + }else if (!sev->cmd_buf_backup_active) { + cmd_buf = sev->cmd_buf_backup; + sev->cmd_buf_backup_active = true; + }else { +dev_err (sev->dev, +"SEV: too many firmware commands in progress, no command buffers available.\n" ); +return -EBUSY ; + } + +memcpy (cmd_buf, data, buf_len); + +/* +* The behavior of the SEV-legacy commands is altered when the +* SNP firmware is in the INIT state. +*/ + ret =snp_prep_cmd_buf (cmd, cmd_buf, desc_list); +if (ret) { +dev_err (sev->dev, +"SEV: failed to prepare buffer for legacy command 0x%x. Error: %d\n" , + cmd, ret); +return ret; + } + }else { + cmd_buf = sev->cmd_buf; + }/* Get the physical address of the command buffer */ - phys_lsb = data ?lower_32_bits (__psp_pa (sev->cmd_buf)) :0 ; - phys_msb = data ?upper_32_bits (__psp_pa (sev->cmd_buf)) :0 ; + phys_lsb = data ?lower_32_bits (__psp_pa (cmd_buf)) :0 ; + phys_msb = data ?upper_32_bits (__psp_pa (cmd_buf)) :0 ;dev_dbg (sev->dev,"sev command id %#x buffer 0x%08x%08x timeout %us\n" , cmd, phys_msb, phys_lsb, psp_timeout);
@@ -549,15 +929,36 @@ static int __sev_do_cmd_locked(int cmd, void *data, int *psp_ret) ret =sev_write_init_ex_file_if_required (cmd); } -print_hex_dump_debug ("(out): " ,DUMP_PREFIX_OFFSET ,16 ,2 , data, - buf_len, false); -/* * Copy potential output from the PSP back to data. Do this even on * failure in case the caller wants to glean something from the error. */ -if (data) -memcpy (data, sev->cmd_buf, buf_len); +if (data) { +int ret_reclaim; +/* +* Restore the page state after the command completes. +*/ + ret_reclaim =snp_reclaim_cmd_buf (cmd, cmd_buf); +if (ret_reclaim) { +dev_err (sev->dev, +"SEV: failed to reclaim buffer for legacy command %#x. Error: %d\n" , + cmd, ret_reclaim); +return ret_reclaim; + } + +memcpy (data, cmd_buf, buf_len); + +if (sev->cmd_buf_backup_active) + sev->cmd_buf_backup_active = false; +else + sev->cmd_buf_active = false; + +if (snp_unmap_cmd_buf_desc_list (desc_list)) +return -EFAULT ; + } + +print_hex_dump_debug ("(out): " ,DUMP_PREFIX_OFFSET ,16 ,2 , data, + buf_len, false);return ret; }
@@ -1674,10 +2075,12 @@ int sev_dev_init(struct psp_device *psp)if (!sev) goto e_err; - sev->cmd_buf = (void *)devm_get_free_pages (dev,GFP_KERNEL ,0 ); + sev->cmd_buf = (void *)devm_get_free_pages (dev,GFP_KERNEL ,1 );if (!sev->cmd_buf) goto e_sev; + sev->cmd_buf_backup = (uint8_t *)sev->cmd_buf +PAGE_SIZE ; + psp->sev_data = sev; sev->dev = dev;
drivers/crypto/ccp/sev-dev.h +3 -0
@@ -52,6 +52,9 @@ struct sev_device {u8 build;void *cmd_buf; +void *cmd_buf_backup; +bool cmd_buf_active; +bool cmd_buf_backup_active;bool snp_initialized; };
PATCH v2 19/25 iommu/amd: Clean up RMP entries for IOMMU pages during SNP shutdown
In reply to Michael RothFrom: Ashish Kalra <[email protected]>
Add a new IOMMU API interface amd_iommu_snp_disable() to transition IOMMU pages to Hypervisor state from Reclaim state after SNP_SHUTDOWN_EX command. Invoke this API from the CCP driver after SNP_SHUTDOWN_EX command.
--- drivers/crypto/ccp/sev-dev.c | 20 +++++++++ drivers/iommu/amd/init.c | 79 ++++++++++++++++++++++++++++++++++++ include/linux/amd-iommu.h | 6 +++ 3 files changed, 105 insertions(+)
drivers/crypto/ccp/sev-dev.c +20 -0
@@ -26,6 +26,7 @@#include <linux/fs.h> #include <linux/fs_struct.h> #include <linux/psp.h> +#include <linux/amd-iommu.h> #include <asm/smp.h> #include <asm/cacheflush.h>
@@ -1633,6 +1634,25 @@ static int __sev_snp_shutdown_locked(int *error)return ret; } +/* +* SNP_SHUTDOWN_EX with IOMMU_SNP_SHUTDOWN set to 1 disables SNP +* enforcement by the IOMMU and also transitions all pages +* associated with the IOMMU to the Reclaim state. +* Firmware was transitioning the IOMMU pages to Hypervisor state +* before version 1.53. But, accounting for the number of assigned +* 4kB pages in a 2M page was done incorrectly by not transitioning +* to the Reclaim state. This resulted in RMP #PF when later accessing +* the 2M page containing those pages during kexec boot. Hence, the +* firmware now transitions these pages to Reclaim state and hypervisor +* needs to transition these pages to shared state. SNP Firmware +* version 1.53 and above are needed for kexec boot. +*/ + ret =amd_iommu_snp_disable (); +if (ret) { +dev_err (sev->dev,"SNP IOMMU shutdown failed\n" ); +return ret; + } + sev->snp_initialized = false;dev_dbg (sev->dev,"SEV-SNP firmware shutdown\n" );
drivers/iommu/amd/init.c +79 -0
88 lines not on this page · raw message
include/linux/amd-iommu.h +6 -0
@@ -85,4 +85,10 @@ 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_KVM_AMD_SEV +int amd_iommu_snp_disable (void ); +#else +static inline int amd_iommu_snp_disable (void ) {return 0 ; } +#endif +#endif /* _ASM_X86_AMD_IOMMU_H */
Signature · 1 line
2.25.1
[tip: x86/sev] iommu/amd: Clean up RMP entries for IOMMU pages during SNP shutdown
The following commit has been merged into the x86/sev branch of tip:
The following commit has been merged into the x86/sev branch of tip:
Commit-ID: f366a8dac1b8fef28a470d4e67b9843ebb8e2a1f Gitweb: https://git.kernel.org/tip/f366a8dac1b8fef28a470d4e67b9843ebb8e2a1f Author: Ashish Kalra <[email protected]> AuthorDate: Thu, 25 Jan 2024 22:11:19 -06:00 Committer: Borislav Petkov (AMD) <[email protected]> CommitterDate: Mon, 29 Jan 2024 20:34:18 +01:00
iommu/amd: Clean up RMP entries for IOMMU pages during SNP shutdown
Add a new IOMMU API interface amd_iommu_snp_disable() to transition IOMMU pages to Hypervisor state from Reclaim state after SNP_SHUTDOWN_EX command. Invoke this API from the CCP driver after SNP_SHUTDOWN_EX command.
--- drivers/crypto/ccp/sev-dev.c | 20 +++++++++- drivers/iommu/amd/init.c | 79 +++++++++++++++++++++++++++++++++++- include/linux/amd-iommu.h | 6 +++- 3 files changed, 105 insertions(+)
drivers/crypto/ccp/sev-dev.c +20 -0
@@ -26,6 +26,7 @@#include <linux/fs.h> #include <linux/fs_struct.h> #include <linux/psp.h> +#include <linux/amd-iommu.h> #include <asm/smp.h> #include <asm/cacheflush.h>
@@ -1655,6 +1656,25 @@ static int __sev_snp_shutdown_locked(int *error)return ret; } +/* +* SNP_SHUTDOWN_EX with IOMMU_SNP_SHUTDOWN set to 1 disables SNP +* enforcement by the IOMMU and also transitions all pages +* associated with the IOMMU to the Reclaim state. +* Firmware was transitioning the IOMMU pages to Hypervisor state +* before version 1.53. But, accounting for the number of assigned +* 4kB pages in a 2M page was done incorrectly by not transitioning +* to the Reclaim state. This resulted in RMP #PF when later accessing +* the 2M page containing those pages during kexec boot. Hence, the +* firmware now transitions these pages to Reclaim state and hypervisor +* needs to transition these pages to shared state. SNP Firmware +* version 1.53 and above are needed for kexec boot. +*/ + ret =amd_iommu_snp_disable (); +if (ret) { +dev_err (sev->dev,"SNP IOMMU shutdown failed\n" ); +return ret; + } + sev->snp_initialized = false;dev_dbg (sev->dev,"SEV-SNP firmware shutdown\n" );
drivers/iommu/amd/init.c +79 -0
88 lines not on this page · raw message
include/linux/amd-iommu.h +6 -0
@@ -85,4 +85,10 @@ 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_KVM_AMD_SEV +int amd_iommu_snp_disable (void ); +#else +static inline int amd_iommu_snp_disable (void ) {return 0 ; } +#endif +#endif /* _ASM_X86_AMD_IOMMU_H */
[tip: x86/sev] iommu/amd: Fix failure return from snp_lookup_rmpentry()
In reply to Michael RothThe following commit has been merged into the x86/sev branch of tip:
The following commit has been merged into the x86/sev branch of tip:
Commit-ID: 45ba5b3c0a02949a4da74ead6e11c43e9b88bdca Gitweb: https://git.kernel.org/tip/45ba5b3c0a02949a4da74ead6e11c43e9b88bdca Author: Ashish Kalra <[email protected]> AuthorDate: Mon, 05 Feb 2024 21:06:54 Committer: Borislav Petkov (AMD) <[email protected]> CommitterDate: Wed, 07 Feb 2024 17:27:03 +01:00
iommu/amd: Fix failure return from snp_lookup_rmpentry()
Commit
f366a8dac1b8: ("iommu/amd: Clean up RMP entries for IOMMU pages during SNP shutdown")
leads to the following Smatch static checker warning:
drivers/iommu/amd/init.c:3820 iommu_page_make_shared() error: uninitialized symbol 'assigned'.
Fix it.
[ bp: Address the other error cases too. ]
--- drivers/iommu/amd/init.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-)
drivers/iommu/amd/init.c +15 -10
@@ -3813,23 +3813,28 @@ static int iommu_page_make_shared(void *page)bool assigned; ret =snp_lookup_rmpentry (pfn, &assigned, &level); -if (ret) -pr_warn ("IOMMU PFN %lx RMP lookup failed, ret %d\n" , - pfn, ret); +if (ret) { +pr_warn ("IOMMU PFN %lx RMP lookup failed, ret %d\n" , pfn, ret); +return ret; + } -if (!assigned) -pr_warn ("IOMMU PFN %lx not assigned in RMP table\n" , - pfn); +if (!assigned) { +pr_warn ("IOMMU PFN %lx not assigned in RMP table\n" , pfn); +return -EINVAL ; + }if (level >PG_LEVEL_4K ) { ret =psmash (pfn); -if (ret) { -pr_warn ("IOMMU PFN %lx had a huge RMP entry, but attempted psmash failed, ret: %d, level: %d\n" , - pfn, ret, level); - } +if (!ret) + goto done; + +pr_warn ("PSMASH failed for IOMMU PFN %lx huge RMP entry, ret: %d, level: %d\n" , + pfn, ret, level); +return ret; } } +done:return rmp_make_shared (pfn,PG_LEVEL_4K ); }
PATCH v2 20/25 crypto: ccp: Add panic notifier for SEV/SNP firmware shutdown on kdump
In reply to Michael RothFrom: Ashish Kalra <[email protected]>
Add a kdump safe version of sev_firmware_shutdown() and register it as a crash_kexec_post_notifier so it will be invoked during panic/crash to do SEV/SNP shutdown. This is required for transitioning all IOMMU pages to reclaim/hypervisor state, otherwise re-init of IOMMU pages during crashdump kernel boot fails and panics the crashdump kernel.
This panic notifier runs in atomic context, hence it ensures not to acquire any locks/mutexes and polls for PSP command completion instead of depending on PSP command completion interrupt.
--- arch/x86/include/asm/sev.h | 2 + arch/x86/kernel/crash.c | 3 + arch/x86/kernel/sev.c | 10 ++++ arch/x86/virt/svm/sev.c | 6 ++ drivers/crypto/ccp/sev-dev.c | 111 +++++++++++++++++++++++++---------- 5 files changed, 102 insertions(+), 30 deletions(-)
arch/x86/include/asm/sev.h +2 -0
@@ -227,6 +227,7 @@ int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, struct snvoid snp_accept_memory (phys_addr_t start,phys_addr_t end);u64 snp_get_unsupported_features (u64 status);u64 sev_get_status (void ); +void kdump_sev_callback (void );#else static inline void sev_es_ist_enter (struct pt_regs *regs) { }static inline void sev_es_ist_exit (void ) { }
@@ -255,6 +256,7 @@ static inline int snp_issue_guest_request(u64 exit_code, struct snp_req_data *instatic inline void snp_accept_memory (phys_addr_t start,phys_addr_t end) { }static inline u64 snp_get_unsupported_features (u64 status) {return 0 ; }static inline u64 sev_get_status (void ) {return 0 ; } +static inline void kdump_sev_callback (void ) { }#endif #ifdef CONFIG_KVM_AMD_SEV
arch/x86/kernel/crash.c +3 -0
@@ -40,6 +40,7 @@#include <asm/intel_pt.h> #include <asm/crash.h> #include <asm/cmdline.h> +#include <asm/sev.h> /* Used while preparing memory map entries for second kernel */ struct crash_memmap_data {
@@ -59,6 +60,8 @@ static void kdump_nmi_callback(int cpu, struct pt_regs *regs)*/ cpu_emergency_stop_pt (); +kdump_sev_callback (); +disable_local_APIC (); }
arch/x86/kernel/sev.c +10 -0
@@ -2265,3 +2265,13 @@ static int __init snp_init_platform_device(void)return 0 ; }device_initcall (snp_init_platform_device); + +void kdump_sev_callback (void ) +{ +/* +* Do wbinvd() on remote CPUs when SNP is enabled in order to +* safely do SNP_SHUTDOWN on the local CPU. +*/ +if (cpu_feature_enabled (X86_FEATURE_SEV_SNP )) +wbinvd (); +}
arch/x86/virt/svm/sev.c +6 -0
@@ -216,6 +216,12 @@ static int __init snp_rmptable_init(void)cpuhp_setup_state (CPUHP_AP_ONLINE_DYN ,"x86/rmptable_init:online" , __snp_enable,NULL ); +/* +* Setting crash_kexec_post_notifiers to 'true' to ensure that SNP panic +* notifier is invoked to do SNP IOMMU shutdown before kdump. +*/ + crash_kexec_post_notifiers = true; +return 0 ; nosnp:
drivers/crypto/ccp/sev-dev.c +81 -30
183 lines not on this page · raw message
Signature · 1 line
2.25.1
[tip: x86/sev] crypto: ccp: Add panic notifier for SEV/SNP firmware shutdown on kdump
The following commit has been merged into the x86/sev branch of tip:
The following commit has been merged into the x86/sev branch of tip:
Commit-ID: 8ef979584ea86c247b768f4420148721a842835f Gitweb: https://git.kernel.org/tip/8ef979584ea86c247b768f4420148721a842835f Author: Ashish Kalra <[email protected]> AuthorDate: Thu, 25 Jan 2024 22:11:20 -06:00 Committer: Borislav Petkov (AMD) <[email protected]> CommitterDate: Mon, 29 Jan 2024 20:34:19 +01:00
crypto: ccp: Add panic notifier for SEV/SNP firmware shutdown on kdump
Add a kdump safe version of sev_firmware_shutdown() and register it as a crash_kexec_post_notifier so it will be invoked during panic/crash to do SEV/SNP shutdown. This is required for transitioning all IOMMU pages to reclaim/hypervisor state, otherwise re-init of IOMMU pages during crashdump kernel boot fails and panics the crashdump kernel.
This panic notifier runs in atomic context, hence it ensures not to acquire any locks/mutexes and polls for PSP command completion instead of depending on PSP command completion interrupt.
[ mdr: Remove use of "we" in comments. ]
--- arch/x86/include/asm/sev.h | 2 +- arch/x86/kernel/crash.c | 3 +- arch/x86/kernel/sev.c | 10 +++- arch/x86/virt/svm/sev.c | 6 ++- drivers/crypto/ccp/sev-dev.c | 111 ++++++++++++++++++++++++---------- 5 files changed, 102 insertions(+), 30 deletions(-)
arch/x86/include/asm/sev.h +2 -0
@@ -227,6 +227,7 @@ int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, struct snvoid snp_accept_memory (phys_addr_t start,phys_addr_t end);u64 snp_get_unsupported_features (u64 status);u64 sev_get_status (void ); +void kdump_sev_callback (void );#else static inline void sev_es_ist_enter (struct pt_regs *regs) { }static inline void sev_es_ist_exit (void ) { }
@@ -255,6 +256,7 @@ static inline int snp_issue_guest_request(u64 exit_code, struct snp_req_data *instatic inline void snp_accept_memory (phys_addr_t start,phys_addr_t end) { }static inline u64 snp_get_unsupported_features (u64 status) {return 0 ; }static inline u64 sev_get_status (void ) {return 0 ; } +static inline void kdump_sev_callback (void ) { }#endif #ifdef CONFIG_KVM_AMD_SEV
arch/x86/kernel/crash.c +3 -0
@@ -40,6 +40,7 @@#include <asm/intel_pt.h> #include <asm/crash.h> #include <asm/cmdline.h> +#include <asm/sev.h> /* Used while preparing memory map entries for second kernel */ struct crash_memmap_data {
@@ -59,6 +60,8 @@ static void kdump_nmi_callback(int cpu, struct pt_regs *regs)*/ cpu_emergency_stop_pt (); +kdump_sev_callback (); +disable_local_APIC (); }
arch/x86/kernel/sev.c +10 -0
@@ -2265,3 +2265,13 @@ static int __init snp_init_platform_device(void)return 0 ; }device_initcall (snp_init_platform_device); + +void kdump_sev_callback (void ) +{ +/* +* Do wbinvd() on remote CPUs when SNP is enabled in order to +* safely do SNP_SHUTDOWN on the local CPU. +*/ +if (cpu_feature_enabled (X86_FEATURE_SEV_SNP )) +wbinvd (); +}
arch/x86/virt/svm/sev.c +6 -0
@@ -216,6 +216,12 @@ skip_enable:cpuhp_setup_state (CPUHP_AP_ONLINE_DYN ,"x86/rmptable_init:online" , __snp_enable,NULL ); +/* +* Setting crash_kexec_post_notifiers to 'true' to ensure that SNP panic +* notifier is invoked to do SNP IOMMU shutdown before kdump. +*/ + crash_kexec_post_notifiers = true; +return 0 ; nosnp:
drivers/crypto/ccp/sev-dev.c +81 -30
183 lines not on this page · raw message
PATCH v2 21/25 KVM: SEV: Make AVIC backing, VMSA and VMCB memory allocation SNP safe
In reply to Michael RothFrom: Brijesh Singh <[email protected]>
Implement a workaround for an SNP erratum where the CPU will incorrectly signal an RMP violation #PF if a hugepage (2MB or 1GB) collides with the RMP entry of a VMCB, VMSA or AVIC backing page.
When SEV-SNP is globally enabled, the CPU marks the VMCB, VMSA, and AVIC backing pages as "in-use" via a reserved bit in the corresponding RMP entry after a successful VMRUN. This is done for _all_ VMs, not just SNP-Active VMs.
If the hypervisor accesses an in-use page through a writable translation, the CPU will throw an RMP violation #PF. On early SNP hardware, if an in-use page is 2MB-aligned and software accesses any part of the associated 2MB region with a hugepage, the CPU will incorrectly treat the entire 2MB region as in-use and signal a an RMP violation #PF.
To avoid this, the recommendation is to not use a 2MB-aligned page for the VMCB, VMSA or AVIC pages. Add a generic allocator that will ensure that the page returned is not 2MB-aligned and is safe to be used when SEV-SNP is enabled. Also implement similar handling for the VMCB/VMSA pages of nested guests.
--- arch/x86/include/asm/kvm-x86-ops.h | 1 + arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/lapic.c | 5 ++++- arch/x86/kvm/svm/nested.c | 2 +- arch/x86/kvm/svm/sev.c | 32 ++++++++++++++++++++++++++++++ arch/x86/kvm/svm/svm.c | 17 +++++++++++++--- arch/x86/kvm/svm/svm.h | 1 + 7 files changed, 54 insertions(+), 5 deletions(-)
arch/x86/include/asm/kvm-x86-ops.h +1 -0
@@ -138,6 +138,7 @@ KVM_X86_OP(complete_emulated_msr)KVM_X86_OP (vcpu_deliver_sipi_vector)KVM_X86_OP_OPTIONAL_RET0 (vcpu_get_apicv_inhibit_reasons); KVM_X86_OP_OPTIONAL(get_untagged_addr) +KVM_X86_OP_OPTIONAL(alloc_apic_backing_page)#undef KVM_X86_OP#undef KVM_X86_OP_OPTIONAL
arch/x86/include/asm/kvm_host.h +1 -0
@@ -1794,6 +1794,7 @@ struct kvm_x86_ops {unsigned long (*vcpu_get_apicv_inhibit_reasons)(struct kvm_vcpu *vcpu);gva_t (*get_untagged_addr)(struct kvm_vcpu *vcpu,gva_t gva,unsigned int flags); +void *(*alloc_apic_backing_page)(struct kvm_vcpu *vcpu); };struct kvm_x86_nested_ops {
arch/x86/kvm/lapic.c +4 -1
@@ -2815,7 +2815,10 @@ int kvm_create_lapic(struct kvm_vcpu *vcpu, int timer_advance_ns) vcpu->arch.apic = apic; - apic->regs = (void *)get_zeroed_page (GFP_KERNEL_ACCOUNT ); +if (kvm_x86_ops.alloc_apic_backing_page) + apic->regs =static_call (kvm_x86_alloc_apic_backing_page)(vcpu); +else + apic->regs = (void *)get_zeroed_page (GFP_KERNEL_ACCOUNT );if (!apic->regs) {printk (KERN_ERR "malloc apic regs error for vcpu %x\n" , vcpu->vcpu_id);
arch/x86/kvm/svm/nested.c +1 -1
@@ -1181,7 +1181,7 @@ int svm_allocate_nested(struct vcpu_svm *svm)if (svm->nested.initialized)return 0 ; - vmcb02_page =alloc_page (GFP_KERNEL_ACCOUNT | __GFP_ZERO); + vmcb02_page =snp_safe_alloc_page (&svm->vcpu);if (!vmcb02_page)return -ENOMEM ; svm->nested.vmcb02.ptr =page_address (vmcb02_page);
arch/x86/kvm/svm/sev.c +32 -0
@@ -3163,3 +3163,35 @@ void sev_vcpu_deliver_sipi_vector(struct kvm_vcpu *vcpu, u8 vector)ghcb_set_sw_exit_info_2 (svm->sev_es.ghcb,1 ); } + +struct page *snp_safe_alloc_page (struct kvm_vcpu *vcpu) +{ +unsigned long pfn; +struct page *p; + +if (!cpu_feature_enabled (X86_FEATURE_SEV_SNP )) +return alloc_page (GFP_KERNEL_ACCOUNT | __GFP_ZERO); + +/* +* Allocate an SNP-safe page to workaround the SNP erratum where +* the CPU will incorrectly signal an RMP violation #PF if a +* hugepage (2MB or 1GB) collides with the RMP entry of a +* 2MB-aligned VMCB, VMSA, or AVIC backing page. +* +* Allocate one extra page, choose a page which is not +* 2MB-aligned, and free the other. +*/ + p =alloc_pages (GFP_KERNEL_ACCOUNT | __GFP_ZERO,1 ); +if (!p) +return NULL ; + +split_page (p,1 ); + + pfn =page_to_pfn (p); +if (IS_ALIGNED (pfn,PTRS_PER_PMD )) +__free_page (p++); +else +__free_page (p +1 ); + +return p; +}
arch/x86/kvm/svm/svm.c +14 -3
@@ -703,7 +703,7 @@ static int svm_cpu_init(int cpu)int ret = -ENOMEM ;memset (sd,0 ,sizeof (struct svm_cpu_data )); - sd->save_area =alloc_page (GFP_KERNEL | __GFP_ZERO); + sd->save_area =snp_safe_alloc_page (NULL );if (!sd->save_area)return ret;
@@ -1421,7 +1421,7 @@ static int svm_vcpu_create(struct kvm_vcpu *vcpu) svm =to_svm (vcpu); err = -ENOMEM ; - vmcb01_page =alloc_page (GFP_KERNEL_ACCOUNT | __GFP_ZERO); + vmcb01_page =snp_safe_alloc_page (vcpu);if (!vmcb01_page) goto out;
@@ -1430,7 +1430,7 @@ static int svm_vcpu_create(struct kvm_vcpu *vcpu)* SEV-ES guests require a separate VMSA page used to contain * the encrypted register state of the guest. */ - vmsa_page =alloc_page (GFP_KERNEL_ACCOUNT | __GFP_ZERO); + vmsa_page =snp_safe_alloc_page (vcpu);if (!vmsa_page) goto error_free_vmcb_page;
@@ -4900,6 +4900,16 @@ static int svm_vm_init(struct kvm *kvm)return 0 ; } +static void *svm_alloc_apic_backing_page (struct kvm_vcpu *vcpu) +{ +struct page *page =snp_safe_alloc_page (vcpu); + +if (!page) +return NULL ; + +return page_address (page); +} +static struct kvm_x86_ops svm_x86_ops__initdata = { .name =KBUILD_MODNAME ,
@@ -5031,6 +5041,7 @@ static struct kvm_x86_ops svm_x86_ops __initdata = { .vcpu_deliver_sipi_vector = svm_vcpu_deliver_sipi_vector, .vcpu_get_apicv_inhibit_reasons = avic_vcpu_get_apicv_inhibit_reasons, + .alloc_apic_backing_page = svm_alloc_apic_backing_page, };/*
arch/x86/kvm/svm/svm.h +1 -0
@@ -694,6 +694,7 @@ void sev_es_vcpu_reset(struct vcpu_svm *svm);void sev_vcpu_deliver_sipi_vector (struct kvm_vcpu *vcpu,u8 vector);void sev_es_prepare_switch_to_guest (struct sev_es_save_area *hostsa);void sev_es_unmap_ghcb (struct vcpu_svm *svm); +struct page *snp_safe_alloc_page (struct kvm_vcpu *vcpu);/* vmenter.S */
Signature · 1 line
2.25.1
Re: [PATCH v2 21/25] KVM: SEV: Make AVIC backing, VMSA and VMCB memory allocation SNP safe
Quoted 25 lines · Michael Roth
From: Brijesh Singh <[email protected]>
Implement a workaround for an SNP erratum where the CPU will incorrectly signal an RMP violation #PF if a hugepage (2MB or 1GB) collides with the RMP entry of a VMCB, VMSA or AVIC backing page.
When SEV-SNP is globally enabled, the CPU marks the VMCB, VMSA, and AVIC backing pages as "in-use" via a reserved bit in the corresponding RMP entry after a successful VMRUN. This is done for _all_ VMs, not just SNP-Active VMs.
If the hypervisor accesses an in-use page through a writable translation, the CPU will throw an RMP violation #PF. On early SNP hardware, if an in-use page is 2MB-aligned and software accesses any part of the associated 2MB region with a hugepage, the CPU will incorrectly treat the entire 2MB region as in-use and signal a an RMP violation #PF.
To avoid this, the recommendation is to not use a 2MB-aligned page for the VMCB, VMSA or AVIC pages. Add a generic allocator that will ensure that the page returned is not 2MB-aligned and is safe to be used when SEV-SNP is enabled. Also implement similar handling for the VMCB/VMSA pages of nested guests.
Quoted 9 lines
Signed-off-by Brijesh Singh <[email protected]>Co-developed-by Marc Orr <[email protected]>Signed-off-by Marc Orr <[email protected]>Co-developed-by Ashish Kalra <[email protected]>Signed-off-by Ashish Kalra <[email protected]>Acked-by Vlastimil Babka <[email protected]>[mdr: squash in nested guest handling from Ashish, commit msg fixups]Signed-off-by Michael Roth <[email protected]>
Quoted 174 lines
--- arch/x86/include/asm/kvm-x86-ops.h | 1 + arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/lapic.c | 5 ++++- arch/x86/kvm/svm/nested.c | 2 +- arch/x86/kvm/svm/sev.c | 32 ++++++++++++++++++++++++++++++ arch/x86/kvm/svm/svm.c | 17 +++++++++++++--- arch/x86/kvm/svm/svm.h | 1 + 7 files changed, 54 insertions(+), 5 deletions(-)arch/x86/include/asm/kvm-x86-ops.h +1
-0@@ -138,6 +138,7 @@ KVM_X86_OP(complete_emulated_msr)KVM_X86_OP (vcpu_deliver_sipi_vector)KVM_X86_OP_OPTIONAL_RET0 (vcpu_get_apicv_inhibit_reasons); KVM_X86_OP_OPTIONAL(get_untagged_addr) +KVM_X86_OP_OPTIONAL(alloc_apic_backing_page)#undef KVM_X86_OP#undef KVM_X86_OP_OPTIONALarch/x86/include/asm/kvm_host.h +1
-0@@ -1794,6 +1794,7 @@ struct kvm_x86_ops {unsigned long (*vcpu_get_apicv_inhibit_reasons)(struct kvm_vcpu *vcpu);gva_t (*get_untagged_addr)(struct kvm_vcpu *vcpu,gva_t gva,unsigned int flags); +void *(*alloc_apic_backing_page)(struct kvm_vcpu *vcpu); };struct kvm_x86_nested_ops {arch/x86/kvm/lapic.c +4
-1@@ -2815,7 +2815,10 @@ int kvm_create_lapic(struct kvm_vcpu *vcpu, int timer_advance_ns) vcpu->arch.apic = apic; - apic->regs = (void *)get_zeroed_page (GFP_KERNEL_ACCOUNT ); +if (kvm_x86_ops.alloc_apic_backing_page) + apic->regs =static_call (kvm_x86_alloc_apic_backing_page)(vcpu); +else + apic->regs = (void *)get_zeroed_page (GFP_KERNEL_ACCOUNT );if (!apic->regs) {printk (KERN_ERR "malloc apic regs error for vcpu %x\n" , vcpu->vcpu_id);arch/x86/kvm/svm/nested.c +1
-1@@ -1181,7 +1181,7 @@ int svm_allocate_nested(struct vcpu_svm *svm)if (svm->nested.initialized)return 0 ; - vmcb02_page =alloc_page (GFP_KERNEL_ACCOUNT | __GFP_ZERO); + vmcb02_page =snp_safe_alloc_page (&svm->vcpu);if (!vmcb02_page)return -ENOMEM ; svm->nested.vmcb02.ptr =page_address (vmcb02_page);arch/x86/kvm/svm/sev.c +32
-0@@ -3163,3 +3163,35 @@ void sev_vcpu_deliver_sipi_vector(struct kvm_vcpu *vcpu, u8 vector)ghcb_set_sw_exit_info_2 (svm->sev_es.ghcb,1 ); } + +struct page *snp_safe_alloc_page (struct kvm_vcpu *vcpu) +{ +unsigned long pfn; +struct page *p; + +if (!cpu_feature_enabled (X86_FEATURE_SEV_SNP )) +return alloc_page (GFP_KERNEL_ACCOUNT | __GFP_ZERO); + +/* +* Allocate an SNP-safe page to workaround the SNP erratum where +* the CPU will incorrectly signal an RMP violation #PF if a +* hugepage (2MB or 1GB) collides with the RMP entry of a +* 2MB-aligned VMCB, VMSA, or AVIC backing page. +* +* Allocate one extra page, choose a page which is not +* 2MB-aligned, and free the other. +*/ + p =alloc_pages (GFP_KERNEL_ACCOUNT | __GFP_ZERO,1 ); +if (!p) +return NULL ; + +split_page (p,1 ); + + pfn =page_to_pfn (p); +if (IS_ALIGNED (pfn,PTRS_PER_PMD )) +__free_page (p++); +else +__free_page (p +1 ); + +return p; +}arch/x86/kvm/svm/svm.c +14
-3@@ -703,7 +703,7 @@ static int svm_cpu_init(int cpu)int ret = -ENOMEM ;memset (sd,0 ,sizeof (struct svm_cpu_data )); - sd->save_area =alloc_page (GFP_KERNEL | __GFP_ZERO); + sd->save_area =snp_safe_alloc_page (NULL );if (!sd->save_area)return ret;@@ -1421,7 +1421,7 @@ static int svm_vcpu_create(struct kvm_vcpu *vcpu) svm =to_svm (vcpu); err = -ENOMEM ; - vmcb01_page =alloc_page (GFP_KERNEL_ACCOUNT | __GFP_ZERO); + vmcb01_page =snp_safe_alloc_page (vcpu);if (!vmcb01_page) goto out;@@ -1430,7 +1430,7 @@ static int svm_vcpu_create(struct kvm_vcpu *vcpu)* SEV-ES guests require a separate VMSA page used to contain * the encrypted register state of the guest. */ - vmsa_page =alloc_page (GFP_KERNEL_ACCOUNT | __GFP_ZERO); + vmsa_page =snp_safe_alloc_page (vcpu);if (!vmsa_page) goto error_free_vmcb_page;@@ -4900,6 +4900,16 @@ static int svm_vm_init(struct kvm *kvm)return 0 ; } +static void *svm_alloc_apic_backing_page (struct kvm_vcpu *vcpu) +{ +struct page *page =snp_safe_alloc_page (vcpu); + +if (!page) +return NULL ; + +return page_address (page); +} +static struct kvm_x86_ops svm_x86_ops__initdata = { .name =KBUILD_MODNAME ,@@ -5031,6 +5041,7 @@ static struct kvm_x86_ops svm_x86_ops __initdata = { .vcpu_deliver_sipi_vector = svm_vcpu_deliver_sipi_vector, .vcpu_get_apicv_inhibit_reasons = avic_vcpu_get_apicv_inhibit_reasons, + .alloc_apic_backing_page = svm_alloc_apic_backing_page, };/* arch/x86/kvm/svm/svm.h +1
-1@@ -694,6 +694,7 @@ void sev_es_vcpu_reset(struct vcpu_svm *svm);void sev_vcpu_deliver_sipi_vector (struct kvm_vcpu *vcpu,u8 vector);void sev_es_prepare_switch_to_guest (struct sev_es_save_area *hostsa);void sev_es_unmap_ghcb (struct vcpu_svm *svm); +struct page *snp_safe_alloc_page (struct kvm_vcpu *vcpu);/* vmenter.S */ --2.25.1
[tip: x86/sev] KVM: SEV: Make AVIC backing, VMSA and VMCB memory allocation SNP safe
In reply to Michael RothThe following commit has been merged into the x86/sev branch of tip:
The following commit has been merged into the x86/sev branch of tip:
Commit-ID: 75253db41a467ab7983b62616b25ff083c28803a Gitweb: https://git.kernel.org/tip/75253db41a467ab7983b62616b25ff083c28803a Author: Brijesh Singh <[email protected]> AuthorDate: Thu, 25 Jan 2024 22:11:21 -06:00 Committer: Borislav Petkov (AMD) <[email protected]> CommitterDate: Mon, 29 Jan 2024 20:34:19 +01:00
KVM: SEV: Make AVIC backing, VMSA and VMCB memory allocation SNP safe
Implement a workaround for an SNP erratum where the CPU will incorrectly signal an RMP violation #PF if a hugepage (2MB or 1GB) collides with the RMP entry of a VMCB, VMSA or AVIC backing page.
When SEV-SNP is globally enabled, the CPU marks the VMCB, VMSA, and AVIC backing pages as "in-use" via a reserved bit in the corresponding RMP entry after a successful VMRUN. This is done for _all_ VMs, not just SNP-Active VMs.
If the hypervisor accesses an in-use page through a writable translation, the CPU will throw an RMP violation #PF. On early SNP hardware, if an in-use page is 2MB-aligned and software accesses any part of the associated 2MB region with a hugepage, the CPU will incorrectly treat the entire 2MB region as in-use and signal a an RMP violation #PF.
To avoid this, the recommendation is to not use a 2MB-aligned page for the VMCB, VMSA or AVIC pages. Add a generic allocator that will ensure that the page returned is not 2MB-aligned and is safe to be used when SEV-SNP is enabled. Also implement similar handling for the VMCB/VMSA pages of nested guests.
[ mdr: Squash in nested guest handling from Ashish, commit msg fixups. ]
--- arch/x86/include/asm/kvm-x86-ops.h | 1 +- arch/x86/include/asm/kvm_host.h | 1 +- arch/x86/kvm/lapic.c | 5 ++++- arch/x86/kvm/svm/nested.c | 2 +- arch/x86/kvm/svm/sev.c | 32 +++++++++++++++++++++++++++++- arch/x86/kvm/svm/svm.c | 17 ++++++++++++--- arch/x86/kvm/svm/svm.h | 1 +- 7 files changed, 54 insertions(+), 5 deletions(-)
arch/x86/include/asm/kvm-x86-ops.h +1 -0
@@ -138,6 +138,7 @@ KVM_X86_OP(complete_emulated_msr)KVM_X86_OP (vcpu_deliver_sipi_vector)KVM_X86_OP_OPTIONAL_RET0 (vcpu_get_apicv_inhibit_reasons); KVM_X86_OP_OPTIONAL(get_untagged_addr) +KVM_X86_OP_OPTIONAL(alloc_apic_backing_page)#undef KVM_X86_OP#undef KVM_X86_OP_OPTIONAL
arch/x86/include/asm/kvm_host.h +1 -0
@@ -1794,6 +1794,7 @@ struct kvm_x86_ops {unsigned long (*vcpu_get_apicv_inhibit_reasons)(struct kvm_vcpu *vcpu);gva_t (*get_untagged_addr)(struct kvm_vcpu *vcpu,gva_t gva,unsigned int flags); +void *(*alloc_apic_backing_page)(struct kvm_vcpu *vcpu); };struct kvm_x86_nested_ops {
arch/x86/kvm/lapic.c +4 -1
@@ -2815,7 +2815,10 @@ int kvm_create_lapic(struct kvm_vcpu *vcpu, int timer_advance_ns) vcpu->arch.apic = apic; - apic->regs = (void *)get_zeroed_page (GFP_KERNEL_ACCOUNT ); +if (kvm_x86_ops.alloc_apic_backing_page) + apic->regs =static_call (kvm_x86_alloc_apic_backing_page)(vcpu); +else + apic->regs = (void *)get_zeroed_page (GFP_KERNEL_ACCOUNT );if (!apic->regs) {printk (KERN_ERR "malloc apic regs error for vcpu %x\n" , vcpu->vcpu_id);
arch/x86/kvm/svm/nested.c +1 -1
@@ -1181,7 +1181,7 @@ int svm_allocate_nested(struct vcpu_svm *svm)if (svm->nested.initialized)return 0 ; - vmcb02_page =alloc_page (GFP_KERNEL_ACCOUNT | __GFP_ZERO); + vmcb02_page =snp_safe_alloc_page (&svm->vcpu);if (!vmcb02_page)return -ENOMEM ; svm->nested.vmcb02.ptr =page_address (vmcb02_page);
arch/x86/kvm/svm/sev.c +32 -0
@@ -3163,3 +3163,35 @@ void sev_vcpu_deliver_sipi_vector(struct kvm_vcpu *vcpu, u8 vector)ghcb_set_sw_exit_info_2 (svm->sev_es.ghcb,1 ); } + +struct page *snp_safe_alloc_page (struct kvm_vcpu *vcpu) +{ +unsigned long pfn; +struct page *p; + +if (!cpu_feature_enabled (X86_FEATURE_SEV_SNP )) +return alloc_page (GFP_KERNEL_ACCOUNT | __GFP_ZERO); + +/* +* Allocate an SNP-safe page to workaround the SNP erratum where +* the CPU will incorrectly signal an RMP violation #PF if a +* hugepage (2MB or 1GB) collides with the RMP entry of a +* 2MB-aligned VMCB, VMSA, or AVIC backing page. +* +* Allocate one extra page, choose a page which is not +* 2MB-aligned, and free the other. +*/ + p =alloc_pages (GFP_KERNEL_ACCOUNT | __GFP_ZERO,1 ); +if (!p) +return NULL ; + +split_page (p,1 ); + + pfn =page_to_pfn (p); +if (IS_ALIGNED (pfn,PTRS_PER_PMD )) +__free_page (p++); +else +__free_page (p +1 ); + +return p; +}
arch/x86/kvm/svm/svm.c +14 -3
@@ -703,7 +703,7 @@ static int svm_cpu_init(int cpu)int ret = -ENOMEM ;memset (sd,0 ,sizeof (struct svm_cpu_data )); - sd->save_area =alloc_page (GFP_KERNEL | __GFP_ZERO); + sd->save_area =snp_safe_alloc_page (NULL );if (!sd->save_area)return ret;
@@ -1421,7 +1421,7 @@ static int svm_vcpu_create(struct kvm_vcpu *vcpu) svm =to_svm (vcpu); err = -ENOMEM ; - vmcb01_page =alloc_page (GFP_KERNEL_ACCOUNT | __GFP_ZERO); + vmcb01_page =snp_safe_alloc_page (vcpu);if (!vmcb01_page) goto out;
@@ -1430,7 +1430,7 @@ static int svm_vcpu_create(struct kvm_vcpu *vcpu)* SEV-ES guests require a separate VMSA page used to contain * the encrypted register state of the guest. */ - vmsa_page =alloc_page (GFP_KERNEL_ACCOUNT | __GFP_ZERO); + vmsa_page =snp_safe_alloc_page (vcpu);if (!vmsa_page) goto error_free_vmcb_page;
@@ -4900,6 +4900,16 @@ static int svm_vm_init(struct kvm *kvm)return 0 ; } +static void *svm_alloc_apic_backing_page (struct kvm_vcpu *vcpu) +{ +struct page *page =snp_safe_alloc_page (vcpu); + +if (!page) +return NULL ; + +return page_address (page); +} +static struct kvm_x86_ops svm_x86_ops__initdata = { .name =KBUILD_MODNAME ,
@@ -5031,6 +5041,7 @@ static struct kvm_x86_ops svm_x86_ops __initdata = { .vcpu_deliver_sipi_vector = svm_vcpu_deliver_sipi_vector, .vcpu_get_apicv_inhibit_reasons = avic_vcpu_get_apicv_inhibit_reasons, + .alloc_apic_backing_page = svm_alloc_apic_backing_page, };/*
arch/x86/kvm/svm/svm.h +1 -0
@@ -694,6 +694,7 @@ void sev_es_vcpu_reset(struct vcpu_svm *svm);void sev_vcpu_deliver_sipi_vector (struct kvm_vcpu *vcpu,u8 vector);void sev_es_prepare_switch_to_guest (struct sev_es_save_area *hostsa);void sev_es_unmap_ghcb (struct vcpu_svm *svm); +struct page *snp_safe_alloc_page (struct kvm_vcpu *vcpu);/* vmenter.S */
PATCH v2 22/25 x86/cpufeatures: Enable/unmask SEV-SNP CPU feature
In reply to Michael RothWith all the required host changes in place, it should now be possible to initialize SNP-related MSR bits, set up RMP table enforcement, and initialize SNP support in firmware while maintaining legacy support for SEV/SEV-ES guests. Go ahead and enable the SNP feature now.
--- arch/x86/include/asm/disabled-features.h | 4 ++++ 1 file changed, 4 insertions(+)
arch/x86/include/asm/disabled-features.h +4 -0
@@ -117,7 +117,11 @@#define DISABLE_IBT (1 << (X86_FEATURE_IBT & 31))#endif +#ifdef CONFIG_KVM_AMD_SEV +#define DISABLE_SEV_SNP 0 +#else #define DISABLE_SEV_SNP (1 << (X86_FEATURE_SEV_SNP & 31)) +#endif /* * Make sure to add features to the correct mask
Signature · 1 line
2.25.1
[tip: x86/sev] x86/cpufeatures: Enable/unmask SEV-SNP CPU feature
The following commit has been merged into the x86/sev branch of tip:
The following commit has been merged into the x86/sev branch of tip:
Commit-ID: c3b86e61b75645276aa2565649a6da5d6e77030f Gitweb: https://git.kernel.org/tip/c3b86e61b75645276aa2565649a6da5d6e77030f Author: Michael Roth <[email protected]> AuthorDate: Thu, 25 Jan 2024 22:11:22 -06:00 Committer: Borislav Petkov (AMD) <[email protected]> CommitterDate: Mon, 29 Jan 2024 20:34:19 +01:00
x86/cpufeatures: Enable/unmask SEV-SNP CPU feature
With all the required host changes in place, it should now be possible to initialize SNP-related MSR bits, set up RMP table enforcement, and initialize SNP support in firmware while maintaining legacy support for SEV/SEV-ES guests. Go ahead and enable the SNP feature now.
--- arch/x86/include/asm/disabled-features.h | 4 ++++ 1 file changed, 4 insertions(+)
arch/x86/include/asm/disabled-features.h +4 -0
@@ -117,7 +117,11 @@#define DISABLE_IBT (1 << (X86_FEATURE_IBT & 31))#endif +#ifdef CONFIG_KVM_AMD_SEV +#define DISABLE_SEV_SNP 0 +#else #define DISABLE_SEV_SNP (1 << (X86_FEATURE_SEV_SNP & 31)) +#endif /* * Make sure to add features to the correct mask
PATCH v2 23/25 crypto: ccp: Add the SNP_PLATFORM_STATUS command
In reply to Michael RothFrom: Brijesh Singh <[email protected]>
This command is used to query the SNP platform status. See the SEV-SNP spec for more details.
--- Documentation/virt/coco/sev-guest.rst | 27 ++++++++++++++ drivers/crypto/ccp/sev-dev.c | 52 +++++++++++++++++++++++++++ include/uapi/linux/psp-sev.h | 1 + 3 files changed, 80 insertions(+)
Documentation/virt/coco/sev-guest.rst +27 -0
@@ -67,6 +67,22 @@ counter (e.g. counter overflow), then -EIO will be returned. }; }; +The host ioctls are issued to a file descriptor of the /dev/sev device. +The ioctl accepts the command ID/input structure documented below. + +:: + struct sev_issue_cmd { + /* Command ID */ + __u32 cmd; + + /* Command request structure */ + __u64 data; + + /* Firmware error code on failure (see psp-sev.h) */ + __u32 error; + }; + + 2.1 SNP_GET_REPORT ------------------
@@ -124,6 +140,17 @@ be updated with the expected value. See GHCB specification for further detail on how to parse the certificate blob. +2.4 SNP_PLATFORM_STATUS +----------------------- +:Technology: sev-snp +:Type: hypervisor ioctl cmd +:Parameters (out): struct sev_user_data_snp_status +:Returns (out): 0 on success, -negative on error + +The SNP_PLATFORM_STATUS command is used to query the SNP platform status. The +status includes API major, minor version and more. See the SEV-SNP +specification for further details. + 3. SEV-SNP CPUID Enforcement ============================
drivers/crypto/ccp/sev-dev.c +52 -0
64 lines not on this page · raw message
include/uapi/linux/psp-sev.h +1 -0
@@ -28,6 +28,7 @@ enum {SEV_PEK_CERT_IMPORT ,SEV_GET_ID ,/* This command is deprecated, use SEV_GET_ID2 */ SEV_GET_ID2 , +SNP_PLATFORM_STATUS , SEV_MAX, };
Signature · 1 line
2.25.1
[tip: x86/sev] crypto: ccp: Add the SNP_PLATFORM_STATUS command
The following commit has been merged into the x86/sev branch of tip:
The following commit has been merged into the x86/sev branch of tip:
Commit-ID: f5db8841ebe59dbdf07fda797c88ccb51e0c893d Gitweb: https://git.kernel.org/tip/f5db8841ebe59dbdf07fda797c88ccb51e0c893d Author: Brijesh Singh <[email protected]> AuthorDate: Thu, 25 Jan 2024 22:11:23 -06:00 Committer: Borislav Petkov (AMD) <[email protected]> CommitterDate: Mon, 29 Jan 2024 20:34:19 +01:00
crypto: ccp: Add the SNP_PLATFORM_STATUS command
This command is used to query the SNP platform status. See the SEV-SNP spec for more details.
--- Documentation/virt/coco/sev-guest.rst | 27 ++++++++++++++- drivers/crypto/ccp/sev-dev.c | 52 ++++++++++++++++++++++++++- include/uapi/linux/psp-sev.h | 1 +- 3 files changed, 80 insertions(+)
Documentation/virt/coco/sev-guest.rst +27 -0
@@ -67,6 +67,22 @@ counter (e.g. counter overflow), then -EIO will be returned. }; }; +The host ioctls are issued to a file descriptor of the /dev/sev device. +The ioctl accepts the command ID/input structure documented below. + +:: + struct sev_issue_cmd { + /* Command ID */ + __u32 cmd; + + /* Command request structure */ + __u64 data; + + /* Firmware error code on failure (see psp-sev.h) */ + __u32 error; + }; + + 2.1 SNP_GET_REPORT ------------------
@@ -124,6 +140,17 @@ be updated with the expected value. See GHCB specification for further detail on how to parse the certificate blob. +2.4 SNP_PLATFORM_STATUS +----------------------- +:Technology: sev-snp +:Type: hypervisor ioctl cmd +:Parameters (out): struct sev_user_data_snp_status +:Returns (out): 0 on success, -negative on error + +The SNP_PLATFORM_STATUS command is used to query the SNP platform status. The +status includes API major, minor version and more. See the SEV-SNP +specification for further details. + 3. SEV-SNP CPUID Enforcement ============================
drivers/crypto/ccp/sev-dev.c +52 -0
64 lines not on this page · raw message
include/uapi/linux/psp-sev.h +1 -0
@@ -28,6 +28,7 @@ enum {SEV_PEK_CERT_IMPORT ,SEV_GET_ID ,/* This command is deprecated, use SEV_GET_ID2 */ SEV_GET_ID2 , +SNP_PLATFORM_STATUS , SEV_MAX, };
PATCH v2 24/25 crypto: ccp: Add the SNP_COMMIT command
In reply to Michael RothFrom: Tom Lendacky <[email protected]>
The SNP_COMMIT command is used to commit the currently installed version of the SEV firmware. Once committed, the firmware cannot be replaced with a previous firmware version (cannot be rolled back). This command will also update the reported TCB to match that of the currently installed firmware.
--- Documentation/virt/coco/sev-guest.rst | 11 +++++++++++ drivers/crypto/ccp/sev-dev.c | 17 +++++++++++++++++ include/linux/psp-sev.h | 9 +++++++++ include/uapi/linux/psp-sev.h | 1 + 4 files changed, 38 insertions(+)
Documentation/virt/coco/sev-guest.rst +11 -0
@@ -151,6 +151,17 @@ The SNP_PLATFORM_STATUS command is used to query the SNP platform status. The status includes API major, minor version and more. See the SEV-SNP specification for further details. +2.5 SNP_COMMIT +-------------- +:Technology: sev-snp +:Type: hypervisor ioctl cmd +:Returns (out): 0 on success, -negative on error + +SNP_COMMIT is used to commit the currently installed firmware using the +SEV-SNP firmware SNP_COMMIT command. This prevents roll-back to a previously +committed firmware version. This will also update the reported TCB to match +that of the currently installed firmware. + 3. SEV-SNP CPUID Enforcement ============================
drivers/crypto/ccp/sev-dev.c +17 -0
@@ -222,6 +222,7 @@ static int sev_cmd_buffer_len(int cmd)case SEV_CMD_SNP_PLATFORM_STATUS :return sizeof (struct sev_data_snp_addr );case SEV_CMD_SNP_GUEST_REQUEST :return sizeof (struct sev_data_snp_guest_request );case SEV_CMD_SNP_CONFIG :return sizeof (struct sev_user_data_snp_config ); +case SEV_CMD_SNP_COMMIT :return sizeof (struct sev_data_snp_commit );default :return 0 ; }
@@ -1968,6 +1969,19 @@ static int sev_ioctl_do_snp_platform_status(struct sev_issue_cmd *argp)return ret; } +static int sev_ioctl_do_snp_commit (struct sev_issue_cmd *argp) +{ +struct sev_device *sev = psp_master->sev_data; +struct sev_data_snp_commit buf; + +if (!sev->snp_initialized) +return -EINVAL ; + + buf.len =sizeof (buf); + +return __sev_do_cmd_locked (SEV_CMD_SNP_COMMIT , &buf, &argp->error); +} +static long sev_ioctl (struct file *file,unsigned int ioctl,unsigned long arg) {void __user *argp = (void __user *)arg;
@@ -2022,6 +2036,9 @@ static long sev_ioctl(struct file *file, unsigned int ioctl, unsigned long arg)case SNP_PLATFORM_STATUS : ret =sev_ioctl_do_snp_platform_status (&input);break ; +case SNP_COMMIT : + ret =sev_ioctl_do_snp_commit (&input); +break ;default : ret = -EINVAL ; goto out;
include/linux/psp-sev.h +9 -0
@@ -788,6 +788,15 @@ struct sev_data_snp_shutdown_ex { u32 rsvd1:31; } __packed; +/** +* struct sev_data_snp_commit - SNP_COMMIT structure +* +* @len: length of the command buffer read by the PSP +*/ +struct sev_data_snp_commit { +u32 len; +} __packed; +#ifdef CONFIG_CRYPTO_DEV_SP_PSP /**
include/uapi/linux/psp-sev.h +1 -0
@@ -29,6 +29,7 @@ enum {SEV_GET_ID ,/* This command is deprecated, use SEV_GET_ID2 */ SEV_GET_ID2 ,SNP_PLATFORM_STATUS , +SNP_COMMIT , SEV_MAX, };
Signature · 1 line
2.25.1
[tip: x86/sev] crypto: ccp: Add the SNP_COMMIT command
The following commit has been merged into the x86/sev branch of tip:
The following commit has been merged into the x86/sev branch of tip:
Commit-ID: fad133c79afa02344d05001324a0474e20f3e055 Gitweb: https://git.kernel.org/tip/fad133c79afa02344d05001324a0474e20f3e055 Author: Tom Lendacky <[email protected]> AuthorDate: Thu, 25 Jan 2024 22:11:24 -06:00 Committer: Borislav Petkov (AMD) <[email protected]> CommitterDate: Mon, 29 Jan 2024 20:34:19 +01:00
crypto: ccp: Add the SNP_COMMIT command
The SNP_COMMIT command is used to commit the currently installed version of the SEV firmware. Once committed, the firmware cannot be replaced with a previous firmware version (cannot be rolled back). This command will also update the reported TCB to match that of the currently installed firmware.
[ mdr: Note the reported TCB update in the documentation/commit. ]
--- Documentation/virt/coco/sev-guest.rst | 11 +++++++++++ drivers/crypto/ccp/sev-dev.c | 17 +++++++++++++++++ include/linux/psp-sev.h | 9 +++++++++ include/uapi/linux/psp-sev.h | 1 + 4 files changed, 38 insertions(+)
Documentation/virt/coco/sev-guest.rst +11 -0
@@ -151,6 +151,17 @@ The SNP_PLATFORM_STATUS command is used to query the SNP platform status. The status includes API major, minor version and more. See the SEV-SNP specification for further details. +2.5 SNP_COMMIT +-------------- +:Technology: sev-snp +:Type: hypervisor ioctl cmd +:Returns (out): 0 on success, -negative on error + +SNP_COMMIT is used to commit the currently installed firmware using the +SEV-SNP firmware SNP_COMMIT command. This prevents roll-back to a previously +committed firmware version. This will also update the reported TCB to match +that of the currently installed firmware. + 3. SEV-SNP CPUID Enforcement ============================
drivers/crypto/ccp/sev-dev.c +17 -0
@@ -222,6 +222,7 @@ static int sev_cmd_buffer_len(int cmd)case SEV_CMD_SNP_PLATFORM_STATUS :return sizeof (struct sev_data_snp_addr );case SEV_CMD_SNP_GUEST_REQUEST :return sizeof (struct sev_data_snp_guest_request );case SEV_CMD_SNP_CONFIG :return sizeof (struct sev_user_data_snp_config ); +case SEV_CMD_SNP_COMMIT :return sizeof (struct sev_data_snp_commit );default :return 0 ; }
@@ -1990,6 +1991,19 @@ cleanup:return ret; } +static int sev_ioctl_do_snp_commit (struct sev_issue_cmd *argp) +{ +struct sev_device *sev = psp_master->sev_data; +struct sev_data_snp_commit buf; + +if (!sev->snp_initialized) +return -EINVAL ; + + buf.len =sizeof (buf); + +return __sev_do_cmd_locked (SEV_CMD_SNP_COMMIT , &buf, &argp->error); +} +static long sev_ioctl (struct file *file,unsigned int ioctl,unsigned long arg) {void __user *argp = (void __user *)arg;
@@ -2044,6 +2058,9 @@ static long sev_ioctl(struct file *file, unsigned int ioctl, unsigned long arg)case SNP_PLATFORM_STATUS : ret =sev_ioctl_do_snp_platform_status (&input);break ; +case SNP_COMMIT : + ret =sev_ioctl_do_snp_commit (&input); +break ;default : ret = -EINVAL ; goto out;
include/linux/psp-sev.h +9 -0
@@ -801,6 +801,15 @@ struct sev_platform_init_args {bool probe; }; +/** +* struct sev_data_snp_commit - SNP_COMMIT structure +* +* @len: length of the command buffer read by the PSP +*/ +struct sev_data_snp_commit { +u32 len; +} __packed; +#ifdef CONFIG_CRYPTO_DEV_SP_PSP /**
include/uapi/linux/psp-sev.h +1 -0
@@ -29,6 +29,7 @@ enum {SEV_GET_ID ,/* This command is deprecated, use SEV_GET_ID2 */ SEV_GET_ID2 ,SNP_PLATFORM_STATUS , +SNP_COMMIT , SEV_MAX, };
PATCH v2 25/25 crypto: ccp: Add the SNP_SET_CONFIG command
In reply to Michael RothFrom: Brijesh Singh <[email protected]>
The SEV-SNP firmware provides the SNP_CONFIG command used to set various system-wide configuration values for SNP guests, such as the reported TCB version used when signing guest attestation reports. Add an interface to set this via userspace.
[mdr: squash in doc patch from Dionna, drop extended request/certificate
handling and simplify this to a simple wrapper around SNP_CONFIG fw cmd]
--- Documentation/virt/coco/sev-guest.rst | 13 +++++++++++++ drivers/crypto/ccp/sev-dev.c | 20 ++++++++++++++++++++ include/uapi/linux/psp-sev.h | 1 + 3 files changed, 34 insertions(+)
Documentation/virt/coco/sev-guest.rst +13 -0
@@ -162,6 +162,19 @@ SEV-SNP firmware SNP_COMMIT command. This prevents roll-back to a previously committed firmware version. This will also update the reported TCB to match that of the currently installed firmware. +2.6 SNP_SET_CONFIG +------------------ +:Technology: sev-snp +:Type: hypervisor ioctl cmd +:Parameters (in): struct sev_user_data_snp_config +:Returns (out): 0 on success, -negative on error + +SNP_SET_CONFIG is used to set the system-wide configuration such as +reported TCB version in the attestation report. The command is similar +to SNP_CONFIG command defined in the SEV-SNP spec. The current values of +the firmware parameters affected by this command can be queried via +SNP_PLATFORM_STATUS. + 3. SEV-SNP CPUID Enforcement ============================
drivers/crypto/ccp/sev-dev.c +20 -0
@@ -1982,6 +1982,23 @@ static int sev_ioctl_do_snp_commit(struct sev_issue_cmd *argp)return __sev_do_cmd_locked (SEV_CMD_SNP_COMMIT , &buf, &argp->error); } +static int sev_ioctl_do_snp_set_config (struct sev_issue_cmd *argp,bool writable) +{ +struct sev_device *sev = psp_master->sev_data; +struct sev_user_data_snp_config config; + +if (!sev->snp_initialized || !argp->data) +return -EINVAL ; + +if (!writable) +return -EPERM ; + +if (copy_from_user (&config, (void __user *)argp->data,sizeof (config))) +return -EFAULT ; + +return __sev_do_cmd_locked (SEV_CMD_SNP_CONFIG , &config, &argp->error); +} +static long sev_ioctl (struct file *file,unsigned int ioctl,unsigned long arg) {void __user *argp = (void __user *)arg;
@@ -2039,6 +2056,9 @@ static long sev_ioctl(struct file *file, unsigned int ioctl, unsigned long arg)case SNP_COMMIT : ret =sev_ioctl_do_snp_commit (&input);break ; +case SNP_SET_CONFIG : + ret =sev_ioctl_do_snp_set_config (&input, writable); +break ;default : ret = -EINVAL ; goto out;
include/uapi/linux/psp-sev.h +1 -0
@@ -30,6 +30,7 @@ enum {SEV_GET_ID2 ,SNP_PLATFORM_STATUS ,SNP_COMMIT , +SNP_SET_CONFIG , SEV_MAX, };
Signature · 1 line
2.25.1
Re: [PATCH v2 25/25] crypto: ccp: Add the SNP_SET_CONFIG command
Quoted 52 lines · Michael Roth
From: Brijesh Singh <[email protected]>
The SEV-SNP firmware provides the
SNP_CONFIGcommand used to set various system-wide configuration values for SNP guests, such as the reported TCB version used when signing guest attestation reports. Add an interface to set this via userspace.Signed-off-by Brijesh Singh <[email protected]>Co-developed-by Alexey Kardashevskiy <[email protected]>Signed-off-by Alexey Kardashevskiy <[email protected]>Co-developed-by Dionna Glaze <[email protected]>Signed-off-by Dionna Glaze <[email protected]>Signed-off-by Ashish Kalra <[email protected]>[mdr: squash in doc patch from Dionna, drop extended request/certificate
handling and simplify this to a simple wrapper aroundSNP_CONFIGfw cmd]Signed-off-by Michael Roth <[email protected]>--- Documentation/virt/coco/sev-guest.rst | 13 +++++++++++++ drivers/crypto/ccp/sev-dev.c | 20 ++++++++++++++++++++ include/uapi/linux/psp-sev.h | 1 + 3 files changed, 34 insertions(+)Documentation/virt/coco/sev-guest.rst +13
-0@@ -162,6 +162,19 @@ SEV-SNP firmware SNP_COMMIT command. This prevents roll-back to a previously committed firmware version. This will also update the reported TCB to match that of the currently installed firmware. +2.6 SNP_SET_CONFIG +------------------ +:Technology: sev-snp +:Type: hypervisor ioctl cmd +:Parameters (in): struct sev_user_data_snp_config +:Returns (out): 0 on success, -negative on error + +SNP_SET_CONFIG is used to set the system-wide configuration such as +reported TCB version in the attestation report. The command is similar +to SNP_CONFIG command defined in the SEV-SNP spec. The current values of +the firmware parameters affected by this command can be queried via +SNP_PLATFORM_STATUS. + 3. SEV-SNP CPUID Enforcement ============================drivers/crypto/ccp/sev-dev.c +0
-0
drivers/crypto/ccp/sev-dev.c static int sev_ioctl_do_snp_commit(struct sev_issue_cmd *argp) · line 1987
Quoted 6 lines
drivers/crypto/ccp/sev-dev.c +3
-0@@ -1982,6 +1982,23 @@ static int sev_ioctl_do_snp_commit(struct sev_issue_cmd *argp)return __sev_do_cmd_locked (SEV_CMD_SNP_COMMIT , &buf, &argp->error); } +static int sev_ioctl_do_snp_set_config (struct sev_issue_cmd *argp,bool writable) +{ +struct sev_device *sev = psp_master->sev_data;
Should this check that psp_master is not NULL? Like the fix for
https://lore.kernel.org/all/[email protected]/
Otherwise,
Quoted 39 lines
drivers/crypto/ccp/sev-dev.c +17
-0+struct sev_user_data_snp_config config; + +if (!sev->snp_initialized || !argp->data) +return -EINVAL ; + +if (!writable) +return -EPERM ; + +if (copy_from_user (&config, (void __user *)argp->data,sizeof (config))) +return -EFAULT ; + +return __sev_do_cmd_locked (SEV_CMD_SNP_CONFIG , &config, &argp->error); +} +static long sev_ioctl (struct file *file,unsigned int ioctl,unsigned long arg) {void __user *argp = (void __user *)arg;@@ -2039,6 +2056,9 @@ static long sev_ioctl(struct file *file, unsigned int ioctl, unsigned long arg)case SNP_COMMIT : ret =sev_ioctl_do_snp_commit (&input);break ; +case SNP_SET_CONFIG : + ret =sev_ioctl_do_snp_set_config (&input, writable); +break ;default : ret = -EINVAL ; goto out;include/uapi/linux/psp-sev.h +1
-0@@ -30,6 +30,7 @@ enum {SEV_GET_ID2 ,SNP_PLATFORM_STATUS ,SNP_COMMIT , +SNP_SET_CONFIG , SEV_MAX, };
Re: [PATCH v2 25/25] crypto: ccp: Add the SNP_SET_CONFIG command
Quoted 56 lines · Liam Merwick
Quoted 55 lines · Michael Roth
From: Brijesh Singh <[email protected]>
The SEV-SNP firmware provides the
SNP_CONFIGcommand used to set various system-wide configuration values for SNP guests, such as the reported TCB version used when signing guest attestation reports. Add an interface to set this via userspace.Signed-off-by Brijesh Singh <[email protected]>Co-developed-by Alexey Kardashevskiy <[email protected]>Signed-off-by Alexey Kardashevskiy <[email protected]>Co-developed-by Dionna Glaze <[email protected]>Signed-off-by Dionna Glaze <[email protected]>Signed-off-by Ashish Kalra <[email protected]>[mdr: squash in doc patch from Dionna, drop extended request/certificate
handling and simplify this to a simple wrapper aroundSNP_CONFIGfw cmd]Signed-off-by Michael Roth <[email protected]>--- Documentation/virt/coco/sev-guest.rst | 13 +++++++++++++ drivers/crypto/ccp/sev-dev.c | 20 ++++++++++++++++++++ include/uapi/linux/psp-sev.h | 1 + 3 files changed, 34 insertions(+)Documentation/virt/coco/sev-guest.rst +13
-0@@ -162,6 +162,19 @@ SEV-SNP firmware SNP_COMMIT command. This prevents roll-back to a previously committed firmware version. This will also update the reported TCB to match that of the currently installed firmware. +2.6 SNP_SET_CONFIG +------------------ +:Technology: sev-snp +:Type: hypervisor ioctl cmd +:Parameters (in): struct sev_user_data_snp_config +:Returns (out): 0 on success, -negative on error + +SNP_SET_CONFIG is used to set the system-wide configuration such as +reported TCB version in the attestation report. The command is similar +to SNP_CONFIG command defined in the SEV-SNP spec. The current values of +the firmware parameters affected by this command can be queried via +SNP_PLATFORM_STATUS. + 3. SEV-SNP CPUID Enforcement ============================drivers/crypto/ccp/sev-dev.c +0
-0@@ -1982,6 +1982,23 @@ static int sev_ioctl_do_snp_commit(struct sev_issue_cmd *argp)return __sev_do_cmd_locked (SEV_CMD_SNP_COMMIT , &buf, &argp->error); }
Quoted 6 lines
drivers/crypto/ccp/sev-dev.c static int sev_ioctl_do_snp_commit(struct sev_issue_cmd *argp) · line 1987
Quoted 3 lines
drivers/crypto/ccp/sev-dev.c +3
-0+static int sev_ioctl_do_snp_set_config (struct sev_issue_cmd *argp,bool writable) +{ +struct sev_device *sev = psp_master->sev_data;Should this check that
psp_masteris not NULL? Like the fix for
https://lore.kernel.org/all/[email protected]/
It wouldn't hurt, but sev_ioctl() will check for it before getting here, so I don't think it's a reachable condition.
There's a number of spots where existing SEV/legacy code relies on psp_pci_init() always setting psp_master before sev_pci_init(). Kim's patch fixes 1 exception that's reachable if you synthetically force a driver remove via DEBUG_TEST_DRIVER_REMOVE before the psp_pci_init() call is made during probe, which generally wouldn't happen in practice.
But there may be other cases like the one Kim hit where "not work" isn't handled gracefully. Fortunately the other obvious spot, sev_do_cmd(), also checks for !psp_master, so that covers most of the internal users of sev-dev.c.
But a general cleanup to introduce a helper like get_sev_device() that handles !psp_master might be a nice follow-up to make things more robust.
-Mike
Quoted 44 lines
Otherwise,
Reviewed-by Liam Merwick <[email protected]>Quoted 39 lines
drivers/crypto/ccp/sev-dev.c +17
-0+struct sev_user_data_snp_config config; + +if (!sev->snp_initialized || !argp->data) +return -EINVAL ; + +if (!writable) +return -EPERM ; + +if (copy_from_user (&config, (void __user *)argp->data,sizeof (config))) +return -EFAULT ; + +return __sev_do_cmd_locked (SEV_CMD_SNP_CONFIG , &config, &argp->error); +} +static long sev_ioctl (struct file *file,unsigned int ioctl,unsigned long arg) {void __user *argp = (void __user *)arg;@@ -2039,6 +2056,9 @@ static long sev_ioctl(struct file *file, unsigned int ioctl, unsigned long arg)case SNP_COMMIT : ret =sev_ioctl_do_snp_commit (&input);break ; +case SNP_SET_CONFIG : + ret =sev_ioctl_do_snp_set_config (&input, writable); +break ;default : ret = -EINVAL ; goto out;include/uapi/linux/psp-sev.h +1
-0@@ -30,6 +30,7 @@ enum {SEV_GET_ID2 ,SNP_PLATFORM_STATUS ,SNP_COMMIT , +SNP_SET_CONFIG , SEV_MAX, };
[tip: x86/sev] crypto: ccp: Add the SNP_SET_CONFIG command
In reply to Michael RothThe following commit has been merged into the x86/sev branch of tip:
The following commit has been merged into the x86/sev branch of tip:
Commit-ID: cb645fe478eaad32b6168059bb6b584295af863e Gitweb: https://git.kernel.org/tip/cb645fe478eaad32b6168059bb6b584295af863e Author: Brijesh Singh <[email protected]> AuthorDate: Thu, 25 Jan 2024 22:11:25 -06:00 Committer: Borislav Petkov (AMD) <[email protected]> CommitterDate: Mon, 29 Jan 2024 20:34:19 +01:00
crypto: ccp: Add the SNP_SET_CONFIG command
The SEV-SNP firmware provides the SNP_CONFIG command used to set various system-wide configuration values for SNP guests, such as the reported TCB version used when signing guest attestation reports. Add an interface to set this via userspace.
[ mdr: Squash in doc patch from Dionna, drop extended request/
certificate handling and simplify this to a simple wrapper around SNP_CONFIG fw cmd. ]
--- Documentation/virt/coco/sev-guest.rst | 13 +++++++++++++ drivers/crypto/ccp/sev-dev.c | 20 ++++++++++++++++++++ include/uapi/linux/psp-sev.h | 1 + 3 files changed, 34 insertions(+)
Documentation/virt/coco/sev-guest.rst +13 -0
@@ -162,6 +162,19 @@ SEV-SNP firmware SNP_COMMIT command. This prevents roll-back to a previously committed firmware version. This will also update the reported TCB to match that of the currently installed firmware. +2.6 SNP_SET_CONFIG +------------------ +:Technology: sev-snp +:Type: hypervisor ioctl cmd +:Parameters (in): struct sev_user_data_snp_config +:Returns (out): 0 on success, -negative on error + +SNP_SET_CONFIG is used to set the system-wide configuration such as +reported TCB version in the attestation report. The command is similar +to SNP_CONFIG command defined in the SEV-SNP spec. The current values of +the firmware parameters affected by this command can be queried via +SNP_PLATFORM_STATUS. + 3. SEV-SNP CPUID Enforcement ============================
drivers/crypto/ccp/sev-dev.c +20 -0
@@ -2004,6 +2004,23 @@ static int sev_ioctl_do_snp_commit(struct sev_issue_cmd *argp)return __sev_do_cmd_locked (SEV_CMD_SNP_COMMIT , &buf, &argp->error); } +static int sev_ioctl_do_snp_set_config (struct sev_issue_cmd *argp,bool writable) +{ +struct sev_device *sev = psp_master->sev_data; +struct sev_user_data_snp_config config; + +if (!sev->snp_initialized || !argp->data) +return -EINVAL ; + +if (!writable) +return -EPERM ; + +if (copy_from_user (&config, (void __user *)argp->data,sizeof (config))) +return -EFAULT ; + +return __sev_do_cmd_locked (SEV_CMD_SNP_CONFIG , &config, &argp->error); +} +static long sev_ioctl (struct file *file,unsigned int ioctl,unsigned long arg) {void __user *argp = (void __user *)arg;
@@ -2061,6 +2078,9 @@ static long sev_ioctl(struct file *file, unsigned int ioctl, unsigned long arg)case SNP_COMMIT : ret =sev_ioctl_do_snp_commit (&input);break ; +case SNP_SET_CONFIG : + ret =sev_ioctl_do_snp_set_config (&input, writable); +break ;default : ret = -EINVAL ; goto out;
include/uapi/linux/psp-sev.h +1 -0
@@ -30,6 +30,7 @@ enum {SEV_GET_ID2 ,SNP_PLATFORM_STATUS ,SNP_COMMIT , +SNP_SET_CONFIG , SEV_MAX, };
Borislav Petkov
maintainerQuoted 5 lines · Michael Roth
This patchset is also available at:
https://github.com/amdese/linux/commits/snp-host-init-v2
and is based on top of linux-next tag next-20240125
Ok, I've queued this now after some testing.
As of now
https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/log/?h=x86/sev
is frozen and fixes should come ontop so that Paolo/Sean can use that branch to base the remaining SNP host stuff ontop.
Thx.
Signature · 4 lines
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette