lkml.org 
[lkml]   [2008]   [Jan]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] x86: ioremap_nocache fix

* Huang, Ying <ying.huang@intel.com> wrote:

> This patch fixes a bug of ioremap_nocache. ioremap_nocache() will call
> __ioremap() with flags != 0 to do the real work, which will call
> change_page_attr_addr() if phys_addr + size - 1 < (end_pfn_map <<
> PAGE_SHIFT). But some pages between 0 ~ end_pfn_map << PAGE_SHIFT are
> not mapped by identity map, this will make change_page_attr_addr
> failed.

very interesting! Is this in response to a bug you've triggered?

i believe the scenario you outlne could trigger on 64-bit boxen, if they
try to ioremap an area not covered by the direct ptes:

> @@ -41,8 +41,15 @@ ioremap_change_attr(unsigned long phys_a
> if (phys_addr + size - 1 < (end_pfn_map << PAGE_SHIFT)) {
> unsigned long npages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
> unsigned long vaddr = (unsigned long) __va(phys_addr);
> + int level;
>
> /*
> + * If there is no identity map for this address,
> + * change_page_attr_addr is unnecessary
> + */
> + if (!lookup_address(vaddr, &level))
> + return err;
> + /*

and we'd incorrectly fail the ioremap() with -EINVAL, and return a hard
error to the driver - and causing broken boxes, right?

Thomas has unified most of ioremap*.c as well, and we'll make sure this
fix survives the unification. The 64-bit code limped along before by
accident, because the (pre-cleanup) 64-bit __change_page_attr() code
incorrectly returned 0 for invalid ranges:

kpte = lookup_address(address);
if (!kpte) return 0;

which masked the bug you fixed now. 32-bit always returned -EINVAL on
invalid ranges.

Ingo


\
 
 \ /
  Last update: 2008-01-25 11:31    [W:0.824 / U:0.424 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site