Simon Horman wrote: > On Sun, Nov 04, 2007 at 12:30:14AM +0100, Roel Kluin wrote: >> I am less certain about this one, so please review >> -- >> Iounmap when EFI won't switch to virtual mode >> > > Hi Roel, > > I'm not really sure what the intention of this patch is. > But I'm pretty sure its not doing what you want it to do. > > I guess that you wish to reverse all the calls to ioremap() > that are made in efi_enter_virtual_mode(). ioremap() is > called during iterating through efi_map_start, but you only > seem to call iounmap on whatever md happens to be set > to at the end of the iteration. Surely you need to run through > efi_map_start again? Yes, you are right, this was what I had in mind: -- if (status != EFI_SUCCESS) { + for (p = efi_map_start; p < efi_map_end; p += efi_desc_size) { + md = p; + if ((md->attribute & EFI_MEMORY_RUNTIME) && + (md->attribute & EFI_MEMORY_UC) || + (md->attribute & EFI_MEMORY_WC)) + iounmap(md->virt_addr); + } printk(KERN_WARNING "warning: unable to switch EFI into virtual mode " "(status=%lu)\n", status); return; --- > The next thing that I wonder about is weather calling iounmap() > actually reverses ioremap() in this case. Though now that I look > at it and see that basically iounmap() will do nothing in > this case, which seems appropriate as in this case ioremap() > ends up being: > > return (void __iomem *) (__IA64_UNCACHED_OFFSET | phys_addr) > > Its probably not to much of a bother that all the md->virt_addr have > been mangled and stay mangled. Though if we are concerned about such > things, perhaps it would be cleaner to zero them on error? > Like this? --- if (status != EFI_SUCCESS) { + for (p = efi_map_start; p < efi_map_end; p += efi_desc_size) { + md = p; + if ((md->attribute & EFI_MEMORY_RUNTIME) && + (md->attribute & EFI_MEMORY_UC) || + (md->attribute & EFI_MEMORY_WC)) + md->virt_addr = 0; + } printk(KERN_WARNING "warning: unable to switch EFI into virtual mode " "(status=%lu)\n", status); return; --- I fear the remainder of your review is beyond my scope. My field of expertise is biomedical sciences :) Roel - To unsubscribe from this list: send the line "unsubscribe linux-ia64" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
![]() |