Messages in this thread Patch in this message | ![/](/images/icornerl.gif) | | Date | Tue, 3 May 2016 11:48:20 +0200 | From | Borislav Petkov <> | Subject | Re: [BUG] x86/efi: MMRs no longer properly mapped after switch to isolated page table |
| |
On Mon, May 02, 2016 at 07:10:36PM -0500, Alex Thorlton wrote: > +#define uv_call_virt(f, args...) \ > +({ \ > + efi_status_t __s; \ > + kernel_fpu_begin(); \ > + __s = ((efi_##f##_t __attribute__((regparm(0)))*) \ > + f)(args); \ > + kernel_fpu_end(); \ > + __s; \ > +})
Right, can you use the EFI-ones in drivers/firmware/efi/runtime-wrappers.c directly?
(So they're going to land there, I'm staring at latest -tip and those calls have become all fancy now:
#define efi_call_virt(f, args...) \ ({ \ efi_status_t __s; \ unsigned long flags; \ arch_efi_call_virt_setup(); \ local_save_flags(flags); \ __s = arch_efi_call_virt(f, args); \ efi_call_virt_check_flags(flags, __stringify(f)); \ arch_efi_call_virt_teardown(); \ __s; \ })
with efi_call_virt_check_flags() checking for IRQ flags corruption... And so on, but that's beside the point... )
> + > /* Use this macro if your virtual call does not return any value */ > #define __efi_call_virt(f, args...) \ > ({ \ > @@ -104,6 +114,32 @@ struct efi_scratch { > __s; \ > }) > > +#define uv_call_virt(f, ...) \ > +({ \ > + efi_status_t __s; \ > + \ > + efi_sync_low_kernel_mappings(); \ > + preempt_disable(); \ > + __kernel_fpu_begin(); \ > + \ > + if (efi_scratch.use_pgd) { \ > + efi_scratch.prev_cr3 = read_cr3(); \ > + write_cr3((unsigned long)efi_scratch.efi_pgt); \ > + __flush_tlb_all(); \ > + } \ > + \ > + __s = efi_call((void *)f, __VA_ARGS__); \ > + \ > + if (efi_scratch.use_pgd) { \ > + write_cr3(efi_scratch.prev_cr3); \ > + __flush_tlb_all(); \ > + } \ > + \ > + __kernel_fpu_end(); \ > + preempt_enable(); \ > + __s; \ > +}) > + > /* > * All X86_64 virt calls return non-void values. Thus, use non-void call for > * virt calls that would be void on X86_32. > > diff --git a/arch/x86/platform/uv/bios_uv.c b/arch/x86/platform/uv/bios_uv.c > index 1584cbe..6e99f81 100644 > --- a/arch/x86/platform/uv/bios_uv.c > +++ b/arch/x86/platform/uv/bios_uv.c > @@ -39,8 +39,8 @@ s64 uv_bios_call(enum uv_bios_cmd which, u64 a1, u64 a2, u64 a3, u64 a4, u64 a5) > */ > return BIOS_STATUS_UNIMPLEMENTED; > > - ret = efi_call((void *)__va(tab->function), (u64)which, > - a1, a2, a3, a4, a5); > + ret = uv_call_virt(tab->function, (u64)which, a1, a2, a3, a4, a5); > +
That could be simply
efi_call_virt(tab->function, ...)
methinks.
> return ret; > } > EXPORT_SYMBOL_GPL(uv_bios_call); > --->8 > > Note that the only change I made to efi_call_virt was to change > efi.systab->runtime->f to simply f in the efi_call line. This works up > until we try to do callbacks from a loaded module. When we try that we > hit this: > > [ 56.232086] BUG: unable to handle kernel paging request at ffffffff8106148f > [ 56.239880] IP: [<fffffffedbb408ce>] 0xfffffffedbb408ce > [ 56.245721] PGD 8698e0067 PUD 1a08063 PMD 10001e1
PMD looks ok to me.
Have you tried CONFIG_EFI_PGT_DUMP ? It will dump to dmesg so you might be able to spot stuff.
Also, you could dump them from debugfs *right* before loading the module and then look at stuff.
Also 2, booting with efi=debug should give you how the EFI regions get mapped. ...
> The bad paging request here appears to be on the: > > if (efi_scratch.use_pgd) > > Line of uv_call_virt. It looks like it's having trouble accessing the > efi_scratch struct using the EFI page table. I'm not sure why this > is an issue with callbacks from modules and not with the ones in > uv_system_init and friends.
Just this one module or all modules doing EFI calls?
> I'll keep investigating the module issue. Looks like we're getting > closer to sorting this out! > > Let me know if you have thoughts about the way I'm getting stuff > working. I'm thinking there's probably a better way to do this than by > copying the whole efi_call_virt macro - this was a quick and dirty > solution.
Yeah, try using the generic facilities. We should be able to accomodate all users...
HTH.
-- Regards/Gruss, Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --
| ![\](/images/icornerr.gif) |