lkml.org 
[lkml]   [2023]   [Jun]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v2] x86/kexec: Add EFI config table identity mapping for kexec kernel
    Hello maintainers,

    Sorry to interrupt. Currently I'm holding a machine which can be used
    to reproduce the original issue and test the patch. However I may need
    to return the machine in a short time. So if any updating and testing
    needed for patch v3, please let me know. Thanks in advance!

    Thanks,
    Tao Liu

    On Thu, Jun 1, 2023 at 4:25 PM Tao Liu <ltao@redhat.com> wrote:
    >
    > Hi Baoquan,
    >
    > On Thu, Jun 1, 2023 at 4:13 PM Baoquan He <bhe@redhat.com> wrote:
    > >
    > > On 06/01/23 at 03:20pm, 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.
    > > >
    > > > This patch will make sure the EFI config table is always mapped.
    > > >
    > > > Signed-off-by: Tao Liu <ltao@redhat.com>
    > > > ---
    > > > Changes in v2:
    > > > - Rephrase the change log based on Baoquan's suggestion.
    > > > - Rename map_efi_sys_cfg_tab() to map_efi_tables().
    > > > - Link to v1: https://lore.kernel.org/kexec/20230525094914.23420-1-ltao@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..664aefa6e896 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_tables(struct x86_mapping_info *info, pgd_t *level4p)
    > > > {
    > > > #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
    > >
    > > The code comment need be updated too?
    > >
    > > * Prepare EFI tables and ACPI tables for kexec kernel since they are
    > > * not covered by pfn_mapped.
    > >
    > > Other than this nit, this patch looks good to me, thanks.
    > >
    >
    > Thanks for the patch review! I'm OK with the comment update, but I
    > prefer to leave it as it is. Since the comment provides more details:
    > there are systab and config tables mapped instead of all efi tables.
    >
    > Thanks,
    > Tao Liu
    >
    > > Acked-by: Baoquan He <bhe@redhat.com>
    > >
    > >
    > > > + * since they are not covered by pfn_mapped.
    > > > */
    > > > - result = map_efi_systab(&info, level4p);
    > > > + result = map_efi_tables(&info, level4p);
    > > > if (result)
    > > > return result;
    > > >
    > > > --
    > > > 2.33.1
    > > >
    > >

    \
     
     \ /
      Last update: 2023-06-08 09:16    [W:3.442 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site