Re: [PATCH v2] x86/boot: Use efi_setup_data for searching RSDP on kexec-ed kernel

Chao Fan
2019-04-04T07:20:18Z

On Thu, Apr 04, 2019 at 02:41:30PM +0800, Dave Young wrote:
>On 04/04/19 at 11:22am, Dave Young wrote:
>> On 04/04/19 at 11:10am, Baoquan He wrote:
>> > On 04/04/19 at 11:00am, Baoquan He wrote:
>> > > On 04/04/19 at 10:52am, Dave Young wrote:
>> > > > On 04/04/19 at 01:23am, Junichi Nomura wrote:
>> > + /* Save RSDP address for later use. */
>> > + boot_params->acpi_rsdp_addr = get_rsdp_addr();
>> > +
>> > + error("Hang kernel for kexec debugging");
>> >
>> > Sorry, here I means calling error() to hang kernel after calling
>> > get_rsdp_addr().
>>
>> Thanks, it did not hang, it always reset to firmware/grub boot menu.
>> I'm pretty sure now the bug exists in get_rsdp_addr().
>
>static acpi_physical_address kexec_get_rsdp_addr(void)
>{
>...
> /* Get systab from boot params. */
> systab = (efi_system_table_64_t *) (ei->efi_systab | ((__u64)ei->efi_systab_hi << 32));
> if (!systab)
> error("EFI system table not found in kexec boot_params.");
>
>...
> -> add error("hang me") here will have a hang
>...
> return __efi_get_rsdp_addr((unsigned long)esd->tables,
> systab->nr_tables, true);
>

I have an idea, but not sure whether is a problem.
In code of Nomura:

#if defined(CONFIG_EFI) && defined(CONFIG_X86_64)
[...]
if (strncmp(sig, EFI64_LOADER_SIGNATURE, 4)) {
debug_putstr("Wrong kexec EFI loader signature.\n");
return 0;
}

/* Get systab from boot params. */
systab = (efi_system_table_64_t *) (ei->efi_systab | ((__u64)ei->efi_systab_hi << 32));
[...]
#endif

After review agian, I wonder what will happen if 32bit-efi boot 64bit
OS.

Ever meet a problem:
https://lkml.org/lkml/2019/2/8/845

It's a efi32 bootloader to boot a 64bit OS, then a problem happened.

Thanks,
Chao Fan

>But add error("hang me") in __efi_get_rsdp_addr it did not hang.
>
>It seems reference the systab pointer cause a system reset.
>
>A question is does the identity mapping covered the memory address of
>systab?
>
>In my case it is 0xdad9ef18
>
>If the memory is mapped on demand, then there will be problems, it
>should cover setup_data and efi table space.
>
>Thanks
>Dave
>
>
>