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

[Chao Fan <fanc.fnst@cn.fujitsu.com>]

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
>
>
>