| Message ID | c6c38d6253dc78381f9ff0f1823b6ee5ddeefacc.1628873970.git.thomas.lendacky@amd.com (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
| Series | Implement generic prot_guest_has() helper function | expand |
On Fri, Aug 13, 2021 at 11:59:24AM -0500, Tom Lendacky wrote: > diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c > index edc67ddf065d..5635ca9a1fbe 100644 > --- a/arch/x86/mm/mem_encrypt.c > +++ b/arch/x86/mm/mem_encrypt.c > @@ -144,7 +144,7 @@ void __init sme_unmap_bootdata(char *real_mode_data) > struct boot_params *boot_data; > unsigned long cmdline_paddr; > > - if (!sme_active()) > + if (!amd_prot_guest_has(PATTR_SME)) > return; > > /* Get the command line address before unmapping the real_mode_data */ > @@ -164,7 +164,7 @@ void __init sme_map_bootdata(char *real_mode_data) > struct boot_params *boot_data; > unsigned long cmdline_paddr; > > - if (!sme_active()) > + if (!amd_prot_guest_has(PATTR_SME)) > return; > > __sme_early_map_unmap_mem(real_mode_data, sizeof(boot_params), true); > @@ -378,7 +378,7 @@ bool sev_active(void) > return sev_status & MSR_AMD64_SEV_ENABLED; > } > > -bool sme_active(void) > +static bool sme_active(void) Just get rid of it altogether. Also, there's an EXPORT_SYMBOL_GPL(sev_active); which needs to go under the actual function. Here's a diff ontop: --- diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c index 5635ca9a1fbe..a3a2396362a5 100644 --- a/arch/x86/mm/mem_encrypt.c +++ b/arch/x86/mm/mem_encrypt.c @@ -364,8 +364,9 @@ int __init early_set_memory_encrypted(unsigned long vaddr, unsigned long size) /* * SME and SEV are very similar but they are not the same, so there are * times that the kernel will need to distinguish between SME and SEV. The - * sme_active() and sev_active() functions are used for this. When a - * distinction isn't needed, the mem_encrypt_active() function can be used. + * PATTR_HOST_MEM_ENCRYPT and PATTR_GUEST_MEM_ENCRYPT flags to + * amd_prot_guest_has() are used for this. When a distinction isn't needed, + * the mem_encrypt_active() function can be used. * * The trampoline code is a good example for this requirement. Before * paging is activated, SME will access all memory as decrypted, but SEV @@ -377,11 +378,6 @@ bool sev_active(void) { return sev_status & MSR_AMD64_SEV_ENABLED; } - -static bool sme_active(void) -{ - return sme_me_mask && !sev_active(); -} EXPORT_SYMBOL_GPL(sev_active); /* Needs to be called from non-instrumentable code */ @@ -398,7 +394,7 @@ bool amd_prot_guest_has(unsigned int attr) case PATTR_SME: case PATTR_HOST_MEM_ENCRYPT: - return sme_active(); + return sme_me_mask && !sev_active(); case PATTR_SEV: case PATTR_GUEST_MEM_ENCRYPT: > { > return sme_me_mask && !sev_active(); > } > @@ -428,7 +428,7 @@ bool force_dma_unencrypted(struct device *dev) > * device does not support DMA to addresses that include the > * encryption mask. > */ > - if (sme_active()) { > + if (amd_prot_guest_has(PATTR_SME)) { So I'm not sure: you add PATTR_SME which you call with amd_prot_guest_has() and PATTR_HOST_MEM_ENCRYPT which you call with prot_guest_has() and they both end up being the same thing on AMD. So why even bother with PATTR_SME? This is only going to cause confusion later and I'd say let's simply use prot_guest_has(PATTR_HOST_MEM_ENCRYPT) everywhere...
On 8/17/21 4:00 AM, Borislav Petkov wrote: > On Fri, Aug 13, 2021 at 11:59:24AM -0500, Tom Lendacky wrote: >> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c >> index edc67ddf065d..5635ca9a1fbe 100644 >> --- a/arch/x86/mm/mem_encrypt.c >> +++ b/arch/x86/mm/mem_encrypt.c >> @@ -144,7 +144,7 @@ void __init sme_unmap_bootdata(char *real_mode_data) >> struct boot_params *boot_data; >> unsigned long cmdline_paddr; >> >> - if (!sme_active()) >> + if (!amd_prot_guest_has(PATTR_SME)) >> return; >> >> /* Get the command line address before unmapping the real_mode_data */ >> @@ -164,7 +164,7 @@ void __init sme_map_bootdata(char *real_mode_data) >> struct boot_params *boot_data; >> unsigned long cmdline_paddr; >> >> - if (!sme_active()) >> + if (!amd_prot_guest_has(PATTR_SME)) >> return; >> >> __sme_early_map_unmap_mem(real_mode_data, sizeof(boot_params), true); >> @@ -378,7 +378,7 @@ bool sev_active(void) >> return sev_status & MSR_AMD64_SEV_ENABLED; >> } >> >> -bool sme_active(void) >> +static bool sme_active(void) > > Just get rid of it altogether. Also, there's an > > EXPORT_SYMBOL_GPL(sev_active); > > which needs to go under the actual function. Here's a diff ontop: Will do. > > --- > diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c > index 5635ca9a1fbe..a3a2396362a5 100644 > --- a/arch/x86/mm/mem_encrypt.c > +++ b/arch/x86/mm/mem_encrypt.c > @@ -364,8 +364,9 @@ int __init early_set_memory_encrypted(unsigned long vaddr, unsigned long size) > /* > * SME and SEV are very similar but they are not the same, so there are > * times that the kernel will need to distinguish between SME and SEV. The > - * sme_active() and sev_active() functions are used for this. When a > - * distinction isn't needed, the mem_encrypt_active() function can be used. > + * PATTR_HOST_MEM_ENCRYPT and PATTR_GUEST_MEM_ENCRYPT flags to > + * amd_prot_guest_has() are used for this. When a distinction isn't needed, > + * the mem_encrypt_active() function can be used. > * > * The trampoline code is a good example for this requirement. Before > * paging is activated, SME will access all memory as decrypted, but SEV > @@ -377,11 +378,6 @@ bool sev_active(void) > { > return sev_status & MSR_AMD64_SEV_ENABLED; > } > - > -static bool sme_active(void) > -{ > - return sme_me_mask && !sev_active(); > -} > EXPORT_SYMBOL_GPL(sev_active); > > /* Needs to be called from non-instrumentable code */ > @@ -398,7 +394,7 @@ bool amd_prot_guest_has(unsigned int attr) > > case PATTR_SME: > case PATTR_HOST_MEM_ENCRYPT: > - return sme_active(); > + return sme_me_mask && !sev_active(); > > case PATTR_SEV: > case PATTR_GUEST_MEM_ENCRYPT: > >> { >> return sme_me_mask && !sev_active(); >> } >> @@ -428,7 +428,7 @@ bool force_dma_unencrypted(struct device *dev) >> * device does not support DMA to addresses that include the >> * encryption mask. >> */ >> - if (sme_active()) { >> + if (amd_prot_guest_has(PATTR_SME)) { > > So I'm not sure: you add PATTR_SME which you call with > amd_prot_guest_has() and PATTR_HOST_MEM_ENCRYPT which you call with > prot_guest_has() and they both end up being the same thing on AMD. > > So why even bother with PATTR_SME? > > This is only going to cause confusion later and I'd say let's simply use > prot_guest_has(PATTR_HOST_MEM_ENCRYPT) everywhere... Ok, I can do that. I was trying to ensure that anything that is truly SME or SEV specific would be called out now. I'm ok with letting the TDX folks make changes to these calls to be SME or SEV specific, if necessary, later. Thanks, Tom >
On Tue, Aug 17, 2021 at 09:46:58AM -0500, Tom Lendacky wrote: > I'm ok with letting the TDX folks make changes to these calls to be SME or > SEV specific, if necessary, later. Yap, exactly. Let's add the specific stuff only when really needed. Thx.
diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h index 0a6e34b07017..11b7c06e2828 100644 --- a/arch/x86/include/asm/kexec.h +++ b/arch/x86/include/asm/kexec.h @@ -129,7 +129,7 @@ relocate_kernel(unsigned long indirection_page, unsigned long page_list, unsigned long start_address, unsigned int preserve_context, - unsigned int sme_active); + unsigned int host_mem_enc_active); #endif #define ARCH_HAS_KIMAGE_ARCH diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h index a46d47662772..956338406cec 100644 --- a/arch/x86/include/asm/mem_encrypt.h +++ b/arch/x86/include/asm/mem_encrypt.h @@ -50,7 +50,6 @@ void __init mem_encrypt_free_decrypted_mem(void); void __init mem_encrypt_init(void); void __init sev_es_init_vc_handling(void); -bool sme_active(void); bool sev_active(void); bool sev_es_active(void); bool amd_prot_guest_has(unsigned int attr); @@ -76,7 +75,6 @@ static inline void __init sme_encrypt_kernel(struct boot_params *bp) { } static inline void __init sme_enable(struct boot_params *bp) { } static inline void sev_es_init_vc_handling(void) { } -static inline bool sme_active(void) { return false; } static inline bool sev_active(void) { return false; } static inline bool sev_es_active(void) { return false; } static inline bool amd_prot_guest_has(unsigned int attr) { return false; } diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c index 131f30fdcfbd..8e7b517ad738 100644 --- a/arch/x86/kernel/machine_kexec_64.c +++ b/arch/x86/kernel/machine_kexec_64.c @@ -17,6 +17,7 @@ #include <linux/suspend.h> #include <linux/vmalloc.h> #include <linux/efi.h> +#include <linux/protected_guest.h> #include <asm/init.h> #include <asm/tlbflush.h> @@ -358,7 +359,7 @@ void machine_kexec(struct kimage *image) (unsigned long)page_list, image->start, image->preserve_context, - sme_active()); + prot_guest_has(PATTR_HOST_MEM_ENCRYPT)); #ifdef CONFIG_KEXEC_JUMP if (image->preserve_context) diff --git a/arch/x86/kernel/pci-swiotlb.c b/arch/x86/kernel/pci-swiotlb.c index c2cfa5e7c152..bd9a9cfbc9a2 100644 --- a/arch/x86/kernel/pci-swiotlb.c +++ b/arch/x86/kernel/pci-swiotlb.c @@ -6,7 +6,7 @@ #include <linux/swiotlb.h> #include <linux/memblock.h> #include <linux/dma-direct.h> -#include <linux/mem_encrypt.h> +#include <linux/protected_guest.h> #include <asm/iommu.h> #include <asm/swiotlb.h> @@ -45,11 +45,10 @@ int __init pci_swiotlb_detect_4gb(void) swiotlb = 1; /* - * If SME is active then swiotlb will be set to 1 so that bounce - * buffers are allocated and used for devices that do not support - * the addressing range required for the encryption mask. + * Set swiotlb to 1 so that bounce buffers are allocated and used for + * devices that can't support DMA to encrypted memory. */ - if (sme_active()) + if (prot_guest_has(PATTR_HOST_MEM_ENCRYPT)) swiotlb = 1; return swiotlb; diff --git a/arch/x86/kernel/relocate_kernel_64.S b/arch/x86/kernel/relocate_kernel_64.S index c53271aebb64..c8fe74a28143 100644 --- a/arch/x86/kernel/relocate_kernel_64.S +++ b/arch/x86/kernel/relocate_kernel_64.S @@ -47,7 +47,7 @@ SYM_CODE_START_NOALIGN(relocate_kernel) * %rsi page_list * %rdx start address * %rcx preserve_context - * %r8 sme_active + * %r8 host_mem_enc_active */ /* Save the CPU context, used for jumping back */ diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c index ccff76cedd8f..583afd54c7e1 100644 --- a/arch/x86/mm/ioremap.c +++ b/arch/x86/mm/ioremap.c @@ -14,7 +14,7 @@ #include <linux/slab.h> #include <linux/vmalloc.h> #include <linux/mmiotrace.h> -#include <linux/mem_encrypt.h> +#include <linux/protected_guest.h> #include <linux/efi.h> #include <linux/pgtable.h> @@ -703,7 +703,7 @@ bool arch_memremap_can_ram_remap(resource_size_t phys_addr, unsigned long size, if (flags & MEMREMAP_DEC) return false; - if (sme_active()) { + if (prot_guest_has(PATTR_HOST_MEM_ENCRYPT)) { if (memremap_is_setup_data(phys_addr, size) || memremap_is_efi_data(phys_addr, size)) return false; @@ -729,7 +729,7 @@ pgprot_t __init early_memremap_pgprot_adjust(resource_size_t phys_addr, encrypted_prot = true; - if (sme_active()) { + if (prot_guest_has(PATTR_HOST_MEM_ENCRYPT)) { if (early_memremap_is_setup_data(phys_addr, size) || memremap_is_efi_data(phys_addr, size)) encrypted_prot = false; diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c index edc67ddf065d..5635ca9a1fbe 100644 --- a/arch/x86/mm/mem_encrypt.c +++ b/arch/x86/mm/mem_encrypt.c @@ -144,7 +144,7 @@ void __init sme_unmap_bootdata(char *real_mode_data) struct boot_params *boot_data; unsigned long cmdline_paddr; - if (!sme_active()) + if (!amd_prot_guest_has(PATTR_SME)) return; /* Get the command line address before unmapping the real_mode_data */ @@ -164,7 +164,7 @@ void __init sme_map_bootdata(char *real_mode_data) struct boot_params *boot_data; unsigned long cmdline_paddr; - if (!sme_active()) + if (!amd_prot_guest_has(PATTR_SME)) return; __sme_early_map_unmap_mem(real_mode_data, sizeof(boot_params), true); @@ -378,7 +378,7 @@ bool sev_active(void) return sev_status & MSR_AMD64_SEV_ENABLED; } -bool sme_active(void) +static bool sme_active(void) { return sme_me_mask && !sev_active(); } @@ -428,7 +428,7 @@ bool force_dma_unencrypted(struct device *dev) * device does not support DMA to addresses that include the * encryption mask. */ - if (sme_active()) { + if (amd_prot_guest_has(PATTR_SME)) { u64 dma_enc_mask = DMA_BIT_MASK(__ffs64(sme_me_mask)); u64 dma_dev_mask = min_not_zero(dev->coherent_dma_mask, dev->bus_dma_limit); @@ -469,7 +469,7 @@ static void print_mem_encrypt_feature_info(void) pr_info("AMD Memory Encryption Features active:"); /* Secure Memory Encryption */ - if (sme_active()) { + if (amd_prot_guest_has(PATTR_SME)) { /* * SME is mutually exclusive with any of the SEV * features below. diff --git a/arch/x86/mm/mem_encrypt_identity.c b/arch/x86/mm/mem_encrypt_identity.c index 470b20208430..088c8ab7dcc1 100644 --- a/arch/x86/mm/mem_encrypt_identity.c +++ b/arch/x86/mm/mem_encrypt_identity.c @@ -30,6 +30,7 @@ #include <linux/kernel.h> #include <linux/mm.h> #include <linux/mem_encrypt.h> +#include <linux/protected_guest.h> #include <asm/setup.h> #include <asm/sections.h> @@ -287,7 +288,7 @@ void __init sme_encrypt_kernel(struct boot_params *bp) unsigned long pgtable_area_len; unsigned long decrypted_base; - if (!sme_active()) + if (!prot_guest_has(PATTR_SME)) return; /* diff --git a/arch/x86/realmode/init.c b/arch/x86/realmode/init.c index 6534c92d0f83..2109ae569c67 100644 --- a/arch/x86/realmode/init.c +++ b/arch/x86/realmode/init.c @@ -3,6 +3,7 @@ #include <linux/slab.h> #include <linux/memblock.h> #include <linux/mem_encrypt.h> +#include <linux/protected_guest.h> #include <linux/pgtable.h> #include <asm/set_memory.h> @@ -44,7 +45,7 @@ void __init reserve_real_mode(void) static void sme_sev_setup_real_mode(struct trampoline_header *th) { #ifdef CONFIG_AMD_MEM_ENCRYPT - if (sme_active()) + if (prot_guest_has(PATTR_HOST_MEM_ENCRYPT)) th->flags |= TH_FLAGS_SME_ACTIVE; if (sev_es_active()) { @@ -81,7 +82,7 @@ static void __init setup_real_mode(void) * decrypted memory in order to bring up other processors * successfully. This is not needed for SEV. */ - if (sme_active()) + if (prot_guest_has(PATTR_HOST_MEM_ENCRYPT)) set_memory_decrypted((unsigned long)base, size >> PAGE_SHIFT); memcpy(base, real_mode_blob, size); diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c index 46280e6e1535..05e770e3e631 100644 --- a/drivers/iommu/amd/init.c +++ b/drivers/iommu/amd/init.c @@ -20,7 +20,7 @@ #include <linux/amd-iommu.h> #include <linux/export.h> #include <linux/kmemleak.h> -#include <linux/mem_encrypt.h> +#include <linux/protected_guest.h> #include <asm/pci-direct.h> #include <asm/iommu.h> #include <asm/apic.h> @@ -965,7 +965,7 @@ static bool copy_device_table(void) pr_err("The address of old device table is above 4G, not trustworthy!\n"); return false; } - old_devtb = (sme_active() && is_kdump_kernel()) + old_devtb = (prot_guest_has(PATTR_HOST_MEM_ENCRYPT) && is_kdump_kernel()) ? (__force void *)ioremap_encrypted(old_devtb_phys, dev_table_size) : memremap(old_devtb_phys, dev_table_size, MEMREMAP_WB); @@ -3022,7 +3022,8 @@ static int __init amd_iommu_init(void) static bool amd_iommu_sme_check(void) { - if (!sme_active() || (boot_cpu_data.x86 != 0x17)) + if (!prot_guest_has(PATTR_HOST_MEM_ENCRYPT) || + (boot_cpu_data.x86 != 0x17)) return true; /* For Fam17h, a specific level of support is required */