On Mon, Jan 26, 2015 at 10:47:10AM +0000, Chris Wilson wrote: > An interesting bug occurs on Pineview through which the root cause is > that the writes of the PTE values into the GTT is not serialised with > subsequent memory access through the GTT (when using WC updates of the > PTE values). This is despite there being a posting read after the GTT > update. However, by changing the address of the posting read, the memory > access is indeed serialised correctly. > > Whilst we are manipulating the memory barriers, we can remove the > compiler :memory restraint on the intermediate PTE writes knowing that > we explicitly perform a posting read afterwards. > > v2: Replace posting reads with explicit write memory barriers - in > particular this is advantages in case of single page objects. Update > comments to mention this issue is only with WC writes. > > Testcase: igt/gem_exec_big #pnv > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88191 > Tested-by: huax.lu@xxxxxxxxx (v1) > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> Shouldn't we Cc: stable@xxxxxxxxxxxxxxx too? -Daniel > --- > drivers/char/agp/intel-gtt.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c > index 92aa43fa8d70..0b4188b9af7c 100644 > --- a/drivers/char/agp/intel-gtt.c > +++ b/drivers/char/agp/intel-gtt.c > @@ -225,7 +225,7 @@ static int i810_insert_dcache_entries(struct agp_memory *mem, off_t pg_start, > intel_private.driver->write_entry(addr, > i, type); > } > - readl(intel_private.gtt+i-1); > + wmb(); > > return 0; > } > @@ -329,7 +329,7 @@ static void i810_write_entry(dma_addr_t addr, unsigned int entry, > break; > } > > - writel(addr | pte_flags, intel_private.gtt + entry); > + writel_relaxed(addr | pte_flags, intel_private.gtt + entry); > } > > static const struct aper_size_info_fixed intel_fake_agp_sizes[] = { > @@ -735,7 +735,7 @@ static void i830_write_entry(dma_addr_t addr, unsigned int entry, > if (flags == AGP_USER_CACHED_MEMORY) > pte_flags |= I830_PTE_SYSTEM_CACHED; > > - writel(addr | pte_flags, intel_private.gtt + entry); > + writel_relaxed(addr | pte_flags, intel_private.gtt + entry); > } > > bool intel_enable_gtt(void) > @@ -858,7 +858,7 @@ void intel_gtt_insert_sg_entries(struct sg_table *st, > j++; > } > } > - readl(intel_private.gtt+j-1); > + wmb(); > } > EXPORT_SYMBOL(intel_gtt_insert_sg_entries); > > @@ -875,7 +875,7 @@ static void intel_gtt_insert_pages(unsigned int first_entry, > intel_private.driver->write_entry(addr, > j, flags); > } > - readl(intel_private.gtt+j-1); > + wmb(); > } > > static int intel_fake_agp_insert_entries(struct agp_memory *mem, > @@ -938,7 +938,7 @@ void intel_gtt_clear_range(unsigned int first_entry, unsigned int num_entries) > intel_private.driver->write_entry(intel_private.scratch_page_dma, > i, 0); > } > - readl(intel_private.gtt+i-1); > + wmb(); > } > EXPORT_SYMBOL(intel_gtt_clear_range); > > @@ -1106,7 +1106,7 @@ static void i965_write_entry(dma_addr_t addr, > > /* Shift high bits down */ > addr |= (addr >> 28) & 0xf0; > - writel(addr | pte_flags, intel_private.gtt + entry); > + writel_relaxed(addr | pte_flags, intel_private.gtt + entry); > } > > static int i9xx_setup(void) > -- > 2.1.4 > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
