Re: [tip:x86/urgent] x86, efi: Delete efi_ioremap() and fix CONFIG_X86_32 oops

[Yinghai Lu <yinghai@xxxxxxxxxx>]

On Thu, Mar 15, 2012 at 5:40 AM, Matt Fleming <matt.fleming@xxxxxxxxx> wrote:
> On Mon, 2012-03-12 at 22:39 -0700, Yinghai Lu wrote:
>> On Mon, Mar 12, 2012 at 5:38 AM, Matt Fleming <matt.fleming@xxxxxxxxx> wrote:
>>
>> > Have you tested my patch? Have you hit this bug or is it just from code
>> > inspection. I'm starting to feel a bit silly now because I can't see the
>> > problem you're describing.
>>
>> from code inspection.
>>
>> your new init_memory_mapping() will only map mem under max_low_pfn ?
>
> No, that's not true for x86_64, look,
>
>        for (i = 0; i < e820.nr_map; i++) {
>                entry = &e820.map[i];
>                start = entry->addr;
>                end = start + entry->size;
>
>                /* We've already mapped below 1MB */
>                if (end < (1 << 20))
>                        continue;
>
>                if (start < (1 << 20))
>                        start = 1 << 20;
> #ifdef CONFIG_X86_32
>                /*
>                 * The map is sorted, so bail once we hit a region
>                 * that's above max_low_pfn.
>                 */
>                if (start >= max_low_pfn << PAGE_SHIFT)
>                        break;
>
>                if (end > max_low_pfn << PAGE_SHIFT)
>                        end = max_low_pfn << PAGE_SHIFT;
> #endif
>                switch (entry->type) {
>                case E820_RAM:
>                case E820_RESERVED_EFI:
>                case E820_ACPI:
>                case E820_NVS:
>                        last_pfn_mapped = __init_memory_mapping(start, end);
>                        break;
>                default:
>                        continue;
>                }
>
>                if (end <= max_low_pfn << PAGE_SHIFT)
>                        max_low_pfn_mapped = last_pfn_mapped;

why max_low_pfn is used here?

>        }
>
> The max_low_pfn checks are only for CONFIG_X86_32 so that the behaviour
> is the same as before this patch, i.e. we don't try to map above
> max_low_pfn.

ok, to simplify the code, in setup.c you could move
#ifdef CONFIG_X86_64
        if (max_pfn > max_low_pfn) {
                /* can we preseve max_low_pfn ?*/
                max_low_pfn = max_pfn;
        }
#endif

before calling new init_memory_mapping()...

so you could remove the #idef. in init_memory_mapping.

Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-tip-commits" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html