[rfc][patch] x86: avoid highmem cache attribute aliasing

[rfc][patch] x86: avoid highmem cache attribute aliasing

[Nick Piggin]

Highmem code can leave ptes and tlb entries around for a given page even after
kunmap, and after it has been freed.

>From what I can gather, the PAT code may change the cache attributes of
arbitrary physical addresses (ie. including highmem pages), which would result
in aliases in the case that it operates on one of these lazy tlb highmem
pages.

Flushing kmaps should solve the problem.

I've also just added code for conditional flushing if we haven't got 
any dangling highmem aliases -- this should help performance if we
change page attributes frequently or systems that aren't using much
highmem pages (eg. if < 4G RAM). Should be turned into 2 patches, but
just for RFC...

---
Index: linux-2.6/arch/x86/mm/pageattr.c
===================================================================
--- linux-2.6.orig/arch/x86/mm/pageattr.c
+++ linux-2.6/arch/x86/mm/pageattr.c
@@ -777,6 +777,9 @@ static int change_page_attr_set_clr(unsi
 		WARN_ON_ONCE(1);
 	}
 
+	/* Must avoid aliasing mappings in the highmem code */
+	kmap_flush_unused();
+
 	cpa.vaddr = addr;
 	cpa.numpages = numpages;
 	cpa.mask_set = mask_set;
Index: linux-2.6/mm/highmem.c
===================================================================
--- linux-2.6.orig/mm/highmem.c
+++ linux-2.6/mm/highmem.c
@@ -70,6 +70,7 @@ static DECLARE_WAIT_QUEUE_HEAD(pkmap_map
 static void flush_all_zero_pkmaps(void)
 {
 	int i;
+	int need_flush = 0;
 
 	flush_cache_kmaps();
 
@@ -101,8 +102,10 @@ static void flush_all_zero_pkmaps(void)
 			  &pkmap_page_table[i]);
 
 		set_page_address(page, NULL);
+		need_flush = 1;
 	}
-	flush_tlb_kernel_range(PKMAP_ADDR(0), PKMAP_ADDR(LAST_PKMAP));
+	if (need_flush)
+		flush_tlb_kernel_range(PKMAP_ADDR(0), PKMAP_ADDR(LAST_PKMAP));
 }
 
 /**

Re: [rfc][patch] x86: avoid highmem cache attribute aliasing

[Ingo Molnar]

* Nick Piggin <npiggin@suse.de> wrote:

> Highmem code can leave ptes and tlb entries around for a given page 
> even after kunmap, and after it has been freed.
> 
> From what I can gather, the PAT code may change the cache attributes 
> of arbitrary physical addresses (ie. including highmem pages), which 
> would result in aliases in the case that it operates on one of these 
> lazy tlb highmem pages.
> 
> Flushing kmaps should solve the problem.
> 
> I've also just added code for conditional flushing if we haven't got 
> any dangling highmem aliases -- this should help performance if we 
> change page attributes frequently or systems that aren't using much 
> highmem pages (eg. if < 4G RAM). Should be turned into 2 patches, but 
> just for RFC...

hm, such aliasing might happen in theory - and i guess in practice too 
if the AGP driver allocates/deallocates aperture in short succession. 

Maybe this could corrupt the X framebuffer - or even generic kernel RAM. 
Mind resending the two split up patches? The fix we might want to take 
into v2.6.27, the speedup probably for v2.6.28.

But maybe i'm missing something obvious that prevents such problems on 
32-bit systems - Venki, Suresh, what do you think?

	Ingo

Re: [rfc][patch] x86: avoid highmem cache attribute aliasing

[Suresh Siddha]

On Mon, Aug 11, 2008 at 10:10:19AM -0700, Ingo Molnar wrote:
> 
> * Nick Piggin <npiggin@suse.de> wrote:
> 
> > Highmem code can leave ptes and tlb entries around for a given page
> > even after kunmap, and after it has been freed.
> >
> > From what I can gather, the PAT code may change the cache attributes
> > of arbitrary physical addresses (ie. including highmem pages), which
> > would result in aliases in the case that it operates on one of these
> > lazy tlb highmem pages.
> >
> > Flushing kmaps should solve the problem.
> >
> > I've also just added code for conditional flushing if we haven't got
> > any dangling highmem aliases -- this should help performance if we
> > change page attributes frequently or systems that aren't using much
> > highmem pages (eg. if < 4G RAM). Should be turned into 2 patches, but
> > just for RFC...
> 
> hm, such aliasing might happen in theory - and i guess in practice too
> if the AGP driver allocates/deallocates aperture in short succession.
> 
> Maybe this could corrupt the X framebuffer - or even generic kernel RAM.
> Mind resending the two split up patches? The fix we might want to take
> into v2.6.27, the speedup probably for v2.6.28.
> 
> But maybe i'm missing something obvious that prevents such problems on
> 32-bit systems - Venki, Suresh, what do you think?

Andi pointed out in another thread that we don't use GFP_HIGHMEM in the
AGP drivers. And hence we haven't encountered an issue so far. But yes,
this is a good fix to close the hole.

thanks,
suresh

Re: [rfc][patch] x86: avoid highmem cache attribute aliasing

[Ingo Molnar]

* Suresh Siddha <suresh.b.siddha@intel.com> wrote:

> [...] But yes, this is a good fix to close the hole.

ok, i've queued it up in tip/x86/pat.

	Ingo