Re: [PATCH 1/2] x86: ioremap: fix wrong physical address handling

Kenji Kaneshige
2010-06-17T06:29:18Z

(2010/06/17 11:50), Matthew Wilcox wrote:
> On Thu, Jun 17, 2010 at 10:30:06AM +0900, Kenji Kaneshige wrote:
>> Index: linux-2.6.34/arch/x86/mm/ioremap.c
>> ===================================================================
>> --- linux-2.6.34.orig/arch/x86/mm/ioremap.c 2010-06-15 04:43:00.978332015 +0900
>> +++ linux-2.6.34/arch/x86/mm/ioremap.c 2010-06-15 05:32:59.291693007 +0900
>> @@ -62,8 +62,8 @@
>> static void __iomem *__ioremap_caller(resource_size_t phys_addr,
>> unsigned long size, unsigned long prot_val, void *caller)
>> {
>> - unsigned long pfn, offset, vaddr;
>> - resource_size_t last_addr;
>> + unsigned long offset, vaddr;
>> + resource_size_t pfn, last_pfn, last_addr;
>
> I have a hard time understanding this change. pfn is always a physical
> address shifted by PAGE_SHIFT. So a 32-bit pfn supports up to 44-bit
> physical addresses. Are your addresses above 44-bits?
>
>> @@ -115,7 +113,7 @@
>> * Mappings have to be page-aligned
>> */
>> offset = phys_addr& ~PAGE_MASK;
>> - phys_addr&= PAGE_MASK;
>> + phys_addr = (phys_addr>> PAGE_SHIFT)<< PAGE_SHIFT;
>
> I'd rather see PAGE_MASK fixed. Would this work?
>
> #define PAGE_SIZE (_AC(1,UL)<< PAGE_SHIFT)
> -#define PAGE_MASK (~(PAGE_SIZE-1))
> +#define PAGE_MASK (~(PAGE_SIZE-1ULL))
>

I think it should work. But I'm worrying about regressions.
Now I think using PHYSICAL_PAGE_MASK (as my v.1 patch did) is good idea
again. What do you think about this?

Thanks,
Kenji Kaneshige