Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it

[Jan Beulich <jbeulich@suse.com>]

On 08.10.2019 18:29, Marek Marczykowski-Górecki  wrote:
> On Tue, Oct 08, 2019 at 04:19:13PM +0200, Jan Beulich wrote:
>> On 08.10.2019 15:52, Marek Marczykowski-Górecki  wrote:
>>> Regardless of SetVirtualAddressMap() discussion, I propose to
>>> automatically map boot services code/data, to make Xen work on more
>>> machines (even if _we_ consider those buggy). 
>>
>> I remain on my prior position: Adding command line triggerable
>> workarounds for such cases is fine. Defaulting to assume buggy
>> firmware is acceptable only if this means no extra penalty to
>> systems with conforming firmware. Hence, for the case at hand,
>> I object to doing this automatically; we already have the
>> /mapbs workaround in place to deal with the case when xen.efi
>> is used. Judging from the title here there may need to be an
>> addition to also allow triggering this from the MB2 boot path.
> 
> What about mirroring Linux behavior? I.e. mapping those regions for the
> SetVirtualAddressMap() time (when enabled) and unmapping after - unless
> tagged with EFI_MEMORY_RUNTIME? 
> Similarly to Andrew, I'd really prefer for Xen to work out of the box,
> with as little as possible manual tweaks needed.

If there's going to be a config where SetVirtualAddressMap() is to
be called - why not? But the same logic doesn't make sense when
such a call won#t happen in the first place.

>>>>> Then I've tried a different approach: call SetVirtualAddressMap(), but
>>>>> with an address map that tries to pretend physical addressing (the code
>>>>> under #ifndef USE_SET_VIRTUAL_ADDRESS_MAP). This mostly worked, I needed
>>>>> only few changes:
>>>>>  - set VirtualStart back to PhysicalStart in that memory map (it was set
>>>>>    to directmap)
>>>>>  - map boot services (at least for the SetVirtualAddressMap() call time,
>>>>>    but haven't tried unmapping it later)
>>>>>  - call SetVirtualAddressMap() with that "1:1" map in place, using
>>>>>    efi_rs_enter/efi_rs_leave.
>>>>>
>>>>> This fixed the issue for me, now runtime services do work even without
>>>>> disabling ExitBootServices() call. And without any extra
>>>>> platform-specific command line arguments. And I think it also shouldn't break
>>>>> kexec, since it uses 1:1-like map, but I haven't tried. One should
>>>>> simply ignore EFI_UNSUPPORTED return code (I don't know how to avoid the
>>>>> call at all after kexec).
>>>>
>>>> That's the point - it can't be avoided, and hence it failing is not
>>>> an option. Or else there needs to be a protocol telling kexec what
>>>> it is to expect, and that it in particular can't change the virtual
>>>> address map to its liking. Back at the time when I put together the
>>>> EFI booting code, no such protocol existed, and hence calling
>>>> SetVirtualAddressMap() was not an option. I have no idea whether
>>>> things have changed in the meantime.
>>>
>>> Hmm, how is it different from the current situation? Not calling
>>> SetVirtualAddressMap() means UEFI will not be notified about new address
>>> map. It does _not_ mean it will use 1:1 map, it will use what was
>>> previously set.
>>
>> What do you mean by "previously set"? SetVirtualAddressMap() can be
>> invoked only once during a given session (i.e. without intervening
>> boot). How would other than a 1:1 map have got set?
> 
> Like, in the very next sentence of my answer:
> 
>>> What if Xen was kexec'ed from Linux?

Honestly - I'm getting tired. You said yourself ...

>>> In Linux case, it looks like it passes around the EFI memory map using
>>> some Linux-specific mechanism, but I don't find it particularly
>>> appealing option.

... that this would require Xen following a Linux protocol.
This is nothing that can work building on EFI interfaces alone.

>>> What about something in between: make this SetVirtualAddressMap() call
>>> compile-time option (kconfig), depending on !CONFIG_KEXEC ? And when
>>> enabled, properly handle SetVirtualAddressMap() failure.
>>
>> What is "proper handling" here?
> 
> Logging the error and either panic or disabling runtime services (I tend
> to the latter).

Hmm, yes, disabling runtime services in this case makes sense.
But are you sure a SetVirtualAddressMap() failure doesn't incur
other potential issues later on? (Calling panic() is what I'd
rather not call "proper handling", but rather "emergency
handling".)

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel