Skip site navigation (1)Skip section navigation (2) 
Date:      Tue, 29 Jul 2014 13:08:46 +0000 (UTC)
From:      Marius Strobl <marius@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-9@freebsd.org
Subject:   svn commit: r269236 - stable/9/sys/i386/i386
Message-ID:  <201407291308.s6TD8kL5066969@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: marius
Date: Tue Jul 29 13:08:46 2014
New Revision: 269236
URL: http://svnweb.freebsd.org/changeset/base/269236

Log:
  MFC: r269050
  
  - Copying and zeroing pages via temporary mappings involves updating the
    corresponding page tables followed by accesses to the pages in question.
    This sequence is subject to the situation exactly described in the "AMD64
    Architecture Programmer's Manual Volume 2: System Programming" rev. 3.23,
    "7.3.1 Special Coherency Considerations" [1, p. 171 f.]. Therefore, issuing
    the INVLPG right after modifying the PTE bits is crucial.
    For pmap_copy_page(), this has been broken in r124956 and later on carried
    over to pmap_copy_pages() derived from the former, while all other places
    in the i386 PMAP code use the correct order of instructions in this regard.
    Fixing the latter breakage solves the problem of data corruption seen with
    unmapped I/O enabled when running at least bare metal on AMD R-268D APUs.
    However, this might also fix similar corruption reported for virtualized
    environments.
  - In pmap_copy_pages(), correctly set the cache bits on the source page being
    copied. This change is thought to be a NOP for the real world, though. [2]
  
  1: http://amd-dev.wpengine.netdna-cdn.com/wordpress/media/2012/10/24593_APM_v21.pdf
  
  Submitted by:	kib [2]
  Reviewed by:	alc, kib
  Sponsored by:	Bally Wulff Games & Entertainment GmbH

Modified:
  stable/9/sys/i386/i386/pmap.c
Directory Properties:
  stable/9/sys/   (props changed)

Modified: stable/9/sys/i386/i386/pmap.c
==============================================================================
--- stable/9/sys/i386/i386/pmap.c	Tue Jul 29 13:08:37 2014	(r269235)
+++ stable/9/sys/i386/i386/pmap.c	Tue Jul 29 13:08:46 2014	(r269236)
@@ -1302,6 +1302,13 @@ pmap_pte_release(pt_entry_t *pte)
 		mtx_unlock(&PMAP2mutex);
 }
 
+/*
+ * NB:  The sequence of updating a page table followed by accesses to the
+ * corresponding pages is subject to the situation described in the "AMD64
+ * Architecture Programmer's Manual Volume 2: System Programming" rev. 3.23,
+ * "7.3.1 Special Coherency Considerations".  Therefore, issuing the INVLPG
+ * right after modifying the PTE bits is crucial.
+ */
 static __inline void
 invlcaddr(void *caddr)
 {
@@ -4249,12 +4256,12 @@ pmap_copy_page(vm_page_t src, vm_page_t 
 	if (*sysmaps->CMAP2)
 		panic("pmap_copy_page: CMAP2 busy");
 	sched_pin();
-	invlpg((u_int)sysmaps->CADDR1);
-	invlpg((u_int)sysmaps->CADDR2);
 	*sysmaps->CMAP1 = PG_V | VM_PAGE_TO_PHYS(src) | PG_A |
 	    pmap_cache_bits(src->md.pat_mode, 0);
+	invlcaddr(sysmaps->CADDR1);
 	*sysmaps->CMAP2 = PG_V | PG_RW | VM_PAGE_TO_PHYS(dst) | PG_A | PG_M |
 	    pmap_cache_bits(dst->md.pat_mode, 0);
+	invlcaddr(sysmaps->CADDR2);
 	bcopy(sysmaps->CADDR1, sysmaps->CADDR2, PAGE_SIZE);
 	*sysmaps->CMAP1 = 0;
 	*sysmaps->CMAP2 = 0;
@@ -4282,8 +4289,6 @@ pmap_copy_pages(vm_page_t ma[], vm_offse
 		panic("pmap_copy_pages: CMAP2 busy");
 	sched_pin();
 	while (xfersize > 0) {
-		invlpg((u_int)sysmaps->CADDR1);
-		invlpg((u_int)sysmaps->CADDR2);
 		a_pg = ma[a_offset >> PAGE_SHIFT];
 		a_pg_offset = a_offset & PAGE_MASK;
 		cnt = min(xfersize, PAGE_SIZE - a_pg_offset);
@@ -4291,9 +4296,11 @@ pmap_copy_pages(vm_page_t ma[], vm_offse
 		b_pg_offset = b_offset & PAGE_MASK;
 		cnt = min(cnt, PAGE_SIZE - b_pg_offset);
 		*sysmaps->CMAP1 = PG_V | VM_PAGE_TO_PHYS(a_pg) | PG_A |
-		    pmap_cache_bits(b_pg->md.pat_mode, 0);
+		    pmap_cache_bits(a_pg->md.pat_mode, 0);
+		invlcaddr(sysmaps->CADDR1);
 		*sysmaps->CMAP2 = PG_V | PG_RW | VM_PAGE_TO_PHYS(b_pg) | PG_A |
 		    PG_M | pmap_cache_bits(b_pg->md.pat_mode, 0);
+		invlcaddr(sysmaps->CADDR2);
 		a_cp = sysmaps->CADDR1 + a_pg_offset;
 		b_cp = sysmaps->CADDR2 + b_pg_offset;
 		bcopy(a_cp, b_cp, cnt);

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201407291308.s6TD8kL5066969>