On Fri, 2013-01-25 at 07:41 +0000, Jan Beulich wrote: > >>> On 24.01.13 at 19:20, Matt Fleming <matt.fleming@xxxxxxxxx> wrote: > > On Fri, 2013-01-18 at 12:35 +0000, Jan Beulich wrote: > >> This fixes two issues: > >> - wrong memory type used for allocation intended to persist post-boot > > > > [...] > > > >> @@ -311,7 +311,7 @@ static efi_status_t setup_efi_pci(struct > >> size = pci->romsize + sizeof(*rom); > >> > >> status = efi_call_phys3(sys_table->boottime->allocate_pool, > >> - EFI_LOADER_DATA, size, &rom); > >> + EFI_RUNTIME_SERVICES_DATA, size, &rom); > >> > >> if (status != EFI_SUCCESS) > >> continue; > > > > I'm curious why you made this change. No one should be stealing that > > region of memory because that's all handled in parse_e820_ext() - it's > > marked as off limits wrt memory for the kernel's use. And the firmware > > certainly shouldn't start touching it. > > According to my reading of do_add_efi_memmap(), EFI_LOADER_DATA > gets treated the same as EFI_CONVENTIONAL_MEMORY, i.e. gets > marked as available for allocation. That's also how I'd expect things > to be. I don't think parse_e820_ext() matters here at all. Oh, I totally misread the flow of execution here. parse_setup_data() seems to be missing a SETUP_PCI case. Matthew, what guarantees do we have that the memory we've allocated for the PCI ROMs isn't going to be reused? I suspect what's needed is for parse_setup_data() to reserve the SETUP_PCI regions, just like you would for any other key data structure. It's the kernel's responsibility to protect any EFI_LOADER_DATA regions that it needs, we shouldn't have to trick ourselves by using the EFI_RUNTIME_SERVICES_DATA pool for allocations. -- 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
