On Tue, Oct 08, 2019 at 03:08:29PM +0200, Jan Beulich wrote: > On 08.10.2019 13:50, Marek Marczykowski-Górecki wrote: > > On Thu, Aug 08, 2019 at 08:03:49AM +0200, Jan Beulich wrote: > >> On 08.08.2019 04:53, Marek Marczykowski-Górecki wrote: > >>> On Wed, Aug 07, 2019 at 09:26:00PM +0200, Marek Marczykowski-Górecki wrote: > >>>> Ok, regardless of adding proper option for that, I've hardcoded map_bs=1 > >>>> and it still crashes, just slightly differently: > >>>> > >>>> Xen call trace: > >>>> [<0000000000000080>] 0000000000000080 > >>>> [<8c2b0398e0000daa>] 8c2b0398e0000daa > >>>> > >>>> Pagetable walk from ffffffff858483a1: > >>>> L4[0x1ff] = 0000000000000000 ffffffffffffffff > >>>> > >>>> **************************************** > >>>> Panic on CPU 0: > >>>> FATAL PAGE FAULT > >>>> [error_code=0002] > >>>> Faulting linear address: ffffffff858483a1 > >>>> **************************************** > >>>> > >>>> Full message attached. > >>> > >>> After playing more with it and also know workarounds for various EFI > >>> issues, I've found a way to boot it: avoid calling Exit BootServices. > >>> There was a patch from Konrad adding /noexit option, that never get > >>> committed. Similar to efi=mapbs option, I'd add efi=no-exitboot too > >>> (once efi=mapbs patch is accepted). > >>> > >>> Anyway, I'm curious what exactly is wrong here. Is it that the firmware > >>> is not happy about lack of SetVirtualAddressMap call? FWIW, the crash is > >>> during GetVariable RS call. I've verified that the function itself is > >>> within EfiRuntimeServicesCode, but I don't feel like tracing Lenovo > >>> UEFI... > >> > >> This suggests that the firmware zaps a few too many pointers > >> during ExitBootServices(). Perhaps internally they check > >> whether pointers point into BootServices* memory, and hence the > >> wrong marking in the memory map has consequences beyond the OS > >> re-using such memory? > >> > >> A proper answer to your question can of course only be given > >> by someone knowing this specific firmware version. > > > > I explored it a bit more and talked with a few people doing firmware > > development and few conclusions: > > 1. Not calling SetVirtualAddressMap(), while technically legal, is > > pretty uncommon and not recommended if you want to avoid less tested > > (aka buggy) UEFI code paths. > > 2. Every UEFI call before SetVirtualAddressMap() call should be done > > with flat physical memory. This include SetVirtualAddressMap() call > > itself. Implicitly this means such calls can legally access memory areas > > not marked with EFI_MEMORY_RUNTIME. > > I don't think this is quite right - whether non-runtime memory may > be touched depends exclusively on ExitBootServices() (not) having > got called (yet). That would be logical. In practice however we have evidences firmware vendors have different opinion... A comment from Linux (already quoted here 2 months ago): /* * The UEFI specification makes it clear that the operating system is * free to do whatever it wants with boot services code after * ExitBootServices() has been called. Ignoring this recommendation a * significant bunch of EFI implementations continue calling into boot * services code (SetVirtualAddressMap). In order to work around such * buggy implementations we reserve boot services region during EFI * init and make sure it stays executable. Then, after * SetVirtualAddressMap(), it is discarded. * * However, some boot services regions contain data that is required * by drivers, so we need to track which memory ranges can never be * freed. This is done by tagging those regions with the * EFI_MEMORY_RUNTIME attribute. * * Any driver that wants to mark a region as reserved must use * efi_mem_reserve() which will insert a new EFI memory descriptor * into efi.memmap (splitting existing regions if necessary) and tag * it with EFI_MEMORY_RUNTIME. */ 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). > > 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 if Xen was kexec'ed from Linux? 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. What about something in between: make this SetVirtualAddressMap() call compile-time option (kconfig), depending on !CONFIG_KEXEC ? And when enabled, properly handle SetVirtualAddressMap() failure. I my case, where I do care about supporting various UEFI implementations, I don't need kexec support. And apparently people carrying about kexec don't have problems with lack of SetVirtualAddressMap(), so that would be win-win, no? -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing?