Messages in this thread | | | Date | Fri, 25 Jan 2008 11:27:38 +0100 | From | Ingo Molnar <> | Subject | Re: [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
| |