Hi Baoquan, On Fri, May 26, 2023 at 12:08 PM Baoquan He <b...@redhat.com> wrote: > > Hi Tao, > > On 05/25/23 at 05:49pm, Tao Liu wrote: > > A kexec kernel bootup hang is observed on Intel Atom cpu due to unmapped > > EFI config table. > > > > Currently EFI system table is identity-mapped for the kexec kernel, but EFI > > config table is not mapped explicitly: > > > > commit 6bbeb276b71f ("x86/kexec: Add the EFI system tables and ACPI > > tables to the ident map") > > > > Later in the following 2 commits, EFI config table will be accessed when > > enabling sev at kernel startup. This may result in a page fault due to EFI > > config table's unmapped address. Since the page fault occurs at an early > > stage, it is unrecoverable and kernel hangs. > > > > commit ec1c66af3a30 ("x86/compressed/64: Detect/setup SEV/SME features > > earlier during boot") > > commit c01fce9cef84 ("x86/compressed: Add SEV-SNP feature > > detection/setup") > > > > In addition, the issue doesn't appear on all systems, because the kexec > > kernel uses Page Size Extension (PSE) for identity mapping. In most cases, > > EFI config table can end up to be mapped into due to 1 GB page size. > > However if nogbpages is set, or cpu doesn't support pdpe1gb feature > > (e.g Intel Atom x6425RE cpu), EFI config table may not be mapped into > > due to 2 MB page size, thus a page fault hang is more likely to happen. > > > > In this patch, we will make sure the EFI config table is always mapped. > > Nice work. While you may need to rephrase above sentence, x86 > maintainers don't like log with the 'this patch,' or 'we'. Please refer > to 'Changelog' part of Documentation/process/maintainer-tip.rst and > improve it.
OK, Thanks for the suggestion! I will get the sentence rephrased in v2. > > > > > Signed-off-by: Tao Liu <l...@redhat.com> > > --- > > arch/x86/kernel/machine_kexec_64.c | 35 ++++++++++++++++++++++++++---- > > 1 file changed, 31 insertions(+), 4 deletions(-) > > > > diff --git a/arch/x86/kernel/machine_kexec_64.c > > b/arch/x86/kernel/machine_kexec_64.c > > index 1a3e2c05a8a5..755aa12f583f 100644 > > --- a/arch/x86/kernel/machine_kexec_64.c > > +++ b/arch/x86/kernel/machine_kexec_64.c > > @@ -28,6 +28,7 @@ > > #include <asm/setup.h> > > #include <asm/set_memory.h> > > #include <asm/cpu.h> > > +#include <asm/efi.h> > > > > #ifdef CONFIG_ACPI > > /* > > @@ -86,10 +87,12 @@ const struct kexec_file_ops * const > > kexec_file_loaders[] = { > > #endif > > > > static int > > -map_efi_systab(struct x86_mapping_info *info, pgd_t *level4p) > > +map_efi_sys_cfg_tab(struct x86_mapping_info *info, pgd_t *level4p) > > Can we call the function map_efi_tables() since we will map efi system > table, system config table. If you need add another table mapping here, > what would you call it, map_efi_sys_cfg_xxx_tab()? > Yeah, map_efi_sys_cfg_xxx_tab() is surely a bad name. I agree with the map_efi_tables() name. Thanks, Tao Liu > Anyway, not very strong opinion as long as x86 maintainer likes it. > > > { > > #ifdef CONFIG_EFI > > unsigned long mstart, mend; > > + void *kaddr; > > + int ret; > > > > if (!efi_enabled(EFI_BOOT)) > > return 0; > > @@ -105,6 +108,30 @@ map_efi_systab(struct x86_mapping_info *info, pgd_t > > *level4p) > > if (!mstart) > > return 0; > > > > + ret = kernel_ident_mapping_init(info, level4p, mstart, mend); > > + if (ret) > > + return ret; > > + > > + kaddr = memremap(mstart, mend - mstart, MEMREMAP_WB); > > + if (!kaddr) { > > + pr_err("Could not map UEFI system table\n"); > > + return -ENOMEM; > > + } > > + > > + mstart = efi_config_table; > > + > > + if (efi_enabled(EFI_64BIT)) { > > + efi_system_table_64_t *stbl = (efi_system_table_64_t *)kaddr; > > + > > + mend = mstart + sizeof(efi_config_table_64_t) * > > stbl->nr_tables; > > + } else { > > + efi_system_table_32_t *stbl = (efi_system_table_32_t *)kaddr; > > + > > + mend = mstart + sizeof(efi_config_table_32_t) * > > stbl->nr_tables; > > + } > > + > > + memunmap(kaddr); > > + > > return kernel_ident_mapping_init(info, level4p, mstart, mend); > > #endif > > return 0; > > @@ -244,10 +271,10 @@ static int init_pgtable(struct kimage *image, > > unsigned long start_pgtable) > > } > > > > /* > > - * Prepare EFI systab and ACPI tables for kexec kernel since they are > > - * not covered by pfn_mapped. > > + * Prepare EFI systab, config table and ACPI tables for kexec kernel > > + * since they are not covered by pfn_mapped. > > */ > > - result = map_efi_systab(&info, level4p); > > + result = map_efi_sys_cfg_tab(&info, level4p); > > if (result) > > return result; > > > > -- > > 2.33.1 > > > _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec