On Tue, Jul 15, 2014 at 11:04:37AM -0400, Mark Salter wrote: > On Tue, 2014-07-15 at 15:49 +0100, Leif Lindholm wrote: > > On Tue, Jul 15, 2014 at 10:23:26AM -0400, Mark Salter wrote: > > > On Tue, 2014-07-15 at 14:54 +0100, Leif Lindholm wrote: > > > > On Tue, Jul 15, 2014 at 09:11:00AM -0400, Mark Salter wrote: > > > > > On Tue, 2014-07-15 at 11:02 +0100, Leif Lindholm wrote: > > > > > > > @@ -273,6 +282,10 @@ static void __init free_boot_services(void) > > > > > > > continue; > > > > > > > } > > > > > > > > > > > > > > + /* Don't free anything below kernel */ > > > > > > > + if (md->phys_addr < PHYS_OFFSET) > > > > > > > + continue; > > > > > > > + > > > > > > > > > > > > Is the spin table area really allocated as BOOT_SERVICES_*? > > > > > > > > > > No. It is EFI_RESERVED_TYPE. But if UEFI is allowed below the kernel, > > > > > then there could be BS code/data memory which we'd want to ignore. > > > > > > > > Well, if it is boot service code/data - then there is no need for us > > > > to keep it around after ExitBootServices(). > > > > > > One would think, but EFI has proven to be less than strictly compliant > > > in that regard in the past. I'm inclined to keep the boot services > > > around until after SetVirtualAddressMap just in case. > > > > But the function you add this clause to will still throw away all boot > > services code/data regions - just with this modification it skips > > those that happen to lie lower in the address space than the kernel. Returning to the actual code we are discussing here: The hunk above has no bearing on whether boot services regions are generally unmapped or not. It only filters explicitly those boot services regions that happen to be lower in memory than the kernel, and keep them around for the duration of the system. > > > > (And I do agree with Mark R here - let's not work around bugs that > > don't exist yet.) > > > > I'm not sure if they still exist or not, but on Foundation, I saw a > crash in SetVirtualAddressMap unless I kept BS regions around. For the topic of keeping boot services code around: I did also see issues with not keeping boot services regions on v7 - ages ago. I have not seen it this year, and I _really_ want to see if any such issues resurface. So post-3.16 I would quite like to see the call to free_boot_services() moved earlier to flush out any such issues before we see large-scale deployments. The foundation model is a development tool, not a production system, so any issue reproducable only there is a pure bonus - since that firmware can _always_ be updated. / Leif -- 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