[RFC PATCH] arm64: use non-global mappings for UEFI runtime regions

Ard Biesheuvel
2015-11-18T06:42:27Z

On 17 November 2015 at 18:17, Mark Rutland <mark.rutland@arm.com> wrote:
>> >> > For backporting, I'm not sure that this is necessarily safe prior to
>> >> > Will's rework of the ASID allocator. I think we can IPI in this context,
>> >> > and it looks like the cpu_set_reserved_ttbr0() in flush_context() would
>> >> > save us from the problem described above, but I may have missed
>> >> > something.
>> >> >
>> >> > Will, are you aware of anything that could bite us here?
>> >>
>> >> Can we guarantee that efi_virtmap_{load,unload} are called with interrupts
>> >> enabled?
>> >
>> > Unfortuantely, it looks like we can guarantee interrupts are _disabled_.
>> >
>> > Every function in drivers/firmware/efi/runtime-wrappers.c which uses
>> > efi_call_virt (and hence efi_virtmap_{load,unload}) wraps the call in a
>> > spin_lock_irq{save,restore} pair. Those appear to be the only uses of
>> > efi_call_virt.
>> >
>>
>> There is actually no need from the UEFI pov to invoke the UEFI runtime
>> services with interrupts disabled, this is simply an implementation
>> detail of the kernel support, and I think it is primarily for x86 (but
>> I have to dig up the old thread for the details)
>>
>> And even if we stick with spin_lock_irqsave(), we could refactor the
>> runtime wrappers to perform the mm switch outside of them.
>
> Ok.
>
> I'm only thinking about stable here.
>
> In the context of a stable backport, I think the simplest thing to do is
> always go via the resesrved ttbr0 to perform the TLB flush, and
> hand-code the save/restore of the active mm's TTBR0_EL1 value rather
> than going through cpu_switch_mm (which I believe we can't call with
> interrupts disabled).
>

I think calling cpu_switch_mm() should be fine. It does set the ASID
to #0 (which shouldn't matter since we only create global mappings)
but does not invoke any of the logic regarding ASID assignment or
rollover.

> It doesn't look like it's easy to stash the value given
> efi_virtmap_{load,unload} are separate functions, and I don't think we
> can just restore from current->active_mm in case there was a concurrent
> rollover on another CPU.
>

OK, so the fact that we switch to the UEFI page tables and back with
interrupts disabled is keeping us safe here, and whether we use global
mappings or not is completely irrelevant.

In any case, I am going to check with Matt if we can leave interrupts
enabled during the UEFI Runtime Services (since the execution time is
not bounded by the spec), but I will keep this in mind in case anyone
tries to backport that.

--
Ard.