On Mon, Aug 15, 2016 at 05:07:09PM +0200, Borislav Petkov wrote: > On Mon, Aug 15, 2016 at 01:42:58PM +0100, Matt Fleming wrote: > > (Cc'ing Boris and Dave) > > > > On Fri, 05 Aug, at 06:59:35PM, Alex Thorlton wrote: > > > This is a simple change to add in the physical mappings as well as the > > > virtual mappings in efi_map_region_fixed. The motivation here is to > > > get access to EFI runtime code that is only available via the 1:1 > > > mappings on a kexec'd kernel. > > So I don't understand: the whole jumping through hoops so that we have > stable virtual mappings just so that the kexec-ed kernel can call EFI > runtime, is now useless?! Well, anyone who is using the mappings in -4G down range still needs your code to map their stuff into the appropriate spot in the kexec'd kernel, so anybody who is doing things in the most up-to-date manner still makes use of your new code, right? AFAIK, we pass up the efi_runtime_map to the kexec'd kernel, and then process the memory descriptors one by one, mapping in their virtual addresses during kexec_enter_virtual_mode. All of this relies on your efforts to pass up the virtual mappings. The only thing we're adding here is the physical mappings, to match what is availble in the primary kernel. > What if the physical address is occupied by the kexec kernel? Ah, that's a good question. I usually don't force the placement of my crashkernel, so I can't exactly comment intelligently on what will happen in that situation. I will look into this, but in our case, I believe that the primary kernel would either fail to boot, or fail to load the kexec kernel if we placed it over the range where are EFI code gets mapped in, which I think is an issue even without this patch. IIRC, the crashkernel memory gets reserved really early, so I'd imagine we'd hit the former problem, since we will try to remap into that same space in uv_bios_init. This is sort of a hand-wavey answer - I will investigate the his further to make sure that I'm not making any stupid assumptions (there's a good chance that I am :) > Why do you guys need the physical mapping all of a sudden? It's not that we need it all of the sudden, necessarily, it's just that we've had to make other changes to make things work with the new, (almost) completely isolated, EFI page tables. We ended up choosing the lesser of two evils, and have decided to temporarily rely on the physical address of our runtime code, instead of continuing to rely on EFI_OLD_MEMMAP. I guess what I'm saying is, if we hadn't been relying on some semi-undefined behavior in the EFI memory mapping scheme, we would've been relying on the physical address for quite a while, since nobody would have been mapping in the virtual address for us. It's important to note that we've been dancing back and forth between workarounds for some slightly inorrect assumptions, and some actual bug fixes. We're working to get everything 100% compatible with the new memory mapping schemes, but there're a few pieces of the puzzle we haven't gotten around to yet. > Your patch is basically rendering all the effort moot and we could've > saved ourselves all that trouble of doing all that virtual address > mapping and done the 1:1 thing. > > Which really is probably simpler since we have an EFI-specific page > table and running EFI in the kexec-ed kernel would mean basically > recreating it. > > What am I missing? I don't think it renders all of your effort worthless. It just allows those who haven't had a chance to completely update their code to work with the new mapping schemes a way to utilize EFI runtime callbacks, in a kexec'd kernel. I do understand that, in a perfect world, we would be able to just hop right in and use your memory mapping scheme. At the same time, I don't see how this is that much different from what we already do in the primary, non-kexec'd kernel. If there are strong objections to this change, I won't pursue it further. We will be able to achieve the same effect once we've had a chance to update our code to register a callback with SetVirtualAddressMap to fix up our function pointer. This is on my upcoming to-do list, but it'll be a little bit before I have a chance to get it finished. Thanks for the input, Boris! - Alex -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
