Re: [Linux-ia64] Re: Status on ioremap patch

[Steffen Persvold <sp@scali.no>]

Keith Owens wrote:
> 
> On Sun, 06 Jan 2002 12:02:01 +0100,
> Steffen Persvold <sp@scali.no> wrote:
> >Keith Owens wrote:
> >>
> >> On Sat, 05 Jan 2002 16:01:29 +0100,
> >> Steffen Persvold <sp@scali.no> wrote:
> >> >--- linux-2.4.17/mm/memory.c.orig      Fri Dec 21 18:42:05 2001
> >> >+++ linux-2.4.17/mm/memory.c   Sat Jan  5 13:13:26 2002
> >> >@@ -791,6 +791,9 @@
> >> >  * maps a range of physical memory into the requested pages. the old
> >> >  * mappings are removed. any references to nonexistent pages results
> >> >  * in null mappings (currently treated as "copy-on-access")
> >> >+ *
> >> >+ * For physical (or I/O) memory mapped into the kernel virtual space,
> >> >+ * the old mappings will not be removed.
> >>
> >> That comment worries me.  If you have multiple mappings for the same
> >> page then you may have problems on hardware that uses virtually indexed
> >> caches.  Two virtual addresses could map to the same physical page but
> >> index to different cache lines, destroying cache coherency.  How do you
> >> prevent that?
> >>
> >
> >Well, actually multiple mappings in kernel space is denied :
> >
> >+               if (address > VMALLOC_START && address < VMALLOC_END) {
> >+                       if (!pte_none(*pte)) {
> >+                               printk("remap_area_pte: page already exists\n");
> >+                               BUG();
> >+                       }
> >+                       set_pte(pte, mk_pte_phys(phys_addr, prot));
> >+               } else {
> >+                       struct page *page;
> >+                       pte_t oldpage;
> >+                       oldpage = ptep_get_and_clear(pte);
> >+
> >+                       page = virt_to_page(__va(phys_addr));
> >+                       if ((!VALID_PAGE(page)) || PageReserved(page))
> >+                               set_pte(pte, mk_pte_phys(phys_addr, prot));
> >+                       forget_pte(oldpage);
> >+               }
> 
> What about one mapping in kernel space and one in user space, or two
> mappings in user space?  As long as you have more than one mapping to
> the same physical page you must handle virtual cache aliasing.
> 

If you do two mappings from userspace (via mmap() and from there remap_page_range()), the "else"
case would be true because then the addess isn't a kernel virtual address and the old mapping would
be forgotten. The case where ioremap() is called first and then remap_page_range() is called later
(via mmap() function) will also work fine. The two cases I can think of where the BUG() case wold
trigger, are when remap_page_range() is called first and then ioremap() is called, or when ioremap()
is called twice. None of these two cases is something that I would consider "legal" and this would
also happen with the old (similar) ioremap() implementation.

Do you think I should drop the if/else statements and just always treat it as it was two mappings
from userspace (currently the "else" case) ?

Regards,
-- 
  Steffen Persvold   | Scalable Linux Systems |   Try out the world's best   
 mailto:sp@scali.no  |  http://www.scali.com  | performing MPI implementation:
Tel: (+47) 2262 8950 |   Olaf Helsets vei 6   |      - ScaMPI 1.12.2 -         
Fax: (+47) 2262 8951 |   N0621 Oslo, NORWAY   | >300MBytes/s and <4uS latency