lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <809e6805f78212758097c0a74f9f59053cfdafaf.1368485053.git.luto@amacapital.net>
Date:	Mon, 13 May 2013 16:58:43 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
	linux-fbdev@...r.kernel.org
Cc:	Daniel Vetter <daniel.vetter@...ll.ch>,
	Jerome Glisse <j.glisse@...il.com>,
	Alex Deucher <alexdeucher@...il.com>,
	Dave Airlie <airlied@...il.com>,
	Andy Lutomirski <luto@...capital.net>
Subject: [PATCH v3 4/9] drm,agpgart: Use pgprot_writecombine for AGP maps and make the MTRR optional

I'm not sure I understand the intent of the previous behavior.  mmap
on /dev/agpgart and DRM_AGP maps had no cache flags set, so they
would be fully cacheable.  But the DRM code (most of the time) would
add a write-combining MTRR that would change the effective memory
type to WC.

The new behavior just requests WC explicitly for all AGP maps.

If there is any code out there that expects cacheable access to the
AGP aperture (because the drm driver doesn't request an MTRR or
because it's using /dev/agpgart directly), then it will now end up
with a UC or WC mapping, depending on the architecture and PAT
availability.  But cacheable access to the aperture seems like it's
asking for trouble, because, AIUI, the aperture is an alias of RAM.

Reviewed-by: Daniel Vetter <daniel.vetter@...ll.ch>
Signed-off-by: Andy Lutomirski <luto@...capital.net>
---

It's conceivable that libpciaccess could have issues with this due to
memtype conflicts, but I think this is unlikely (as long as a WC
mapping gets there first, I think the conflict resolution rules will
all work out).  In any case, everything that maps the AGP aperture
ought to be using /dev/agpgart or the DRM API, in which case
everything should be okay.

I don't have anything with an AGP slot to test AFAIK.

 drivers/char/agp/frontend.c |  8 +++++---
 drivers/gpu/drm/drm_pci.c   |  8 ++++----
 drivers/gpu/drm/drm_stub.c  | 10 ++--------
 drivers/gpu/drm/drm_vm.c    | 11 ++++-------
 4 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/drivers/char/agp/frontend.c b/drivers/char/agp/frontend.c
index 2e04433..1b19239 100644
--- a/drivers/char/agp/frontend.c
+++ b/drivers/char/agp/frontend.c
@@ -603,7 +603,8 @@ static int agp_mmap(struct file *file, struct vm_area_struct *vma)
 			vma->vm_ops = kerninfo.vm_ops;
 		} else if (io_remap_pfn_range(vma, vma->vm_start,
 				(kerninfo.aper_base + offset) >> PAGE_SHIFT,
-					    size, vma->vm_page_prot)) {
+				size,
+				pgprot_writecombine(vma->vm_page_prot))) {
 			goto out_again;
 		}
 		mutex_unlock(&(agp_fe.agp_mutex));
@@ -618,8 +619,9 @@ static int agp_mmap(struct file *file, struct vm_area_struct *vma)
 		if (kerninfo.vm_ops) {
 			vma->vm_ops = kerninfo.vm_ops;
 		} else if (io_remap_pfn_range(vma, vma->vm_start,
-					    kerninfo.aper_base >> PAGE_SHIFT,
-					    size, vma->vm_page_prot)) {
+				kerninfo.aper_base >> PAGE_SHIFT,
+				size,
+				pgprot_writecombine(vma->vm_page_prot))) {
 			goto out_again;
 		}
 		mutex_unlock(&(agp_fe.agp_mutex));
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index bd719e9..d0f6699 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -278,10 +278,10 @@ int drm_pci_agp_init(struct drm_device *dev)
 		}
 		if (drm_core_has_MTRR(dev)) {
 			if (dev->agp)
-				dev->agp->agp_mtrr =
-					mtrr_add(dev->agp->agp_info.aper_base,
-						 dev->agp->agp_info.aper_size *
-						 1024 * 1024, MTRR_TYPE_WRCOMB, 1);
+				dev->agp->agp_mtrr = arch_phys_wc_add(
+					dev->agp->agp_info.aper_base,
+					dev->agp->agp_info.aper_size *
+					1024 * 1024);
 		}
 	}
 	return 0;
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 7d30802..9e2acdf 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -451,14 +451,8 @@ void drm_put_dev(struct drm_device *dev)
 
 	drm_lastclose(dev);
 
-	if (drm_core_has_MTRR(dev) && drm_core_has_AGP(dev) &&
-	    dev->agp && dev->agp->agp_mtrr >= 0) {
-		int retval;
-		retval = mtrr_del(dev->agp->agp_mtrr,
-				  dev->agp->agp_info.aper_base,
-				  dev->agp->agp_info.aper_size * 1024 * 1024);
-		DRM_DEBUG("mtrr_del=%d\n", retval);
-	}
+	if (drm_core_has_MTRR(dev) && drm_core_has_AGP(dev) && dev->agp)
+		arch_phys_wc_del(dev->agp->agp_mtrr);
 
 	if (dev->driver->unload)
 		dev->driver->unload(dev);
diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index 163f436..b1e4ec8 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -49,13 +49,10 @@ static pgprot_t drm_io_prot(struct drm_local_map *map,
 	pgprot_t tmp = vm_get_page_prot(vma->vm_flags);
 
 #if defined(__i386__) || defined(__x86_64__)
-	if (map->type != _DRM_AGP) {
-		if (map->type == _DRM_FRAME_BUFFER ||
-		    map->flags & _DRM_WRITE_COMBINING)
-			tmp = pgprot_writecombine(tmp);
-		else
-			tmp = pgprot_noncached(tmp);
-	}
+	if (map->type == _DRM_REGISTERS && !(map->flags & _DRM_WRITE_COMBINING))
+		tmp = pgprot_noncached(tmp);
+	else
+		tmp = pgprot_writecombine(tmp);
 #elif defined(__powerpc__)
 	pgprot_val(tmp) |= _PAGE_NO_CACHE;
 	if (map_type == _DRM_REGISTERS)
-- 
1.8.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ