[PATCH] drm/i915: fix memory corruption with GM965 and >4GB RAM

Jan Niehusmann
2011-03-01T22:24:19Z

On Sat, Feb 26, 2011 at 12:05:27AM +0100, Daniel Vetter wrote:
> Actually, on style points I prefer your patch: The hw status page is
> allocated with drm_pci_alloc which calls dma_alloc_coherent, so setting
> the coherent mask is sufficient. The dma mask set in the gtt is
> essentially useless, because we call get_user_pages on everything anyway
> (in gem - iirc agp uses it). I just think it's confusing to limit the
> general dma mask and continue to happily map pages above 4G.

Indeed, setting dma_alloc_coherent should be sufficient, AFAIKT. After
all, only the HWS seems to be affected, which is allocated with
drm_pci_alloc, which in turn uses dma_alloc_coherent.

I just tried the patch below, it also works for me (as expected). Added
the comment because otherwise it wouldn't be obvious why the mask gets
set to 32 bit.

What I don't know is if really only Broadwater and Crestline chips are
affected. The tests were done with a Crestline one. But I think it's a
fair guess that the bug would have been noticed earlier if more recent
chips were affected, as >4GB RAM have become much more common since then.

Signed-off-by: Jan Niehusmann <jan@gondor.com>

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 17bd766..1961580 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1895,6 +1895,12 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
if (IS_GEN2(dev))
dma_set_coherent_mask(&dev->pdev->dev, DMA_BIT_MASK(30));

+ /* 965GM sometimes incorrectly writes to hardware status page (HWS)
+ * using 32bit addressing, overwriting memory if HWS is located
+ * above 4GB. */
+ if (IS_BROADWATER(dev) || IS_CRESTLINE(dev))
+ dma_set_coherent_mask(&dev->pdev->dev, DMA_BIT_MASK(32));
+
mmio_bar = IS_GEN2(dev) ? 1 : 0;
dev_priv->regs = pci_iomap(dev->pdev, mmio_bar, 0);
if (!dev_priv->regs) {