arch/x86/Kconfig | 7 + arch/x86/coco/core.c | 1 - arch/x86/coco/tdx/tdx.c | 99 ++++++++- arch/x86/hyperv/ivm.c | 22 +- arch/x86/include/asm/acpi.h | 7 + arch/x86/include/asm/init.h | 3 + arch/x86/include/asm/pgtable.h | 5 + arch/x86/include/asm/pgtable_types.h | 1 + arch/x86/include/asm/set_memory.h | 3 + arch/x86/include/asm/smp.h | 1 + arch/x86/include/asm/x86_init.h | 6 +- arch/x86/kernel/acpi/Makefile | 11 +- arch/x86/kernel/acpi/boot.c | 86 +------- arch/x86/kernel/acpi/madt_playdead.S | 28 +++ arch/x86/kernel/acpi/madt_wakeup.c | 292 +++++++++++++++++++++++++++ arch/x86/kernel/crash.c | 6 + arch/x86/kernel/e820.c | 9 +- arch/x86/kernel/process.c | 7 + arch/x86/kernel/reboot.c | 18 ++ arch/x86/kernel/relocate_kernel_64.S | 8 + arch/x86/kernel/x86_init.c | 8 +- arch/x86/mm/ident_map.c | 73 +++++++ arch/x86/mm/init_64.c | 16 +- arch/x86/mm/mem_encrypt_amd.c | 8 +- arch/x86/mm/pat/set_memory.c | 59 ++++-- drivers/acpi/tables.c | 14 ++ include/acpi/actbl2.h | 19 +- include/linux/cc_platform.h | 10 - include/linux/cpu.h | 2 + kernel/cpu.c | 12 +- 30 files changed, 685 insertions(+), 156 deletions(-) create mode 100644 arch/x86/kernel/acpi/madt_playdead.S create mode 100644 arch/x86/kernel/acpi/madt_wakeup.c
The patchset adds bits and pieces to get kexec (and crashkernel) work on
TDX guest.
The last patch implements CPU offlining according to the approved ACPI
spec change poposal[1]. It unlocks kexec with all CPUs visible in the target
kernel. It requires BIOS-side enabling. If it missing we fallback to booting
2nd kernel with single CPU.
Please review. I would be glad for any feedback.
[1] https://lore.kernel.org/all/13356251.uLZWGnKmhe@kreacher
v10:
- Rebased to current tip/master;
- Preserve CR4.MCE instead of setting it unconditionally;
- Fix build error in Hyper-V code after rebase;
- Include Ashish's patch for real;
v9:
- Rebased;
- Keep page tables that maps E820_TYPE_ACPI (Ashish);
- Ack/Reviewed/Tested-bys from Sathya, Kai, Tao;
- Minor printk() message adjustments;
v8:
- Rework serialization of around conversion memory back to private;
- Print ACPI_MADT_TYPE_MULTIPROC_WAKEUP in acpi_table_print_madt_entry();
- Drop debugfs interface to dump info on shared memory;
- Adjust comments and commit messages;
- Reviewed-bys by Baoquan, Dave and Thomas;
v7:
- Call enc_kexec_stop_conversion() and enc_kexec_unshare_mem() after shutting
down IO-APIC, lapic and hpet. It meets AMD requirements.
- Minor style changes;
- Add Acked/Reviewed-bys;
v6:
- Rebased to v6.8-rc1;
- Provide default noop callbacks from .enc_kexec_stop_conversion and
.enc_kexec_unshare_mem;
- Split off patch that introduces .enc_kexec_* callbacks;
- asm_acpi_mp_play_dead(): program CR3 directly from RSI, no MOV to RAX
required;
- Restructure how smp_ops.stop_this_cpu() hooked up in crash_nmi_callback();
- kvmclock patch got merged via KVM tree;
v5:
- Rename smp_ops.crash_play_dead to smp_ops.stop_this_cpu and use it in
stop_this_cpu();
- Split off enc_kexec_stop_conversion() from enc_kexec_unshare_mem();
- Introduce kernel_ident_mapping_free();
- Add explicit include for alternatives and stringify.
- Add barrier() after setting conversion_allowed to false;
- Mark cpu_hotplug_offline_disabled __ro_after_init;
- Print error if failed to hand over CPU to BIOS;
- Update comments and commit messages;
v4:
- Fix build for !KEXEC_CORE;
- Cleaner ATLERNATIVE use;
- Update commit messages and comments;
- Add Reviewed-bys;
v3:
- Rework acpi_mp_crash_stop_other_cpus() to avoid invoking hotplug state
machine;
- Free page tables if reset vector setup failed;
- Change asm_acpi_mp_play_dead() to pass reset vector and PGD as arguments;
- Mark acpi_mp_* variables as static and __ro_after_init;
- Use u32 for apicid;
- Disable CPU offlining if reset vector setup failed;
- Rename madt.S -> madt_playdead.S;
- Mark tdx_kexec_unshare_mem() as static;
- Rebase onto up-to-date tip/master;
- Whitespace fixes;
- Reorder patches;
- Add Reviewed-bys;
- Update comments and commit messages;
v2:
- Rework how unsharing hook ups into kexec codepath;
- Rework kvmclock_disable() fix based on Sean's;
- s/cpu_hotplug_not_supported()/cpu_hotplug_disable_offlining()/;
- use play_dead_common() to implement acpi_mp_play_dead();
- cond_resched() in tdx_shared_memory_show();
- s/target kernel/second kernel/;
- Update commit messages and comments;
Ashish Kalra (1):
x86/mm: Do not zap page table entries mapping unaccepted memory table
during kdump.
Kirill A. Shutemov (17):
x86/acpi: Extract ACPI MADT wakeup code into a separate file
x86/apic: Mark acpi_mp_wake_* variables as __ro_after_init
cpu/hotplug: Add support for declaring CPU offlining not supported
cpu/hotplug, x86/acpi: Disable CPU offlining for ACPI MADT wakeup
x86/kexec: Keep CR4.MCE set during kexec for TDX guest
x86/mm: Make x86_platform.guest.enc_status_change_*() return errno
x86/mm: Return correct level from lookup_address() if pte is none
x86/tdx: Account shared memory
x86/mm: Adding callbacks to prepare encrypted memory for kexec
x86/tdx: Convert shared memory back to private on kexec
x86/mm: Make e820_end_ram_pfn() cover E820_TYPE_ACPI ranges
x86/acpi: Rename fields in acpi_madt_multiproc_wakeup structure
x86/acpi: Do not attempt to bring up secondary CPUs in kexec case
x86/smp: Add smp_ops.stop_this_cpu() callback
x86/mm: Introduce kernel_ident_mapping_free()
x86/acpi: Add support for CPU offlining for ACPI MADT wakeup method
ACPI: tables: Print MULTIPROC_WAKEUP when MADT is parsed
arch/x86/Kconfig | 7 +
arch/x86/coco/core.c | 1 -
arch/x86/coco/tdx/tdx.c | 99 ++++++++-
arch/x86/hyperv/ivm.c | 22 +-
arch/x86/include/asm/acpi.h | 7 +
arch/x86/include/asm/init.h | 3 +
arch/x86/include/asm/pgtable.h | 5 +
arch/x86/include/asm/pgtable_types.h | 1 +
arch/x86/include/asm/set_memory.h | 3 +
arch/x86/include/asm/smp.h | 1 +
arch/x86/include/asm/x86_init.h | 6 +-
arch/x86/kernel/acpi/Makefile | 11 +-
arch/x86/kernel/acpi/boot.c | 86 +-------
arch/x86/kernel/acpi/madt_playdead.S | 28 +++
arch/x86/kernel/acpi/madt_wakeup.c | 292 +++++++++++++++++++++++++++
arch/x86/kernel/crash.c | 6 +
arch/x86/kernel/e820.c | 9 +-
arch/x86/kernel/process.c | 7 +
arch/x86/kernel/reboot.c | 18 ++
arch/x86/kernel/relocate_kernel_64.S | 8 +
arch/x86/kernel/x86_init.c | 8 +-
arch/x86/mm/ident_map.c | 73 +++++++
arch/x86/mm/init_64.c | 16 +-
arch/x86/mm/mem_encrypt_amd.c | 8 +-
arch/x86/mm/pat/set_memory.c | 59 ++++--
drivers/acpi/tables.c | 14 ++
include/acpi/actbl2.h | 19 +-
include/linux/cc_platform.h | 10 -
include/linux/cpu.h | 2 +
kernel/cpu.c | 12 +-
30 files changed, 685 insertions(+), 156 deletions(-)
create mode 100644 arch/x86/kernel/acpi/madt_playdead.S
create mode 100644 arch/x86/kernel/acpi/madt_wakeup.c
--
2.43.0
From: Ashish Kalra <ashish.kalra@amd.com>
The patchset adds bits and pieces to get kexec (and crashkernel) work on
SNP guest.
v4:
- Rebased to current tip/master.
- Reviewed-bys from Sathya.
- Remove snp_kexec_unprep_rom_memory() as it is not needed any more as
SEV-SNP code is not validating the ROM range in probe_roms() anymore.
- Fix kernel test robot build error/warnings.
v3:
- Rebased;
- moved Keep page tables that maps E820_TYPE_ACPI patch to Kirill's tdx
guest kexec patch series.
- checking the md attribute instead of checking the efi_setup for
detecting if running under kexec kernel.
- added new sev_es_enabled() function.
- skip video memory access in decompressor for SEV-ES/SNP systems to
prevent guest termination as boot stage2 #VC handler does not handle
MMIO.
v2:
- address zeroing of unaccepted memory table mappings at all page table levels
adding phys_pte_init(), phys_pud_init() and phys_p4d_init().
- include skip efi_arch_mem_reserve() in case of kexec as part of this
patch set.
- rename last_address_shd_kexec to a more appropriate
kexec_last_address_to_make_private.
- remove duplicate code shared with TDX and use common interfaces
defined for SNP and TDX for kexec/kdump.
- remove set_pte_enc() dependency on pg_level_to_pfn() and make the
function simpler.
- rename unshare_pte() to make_pte_private().
- clarify and make the comment for using kexec_last_address_to_make_private
more understandable.
- general cleanup.
Ashish Kalra (4):
efi/x86: skip efi_arch_mem_reserve() in case of kexec.
x86/sev: add sev_es_enabled() function.
x86/boot/compressed: Skip Video Memory access in Decompressor for
SEV-ES/SNP.
x86/snp: Convert shared memory back to private on kexec
arch/x86/boot/compressed/misc.c | 6 +-
arch/x86/boot/compressed/misc.h | 1 +
arch/x86/boot/compressed/sev.c | 5 +
arch/x86/boot/compressed/sev.h | 2 +
arch/x86/include/asm/sev.h | 4 +
arch/x86/kernel/sev.c | 161 ++++++++++++++++++++++++++++++++
arch/x86/mm/mem_encrypt_amd.c | 3 +
arch/x86/platform/efi/quirks.c | 20 +++-
8 files changed, 198 insertions(+), 4 deletions(-)
--
2.34.1
Hey Ashish, On 09.04.24 22:42, Ashish Kalra wrote: > From: Ashish Kalra <ashish.kalra@amd.com> > > The patchset adds bits and pieces to get kexec (and crashkernel) work on > SNP guest. With this patch set (and similar for the TDX one), you enable the typical kdump case, which is great! However, if a user is running with direct kernel boot - which is very typical in SEV-SNP setup, especially for Kata Containers and similar - the initial launch measurement is a natural indicator of the target environment. Kexec basically allows them to completely bypass that: You would be able to run a completely different environment than the one you measure through the launch digest. I'm not sure it's a good idea to even allow that by default in CoCo environments - at least not if the kernel is locked down. Do you have any plans to build a CoCo native kexec where you allow a VM to create a new VM context with a guest provided seed? The new context could rerun all of the attestation and so enable users to generate a new launch digest. If you then atomically swap into the new context, it would in turn enable them to natively "kexec" into a completely new VM context including measurements. I understand that an SVSM + TPM implementation may help to some extent here by integrating with IMA and adding the new kernel into the IMA log. But that quickly becomes very convoluted (hence difficult to assess correctness for) and the same measurement question arises just one level up then: How do you update your SVSM while maintaining a full measurement and trust chain? Thanks, Alex
Hello Alexander, On 5/2/2024 7:01 AM, Alexander Graf wrote: > Hey Ashish, > > On 09.04.24 22:42, Ashish Kalra wrote: >> From: Ashish Kalra <ashish.kalra@amd.com> >> >> The patchset adds bits and pieces to get kexec (and crashkernel) work on >> SNP guest. > > > With this patch set (and similar for the TDX one), you enable the > typical kdump case, which is great! > > However, if a user is running with direct kernel boot - which is very > typical in SEV-SNP setup, especially for Kata Containers and similar - > the initial launch measurement is a natural indicator of the target > environment. Kexec basically allows them to completely bypass that: > You would be able to run a completely different environment than the > one you measure through the launch digest. I'm not sure it's a good > idea to even allow that by default in CoCo environments - at least not > if the kernel is locked down. > I thought that kexec is disabled if kernel is in locked-down mode. Or is it that KEXEC_LOAD syscall is not supported/disabled in kernel locked-down mode and KEXEC_FILE_LOAD syscall is supported ? > Do you have any plans to build a CoCo native kexec where you allow a > VM to create a new VM context with a guest provided seed? The new > context could rerun all of the attestation and so enable users to > generate a new launch digest. If you then atomically swap into the new > context, it would in turn enable them to natively "kexec" into a > completely new VM context including measurements. No, currently i don't think there any any such plans. Thanks, Ashish > > I understand that an SVSM + TPM implementation may help to some extent > here by integrating with IMA and adding the new kernel into the IMA > log. But that quickly becomes very convoluted (hence difficult to > assess correctness for) and the same measurement question arises just > one level up then: How do you update your SVSM while maintaining a > full measurement and trust chain? > > > Thanks, > > Alex >
Alexander Graf <graf@amazon.com> writes: > Hey Ashish, > > On 09.04.24 22:42, Ashish Kalra wrote: >> From: Ashish Kalra <ashish.kalra@amd.com> >> >> The patchset adds bits and pieces to get kexec (and crashkernel) work on >> SNP guest. > > > With this patch set (and similar for the TDX one), you enable the > typical kdump case, which is great! > > However, if a user is running with direct kernel boot - which is very > typical in SEV-SNP setup, especially for Kata Containers and similar - > the initial launch measurement is a natural indicator of the target > environment. Kexec basically allows them to completely bypass that: You > would be able to run a completely different environment than the one you > measure through the launch digest. I'm not sure it's a good idea to even > allow that by default in CoCo environments - at least not if the kernel > is locked down. Isn't it the same when we just allow loading kernel modules? I'm sure you can also achieve a 'completely different environment' with that :-) With SecureBoot / lockdown we normally require modules to pass signature check, I guess we can employ the same mechanism for kexec. I.e. in lockdown, we require signature check on the kexec-ed kernel. Also, it may make sense to check initramfs too (with direct kernel boot it's also part of launch measurements, right?) and there's UKI for that already). Personally, I believe that if we simply forbid kexec for CoCo in lockdown mode, the feature will become mostly useless in 'full stack' (which boot through firmware) production envrironments. -- Vitaly
On 02.05.24 14:18, Vitaly Kuznetsov wrote: > Alexander Graf <graf@amazon.com> writes: > >> Hey Ashish, >> >> On 09.04.24 22:42, Ashish Kalra wrote: >>> From: Ashish Kalra <ashish.kalra@amd.com> >>> >>> The patchset adds bits and pieces to get kexec (and crashkernel) work on >>> SNP guest. >> >> With this patch set (and similar for the TDX one), you enable the >> typical kdump case, which is great! >> >> However, if a user is running with direct kernel boot - which is very >> typical in SEV-SNP setup, especially for Kata Containers and similar - >> the initial launch measurement is a natural indicator of the target >> environment. Kexec basically allows them to completely bypass that: You >> would be able to run a completely different environment than the one you >> measure through the launch digest. I'm not sure it's a good idea to even >> allow that by default in CoCo environments - at least not if the kernel >> is locked down. > Isn't it the same when we just allow loading kernel modules? I'm sure > you can also achieve a 'completely different environment' with that :-) > With SecureBoot / lockdown we normally require modules to pass signature > check, I guess we can employ the same mechanism for kexec. I.e. in > lockdown, we require signature check on the kexec-ed kernel. Also, it > may make sense to check initramfs too (with direct kernel boot it's also > part of launch measurements, right?) and there's UKI for that already). Correct. With IMA, you even do exactly that: Enforce a signature check of the next binary with kexec. The problem is that you typically want to update the system because something is broken; most likely your original environment had a security issue somewhere. From a pure SEV-SNP attestation point of view, you can not distinguish between the patched and unpatched environment: Both look the same. So while kexec isn't the problem, it's the fact that you can't tell anyone that you're now running a fixed version of the code :). > Personally, I believe that if we simply forbid kexec for CoCo in > lockdown mode, the feature will become mostly useless in 'full stack' > (which boot through firmware) production envrironments. I'm happy for CoCo to stay smoke and mirrors :). But I believe that if you want to genuinely draw a trust chain back to an AMD/Intel certificate, we need to come up with a good way of making updates work with a working trust chain so that whoever checks whether you're running sanctioned code is able to validate the claim. Alex
Alexander Graf <graf@amazon.com> writes: > Correct. With IMA, you even do exactly that: Enforce a signature check > of the next binary with kexec. > > The problem is that you typically want to update the system because > something is broken; most likely your original environment had a > security issue somewhere. From a pure SEV-SNP attestation point of view, > you can not distinguish between the patched and unpatched environment: > Both look the same. > > So while kexec isn't the problem, it's the fact that you can't tell > anyone that you're now running a fixed version of the code :). ... > > I'm happy for CoCo to stay smoke and mirrors :). "Only a Sith deals in absolutes" :-) > But I believe that if > you want to genuinely draw a trust chain back to an AMD/Intel > certificate, we need to come up with a good way of making updates work > with a working trust chain so that whoever checks whether you're running > sanctioned code is able to validate the claim. Launch measurements are what they are, they describe the state of your guest before it started booting. There are multiple mechanisms in Linux which change CPL0 code already: self-modifying code like static keys, loadable modules, runtime patching, kexec,... In case some specific deployment requires stronger guarantees we can probably introduce something like 'full lockdown' mode (as a compile time option, I guess) which would disable all of the aforementioned mechanisms. It will still not be a hard proof that the running code matches launch measurements (because vulnerabilities/bugs may still exist) I guess but could be an improvement. Basically, what I wanted to argue is that kexec does not need to be treated 'specially' for CVMs if we keep all other ways to modify kernel code. Making these methods 'attestable' is currently a challenge indeed. -- Vitaly
From: Ashish Kalra <ashish.kalra@amd.com>
The patchset adds bits and pieces to get kexec (and crashkernel) work on
SNP guest.
The series is based off of and tested against Kirill Shutemov's tree:
https://github.com/intel/tdx.git guest-kexec
----
v5:
- Removed sev_es_enabled() function and using sev_status directly to
check for SEV-ES/SEV-SNP guest.
- used --base option to generate patches to specify Kirill's TDX guest
kexec patches as prerequisite patches to fix kernel test robot
build errors.
v4:
- Rebased to current tip/master.
- Reviewed-bys from Sathya.
- Remove snp_kexec_unprep_rom_memory() as it is not needed any more as
SEV-SNP code is not validating the ROM range in probe_roms() anymore.
- Fix kernel test robot build error/warnings.
v3:
- Rebased;
- moved Keep page tables that maps E820_TYPE_ACPI patch to Kirill's tdx
guest kexec patch series.
- checking the md attribute instead of checking the efi_setup for
detecting if running under kexec kernel.
- added new sev_es_enabled() function.
- skip video memory access in decompressor for SEV-ES/SNP systems to
prevent guest termination as boot stage2 #VC handler does not handle
MMIO.
v2:
- address zeroing of unaccepted memory table mappings at all page table levels
adding phys_pte_init(), phys_pud_init() and phys_p4d_init().
- include skip efi_arch_mem_reserve() in case of kexec as part of this
patch set.
- rename last_address_shd_kexec to a more appropriate
kexec_last_address_to_make_private.
- remove duplicate code shared with TDX and use common interfaces
defined for SNP and TDX for kexec/kdump.
- remove set_pte_enc() dependency on pg_level_to_pfn() and make the
function simpler.
- rename unshare_pte() to make_pte_private().
- clarify and make the comment for using kexec_last_address_to_make_private
more understandable.
- general cleanup.
Ashish Kalra (3):
efi/x86: skip efi_arch_mem_reserve() in case of kexec.
x86/boot/compressed: Skip Video Memory access in Decompressor for
SEV-ES/SNP.
x86/snp: Convert shared memory back to private on kexec
arch/x86/boot/compressed/misc.c | 6 +-
arch/x86/include/asm/sev.h | 4 +
arch/x86/kernel/sev.c | 161 ++++++++++++++++++++++++++++++++
arch/x86/mm/mem_encrypt_amd.c | 3 +
arch/x86/platform/efi/quirks.c | 20 +++-
5 files changed, 190 insertions(+), 4 deletions(-)
base-commit: a18b42d8997abfd77aa1637c0de6850b0c30b1fe
prerequisite-patch-id: bd8e77f0f12223d21cb2f35b77bfcbdd9ad80b0f
prerequisite-patch-id: bfe2fa046349978ac1825275eb205acecfbc22f3
prerequisite-patch-id: 5e60d292457c7cd98fd3e45c23127e9463b56a69
prerequisite-patch-id: 1f97d0a2edb7509dd58276f628d1a4bda62c154c
prerequisite-patch-id: 8db559385c44e8b6670d74196e8d83d2dfad2f40
prerequisite-patch-id: cbdfea1e50ecb3b4cee3a25a27df4d35bd95d532
prerequisite-patch-id: 1cea0996e0dc3bb9f0059c927c405ca31003791e
prerequisite-patch-id: 469a0a3c78b0eca82527cd85e2205fb8fb89d645
prerequisite-patch-id: 2974ef211db5253d9782018e352d2a6ff0b0ef54
prerequisite-patch-id: 2cfffd80947941892421dae99b7fa0f9f9715884
prerequisite-patch-id: 466c2cb9f0a107bbd1dbd8526f4eff2bdb55f1ce
prerequisite-patch-id: d4966ae63e86d24b0bf578da4dae871cd9002b12
prerequisite-patch-id: fccde6f1fa385b5af0195f81fcb95acd71822428
prerequisite-patch-id: 16048ee15e392b0b9217b8923939b0059311abd2
prerequisite-patch-id: 5c9ae9aa294f72f63ae2c3551507dfbd92525803
prerequisite-patch-id: 758bdb686290c018cbd5b7d005354019f9d15248
prerequisite-patch-id: 4125b799fc9577b1a46427e45618fa0174f7a4b3
prerequisite-patch-id: 60760e0c98ab7ccd2ca22ae3e9f20ff5a94c6e91
--
2.34.1
From: Ashish Kalra <ashish.kalra@amd.com>
For kexec use case, need to use and stick to the EFI memmap passed
from the first kernel via boot-params/setup data, hence,
skip efi_arch_mem_reserve() during kexec.
Additionally during SNP guest kexec testing discovered that EFI memmap
is corrupted during chained kexec. kexec_enter_virtual_mode() during
late init will remap the efi_memmap physical pages allocated in
efi_arch_mem_reserve() via memblock & then subsequently cause random
EFI memmap corruption once memblock is freed/teared-down.
Suggested-by: Dave Young <dyoung@redhat.com>
[Dave Young: checking the md attribute instead of checking the efi_setup]
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
arch/x86/platform/efi/quirks.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index f0cc00032751..982f5e50a4b3 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -258,12 +258,28 @@ void __init efi_arch_mem_reserve(phys_addr_t addr, u64 size)
int num_entries;
void *new;
- if (efi_mem_desc_lookup(addr, &md) ||
- md.type != EFI_BOOT_SERVICES_DATA) {
+ /*
+ * For kexec use case, we need to use the EFI memmap passed from the first
+ * kernel via setup data, so we need to skip this.
+ * Additionally kexec_enter_virtual_mode() during late init will remap
+ * the efi_memmap physical pages allocated here via memboot & then
+ * subsequently cause random EFI memmap corruption once memblock is freed.
+ */
+
+ if (efi_mem_desc_lookup(addr, &md)) {
pr_err("Failed to lookup EFI memory descriptor for %pa\n", &addr);
return;
}
+ if (md.type != EFI_BOOT_SERVICES_DATA) {
+ pr_err("Skip reserving non EFI Boot Service Data memory for %pa\n", &addr);
+ return;
+ }
+
+ /* Kexec copied the efi memmap from the first kernel, thus skip the case */
+ if (md.attribute & EFI_MEMORY_RUNTIME)
+ return;
+
if (addr + size > md.phys_addr + (md.num_pages << EFI_PAGE_SHIFT)) {
pr_err("Region spans EFI memory descriptors, %pa\n", &addr);
return;
--
2.34.1On Mon, Apr 15, 2024 at 11:22:58PM +0000, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
>
> For kexec use case, need to use and stick to the EFI memmap passed
> from the first kernel via boot-params/setup data, hence,
> skip efi_arch_mem_reserve() during kexec.
Please use this or similar scheme when formulating your commit messages.
This above is too laconic.
1. Prepare the context for the explanation briefly.
2. Explain the problem at hand.
3. "It happens because of <...>"
4. "Fix it by doing X"
5. "(Potentially do Y)."
And some of those above are optional depending on the issue being
explained.
For more detailed info, see
Documentation/process/submitting-patches.rst,
Section "2) Describe your changes".
> Additionally during SNP guest kexec testing discovered that EFI memmap
> is corrupted during chained kexec.
That sentence needs sanitization.
> kexec_enter_virtual_mode() during late init will remap the efi_memmap
> physical pages allocated in efi_arch_mem_reserve() via memblock & then
s/&/and/
This is not code. Please take a greater care when writing commit
messages - they're not write-only.
> subsequently cause random EFI memmap corruption once memblock is
> freed/teared-down.
"torn down"
> Suggested-by: Dave Young <dyoung@redhat.com>
> [Dave Young: checking the md attribute instead of checking the efi_setup]
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
> arch/x86/platform/efi/quirks.c | 20 ++++++++++++++++++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
> index f0cc00032751..982f5e50a4b3 100644
> --- a/arch/x86/platform/efi/quirks.c
> +++ b/arch/x86/platform/efi/quirks.c
> @@ -258,12 +258,28 @@ void __init efi_arch_mem_reserve(phys_addr_t addr, u64 size)
> int num_entries;
> void *new;
>
> - if (efi_mem_desc_lookup(addr, &md) ||
> - md.type != EFI_BOOT_SERVICES_DATA) {
> + /*
> + * For kexec use case, we need to use the EFI memmap passed from the first
Make all your text impersonal - no "we", "I", etc.
> + * kernel via setup data, so we need to skip this.
What exactly do we need to skip?
If the EFI memory descriptor lookup fails?
> + * Additionally kexec_enter_virtual_mode() during late init will remap
> + * the efi_memmap physical pages allocated here via memboot & then
> + * subsequently cause random EFI memmap corruption once memblock is freed.
> + */
Why is that comment here and what is its relevance to the line it is
above of?
> + if (efi_mem_desc_lookup(addr, &md)) {
> pr_err("Failed to lookup EFI memory descriptor for %pa\n", &addr);
> return;
> }
>
> + if (md.type != EFI_BOOT_SERVICES_DATA) {
> + pr_err("Skip reserving non EFI Boot Service Data memory for %pa\n", &addr);
What is this pr_err() useful for?
> + return;
> + }
> +
> + /* Kexec copied the efi memmap from the first kernel, thus skip the case */
kexec? This is a generic function - what does it have to do with kexec?
The subject of this patch is:
Subject: [PATCH v5 1/3] efi/x86: skip efi_arch_mem_reserve() in case of kexec
and yet, nothing skips this function - it adds a bunch of checks,
printks and early returns with the intent that those early returns
happen on kexec and thus the actual memremap doesn't happen there.
So it is some sort of: let's check things which will be true in
a kexec-ed kernel and thus avoid the function by returning early.
But I have no clue.
It sounds to me like you need to go back up, to the 10000ft view and
explain how exactly this efi_mem_reserve() causes trouble for the
kexec-ed kernel so that we can think of a proper solution, not some
random hackery.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Hello Boris,
On 4/24/2024 9:48 AM, Borislav Petkov wrote:
> On Mon, Apr 15, 2024 at 11:22:58PM +0000, Ashish Kalra wrote:
>> From: Ashish Kalra <ashish.kalra@amd.com>
>>
>> For kexec use case, need to use and stick to the EFI memmap passed
>> from the first kernel via boot-params/setup data, hence,
>> skip efi_arch_mem_reserve() during kexec.
> Please use this or similar scheme when formulating your commit messages.
> This above is too laconic.
>
> 1. Prepare the context for the explanation briefly.
>
> 2. Explain the problem at hand.
>
> 3. "It happens because of <...>"
>
> 4. "Fix it by doing X"
>
> 5. "(Potentially do Y)."
>
> And some of those above are optional depending on the issue being
> explained.
>
> For more detailed info, see
> Documentation/process/submitting-patches.rst,
> Section "2) Describe your changes".
Here is the more detailed description of the issue:
With SNP guest kexec and during nested guest kexec, observe the
following efi memmap corruption :
[ 0.000000] efi: EFI v2.7 by EDK II^M
[ 0.000000] efi: SMBIOS=0x7e33f000 SMBIOS 3.0=0x7e33d000
ACPI=0x7e57e000 ACPI 2.0=0x7e57e014 MEMATTR=0x7cc3c018
Unaccepted=0x7c09e018 ^M
[ 0.000000] efi: [Firmware Bug]: Invalid EFI memory map entries:^M
[ 0.000000] efi: mem03: [type=269370880|attr=0x0e42100e42180e41]
range=[0x0486200e41038c18-0x200e898a0eee713ac17] (invalid)^M
[ 0.000000] efi: mem04: [type=12336|attr=0x0e410686300e4105]
range=[0x100e420000000176-0x8c290f26248d200e175] (invalid)^M
[ 0.000000] efi: mem06: [type=1124304408|attr=0x000030b400000028]
range=[0x0e51300e45280e77-0xb44ed2142f460c1e76] (invalid)^M
[ 0.000000] efi: mem08: [type=68|attr=0x300e540583280e41]
range=[0x0000011affff3cd8-0x486200e54b38c0bcd7] (invalid)^M
[ 0.000000] efi: mem10: [type=1107529240|attr=0x0e42280e41300e41]
range=[0x300e41058c280e42-0x38010ae54c5c328ee41] (invalid)^M
[ 0.000000] efi: mem11: [type=189335566|attr=0x048d200e42038e18]
range=[0x0000318c00000048-0xe42029228ce4200047] (invalid)^M
[ 0.000000] efi: mem12: [type=239142534|attr=0x0000002400000b4b]
range=[0x0e41380e0a7d700e-0x80f26238f22bfe500d] (invalid)^M
[ 0.000000] efi: mem14: [type=239207055|attr=0x0e41300e43380e0a]
range=[0x8c280e42048d200e-0xc70b028f2f27cc0a00d] (invalid)^M
[ 0.000000] efi: mem15: [type=239210510|attr=0x00080e660b47080e]
range=[0x0000324c0000001c-0xa78028634ce490001b] (invalid)^M
[ 0.000000] efi: mem16: [type=4294848528|attr=0x0000329400000014]
range=[0x0e410286100e4100-0x80f252036a218f20ff] (invalid)^M
[ 0.000000] efi: mem19: [type=2250772033|attr=0x42180e42200e4328]
range=[0x41280e0ab9020683-0xe0e538c28b39e62682] (invalid)^M
[ 0.000000] efi: mem20: [type=16| | | | | | | | | | |WB|
|WC| ] range=[0x00000008ffff4438-0xffff44340090333c437] (invalid)^M
[ 0.000000] efi: mem22: [Reserved |attr=0x000000c1ffff4420]
range=[0xffff442400003398-0x1033a04240003f397] (invalid)^M
[ 0.000000] efi: mem23: [type=1141080856|attr=0x080e41100e43180e]
range=[0x280e66300e4b280e-0x440dc5ee7141f4c080d] (invalid)^M
[ 0.000000] efi: mem25: [Reserved |attr=0x0000000affff44a0]
range=[0xffff44a400003428-0x1034304a400013427] (invalid)^M
[ 0.000000] efi: mem28: [type=16| | | | | | | | | | |WB|
|WC| ] range=[0x0000000affff4488-0xffff448400b034bc487] (invalid)^M
[ 0.000000] efi: mem30: [Reserved |attr=0x0000000affff4470]
range=[0xffff447400003518-0x10352047400013517] (invalid)^M
[ 0.000000] efi: mem33: [type=16| | | | | | | | | | |WB|
|WC| ] range=[0x0000000affff4458-0xffff445400b035ac457] (invalid)^M
[ 0.000000] efi: mem35: [type=269372416|attr=0x0e42100e42180e41]
range=[0x0486200e44038c18-0x200e8b8a0eee823ac17] (invalid)^M
[ 0.000000] efi: mem37: [type=2351435330|attr=0x0e42100e42180e42]
range=[0x470783380e410686-0x2002b2a041c2141e685] (invalid)^M
[ 0.000000] efi: mem38: [type=1093668417|attr=0x100e420000000270]
range=[0x42100e42180e4220-0xfff366a4e421b78c21f] (invalid)^M
[ 0.000000] efi: mem39: [type=76357646|attr=0x180e42200e42280e]
range=[0x0e410686300e4105-0x4130f251a0710ae5104] (invalid)^M
[ 0.000000] efi: mem40: [type=940444268|attr=0x0e42200e42280e41]
range=[0x180e42200e42280e-0x300fc71c300b4f2480d] (invalid)^M
[ 0.000000] efi: mem41: [MMIO |attr=0x8c280e42048d200e]
range=[0xffff479400003728-0x42138e0c87820292727] (invalid)^M
[ 0.000000] efi: mem42: [type=1191674680|attr=0x0000004c0000000b]
range=[0x300e41380e0a0246-0x470b0f26238f22b8245] (invalid)^M
[ 0.000000] efi: mem43: [type=2010|attr=0x0301f00e4d078338]
range=[0x45038e180e42028f-0xe4556bf118f282528e] (invalid)^M
[ 0.000000] efi: mem44: [type=1109921345|attr=0x300e44000000006c]
range=[0x44080e42100e4218-0xfff39254e42138ac217] (invalid)^M
[ 0.000000] efi: mem45: [type=40|attr=0x0e41100e41180e0a]
range=[0x0000008affff5228-0x4702400e53b3830d227] (invalid)^M
[ 0.000000] efi: mem47: [type=1107529240|attr=0x42280e41300e4138]
range=[0x300e44058c280e42-0xe0d049a435c728ee41] (invalid)^M
...
This EFI memap corruption is happening during efi_arch_mem_reserve()
invocation with the previous kexec-ed kernel boot.
( efi_arch_mem_reserve() is invoked with the following call-stack: )
[ 0.310010] efi_arch_mem_reserve+0xb1/0x220^M
[ 0.310686] ? memblock_add_range+0x2a0/0x2e0^M
[ 0.311382] efi_mem_reserve+0x36/0x60^M
[ 0.311973] efi_bgrt_init+0x17d/0x1a0^M
[ 0.312565] ? __pfx_acpi_parse_bgrt+0x10/0x10^M
[ 0.313265] acpi_parse_bgrt+0x12/0x20^M
[ 0.313858] acpi_table_parse+0x77/0xd0^M
[ 0.314463] acpi_boot_init+0x362/0x630^M
[ 0.315069] setup_arch+0xa88/0xf80^M
[ 0.315629] start_kernel+0x68/0xa90^M
[ 0.316194] x86_64_start_reservations+0x1c/0x30^M
[ 0.316921] x86_64_start_kernel+0xbf/0x110^M
[ 0.317582] common_startup_64+0x13e/0x141^M
[ 0.318231] </TASK>^M
Now, efi_arch_mem_reserve() calls efi_memmap_alloc() to allocate memory
for EFI memory map and due to early allocation it uses memblock allocation.
Later in the boot flow, efi_enter_virtual_mode() calls
kexec_enter_virtual_mode() in case of a kexec-ed kernel boot.
This function kexec_enter_virtual_mode() installs the new EFI memory map
by calling efi_memmap_init_late() which remaps the efi_memmap physically
allocated above in efi_arch_mem_reserve(), but please note that this
remapping is still using memblock allocation.
Subsequently, when memblock is freed later in boot flow, the above
remapped efi_memmap will have random corruption (similar to a
use-after-free scenario).
This corrupted EFI memory map is then passed to the next kexec-ed kernel
which causes a panic when trying to use the corrupted EFI memory map.
>> Additionally during SNP guest kexec testing discovered that EFI memmap
>> is corrupted during chained kexec.
> That sentence needs sanitization.
>
>> kexec_enter_virtual_mode() during late init will remap the efi_memmap
>> physical pages allocated in efi_arch_mem_reserve() via memblock & then
> s/&/and/
>
> This is not code. Please take a greater care when writing commit
> messages - they're not write-only.
>
>> subsequently cause random EFI memmap corruption once memblock is
>> freed/teared-down.
> "torn down"
>
>> Suggested-by: Dave Young <dyoung@redhat.com>
>> [Dave Young: checking the md attribute instead of checking the efi_setup]
>> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
>> ---
>> arch/x86/platform/efi/quirks.c | 20 ++++++++++++++++++--
>> 1 file changed, 18 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
>> index f0cc00032751..982f5e50a4b3 100644
>> --- a/arch/x86/platform/efi/quirks.c
>> +++ b/arch/x86/platform/efi/quirks.c
>> @@ -258,12 +258,28 @@ void __init efi_arch_mem_reserve(phys_addr_t addr, u64 size)
>> int num_entries;
>> void *new;
>>
>> - if (efi_mem_desc_lookup(addr, &md) ||
>> - md.type != EFI_BOOT_SERVICES_DATA) {
>> + /*
>> + * For kexec use case, we need to use the EFI memmap passed from the first
> Make all your text impersonal - no "we", "I", etc.
>
>> + * kernel via setup data, so we need to skip this.
> What exactly do we need to skip?
>
> If the EFI memory descriptor lookup fails?
>
>> + * Additionally kexec_enter_virtual_mode() during late init will remap
>> + * the efi_memmap physical pages allocated here via memboot & then
>> + * subsequently cause random EFI memmap corruption once memblock is freed.
>> + */
> Why is that comment here and what is its relevance to the line it is
> above of?
>
>> + if (efi_mem_desc_lookup(addr, &md)) {
>> pr_err("Failed to lookup EFI memory descriptor for %pa\n", &addr);
>> return;
>> }
>>
>> + if (md.type != EFI_BOOT_SERVICES_DATA) {
>> + pr_err("Skip reserving non EFI Boot Service Data memory for %pa\n", &addr);
> What is this pr_err() useful for?
>
>> + return;
>> + }
>> +
>> + /* Kexec copied the efi memmap from the first kernel, thus skip the case */
> kexec? This is a generic function - what does it have to do with kexec?
>
> The subject of this patch is:
>
> Subject: [PATCH v5 1/3] efi/x86: skip efi_arch_mem_reserve() in case of kexec
>
> and yet, nothing skips this function - it adds a bunch of checks,
> printks and early returns with the intent that those early returns
> happen on kexec and thus the actual memremap doesn't happen there.
>
> So it is some sort of: let's check things which will be true in
> a kexec-ed kernel and thus avoid the function by returning early.
>
> But I have no clue.
>
> It sounds to me like you need to go back up, to the 10000ft view and
> explain how exactly this efi_mem_reserve() causes trouble for the
> kexec-ed kernel so that we can think of a proper solution, not some
> random hackery.
The above details explain why and how efi_arch_mem_reserve() causes
trouble for the (nested) kexec-ed kernel, additionally, there is a
another reason to skip efi_arch_mem_reserve() altogether for the kexec
case, as for kexec use case we need to use the EFI memmap passed from
the 1st kernel via setup_data and probably need to avoid any additional
EFI memory map additions/updates.
Therefore, the first revision of this patch had the following code to
skip efi_arch_mem_reserve():
void __init efi_arch_mem_reserve(..) {
+ if (efi_setup) + return;
But then based on upstream review/feedback, the second revision of this
patch, updated the patch to check the md attribute of the EFI memory
descriptor instead of checking for efi_setup for detecting if running
under kexec kernel and the checking of the md attribute of the EFI
memory descriptor introduces these additional checks and pr_err() which
you commented on above.
Hopefully, the above detailed explanation captures the reason to skip
efi_arch_mem_reserve() in case of (SNP) guest kexec, looking forward to
your comments/feedback on the same for me to rework this patch
(especially the commit message) and post it again.
Thanks, Ashish
On Wed, Apr 24, 2024 at 04:17:09PM -0500, Kalra, Ashish wrote:
> With SNP guest kexec and during nested guest kexec, observe the following
> efi memmap corruption :
Before we delve any deeper here, lemme make sure I understand this
correctly:
* You're in a SNP guest and you're kexec-ing into a new kernel?
or
* You have a plain hypervisor which runs a non-CoCo guest and that guest
is a hypervisor too and it starts a level 2 guest and *in* *that* level
2 guest you kexec a kernel?
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On 4/26/2024 9:21 AM, Borislav Petkov wrote: > On Wed, Apr 24, 2024 at 04:17:09PM -0500, Kalra, Ashish wrote: >> With SNP guest kexec and during nested guest kexec, observe the following >> efi memmap corruption : > Before we delve any deeper here, lemme make sure I understand this > correctly: > > * You're in a SNP guest and you're kexec-ing into a new kernel? Yes this is the case, where i am in a SNP guest and kexec-ing into a new kernel. I should have mentioned *chained* guest kexec above instead of nested guest kexec. Thanks, Ashish
On Fri, Apr 26, 2024 at 09:47:02AM -0500, Kalra, Ashish wrote:
> I should have mentioned *chained* guest kexec above instead of nested guest
> kexec.
What is a "chained guest kexec" now?
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On 4/26/2024 10:22 AM, Borislav Petkov wrote: > On Fri, Apr 26, 2024 at 09:47:02AM -0500, Kalra, Ashish wrote: >> I should have mentioned *chained* guest kexec above instead of nested guest >> kexec. > What is a "chained guest kexec" now? "Chained guest kexec" is when we are in a guest and kexec-ing into a new kernel and then this kernel kexecs into another and so on ... Thanks, Ashish
On Fri, Apr 26, 2024 at 10:28:41AM -0500, Kalra, Ashish wrote:
> "Chained guest kexec" is when we are in a guest and kexec-ing into a new
> kernel and then this kernel kexecs into another and so on ...
Make sure to explain your terminology:
$ git grep -rE "chained.*kexec"
$
and there's nothing "chained" about it - you're simply kexec-ing in
a loop.
Please don't make it sound more complicated than it is.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On 4/26/2024 10:34 AM, Borislav Petkov wrote: > On Fri, Apr 26, 2024 at 10:28:41AM -0500, Kalra, Ashish wrote: >> "Chained guest kexec" is when we are in a guest and kexec-ing into a new >> kernel and then this kernel kexecs into another and so on ... > Make sure to explain your terminology: > > $ git grep -rE "chained.*kexec" > $ > > and there's nothing "chained" about it - you're simply kexec-ing in > a loop. > > Please don't make it sound more complicated than it is. I have posted another version of this patch series (v6) with restructured and updated commit message for this patch, so please review this v6 series. Thanks, Ashish
>It sounds to me like you need to go back up, to the 10000ft view and
>> explain how exactly this efi_mem_reserve() causes trouble for the
>> kexec-ed kernel so that we can think of a proper solution, not some
>> random hackery.
>
> The above details explain why and how efi_arch_mem_reserve() causes
> trouble for the (nested) kexec-ed kernel, additionally, there is a
> another reason to skip efi_arch_mem_reserve() altogether for the kexec
> case, as for kexec use case we need to use the EFI memmap passed from
> the 1st kernel via setup_data and probably need to avoid any
> additional EFI memory map additions/updates.
>
> Therefore, the first revision of this patch had the following code to
> skip efi_arch_mem_reserve():
>
> void __init efi_arch_mem_reserve(..) {
>
> + if (efi_setup) + return;
>
> But then based on upstream review/feedback, the second revision of
> this patch, updated the patch to check the md attribute of the EFI
> memory descriptor instead of checking for efi_setup for detecting if
> running under kexec kernel and the checking of the md attribute of the
> EFI memory descriptor introduces these additional checks and pr_err()
> which you commented on above.
>
> Hopefully, the above detailed explanation captures the reason to skip
> efi_arch_mem_reserve() in case of (SNP) guest kexec, looking forward
> to your comments/feedback on the same for me to rework this patch
> (especially the commit message) and post it again.
<snip>
I am actually going to rename this patch to something more appropriate like:
Fix EFI memory map corruption during SNP guest kexec
And in the patch itself, go back to skipping efi_arch_mem_reserve() by
checking efi_setup to check for running under kexec kernel similar to
how it used by efi_enter_virtual_mode().
Thanks, Ashish
From: Ashish Kalra <ashish.kalra@amd.com>
Accessing guest video memory/RAM during kernel decompressor
causes guest termination as boot stage2 #VC handler for
SEV-ES/SNP systems does not support MMIO handling.
This issue is observed with SEV-ES/SNP guest kexec as
kexec -c adds screen_info to the boot parameters
passed to the kexec kernel, which causes console output to
be dumped to both video and serial.
As the decompressor output gets cleared really fast, it is
preferable to get the console output only on serial, hence,
skip accessing video RAM during decompressor stage to
prevent guest termination.
Serial console output during decompressor stage works as
boot stage2 #VC handler already supports handling port I/O.
Suggested-by: Thomas Lendacy <thomas.lendacky@amd.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
arch/x86/boot/compressed/misc.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index b70e4a21c15f..3b9f96b3dbcc 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -427,8 +427,10 @@ asmlinkage __visible void *extract_kernel(void *rmode, unsigned char *output)
vidport = 0x3d4;
}
- lines = boot_params_ptr->screen_info.orig_video_lines;
- cols = boot_params_ptr->screen_info.orig_video_cols;
+ if (!(sev_status & MSR_AMD64_SEV_ES_ENABLED)) {
+ lines = boot_params_ptr->screen_info.orig_video_lines;
+ cols = boot_params_ptr->screen_info.orig_video_cols;
+ }
init_default_io_ops();
--
2.34.1From: Ashish Kalra <ashish.kalra@amd.com>
SNP guests allocate shared buffers to perform I/O. It is done by
allocating pages normally from the buddy allocator and converting them
to shared with set_memory_decrypted().
The second kernel has no idea what memory is converted this way. It only
sees E820_TYPE_RAM.
Accessing shared memory via private mapping will cause unrecoverable RMP
page-faults.
On kexec walk direct mapping and convert all shared memory back to
private. It makes all RAM private again and second kernel may use it
normally. Additionally for SNP guests convert all bss decrypted section
pages back to private and switch back ROM regions to shared so that
their revalidation does not fail during kexec kernel boot.
The conversion occurs in two steps: stopping new conversions and
unsharing all memory. In the case of normal kexec, the stopping of
conversions takes place while scheduling is still functioning. This
allows for waiting until any ongoing conversions are finished. The
second step is carried out when all CPUs except one are inactive and
interrupts are disabled. This prevents any conflicts with code that may
access shared memory.
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
arch/x86/include/asm/sev.h | 4 +
arch/x86/kernel/sev.c | 161 ++++++++++++++++++++++++++++++++++
arch/x86/mm/mem_encrypt_amd.c | 3 +
3 files changed, 168 insertions(+)
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 7f57382afee4..78d40d08d201 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -229,6 +229,8 @@ void snp_accept_memory(phys_addr_t start, phys_addr_t end);
u64 snp_get_unsupported_features(u64 status);
u64 sev_get_status(void);
void sev_show_status(void);
+void snp_kexec_unshare_mem(void);
+void snp_kexec_stop_conversion(bool crash);
#else
static inline void sev_es_ist_enter(struct pt_regs *regs) { }
static inline void sev_es_ist_exit(void) { }
@@ -258,6 +260,8 @@ static 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 sev_show_status(void) { }
+static inline void snp_kexec_unshare_mem(void) { }
+static inline void snp_kexec_stop_conversion(bool crash) { }
#endif
#ifdef CONFIG_KVM_AMD_SEV
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index 38ad066179d8..17f616963beb 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -42,6 +42,8 @@
#include <asm/apic.h>
#include <asm/cpuid.h>
#include <asm/cmdline.h>
+#include <asm/pgtable.h>
+#include <asm/set_memory.h>
#define DR7_RESET_VALUE 0x400
@@ -92,6 +94,9 @@ static struct ghcb *boot_ghcb __section(".data");
/* Bitmap of SEV features supported by the hypervisor */
static u64 sev_hv_features __ro_after_init;
+/* Last address to be switched to private during kexec */
+static unsigned long kexec_last_addr_to_make_private;
+
/* #VC handler runtime per-CPU data */
struct sev_es_runtime_data {
struct ghcb ghcb_page;
@@ -913,6 +918,162 @@ void snp_accept_memory(phys_addr_t start, phys_addr_t end)
set_pages_state(vaddr, npages, SNP_PAGE_STATE_PRIVATE);
}
+static bool set_pte_enc(pte_t *kpte, int level, void *va)
+{
+ pte_t new_pte;
+
+ if (pte_none(*kpte))
+ return false;
+
+ /*
+ * Change the physical page attribute from C=0 to C=1. Flush the
+ * caches to ensure that data gets accessed with the correct C-bit.
+ */
+ if (pte_present(*kpte))
+ clflush_cache_range(va, page_level_size(level));
+
+ new_pte = __pte(cc_mkenc(pte_val(*kpte)));
+ set_pte_atomic(kpte, new_pte);
+
+ return true;
+}
+
+static bool make_pte_private(pte_t *pte, unsigned long addr, int pages, int level)
+{
+ struct sev_es_runtime_data *data;
+ struct ghcb *ghcb;
+
+ data = this_cpu_read(runtime_data);
+ ghcb = &data->ghcb_page;
+
+ /* Check for GHCB for being part of a PMD range. */
+ if ((unsigned long)ghcb >= addr &&
+ (unsigned long)ghcb <= (addr + (pages * PAGE_SIZE))) {
+ /*
+ * Ensure that the current cpu's GHCB is made private
+ * at the end of unshared loop so that we continue to use the
+ * optimized GHCB protocol and not force the switch to
+ * MSR protocol till the very end.
+ */
+ pr_debug("setting boot_ghcb to NULL for this cpu ghcb\n");
+ kexec_last_addr_to_make_private = addr;
+ return true;
+ }
+
+ if (!set_pte_enc(pte, level, (void *)addr))
+ return false;
+
+ snp_set_memory_private(addr, pages);
+
+ return true;
+}
+
+static void unshare_all_memory(void)
+{
+ unsigned long addr, end;
+
+ /*
+ * Walk direct mapping and convert all shared memory back to private,
+ */
+
+ addr = PAGE_OFFSET;
+ end = PAGE_OFFSET + get_max_mapped();
+
+ while (addr < end) {
+ unsigned long size;
+ unsigned int level;
+ pte_t *pte;
+
+ pte = lookup_address(addr, &level);
+ size = page_level_size(level);
+
+ /*
+ * pte_none() check is required to skip physical memory holes in direct mapped.
+ */
+ if (pte && pte_decrypted(*pte) && !pte_none(*pte)) {
+ int pages = size / PAGE_SIZE;
+
+ if (!make_pte_private(pte, addr, pages, level)) {
+ pr_err("Failed to unshare range %#lx-%#lx\n",
+ addr, addr + size);
+ }
+
+ }
+
+ addr += size;
+ }
+ __flush_tlb_all();
+
+}
+
+static void unshare_all_bss_decrypted_memory(void)
+{
+ unsigned long vaddr, vaddr_end;
+ unsigned int level;
+ unsigned int npages;
+ pte_t *pte;
+
+ vaddr = (unsigned long)__start_bss_decrypted;
+ vaddr_end = (unsigned long)__start_bss_decrypted_unused;
+ npages = (vaddr_end - vaddr) >> PAGE_SHIFT;
+ for (; vaddr < vaddr_end; vaddr += PAGE_SIZE) {
+ pte = lookup_address(vaddr, &level);
+ if (!pte || !pte_decrypted(*pte) || pte_none(*pte))
+ continue;
+
+ set_pte_enc(pte, level, (void *)vaddr);
+ }
+ vaddr = (unsigned long)__start_bss_decrypted;
+ snp_set_memory_private(vaddr, npages);
+}
+
+/* Stop new private<->shared conversions */
+void snp_kexec_stop_conversion(bool crash)
+{
+ /*
+ * Crash kernel reaches here with interrupts disabled: can't wait for
+ * conversions to finish.
+ *
+ * If race happened, just report and proceed.
+ */
+ bool wait_for_lock = !crash;
+
+ if (!stop_memory_enc_conversion(wait_for_lock))
+ pr_warn("Failed to finish shared<->private conversions\n");
+}
+
+void snp_kexec_unshare_mem(void)
+{
+ if (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
+ return;
+
+ unshare_all_memory();
+
+ unshare_all_bss_decrypted_memory();
+
+ if (kexec_last_addr_to_make_private) {
+ unsigned long size;
+ unsigned int level;
+ pte_t *pte;
+
+ /*
+ * Switch to using the MSR protocol to change this cpu's
+ * GHCB to private.
+ * All the per-cpu GHCBs have been switched back to private,
+ * so can't do any more GHCB calls to the hypervisor beyond
+ * this point till the kexec kernel starts running.
+ */
+ boot_ghcb = NULL;
+ sev_cfg.ghcbs_initialized = false;
+
+ pr_debug("boot ghcb 0x%lx\n", kexec_last_addr_to_make_private);
+ pte = lookup_address(kexec_last_addr_to_make_private, &level);
+ size = page_level_size(level);
+ set_pte_enc(pte, level, (void *)kexec_last_addr_to_make_private);
+ snp_set_memory_private(kexec_last_addr_to_make_private, (size / PAGE_SIZE));
+ }
+}
+
static int snp_set_vmsa(void *va, bool vmsa)
{
u64 attrs;
diff --git a/arch/x86/mm/mem_encrypt_amd.c b/arch/x86/mm/mem_encrypt_amd.c
index e7b67519ddb5..49c40c2ed809 100644
--- a/arch/x86/mm/mem_encrypt_amd.c
+++ b/arch/x86/mm/mem_encrypt_amd.c
@@ -468,6 +468,9 @@ void __init sme_early_init(void)
x86_platform.guest.enc_tlb_flush_required = amd_enc_tlb_flush_required;
x86_platform.guest.enc_cache_flush_required = amd_enc_cache_flush_required;
+ x86_platform.guest.enc_kexec_stop_conversion = snp_kexec_stop_conversion;
+ x86_platform.guest.enc_kexec_unshare_mem = snp_kexec_unshare_mem;
+
/*
* AMD-SEV-ES intercepts the RDMSR to read the X2APIC ID in the
* parallel bringup low level code. That raises #VC which cannot be
--
2.34.1From: Ashish Kalra <ashish.kalra@amd.com>
The patchset adds bits and pieces to get kexec (and crashkernel) work on
SNP guest.
The series is based off of and tested against Kirill Shutemov's tree:
https://github.com/intel/tdx.git guest-kexec
----
v6:
- Updated and restructured the commit message for patch 1/3 to
explain the issue in detail.
- Updated inline comments in patch 1/3 to explain the issue in
detail.
- Moved back to checking efi_setup for detecting if running
under kexec kernel.
v5:
- Removed sev_es_enabled() function and using sev_status directly to
check for SEV-ES/SEV-SNP guest.
- used --base option to generate patches to specify Kirill's TDX guest
kexec patches as prerequisite patches to fix kernel test robot
build errors.
v4:
- Rebased to current tip/master.
- Reviewed-bys from Sathya.
- Remove snp_kexec_unprep_rom_memory() as it is not needed any more as
SEV-SNP code is not validating the ROM range in probe_roms() anymore.
- Fix kernel test robot build error/warnings.
v3:
- Rebased;
- moved Keep page tables that maps E820_TYPE_ACPI patch to Kirill's tdx
guest kexec patch series.
- checking the md attribute instead of checking the efi_setup for
detecting if running under kexec kernel.
- added new sev_es_enabled() function.
- skip video memory access in decompressor for SEV-ES/SNP systems to
prevent guest termination as boot stage2 #VC handler does not handle
MMIO.
v2:
- address zeroing of unaccepted memory table mappings at all page table levels
adding phys_pte_init(), phys_pud_init() and phys_p4d_init().
- include skip efi_arch_mem_reserve() in case of kexec as part of this
patch set.
- rename last_address_shd_kexec to a more appropriate
kexec_last_address_to_make_private.
- remove duplicate code shared with TDX and use common interfaces
defined for SNP and TDX for kexec/kdump.
- remove set_pte_enc() dependency on pg_level_to_pfn() and make the
function simpler.
- rename unshare_pte() to make_pte_private().
- clarify and make the comment for using kexec_last_address_to_make_private
more understandable.
- general cleanup.
Ashish Kalra (3):
efi/x86: Fix EFI memory map corruption with kexec
x86/boot/compressed: Skip Video Memory access in Decompressor for
SEV-ES/SNP.
x86/snp: Convert shared memory back to private on kexec
arch/x86/boot/compressed/misc.c | 6 +-
arch/x86/include/asm/sev.h | 4 +
arch/x86/kernel/sev.c | 161 ++++++++++++++++++++++++++++++++
arch/x86/mm/mem_encrypt_amd.c | 3 +
arch/x86/platform/efi/quirks.c | 20 ++++
5 files changed, 192 insertions(+), 2 deletions(-)
base-commit: 7fcd76de8a7bc12e930ef383a157ce99d711715d
prerequisite-patch-id: a911f230c2524bd791c47f62f17f0a93cbf726b6
prerequisite-patch-id: bfe2fa046349978ac1825275eb205acecfbc22f3
prerequisite-patch-id: 5e60d292457c7cd98fd3e45c23127e9463b56a69
prerequisite-patch-id: 1f97d0a2edb7509dd58276f628d1a4bda62c154c
prerequisite-patch-id: cbc2507b5c2810c3015aaf836d774d32f969c19a
prerequisite-patch-id: cbdfea1e50ecb3b4cee3a25a27df4d35bd95d532
prerequisite-patch-id: 99382c42348b9a076ba930eca0dfc9d000ec951d
prerequisite-patch-id: 469a0a3c78b0eca82527cd85e2205fb8fb89d645
prerequisite-patch-id: 2974ef211db5253d9782018e352d2a6ff0b0ef54
prerequisite-patch-id: 2cfffd80947941892421dae99b7fa0f9f9715884
prerequisite-patch-id: 466c2cb9f0a107bbd1dbd8526f4eff2bdb55f1ce
prerequisite-patch-id: d4966ae63e86d24b0bf578da4dae871cd9002b12
prerequisite-patch-id: fccde6f1fa385b5af0195f81fcb95acd71822428
prerequisite-patch-id: 16048ee15e392b0b9217b8923939b0059311abd2
prerequisite-patch-id: 5c9ae9aa294f72f63ae2c3551507dfbd92525803
prerequisite-patch-id: 758bdb686290c018cbd5b7d005354019f9d15248
prerequisite-patch-id: c85fd0bb6d183a40da73720eaa607481b1d51daf
prerequisite-patch-id: 60760e0c98ab7ccd2ca22ae3e9f20ff5a94c6e91
--
2.34.1
From: Ashish Kalra <ashish.kalra@amd.com>
With SNP guest kexec observe the following efi memmap corruption :
[ 0.000000] efi: EFI v2.7 by EDK II
[ 0.000000] efi: SMBIOS=0x7e33f000 SMBIOS 3.0=0x7e33d000 ACPI=0x7e57e000 ACPI 2.0=0x7e57e014 MEMATTR=0x7cc3c018 Unaccepted=0x7c09e018
[ 0.000000] efi: [Firmware Bug]: Invalid EFI memory map entries:
[ 0.000000] efi: mem03: [type=269370880|attr=0x0e42100e42180e41] range=[0x0486200e41038c18-0x200e898a0eee713ac17] (invalid)
[ 0.000000] efi: mem04: [type=12336|attr=0x0e410686300e4105] range=[0x100e420000000176-0x8c290f26248d200e175] (invalid)
[ 0.000000] efi: mem06: [type=1124304408|attr=0x000030b400000028] range=[0x0e51300e45280e77-0xb44ed2142f460c1e76] (invalid)
[ 0.000000] efi: mem08: [type=68|attr=0x300e540583280e41] range=[0x0000011affff3cd8-0x486200e54b38c0bcd7] (invalid)
[ 0.000000] efi: mem10: [type=1107529240|attr=0x0e42280e41300e41] range=[0x300e41058c280e42-0x38010ae54c5c328ee41] (invalid)
[ 0.000000] efi: mem11: [type=189335566|attr=0x048d200e42038e18] range=[0x0000318c00000048-0xe42029228ce4200047] (invalid)
[ 0.000000] efi: mem12: [type=239142534|attr=0x0000002400000b4b] range=[0x0e41380e0a7d700e-0x80f26238f22bfe500d] (invalid)
[ 0.000000] efi: mem14: [type=239207055|attr=0x0e41300e43380e0a] range=[0x8c280e42048d200e-0xc70b028f2f27cc0a00d] (invalid)
[ 0.000000] efi: mem15: [type=239210510|attr=0x00080e660b47080e] range=[0x0000324c0000001c-0xa78028634ce490001b] (invalid)
[ 0.000000] efi: mem16: [type=4294848528|attr=0x0000329400000014] range=[0x0e410286100e4100-0x80f252036a218f20ff] (invalid)
[ 0.000000] efi: mem19: [type=2250772033|attr=0x42180e42200e4328] range=[0x41280e0ab9020683-0xe0e538c28b39e62682] (invalid)
[ 0.000000] efi: mem20: [type=16| | | | | | | | | | |WB| |WC| ] range=[0x00000008ffff4438-0xffff44340090333c437] (invalid)
[ 0.000000] efi: mem22: [Reserved |attr=0x000000c1ffff4420] range=[0xffff442400003398-0x1033a04240003f397] (invalid)
[ 0.000000] efi: mem23: [type=1141080856|attr=0x080e41100e43180e] range=[0x280e66300e4b280e-0x440dc5ee7141f4c080d] (invalid)
[ 0.000000] efi: mem25: [Reserved |attr=0x0000000affff44a0] range=[0xffff44a400003428-0x1034304a400013427] (invalid)
[ 0.000000] efi: mem28: [type=16| | | | | | | | | | |WB| |WC| ] range=[0x0000000affff4488-0xffff448400b034bc487] (invalid)
[ 0.000000] efi: mem30: [Reserved |attr=0x0000000affff4470] range=[0xffff447400003518-0x10352047400013517] (invalid)
[ 0.000000] efi: mem33: [type=16| | | | | | | | | | |WB| |WC| ] range=[0x0000000affff4458-0xffff445400b035ac457] (invalid)
[ 0.000000] efi: mem35: [type=269372416|attr=0x0e42100e42180e41] range=[0x0486200e44038c18-0x200e8b8a0eee823ac17] (invalid)
[ 0.000000] efi: mem37: [type=2351435330|attr=0x0e42100e42180e42] range=[0x470783380e410686-0x2002b2a041c2141e685] (invalid)
[ 0.000000] efi: mem38: [type=1093668417|attr=0x100e420000000270] range=[0x42100e42180e4220-0xfff366a4e421b78c21f] (invalid)
[ 0.000000] efi: mem39: [type=76357646|attr=0x180e42200e42280e] range=[0x0e410686300e4105-0x4130f251a0710ae5104] (invalid)
[ 0.000000] efi: mem40: [type=940444268|attr=0x0e42200e42280e41] range=[0x180e42200e42280e-0x300fc71c300b4f2480d] (invalid)
[ 0.000000] efi: mem41: [MMIO |attr=0x8c280e42048d200e] range=[0xffff479400003728-0x42138e0c87820292727] (invalid)
[ 0.000000] efi: mem42: [type=1191674680|attr=0x0000004c0000000b] range=[0x300e41380e0a0246-0x470b0f26238f22b8245] (invalid)
[ 0.000000] efi: mem43: [type=2010|attr=0x0301f00e4d078338] range=[0x45038e180e42028f-0xe4556bf118f282528e] (invalid)
[ 0.000000] efi: mem44: [type=1109921345|attr=0x300e44000000006c] range=[0x44080e42100e4218-0xfff39254e42138ac217] (invalid)
...
This EFI memap corruption is happening with efi_arch_mem_reserve() invocation in case of kexec boot.
( efi_arch_mem_reserve() is invoked with the following call-stack: )
[ 0.310010] efi_arch_mem_reserve+0xb1/0x220
[ 0.311382] efi_mem_reserve+0x36/0x60
[ 0.311973] efi_bgrt_init+0x17d/0x1a0
[ 0.313265] acpi_parse_bgrt+0x12/0x20
[ 0.313858] acpi_table_parse+0x77/0xd0
[ 0.314463] acpi_boot_init+0x362/0x630
[ 0.315069] setup_arch+0xa88/0xf80
[ 0.315629] start_kernel+0x68/0xa90
[ 0.316194] x86_64_start_reservations+0x1c/0x30
[ 0.316921] x86_64_start_kernel+0xbf/0x110
[ 0.317582] common_startup_64+0x13e/0x141
efi_arch_mem_reserve() calls efi_memmap_alloc() to allocate memory for
EFI memory map and due to early allocation it uses memblock allocation.
Later during boot, efi_enter_virtual_mode() calls kexec_enter_virtual_mode()
in case of a kexec-ed kernel boot.
This function kexec_enter_virtual_mode() installs the new EFI memory map by
calling efi_memmap_init_late() which remaps the efi_memmap physically allocated
in efi_arch_mem_reserve(), but this remapping is still using memblock allocation.
Subsequently, when memblock is freed later in boot flow, this remapped
efi_memmap will have random corruption (similar to a use-after-free scenario).
The corrupted EFI memory map is then passed to the next kexec-ed kernel
which causes a panic when trying to use the corrupted EFI memory map.
Fix this EFI memory map corruption by skipping efi_arch_mem_reserve() for kexec.
Additionally, skipping this function for kexec altogther makes sense
as for kexec use case need to use the the EFI memmap passed from first
kernel via setup_data and avoid any additional EFI memory map
additions/updates.
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
arch/x86/platform/efi/quirks.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index f0cc00032751..af7126d9c540 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -258,6 +258,26 @@ void __init efi_arch_mem_reserve(phys_addr_t addr, u64 size)
int num_entries;
void *new;
+ /*
+ * efi_arch_mem_reserve() calls efi_memmap_alloc() to allocate memory for
+ * EFI memory map and due to early allocation it uses memblock allocation.
+ * Later during boot, efi_enter_virtual_mode() calls kexec_enter_virtual_mode()
+ * in case of a kexec-ed kernel boot. This function kexec_enter_virtual_mode()
+ * installs the new EFI memory map by calling efi_memmap_init_late() which
+ * remaps the efi_memmap physically allocated here in efi_arch_mem_reserve(),
+ * but this remapping is still using memblock allocation.
+ * Subsequently, when memblock is freed later in boot flow, this remapped
+ * efi_memmap will have random corruption (similar to a use-after-free scenario).
+ * The corrupted EFI memory map is then passed to the next kexec-ed kernel
+ * which causes a panic when trying to use the corrupted EFI memory map.
+ * Additionally, skipping this function for kexec altogther makes sense
+ * as for kexec use case need to use the the EFI memmap passed from first
+ * kernel via setup_data and avoid any additional EFI memory map
+ * additions/updates.
+ */
+ if (efi_setup)
+ return;
+
if (efi_mem_desc_lookup(addr, &md) ||
md.type != EFI_BOOT_SERVICES_DATA) {
pr_err("Failed to lookup EFI memory descriptor for %pa\n", &addr);
--
2.34.1On Fri, Apr 26, 2024 at 04:33:48PM +0000, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
>
> With SNP guest kexec observe the following efi memmap corruption :
>
> [ 0.000000] efi: EFI v2.7 by EDK II
> [ 0.000000] efi: SMBIOS=0x7e33f000 SMBIOS 3.0=0x7e33d000 ACPI=0x7e57e000 ACPI 2.0=0x7e57e014 MEMATTR=0x7cc3c018 Unaccepted=0x7c09e018
> [ 0.000000] efi: [Firmware Bug]: Invalid EFI memory map entries:
> [ 0.000000] efi: mem03: [type=269370880|attr=0x0e42100e42180e41] range=[0x0486200e41038c18-0x200e898a0eee713ac17] (invalid)
> [ 0.000000] efi: mem04: [type=12336|attr=0x0e410686300e4105] range=[0x100e420000000176-0x8c290f26248d200e175] (invalid)
> [ 0.000000] efi: mem06: [type=1124304408|attr=0x000030b400000028] range=[0x0e51300e45280e77-0xb44ed2142f460c1e76] (invalid)
> [ 0.000000] efi: mem08: [type=68|attr=0x300e540583280e41] range=[0x0000011affff3cd8-0x486200e54b38c0bcd7] (invalid)
> [ 0.000000] efi: mem10: [type=1107529240|attr=0x0e42280e41300e41] range=[0x300e41058c280e42-0x38010ae54c5c328ee41] (invalid)
> [ 0.000000] efi: mem11: [type=189335566|attr=0x048d200e42038e18] range=[0x0000318c00000048-0xe42029228ce4200047] (invalid)
> [ 0.000000] efi: mem12: [type=239142534|attr=0x0000002400000b4b] range=[0x0e41380e0a7d700e-0x80f26238f22bfe500d] (invalid)
> [ 0.000000] efi: mem14: [type=239207055|attr=0x0e41300e43380e0a] range=[0x8c280e42048d200e-0xc70b028f2f27cc0a00d] (invalid)
> [ 0.000000] efi: mem15: [type=239210510|attr=0x00080e660b47080e] range=[0x0000324c0000001c-0xa78028634ce490001b] (invalid)
> [ 0.000000] efi: mem16: [type=4294848528|attr=0x0000329400000014] range=[0x0e410286100e4100-0x80f252036a218f20ff] (invalid)
> [ 0.000000] efi: mem19: [type=2250772033|attr=0x42180e42200e4328] range=[0x41280e0ab9020683-0xe0e538c28b39e62682] (invalid)
> [ 0.000000] efi: mem20: [type=16| | | | | | | | | | |WB| |WC| ] range=[0x00000008ffff4438-0xffff44340090333c437] (invalid)
> [ 0.000000] efi: mem22: [Reserved |attr=0x000000c1ffff4420] range=[0xffff442400003398-0x1033a04240003f397] (invalid)
> [ 0.000000] efi: mem23: [type=1141080856|attr=0x080e41100e43180e] range=[0x280e66300e4b280e-0x440dc5ee7141f4c080d] (invalid)
> [ 0.000000] efi: mem25: [Reserved |attr=0x0000000affff44a0] range=[0xffff44a400003428-0x1034304a400013427] (invalid)
> [ 0.000000] efi: mem28: [type=16| | | | | | | | | | |WB| |WC| ] range=[0x0000000affff4488-0xffff448400b034bc487] (invalid)
> [ 0.000000] efi: mem30: [Reserved |attr=0x0000000affff4470] range=[0xffff447400003518-0x10352047400013517] (invalid)
> [ 0.000000] efi: mem33: [type=16| | | | | | | | | | |WB| |WC| ] range=[0x0000000affff4458-0xffff445400b035ac457] (invalid)
> [ 0.000000] efi: mem35: [type=269372416|attr=0x0e42100e42180e41] range=[0x0486200e44038c18-0x200e8b8a0eee823ac17] (invalid)
> [ 0.000000] efi: mem37: [type=2351435330|attr=0x0e42100e42180e42] range=[0x470783380e410686-0x2002b2a041c2141e685] (invalid)
> [ 0.000000] efi: mem38: [type=1093668417|attr=0x100e420000000270] range=[0x42100e42180e4220-0xfff366a4e421b78c21f] (invalid)
> [ 0.000000] efi: mem39: [type=76357646|attr=0x180e42200e42280e] range=[0x0e410686300e4105-0x4130f251a0710ae5104] (invalid)
> [ 0.000000] efi: mem40: [type=940444268|attr=0x0e42200e42280e41] range=[0x180e42200e42280e-0x300fc71c300b4f2480d] (invalid)
> [ 0.000000] efi: mem41: [MMIO |attr=0x8c280e42048d200e] range=[0xffff479400003728-0x42138e0c87820292727] (invalid)
> [ 0.000000] efi: mem42: [type=1191674680|attr=0x0000004c0000000b] range=[0x300e41380e0a0246-0x470b0f26238f22b8245] (invalid)
> [ 0.000000] efi: mem43: [type=2010|attr=0x0301f00e4d078338] range=[0x45038e180e42028f-0xe4556bf118f282528e] (invalid)
> [ 0.000000] efi: mem44: [type=1109921345|attr=0x300e44000000006c] range=[0x44080e42100e4218-0xfff39254e42138ac217] (invalid)
> ...
>
> This EFI memap corruption is happening with efi_arch_mem_reserve() invocation in case of kexec boot.
>
> ( efi_arch_mem_reserve() is invoked with the following call-stack: )
>
> [ 0.310010] efi_arch_mem_reserve+0xb1/0x220
> [ 0.311382] efi_mem_reserve+0x36/0x60
> [ 0.311973] efi_bgrt_init+0x17d/0x1a0
> [ 0.313265] acpi_parse_bgrt+0x12/0x20
> [ 0.313858] acpi_table_parse+0x77/0xd0
> [ 0.314463] acpi_boot_init+0x362/0x630
> [ 0.315069] setup_arch+0xa88/0xf80
> [ 0.315629] start_kernel+0x68/0xa90
> [ 0.316194] x86_64_start_reservations+0x1c/0x30
> [ 0.316921] x86_64_start_kernel+0xbf/0x110
> [ 0.317582] common_startup_64+0x13e/0x141
>
> efi_arch_mem_reserve() calls efi_memmap_alloc() to allocate memory for
> EFI memory map and due to early allocation it uses memblock allocation.
>
> Later during boot, efi_enter_virtual_mode() calls kexec_enter_virtual_mode()
> in case of a kexec-ed kernel boot.
>
> This function kexec_enter_virtual_mode() installs the new EFI memory map by
> calling efi_memmap_init_late() which remaps the efi_memmap physically allocated
> in efi_arch_mem_reserve(), but this remapping is still using memblock allocation.
>
> Subsequently, when memblock is freed later in boot flow, this remapped
> efi_memmap will have random corruption (similar to a use-after-free scenario).
>
> The corrupted EFI memory map is then passed to the next kexec-ed kernel
> which causes a panic when trying to use the corrupted EFI memory map.
>
> Fix this EFI memory map corruption by skipping efi_arch_mem_reserve() for kexec.
>
> Additionally, skipping this function for kexec altogther makes sense
> as for kexec use case need to use the the EFI memmap passed from first
> kernel via setup_data and avoid any additional EFI memory map
> additions/updates.
>
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
> arch/x86/platform/efi/quirks.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
> index f0cc00032751..af7126d9c540 100644
> --- a/arch/x86/platform/efi/quirks.c
> +++ b/arch/x86/platform/efi/quirks.c
> @@ -258,6 +258,26 @@ void __init efi_arch_mem_reserve(phys_addr_t addr, u64 size)
> int num_entries;
> void *new;
>
> + /*
> + * efi_arch_mem_reserve() calls efi_memmap_alloc() to allocate memory for
> + * EFI memory map and due to early allocation it uses memblock allocation.
> + * Later during boot, efi_enter_virtual_mode() calls kexec_enter_virtual_mode()
> + * in case of a kexec-ed kernel boot. This function kexec_enter_virtual_mode()
> + * installs the new EFI memory map by calling efi_memmap_init_late() which
> + * remaps the efi_memmap physically allocated here in efi_arch_mem_reserve(),
> + * but this remapping is still using memblock allocation.
> + * Subsequently, when memblock is freed later in boot flow, this remapped
> + * efi_memmap will have random corruption (similar to a use-after-free scenario).
> + * The corrupted EFI memory map is then passed to the next kexec-ed kernel
> + * which causes a panic when trying to use the corrupted EFI memory map.
> + * Additionally, skipping this function for kexec altogther makes sense
> + * as for kexec use case need to use the the EFI memmap passed from first
> + * kernel via setup_data and avoid any additional EFI memory map
> + * additions/updates.
> + */
> + if (efi_setup)
> + return;
> +
efi_mem_reserve is used to reserve boot service memory eg. bgrt, but
it is not necessary for kexec boot, as there are no boot services in
kexec reboot at all after the 1st kernel ExitBootServices().
The UEFI memmap passed to kexec kernel includes not only the runtime
service memory map but also the boot service memory ranges which were
reserved by the 1st kernel with efi_mem_reserve, and those boot service
memory ranges have already been marked "EFI_MEMORY_RUNTIME" attribute.
Take example of bgrt, the saved memory is there only for people to check
the bgrt image info via /sys/firmware/acpi/bgrt/*, and it is not used in
early boot phase by boot services.
Above is the reason why the efi_mem_reserve can be skipped for kexec
booting. But as I suggested before I personally think that checking
EFI_MEMORY_RUNTIME attribute set or not looks better than checking
efi_setup.
> if (efi_mem_desc_lookup(addr, &md) ||
> md.type != EFI_BOOT_SERVICES_DATA) {
> pr_err("Failed to lookup EFI memory descriptor for %pa\n", &addr);
> --
> 2.34.1
>
>
On 5/9/2024 4:56 AM, Ruirui Yang wrote:
> On Fri, Apr 26, 2024 at 04:33:48PM +0000, Ashish Kalra wrote:
>> From: Ashish Kalra <ashish.kalra@amd.com>
>>
>> With SNP guest kexec observe the following efi memmap corruption :
>>
>> [ 0.000000] efi: EFI v2.7 by EDK II
>> [ 0.000000] efi: SMBIOS=0x7e33f000 SMBIOS 3.0=0x7e33d000 ACPI=0x7e57e000 ACPI 2.0=0x7e57e014 MEMATTR=0x7cc3c018 Unaccepted=0x7c09e018
>> [ 0.000000] efi: [Firmware Bug]: Invalid EFI memory map entries:
>> [ 0.000000] efi: mem03: [type=269370880|attr=0x0e42100e42180e41] range=[0x0486200e41038c18-0x200e898a0eee713ac17] (invalid)
>> [ 0.000000] efi: mem04: [type=12336|attr=0x0e410686300e4105] range=[0x100e420000000176-0x8c290f26248d200e175] (invalid)
>> [ 0.000000] efi: mem06: [type=1124304408|attr=0x000030b400000028] range=[0x0e51300e45280e77-0xb44ed2142f460c1e76] (invalid)
>> [ 0.000000] efi: mem08: [type=68|attr=0x300e540583280e41] range=[0x0000011affff3cd8-0x486200e54b38c0bcd7] (invalid)
>> [ 0.000000] efi: mem10: [type=1107529240|attr=0x0e42280e41300e41] range=[0x300e41058c280e42-0x38010ae54c5c328ee41] (invalid)
>> [ 0.000000] efi: mem11: [type=189335566|attr=0x048d200e42038e18] range=[0x0000318c00000048-0xe42029228ce4200047] (invalid)
>> [ 0.000000] efi: mem12: [type=239142534|attr=0x0000002400000b4b] range=[0x0e41380e0a7d700e-0x80f26238f22bfe500d] (invalid)
>> [ 0.000000] efi: mem14: [type=239207055|attr=0x0e41300e43380e0a] range=[0x8c280e42048d200e-0xc70b028f2f27cc0a00d] (invalid)
>> [ 0.000000] efi: mem15: [type=239210510|attr=0x00080e660b47080e] range=[0x0000324c0000001c-0xa78028634ce490001b] (invalid)
>> [ 0.000000] efi: mem16: [type=4294848528|attr=0x0000329400000014] range=[0x0e410286100e4100-0x80f252036a218f20ff] (invalid)
>> [ 0.000000] efi: mem19: [type=2250772033|attr=0x42180e42200e4328] range=[0x41280e0ab9020683-0xe0e538c28b39e62682] (invalid)
>> [ 0.000000] efi: mem20: [type=16| | | | | | | | | | |WB| |WC| ] range=[0x00000008ffff4438-0xffff44340090333c437] (invalid)
>> [ 0.000000] efi: mem22: [Reserved |attr=0x000000c1ffff4420] range=[0xffff442400003398-0x1033a04240003f397] (invalid)
>> [ 0.000000] efi: mem23: [type=1141080856|attr=0x080e41100e43180e] range=[0x280e66300e4b280e-0x440dc5ee7141f4c080d] (invalid)
>> [ 0.000000] efi: mem25: [Reserved |attr=0x0000000affff44a0] range=[0xffff44a400003428-0x1034304a400013427] (invalid)
>> [ 0.000000] efi: mem28: [type=16| | | | | | | | | | |WB| |WC| ] range=[0x0000000affff4488-0xffff448400b034bc487] (invalid)
>> [ 0.000000] efi: mem30: [Reserved |attr=0x0000000affff4470] range=[0xffff447400003518-0x10352047400013517] (invalid)
>> [ 0.000000] efi: mem33: [type=16| | | | | | | | | | |WB| |WC| ] range=[0x0000000affff4458-0xffff445400b035ac457] (invalid)
>> [ 0.000000] efi: mem35: [type=269372416|attr=0x0e42100e42180e41] range=[0x0486200e44038c18-0x200e8b8a0eee823ac17] (invalid)
>> [ 0.000000] efi: mem37: [type=2351435330|attr=0x0e42100e42180e42] range=[0x470783380e410686-0x2002b2a041c2141e685] (invalid)
>> [ 0.000000] efi: mem38: [type=1093668417|attr=0x100e420000000270] range=[0x42100e42180e4220-0xfff366a4e421b78c21f] (invalid)
>> [ 0.000000] efi: mem39: [type=76357646|attr=0x180e42200e42280e] range=[0x0e410686300e4105-0x4130f251a0710ae5104] (invalid)
>> [ 0.000000] efi: mem40: [type=940444268|attr=0x0e42200e42280e41] range=[0x180e42200e42280e-0x300fc71c300b4f2480d] (invalid)
>> [ 0.000000] efi: mem41: [MMIO |attr=0x8c280e42048d200e] range=[0xffff479400003728-0x42138e0c87820292727] (invalid)
>> [ 0.000000] efi: mem42: [type=1191674680|attr=0x0000004c0000000b] range=[0x300e41380e0a0246-0x470b0f26238f22b8245] (invalid)
>> [ 0.000000] efi: mem43: [type=2010|attr=0x0301f00e4d078338] range=[0x45038e180e42028f-0xe4556bf118f282528e] (invalid)
>> [ 0.000000] efi: mem44: [type=1109921345|attr=0x300e44000000006c] range=[0x44080e42100e4218-0xfff39254e42138ac217] (invalid)
>> ...
>>
>> This EFI memap corruption is happening with efi_arch_mem_reserve() invocation in case of kexec boot.
>>
>> ( efi_arch_mem_reserve() is invoked with the following call-stack: )
>>
>> [ 0.310010] efi_arch_mem_reserve+0xb1/0x220
>> [ 0.311382] efi_mem_reserve+0x36/0x60
>> [ 0.311973] efi_bgrt_init+0x17d/0x1a0
>> [ 0.313265] acpi_parse_bgrt+0x12/0x20
>> [ 0.313858] acpi_table_parse+0x77/0xd0
>> [ 0.314463] acpi_boot_init+0x362/0x630
>> [ 0.315069] setup_arch+0xa88/0xf80
>> [ 0.315629] start_kernel+0x68/0xa90
>> [ 0.316194] x86_64_start_reservations+0x1c/0x30
>> [ 0.316921] x86_64_start_kernel+0xbf/0x110
>> [ 0.317582] common_startup_64+0x13e/0x141
>>
>> efi_arch_mem_reserve() calls efi_memmap_alloc() to allocate memory for
>> EFI memory map and due to early allocation it uses memblock allocation.
>>
>> Later during boot, efi_enter_virtual_mode() calls kexec_enter_virtual_mode()
>> in case of a kexec-ed kernel boot.
>>
>> This function kexec_enter_virtual_mode() installs the new EFI memory map by
>> calling efi_memmap_init_late() which remaps the efi_memmap physically allocated
>> in efi_arch_mem_reserve(), but this remapping is still using memblock allocation.
>>
>> Subsequently, when memblock is freed later in boot flow, this remapped
>> efi_memmap will have random corruption (similar to a use-after-free scenario).
>>
>> The corrupted EFI memory map is then passed to the next kexec-ed kernel
>> which causes a panic when trying to use the corrupted EFI memory map.
>>
>> Fix this EFI memory map corruption by skipping efi_arch_mem_reserve() for kexec.
>>
>> Additionally, skipping this function for kexec altogther makes sense
>> as for kexec use case need to use the the EFI memmap passed from first
>> kernel via setup_data and avoid any additional EFI memory map
>> additions/updates.
>>
>> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
>> ---
>> arch/x86/platform/efi/quirks.c | 20 ++++++++++++++++++++
>> 1 file changed, 20 insertions(+)
>>
>> diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
>> index f0cc00032751..af7126d9c540 100644
>> --- a/arch/x86/platform/efi/quirks.c
>> +++ b/arch/x86/platform/efi/quirks.c
>> @@ -258,6 +258,26 @@ void __init efi_arch_mem_reserve(phys_addr_t addr, u64 size)
>> int num_entries;
>> void *new;
>>
>> + /*
>> + * efi_arch_mem_reserve() calls efi_memmap_alloc() to allocate memory for
>> + * EFI memory map and due to early allocation it uses memblock allocation.
>> + * Later during boot, efi_enter_virtual_mode() calls kexec_enter_virtual_mode()
>> + * in case of a kexec-ed kernel boot. This function kexec_enter_virtual_mode()
>> + * installs the new EFI memory map by calling efi_memmap_init_late() which
>> + * remaps the efi_memmap physically allocated here in efi_arch_mem_reserve(),
>> + * but this remapping is still using memblock allocation.
>> + * Subsequently, when memblock is freed later in boot flow, this remapped
>> + * efi_memmap will have random corruption (similar to a use-after-free scenario).
>> + * The corrupted EFI memory map is then passed to the next kexec-ed kernel
>> + * which causes a panic when trying to use the corrupted EFI memory map.
>> + * Additionally, skipping this function for kexec altogther makes sense
>> + * as for kexec use case need to use the the EFI memmap passed from first
>> + * kernel via setup_data and avoid any additional EFI memory map
>> + * additions/updates.
>> + */
>> + if (efi_setup)
>> + return;
>> +
> efi_mem_reserve is used to reserve boot service memory eg. bgrt, but
> it is not necessary for kexec boot, as there are no boot services in
> kexec reboot at all after the 1st kernel ExitBootServices().
>
> The UEFI memmap passed to kexec kernel includes not only the runtime
> service memory map but also the boot service memory ranges which were
> reserved by the 1st kernel with efi_mem_reserve, and those boot service
> memory ranges have already been marked "EFI_MEMORY_RUNTIME" attribute.
>
> Take example of bgrt, the saved memory is there only for people to check
> the bgrt image info via /sys/firmware/acpi/bgrt/*, and it is not used in
> early boot phase by boot services.
>
> Above is the reason why the efi_mem_reserve can be skipped for kexec
> booting. But as I suggested before I personally think that checking
> EFI_MEMORY_RUNTIME attribute set or not looks better than checking
> efi_setup.
Thanks for reviewing the patch.
I will move back to checking the md attribute instead of checking
efi_setup as i was doing previously and resubmit this patch.
Thanks, Ashish
>
>> if (efi_mem_desc_lookup(addr, &md) ||
>> md.type != EFI_BOOT_SERVICES_DATA) {
>> pr_err("Failed to lookup EFI memory descriptor for %pa\n", &addr);
>> --
>> 2.34.1
>>
>>
On Thu, 9 May 2024 at 17:56, Ruirui Yang <ruirui.yang@linux.dev> wrote: > > On Fri, Apr 26, 2024 at 04:33:48PM +0000, Ashish Kalra wrote: > > From: Ashish Kalra <ashish.kalra@amd.com> > > > > With SNP guest kexec observe the following efi memmap corruption : > > > > [ 0.000000] efi: EFI v2.7 by EDK II > > [ 0.000000] efi: SMBIOS=0x7e33f000 SMBIOS 3.0=0x7e33d000 ACPI=0x7e57e000 ACPI 2.0=0x7e57e014 MEMATTR=0x7cc3c018 Unaccepted=0x7c09e018 > > [ 0.000000] efi: [Firmware Bug]: Invalid EFI memory map entries: > > [ 0.000000] efi: mem03: [type=269370880|attr=0x0e42100e42180e41] range=[0x0486200e41038c18-0x200e898a0eee713ac17] (invalid) > > [ 0.000000] efi: mem04: [type=12336|attr=0x0e410686300e4105] range=[0x100e420000000176-0x8c290f26248d200e175] (invalid) > > [ 0.000000] efi: mem06: [type=1124304408|attr=0x000030b400000028] range=[0x0e51300e45280e77-0xb44ed2142f460c1e76] (invalid) > > [ 0.000000] efi: mem08: [type=68|attr=0x300e540583280e41] range=[0x0000011affff3cd8-0x486200e54b38c0bcd7] (invalid) > > [ 0.000000] efi: mem10: [type=1107529240|attr=0x0e42280e41300e41] range=[0x300e41058c280e42-0x38010ae54c5c328ee41] (invalid) > > [ 0.000000] efi: mem11: [type=189335566|attr=0x048d200e42038e18] range=[0x0000318c00000048-0xe42029228ce4200047] (invalid) > > [ 0.000000] efi: mem12: [type=239142534|attr=0x0000002400000b4b] range=[0x0e41380e0a7d700e-0x80f26238f22bfe500d] (invalid) > > [ 0.000000] efi: mem14: [type=239207055|attr=0x0e41300e43380e0a] range=[0x8c280e42048d200e-0xc70b028f2f27cc0a00d] (invalid) > > [ 0.000000] efi: mem15: [type=239210510|attr=0x00080e660b47080e] range=[0x0000324c0000001c-0xa78028634ce490001b] (invalid) > > [ 0.000000] efi: mem16: [type=4294848528|attr=0x0000329400000014] range=[0x0e410286100e4100-0x80f252036a218f20ff] (invalid) > > [ 0.000000] efi: mem19: [type=2250772033|attr=0x42180e42200e4328] range=[0x41280e0ab9020683-0xe0e538c28b39e62682] (invalid) > > [ 0.000000] efi: mem20: [type=16| | | | | | | | | | |WB| |WC| ] range=[0x00000008ffff4438-0xffff44340090333c437] (invalid) > > [ 0.000000] efi: mem22: [Reserved |attr=0x000000c1ffff4420] range=[0xffff442400003398-0x1033a04240003f397] (invalid) > > [ 0.000000] efi: mem23: [type=1141080856|attr=0x080e41100e43180e] range=[0x280e66300e4b280e-0x440dc5ee7141f4c080d] (invalid) > > [ 0.000000] efi: mem25: [Reserved |attr=0x0000000affff44a0] range=[0xffff44a400003428-0x1034304a400013427] (invalid) > > [ 0.000000] efi: mem28: [type=16| | | | | | | | | | |WB| |WC| ] range=[0x0000000affff4488-0xffff448400b034bc487] (invalid) > > [ 0.000000] efi: mem30: [Reserved |attr=0x0000000affff4470] range=[0xffff447400003518-0x10352047400013517] (invalid) > > [ 0.000000] efi: mem33: [type=16| | | | | | | | | | |WB| |WC| ] range=[0x0000000affff4458-0xffff445400b035ac457] (invalid) > > [ 0.000000] efi: mem35: [type=269372416|attr=0x0e42100e42180e41] range=[0x0486200e44038c18-0x200e8b8a0eee823ac17] (invalid) > > [ 0.000000] efi: mem37: [type=2351435330|attr=0x0e42100e42180e42] range=[0x470783380e410686-0x2002b2a041c2141e685] (invalid) > > [ 0.000000] efi: mem38: [type=1093668417|attr=0x100e420000000270] range=[0x42100e42180e4220-0xfff366a4e421b78c21f] (invalid) > > [ 0.000000] efi: mem39: [type=76357646|attr=0x180e42200e42280e] range=[0x0e410686300e4105-0x4130f251a0710ae5104] (invalid) > > [ 0.000000] efi: mem40: [type=940444268|attr=0x0e42200e42280e41] range=[0x180e42200e42280e-0x300fc71c300b4f2480d] (invalid) > > [ 0.000000] efi: mem41: [MMIO |attr=0x8c280e42048d200e] range=[0xffff479400003728-0x42138e0c87820292727] (invalid) > > [ 0.000000] efi: mem42: [type=1191674680|attr=0x0000004c0000000b] range=[0x300e41380e0a0246-0x470b0f26238f22b8245] (invalid) > > [ 0.000000] efi: mem43: [type=2010|attr=0x0301f00e4d078338] range=[0x45038e180e42028f-0xe4556bf118f282528e] (invalid) > > [ 0.000000] efi: mem44: [type=1109921345|attr=0x300e44000000006c] range=[0x44080e42100e4218-0xfff39254e42138ac217] (invalid) > > ... > > > > This EFI memap corruption is happening with efi_arch_mem_reserve() invocation in case of kexec boot. > > > > ( efi_arch_mem_reserve() is invoked with the following call-stack: ) > > > > [ 0.310010] efi_arch_mem_reserve+0xb1/0x220 > > [ 0.311382] efi_mem_reserve+0x36/0x60 > > [ 0.311973] efi_bgrt_init+0x17d/0x1a0 > > [ 0.313265] acpi_parse_bgrt+0x12/0x20 > > [ 0.313858] acpi_table_parse+0x77/0xd0 > > [ 0.314463] acpi_boot_init+0x362/0x630 > > [ 0.315069] setup_arch+0xa88/0xf80 > > [ 0.315629] start_kernel+0x68/0xa90 > > [ 0.316194] x86_64_start_reservations+0x1c/0x30 > > [ 0.316921] x86_64_start_kernel+0xbf/0x110 > > [ 0.317582] common_startup_64+0x13e/0x141 > > > > efi_arch_mem_reserve() calls efi_memmap_alloc() to allocate memory for > > EFI memory map and due to early allocation it uses memblock allocation. > > > > Later during boot, efi_enter_virtual_mode() calls kexec_enter_virtual_mode() > > in case of a kexec-ed kernel boot. > > > > This function kexec_enter_virtual_mode() installs the new EFI memory map by > > calling efi_memmap_init_late() which remaps the efi_memmap physically allocated > > in efi_arch_mem_reserve(), but this remapping is still using memblock allocation. > > > > Subsequently, when memblock is freed later in boot flow, this remapped > > efi_memmap will have random corruption (similar to a use-after-free scenario). > > > > The corrupted EFI memory map is then passed to the next kexec-ed kernel > > which causes a panic when trying to use the corrupted EFI memory map. > > > > Fix this EFI memory map corruption by skipping efi_arch_mem_reserve() for kexec. > > > > Additionally, skipping this function for kexec altogther makes sense > > as for kexec use case need to use the the EFI memmap passed from first > > kernel via setup_data and avoid any additional EFI memory map > > additions/updates. > > > > Signed-off-by: Ashish Kalra <ashish.kalra@amd.com> > > --- > > arch/x86/platform/efi/quirks.c | 20 ++++++++++++++++++++ > > 1 file changed, 20 insertions(+) > > > > diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c > > index f0cc00032751..af7126d9c540 100644 > > --- a/arch/x86/platform/efi/quirks.c > > +++ b/arch/x86/platform/efi/quirks.c > > @@ -258,6 +258,26 @@ void __init efi_arch_mem_reserve(phys_addr_t addr, u64 size) > > int num_entries; > > void *new; > > > > + /* > > + * efi_arch_mem_reserve() calls efi_memmap_alloc() to allocate memory for > > + * EFI memory map and due to early allocation it uses memblock allocation. > > + * Later during boot, efi_enter_virtual_mode() calls kexec_enter_virtual_mode() > > + * in case of a kexec-ed kernel boot. This function kexec_enter_virtual_mode() > > + * installs the new EFI memory map by calling efi_memmap_init_late() which > > + * remaps the efi_memmap physically allocated here in efi_arch_mem_reserve(), > > + * but this remapping is still using memblock allocation. > > + * Subsequently, when memblock is freed later in boot flow, this remapped > > + * efi_memmap will have random corruption (similar to a use-after-free scenario). > > + * The corrupted EFI memory map is then passed to the next kexec-ed kernel > > + * which causes a panic when trying to use the corrupted EFI memory map. > > + * Additionally, skipping this function for kexec altogther makes sense > > + * as for kexec use case need to use the the EFI memmap passed from first > > + * kernel via setup_data and avoid any additional EFI memory map > > + * additions/updates. > > + */ > > + if (efi_setup) > > + return; > > + > > efi_mem_reserve is used to reserve boot service memory eg. bgrt, but > it is not necessary for kexec boot, as there are no boot services in > kexec reboot at all after the 1st kernel ExitBootServices(). > > The UEFI memmap passed to kexec kernel includes not only the runtime > service memory map but also the boot service memory ranges which were > reserved by the 1st kernel with efi_mem_reserve, and those boot service > memory ranges have already been marked "EFI_MEMORY_RUNTIME" attribute. > > Take example of bgrt, the saved memory is there only for people to check > the bgrt image info via /sys/firmware/acpi/bgrt/*, and it is not used in > early boot phase by boot services. > > Above is the reason why the efi_mem_reserve can be skipped for kexec > booting. But as I suggested before I personally think that checking > EFI_MEMORY_RUNTIME attribute set or not looks better than checking > efi_setup. > I recently applied the linux.dev mail with my Chinese pinyin name for use when I do not have vpn access. So just to clarify a bit, I'm the same person here :)
From: Ashish Kalra <ashish.kalra@amd.com>
Accessing guest video memory/RAM during kernel decompressor
causes guest termination as boot stage2 #VC handler for
SEV-ES/SNP systems does not support MMIO handling.
This issue is observed with SEV-ES/SNP guest kexec as
kexec -c adds screen_info to the boot parameters
passed to the kexec kernel, which causes console output to
be dumped to both video and serial.
As the decompressor output gets cleared really fast, it is
preferable to get the console output only on serial, hence,
skip accessing video RAM during decompressor stage to
prevent guest termination.
Serial console output during decompressor stage works as
boot stage2 #VC handler already supports handling port I/O.
Suggested-by: Thomas Lendacy <thomas.lendacky@amd.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
arch/x86/boot/compressed/misc.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index b70e4a21c15f..3b9f96b3dbcc 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -427,8 +427,10 @@ asmlinkage __visible void *extract_kernel(void *rmode, unsigned char *output)
vidport = 0x3d4;
}
- lines = boot_params_ptr->screen_info.orig_video_lines;
- cols = boot_params_ptr->screen_info.orig_video_cols;
+ if (!(sev_status & MSR_AMD64_SEV_ES_ENABLED)) {
+ lines = boot_params_ptr->screen_info.orig_video_lines;
+ cols = boot_params_ptr->screen_info.orig_video_cols;
+ }
init_default_io_ops();
--
2.34.1From: Ashish Kalra <ashish.kalra@amd.com>
SNP guests allocate shared buffers to perform I/O. It is done by
allocating pages normally from the buddy allocator and converting them
to shared with set_memory_decrypted().
The second kernel has no idea what memory is converted this way. It only
sees E820_TYPE_RAM.
Accessing shared memory via private mapping will cause unrecoverable RMP
page-faults.
On kexec walk direct mapping and convert all shared memory back to
private. It makes all RAM private again and second kernel may use it
normally. Additionally for SNP guests convert all bss decrypted section
pages back to private and switch back ROM regions to shared so that
their revalidation does not fail during kexec kernel boot.
The conversion occurs in two steps: stopping new conversions and
unsharing all memory. In the case of normal kexec, the stopping of
conversions takes place while scheduling is still functioning. This
allows for waiting until any ongoing conversions are finished. The
second step is carried out when all CPUs except one are inactive and
interrupts are disabled. This prevents any conflicts with code that may
access shared memory.
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
arch/x86/include/asm/sev.h | 4 +
arch/x86/kernel/sev.c | 161 ++++++++++++++++++++++++++++++++++
arch/x86/mm/mem_encrypt_amd.c | 3 +
3 files changed, 168 insertions(+)
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 7f57382afee4..78d40d08d201 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -229,6 +229,8 @@ void snp_accept_memory(phys_addr_t start, phys_addr_t end);
u64 snp_get_unsupported_features(u64 status);
u64 sev_get_status(void);
void sev_show_status(void);
+void snp_kexec_unshare_mem(void);
+void snp_kexec_stop_conversion(bool crash);
#else
static inline void sev_es_ist_enter(struct pt_regs *regs) { }
static inline void sev_es_ist_exit(void) { }
@@ -258,6 +260,8 @@ static 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 sev_show_status(void) { }
+static inline void snp_kexec_unshare_mem(void) { }
+static inline void snp_kexec_stop_conversion(bool crash) { }
#endif
#ifdef CONFIG_KVM_AMD_SEV
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index 995f94467101..891257fde810 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -42,6 +42,8 @@
#include <asm/apic.h>
#include <asm/cpuid.h>
#include <asm/cmdline.h>
+#include <asm/pgtable.h>
+#include <asm/set_memory.h>
#define DR7_RESET_VALUE 0x400
@@ -92,6 +94,9 @@ static struct ghcb *boot_ghcb __section(".data");
/* Bitmap of SEV features supported by the hypervisor */
static u64 sev_hv_features __ro_after_init;
+/* Last address to be switched to private during kexec */
+static unsigned long kexec_last_addr_to_make_private;
+
/* #VC handler runtime per-CPU data */
struct sev_es_runtime_data {
struct ghcb ghcb_page;
@@ -913,6 +918,162 @@ void snp_accept_memory(phys_addr_t start, phys_addr_t end)
set_pages_state(vaddr, npages, SNP_PAGE_STATE_PRIVATE);
}
+static bool set_pte_enc(pte_t *kpte, int level, void *va)
+{
+ pte_t new_pte;
+
+ if (pte_none(*kpte))
+ return false;
+
+ /*
+ * Change the physical page attribute from C=0 to C=1. Flush the
+ * caches to ensure that data gets accessed with the correct C-bit.
+ */
+ if (pte_present(*kpte))
+ clflush_cache_range(va, page_level_size(level));
+
+ new_pte = __pte(cc_mkenc(pte_val(*kpte)));
+ set_pte_atomic(kpte, new_pte);
+
+ return true;
+}
+
+static bool make_pte_private(pte_t *pte, unsigned long addr, int pages, int level)
+{
+ struct sev_es_runtime_data *data;
+ struct ghcb *ghcb;
+
+ data = this_cpu_read(runtime_data);
+ ghcb = &data->ghcb_page;
+
+ /* Check for GHCB for being part of a PMD range. */
+ if ((unsigned long)ghcb >= addr &&
+ (unsigned long)ghcb <= (addr + (pages * PAGE_SIZE))) {
+ /*
+ * Ensure that the current cpu's GHCB is made private
+ * at the end of unshared loop so that we continue to use the
+ * optimized GHCB protocol and not force the switch to
+ * MSR protocol till the very end.
+ */
+ pr_debug("setting boot_ghcb to NULL for this cpu ghcb\n");
+ kexec_last_addr_to_make_private = addr;
+ return true;
+ }
+
+ if (!set_pte_enc(pte, level, (void *)addr))
+ return false;
+
+ snp_set_memory_private(addr, pages);
+
+ return true;
+}
+
+static void unshare_all_memory(void)
+{
+ unsigned long addr, end;
+
+ /*
+ * Walk direct mapping and convert all shared memory back to private,
+ */
+
+ addr = PAGE_OFFSET;
+ end = PAGE_OFFSET + get_max_mapped();
+
+ while (addr < end) {
+ unsigned long size;
+ unsigned int level;
+ pte_t *pte;
+
+ pte = lookup_address(addr, &level);
+ size = page_level_size(level);
+
+ /*
+ * pte_none() check is required to skip physical memory holes in direct mapped.
+ */
+ if (pte && pte_decrypted(*pte) && !pte_none(*pte)) {
+ int pages = size / PAGE_SIZE;
+
+ if (!make_pte_private(pte, addr, pages, level)) {
+ pr_err("Failed to unshare range %#lx-%#lx\n",
+ addr, addr + size);
+ }
+
+ }
+
+ addr += size;
+ }
+ __flush_tlb_all();
+
+}
+
+static void unshare_all_bss_decrypted_memory(void)
+{
+ unsigned long vaddr, vaddr_end;
+ unsigned int level;
+ unsigned int npages;
+ pte_t *pte;
+
+ vaddr = (unsigned long)__start_bss_decrypted;
+ vaddr_end = (unsigned long)__start_bss_decrypted_unused;
+ npages = (vaddr_end - vaddr) >> PAGE_SHIFT;
+ for (; vaddr < vaddr_end; vaddr += PAGE_SIZE) {
+ pte = lookup_address(vaddr, &level);
+ if (!pte || !pte_decrypted(*pte) || pte_none(*pte))
+ continue;
+
+ set_pte_enc(pte, level, (void *)vaddr);
+ }
+ vaddr = (unsigned long)__start_bss_decrypted;
+ snp_set_memory_private(vaddr, npages);
+}
+
+/* Stop new private<->shared conversions */
+void snp_kexec_stop_conversion(bool crash)
+{
+ /*
+ * Crash kernel reaches here with interrupts disabled: can't wait for
+ * conversions to finish.
+ *
+ * If race happened, just report and proceed.
+ */
+ bool wait_for_lock = !crash;
+
+ if (!stop_memory_enc_conversion(wait_for_lock))
+ pr_warn("Failed to finish shared<->private conversions\n");
+}
+
+void snp_kexec_unshare_mem(void)
+{
+ if (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
+ return;
+
+ unshare_all_memory();
+
+ unshare_all_bss_decrypted_memory();
+
+ if (kexec_last_addr_to_make_private) {
+ unsigned long size;
+ unsigned int level;
+ pte_t *pte;
+
+ /*
+ * Switch to using the MSR protocol to change this cpu's
+ * GHCB to private.
+ * All the per-cpu GHCBs have been switched back to private,
+ * so can't do any more GHCB calls to the hypervisor beyond
+ * this point till the kexec kernel starts running.
+ */
+ boot_ghcb = NULL;
+ sev_cfg.ghcbs_initialized = false;
+
+ pr_debug("boot ghcb 0x%lx\n", kexec_last_addr_to_make_private);
+ pte = lookup_address(kexec_last_addr_to_make_private, &level);
+ size = page_level_size(level);
+ set_pte_enc(pte, level, (void *)kexec_last_addr_to_make_private);
+ snp_set_memory_private(kexec_last_addr_to_make_private, (size / PAGE_SIZE));
+ }
+}
+
static int snp_set_vmsa(void *va, bool vmsa)
{
u64 attrs;
diff --git a/arch/x86/mm/mem_encrypt_amd.c b/arch/x86/mm/mem_encrypt_amd.c
index e7b67519ddb5..49c40c2ed809 100644
--- a/arch/x86/mm/mem_encrypt_amd.c
+++ b/arch/x86/mm/mem_encrypt_amd.c
@@ -468,6 +468,9 @@ void __init sme_early_init(void)
x86_platform.guest.enc_tlb_flush_required = amd_enc_tlb_flush_required;
x86_platform.guest.enc_cache_flush_required = amd_enc_cache_flush_required;
+ x86_platform.guest.enc_kexec_stop_conversion = snp_kexec_stop_conversion;
+ x86_platform.guest.enc_kexec_unshare_mem = snp_kexec_unshare_mem;
+
/*
* AMD-SEV-ES intercepts the RDMSR to read the X2APIC ID in the
* parallel bringup low level code. That raises #VC which cannot be
--
2.34.1From: Ashish Kalra <ashish.kalra@amd.com>
For kexec use case, need to use and stick to the EFI memmap passed
from the first kernel via boot-params/setup data, hence,
skip efi_arch_mem_reserve() during kexec.
Additionally during SNP guest kexec testing discovered that EFI memmap
is corrupted during chained kexec. kexec_enter_virtual_mode() during
late init will remap the efi_memmap physical pages allocated in
efi_arch_mem_reserve() via memblock & then subsequently cause random
EFI memmap corruption once memblock is freed/teared-down.
Suggested-by: Dave Young <dyoung@redhat.com>
[Dave Young: checking the md attribute instead of checking the efi_setup]
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
arch/x86/platform/efi/quirks.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index f0cc00032751..982f5e50a4b3 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -258,12 +258,28 @@ void __init efi_arch_mem_reserve(phys_addr_t addr, u64 size)
int num_entries;
void *new;
- if (efi_mem_desc_lookup(addr, &md) ||
- md.type != EFI_BOOT_SERVICES_DATA) {
+ /*
+ * For kexec use case, we need to use the EFI memmap passed from the first
+ * kernel via setup data, so we need to skip this.
+ * Additionally kexec_enter_virtual_mode() during late init will remap
+ * the efi_memmap physical pages allocated here via memboot & then
+ * subsequently cause random EFI memmap corruption once memblock is freed.
+ */
+
+ if (efi_mem_desc_lookup(addr, &md)) {
pr_err("Failed to lookup EFI memory descriptor for %pa\n", &addr);
return;
}
+ if (md.type != EFI_BOOT_SERVICES_DATA) {
+ pr_err("Skip reserving non EFI Boot Service Data memory for %pa\n", &addr);
+ return;
+ }
+
+ /* Kexec copied the efi memmap from the first kernel, thus skip the case */
+ if (md.attribute & EFI_MEMORY_RUNTIME)
+ return;
+
if (addr + size > md.phys_addr + (md.num_pages << EFI_PAGE_SHIFT)) {
pr_err("Region spans EFI memory descriptors, %pa\n", &addr);
return;
--
2.34.1From: Ashish Kalra <ashish.kalra@amd.com>
Add sev_es_enabled() function to detect if SEV-ES
support is enabled.
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
arch/x86/boot/compressed/sev.c | 5 +++++
arch/x86/boot/compressed/sev.h | 2 ++
2 files changed, 7 insertions(+)
diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
index ec71846d28c9..4ae4cc51e6b8 100644
--- a/arch/x86/boot/compressed/sev.c
+++ b/arch/x86/boot/compressed/sev.c
@@ -134,6 +134,11 @@ bool sev_snp_enabled(void)
return sev_status & MSR_AMD64_SEV_SNP_ENABLED;
}
+bool sev_es_enabled(void)
+{
+ return sev_status & MSR_AMD64_SEV_ES_ENABLED;
+}
+
static void __page_state_change(unsigned long paddr, enum psc_op op)
{
u64 val;
diff --git a/arch/x86/boot/compressed/sev.h b/arch/x86/boot/compressed/sev.h
index fc725a981b09..5008c80e66e6 100644
--- a/arch/x86/boot/compressed/sev.h
+++ b/arch/x86/boot/compressed/sev.h
@@ -11,11 +11,13 @@
#ifdef CONFIG_AMD_MEM_ENCRYPT
bool sev_snp_enabled(void);
+bool sev_es_enabled(void);
void snp_accept_memory(phys_addr_t start, phys_addr_t end);
#else
static inline bool sev_snp_enabled(void) { return false; }
+static inline bool sev_es_enabled(void) { return false; }
static inline void snp_accept_memory(phys_addr_t start, phys_addr_t end) { }
#endif
--
2.34.1On Tue, Apr 09, 2024 at 08:42:38PM +0000, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
>
> Add sev_es_enabled() function to detect if SEV-ES
> support is enabled.
And use it exactly once?
Nah, use sev_status directly.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
From: Ashish Kalra <ashish.kalra@amd.com>
Accessing guest video memory/RAM during kernel decompressor
causes guest termination as boot stage2 #VC handler for
SEV-ES/SNP systems does not support MMIO handling.
This issue is observed with SEV-ES/SNP guest kexec as
kexec -c adds screen_info to the boot parameters
passed to the kexec kernel, which causes console output to
be dumped to both video and serial.
As the decompressor output gets cleared really fast, it is
preferable to get the console output only on serial, hence,
skip accessing video RAM during decompressor stage to
prevent guest termination.
Serial console output during decompressor stage works as
boot stage2 #VC handler already supports handling port I/O.
Suggested-by: Thomas Lendacy <thomas.lendacky@amd.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
arch/x86/boot/compressed/misc.c | 6 ++++--
arch/x86/boot/compressed/misc.h | 1 +
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index b70e4a21c15f..47b4db200e1f 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -427,8 +427,10 @@ asmlinkage __visible void *extract_kernel(void *rmode, unsigned char *output)
vidport = 0x3d4;
}
- lines = boot_params_ptr->screen_info.orig_video_lines;
- cols = boot_params_ptr->screen_info.orig_video_cols;
+ if (!sev_es_enabled()) {
+ lines = boot_params_ptr->screen_info.orig_video_lines;
+ cols = boot_params_ptr->screen_info.orig_video_cols;
+ }
init_default_io_ops();
diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index b353a7be380c..3c12ca987554 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -37,6 +37,7 @@
#include <asm/desc_defs.h>
#include "tdx.h"
+#include "sev.h"
#define BOOT_CTYPE_H
#include <linux/acpi.h>
--
2.34.1From: Ashish Kalra <ashish.kalra@amd.com>
SNP guests allocate shared buffers to perform I/O. It is done by
allocating pages normally from the buddy allocator and converting them
to shared with set_memory_decrypted().
The second kernel has no idea what memory is converted this way. It only
sees E820_TYPE_RAM.
Accessing shared memory via private mapping will cause unrecoverable RMP
page-faults.
On kexec walk direct mapping and convert all shared memory back to
private. It makes all RAM private again and second kernel may use it
normally. Additionally for SNP guests convert all bss decrypted section
pages back to private and switch back ROM regions to shared so that
their revalidation does not fail during kexec kernel boot.
The conversion occurs in two steps: stopping new conversions and
unsharing all memory. In the case of normal kexec, the stopping of
conversions takes place while scheduling is still functioning. This
allows for waiting until any ongoing conversions are finished. The
second step is carried out when all CPUs except one are inactive and
interrupts are disabled. This prevents any conflicts with code that may
access shared memory.
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
arch/x86/include/asm/sev.h | 4 +
arch/x86/kernel/sev.c | 161 ++++++++++++++++++++++++++++++++++
arch/x86/mm/mem_encrypt_amd.c | 3 +
3 files changed, 168 insertions(+)
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 7f57382afee4..78d40d08d201 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -229,6 +229,8 @@ void snp_accept_memory(phys_addr_t start, phys_addr_t end);
u64 snp_get_unsupported_features(u64 status);
u64 sev_get_status(void);
void sev_show_status(void);
+void snp_kexec_unshare_mem(void);
+void snp_kexec_stop_conversion(bool crash);
#else
static inline void sev_es_ist_enter(struct pt_regs *regs) { }
static inline void sev_es_ist_exit(void) { }
@@ -258,6 +260,8 @@ static 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 sev_show_status(void) { }
+static inline void snp_kexec_unshare_mem(void) { }
+static inline void snp_kexec_stop_conversion(bool crash) { }
#endif
#ifdef CONFIG_KVM_AMD_SEV
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index 38ad066179d8..17f616963beb 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -42,6 +42,8 @@
#include <asm/apic.h>
#include <asm/cpuid.h>
#include <asm/cmdline.h>
+#include <asm/pgtable.h>
+#include <asm/set_memory.h>
#define DR7_RESET_VALUE 0x400
@@ -92,6 +94,9 @@ static struct ghcb *boot_ghcb __section(".data");
/* Bitmap of SEV features supported by the hypervisor */
static u64 sev_hv_features __ro_after_init;
+/* Last address to be switched to private during kexec */
+static unsigned long kexec_last_addr_to_make_private;
+
/* #VC handler runtime per-CPU data */
struct sev_es_runtime_data {
struct ghcb ghcb_page;
@@ -913,6 +918,162 @@ void snp_accept_memory(phys_addr_t start, phys_addr_t end)
set_pages_state(vaddr, npages, SNP_PAGE_STATE_PRIVATE);
}
+static bool set_pte_enc(pte_t *kpte, int level, void *va)
+{
+ pte_t new_pte;
+
+ if (pte_none(*kpte))
+ return false;
+
+ /*
+ * Change the physical page attribute from C=0 to C=1. Flush the
+ * caches to ensure that data gets accessed with the correct C-bit.
+ */
+ if (pte_present(*kpte))
+ clflush_cache_range(va, page_level_size(level));
+
+ new_pte = __pte(cc_mkenc(pte_val(*kpte)));
+ set_pte_atomic(kpte, new_pte);
+
+ return true;
+}
+
+static bool make_pte_private(pte_t *pte, unsigned long addr, int pages, int level)
+{
+ struct sev_es_runtime_data *data;
+ struct ghcb *ghcb;
+
+ data = this_cpu_read(runtime_data);
+ ghcb = &data->ghcb_page;
+
+ /* Check for GHCB for being part of a PMD range. */
+ if ((unsigned long)ghcb >= addr &&
+ (unsigned long)ghcb <= (addr + (pages * PAGE_SIZE))) {
+ /*
+ * Ensure that the current cpu's GHCB is made private
+ * at the end of unshared loop so that we continue to use the
+ * optimized GHCB protocol and not force the switch to
+ * MSR protocol till the very end.
+ */
+ pr_debug("setting boot_ghcb to NULL for this cpu ghcb\n");
+ kexec_last_addr_to_make_private = addr;
+ return true;
+ }
+
+ if (!set_pte_enc(pte, level, (void *)addr))
+ return false;
+
+ snp_set_memory_private(addr, pages);
+
+ return true;
+}
+
+static void unshare_all_memory(void)
+{
+ unsigned long addr, end;
+
+ /*
+ * Walk direct mapping and convert all shared memory back to private,
+ */
+
+ addr = PAGE_OFFSET;
+ end = PAGE_OFFSET + get_max_mapped();
+
+ while (addr < end) {
+ unsigned long size;
+ unsigned int level;
+ pte_t *pte;
+
+ pte = lookup_address(addr, &level);
+ size = page_level_size(level);
+
+ /*
+ * pte_none() check is required to skip physical memory holes in direct mapped.
+ */
+ if (pte && pte_decrypted(*pte) && !pte_none(*pte)) {
+ int pages = size / PAGE_SIZE;
+
+ if (!make_pte_private(pte, addr, pages, level)) {
+ pr_err("Failed to unshare range %#lx-%#lx\n",
+ addr, addr + size);
+ }
+
+ }
+
+ addr += size;
+ }
+ __flush_tlb_all();
+
+}
+
+static void unshare_all_bss_decrypted_memory(void)
+{
+ unsigned long vaddr, vaddr_end;
+ unsigned int level;
+ unsigned int npages;
+ pte_t *pte;
+
+ vaddr = (unsigned long)__start_bss_decrypted;
+ vaddr_end = (unsigned long)__start_bss_decrypted_unused;
+ npages = (vaddr_end - vaddr) >> PAGE_SHIFT;
+ for (; vaddr < vaddr_end; vaddr += PAGE_SIZE) {
+ pte = lookup_address(vaddr, &level);
+ if (!pte || !pte_decrypted(*pte) || pte_none(*pte))
+ continue;
+
+ set_pte_enc(pte, level, (void *)vaddr);
+ }
+ vaddr = (unsigned long)__start_bss_decrypted;
+ snp_set_memory_private(vaddr, npages);
+}
+
+/* Stop new private<->shared conversions */
+void snp_kexec_stop_conversion(bool crash)
+{
+ /*
+ * Crash kernel reaches here with interrupts disabled: can't wait for
+ * conversions to finish.
+ *
+ * If race happened, just report and proceed.
+ */
+ bool wait_for_lock = !crash;
+
+ if (!stop_memory_enc_conversion(wait_for_lock))
+ pr_warn("Failed to finish shared<->private conversions\n");
+}
+
+void snp_kexec_unshare_mem(void)
+{
+ if (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
+ return;
+
+ unshare_all_memory();
+
+ unshare_all_bss_decrypted_memory();
+
+ if (kexec_last_addr_to_make_private) {
+ unsigned long size;
+ unsigned int level;
+ pte_t *pte;
+
+ /*
+ * Switch to using the MSR protocol to change this cpu's
+ * GHCB to private.
+ * All the per-cpu GHCBs have been switched back to private,
+ * so can't do any more GHCB calls to the hypervisor beyond
+ * this point till the kexec kernel starts running.
+ */
+ boot_ghcb = NULL;
+ sev_cfg.ghcbs_initialized = false;
+
+ pr_debug("boot ghcb 0x%lx\n", kexec_last_addr_to_make_private);
+ pte = lookup_address(kexec_last_addr_to_make_private, &level);
+ size = page_level_size(level);
+ set_pte_enc(pte, level, (void *)kexec_last_addr_to_make_private);
+ snp_set_memory_private(kexec_last_addr_to_make_private, (size / PAGE_SIZE));
+ }
+}
+
static int snp_set_vmsa(void *va, bool vmsa)
{
u64 attrs;
diff --git a/arch/x86/mm/mem_encrypt_amd.c b/arch/x86/mm/mem_encrypt_amd.c
index e7b67519ddb5..49c40c2ed809 100644
--- a/arch/x86/mm/mem_encrypt_amd.c
+++ b/arch/x86/mm/mem_encrypt_amd.c
@@ -468,6 +468,9 @@ void __init sme_early_init(void)
x86_platform.guest.enc_tlb_flush_required = amd_enc_tlb_flush_required;
x86_platform.guest.enc_cache_flush_required = amd_enc_cache_flush_required;
+ x86_platform.guest.enc_kexec_stop_conversion = snp_kexec_stop_conversion;
+ x86_platform.guest.enc_kexec_unshare_mem = snp_kexec_unshare_mem;
+
/*
* AMD-SEV-ES intercepts the RDMSR to read the X2APIC ID in the
* parallel bringup low level code. That raises #VC which cannot be
--
2.34.1Hi Ashish,
kernel test robot noticed the following build errors:
[auto build test ERROR on tip/master]
[also build test ERROR on linus/master next-20240410]
[cannot apply to tip/x86/core tip/x86/mm tip/auto-latest]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Ashish-Kalra/efi-x86-skip-efi_arch_mem_reserve-in-case-of-kexec/20240410-044512
base: tip/master
patch link: https://lore.kernel.org/r/b24885f5495f6b8ba2f9e825fda9188fcbf28231.1712694667.git.ashish.kalra%40amd.com
patch subject: [PATCH v4 4/4] x86/snp: Convert shared memory back to private on kexec
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20240410/202404102232.UKwWHSTE-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240410/202404102232.UKwWHSTE-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202404102232.UKwWHSTE-lkp@intel.com/
All errors (new ones prefixed by >>):
>> arch/x86/kernel/sev.c:993:14: error: call to undeclared function 'pte_decrypted'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
993 | if (pte && pte_decrypted(*pte) && !pte_none(*pte)) {
| ^
arch/x86/kernel/sev.c:1021:16: error: call to undeclared function 'pte_decrypted'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1021 | if (!pte || !pte_decrypted(*pte) || pte_none(*pte))
| ^
>> arch/x86/kernel/sev.c:1041:7: error: call to undeclared function 'stop_memory_enc_conversion'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1041 | if (!stop_memory_enc_conversion(wait_for_lock))
| ^
3 errors generated.
--
>> arch/x86/mm/mem_encrypt_amd.c:471:21: error: no member named 'enc_kexec_stop_conversion' in 'struct x86_guest'
471 | x86_platform.guest.enc_kexec_stop_conversion = snp_kexec_stop_conversion;
| ~~~~~~~~~~~~~~~~~~ ^
>> arch/x86/mm/mem_encrypt_amd.c:472:21: error: no member named 'enc_kexec_unshare_mem' in 'struct x86_guest'
472 | x86_platform.guest.enc_kexec_unshare_mem = snp_kexec_unshare_mem;
| ~~~~~~~~~~~~~~~~~~ ^
2 errors generated.
vim +/pte_decrypted +993 arch/x86/kernel/sev.c
970
971 static void unshare_all_memory(void)
972 {
973 unsigned long addr, end;
974
975 /*
976 * Walk direct mapping and convert all shared memory back to private,
977 */
978
979 addr = PAGE_OFFSET;
980 end = PAGE_OFFSET + get_max_mapped();
981
982 while (addr < end) {
983 unsigned long size;
984 unsigned int level;
985 pte_t *pte;
986
987 pte = lookup_address(addr, &level);
988 size = page_level_size(level);
989
990 /*
991 * pte_none() check is required to skip physical memory holes in direct mapped.
992 */
> 993 if (pte && pte_decrypted(*pte) && !pte_none(*pte)) {
994 int pages = size / PAGE_SIZE;
995
996 if (!make_pte_private(pte, addr, pages, level)) {
997 pr_err("Failed to unshare range %#lx-%#lx\n",
998 addr, addr + size);
999 }
1000
1001 }
1002
1003 addr += size;
1004 }
1005 __flush_tlb_all();
1006
1007 }
1008
1009 static void unshare_all_bss_decrypted_memory(void)
1010 {
1011 unsigned long vaddr, vaddr_end;
1012 unsigned int level;
1013 unsigned int npages;
1014 pte_t *pte;
1015
1016 vaddr = (unsigned long)__start_bss_decrypted;
1017 vaddr_end = (unsigned long)__start_bss_decrypted_unused;
1018 npages = (vaddr_end - vaddr) >> PAGE_SHIFT;
1019 for (; vaddr < vaddr_end; vaddr += PAGE_SIZE) {
1020 pte = lookup_address(vaddr, &level);
1021 if (!pte || !pte_decrypted(*pte) || pte_none(*pte))
1022 continue;
1023
1024 set_pte_enc(pte, level, (void *)vaddr);
1025 }
1026 vaddr = (unsigned long)__start_bss_decrypted;
1027 snp_set_memory_private(vaddr, npages);
1028 }
1029
1030 /* Stop new private<->shared conversions */
1031 void snp_kexec_stop_conversion(bool crash)
1032 {
1033 /*
1034 * Crash kernel reaches here with interrupts disabled: can't wait for
1035 * conversions to finish.
1036 *
1037 * If race happened, just report and proceed.
1038 */
1039 bool wait_for_lock = !crash;
1040
> 1041 if (!stop_memory_enc_conversion(wait_for_lock))
1042 pr_warn("Failed to finish shared<->private conversions\n");
1043 }
1044
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
© 2016 - 2026 Red Hat, Inc.