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
|