All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86, AMD: Correct F15h IC aliasing issue
@ 2011-07-22 13:15 Borislav Petkov
  2011-07-24 11:13 ` Ingo Molnar
                   ` (2 more replies)
  0 siblings, 3 replies; 46+ messages in thread
From: Borislav Petkov @ 2011-07-22 13:15 UTC (permalink / raw)
  To: H. Peter Anvin, Ingo Molnar, Thomas Gleixner
  Cc: LKML, Borislav Petkov, Andre Przywara, Martin Pohlack

From: Borislav Petkov <borislav.petkov@amd.com>

This patch provides performance tuning for the "Bulldozer" CPU. With its
shared instruction cache there is a chance of generating an excessive
number of cache cross-invalidates when running specific workloads on the
cores of a compute module.

This excessive amount of cross-invalidations can be observed if cache
lines backed by shared physical memory alias in bits [14:12] of their
virtual addresses, as those bits are used for the index generation.

This patch addresses the issue by zeroing out the slice [14:12] of
the file mapping's virtual address at generation time, thus forcing
those bits the same for all mappings of a single shared library across
processes and, in doing so, avoids instruction cache aliases.

It also adds the kernel command line option
"unalias_va_addr=(32|64|off)" with which virtual address unaliasing
can be enabled for 32-bit or 64-bit x86 individually, or be completely
disabled.

This change leaves virtual region address allocation on other families
and/or vendors unaffected.

Signed-off-by: Andre Przywara <andre.przywara@amd.com>
Signed-off-by: Martin Pohlack <martin.pohlack@amd.com>
Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 Documentation/kernel-parameters.txt |    6 ++++
 arch/x86/include/asm/elf.h          |   36 +++++++++++++++++++++++++++
 arch/x86/kernel/cpu/amd.c           |    7 +++++
 arch/x86/kernel/sys_x86_64.c        |   46 ++++++++++++++++++++++++++++++++--
 arch/x86/mm/mmap.c                  |   15 -----------
 arch/x86/vdso/vma.c                 |   10 +++++++
 6 files changed, 102 insertions(+), 18 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index fd248a31..17eda04 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2535,6 +2535,12 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			Note that genuine overcurrent events won't be
 			reported either.
 
+	unalias_va_addr
+			[X86-64] Unalias VA address by clearing slice [14:12]
+			1: only on 32-bit
+			2: only on 64-bit
+			off: disabled on both 32 and 64-bit
+
 	unknown_nmi_panic
 			[X86] Cause panic on unknown NMI.
 
diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h
index f2ad216..89e38a4 100644
--- a/arch/x86/include/asm/elf.h
+++ b/arch/x86/include/asm/elf.h
@@ -4,6 +4,7 @@
 /*
  * ELF register definitions..
  */
+#include <linux/thread_info.h>
 
 #include <asm/ptrace.h>
 #include <asm/user.h>
@@ -320,4 +321,39 @@ extern int syscall32_setup_pages(struct linux_binprm *, int exstack);
 extern unsigned long arch_randomize_brk(struct mm_struct *mm);
 #define arch_randomize_brk arch_randomize_brk
 
+/*
+ * True on X86_32 or when emulating IA32 on X86_64
+ */
+static inline int mmap_is_ia32(void)
+{
+#ifdef CONFIG_X86_32
+	return 1;
+#endif
+#ifdef CONFIG_IA32_EMULATION
+	if (test_thread_flag(TIF_IA32))
+		return 1;
+#endif
+	return 0;
+}
+
+#define UNALIAS_VA_32 1
+#define UNALIAS_VA_64 2
+
+extern int unalias_va_addr;
+static inline unsigned long unalias_addr(unsigned long addr, bool incr)
+{
+	/* handle both 32- and 64-bit with a single conditional */
+	if (!(unalias_va_addr & (2 - mmap_is_ia32())))
+		return addr;
+
+	/* check if [14:12] are already cleared */
+	if (!(addr & (0x7 << PAGE_SHIFT)))
+		return addr;
+
+	addr = addr & ~(0x7 << PAGE_SHIFT);
+	if (incr)
+		addr += (0x8 << PAGE_SHIFT);
+
+	return addr;
+}
 #endif /* _ASM_X86_ELF_H */
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index b13ed39..2d380c6 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -458,6 +458,13 @@ static void __cpuinit early_init_amd(struct cpuinfo_x86 *c)
 					"with P0 frequency!\n");
 		}
 	}
+
+	if (unalias_va_addr == -1) {
+		if (c->x86 == 0x15)
+			unalias_va_addr = UNALIAS_VA_32 | UNALIAS_VA_64;
+		else
+			unalias_va_addr = 0;
+	}
 }
 
 static void __cpuinit init_amd(struct cpuinfo_x86 *c)
diff --git a/arch/x86/kernel/sys_x86_64.c b/arch/x86/kernel/sys_x86_64.c
index ff14a50..323c411 100644
--- a/arch/x86/kernel/sys_x86_64.c
+++ b/arch/x86/kernel/sys_x86_64.c
@@ -18,6 +18,30 @@
 #include <asm/ia32.h>
 #include <asm/syscalls.h>
 
+/* see Documentation/kernel-parameters.txt */
+int unalias_va_addr = -1;
+
+static int __init control_va_addr_unaliasing(char *str)
+{
+	if (*str == 0)
+		return 1;
+
+	if (*str == '=')
+		str++;
+
+	if (!strcmp(str, "32"))
+		unalias_va_addr = UNALIAS_VA_32;
+	else if (!strcmp(str, "64"))
+		unalias_va_addr = UNALIAS_VA_64;
+	else if (!strcmp(str, "off"))
+		unalias_va_addr = 0;
+	else
+		return 0;
+
+	return 1;
+}
+__setup("unalias_va_addr", control_va_addr_unaliasing);
+
 SYSCALL_DEFINE6(mmap, unsigned long, addr, unsigned long, len,
 		unsigned long, prot, unsigned long, flags,
 		unsigned long, fd, unsigned long, off)
@@ -92,6 +116,9 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
 	start_addr = addr;
 
 full_search:
+	if (filp)
+		addr = unalias_addr(addr, true);
+
 	for (vma = find_vma(mm, addr); ; vma = vma->vm_next) {
 		/* At this point:  (!vma || addr < vma->vm_end). */
 		if (end - len < addr) {
@@ -117,6 +144,9 @@ full_search:
 			mm->cached_hole_size = vma->vm_start - addr;
 
 		addr = vma->vm_end;
+
+		if (filp)
+			addr = unalias_addr(addr, true);
 	}
 }
 
@@ -161,10 +191,17 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
 
 	/* make sure it can fit in the remaining address space */
 	if (addr > len) {
-		vma = find_vma(mm, addr-len);
-		if (!vma || addr <= vma->vm_start)
+		unsigned long tmp_addr = addr;
+
+		if (filp)
+			tmp_addr = unalias_addr(addr - len, false);
+		else
+			tmp_addr = tmp_addr - len;
+
+		vma = find_vma(mm, tmp_addr);
+		if (!vma || tmp_addr + len <= vma->vm_start)
 			/* remember the address as a hint for next time */
-			return mm->free_area_cache = addr-len;
+			return mm->free_area_cache = tmp_addr;
 	}
 
 	if (mm->mmap_base < len)
@@ -173,6 +210,9 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
 	addr = mm->mmap_base-len;
 
 	do {
+		if (filp)
+			addr = unalias_addr(addr, false);
+
 		/*
 		 * Lookup failure means no vma is above this address,
 		 * else if new region fits below vma->vm_start,
diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
index 1dab519..d4c0736 100644
--- a/arch/x86/mm/mmap.c
+++ b/arch/x86/mm/mmap.c
@@ -51,21 +51,6 @@ static unsigned int stack_maxrandom_size(void)
 #define MIN_GAP (128*1024*1024UL + stack_maxrandom_size())
 #define MAX_GAP (TASK_SIZE/6*5)
 
-/*
- * True on X86_32 or when emulating IA32 on X86_64
- */
-static int mmap_is_ia32(void)
-{
-#ifdef CONFIG_X86_32
-	return 1;
-#endif
-#ifdef CONFIG_IA32_EMULATION
-	if (test_thread_flag(TIF_IA32))
-		return 1;
-#endif
-	return 0;
-}
-
 static int mmap_is_legacy(void)
 {
 	if (current->personality & ADDR_COMPAT_LAYOUT)
diff --git a/arch/x86/vdso/vma.c b/arch/x86/vdso/vma.c
index 7abd2be..a48dfd2 100644
--- a/arch/x86/vdso/vma.c
+++ b/arch/x86/vdso/vma.c
@@ -69,6 +69,16 @@ static unsigned long vdso_addr(unsigned long start, unsigned len)
 	addr = start + (offset << PAGE_SHIFT);
 	if (addr >= end)
 		addr = end;
+
+	if (unalias_va_addr) {
+		/*
+		 * page-align it here so that get_unmapped_area doesn't
+		 * align it wrongfully again to the next page
+		 */
+		addr = PAGE_ALIGN(addr);
+		addr = unalias_addr(addr, false);
+	}
+
 	return addr;
 }
 
-- 
1.7.4.rc2


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-22 13:15 [PATCH] x86, AMD: Correct F15h IC aliasing issue Borislav Petkov
@ 2011-07-24 11:13 ` Ingo Molnar
  2011-07-24 13:40   ` Borislav Petkov
  2011-07-24 16:04 ` Linus Torvalds
  2011-07-26 17:59 ` Avi Kivity
  2 siblings, 1 reply; 46+ messages in thread
From: Ingo Molnar @ 2011-07-24 11:13 UTC (permalink / raw)
  To: Borislav Petkov, Linus Torvalds, Andrew Morton
  Cc: H. Peter Anvin, Thomas Gleixner, LKML, Borislav Petkov,
	Andre Przywara, Martin Pohlack


(Cc:-ed Linus and akpm)

* Borislav Petkov <bp@amd64.org> wrote:

> From: Borislav Petkov <borislav.petkov@amd.com>
> 
> This patch provides performance tuning for the "Bulldozer" CPU. With its
> shared instruction cache there is a chance of generating an excessive
> number of cache cross-invalidates when running specific workloads on the
> cores of a compute module.
> 
> This excessive amount of cross-invalidations can be observed if cache
> lines backed by shared physical memory alias in bits [14:12] of their
> virtual addresses, as those bits are used for the index generation.
> 
> This patch addresses the issue by zeroing out the slice [14:12] of
> the file mapping's virtual address at generation time, thus forcing
> those bits the same for all mappings of a single shared library across
> processes and, in doing so, avoids instruction cache aliases.

So all commonly-aliased virtual mappings of the same pages should be 
32K granular aligned for good performance? Seems rather unfortunate 
for a modern CPU - is this going to be the case for all family 0x15 
CPUs?

> It also adds the kernel command line option
> "unalias_va_addr=(32|64|off)" with which virtual address unaliasing
> can be enabled for 32-bit or 64-bit x86 individually, or be completely
> disabled.

So the default behavior is to have this aligning enabled.

> 
> This change leaves virtual region address allocation on other families
> and/or vendors unaffected.
> 
> Signed-off-by: Andre Przywara <andre.przywara@amd.com>
> Signed-off-by: Martin Pohlack <martin.pohlack@amd.com>
> Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
> ---
>  Documentation/kernel-parameters.txt |    6 ++++
>  arch/x86/include/asm/elf.h          |   36 +++++++++++++++++++++++++++
>  arch/x86/kernel/cpu/amd.c           |    7 +++++
>  arch/x86/kernel/sys_x86_64.c        |   46 ++++++++++++++++++++++++++++++++--
>  arch/x86/mm/mmap.c                  |   15 -----------
>  arch/x86/vdso/vma.c                 |   10 +++++++
>  6 files changed, 102 insertions(+), 18 deletions(-)
> 
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index fd248a31..17eda04 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -2535,6 +2535,12 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>  			Note that genuine overcurrent events won't be
>  			reported either.
>  
> +	unalias_va_addr
> +			[X86-64] Unalias VA address by clearing slice [14:12]
> +			1: only on 32-bit
> +			2: only on 64-bit
> +			off: disabled on both 32 and 64-bit

This says nothing about why it's cleared, what it does and why one 
would want to handle it differently on 32-bit and 64-bit.

> +
>  	unknown_nmi_panic
>  			[X86] Cause panic on unknown NMI.
>  
> diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h
> index f2ad216..89e38a4 100644
> --- a/arch/x86/include/asm/elf.h
> +++ b/arch/x86/include/asm/elf.h
> @@ -4,6 +4,7 @@
>  /*
>   * ELF register definitions..
>   */
> +#include <linux/thread_info.h>
>  
>  #include <asm/ptrace.h>
>  #include <asm/user.h>
> @@ -320,4 +321,39 @@ extern int syscall32_setup_pages(struct linux_binprm *, int exstack);
>  extern unsigned long arch_randomize_brk(struct mm_struct *mm);
>  #define arch_randomize_brk arch_randomize_brk
>  
> +/*
> + * True on X86_32 or when emulating IA32 on X86_64
> + */
> +static inline int mmap_is_ia32(void)
> +{
> +#ifdef CONFIG_X86_32
> +	return 1;
> +#endif
> +#ifdef CONFIG_IA32_EMULATION
> +	if (test_thread_flag(TIF_IA32))
> +		return 1;
> +#endif
> +	return 0;
> +}
> +
> +#define UNALIAS_VA_32 1
> +#define UNALIAS_VA_64 2
> +
> +extern int unalias_va_addr;
> +static inline unsigned long unalias_addr(unsigned long addr, bool incr)
> +{
> +	/* handle both 32- and 64-bit with a single conditional */
> +	if (!(unalias_va_addr & (2 - mmap_is_ia32())))
> +		return addr;
> +
> +	/* check if [14:12] are already cleared */
> +	if (!(addr & (0x7 << PAGE_SHIFT)))
> +		return addr;
> +
> +	addr = addr & ~(0x7 << PAGE_SHIFT);
> +	if (incr)
> +		addr += (0x8 << PAGE_SHIFT);
> +
> +	return addr;
> +}

Looks too big to be inlined.

Also, there's no explanation what the 'incr' logic is about and why 
it adds 32K to the address. Is this round-up logic in disguise?

>  #endif /* _ASM_X86_ELF_H */
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index b13ed39..2d380c6 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -458,6 +458,13 @@ static void __cpuinit early_init_amd(struct cpuinfo_x86 *c)
>  					"with P0 frequency!\n");
>  		}
>  	}
> +
> +	if (unalias_va_addr == -1) {
> +		if (c->x86 == 0x15)
> +			unalias_va_addr = UNALIAS_VA_32 | UNALIAS_VA_64;
> +		else
> +			unalias_va_addr = 0;

the placement here feels a bit wrong - don't we have run-once CPU 
quirks?

> +	}
>  }
>  
>  static void __cpuinit init_amd(struct cpuinfo_x86 *c)
> diff --git a/arch/x86/kernel/sys_x86_64.c b/arch/x86/kernel/sys_x86_64.c
> index ff14a50..323c411 100644
> --- a/arch/x86/kernel/sys_x86_64.c
> +++ b/arch/x86/kernel/sys_x86_64.c
> @@ -18,6 +18,30 @@
>  #include <asm/ia32.h>
>  #include <asm/syscalls.h>
>  
> +/* see Documentation/kernel-parameters.txt */
> +int unalias_va_addr = -1;

used by hot VM code so this really wants to be __read_mostly ...

> +
> +static int __init control_va_addr_unaliasing(char *str)
> +{
> +	if (*str == 0)
> +		return 1;
> +
> +	if (*str == '=')
> +		str++;
> +
> +	if (!strcmp(str, "32"))
> +		unalias_va_addr = UNALIAS_VA_32;
> +	else if (!strcmp(str, "64"))
> +		unalias_va_addr = UNALIAS_VA_64;
> +	else if (!strcmp(str, "off"))
> +		unalias_va_addr = 0;
> +	else
> +		return 0;

there's no 'both'/'all' setting: while it can be achieved by leaving 
out this boot option (i.e. using the default) we generally try to 
allow the setting of all combinations.

> +
> +	return 1;
> +}
> +__setup("unalias_va_addr", control_va_addr_unaliasing);
> +
>  SYSCALL_DEFINE6(mmap, unsigned long, addr, unsigned long, len,
>  		unsigned long, prot, unsigned long, flags,
>  		unsigned long, fd, unsigned long, off)
> @@ -92,6 +116,9 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
>  	start_addr = addr;
>  
>  full_search:
> +	if (filp)
> +		addr = unalias_addr(addr, true);
> +
>  	for (vma = find_vma(mm, addr); ; vma = vma->vm_next) {
>  		/* At this point:  (!vma || addr < vma->vm_end). */
>  		if (end - len < addr) {
> @@ -117,6 +144,9 @@ full_search:
>  			mm->cached_hole_size = vma->vm_start - addr;
>  
>  		addr = vma->vm_end;
> +
> +		if (filp)
> +			addr = unalias_addr(addr, true);

Instead of polluting the code with 'if (filp)' conditions (which is 
really a property specific to this VA placement quirk) please push it 
into the address alignment function so that it looks like this:

	addr = cache_aliasing_quirk(addr, filp);

or so. (see further below about the 'unalias' naming.)

>  	}
>  }
>  
> @@ -161,10 +191,17 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
>  
>  	/* make sure it can fit in the remaining address space */
>  	if (addr > len) {
> -		vma = find_vma(mm, addr-len);
> -		if (!vma || addr <= vma->vm_start)
> +		unsigned long tmp_addr = addr;
> +
> +		if (filp)
> +			tmp_addr = unalias_addr(addr - len, false);
> +		else
> +			tmp_addr = tmp_addr - len;

This too could be written cleaner with the updated helper.

> +
> +		vma = find_vma(mm, tmp_addr);
> +		if (!vma || tmp_addr + len <= vma->vm_start)
>  			/* remember the address as a hint for next time */
> -			return mm->free_area_cache = addr-len;
> +			return mm->free_area_cache = tmp_addr;
>  	}
>  
>  	if (mm->mmap_base < len)
> @@ -173,6 +210,9 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
>  	addr = mm->mmap_base-len;
>  
>  	do {
> +		if (filp)
> +			addr = unalias_addr(addr, false);
> +
>  		/*
>  		 * Lookup failure means no vma is above this address,
>  		 * else if new region fits below vma->vm_start,
> diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
> index 1dab519..d4c0736 100644
> --- a/arch/x86/mm/mmap.c
> +++ b/arch/x86/mm/mmap.c
> @@ -51,21 +51,6 @@ static unsigned int stack_maxrandom_size(void)
>  #define MIN_GAP (128*1024*1024UL + stack_maxrandom_size())
>  #define MAX_GAP (TASK_SIZE/6*5)
>  
> -/*
> - * True on X86_32 or when emulating IA32 on X86_64
> - */
> -static int mmap_is_ia32(void)
> -{
> -#ifdef CONFIG_X86_32
> -	return 1;
> -#endif
> -#ifdef CONFIG_IA32_EMULATION
> -	if (test_thread_flag(TIF_IA32))
> -		return 1;
> -#endif
> -	return 0;
> -}
> -
>  static int mmap_is_legacy(void)
>  {
>  	if (current->personality & ADDR_COMPAT_LAYOUT)
> diff --git a/arch/x86/vdso/vma.c b/arch/x86/vdso/vma.c
> index 7abd2be..a48dfd2 100644
> --- a/arch/x86/vdso/vma.c
> +++ b/arch/x86/vdso/vma.c
> @@ -69,6 +69,16 @@ static unsigned long vdso_addr(unsigned long start, unsigned len)
>  	addr = start + (offset << PAGE_SHIFT);
>  	if (addr >= end)
>  		addr = end;
> +
> +	if (unalias_va_addr) {
> +		/*
> +		 * page-align it here so that get_unmapped_area doesn't
> +		 * align it wrongfully again to the next page
> +		 */
> +		addr = PAGE_ALIGN(addr);
> +		addr = unalias_addr(addr, false);
> +	}

Well, this cuts 3 bits from the randomization granularity. It would 
be better to extend the scope/range of randomization beyond the 
current PMD_SIZE range instead of cutting it.

(Arguably it would also make sense to increase vdso randomization 
beyond a 2MB range as well, but that's beyond this patch.)

also, the PAGE_ALIGN() complication here looks unnecessary - can the 
vdso 'addr' ever be not 4K aligned above?

Thirdly, we stick this vdso_addr() result into a get_unmapped_area() 
call - which does an unalias_addr() call yet another time, right? So 
why the two separate calls?

> +
>  	return addr;
>  }

Also, a naming nit: i find the whole 'unalias' language above rather 
confusing in this context: AFAICS the point is to intentionally 
*align* those mappings. While that technically 'unaliases' them in 
CPU cache coherency lingo, the code you are modifying here is VM code 
where 'unaliasing' can mean a number of other things, none of which 
are related to this problem.

'Aligning' a mapping to a given granularity is a common expression 
though.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-24 11:13 ` Ingo Molnar
@ 2011-07-24 13:40   ` Borislav Petkov
  2011-07-24 13:47     ` Ingo Molnar
                       ` (2 more replies)
  0 siblings, 3 replies; 46+ messages in thread
From: Borislav Petkov @ 2011-07-24 13:40 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Borislav Petkov, Linus Torvalds, Andrew Morton, H. Peter Anvin,
	Thomas Gleixner, LKML, Przywara, Andre, Pohlack, Martin

Hi Ingo,

thanks for taking the time to look at this in detail.

On Sun, Jul 24, 2011 at 07:13:50AM -0400, Ingo Molnar wrote:
> 
> (Cc:-ed Linus and akpm)
> 
> * Borislav Petkov <bp@amd64.org> wrote:
> 
> > From: Borislav Petkov <borislav.petkov@amd.com>
> > 
> > This patch provides performance tuning for the "Bulldozer" CPU. With its
> > shared instruction cache there is a chance of generating an excessive
> > number of cache cross-invalidates when running specific workloads on the
> > cores of a compute module.
> > 
> > This excessive amount of cross-invalidations can be observed if cache
> > lines backed by shared physical memory alias in bits [14:12] of their
> > virtual addresses, as those bits are used for the index generation.
> > 
> > This patch addresses the issue by zeroing out the slice [14:12] of
> > the file mapping's virtual address at generation time, thus forcing
> > those bits the same for all mappings of a single shared library across
> > processes and, in doing so, avoids instruction cache aliases.
> 
> So all commonly-aliased virtual mappings of the same pages should be 
> 32K granular aligned for good performance? Seems rather unfortunate 
> for a modern CPU - is this going to be the case for all family 0x15 
> CPUs?

Right, this gets enabled on all F15h by default for now... The thing
is, the invalidations appear only in certain cases and when the two
tasks run on the cores of one compute module. However, due to the
scheduler migrating them, we need to have those VA assignments aligned
(or unaliased) at process creation.

Another possibility would be to not 32K align but never schedule them
on the same compute module but that won't fly with n tasks and n cores
scenario.

> > It also adds the kernel command line option
> > "unalias_va_addr=(32|64|off)" with which virtual address unaliasing
> > can be enabled for 32-bit or 64-bit x86 individually, or be completely
> > disabled.
> 
> So the default behavior is to have this aligning enabled.

Yes, it gets enabled on F15h by default.

> > This change leaves virtual region address allocation on other families
> > and/or vendors unaffected.
> > 
> > Signed-off-by: Andre Przywara <andre.przywara@amd.com>
> > Signed-off-by: Martin Pohlack <martin.pohlack@amd.com>
> > Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
> > ---
> >  Documentation/kernel-parameters.txt |    6 ++++
> >  arch/x86/include/asm/elf.h          |   36 +++++++++++++++++++++++++++
> >  arch/x86/kernel/cpu/amd.c           |    7 +++++
> >  arch/x86/kernel/sys_x86_64.c        |   46 ++++++++++++++++++++++++++++++++--
> >  arch/x86/mm/mmap.c                  |   15 -----------
> >  arch/x86/vdso/vma.c                 |   10 +++++++
> >  6 files changed, 102 insertions(+), 18 deletions(-)
> > 
> > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> > index fd248a31..17eda04 100644
> > --- a/Documentation/kernel-parameters.txt
> > +++ b/Documentation/kernel-parameters.txt
> > @@ -2535,6 +2535,12 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
> >  			Note that genuine overcurrent events won't be
> >  			reported either.
> >  
> > +	unalias_va_addr
> > +			[X86-64] Unalias VA address by clearing slice [14:12]
> > +			1: only on 32-bit
> > +			2: only on 64-bit
> > +			off: disabled on both 32 and 64-bit
> 
> This says nothing about why it's cleared, what it does and why one 
> would want to handle it differently on 32-bit and 64-bit.

This is there in case users care more about the 3bits ASLR granularity
than invalidations amount, for example. I'll make this more descriptive.

[..]

> > +extern int unalias_va_addr;
> > +static inline unsigned long unalias_addr(unsigned long addr, bool incr)
> > +{
> > +	/* handle both 32- and 64-bit with a single conditional */
> > +	if (!(unalias_va_addr & (2 - mmap_is_ia32())))
> > +		return addr;
> > +
> > +	/* check if [14:12] are already cleared */
> > +	if (!(addr & (0x7 << PAGE_SHIFT)))
> > +		return addr;
> > +
> > +	addr = addr & ~(0x7 << PAGE_SHIFT);
> > +	if (incr)
> > +		addr += (0x8 << PAGE_SHIFT);
> > +
> > +	return addr;
> > +}
> 
> Looks too big to be inlined.

Ok, will change.

> Also, there's no explanation what the 'incr' logic is about and why it
> adds 32K to the address. Is this round-up logic in disguise?

Basically yes. Round-up and -down, actually. The idea
is for the unalias_addr() helper to be called both from
arch_get_unmapped_area_topdown() and arch_get_unmapped_area() depending
on the search direction we take in the VA space. So 'incr' says whether
we increment the last cached VA addr (mm->free_area_cache) or we
decrement. In both cases, it changes addr so that its bits [14:12] are
zeros.

> >  #endif /* _ASM_X86_ELF_H */
> > diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> > index b13ed39..2d380c6 100644
> > --- a/arch/x86/kernel/cpu/amd.c
> > +++ b/arch/x86/kernel/cpu/amd.c
> > @@ -458,6 +458,13 @@ static void __cpuinit early_init_amd(struct cpuinfo_x86 *c)
> >  					"with P0 frequency!\n");
> >  		}
> >  	}
> > +
> > +	if (unalias_va_addr == -1) {
> > +		if (c->x86 == 0x15)
> > +			unalias_va_addr = UNALIAS_VA_32 | UNALIAS_VA_64;
> > +		else
> > +			unalias_va_addr = 0;
> 
> the placement here feels a bit wrong - don't we have run-once CPU 
> quirks?

Maybe another func ptr in struct x86_cpuinit_ops
(arch/x86/kernel/x86_init.c) would be a better place?

> > +	}
> >  }
> >  
> >  static void __cpuinit init_amd(struct cpuinfo_x86 *c)
> > diff --git a/arch/x86/kernel/sys_x86_64.c b/arch/x86/kernel/sys_x86_64.c
> > index ff14a50..323c411 100644
> > --- a/arch/x86/kernel/sys_x86_64.c
> > +++ b/arch/x86/kernel/sys_x86_64.c
> > @@ -18,6 +18,30 @@
> >  #include <asm/ia32.h>
> >  #include <asm/syscalls.h>
> >  
> > +/* see Documentation/kernel-parameters.txt */
> > +int unalias_va_addr = -1;
> 
> used by hot VM code so this really wants to be __read_mostly ...

Ok.

> > +
> > +static int __init control_va_addr_unaliasing(char *str)
> > +{
> > +	if (*str == 0)
> > +		return 1;
> > +
> > +	if (*str == '=')
> > +		str++;
> > +
> > +	if (!strcmp(str, "32"))
> > +		unalias_va_addr = UNALIAS_VA_32;
> > +	else if (!strcmp(str, "64"))
> > +		unalias_va_addr = UNALIAS_VA_64;
> > +	else if (!strcmp(str, "off"))
> > +		unalias_va_addr = 0;
> > +	else
> > +		return 0;
> 
> there's no 'both'/'all' setting: while it can be achieved by leaving 
> out this boot option (i.e. using the default) we generally try to 
> allow the setting of all combinations.

My reasoning here was that it gets enabled on F15h by default but we
don't want it on the remaining families. This is additionally to tweak
the behavior on F15h only but theoretically you could enable it on
everything else. I think I want to restrict the visibility/modifiability
of this option to F15h only.

> > +
> > +	return 1;
> > +}
> > +__setup("unalias_va_addr", control_va_addr_unaliasing);
> > +
> >  SYSCALL_DEFINE6(mmap, unsigned long, addr, unsigned long, len,
> >  		unsigned long, prot, unsigned long, flags,
> >  		unsigned long, fd, unsigned long, off)
> > @@ -92,6 +116,9 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
> >  	start_addr = addr;
> >  
> >  full_search:
> > +	if (filp)
> > +		addr = unalias_addr(addr, true);
> > +
> >  	for (vma = find_vma(mm, addr); ; vma = vma->vm_next) {
> >  		/* At this point:  (!vma || addr < vma->vm_end). */
> >  		if (end - len < addr) {
> > @@ -117,6 +144,9 @@ full_search:
> >  			mm->cached_hole_size = vma->vm_start - addr;
> >  
> >  		addr = vma->vm_end;
> > +
> > +		if (filp)
> > +			addr = unalias_addr(addr, true);
> 
> Instead of polluting the code with 'if (filp)' conditions (which is 
> really a property specific to this VA placement quirk) please push it 
> into the address alignment function so that it looks like this:
> 
> 	addr = cache_aliasing_quirk(addr, filp);
> 
> or so. (see further below about the 'unalias' naming.)

Ok, will do.

I was also thinking of doing something similar to ALTERNATIVE_JUMP
but instead of looking at a CPU feature bit, it can check a generic
conditional evaluated at runtime and patch in an unconditional 'jmp' to
make this codepath even cheaper. But this maybe later.

> >  
> > @@ -161,10 +191,17 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
> >  
> >  	/* make sure it can fit in the remaining address space */
> >  	if (addr > len) {
> > -		vma = find_vma(mm, addr-len);
> > -		if (!vma || addr <= vma->vm_start)
> > +		unsigned long tmp_addr = addr;
> > +
> > +		if (filp)
> > +			tmp_addr = unalias_addr(addr - len, false);
> > +		else
> > +			tmp_addr = tmp_addr - len;
> 
> This too could be written cleaner with the updated helper.

Sure, will do.

[..]

> > diff --git a/arch/x86/vdso/vma.c b/arch/x86/vdso/vma.c
> > index 7abd2be..a48dfd2 100644
> > --- a/arch/x86/vdso/vma.c
> > +++ b/arch/x86/vdso/vma.c
> > @@ -69,6 +69,16 @@ static unsigned long vdso_addr(unsigned long start, unsigned len)
> >  	addr = start + (offset << PAGE_SHIFT);
> >  	if (addr >= end)
> >  		addr = end;
> > +
> > +	if (unalias_va_addr) {
> > +		/*
> > +		 * page-align it here so that get_unmapped_area doesn't
> > +		 * align it wrongfully again to the next page
> > +		 */
> > +		addr = PAGE_ALIGN(addr);
> > +		addr = unalias_addr(addr, false);
> > +	}
> 
> Well, this cuts 3 bits from the randomization granularity. It would 
> be better to extend the scope/range of randomization beyond the 
> current PMD_SIZE range instead of cutting it.
> 
> (Arguably it would also make sense to increase vdso randomization 
> beyond a 2MB range as well, but that's beyond this patch.)

AFAIU, this would cost us 2 pages: a PMD and a PTE if we don't keep the
vDSO in the same PTE as the comment above vdso_addr() puts it:

/* Put the vdso above the (randomized) stack with another randomized offset.
   This way there is no hole in the middle of address space.
   To save memory make sure it is still in the same PTE as the stack top.
   This doesn't give that many random bits */

Hmm..

> also, the PAGE_ALIGN() complication here looks unnecessary - can the 
> vdso 'addr' ever be not 4K aligned above?

Yeah, it can. I did some trace_printk runs and 'addr' wasn't 4K aligned
in some cases. I think this is because of the stack randomization we do
and we use mm->start_stack to get the vdso base address. Unfortunately,
I can't find those traces anymore but will do some again tomorrow.

> Thirdly, we stick this vdso_addr() result into a get_unmapped_area() 
> call - which does an unalias_addr() call yet another time, right? So 
> why the two separate calls?

Right, in order to unalias the address, I check if we have a file
mapping which backs it - if (filp) - and this is not the case for the
vDSO. And if the address is not 4K aligned and we unalias it first and
_then_ align it, resulting in not what we want.

But speaking of file mappings, the better fix should be to pass
protection flags down to get_unmapped_area() from do_mmap_pgoff()
so that we can check against PROT_EXEC and only then change the
address. However, this would require changes all over the place since
->get_unmapped_area is part of fs file_operations, etc, etc.

It still works this way too, anyway, because for example:

$ cat /proc/self/maps
...

7f19429b1000-7f1942b2b000 r-xp 00000000 08:01 4112535                    /lib/x86_64-linux-gnu/libc-2.13.so
7f1942b2b000-7f1942d2b000 ---p 0017a000 08:01 4112535                    /lib/x86_64-linux-gnu/libc-2.13.so
7f1942d2b000-7f1942d2f000 r--p 0017a000 08:01 4112535                    /lib/x86_64-linux-gnu/libc-2.13.so
7f1942d2f000-7f1942d30000 rw-p 0017e000 08:01 4112535                    /lib/x86_64-linux-gnu/libc-2.13.so

when we do the first "r-xp" mapping above, we do

get_unmapped_area(filp != NULL, addr = 0...)

and we change the address accordingly, but the follow-up mappings come
in either with MAP_FIXED set so we return the requested address early
or they're anonymous mappings for .bss and RO data which have no file
backing anyways.

> Also, a naming nit: i find the whole 'unalias' language above rather 
> confusing in this context: AFAICS the point is to intentionally 
> *align* those mappings. While that technically 'unaliases' them in 
> CPU cache coherency lingo, the code you are modifying here is VM code 
> where 'unaliasing' can mean a number of other things, none of which 
> are related to this problem.
> 
> 'Aligning' a mapping to a given granularity is a common expression 
> though.

Yes, makes sense, will change.

Thanks again for the review.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-24 13:40   ` Borislav Petkov
@ 2011-07-24 13:47     ` Ingo Molnar
  2011-07-24 16:16     ` Andrew Morton
  2011-07-26 18:33     ` Borislav Petkov
  2 siblings, 0 replies; 46+ messages in thread
From: Ingo Molnar @ 2011-07-24 13:47 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Linus Torvalds, Andrew Morton, H. Peter Anvin, Thomas Gleixner,
	LKML, Przywara, Andre, Pohlack, Martin


* Borislav Petkov <bp@amd64.org> wrote:

> > > diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> > > index b13ed39..2d380c6 100644
> > > --- a/arch/x86/kernel/cpu/amd.c
> > > +++ b/arch/x86/kernel/cpu/amd.c
> > > @@ -458,6 +458,13 @@ static void __cpuinit early_init_amd(struct cpuinfo_x86 *c)
> > >  					"with P0 frequency!\n");
> > >  		}
> > >  	}
> > > +
> > > +	if (unalias_va_addr == -1) {
> > > +		if (c->x86 == 0x15)
> > > +			unalias_va_addr = UNALIAS_VA_32 | UNALIAS_VA_64;
> > > +		else
> > > +			unalias_va_addr = 0;
> > 
> > the placement here feels a bit wrong - don't we have run-once CPU 
> > quirks?
> 
> Maybe another func ptr in struct x86_cpuinit_ops
> (arch/x86/kernel/x86_init.c) would be a better place?

Yeah.

Assuming no objections come i'd suggest to do that as an add-on patch 
- so the first patch can be backported, the second patch cleans up 
the x86_cpuinit_ops aspect.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-22 13:15 [PATCH] x86, AMD: Correct F15h IC aliasing issue Borislav Petkov
  2011-07-24 11:13 ` Ingo Molnar
@ 2011-07-24 16:04 ` Linus Torvalds
  2011-07-24 17:22   ` Borislav Petkov
  2011-07-26 17:59 ` Avi Kivity
  2 siblings, 1 reply; 46+ messages in thread
From: Linus Torvalds @ 2011-07-24 16:04 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: H. Peter Anvin, Ingo Molnar, Thomas Gleixner, LKML,
	Borislav Petkov, Andre Przywara, Martin Pohlack

Argh. This is a small disaster, you know that, right? Suddenly we have
user-visible allocation changes depending on which CPU you are running
on. I just hope that the address-space randomization has caught all
the code that depended on specific layouts.

And even with ASLR, I wouldn't be surprised if there are binaries out
there that "know" that they get dense virtual memory when they do
back-to-back allocations, even when they don't pass in the address
explicitly.

How much testing has AMD done with this change and various legacy
Linux distros? The 32-bit case in particular makes me nervous, that's
where I'd expect a higher likelihood of binaries that depend on the
layout.

You guys do realize that we had to disable ASLR on many machines?

So at a MINIMUM, I would say that this is acceptable only when the
process doing the allocation hasn't got ASLR disabled.

On Fri, Jul 22, 2011 at 6:15 AM, Borislav Petkov <bp@amd64.org> wrote:
> +static inline unsigned long unalias_addr(unsigned long addr, bool incr)
> +{
> +       /* handle both 32- and 64-bit with a single conditional */
> +       if (!(unalias_va_addr & (2 - mmap_is_ia32())))
> +               return addr;

Ugh. I guess it works, but the actual values you used did not have a
comment about those particular values being magical. You should do
that, otherwise somebody will start adding bits and moving things
around, and breaking this "bits 2/1" logic.

> +       /* check if [14:12] are already cleared */
> +       if (!(addr & (0x7 << PAGE_SHIFT)))
> +               return addr;
> +
> +       addr = addr & ~(0x7 << PAGE_SHIFT);
> +       if (incr)
> +               addr += (0x8 << PAGE_SHIFT);

This is just really hard to look at. First you talk about "bits
14:12", and then you use odd values like "8 << PAGE_SHIFT".

Yes, yes, I can do the math in my head, and say "8 is 1<<3, and
PAGE_SHIFT is 12, so he's adding things up to the next bit 15".

But is that really sensible?

If we don't already have helpers for this, it would still be prettier
with something like

  #define BIT(a) (1ul << (a))
  #define BITS(a,b) (BIT((a)+1) - BIT(b))

and then that "0x7 << PAGE_SHIFT" ends up being BITS(14,12) like in
the comment (you should really double-check that I got it right
though).

Or alternatively, make the comment match the code, and explain the
14:12 with something like "the three bits above the page mask",
although that just sounds odd.

Anyway, I seriously think that this patch is completely unacceptable
in this form, and is quite possibly going to break real applications.
Maybe most of the applications that had problems with ASLR only had
trouble with anonymous memory, and the fact that you only do this for
file mappings might mean that it's ok. But I'd be really worried.
Changing address space layout is not a small decision.

                         Linus

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-24 13:40   ` Borislav Petkov
  2011-07-24 13:47     ` Ingo Molnar
@ 2011-07-24 16:16     ` Andrew Morton
  2011-07-26 18:33     ` Borislav Petkov
  2 siblings, 0 replies; 46+ messages in thread
From: Andrew Morton @ 2011-07-24 16:16 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Ingo Molnar, Linus Torvalds, H. Peter Anvin, Thomas Gleixner,
	LKML, Przywara, Andre, Pohlack, Martin

On Sun, 24 Jul 2011 15:40:46 +0200 Borislav Petkov <bp@amd64.org> wrote:

> > > +	unalias_va_addr
> > > +			[X86-64] Unalias VA address by clearing slice [14:12]
> > > +			1: only on 32-bit
> > > +			2: only on 64-bit
> > > +			off: disabled on both 32 and 64-bit

How much performance difference does this actually make?

> > This says nothing about why it's cleared, what it does and why one 
> > would want to handle it differently on 32-bit and 64-bit.
> 
> This is there in case users care more about the 3bits ASLR granularity
> than invalidations amount, for example.

To make this decision the users will want to know how much their
computers will slow down.  We should tell them.

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-24 16:04 ` Linus Torvalds
@ 2011-07-24 17:22   ` Borislav Petkov
  2011-07-24 17:39     ` Linus Torvalds
  0 siblings, 1 reply; 46+ messages in thread
From: Borislav Petkov @ 2011-07-24 17:22 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Borislav Petkov, H. Peter Anvin, Ingo Molnar, Thomas Gleixner,
	LKML, Przywara, Andre, Pohlack, Martin

Hi Linus,

On Sun, Jul 24, 2011 at 12:04:27PM -0400, Linus Torvalds wrote:
> Argh. This is a small disaster, you know that, right? Suddenly we have
> user-visible allocation changes depending on which CPU you are running
> on. I just hope that the address-space randomization has caught all
> the code that depended on specific layouts.
> 
> And even with ASLR, I wouldn't be surprised if there are binaries out
> there that "know" that they get dense virtual memory when they do
> back-to-back allocations, even when they don't pass in the address
> explicitly.

That's what I'm afraid of. Thus the boot option to disable it, I'm
afraid the patch won't be of help in such situations.

> How much testing has AMD done with this change and various legacy
> Linux distros?

Currently underway. Just to make sure: this change currently addresses
only the case with 32-bit binaries running on a 64-bit kernel and not
the 32-bit kernel case.

> The 32-bit case in particular makes me nervous, that's where I'd
> expect a higher likelihood of binaries that depend on the layout.

Ok, noted.

> You guys do realize that we had to disable ASLR on many machines?

Disabling ASLR is actually ok because it also fixes the aliasing issue
in 99% of the cases by keeping the VA slice [14:12] the same across
processes.

Btw, one of the joke-fixes floating around here was to disable ASLR
completely as on Transmeta:

<arch/x86/kernel/cpu/transmeta.c:92>
#ifdef CONFIG_SYSCTL
	/*
	 * randomize_va_space slows us down enormously;
	 * it probably triggers retranslation of x86->native bytecode
	 */
	randomize_va_space = 0;
#endif

> So at a MINIMUM, I would say that this is acceptable only when the
> process doing the allocation hasn't got ASLR disabled.

I guess I could look at randomize_va_space before enabling it.

But this won't address the case where one of the processes was created
with ASLR off and the other with ASLR on and they map the same library
at VAs differing at bits [14:12].

> On Fri, Jul 22, 2011 at 6:15 AM, Borislav Petkov <bp@amd64.org> wrote:
> > +static inline unsigned long unalias_addr(unsigned long addr, bool incr)
> > +{
> > +       /* handle both 32- and 64-bit with a single conditional */
> > +       if (!(unalias_va_addr & (2 - mmap_is_ia32())))
> > +               return addr;
> 
> Ugh. I guess it works, but the actual values you used did not have a
> comment about those particular values being magical. You should do
> that, otherwise somebody will start adding bits and moving things
> around, and breaking this "bits 2/1" logic.

Good point, will do.

> > +       /* check if [14:12] are already cleared */
> > +       if (!(addr & (0x7 << PAGE_SHIFT)))
> > +               return addr;
> > +
> > +       addr = addr & ~(0x7 << PAGE_SHIFT);
> > +       if (incr)
> > +               addr += (0x8 << PAGE_SHIFT);
> 
> This is just really hard to look at. First you talk about "bits
> 14:12", and then you use odd values like "8 << PAGE_SHIFT".
> 
> Yes, yes, I can do the math in my head, and say "8 is 1<<3, and
> PAGE_SHIFT is 12, so he's adding things up to the next bit 15".
> 
> But is that really sensible?
> 
> If we don't already have helpers for this, it would still be prettier
> with something like
> 
>   #define BIT(a) (1ul << (a))
>   #define BITS(a,b) (BIT((a)+1) - BIT(b))
> 
> and then that "0x7 << PAGE_SHIFT" ends up being BITS(14,12) like in
> the comment (you should really double-check that I got it right
> though).

Yeah, I like the BITS() thing - will change. I actually have a similar
macro GENMASK(o, hi) in <drivers/edac/amd64_edac.h> - I should move it
to <linux/bitops.h> and rename it to BITS().

> Or alternatively, make the comment match the code, and explain the
> 14:12 with something like "the three bits above the page mask",
> although that just sounds odd.
> 
> Anyway, I seriously think that this patch is completely unacceptable
> in this form, and is quite possibly going to break real applications.
> Maybe most of the applications that had problems with ASLR only had
> trouble with anonymous memory, and the fact that you only do this for
> file mappings might mean that it's ok. But I'd be really worried.
> Changing address space layout is not a small decision.

I suspected as much - thus the boot option to disable it. We'll do our
extensive testing as good as we can and we'll keep our fingers crossed.

Big thanks for your review.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-24 17:22   ` Borislav Petkov
@ 2011-07-24 17:39     ` Linus Torvalds
  2011-07-24 18:12       ` Ingo Molnar
                         ` (2 more replies)
  0 siblings, 3 replies; 46+ messages in thread
From: Linus Torvalds @ 2011-07-24 17:39 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: H. Peter Anvin, Ingo Molnar, Thomas Gleixner, LKML, Przywara,
	Andre, Pohlack, Martin

On Sun, Jul 24, 2011 at 10:22 AM, Borislav Petkov <bp@amd64.org> wrote:
>
>> So at a MINIMUM, I would say that this is acceptable only when the
>> process doing the allocation hasn't got ASLR disabled.
>
> I guess I could look at randomize_va_space before enabling it.

That's not what I meant - I meant the per-process PF_RANDOMIZE and
ADDR_NO_RANDOMIZE personality flags (although the global
"randomize_va_space" thing obviously is one input to that too)

In fact, if 99% of your problem is ASLR-induced, might I suggest just
making the whole thing a tweak to ASLR instead, and not use ASLR for
bits 14:12? That should be fundamentally much safer: it doesn't change
any semantics at all, it just makes for slightly less random bits to
be used.

So I really think that you might be *much* better off just changing
mmap_rnd(), and nothing else. Just make *that* mask off the three low
bits of the random address, ie something like

  diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
  index 1dab5194fd9d..6b62ab5a5ae1 100644
  --- a/arch/x86/mm/mmap.c
  +++ b/arch/x86/mm/mmap.c
  @@ -90,6 +90,9 @@ static unsigned long mmap_rnd(void)
                          rnd = (long)get_random_int() % (1<<8);
                  else
                          rnd = (long)(get_random_int() % (1<<28));
  +
  +               if (avoid_aliasing_in_bits_14_12)
  +                       rnd &= ~7;
          }
          return rnd << PAGE_SHIFT;
   }

would be fundamentally very safe - it would already take all our
current anti-randomization code into account.

No?

> But this won't address the case where one of the processes was created
> with ASLR off and the other with ASLR on and they map the same library
> at VAs differing at bits [14:12].

I wouldn't worry about some corner-case like that _nearly_ as much as
worrying about the non-ASLR process working at all.

> Yeah, I like the BITS() thing - will change. I actually have a similar
> macro GENMASK(o, hi) in <drivers/edac/amd64_edac.h> - I should move it
> to <linux/bitops.h> and rename it to BITS().

So it may be that BITS() is much too generic a name, and will cause
problems. A quick "git grep -w BITS" certainly finds a fair number of
hits. So I don't think it's usable as-is, it was meant more as
pseudo-code.

>> Changing address space layout is not a small decision.
>
> I suspected as much - thus the boot option to disable it.

I understand that the boot option is worth it, but since we _already_
have a way to mark binaries as not wanting address space layout
changes, I really think it should use that as the primary method. When
that bit is set, I think it's a big hint that the process is "fragile"
wrt address space changes.

A boot option might be left as a last ditch thing, but I don't think
it should be the primary model.

                              Linus

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-24 17:39     ` Linus Torvalds
@ 2011-07-24 18:12       ` Ingo Molnar
  2011-07-24 18:23       ` Borislav Petkov
  2011-07-27 17:10       ` Borislav Petkov
  2 siblings, 0 replies; 46+ messages in thread
From: Ingo Molnar @ 2011-07-24 18:12 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Borislav Petkov, H. Peter Anvin, Thomas Gleixner, LKML, Przywara,
	Andre, Pohlack, Martin


* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Sun, Jul 24, 2011 at 10:22 AM, Borislav Petkov <bp@amd64.org> wrote:
> >
> >> So at a MINIMUM, I would say that this is acceptable only when the
> >> process doing the allocation hasn't got ASLR disabled.
> >
> > I guess I could look at randomize_va_space before enabling it.
> 
> That's not what I meant - I meant the per-process PF_RANDOMIZE and 
> ADDR_NO_RANDOMIZE personality flags (although the global 
> "randomize_va_space" thing obviously is one input to that too)
> 
> In fact, if 99% of your problem is ASLR-induced, might I suggest 
> just making the whole thing a tweak to ASLR instead, and not use 
> ASLR for bits 14:12? That should be fundamentally much safer: it 
> doesn't change any semantics at all, it just makes for slightly 
> less random bits to be used.

Indeed - that would be much nicer and smaller as well. It could also 
go away easily if this bug is fixed in a future CPU.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-24 17:39     ` Linus Torvalds
  2011-07-24 18:12       ` Ingo Molnar
@ 2011-07-24 18:23       ` Borislav Petkov
  2011-07-24 18:30         ` Ingo Molnar
  2011-07-27 17:10       ` Borislav Petkov
  2 siblings, 1 reply; 46+ messages in thread
From: Borislav Petkov @ 2011-07-24 18:23 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Borislav Petkov, H. Peter Anvin, Ingo Molnar, Thomas Gleixner,
	LKML, Przywara, Andre, Pohlack, Martin

On Sun, Jul 24, 2011 at 01:39:25PM -0400, Linus Torvalds wrote:
> On Sun, Jul 24, 2011 at 10:22 AM, Borislav Petkov <bp@amd64.org> wrote:
> >
> >> So at a MINIMUM, I would say that this is acceptable only when the
> >> process doing the allocation hasn't got ASLR disabled.
> >
> > I guess I could look at randomize_va_space before enabling it.
> 
> That's not what I meant - I meant the per-process PF_RANDOMIZE and
> ADDR_NO_RANDOMIZE personality flags (although the global
> "randomize_va_space" thing obviously is one input to that too)
> 
> In fact, if 99% of your problem is ASLR-induced, might I suggest just
> making the whole thing a tweak to ASLR instead, and not use ASLR for
> bits 14:12? That should be fundamentally much safer: it doesn't change
> any semantics at all, it just makes for slightly less random bits to
> be used.
> 
> So I really think that you might be *much* better off just changing
> mmap_rnd(), and nothing else. Just make *that* mask off the three low
> bits of the random address, ie something like
> 
>   diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
>   index 1dab5194fd9d..6b62ab5a5ae1 100644
>   --- a/arch/x86/mm/mmap.c
>   +++ b/arch/x86/mm/mmap.c
>   @@ -90,6 +90,9 @@ static unsigned long mmap_rnd(void)
>                           rnd = (long)get_random_int() % (1<<8);
>                   else
>                           rnd = (long)(get_random_int() % (1<<28));
>   +
>   +               if (avoid_aliasing_in_bits_14_12)
>   +                       rnd &= ~7;
>           }
>           return rnd << PAGE_SHIFT;
>    }
> 
> would be fundamentally very safe - it would already take all our
> current anti-randomization code into account.
> 
> No?

Hehe, we had that idea initially. However, the special 1% case I was
hinting at is this:

process P0, mapping libraries A, B, C

and

process P1, mapping libraries A, C

Library C ends up possibly with aliasing VAs and there's the problem
again. Also, can we guarantee that the link order is always the same?
IOW, can other two processes P2 and P3, both mapping libraries A,B,C end
up with the following mapping order:

P2: A,B,C
P3: A,C,B

If yes, then no joy.

I'll obviously have to leave in the unaliasing of the vDSO randomization
but yeah, this solution would've be great.

> > But this won't address the case where one of the processes was created
> > with ASLR off and the other with ASLR on and they map the same library
> > at VAs differing at bits [14:12].
> 
> I wouldn't worry about some corner-case like that _nearly_ as much as
> worrying about the non-ASLR process working at all.

Ok, I'll make the 32k aligning contingent on the PF_RANDOMIZE setting.

> > Yeah, I like the BITS() thing - will change. I actually have a similar
> > macro GENMASK(o, hi) in <drivers/edac/amd64_edac.h> - I should move it
> > to <linux/bitops.h> and rename it to BITS().
> 
> So it may be that BITS() is much too generic a name, and will cause
> problems. A quick "git grep -w BITS" certainly finds a fair number of
> hits. So I don't think it's usable as-is, it was meant more as
> pseudo-code.

Ok.

> >> Changing address space layout is not a small decision.
> >
> > I suspected as much - thus the boot option to disable it.
> 
> I understand that the boot option is worth it, but since we _already_
> have a way to mark binaries as not wanting address space layout
> changes, I really think it should use that as the primary method. When
> that bit is set, I think it's a big hint that the process is "fragile"
> wrt address space changes.
> 
> A boot option might be left as a last ditch thing, but I don't think
> it should be the primary model.

Ok, understood. Will change accordingly.

Thanks.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-24 18:23       ` Borislav Petkov
@ 2011-07-24 18:30         ` Ingo Molnar
  2011-07-24 19:07           ` Borislav Petkov
  0 siblings, 1 reply; 46+ messages in thread
From: Ingo Molnar @ 2011-07-24 18:30 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Linus Torvalds, H. Peter Anvin, Thomas Gleixner, LKML, Przywara,
	Andre, Pohlack, Martin


* Borislav Petkov <bp@amd64.org> wrote:

> On Sun, Jul 24, 2011 at 01:39:25PM -0400, Linus Torvalds wrote:
> > On Sun, Jul 24, 2011 at 10:22 AM, Borislav Petkov <bp@amd64.org> wrote:
> > >
> > >> So at a MINIMUM, I would say that this is acceptable only when the
> > >> process doing the allocation hasn't got ASLR disabled.
> > >
> > > I guess I could look at randomize_va_space before enabling it.
> > 
> > That's not what I meant - I meant the per-process PF_RANDOMIZE and
> > ADDR_NO_RANDOMIZE personality flags (although the global
> > "randomize_va_space" thing obviously is one input to that too)
> > 
> > In fact, if 99% of your problem is ASLR-induced, might I suggest just
> > making the whole thing a tweak to ASLR instead, and not use ASLR for
> > bits 14:12? That should be fundamentally much safer: it doesn't change
> > any semantics at all, it just makes for slightly less random bits to
> > be used.
> > 
> > So I really think that you might be *much* better off just changing
> > mmap_rnd(), and nothing else. Just make *that* mask off the three low
> > bits of the random address, ie something like
> > 
> >   diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
> >   index 1dab5194fd9d..6b62ab5a5ae1 100644
> >   --- a/arch/x86/mm/mmap.c
> >   +++ b/arch/x86/mm/mmap.c
> >   @@ -90,6 +90,9 @@ static unsigned long mmap_rnd(void)
> >                           rnd = (long)get_random_int() % (1<<8);
> >                   else
> >                           rnd = (long)(get_random_int() % (1<<28));
> >   +
> >   +               if (avoid_aliasing_in_bits_14_12)
> >   +                       rnd &= ~7;
> >           }
> >           return rnd << PAGE_SHIFT;
> >    }
> > 
> > would be fundamentally very safe - it would already take all our
> > current anti-randomization code into account.
> > 
> > No?
> 
> Hehe, we had that idea initially. However, the special 1% case I was
> hinting at is this:
> 
> process P0, mapping libraries A, B, C
> 
> and
> 
> process P1, mapping libraries A, C
> 
> Library C ends up possibly with aliasing VAs and there's the 
> problem again. [...]

Well, since all library positions are randomized, and the quirk masks 
out bits 12,13,14, all libraries that are not explicitly fix-mapped 
will end up on a 32K granular VA address.

So i don't think this is an issue.

Also, in practice on most distros most libraries will be prelinked to 
the same address in all processes.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-24 18:30         ` Ingo Molnar
@ 2011-07-24 19:07           ` Borislav Petkov
  2011-07-24 20:44             ` Ingo Molnar
  0 siblings, 1 reply; 46+ messages in thread
From: Borislav Petkov @ 2011-07-24 19:07 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Borislav Petkov, Linus Torvalds, H. Peter Anvin, Thomas Gleixner,
	LKML, Przywara, Andre, Pohlack, Martin

On Sun, Jul 24, 2011 at 02:30:46PM -0400, Ingo Molnar wrote:
> > > So I really think that you might be *much* better off just changing
> > > mmap_rnd(), and nothing else. Just make *that* mask off the three low
> > > bits of the random address, ie something like
> > > 
> > >   diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
> > >   index 1dab5194fd9d..6b62ab5a5ae1 100644
> > >   --- a/arch/x86/mm/mmap.c
> > >   +++ b/arch/x86/mm/mmap.c
> > >   @@ -90,6 +90,9 @@ static unsigned long mmap_rnd(void)
> > >                           rnd = (long)get_random_int() % (1<<8);
> > >                   else
> > >                           rnd = (long)(get_random_int() % (1<<28));
> > >   +
> > >   +               if (avoid_aliasing_in_bits_14_12)
> > >   +                       rnd &= ~7;
> > >           }
> > >           return rnd << PAGE_SHIFT;
> > >    }
> > > 
> > > would be fundamentally very safe - it would already take all our
> > > current anti-randomization code into account.
> > > 
> > > No?
> > 
> > Hehe, we had that idea initially. However, the special 1% case I was
> > hinting at is this:
> > 
> > process P0, mapping libraries A, B, C
> > 
> > and
> > 
> > process P1, mapping libraries A, C
> > 
> > Library C ends up possibly with aliasing VAs and there's the 
> > problem again. [...]
> 
> Well, since all library positions are randomized, and the quirk masks 
> out bits 12,13,14, all libraries that are not explicitly fix-mapped 
> will end up on a 32K granular VA address.

Right, but IIUC, mmap_rnd() is used to determine mm->mmap_base so the
mmap starting address will have [14:12] cleared but the initial address
of library C's mapping in the example above will possibly differ in
those bits due to different linking order, right?

> Also, in practice on most distros most libraries will be prelinked to
> the same address in all processes.

I think at least on RHEL there's a daemon doing prelinking every two
weeks or so...

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-24 19:07           ` Borislav Petkov
@ 2011-07-24 20:44             ` Ingo Molnar
  2011-07-25 20:00               ` Borislav Petkov
  0 siblings, 1 reply; 46+ messages in thread
From: Ingo Molnar @ 2011-07-24 20:44 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Linus Torvalds, H. Peter Anvin, Thomas Gleixner, LKML, Przywara,
	Andre, Pohlack, Martin


* Borislav Petkov <bp@amd64.org> wrote:

> On Sun, Jul 24, 2011 at 02:30:46PM -0400, Ingo Molnar wrote:
> > > > So I really think that you might be *much* better off just changing
> > > > mmap_rnd(), and nothing else. Just make *that* mask off the three low
> > > > bits of the random address, ie something like
> > > > 
> > > >   diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
> > > >   index 1dab5194fd9d..6b62ab5a5ae1 100644
> > > >   --- a/arch/x86/mm/mmap.c
> > > >   +++ b/arch/x86/mm/mmap.c
> > > >   @@ -90,6 +90,9 @@ static unsigned long mmap_rnd(void)
> > > >                           rnd = (long)get_random_int() % (1<<8);
> > > >                   else
> > > >                           rnd = (long)(get_random_int() % (1<<28));
> > > >   +
> > > >   +               if (avoid_aliasing_in_bits_14_12)
> > > >   +                       rnd &= ~7;
> > > >           }
> > > >           return rnd << PAGE_SHIFT;
> > > >    }
> > > > 
> > > > would be fundamentally very safe - it would already take all our
> > > > current anti-randomization code into account.
> > > > 
> > > > No?
> > > 
> > > Hehe, we had that idea initially. However, the special 1% case I was
> > > hinting at is this:
> > > 
> > > process P0, mapping libraries A, B, C
> > > 
> > > and
> > > 
> > > process P1, mapping libraries A, C
> > > 
> > > Library C ends up possibly with aliasing VAs and there's the 
> > > problem again. [...]
> > 
> > Well, since all library positions are randomized, and the quirk masks 
> > out bits 12,13,14, all libraries that are not explicitly fix-mapped 
> > will end up on a 32K granular VA address.
> 
> Right, but IIUC, mmap_rnd() is used to determine mm->mmap_base so 
> the mmap starting address will have [14:12] cleared but the initial 
> address of library C's mapping in the example above will possibly 
> differ in those bits due to different linking order, right?

Indeed, because only the mmap base is randomized, not the individual 
vmas.

I'd still suggest to ignore this link order case first, and get the 
99% fix in place ... that one is also obviously backportable - the 
big patch not so much.

Then send in a patch on top of that that solves the remaining 1% also 
with numbers attached, so that we can see the speed/complexity 
tradeoff ...

> > Also, in practice on most distros most libraries will be 
> > prelinked to the same address in all processes.
> 
> I think at least on RHEL there's a daemon doing prelinking every 
> two weeks or so...

I'd suggest to run a script that extracts the actual mapped offsets 
from/proc/*/maps files and measures how much of an issue this all is 
in practice.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-24 20:44             ` Ingo Molnar
@ 2011-07-25 20:00               ` Borislav Petkov
  2011-07-25 20:06                 ` Ingo Molnar
  0 siblings, 1 reply; 46+ messages in thread
From: Borislav Petkov @ 2011-07-25 20:00 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Borislav Petkov, Linus Torvalds, H. Peter Anvin, Thomas Gleixner,
	LKML, Przywara, Andre, Pohlack, Martin

On Sun, Jul 24, 2011 at 04:44:50PM -0400, Ingo Molnar wrote:
> 
> * Borislav Petkov <bp@amd64.org> wrote:
> 
> > On Sun, Jul 24, 2011 at 02:30:46PM -0400, Ingo Molnar wrote:
> > > > > So I really think that you might be *much* better off just changing
> > > > > mmap_rnd(), and nothing else. Just make *that* mask off the three low
> > > > > bits of the random address, ie something like
> > > > > 
> > > > >   diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
> > > > >   index 1dab5194fd9d..6b62ab5a5ae1 100644
> > > > >   --- a/arch/x86/mm/mmap.c
> > > > >   +++ b/arch/x86/mm/mmap.c
> > > > >   @@ -90,6 +90,9 @@ static unsigned long mmap_rnd(void)
> > > > >                           rnd = (long)get_random_int() % (1<<8);
> > > > >                   else
> > > > >                           rnd = (long)(get_random_int() % (1<<28));
> > > > >   +
> > > > >   +               if (avoid_aliasing_in_bits_14_12)
> > > > >   +                       rnd &= ~7;
> > > > >           }
> > > > >           return rnd << PAGE_SHIFT;
> > > > >    }
> > > > > 
> > > > > would be fundamentally very safe - it would already take all our
> > > > > current anti-randomization code into account.
> > > > > 
> > > > > No?
> > > > 
> > > > Hehe, we had that idea initially. However, the special 1% case I was
> > > > hinting at is this:
> > > > 
> > > > process P0, mapping libraries A, B, C
> > > > 
> > > > and
> > > > 
> > > > process P1, mapping libraries A, C
> > > > 
> > > > Library C ends up possibly with aliasing VAs and there's the 
> > > > problem again. [...]
> > > 
> > > Well, since all library positions are randomized, and the quirk masks 
> > > out bits 12,13,14, all libraries that are not explicitly fix-mapped 
> > > will end up on a 32K granular VA address.
> > 
> > Right, but IIUC, mmap_rnd() is used to determine mm->mmap_base so 
> > the mmap starting address will have [14:12] cleared but the initial 
> > address of library C's mapping in the example above will possibly 
> > differ in those bits due to different linking order, right?
> 
> Indeed, because only the mmap base is randomized, not the individual 
> vmas.
> 
> I'd still suggest to ignore this link order case first, and get the 
> 99% fix in place ... that one is also obviously backportable - the 
> big patch not so much.

Hmm, I've done some experiments with Linus' suggestion and I don't think
that flies. Here's why:

With the attached patch, the kernel generates VA spaces for processes
with mmap_base's bits [14:12] cleared. Here's what it looks like:

$ cat /proc/self/maps
00400000-0040c000 r-xp 00000000 08:02 1474641                            /bin/cat
0060b000-0060c000 r--p 0000b000 08:02 1474641                            /bin/cat
0060c000-0060d000 rw-p 0000c000 08:02 1474641                            /bin/cat
0118f000-011b0000 rw-p 00000000 00:00 0                                  [heap]
7f184c7a2000-7f184c9cd000 r--p 00000000 08:02 622671                     /usr/lib64/locale/locale-archive
7f184c9cd000-7f184cb26000 r-xp 00000000 08:02 2113794                    /lib64/libc-2.12.2.so
7f184cb26000-7f184cd26000 ---p 00159000 08:02 2113794                    /lib64/libc-2.12.2.so
7f184cd26000-7f184cd2a000 r--p 00159000 08:02 2113794                    /lib64/libc-2.12.2.so
7f184cd2a000-7f184cd2b000 rw-p 0015d000 08:02 2113794                    /lib64/libc-2.12.2.so
7f184cd2b000-7f184cd30000 rw-p 00000000 00:00 0
7f184cd30000-7f184cd4e000 r-xp 00000000 08:02 2113792                    /lib64/ld-2.12.2.so
7f184cf30000-7f184cf33000 rw-p 00000000 00:00 0
7f184cf4c000-7f184cf4d000 rw-p 00000000 00:00 0
7f184cf4d000-7f184cf4e000 r--p 0001d000 08:02 2113792                    /lib64/ld-2.12.2.so
7f184cf4e000-7f184cf4f000 rw-p 0001e000 08:02 2113792                    /lib64/ld-2.12.2.so
7f184cf4f000-7f184cf50000 rw-p 00000000 00:00 0 							<---- A
7fffcfb8c000-7fffcfbad000 rw-p 00000000 00:00 0                          [stack]
7fffcfbff000-7fffcfc00000 r-xp 00000000 00:00 0                          [vdso]
ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0                  [vsyscall]

The end region address at marker A is that changed address
0x7f184cf50000. We decrement from there because we do topdown
allocation. Take a look at glibc's executable mapping:

7f184c9cd000-7f184cb26000 r-xp 00000000 08:02 2113794                    /lib64/libc-2.12.2.so

slice [14:12] = 101b

Now let's take a look at bash's maps (below). The changed mmap_base is
at marker B below. glibc exec mapping is:

7f63dc061000-7f63dc1ba000 r-xp 00000000 08:02 2113794                    /lib64/libc-2.12.2.so

whereas slice [14:12] = 001b

Not good, aliasing.

This is actually a good example for the case where process P0 (bash)
maps libraries A,B,...C and P1 maps libraries A, C.

So yeah, the simpler fix works for processes mapping libraries in the
same order but not for processes mapping a subset of libraries in a
different order.

$ cat /proc/2473/maps
00400000-004d0000 r-xp 00000000 08:02 2114802                            /bin/bash
006cf000-006d0000 r--p 000cf000 08:02 2114802                            /bin/bash
006d0000-006d9000 rw-p 000d0000 08:02 2114802                            /bin/bash
006d9000-006df000 rw-p 00000000 00:00 0
02030000-02072000 rw-p 00000000 00:00 0                                  [heap]
7f63db3e3000-7f63db3e5000 r-xp 00000000 08:02 2113704                    /usr/lib64/gconv/ISO8859-1.so
7f63db3e5000-7f63db5e4000 ---p 00002000 08:02 2113704                    /usr/lib64/gconv/ISO8859-1.so
7f63db5e4000-7f63db5e5000 r--p 00001000 08:02 2113704                    /usr/lib64/gconv/ISO8859-1.so
7f63db5e5000-7f63db5e6000 rw-p 00002000 08:02 2113704                    /usr/lib64/gconv/ISO8859-1.so
7f63db5e6000-7f63db5f1000 r-xp 00000000 08:02 2113540                    /lib64/libnss_files-2.12.2.so
7f63db5f1000-7f63db7f0000 ---p 0000b000 08:02 2113540                    /lib64/libnss_files-2.12.2.so
7f63db7f0000-7f63db7f1000 r--p 0000a000 08:02 2113540                    /lib64/libnss_files-2.12.2.so
7f63db7f1000-7f63db7f2000 rw-p 0000b000 08:02 2113540                    /lib64/libnss_files-2.12.2.so
7f63db7f2000-7f63db7fc000 r-xp 00000000 08:02 2113793                    /lib64/libnss_nis-2.12.2.so
7f63db7fc000-7f63db9fb000 ---p 0000a000 08:02 2113793                    /lib64/libnss_nis-2.12.2.so
7f63db9fb000-7f63db9fc000 r--p 00009000 08:02 2113793                    /lib64/libnss_nis-2.12.2.so
7f63db9fc000-7f63db9fd000 rw-p 0000a000 08:02 2113793                    /lib64/libnss_nis-2.12.2.so
7f63db9fd000-7f63dba12000 r-xp 00000000 08:02 2113807                    /lib64/libnsl-2.12.2.so
7f63dba12000-7f63dbc11000 ---p 00015000 08:02 2113807                    /lib64/libnsl-2.12.2.so
7f63dbc11000-7f63dbc12000 r--p 00014000 08:02 2113807                    /lib64/libnsl-2.12.2.so
7f63dbc12000-7f63dbc13000 rw-p 00015000 08:02 2113807                    /lib64/libnsl-2.12.2.so
7f63dbc13000-7f63dbc15000 rw-p 00000000 00:00 0
7f63dbc15000-7f63dbc1c000 r-xp 00000000 08:02 2113801                    /lib64/libnss_compat-2.12.2.so
7f63dbc1c000-7f63dbe1b000 ---p 00007000 08:02 2113801                    /lib64/libnss_compat-2.12.2.so
7f63dbe1b000-7f63dbe1c000 r--p 00006000 08:02 2113801                    /lib64/libnss_compat-2.12.2.so
7f63dbe1c000-7f63dbe1d000 rw-p 00007000 08:02 2113801                    /lib64/libnss_compat-2.12.2.so
7f63dbe36000-7f63dc061000 r--p 00000000 08:02 622671                     /usr/lib64/locale/locale-archive
7f63dc061000-7f63dc1ba000 r-xp 00000000 08:02 2113794                    /lib64/libc-2.12.2.so
7f63dc1ba000-7f63dc3ba000 ---p 00159000 08:02 2113794                    /lib64/libc-2.12.2.so
7f63dc3ba000-7f63dc3be000 r--p 00159000 08:02 2113794                    /lib64/libc-2.12.2.so
7f63dc3be000-7f63dc3bf000 rw-p 0015d000 08:02 2113794                    /lib64/libc-2.12.2.so
7f63dc3bf000-7f63dc3c4000 rw-p 00000000 00:00 0
7f63dc3c4000-7f63dc3c6000 r-xp 00000000 08:02 2113802                    /lib64/libdl-2.12.2.so
7f63dc3c6000-7f63dc5c6000 ---p 00002000 08:02 2113802                    /lib64/libdl-2.12.2.so
7f63dc5c6000-7f63dc5c7000 r--p 00002000 08:02 2113802                    /lib64/libdl-2.12.2.so
7f63dc5c7000-7f63dc5c8000 rw-p 00003000 08:02 2113802                    /lib64/libdl-2.12.2.so
7f63dc5c8000-7f63dc613000 r-xp 00000000 08:02 2130033                    /lib64/libncurses.so.5.7
7f63dc613000-7f63dc812000 ---p 0004b000 08:02 2130033                    /lib64/libncurses.so.5.7
7f63dc812000-7f63dc816000 r--p 0004a000 08:02 2130033                    /lib64/libncurses.so.5.7
7f63dc816000-7f63dc817000 rw-p 0004e000 08:02 2130033                    /lib64/libncurses.so.5.7
7f63dc817000-7f63dc818000 rw-p 00000000 00:00 0
7f63dc818000-7f63dc836000 r-xp 00000000 08:02 2113792                    /lib64/ld-2.12.2.so
7f63dca18000-7f63dca1b000 rw-p 00000000 00:00 0
7f63dca2b000-7f63dca2d000 rw-p 00000000 00:00 0
7f63dca2d000-7f63dca34000 r--s 00000000 08:02 622670                     /usr/lib64/gconv/gconv-modules.cache
7f63dca34000-7f63dca35000 rw-p 00000000 00:00 0
7f63dca35000-7f63dca36000 r--p 0001d000 08:02 2113792                    /lib64/ld-2.12.2.so
7f63dca36000-7f63dca37000 rw-p 0001e000 08:02 2113792                    /lib64/ld-2.12.2.so
7f63dca37000-7f63dca38000 rw-p 00000000 00:00 0							<---- B
7fffba432000-7fffba453000 rw-p 00000000 00:00 0                          [stack]
7fffba458000-7fffba459000 r-xp 00000000 00:00 0                          [vdso]
ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0                  [vsyscall]


-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-25 20:00               ` Borislav Petkov
@ 2011-07-25 20:06                 ` Ingo Molnar
  2011-07-25 21:53                   ` Borislav Petkov
  0 siblings, 1 reply; 46+ messages in thread
From: Ingo Molnar @ 2011-07-25 20:06 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Linus Torvalds, H. Peter Anvin, Thomas Gleixner, LKML, Przywara,
	Andre, Pohlack, Martin


* Borislav Petkov <bp@amd64.org> wrote:

> So yeah, the simpler fix works for processes mapping libraries in 
> the same order but not for processes mapping a subset of libraries 
> in a different order.

but what is the proportion of 'good' versus bad alignment of 
libraries, could you collect some stats on a representative enough, 
fully booted up Linux system?

Are only 1% of mappings 'bad'? 5%? 10%? 50%? We have no idea and the 
actual number matters a lot.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-25 20:06                 ` Ingo Molnar
@ 2011-07-25 21:53                   ` Borislav Petkov
  2011-07-26  5:58                     ` Ray Lee
  0 siblings, 1 reply; 46+ messages in thread
From: Borislav Petkov @ 2011-07-25 21:53 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Borislav Petkov, Linus Torvalds, H. Peter Anvin, Thomas Gleixner,
	LKML, Przywara, Andre, Pohlack, Martin

On Mon, Jul 25, 2011 at 04:06:45PM -0400, Ingo Molnar wrote:
> 
> * Borislav Petkov <bp@amd64.org> wrote:
> 
> > So yeah, the simpler fix works for processes mapping libraries in 
> > the same order but not for processes mapping a subset of libraries 
> > in a different order.
> 
> but what is the proportion of 'good' versus bad alignment of 
> libraries, could you collect some stats on a representative enough, 
> fully booted up Linux system?
> 
> Are only 1% of mappings 'bad'? 5%? 10%? 50%? We have no idea and the 
> actual number matters a lot.

Actually, it's hard to say what is a 'good' and 'bad' allocation. If
they don't alias, they're always good :).

Anyway, I did some beginners awk-ery on a gentoo system with ca. 150
processes. Below is some data for the "r-xp" mappings.

ld-2.12.2.so, for example, is always loaded at an address with [14:12]
= 000b while libc's addresses differ in those bits because a different
number of allocations is being done between ld and libc, thus libc lands
at different offsets.

I'll do a more comprehensive collection tomorrow and distribute the
mappings by [14:12] value, that should give us a better idea.

/lib64/ld-2.12.2.so|r-xp|7f07f61d8000 1
/lib64/ld-2.12.2.so|r-xp|7f1d8be10000 1
/lib64/ld-2.12.2.so|r-xp|7f1ea7678000 1
/lib64/ld-2.12.2.so|r-xp|7f272e248000 1
/lib64/ld-2.12.2.so|r-xp|7f37acf88000 1
/lib64/ld-2.12.2.so|r-xp|7f49853f0000 1
/lib64/ld-2.12.2.so|r-xp|7f50c1460000 1
/lib64/ld-2.12.2.so|r-xp|7f59f58e0000 1
/lib64/ld-2.12.2.so|r-xp|7f63dc818000 1
/lib64/ld-2.12.2.so|r-xp|7f7267d70000 1
/lib64/ld-2.12.2.so|r-xp|7fa026ba0000 2
/lib64/ld-2.12.2.so|r-xp|7fa24de88000 1
/lib64/ld-2.12.2.so|r-xp|7fa604cf8000 1
/lib64/ld-2.12.2.so|r-xp|7fb07d418000 1
/lib64/ld-2.12.2.so|r-xp|7fc33cb78000 1
/lib64/ld-2.12.2.so|r-xp|7fd0c2770000 1
/lib64/ld-2.12.2.so|r-xp|7fd1e2a08000 1
/lib64/ld-2.12.2.so|r-xp|7fd214b18000 1
/lib64/ld-2.12.2.so|r-xp|7fda88a58000 1
/lib64/ld-2.12.2.so|r-xp|7fdb87d98000 1
/lib64/ld-2.12.2.so|r-xp|7fe42a588000 1
/lib64/ld-2.12.2.so|r-xp|7ffd79c08000 1
/lib64/libc-2.12.2.so|r-xp|7f07f4e3e000 1
/lib64/libc-2.12.2.so|r-xp|7f1d8b66d000 1
/lib64/libc-2.12.2.so|r-xp|7f1ea7315000 1
/lib64/libc-2.12.2.so|r-xp|7f272ceae000 1
/lib64/libc-2.12.2.so|r-xp|7f37acc25000 1
/lib64/libc-2.12.2.so|r-xp|7f498508d000 1
/lib64/libc-2.12.2.so|r-xp|7f50c0ca9000 1
/lib64/libc-2.12.2.so|r-xp|7f59f557d000 1
/lib64/libc-2.12.2.so|r-xp|7f63dc061000 1
/lib64/libc-2.12.2.so|r-xp|7f7267a0d000 1
/lib64/libc-2.12.2.so|r-xp|7fa02591d000 2
/lib64/libc-2.12.2.so|r-xp|7fa24d8a5000 1
/lib64/libc-2.12.2.so|r-xp|7fa604995000 1
/lib64/libc-2.12.2.so|r-xp|7fb07cc31000 1
/lib64/libc-2.12.2.so|r-xp|7fc33b7de000 1
/lib64/libc-2.12.2.so|r-xp|7fd0c1f89000 1
/lib64/libc-2.12.2.so|r-xp|7fd1e26a5000 1
/lib64/libc-2.12.2.so|r-xp|7fd2147b5000 1
/lib64/libc-2.12.2.so|r-xp|7fda884e8000 1
/lib64/libc-2.12.2.so|r-xp|7fdb87a35000 1
/lib64/libc-2.12.2.so|r-xp|7fe42a01c000 1
/lib64/libc-2.12.2.so|r-xp|7ffd79451000 1

Thanks.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-25 21:53                   ` Borislav Petkov
@ 2011-07-26  5:58                     ` Ray Lee
  2011-07-26 17:28                       ` Borislav Petkov
  0 siblings, 1 reply; 46+ messages in thread
From: Ray Lee @ 2011-07-26  5:58 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Ingo Molnar, Linus Torvalds, H. Peter Anvin, Thomas Gleixner,
	LKML, Przywara, Andre, Pohlack, Martin

On Mon, Jul 25, 2011 at 2:53 PM, Borislav Petkov <bp@amd64.org> wrote:
>
> On Mon, Jul 25, 2011 at 04:06:45PM -0400, Ingo Molnar wrote:
> >
> > * Borislav Petkov <bp@amd64.org> wrote:
> >
> > > So yeah, the simpler fix works for processes mapping libraries in
> > > the same order but not for processes mapping a subset of libraries
> > > in a different order.
> >
> > but what is the proportion of 'good' versus bad alignment of
> > libraries, could you collect some stats on a representative enough,
> > fully booted up Linux system?
> >
> > Are only 1% of mappings 'bad'? 5%? 10%? 50%? We have no idea and the
> > actual number matters a lot.
>
> Actually, it's hard to say what is a 'good' and 'bad' allocation. If
> they don't alias, they're always good :).

Count the number of aliases per total mappings, and report that as a
failure percentage? It doesn't have to be a perfect metric, it just
needs to be a meaningful and repeatable one.

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-26  5:58                     ` Ray Lee
@ 2011-07-26 17:28                       ` Borislav Petkov
  2011-07-26 18:34                         ` Ingo Molnar
  0 siblings, 1 reply; 46+ messages in thread
From: Borislav Petkov @ 2011-07-26 17:28 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Ray Lee, Linus Torvalds, H. Peter Anvin, Thomas Gleixner, LKML,
	Przywara, Andre, Pohlack, Martin

[-- Attachment #1: Type: text/plain, Size: 1790 bytes --]

On Tue, Jul 26, 2011 at 01:58:43AM -0400, Ray Lee wrote:
> > > Are only 1% of mappings 'bad'? 5%? 10%? 50%? We have no idea and the
> > > actual number matters a lot.
> >
> > Actually, it's hard to say what is a 'good' and 'bad' allocation. If
> > they don't alias, they're always good :).
> 
> Count the number of aliases per total mappings, and report that as a
> failure percentage? It doesn't have to be a perfect metric, it just
> needs to be a meaningful and repeatable one.

Ok, I went and installed a Debian wheezy with gnome and started almost
everything I could find in the startup menu. Kernel is 3.0 with Linus'
simpler fix.

The attached file shows all libraries which would create aliases (i.e. 2
or more unique values for bits [14:12] along with the respective count)
in the total of:

Total r-xp mappings: 831, aliasing: 240 (0.289%)

For example, libc gets mapped into all possible slots for [14:12]

Library                                 [14:12] count
=======                                 ======= =====
/lib/libc-2.11.2.so
                                        0       12
                                        1       12
                                        2       11
                                        3       12
                                        4       13
                                        5       16
                                        6       13
                                        7       9


and so on.

So we're close to 30% and could get higher with more apps. I guess I'll
start working on the review comments to the more involved fix.

Thanks.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

[-- Attachment #2: simple-fix-results.txt --]
[-- Type: text/plain, Size: 18292 bytes --]

Library					[14:12]	count
=======					=======	=====
/usr/lib/gtk-2.0/2.10.0/engines/libclearlooks.so
					1	1
					3	1
					4	2
					5	1
					7	1
/usr/lib/libimobiledevice.so.1.0.2
					2	1
					5	1
/usr/lib/python2.6/lib-dynload/datetime.so
					3	1
					7	1
/usr/lib/libxcb-render-util.so.0.0.0
					0	10
					1	11
					2	3
					3	2
					4	7
					5	5
					6	7
					7	6
/lib/libutil-2.11.2.so
					0	3
					1	3
					2	2
					3	2
					4	4
					5	5
					6	1
					7	3
/usr/lib/libatk-1.0.so.0.3009.1
					0	5
					1	10
					2	9
					3	7
					4	1
					5	8
					6	8
					7	3
/usr/lib/libbrasero-media.so.0.2.0
					2	1
					7	1
[vdso]
					0	4
					1	9
					2	4
					3	1
					4	7
					5	6
					6	6
					7	61
/usr/lib/enchant/libenchant_aspell.so
					4	2
					6	1
/usr/lib/libpangocairo-1.0.so.0.2800.3
					0	9
					1	4
					2	4
					3	9
					4	5
					5	7
					6	7
					7	6
/usr/lib/pyshared/python2.6/gtk-2.0/atk.so
					5	2
					7	1
/usr/lib/libICE.so.6.3.0
					0	6
					1	4
					2	5
					3	3
					5	14
					6	2
					7	1
/usr/lib/libebook-1.2.so.9.3.1
					0	1
					1	1
					3	1
					6	1
/usr/lib/libpolkit-gobject-1.so.0.0.0
					2	3
					3	2
					4	2
					5	1
					7	1
/usr/lib/libSM.so.6.0.1
					0	13
					1	2
					2	2
					3	5
					4	3
					5	6
					6	3
					7	1
/usr/lib/libsoup-2.4.so.1.3.0
					0	1
					2	1
					3	2
					4	1
					5	1
					6	1
					7	2
/usr/lib/libXdamage.so.1.1.0
					0	5
					1	5
					2	3
					3	8
					4	9
					5	8
					6	8
					7	5
/lib/libbz2.so.1.0.4
					0	3
					1	3
					2	7
					3	6
					5	1
					7	1
/lib/libpthread-2.11.2.so
					0	9
					1	9
					2	7
					3	12
					4	15
					5	7
					6	11
					7	9
/usr/lib/libgailutil.so.18.0.1
					0	1
					2	1
					3	3
					4	1
					5	2
					6	1
					7	4
/usr/lib/gstreamer-0.10/libgstossaudio.so
					4	1
					6	1
/usr/lib/libavahi-common.so.3.5.2
					0	1
					1	3
					2	3
					4	6
					5	1
					6	1
					7	4
/usr/lib/liblcms.so.1.0.18
					1	1
					2	1
/usr/lib/libcamel-1.2.so.14.0.1
					0	1
					2	1
					6	1
					7	1
/usr/lib/libbonobo-2.so.0.0.0
					0	1
					1	3
					2	3
					3	1
					4	2
					6	3
					7	2
/usr/lib/libtdb.so.1.2.1
					0	1
					1	4
					2	1
					3	1
					4	2
					5	2
/usr/lib/libvte.so.9.13.3
					1	1
					5	1
/lib/libcrypt-2.11.2.so
					0	2
					1	1
					2	6
					3	1
					6	2
					7	1
/usr/lib/libX11.so.6.3.0
					0	5
					1	7
					2	5
					3	7
					4	3
					5	8
					6	8
					7	12
/usr/lib/libcairomm-1.0.so.1.4.0
					2	1
					4	1
/usr/lib/gtk-2.0/2.10.0/loaders/svg_loader.so
					1	1
					2	4
					4	2
					5	1
/lib/libnss_files-2.11.2.so
					0	10
					1	13
					2	7
					3	8
					4	12
					5	11
					6	7
					7	11
/usr/lib/liborc-0.4.so.0.0.0
					0	1
					2	1
					5	1
/usr/lib/libssl3.so.1d
					0	1
					1	2
					2	1
					7	1
/lib/libnss_compat-2.11.2.so
					0	13
					1	6
					2	8
					3	10
					4	12
					5	8
					6	9
					7	11
/usr/lib/gtk-2.0/modules/libgail-gnome.so
					1	2
					2	1
					3	1
					6	1
					7	1
/lib/libdbus-1.so.3.4.0
					0	11
					1	13
					2	15
					3	3
					4	6
					5	8
					6	6
					7	7
/usr/lib/libexpat.so.1.5.2
					0	5
					1	14
					2	4
					3	11
					4	12
					5	6
					6	3
					7	4
/usr/lib/libxcb-event.so.1.0.0
					0	1
					1	1
					3	1
					4	1
					6	1
					7	3
/usr/lib/libz.so.1.2.3.4
					0	15
					1	12
					2	7
					3	9
					4	11
					5	8
					6	3
					7	5
/usr/lib/libpixman-1.so.0.16.4
					0	6
					1	9
					2	10
					3	3
					4	3
					5	8
					6	6
					7	7
/lib/libresolv-2.11.2.so
					0	9
					1	13
					2	7
					3	7
					4	5
					5	2
					6	9
					7	13
/lib/libc-2.11.2.so
					0	12
					1	12
					2	11
					3	12
					4	13
					5	16
					6	13
					7	9
/usr/lib/libnss3.so.1d
					1	2
					2	1
					3	2
/usr/lib/libk5crypto.so.3.1
					0	1
					1	3
					3	2
					4	3
					7	3
/usr/lib/gconv/ISO8859-1.so
					0	4
					1	6
					2	8
					3	5
					4	7
					5	10
					6	10
					7	3
/usr/lib/libpython2.6.so.1.0
					2	1
					4	2
/usr/lib/pyshared/python2.6/gtk-2.0/pango.so
					2	1
					3	1
					4	1
/usr/lib/enchant/libenchant_hspell.so
					5	2
					7	1
/usr/lib/libpangomm-1.4.so.1.0.30
					0	1
					3	1
/usr/lib/libxml2.so.2.7.8
					0	1
					1	5
					2	3
					3	2
					4	6
					5	8
					6	6
					7	10
/usr/lib/libxcb-atom.so.1.0.0
					0	1
					2	1
					3	3
					4	1
					5	1
					7	1
/usr/lib/libxkbfile.so.1.0.2
					0	1
					6	1
/usr/lib/libgstvideo-0.10.so.0.21.0
					1	2
					2	1
					7	1
/usr/lib/libXau.so.6.0.0
					0	8
					1	10
					2	7
					3	4
					4	5
					5	9
					6	8
					7	5
/usr/lib/libORBit-2.so.0.1.0
					0	2
					1	10
					2	4
					3	6
					4	6
					5	9
					6	9
					7	3
/usr/lib/libplist.so.1.1.3
					1	1
					4	1
/usr/lib/libsoup-gnome-2.4.so.1.3.0
					2	1
					4	1
					5	1
					6	1
/usr/lib/librsvg-2.so.2.26.3
					0	2
					2	3
					3	3
					4	4
					5	6
					7	1
/usr/lib/enchant/libenchant_ispell.so
					3	1
					4	2
/usr/lib/libgtop-2.0.so.7.2.0
					0	1
					1	1
/lib/libuuid.so.1.3.0
					0	2
					1	8
					2	7
					3	4
					4	7
					5	4
					6	4
/usr/lib/pyshared/python2.6/gtk-2.0/gtk/_gtk.so
					1	2
					5	1
/usr/lib/gtk-2.0/2.10.0/loaders/libpixbufloader-xpm.so
					0	2
					1	1
					3	2
					4	3
					5	3
					6	2
					7	2
/usr/lib/libsqlite3.so.0.8.6
					1	1
					2	2
					3	2
					4	2
					5	2
					7	1
/usr/lib/libvorbis.so.0.4.4
					0	2
					1	2
					4	1
					5	4
					6	1
					7	1
/usr/lib/libsasl2.so.2.0.23
					2	1
					3	1
					6	2
					7	1
/usr/lib/libXrender.so.1.3.0
					0	9
					1	7
					2	15
					3	4
					4	4
					5	8
					6	1
					7	4
/lib/libm-2.11.2.so
					0	3
					1	6
					2	10
					3	2
					4	7
					5	9
					6	15
					7	9
/usr/lib/libtiff.so.4.3.3
					1	1
					3	1
/usr/lib/libstdc++.so.6.0.13
					0	5
					2	2
					3	2
					4	1
					6	1
					7	2
/usr/lib/libXext.so.6.4.0
					0	2
					1	3
					2	7
					3	8
					4	14
					5	5
					6	6
					7	7
/usr/lib/libenchant.so.1.6.0
					0	1
					2	2
					3	1
					6	1
					7	1
/usr/lib/libgnomeui-2.so.0.2400.3
					1	1
					5	1
/lib/libnss_mdns4_minimal.so.2
					0	1
					2	3
					4	1
					6	1
					7	1
/usr/lib/pyshared/python2.6/gtk-2.0/gconf.so
					1	1
					2	1
					4	1
/usr/lib/libdrm.so.2.4.0
					3	1
					7	1
/usr/lib/libgdkmm-2.4.so.1.1.0
					1	1
					3	1
/usr/lib/libdbus-glib-1.so.2.1.0
					0	10
					1	11
					2	12
					3	6
					4	9
					5	7
					6	2
					7	5
/usr/lib/libicalvcal.so.0.44.0
					4	1
					6	2
/usr/lib/libgtkmm-2.4.so.1.1.0
					5	1
					6	1
/usr/lib/libtasn1.so.3.1.9
					0	4
					1	3
					2	2
					3	5
					4	4
					5	1
					6	4
					7	5
/usr/lib/libgstaudio-0.10.so.0.21.0
					1	1
					3	1
					7	1
/usr/lib/gtk-2.0/2.10.0/loaders/libpixbufloader-png.so
					0	3
					1	2
					2	2
					3	4
					4	1
					5	2
					6	2
					7	1
/usr/lib/libXtst.so.6.1.0
					2	1
					5	3
/usr/lib/libunique-1.0.so.0.100.6
					0	1
					1	2
					2	1
					6	1
/usr/lib/libldap_r-2.4.so.2.5.6
					4	2
					6	1
					7	1
					0	1
/usr/lib/pyshared/python2.6/gtk-2.0/gio/unix.so
					3	1
					5	2
/usr/lib/gvfs/libgvfscommon.so
					0	3
					1	1
					3	2
					5	3
					6	1
					7	1
/lib/libdl-2.11.2.so
					0	7
					1	8
					2	12
					3	7
					4	11
					5	11
					6	17
					7	18
/usr/lib/libORBitCosNaming-2.so.0.1.0
					0	4
					1	2
					2	1
					3	1
					4	1
					5	2
					6	1
					7	3
/usr/lib/libproxy.so.0.0.0
					2	2
					3	1
					4	1
/usr/lib/libnm-glib.so.2.2.3
					5	1
					6	1
/usr/lib/libjpeg.so.62.0.0
					0	1
					1	4
					4	1
					6	1
					7	1
/lib/libnsl-2.11.2.so
					0	15
					1	8
					2	10
					3	6
					4	14
					5	7
					6	10
					7	9
/usr/lib/libgstinterfaces-0.10.so.0.21.0
					2	1
					4	1
					5	1
					6	1
					7	1
/usr/lib/libfontconfig.so.1.4.4
					0	3
					1	9
					2	7
					3	16
					4	5
					5	4
					6	3
					7	4
/usr/lib/libnssutil3.so.1d
					5	2
					6	1
					7	2
/lib/libpam.so.0.82.2
					2	4
					4	3
					6	1
					7	1
/usr/lib/libusbmuxd.so.1.0.4
					5	1
					6	1
/usr/lib/libfreetype.so.6.6.0
					0	13
					1	7
					2	4
					3	4
					4	5
					5	3
					6	8
					7	8
/lib/libselinux.so.1
					0	7
					1	10
					2	8
					3	19
					4	12
					5	3
					6	8
					7	4
/usr/lib/libaudiofile.so.0.0.2
					0	1
					1	1
					2	4
					3	1
					4	2
					5	1
					7	1
/usr/lib/libgstcontroller-0.10.so.0.26.0
					3	1
					4	1
/usr/lib/libgnome-2.so.0.3000.0
					0	1
					1	1
					3	2
					4	1
					5	2
					6	2
					7	2
/usr/lib/libgssapi_krb5.so.2.2
					1	2
					3	2
					4	2
					5	4
					7	2
/usr/lib/libnotify.so.1.2.3
					0	1
					1	1
					3	1
					5	1
					7	3
/usr/lib/libXxf86vm.so.1.0.0
					2	1
					3	1
/usr/lib/libpanel-applet-2.so.0.2.68
					0	1
					2	2
					3	1
					4	2
					7	2
/usr/lib/libwnck-1.so.22.3.29
					1	2
					4	1
/usr/lib/libnm-util.so.1.3.4
					1	1
					4	1
/lib/libattr.so.1.1.0
					0	3
					3	1
					6	2
/usr/lib/libgimpconfig-2.0.so.0.600.10
					1	1
					2	1
/lib/libcom_err.so.2.1
					0	2
					1	3
					2	3
					5	1
					6	3
/lib/libnss_mdns4.so.2
					0	1
					4	1
/usr/lib/libxcb.so.1.1.0
					0	2
					1	11
					2	8
					3	4
					4	10
					5	15
					6	4
					7	1
/usr/lib/libwebkit-1.0.so.2.17.9
					2	1
					6	1
/usr/lib/libxcb-render.so.0.0.0
					0	10
					1	11
					2	3
					3	2
					4	7
					5	5
					6	7
					7	6
/usr/lib/libcups.so.2
					2	1
					5	1
					6	1
/usr/lib/libgdk_pixbuf-2.0.so.0.2000.1
					0	8
					1	9
					2	3
					3	4
					4	8
					5	5
					6	8
					7	6
/usr/lib/libbonobo-activation.so.4.0.0
					0	1
					1	1
					2	1
					3	3
					4	2
					5	1
					6	3
					7	3
/usr/lib/libXi.so.6.1.0
					0	6
					1	15
					2	6
					3	6
					4	7
					6	3
					7	9
/usr/lib/libupower-glib.so.1.0.1
					1	2
					2	1
					4	1
/lib/libusb-0.1.so.4.4.4
					6	2
					7	1
/usr/lib/libgmime-2.4.so.2.4.14
					2	1
					4	1
					5	1
					7	1
/usr/lib/libavahi-client.so.3.2.7
					0	1
					1	3
					2	3
					4	4
					5	1
					6	1
					7	4
/lib/libglib-2.0.so.0.2400.2
					0	11
					1	4
					2	9
					3	4
					4	11
					5	9
					6	7
					7	10
/usr/lib/libgtkspell.so.0.0.0
					1	1
					4	1
/usr/lib/enchant/libenchant_myspell.so
					0	2
					2	1
/usr/lib/libXrandr.so.2.2.0
					0	7
					1	12
					2	8
					3	6
					4	7
					5	1
					6	2
					7	8
/usr/lib/libffi.so.5.0.10
					1	1
					6	1
					7	1
/usr/lib/libnl.so.1.1
					0	1
					3	1
/lib/libwrap.so.0.7.6
					1	1
					6	3
					7	2
/usr/lib/libv4l2.so.0
					4	1
					6	1
/usr/lib/libgimpmodule-2.0.so.0.600.10
					1	1
					3	1
/usr/lib/libgnome-desktop-2.so.17.1.0
					0	2
					5	1
					7	1
/usr/lib/gio/modules/libgvfsdbus.so
					1	2
					7	1
/usr/lib/libpangoft2-1.0.so.0.2800.3
					0	10
					1	8
					2	5
					3	3
					4	7
					5	2
					6	9
					7	7
/lib/libudev.so.0.9.3
					0	2
					2	5
					3	1
					5	1
					6	1
					7	2
/usr/lib/libbonoboui-2.so.0.0.0
					0	1
					1	2
					2	2
					3	1
					4	1
					5	2
					7	1
/usr/lib/libv4lconvert.so.0
					0	1
					6	1
/usr/lib/libvorbisfile.so.3.3.2
					0	1
					1	4
					2	1
					3	1
					4	2
					5	2
/usr/lib/libgobject-2.0.so.0.2400.2
					0	9
					1	10
					2	13
					3	3
					4	6
					5	9
					6	2
					7	13
/usr/lib/libgcrypt.so.11.5.3
					0	1
					1	6
					2	2
					3	2
					4	8
					5	5
					6	3
					7	3
/lib/libpcre.so.3.12.1
					0	4
					1	9
					2	5
					3	5
					4	8
					5	6
					6	13
					7	16
/usr/lib/libesd.so.0.2.39
					0	1
					1	1
					2	4
					3	1
					4	2
					5	1
					7	1
/usr/lib/libgimpmath-2.0.so.0.600.10
					1	1
					6	1
/usr/lib/libhunspell-1.2.so.0.0.0
					0	1
					5	1
					6	2
/lib/libaudit.so.0.0.0
					2	1
					3	1
					4	1
/usr/lib/libicalss.so.0.44.0
					0	1
					2	2
/lib/libcap.so.2.19
					3	1
					5	2
/usr/lib/libXinerama.so.1.0.0
					0	13
					1	5
					2	7
					3	7
					5	4
					6	9
					7	6
/usr/lib/libxklavier.so.16.0.0
					1	1
					4	1
/usr/lib/libgimpwidgets-2.0.so.0.600.10
					6	1
					7	1
/usr/lib/libgimpbase-2.0.so.0.600.10
					0	1
					1	1
/usr/lib/libedataserver-1.2.so.13.0.1
					5	2
					6	1
					7	1
/usr/lib/libicuuc.so.44.1
					0	1
					4	2
/lib/libnss_nis-2.11.2.so
					0	10
					1	12
					2	8
					3	10
					4	10
					5	13
					6	6
					7	8
/usr/lib/liblber-2.4.so.2.5.6
					1	1
					3	1
					4	1
					6	1
					7	1
/usr/lib/libkrb5.so.3.3
					1	2
					2	3
					5	4
					6	1
					7	2
/lib/libkeyutils.so.1.3
					1	1
					3	2
					4	1
					5	4
					6	1
					7	3
/lib/libpng12.so.0.44.0
					0	5
					1	7
					2	6
					3	12
					4	10
					5	2
					6	1
					7	8
/usr/lib/libpyglib-2.0-python2.6.so.0.0.0
					1	1
					6	1
					7	1
/usr/lib/libglibmm-2.4.so.1.3.0
					2	1
					5	1
/usr/lib/libnspr4.so.0d
					0	1
					1	1
					3	1
					4	1
					6	1
/usr/lib/libgnutls.so.26.14.12
					0	2
					1	6
					2	2
					3	7
					4	3
					5	4
					6	4
/usr/lib/libart_lgpl_2.so.2.3.21
					0	1
					1	1
					2	2
					3	1
					4	1
					5	2
					6	2
					7	1
/usr/lib/libgsttag-0.10.so.0.21.0
					5	1
					7	1
/usr/lib/lapack/liblapack.so.3gf.0
					6	1
					7	1
/usr/lib/libgimpcolor-2.0.so.0.600.10
					0	1
					7	1
/usr/lib/gstreamer-0.10/libgstoss4audio.so
					1	1
					2	1
/usr/lib/libgnome-keyring.so.0.1.1
					1	1
					3	1
					4	3
					5	2
					6	2
/lib/librt-2.11.2.so
					0	13
					1	8
					2	9
					3	3
					4	11
					5	8
					6	15
					7	9
/usr/lib/pyshared/python2.6/gtk-2.0/glib/_glib.so
					1	1
					3	1
					5	1
/usr/lib/libXfixes.so.3.1.0
					0	8
					1	9
					2	5
					3	6
					4	5
					5	3
					6	8
					7	8
/usr/lib/pango/1.6.0/modules/pango-basic-fc.so
					0	7
					1	5
					2	6
					3	9
					4	2
					5	1
					6	4
					7	5
/usr/lib/libgudev-1.0.so.0.0.1
					0	2
					4	1
					6	1
					7	4
/usr/lib/libgsf-1.so.114.0.18
					1	2
					2	7
					3	3
					5	6
					6	1
/lib/libncurses.so.5.7
					0	1
					2	5
					4	1
					7	1
/usr/lib/libgstpbutils-0.10.so.0.21.0
					1	2
					2	1
					3	1
					7	1
/usr/lib/pyshared/python2.6/gtk-2.0/gobject/_gobject.so
					2	2
					5	1
/usr/lib/libplds4.so.0d
					0	1
					2	1
					3	1
					5	1
					6	1
/usr/lib/libgpg-error.so.0.4.0
					0	5
					1	7
					2	2
					3	3
					4	5
					5	2
					6	2
					7	4
/usr/lib/libsmime3.so.1d
					0	1
					1	2
					4	1
					7	1
/usr/lib/gstreamer-0.10/libgstalsa.so
					1	1
					6	1
/usr/lib/gio/modules/libgioremote-volume-monitor.so
					0	1
					1	1
					2	1
					4	1
					6	3
/usr/lib/libgnomecanvas-2.so.0.3000.1
					0	1
					1	2
					2	1
					3	1
					4	1
					5	1
					6	2
					7	2
/usr/lib/libcanberra.so.0.2.3
					0	1
					1	6
					2	1
					4	3
/usr/lib/pyshared/python2.6/cairo/_cairo.so
					6	1
					7	2
/usr/lib/libcrypto.so.0.9.8
					2	3
					3	3
					4	2
					6	3
					7	2
/usr/lib/libatkmm-1.6.so.1.1.0
					0	1
					2	1
/usr/lib/libcairo.so.2.10800.10
					0	7
					1	4
					2	11
					3	5
					4	7
					5	7
					6	5
					7	5
/usr/lib/libaspell.so.15.1.4
					2	3
					4	1
/usr/lib/libgstreamer-0.10.so.0.26.0
					0	1
					4	2
					6	1
					7	3
/usr/lib/gtk-2.0/modules/libgail.so
					2	1
					3	1
					5	1
					6	2
					7	1
/usr/lib/libXt.so.6.0.0
					0	1
					2	1
					4	3
/usr/lib/libsigc-2.0.so.0.0.0
					1	1
					2	1
					3	1
/usr/lib/libgdk-x11-2.0.so.0.2000.1
					1	3
					2	9
					3	11
					4	8
					5	2
					6	6
					7	12
/usr/lib/libXRes.so.1.0.0
					2	2
					6	1
/usr/lib/libidn.so.11.5.44
					1	1
					5	1
/usr/lib/libgthread-2.0.so.0.2400.2
					0	8
					1	9
					2	5
					3	9
					4	14
					5	6
					6	10
					7	4
/usr/lib/libgstbase-0.10.so.0.26.0
					0	1
					2	1
					3	1
					4	2
					5	1
/usr/lib/libgio-2.0.so.0.2400.2
					0	13
					1	12
					2	5
					3	9
					4	4
					5	7
					6	5
					7	4
/usr/lib/libedataserverui-1.2.so.8.1.1
					2	1
					7	1
/usr/lib/libkrb5support.so.0.1
					0	1
					1	4
					3	1
					5	1
					6	1
					7	4
/usr/lib/libcroco-0.6.so.3.0.1
					0	3
					2	6
					3	1
					6	2
					7	7
/usr/lib/libogg.so.0.7.0
					0	1
					1	1
					2	2
					3	2
					4	1
					6	1
					7	4
/usr/lib/dri/swrast_dri.so
					0	1
					4	1
/usr/lib/libgtk-x11-2.0.so.0.2000.1
					0	11
					1	5
					2	1
					3	5
					4	12
					5	1
					6	3
					7	13
/usr/lib/libavahi-glib.so.1.0.2
					0	3
					1	1
					2	1
					3	3
					4	1
					5	1
					6	3
					7	1
/usr/lib/libdb-4.8.so
					3	2
					4	1
					7	1
/usr/lib/libeggdbus-1.so.0.0.0
					1	1
					2	2
					3	1
					5	1
					6	3
					7	1
/usr/lib/libicui18n.so.44.1
					3	1
					6	1
					7	1
/usr/lib/libssl.so.0.9.8
					0	2
					3	2
					5	1
					7	3
/usr/lib/libXcursor.so.1.0.2
					0	5
					1	6
					2	8
					3	1
					4	3
					5	9
					6	7
					7	13
/usr/lib/libXcomposite.so.1.0.0
					0	8
					1	5
					2	4
					3	5
					4	4
					5	8
					6	9
					7	8
/usr/lib/libltdl.so.7.2.1
					0	5
					1	2
					2	1
					3	2
					4	2
					7	1
/usr/lib/libcanberra-gtk.so.0.1.6
					0	1
					1	5
					2	1
					4	3
/lib/libpopt.so.0.0.0
					0	2
					1	2
					2	1
					3	2
					4	2
					6	1
					7	2
/usr/lib/libgnomevfs-2.so.0.2400.3
					0	2
					1	1
					3	1
					4	2
					5	3
					6	1
					7	2
/usr/lib/python2.6/lib-dynload/pyexpat.so
					1	1
					2	1
/usr/lib/libpango-1.0.so.0.2800.3
					0	11
					1	4
					2	7
					3	7
					4	4
					5	7
					6	5
					7	6
/usr/lib/libgconf-2.so.4.1.5
					0	1
					1	6
					2	11
					3	5
					4	7
					5	5
					6	5
					7	7
/usr/lib/gtk-2.0/modules/libatk-bridge.so
					0	2
					1	1
					4	1
					5	1
					7	1
/usr/lib/libasound.so.2.0.0
					0	2
					1	3
					2	1
					4	2
					5	1
					6	2
					7	2
/lib/libgcc_s.so.1
					0	1
					1	3
					2	2
					3	2
					4	4
					5	1
					6	1
					7	2
/usr/lib/pyshared/python2.6/gtk-2.0/pangocairo.so
					0	2
					2	1
/usr/lib/libstartup-notification-1.so.0.0.0
					0	1
					1	1
					3	2
					4	1
					6	3
/lib/ld-2.11.2.so
					0	88
					1	6
					3	1
					7	3
/usr/lib/libblas/libblas.so.3gf.0
					5	1
					6	1
/usr/lib/libXdmcp.so.6.0.0
					0	8
					1	7
					2	6
					3	8
					4	10
					5	6
					6	4
					7	7
/usr/lib/libspi.so.0.10.11
					0	1
					1	3
					2	1
					5	1
					6	1
/usr/lib/libxcb-aux.so.0.0.0
					0	1
					2	1
					3	3
					4	1
					5	1
					7	1
/usr/lib/libxslt.so.1.1.26
					0	1
					5	1
					6	1
					7	1
/usr/lib/libgmodule-2.0.so.0.2400.2
					0	15
					1	6
					2	5
					3	5
					4	8
					5	7
					6	12
					7	6
/usr/lib/libtotem-plparser.so.17.0.0
					0	1
					2	1
					6	1
/usr/lib/pyshared/python2.6/gtk-2.0/gio/_gio.so
					3	1
					4	1
					5	1
/usr/lib/libplc4.so.0d
					1	1
					2	1
					4	1
					6	1
					7	1
/lib/libnss_dns-2.11.2.so
					0	1
					1	1
					2	1
					4	3
					6	1
/usr/lib/libecal-1.2.so.7.2.2
					1	1
					2	2
Total r-xp mappings: 831, aliasing: 240 (0.289%)

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-22 13:15 [PATCH] x86, AMD: Correct F15h IC aliasing issue Borislav Petkov
  2011-07-24 11:13 ` Ingo Molnar
  2011-07-24 16:04 ` Linus Torvalds
@ 2011-07-26 17:59 ` Avi Kivity
  2011-07-26 18:13   ` Borislav Petkov
  2 siblings, 1 reply; 46+ messages in thread
From: Avi Kivity @ 2011-07-26 17:59 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: H. Peter Anvin, Ingo Molnar, Thomas Gleixner, LKML,
	Borislav Petkov, Andre Przywara, Martin Pohlack

On 07/22/2011 04:15 PM, Borislav Petkov wrote:
> From: Borislav Petkov<borislav.petkov@amd.com>
>
> This patch provides performance tuning for the "Bulldozer" CPU. With its
> shared instruction cache there is a chance of generating an excessive
> number of cache cross-invalidates when running specific workloads on the
> cores of a compute module.
>
> This excessive amount of cross-invalidations can be observed if cache
> lines backed by shared physical memory alias in bits [14:12] of their
> virtual addresses, as those bits are used for the index generation.
>
> This patch addresses the issue by zeroing out the slice [14:12] of
> the file mapping's virtual address at generation time, thus forcing
> those bits the same for all mappings of a single shared library across
> processes and, in doing so, avoids instruction cache aliases.
>
> It also adds the kernel command line option
> "unalias_va_addr=(32|64|off)" with which virtual address unaliasing
> can be enabled for 32-bit or 64-bit x86 individually, or be completely
> disabled.
>
> This change leaves virtual region address allocation on other families
> and/or vendors unaffected.
>

Is it possible to derive the bit positions (and the need to mask them) 
from the cpuid description of the cache topology and sizes?

-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-26 17:59 ` Avi Kivity
@ 2011-07-26 18:13   ` Borislav Petkov
  2011-07-26 18:16     ` H. Peter Anvin
  0 siblings, 1 reply; 46+ messages in thread
From: Borislav Petkov @ 2011-07-26 18:13 UTC (permalink / raw)
  To: Avi Kivity
  Cc: Borislav Petkov, H. Peter Anvin, Ingo Molnar, Thomas Gleixner,
	LKML, Przywara, Andre, Pohlack, Martin

Hi Avi,

On Tue, Jul 26, 2011 at 01:59:04PM -0400, Avi Kivity wrote:
> > This change leaves virtual region address allocation on other families
> > and/or vendors unaffected.
> >
> 
> Is it possible to derive the bit positions (and the need to mask them) 
> from the cpuid description of the cache topology and sizes?

As far as I understand your question, there's no need for deriving the
bit positions because they're not special. You just have to have bits
[14:12] the same across all processes - we simply opted for clearing
them in order to keep the patch as simple as possible. But we could just
as well hashed the library name and generated the bits from it and thus
keep them same per library (we have that version too, btw. :)).

FWIW, in both cases, the patch should fix even the virtualization
scenario with and without KSM.

Does that answer your question?

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-26 18:13   ` Borislav Petkov
@ 2011-07-26 18:16     ` H. Peter Anvin
  2011-07-26 18:37       ` Borislav Petkov
  0 siblings, 1 reply; 46+ messages in thread
From: H. Peter Anvin @ 2011-07-26 18:16 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Avi Kivity, Ingo Molnar, Thomas Gleixner, LKML, Przywara, Andre,
	Pohlack, Martin

On 07/26/2011 11:13 AM, Borislav Petkov wrote:
> Hi Avi,
> 
> On Tue, Jul 26, 2011 at 01:59:04PM -0400, Avi Kivity wrote:
>>> This change leaves virtual region address allocation on other families
>>> and/or vendors unaffected.
>>>
>>
>> Is it possible to derive the bit positions (and the need to mask them) 
>> from the cpuid description of the cache topology and sizes?
> 
> As far as I understand your question, there's no need for deriving the
> bit positions because they're not special. You just have to have bits
> [14:12] the same across all processes - we simply opted for clearing
> them in order to keep the patch as simple as possible. But we could just
> as well hashed the library name and generated the bits from it and thus
> keep them same per library (we have that version too, btw. :)).
> 
> FWIW, in both cases, the patch should fix even the virtualization
> scenario with and without KSM.
> 
> Does that answer your question?
> 

I think the question was the width (and position) for the mask... i.e.
your [14:12] above which *is* magic.

	-hpa


-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-24 13:40   ` Borislav Petkov
  2011-07-24 13:47     ` Ingo Molnar
  2011-07-24 16:16     ` Andrew Morton
@ 2011-07-26 18:33     ` Borislav Petkov
  2 siblings, 0 replies; 46+ messages in thread
From: Borislav Petkov @ 2011-07-26 18:33 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Linus Torvalds, Andrew Morton, H. Peter Anvin, Thomas Gleixner,
	LKML, Przywara, Andre, Pohlack, Martin

On Sun, Jul 24, 2011 at 09:40:46AM -0400, Borislav Petkov wrote:
> > also, the PAGE_ALIGN() complication here looks unnecessary - can the
> > vdso 'addr' ever be not 4K aligned above?
> 
> Yeah, it can. I did some trace_printk runs and 'addr' wasn't 4K aligned
> in some cases. I think this is because of the stack randomization we do
> and we use mm->start_stack to get the vdso base address. Unfortunately,
> I can't find those traces anymore but will do some again tomorrow.

Better late than never :).

So yes, mm->start_stack comes in unaligned from setup_arg_pages() and we
use it to generate the vdso VA because the vdso is below the stack:

7fff761f9000-7fff7621a000 rw-p 00000000 00:00 0                          [stack]
7fff763ec000-7fff763ed000 r-xp 00000000 00:00 0                          [vdso]
ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0                  [vsyscall]

here's a trace_printk excerpt from vdso_addr() (gets inlined into
arch_setup_additional_pages()):

           <...>-2516  [006]   624.364274: arch_setup_additional_pages: mm->start_stack: 0x7fff7621942a, len: 4096
           <...>-2516  [006]   624.364278: arch_setup_additional_pages: randomized: 0x7fff763eb42a
           <...>-2516  [006]   624.364279: arch_setup_additional_pages: final addr: 0x7fff763eb42a

Then, both arch_get_unmapped_area{,_topdown} page-align it.

For our case, we PAGE_ALIGN it before randomizing it so that we can
control the [14:12] setting.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-26 17:28                       ` Borislav Petkov
@ 2011-07-26 18:34                         ` Ingo Molnar
  2011-07-26 18:39                           ` Borislav Petkov
  0 siblings, 1 reply; 46+ messages in thread
From: Ingo Molnar @ 2011-07-26 18:34 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Ray Lee, Linus Torvalds, H. Peter Anvin, Thomas Gleixner, LKML,
	Przywara, Andre, Pohlack, Martin


* Borislav Petkov <bp@amd64.org> wrote:

> Ok, I went and installed a Debian wheezy with gnome and started 
> almost everything I could find in the startup menu. Kernel is 3.0 
> with Linus' simpler fix.
> 
> The attached file shows all libraries which would create aliases 
> (i.e. 2 or more unique values for bits [14:12] along with the 
> respective count) in the total of:
> 
> Total r-xp mappings: 831, aliasing: 240 (0.289%)

28.9%, right?

> For example, libc gets mapped into all possible slots for [14:12]
> 
> Library                                 [14:12] count
> =======                                 ======= =====
> /lib/libc-2.11.2.so
>                                         0       12
>                                         1       12
>                                         2       11
>                                         3       12
>                                         4       13
>                                         5       16
>                                         6       13
>                                         7       9
> 
> 
> and so on.

Was this done with a stock kernel, or with the simple patch applied?

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-26 18:16     ` H. Peter Anvin
@ 2011-07-26 18:37       ` Borislav Petkov
  2011-07-26 18:38         ` H. Peter Anvin
  0 siblings, 1 reply; 46+ messages in thread
From: Borislav Petkov @ 2011-07-26 18:37 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Borislav Petkov, Avi Kivity, Ingo Molnar, Thomas Gleixner, LKML,
	Przywara, Andre, Pohlack, Martin

On Tue, Jul 26, 2011 at 02:16:56PM -0400, H. Peter Anvin wrote:
> >> Is it possible to derive the bit positions (and the need to mask them) 
> >> from the cpuid description of the cache topology and sizes?
> > 
> > As far as I understand your question, there's no need for deriving the
> > bit positions because they're not special. You just have to have bits
> > [14:12] the same across all processes - we simply opted for clearing
> > them in order to keep the patch as simple as possible. But we could just
> > as well hashed the library name and generated the bits from it and thus
> > keep them same per library (we have that version too, btw. :)).
> > 
> > FWIW, in both cases, the patch should fix even the virtualization
> > scenario with and without KSM.
> > 
> > Does that answer your question?
> > 
> 
> I think the question was the width (and position) for the mask... i.e.
> your [14:12] above which *is* magic.

Oh, that's easy: family 15h means bits [14:12] - those bits are used for
I$ index generation.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-26 18:37       ` Borislav Petkov
@ 2011-07-26 18:38         ` H. Peter Anvin
  2011-07-26 19:42           ` Andre Przywara
  0 siblings, 1 reply; 46+ messages in thread
From: H. Peter Anvin @ 2011-07-26 18:38 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Avi Kivity, Ingo Molnar, Thomas Gleixner, LKML, Przywara, Andre,
	Pohlack, Martin

On 07/26/2011 11:37 AM, Borislav Petkov wrote:
>>
>> I think the question was the width (and position) for the mask... i.e.
>> your [14:12] above which *is* magic.
> 
> Oh, that's easy: family 15h means bits [14:12] - those bits are used for
> I$ index generation.
> 

In other words, it's completely ad hoc.

	-hpa


-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-26 18:34                         ` Ingo Molnar
@ 2011-07-26 18:39                           ` Borislav Petkov
  2011-07-26 18:47                             ` Ingo Molnar
  0 siblings, 1 reply; 46+ messages in thread
From: Borislav Petkov @ 2011-07-26 18:39 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Borislav Petkov, Ray Lee, Linus Torvalds, H. Peter Anvin,
	Thomas Gleixner, LKML, Przywara, Andre, Pohlack, Martin

On Tue, Jul 26, 2011 at 02:34:30PM -0400, Ingo Molnar wrote:
> 
> * Borislav Petkov <bp@amd64.org> wrote:
> 
> > Ok, I went and installed a Debian wheezy with gnome and started 
> > almost everything I could find in the startup menu. Kernel is 3.0 
> > with Linus' simpler fix.
> > 
> > The attached file shows all libraries which would create aliases 
> > (i.e. 2 or more unique values for bits [14:12] along with the 
> > respective count) in the total of:
> > 
> > Total r-xp mappings: 831, aliasing: 240 (0.289%)
> 
> 28.9%, right?

Yessir.

> > For example, libc gets mapped into all possible slots for [14:12]
> > 
> > Library                                 [14:12] count
> > =======                                 ======= =====
> > /lib/libc-2.11.2.so
> >                                         0       12
> >                                         1       12
> >                                         2       11
> >                                         3       12
> >                                         4       13
> >                                         5       16
> >                                         6       13
> >                                         7       9
> > 
> > 
> > and so on.
> 
> Was this done with a stock kernel, or with the simple patch applied?

with the simplest version of Linus' patch:

diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
index 1dab519..6b1094d 100644
--- a/arch/x86/mm/mmap.c
+++ b/arch/x86/mm/mmap.c
@@ -90,6 +90,8 @@ static unsigned long mmap_rnd(void)
                        rnd = (long)get_random_int() % (1<<8);
                else
                        rnd = (long)(get_random_int() % (1<<28));
+
+               rnd &= ~0x7;
        }
        return rnd << PAGE_SHIFT;
 }

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-26 18:39                           ` Borislav Petkov
@ 2011-07-26 18:47                             ` Ingo Molnar
  2011-07-26 19:33                               ` Borislav Petkov
  0 siblings, 1 reply; 46+ messages in thread
From: Ingo Molnar @ 2011-07-26 18:47 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Ray Lee, Linus Torvalds, H. Peter Anvin, Thomas Gleixner, LKML,
	Przywara, Andre, Pohlack, Martin


* Borislav Petkov <bp@amd64.org> wrote:

> > Was this done with a stock kernel, or with the simple patch 
> > applied?
> 
> with the simplest version of Linus' patch:
> 
> diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
> index 1dab519..6b1094d 100644
> --- a/arch/x86/mm/mmap.c
> +++ b/arch/x86/mm/mmap.c
> @@ -90,6 +90,8 @@ static unsigned long mmap_rnd(void)
>                         rnd = (long)get_random_int() % (1<<8);
>                 else
>                         rnd = (long)(get_random_int() % (1<<28));
> +
> +               rnd &= ~0x7;

So, unless i got the stats right, unmodified kernel has like a 75% 
mis-aligned rate, while with the simple fix this goes down to 30%?

Have you run the numbers on the stock, unmodified kernel by any 
chance, just to make sure the stats are ok?

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-26 18:47                             ` Ingo Molnar
@ 2011-07-26 19:33                               ` Borislav Petkov
  0 siblings, 0 replies; 46+ messages in thread
From: Borislav Petkov @ 2011-07-26 19:33 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Borislav Petkov, Ray Lee, Linus Torvalds, H. Peter Anvin,
	Thomas Gleixner, LKML, Przywara, Andre, Pohlack, Martin

[-- Attachment #1: Type: text/plain, Size: 901 bytes --]

On Tue, Jul 26, 2011 at 02:47:44PM -0400, Ingo Molnar wrote:
> So, unless i got the stats right, unmodified kernel has like a 75%
> mis-aligned rate, while with the simple fix this goes down to 30%?

Nah, I don't think this workaround has such an impact. Due
to the differing link order and the workaround tweaking
_only_ the start of the VA space of the process (see
http://marc.info/?l=linux-kernel&m=131163083618197), it actually doesn't
change a lot:

Total r-xp mappings: 902, aliasing: 282 (31.264%)

(I know, I know, those are different numbers but the percentage is in
the same ballpark).

> Have you run the numbers on the stock, unmodified kernel by any 
> chance, just to make sure the stats are ok?

attached.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

[-- Attachment #2: unmodified-results.txt --]
[-- Type: text/plain, Size: 21278 bytes --]

Library					[14:12]	count
=======					=======	=====
/usr/lib/gtk-2.0/2.10.0/engines/libclearlooks.so
					1	1
					2	2
					3	1
					6	2
/usr/lib/libxcb-render-util.so.0.0.0
					0	7
					1	6
					2	5
					3	9
					4	3
					5	11
					6	8
					7	15
/lib/libutil-2.11.2.so
					0	6
					1	3
					2	6
					3	3
					4	1
					5	1
					6	3
					7	4
/usr/lib/libatk-1.0.so.0.3009.1
					0	9
					1	11
					2	9
					3	7
					4	4
					5	7
					6	6
					7	11
[vdso]
					0	7
					1	6
					2	6
					3	8
					4	13
					5	8
					6	8
					7	58
/usr/lib/enchant/libenchant_aspell.so
					1	1
					2	1
					5	1
					6	1
/usr/lib/libical.so.0.44.0
					0	2
					7	2
/usr/lib/libpangocairo-1.0.so.0.2800.3
					0	10
					1	10
					2	5
					3	5
					4	9
					5	12
					6	1
					7	12
/usr/lib/pyshared/python2.6/gtk-2.0/atk.so
					4	2
					6	1
/usr/lib/nss/libsoftokn3.so
					2	1
					7	1
/usr/lib/libICE.so.6.3.0
					0	3
					1	7
					2	7
					3	4
					4	9
					5	5
					6	4
					7	5
/usr/lib/libebook-1.2.so.9.3.1
					0	2
					1	1
					2	1
					4	1
					5	1
					6	1
/usr/lib/libpolkit-gobject-1.so.0.0.0
					2	2
					3	2
					4	2
					5	1
					7	2
/usr/lib/libSM.so.6.0.1
					0	4
					1	5
					2	5
					3	4
					4	6
					5	7
					6	3
					7	10
/usr/lib/libsoup-2.4.so.1.3.0
					1	3
					2	1
					3	2
					5	3
					6	2
					7	2
/usr/lib/libXdamage.so.1.1.0
					0	10
					1	9
					2	10
					3	8
					4	11
					5	5
					6	6
					7	5
/lib/libbz2.so.1.0.4
					0	4
					1	2
					2	5
					3	1
					4	3
					5	4
					6	4
					7	2
/lib/security/pam_limits.so
					1	2
					7	1
/lib/libpthread-2.11.2.so
					0	6
					1	15
					2	15
					3	11
					4	12
					5	12
					6	11
					7	13
/usr/lib/libgailutil.so.18.0.1
					0	1
					1	5
					2	2
					3	2
					4	3
					5	3
					6	1
					7	1
/usr/lib/gstreamer-0.10/libgstossaudio.so
					3	1
					6	1
/lib/security/pam_motd.so
					5	1
					7	2
/usr/lib/libavahi-common.so.3.5.2
					1	4
					2	1
					3	3
					4	3
					5	6
					6	1
					7	2
/usr/lib/liblcms.so.1.0.18
					5	1
					6	1
/usr/lib/libcamel-1.2.so.14.0.1
					1	1
					3	3
					5	1
					6	1
					7	1
/usr/lib/libbonobo-2.so.0.0.0
					0	3
					1	3
					2	1
					3	1
					4	2
					5	1
					7	3
/usr/lib/libtdb.so.1.2.1
					0	5
					1	2
					2	1
					3	1
					5	3
					6	2
					7	3
/usr/lib/libvte.so.9.13.3
					2	1
					7	1
/lib/libcrypt-2.11.2.so
					0	1
					1	4
					2	3
					4	2
					5	3
					7	2
/usr/lib/libX11.so.6.3.0
					0	7
					1	9
					2	7
					3	4
					4	11
					5	7
					6	14
					7	9
/usr/lib/libcairomm-1.0.so.1.4.0
					1	1
					4	1
/usr/lib/libcurl-gnutls.so.4.2.0
					2	1
					4	1
/usr/lib/gtk-2.0/2.10.0/loaders/svg_loader.so
					0	2
					1	2
					3	3
					4	2
					5	1
					7	2
/lib/libnss_files-2.11.2.so
					0	14
					1	9
					2	5
					3	12
					4	13
					5	11
					6	16
					7	11
/usr/lib/libgtkhtml-editor.so.0.0.0
					3	1
					7	1
/lib/security/pam_ck_connector.so
					1	1
					3	2
/usr/lib/liborc-0.4.so.0.0.0
					4	2
					7	1
/usr/lib/libssl3.so.1d
					1	1
					2	1
					3	1
					5	4
					6	1
/lib/libnss_compat-2.11.2.so
					0	8
					1	5
					2	11
					3	14
					4	11
					5	15
					6	11
					7	14
/usr/lib/gtk-2.0/modules/libgail-gnome.so
					0	1
					1	1
					3	1
					4	3
/lib/libdbus-1.so.3.4.0
					0	9
					1	10
					2	9
					3	14
					4	9
					5	7
					6	18
					7	8
/usr/lib/libGL.so.1.2
					0	3
					3	1
					6	1
/usr/lib/libchamplain-gtk-0.4.so.0.3.3
					1	1
					6	1
/usr/lib/libexpat.so.1.5.2
					0	7
					1	5
					2	11
					3	9
					4	9
					5	13
					6	9
					7	9
/usr/lib/libxcb-event.so.1.0.0
					3	2
					4	4
					6	3
					7	2
/usr/lib/libz.so.1.2.3.4
					0	6
					1	9
					2	15
					3	11
					4	11
					5	8
					6	9
					7	17
/usr/lib/libpixman-1.so.0.16.4
					0	12
					1	8
					2	9
					3	5
					4	6
					5	4
					6	14
					7	7
/lib/libresolv-2.11.2.so
					0	15
					1	11
					2	10
					3	11
					4	9
					5	5
					6	9
					7	10
/usr/lib/nss/libfreebl3.so
					2	1
					4	1
/lib/libc-2.11.2.so
					0	14
					1	19
					2	14
					3	19
					4	13
					5	15
					6	11
					7	9
/usr/lib/libnss3.so.1d
					0	1
					1	1
					3	1
					4	1
					5	1
					7	4
/usr/lib/libk5crypto.so.3.1
					0	3
					1	3
					2	2
					3	1
					5	2
					6	1
					7	4
/usr/lib/gconv/ISO8859-1.so
					0	6
					1	12
					2	8
					3	9
					4	5
					5	9
					6	7
					7	4
/lib/security/pam_permit.so
					5	1
					7	2
/usr/lib/libpython2.6.so.1.0
					2	1
					6	1
					7	1
/usr/lib/pyshared/python2.6/gtk-2.0/pango.so
					1	2
					3	1
/usr/lib/enchant/libenchant_hspell.so
					2	1
					3	1
					6	1
					7	1
/usr/lib/libxml2.so.2.7.8
					0	5
					1	9
					2	1
					3	6
					4	5
					5	9
					6	7
					7	5
/usr/lib/libxcb-atom.so.1.0.0
					0	4
					2	3
					3	2
					7	2
/usr/lib/libxkbfile.so.1.0.2
					6	1
					7	1
/usr/lib/libedata-book-1.2.so.2.4.1
					0	1
					5	1
/usr/lib/libgstvideo-0.10.so.0.21.0
					1	1
					2	1
					3	1
					5	1
					6	1
					7	3
/usr/lib/libXau.so.6.0.0
					0	8
					1	9
					2	7
					3	9
					4	8
					5	12
					6	5
					7	11
/usr/lib/libORBit-2.so.0.1.0
					0	12
					1	3
					2	8
					3	9
					4	4
					5	9
					6	10
					7	7
/usr/bin/dbus-daemon
					3	2
					5	1
/usr/lib/libsoup-gnome-2.4.so.1.3.0
					0	1
					2	1
					3	1
					4	1
					5	1
					6	1
					7	1
/usr/lib/librsvg-2.so.2.26.3
					0	6
					1	1
					2	1
					3	3
					4	2
					5	2
					6	4
					7	4
/usr/lib/enchant/libenchant_ispell.so
					0	1
					4	1
					6	1
					7	1
/usr/lib/libjson-glib-1.0.so.0.1000.2
					0	2
					3	1
					6	1
/usr/lib/libgtop-2.0.so.7.2.0
					2	1
					3	1
					4	1
					5	1
/lib/libuuid.so.1.3.0
					0	7
					1	3
					2	6
					3	3
					4	7
					5	5
					6	4
					7	10
/usr/lib/pyshared/python2.6/gtk-2.0/gtk/_gtk.so
					5	1
					6	2
/usr/lib/gtk-2.0/2.10.0/loaders/libpixbufloader-xpm.so
					0	5
					1	2
					2	2
					3	2
					4	1
					5	2
					6	1
					7	3
/usr/lib/libsqlite3.so.0.8.6
					0	2
					2	2
					3	2
					4	2
					5	3
					6	2
					7	1
/usr/lib/libclutter-glx-1.0.so.0.200.12
					4	3
					7	1
/usr/lib/libvorbis.so.0.4.4
					1	3
					2	2
					3	3
					4	5
					5	2
					6	1
					7	1
/usr/lib/libsasl2.so.2.0.23
					1	1
					2	2
					3	1
					4	1
					5	1
/usr/lib/libXrender.so.1.3.0
					0	3
					1	14
					2	6
					3	9
					4	5
					5	6
					6	13
					7	9
/lib/libm-2.11.2.so
					0	12
					1	11
					2	13
					3	5
					4	12
					5	9
					6	6
					7	7
/usr/lib/libtiff.so.4.3.3
					0	1
					2	1
/lib/security/pam_env.so
					1	1
					3	2
/usr/lib/libclutter-gtk-0.10.so.0.0.0
					3	3
					6	1
/usr/lib/libstdc++.so.6.0.13
					0	1
					1	2
					2	1
					3	4
					4	1
					5	2
					6	3
					7	2
/usr/lib/libXext.so.6.4.0
					0	10
					1	11
					2	3
					3	12
					4	5
					5	9
					6	9
					7	6
/usr/lib/libenchant.so.1.6.0
					0	2
					1	2
					2	1
					5	2
					6	1
					7	2
/usr/lib/libgnomeui-2.so.0.2400.3
					0	1
					5	1
/usr/lib/libexif.so.12.3.1
					2	1
					4	1
/lib/libnss_mdns4_minimal.so.2
					0	1
					2	2
					4	3
					6	1
/usr/lib/pyshared/python2.6/gtk-2.0/gconf.so
					4	1
					5	1
					7	1
/usr/lib/libdrm.so.2.4.0
					0	3
					1	1
					4	1
					5	1
/usr/lib/libgdkmm-2.4.so.1.1.0
					0	1
					3	1
/usr/lib/libgdbm.so.3.0.0
					0	1
					4	1
/usr/lib/libdbus-glib-1.so.2.1.0
					0	6
					1	13
					2	9
					3	12
					4	11
					5	7
					6	13
					7	5
/usr/lib/libicalvcal.so.0.44.0
					0	1
					2	1
					3	2
/usr/lib/libgtkmm-2.4.so.1.1.0
					0	1
					2	1
/usr/lib/libtasn1.so.3.1.9
					0	3
					1	3
					2	4
					3	4
					4	4
					5	6
					6	4
					7	4
/lib/libacl.so.1.1.0
					2	1
					3	1
/usr/lib/libgstaudio-0.10.so.0.21.0
					1	1
					4	1
					5	1
					6	1
/usr/lib/gtk-2.0/2.10.0/loaders/libpixbufloader-png.so
					0	7
					1	4
					2	3
					3	3
					4	2
					5	2
					6	1
					7	1
/usr/lib/libXtst.so.6.1.0
					0	1
					3	2
					5	1
					6	1
/usr/lib/libunique-1.0.so.0.100.6
					0	1
					1	1
					3	2
					4	1
					6	2
					7	2
/usr/lib/libldap_r-2.4.so.2.5.6
					1	3
					3	2
					4	1
/usr/lib/pyshared/python2.6/gtk-2.0/gio/unix.so
					2	2
					4	1
/usr/lib/gvfs/libgvfscommon.so
					1	2
					2	1
					4	3
					5	2
					6	2
					7	2
/usr/lib/libegroupwise-1.2.so.13.0.1
					0	1
					5	1
/lib/libdl-2.11.2.so
					0	18
					1	13
					2	9
					3	10
					4	14
					5	10
					6	18
					7	15
/usr/lib/libORBitCosNaming-2.so.0.1.0
					0	4
					1	1
					2	2
					5	3
					6	3
					7	1
/lib/security/pam_nologin.so
					3	1
					5	2
/usr/lib/libproxy.so.0.0.0
					0	1
					2	2
					4	3
					7	1
/usr/lib/libnm-glib.so.2.2.3
					2	1
					3	1
					4	1
					5	1
/usr/lib/libjpeg.so.62.0.0
					0	2
					1	1
					4	1
					5	1
					6	2
					7	4
/lib/libnsl-2.11.2.so
					0	10
					1	7
					2	10
					3	13
					4	12
					5	15
					6	12
					7	12
/usr/lib/libgstinterfaces-0.10.so.0.21.0
					0	1
					2	1
					4	1
					5	1
					7	2
/usr/lib/gstreamer-0.10/libgstgconfelements.so
					0	1
					2	1
/usr/lib/libfontconfig.so.1.4.4
					0	9
					1	6
					2	14
					3	8
					4	7
					5	8
					6	4
					7	8
/usr/lib/libnssutil3.so.1d
					1	1
					3	5
					4	1
					5	1
					7	1
/lib/libpam.so.0.82.2
					0	1
					1	2
					2	1
					4	2
					5	3
					7	1
/usr/lib/libfreetype.so.6.6.0
					0	8
					1	7
					2	7
					3	6
					4	6
					5	12
					6	12
					7	7
/lib/libselinux.so.1
					0	6
					1	10
					2	16
					3	13
					4	8
					5	16
					6	8
					7	10
/usr/lib/libaudiofile.so.0.0.2
					1	2
					3	3
					5	1
					6	2
					7	2
/usr/lib/libgksu2.so.0.0.2
					1	1
					6	1
/usr/lib/libgnome-2.so.0.3000.0
					2	1
					3	2
					4	1
					5	2
					6	3
					7	1
/usr/lib/libgssapi_krb5.so.2.2
					0	5
					1	1
					3	2
					4	2
					5	2
					6	1
					7	3
/usr/lib/libnotify.so.1.2.3
					0	1
					1	1
					2	3
					4	2
					5	1
					6	2
/usr/lib/libXxf86vm.so.1.0.0
					0	3
					1	1
					3	2
					4	1
/usr/lib/libpanel-applet-2.so.0.2.68
					0	1
					1	1
					2	1
					4	1
					5	3
					6	1
/usr/lib/libwnck-1.so.22.3.29
					0	1
					3	1
					6	1
/usr/lib/libnm-util.so.1.3.4
					0	1
					2	1
					4	1
					7	1
/lib/libattr.so.1.1.0
					1	2
					5	2
					6	1
					7	2
/usr/lib/libgdata-google-1.2.so.1.0.0
					2	1
					6	1
/lib/libcom_err.so.2.1
					0	1
					2	3
					3	1
					4	5
					5	4
					6	1
					7	1
/lib/libnss_mdns4.so.2
					0	2
					6	1
/usr/lib/libxcb.so.1.1.0
					0	9
					1	7
					2	7
					3	13
					4	10
					5	5
					6	9
					7	8
/usr/lib/libwebkit-1.0.so.2.17.9
					0	1
					1	1
					4	1
					6	1
					7	1
/usr/lib/libbrasero-utils.so.0.2.0
					2	1
					6	1
/usr/lib/libxcb-render.so.0.0.0
					0	7
					1	6
					2	5
					3	9
					4	3
					5	11
					6	8
					7	15
/usr/lib/libcups.so.2
					0	1
					1	1
					6	1
/usr/lib/libgdk_pixbuf-2.0.so.0.2000.1
					0	4
					1	9
					2	9
					3	9
					4	14
					5	4
					6	7
					7	8
/usr/lib/libbonobo-activation.so.4.0.0
					0	1
					1	2
					2	1
					4	3
					5	3
					6	2
					7	2
/usr/lib/libXi.so.6.1.0
					0	13
					1	5
					2	9
					3	6
					4	6
					5	12
					6	10
					7	4
/usr/lib/libupower-glib.so.1.0.1
					3	1
					4	1
					7	2
/lib/security/pam_unix.so
					1	2
					7	1
/lib/libusb-0.1.so.4.4.4
					4	1
					7	1
/usr/lib/libgmime-2.4.so.2.4.14
					1	1
					4	1
					6	1
					7	1
/usr/lib/libavahi-client.so.3.2.7
					1	2
					2	1
					3	3
					4	3
					5	6
					6	1
					7	2
/usr/lib/libXss.so.1.0.0
					2	1
					7	1
/lib/libglib-2.0.so.0.2400.2
					0	11
					1	7
					2	9
					3	12
					4	7
					5	11
					6	14
					7	8
/usr/lib/libgtkspell.so.0.0.0
					1	1
					5	1
/lib/security/pam_deny.so
					0	2
					6	1
/usr/lib/enchant/libenchant_myspell.so
					1	1
					2	1
					5	1
					6	1
/usr/lib/libXrandr.so.2.2.0
					0	13
					1	5
					2	10
					3	7
					4	6
					5	10
					6	9
					7	4
/usr/lib/libffi.so.5.0.10
					2	2
					3	2
/usr/lib/libnl.so.1.1
					1	1
					5	1
/lib/libwrap.so.0.7.6
					0	2
					1	1
					2	2
					5	1
					6	1
/usr/lib/libv4l2.so.0
					1	1
					7	1
/usr/sbin/sshd
					1	2
					2	1
					7	1
/usr/lib/libgimpmodule-2.0.so.0.600.10
					2	1
					5	1
/usr/lib/libck-connector.so.0.0.0
					0	2
					6	1
/usr/lib/libgnome-desktop-2.so.17.1.0
					3	2
					4	1
					7	2
/usr/lib/gio/modules/libgvfsdbus.so
					0	1
					5	2
/usr/lib/libpangoft2-1.0.so.0.2800.3
					0	10
					1	12
					2	7
					3	10
					4	7
					5	6
					6	6
					7	6
/lib/libudev.so.0.9.3
					0	2
					1	2
					2	1
					3	2
					6	5
					7	1
/usr/lib/libbonoboui-2.so.0.0.0
					0	2
					1	2
					2	1
					4	1
					5	4
/usr/lib/libv4lconvert.so.0
					1	1
					3	1
/usr/lib/libvorbisfile.so.3.3.2
					0	5
					1	2
					2	1
					3	1
					5	3
					6	2
					7	3
/usr/lib/libgobject-2.0.so.0.2400.2
					0	10
					1	12
					2	10
					3	14
					4	7
					5	12
					6	9
					7	5
/usr/lib/libgcrypt.so.11.5.3
					0	3
					1	7
					2	4
					3	7
					4	4
					5	3
					6	5
					7	4
/lib/libpcre.so.3.12.1
					0	10
					1	8
					2	6
					3	8
					4	10
					5	9
					6	17
					7	12
/usr/lib/libesd.so.0.2.39
					1	2
					3	3
					5	1
					6	2
					7	2
/usr/lib/libgimpmath-2.0.so.0.600.10
					3	1
					7	1
/usr/lib/libhunspell-1.2.so.0.0.0
					0	1
					3	1
					4	1
					5	1
					7	1
/lib/libaudit.so.0.0.0
					3	1
					4	1
					5	1
/usr/lib/libicalss.so.0.44.0
					4	1
					6	1
					7	2
/lib/libcap.so.2.19
					2	2
					6	1
/usr/lib/libXinerama.so.1.0.0
					0	5
					1	9
					2	6
					3	6
					4	11
					5	10
					6	4
					7	13
/usr/lib/libxklavier.so.16.0.0
					5	1
					7	1
/usr/lib/libgimpwidgets-2.0.so.0.600.10
					1	1
					7	1
/usr/lib/libgimpbase-2.0.so.0.600.10
					1	1
					3	1
/usr/lib/libedataserver-1.2.so.13.0.1
					1	2
					2	2
					4	2
					5	1
/usr/lib/libicuuc.so.44.1
					1	1
					3	1
					6	3
					7	1
/lib/libnss_nis-2.11.2.so
					0	13
					1	11
					2	15
					3	11
					4	14
					5	8
					6	5
					7	12
/usr/lib/liblber-2.4.so.2.5.6
					2	1
					3	2
					4	1
					5	1
					6	1
/usr/lib/libkrb5.so.3.3
					0	2
					1	1
					3	3
					4	1
					5	3
					6	3
					7	3
/lib/libkeyutils.so.1.3
					0	2
					2	5
					3	3
					4	2
					5	2
					7	2
/lib/libpng12.so.0.44.0
					0	12
					1	9
					2	13
					3	8
					4	7
					5	4
					6	8
					7	3
/usr/lib/libpyglib-2.0-python2.6.so.0.0.0
					2	2
					3	1
/usr/lib/libglibmm-2.4.so.1.3.0
					2	1
					4	1
/usr/lib/libnspr4.so.0d
					0	4
					2	3
					6	2
/usr/lib/libgnutls.so.26.14.12
					0	5
					1	5
					2	5
					3	6
					4	5
					5	1
					6	3
					7	2
/usr/lib/libebackend-1.2.so.0.0.1
					2	1
					3	1
/usr/lib/libart_lgpl_2.so.2.3.21
					1	2
					3	2
					4	1
					5	2
					6	5
/usr/lib/libchamplain-0.4.so.0.3.3
					2	1
					7	1
/usr/lib/libcamel-provider-1.2.so.14.0.1
					0	1
					1	1
/usr/lib/libgsttag-0.10.so.0.21.0
					5	1
					7	1
/usr/lib/lapack/liblapack.so.3gf.0
					0	1
					3	1
/usr/lib/libgnome-keyring.so.0.1.1
					0	1
					1	4
					2	1
					4	5
					5	1
					6	3
					7	1
/lib/librt-2.11.2.so
					0	9
					1	12
					2	12
					3	9
					4	12
					5	8
					6	9
					7	21
/usr/lib/pyshared/python2.6/gtk-2.0/glib/_glib.so
					5	1
					6	2
/usr/lib/evolution/2.30/libeutil.so.0.0.0
					0	1
					1	1
/usr/lib/libXfixes.so.3.1.0
					0	5
					1	7
					2	4
					3	12
					4	8
					5	10
					6	10
					7	9
/lib/security/pam_gnome_keyring.so
					5	1
					7	2
/usr/lib/libtelepathy-glib.so.0.40.0
					0	1
					2	1
					5	1
/usr/lib/pango/1.6.0/modules/pango-basic-fc.so
					0	5
					1	10
					2	6
					3	2
					4	3
					5	5
					6	9
					7	6
/usr/lib/libgudev-1.0.so.0.0.1
					0	1
					1	1
					2	2
					4	2
					5	1
					7	1
/usr/lib/libgsf-1.so.114.0.18
					0	3
					1	4
					2	3
					3	2
					4	6
					5	2
					6	1
					7	2
/lib/libncurses.so.5.7
					0	1
					1	2
					2	1
					4	3
					6	1
					7	1
/usr/lib/libgstpbutils-0.10.so.0.21.0
					1	2
					3	2
					6	1
					7	3
/usr/lib/pyshared/python2.6/gtk-2.0/gobject/_gobject.so
					6	2
					7	1
/usr/lib/libplds4.so.0d
					0	3
					2	4
					4	2
/usr/lib/libgpg-error.so.0.4.0
					0	4
					1	4
					2	4
					3	2
					4	8
					5	3
					6	4
					7	8
/usr/lib/libsmime3.so.1d
					1	1
					3	1
					4	1
					5	4
					6	1
/usr/lib/gstreamer-0.10/libgstalsa.so
					0	1
					6	1
/usr/lib/gio/modules/libgioremote-volume-monitor.so
					0	1
					2	2
					3	1
					5	1
					6	2
					7	1
/usr/lib/libgnomecanvas-2.so.0.3000.1
					2	2
					4	2
					5	2
					6	2
					7	4
/usr/lib/libgstfarsight-0.10.so.0.5.0
					2	1
					5	1
/usr/lib/libcanberra.so.0.2.3
					0	6
					1	1
					2	1
					3	1
					5	5
					6	1
					7	2
/usr/lib/pyshared/python2.6/cairo/_cairo.so
					3	1
					4	1
					7	1
/usr/lib/libcrypto.so.0.9.8
					0	3
					1	1
					2	1
					3	3
					4	2
					5	3
					6	2
/usr/lib/libatkmm-1.6.so.1.1.0
					4	1
					5	1
/usr/lib/libcairo.so.2.10800.10
					0	17
					1	2
					2	4
					3	8
					4	4
					5	8
					6	9
					7	12
/usr/lib/libaspell.so.15.1.4
					0	1
					3	1
					4	1
					7	2
/usr/lib/libgstreamer-0.10.so.0.26.0
					0	1
					1	3
					2	2
					5	3
					7	1
/usr/lib/gtk-2.0/modules/libgail.so
					0	1
					1	1
					3	1
					4	2
					5	1
/usr/lib/libXv.so.1.0.0
					3	1
					6	1
/usr/lib/libgconf2-4/2/libgconfbackend-xml.so
					5	1
					6	1
/usr/lib/libgiomm-2.4.so.1.3.0
					1	1
					4	1
/usr/lib/libXt.so.6.0.0
					2	2
					3	1
					4	1
					5	2
					7	2
/usr/lib/libsigc-2.0.so.0.0.0
					0	1
					4	1
					6	1
/usr/lib/libgeoclue.so.0.0.0
					4	1
					7	1
/usr/lib/libgdk-x11-2.0.so.0.2000.1
					0	13
					1	8
					2	12
					3	7
					4	8
					5	6
					6	7
					7	3
/usr/lib/libgtkhtml-3.14.so.19.1.1
					1	1
					2	1
/usr/lib/libXRes.so.1.0.0
					3	1
					4	1
					6	1
/usr/lib/libidn.so.11.5.44
					2	1
					4	1
/usr/lib/libgthread-2.0.so.0.2400.2
					0	6
					1	11
					2	15
					3	10
					4	12
					5	6
					6	7
					7	12
/usr/lib/libgstbase-0.10.so.0.26.0
					0	1
					1	1
					2	2
					3	1
					4	3
					5	1
/usr/lib/libgio-2.0.so.0.2400.2
					0	4
					1	10
					2	11
					3	8
					4	9
					5	13
					6	12
					7	6
/usr/lib/libedataserverui-1.2.so.8.1.1
					3	1
					5	1
					6	1
/usr/lib/libkrb5support.so.0.1
					1	2
					2	3
					4	5
					5	3
					6	1
					7	2
/usr/lib/libgdata-1.2.so.1.0.0
					3	1
					7	1
/usr/lib/libcroco-0.6.so.3.0.1
					0	2
					1	6
					2	2
					3	1
					4	2
					5	3
					6	4
					7	3
/usr/lib/libogg.so.0.7.0
					0	1
					1	2
					3	3
					4	2
					5	3
					6	5
					7	2
/usr/lib/dri/swrast_dri.so
					1	1
					3	1
					5	1
					6	1
/usr/lib/libgtk-x11-2.0.so.0.2000.1
					0	7
					1	6
					2	9
					3	6
					4	4
					5	13
					6	8
					7	11
/usr/lib/libavahi-glib.so.1.0.2
					0	2
					1	3
					2	1
					4	3
					5	2
					6	1
					7	3
/usr/lib/libdb-4.8.so
					0	1
					1	1
					5	1
					6	2
/usr/lib/libeggdbus-1.so.0.0.0
					0	3
					2	1
					4	2
					5	2
					7	1
/usr/lib/libicui18n.so.44.1
					0	2
					2	1
					5	2
					6	1
/usr/lib/libssl.so.0.9.8
					1	3
					3	1
					4	2
					5	2
					7	1
/usr/lib/libXcursor.so.1.0.2
					0	9
					1	6
					2	5
					3	13
					4	10
					5	4
					6	13
					7	5
/usr/lib/libXcomposite.so.1.0.0
					0	7
					1	4
					2	11
					3	7
					4	10
					5	9
					6	9
					7	7
/usr/lib/nss/libnssckbi.so
					3	1
					5	1
/usr/lib/libltdl.so.7.2.1
					0	2
					1	1
					2	1
					4	3
					5	3
					6	4
					7	5
/usr/lib/libtelepathy-farsight.so.0.1.1
					3	1
					6	1
/usr/lib/libcanberra-gtk.so.0.1.6
					0	6
					1	1
					2	1
					3	1
					5	5
					6	1
					7	2
/lib/libpopt.so.0.0.0
					0	3
					1	2
					2	1
					3	2
					6	1
					7	2
/usr/lib/libgnomevfs-2.so.0.2400.3
					0	1
					1	3
					2	1
					3	1
					4	1
					5	1
					6	3
/usr/lib/python2.6/lib-dynload/pyexpat.so
					2	1
					7	1
/usr/lib/gstreamer-0.10/libgstplaybin.so
					1	1
					7	1
/usr/lib/libavahi-ui.so.0.1.2
					1	1
					3	1
/usr/lib/libgstapp-0.10.so.0.21.0
					1	1
					4	1
					5	3
/usr/lib/libpango-1.0.so.0.2800.3
					0	2
					1	8
					2	10
					3	10
					4	6
					5	12
					6	12
					7	4
/usr/lib/libgconf-2.so.4.1.5
					0	10
					1	2
					2	3
					3	10
					4	5
					5	11
					6	12
					7	7
/usr/lib/gtk-2.0/modules/libatk-bridge.so
					2	1
					3	1
					5	1
					6	2
					7	1
/usr/lib/libasound.so.2.0.0
					1	3
					3	2
					4	1
					5	3
					6	2
					7	1
/lib/libgcc_s.so.1
					0	3
					1	4
					2	1
					3	2
					4	4
					5	4
					6	1
/usr/lib/pyshared/python2.6/gtk-2.0/pangocairo.so
					1	1
					7	2
/usr/lib/libstartup-notification-1.so.0.0.0
					0	1
					2	2
					3	1
					5	3
					6	2
					7	2
/lib/ld-2.11.2.so
					0	8
					1	21
					2	17
					3	11
					4	8
					5	16
					6	15
					7	18
/usr/lib/libblas/libblas.so.3gf.0
					2	1
					7	1
/usr/lib/gstreamer-0.10/libgstcoreelements.so
					4	1
					6	1
/usr/lib/libgfortran.so.3.0.0
					3	1
					7	1
/lib/security/pam_mail.so
					3	1
					5	2
/usr/lib/libXdmcp.so.6.0.0
					0	13
					1	6
					2	12
					3	8
					4	8
					5	7
					6	8
					7	7
/usr/lib/libspi.so.0.10.11
					0	1
					3	1
					4	1
					6	1
					7	3
/usr/lib/libxcb-aux.so.0.0.0
					0	4
					2	3
					3	2
					7	2
/usr/lib/libxslt.so.1.1.26
					0	1
					1	1
					2	1
					3	1
					4	2
					5	1
/usr/lib/libgmodule-2.0.so.0.2400.2
					0	9
					1	5
					2	7
					3	7
					4	8
					5	11
					6	15
					7	16
/usr/lib/libtotem-plparser.so.17.0.0
					0	2
					4	1
/usr/lib/pyshared/python2.6/gtk-2.0/gio/_gio.so
					1	1
					2	1
					4	1
/usr/lib/libplc4.so.0d
					0	3
					4	2
					6	4
/lib/libnss_dns-2.11.2.so
					0	1
					2	2
					4	2
					6	2
/usr/lib/libecal-1.2.so.7.2.2
					5	1
					6	1
					7	2
Total r-xp mappings: 902, aliasing: 282 (31.264%)

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-26 18:38         ` H. Peter Anvin
@ 2011-07-26 19:42           ` Andre Przywara
  2011-07-26 22:34             ` H. Peter Anvin
  2011-07-27  4:14             ` Avi Kivity
  0 siblings, 2 replies; 46+ messages in thread
From: Andre Przywara @ 2011-07-26 19:42 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Borislav Petkov, Avi Kivity, Ingo Molnar, Thomas Gleixner, LKML,
	Pohlack, Martin

On 07/26/2011 08:38 PM, H. Peter Anvin wrote:
> On 07/26/2011 11:37 AM, Borislav Petkov wrote:
>>>
>>> I think the question was the width (and position) for the mask... i.e.
>>> your [14:12] above which *is* magic.
>>
>> Oh, that's easy: family 15h means bits [14:12] - those bits are used for
>> I$ index generation.
>>
>
> In other words, it's completely ad hoc.

There is no need to determine it by calculating, because it caused by 
the special design of the BD L1 cache and thus fixed.
And a calculation would be even more confusing:

The L1I is virtually indexed, but physically tagged.
64KB L1I cache, 64 Bytes per Cacheline = 1024 cache lines
1024 lines / 2 way associative = 512 indexes
64 Bytes per Cacheline (6 bits) + 512 indexes (9 bits) = bits [14:0]
virtual and physical addresses are the same for bits [11:0], which 
leaves the remaining 14:12 susceptible for aliasing.

So bit 12 comes from PAGESIZE and yes, the 14 could be derived from the 
CPUID cache info, but I don't see much value in breaking this down this way.
But I agree that there should be some comment in the patch which at 
least notes that bits [14:12] are due to the L1I design, maybe we can 
copy a nicer version of the above math in the commit message for reference.

Regards,
Andre.

-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-26 19:42           ` Andre Przywara
@ 2011-07-26 22:34             ` H. Peter Anvin
  2011-07-27  4:14             ` Avi Kivity
  1 sibling, 0 replies; 46+ messages in thread
From: H. Peter Anvin @ 2011-07-26 22:34 UTC (permalink / raw)
  To: Andre Przywara
  Cc: Borislav Petkov, Avi Kivity, Ingo Molnar, Thomas Gleixner, LKML,
	Pohlack, Martin

On 07/26/2011 12:42 PM, Andre Przywara wrote:
>>
>> In other words, it's completely ad hoc.
> 
> There is no need to determine it by calculating, because it caused by 
> the special design of the BD L1 cache and thus fixed.
> And a calculation would be even more confusing:
> 
> The L1I is virtually indexed, but physically tagged.
> 64KB L1I cache, 64 Bytes per Cacheline = 1024 cache lines
> 1024 lines / 2 way associative = 512 indexes
> 64 Bytes per Cacheline (6 bits) + 512 indexes (9 bits) = bits [14:0]
> virtual and physical addresses are the same for bits [11:0], which 
> leaves the remaining 14:12 susceptible for aliasing.
> 
> So bit 12 comes from PAGESIZE and yes, the 14 could be derived from the 
> CPUID cache info, but I don't see much value in breaking this down this way.
> But I agree that there should be some comment in the patch which at 
> least notes that bits [14:12] are due to the L1I design, maybe we can 
> copy a nicer version of the above math in the commit message for reference.
> 

The right thing to do this would be to have a bit in CPUID which would
indicate that the kernel has to perform the above calculation -- because
obviously not all CPUs have this issue.  From that we can calculate a
mask to insert into whatever appropriate location... depending on the
exact tack we have to apply to deal with this situation.

	-hpa


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-26 19:42           ` Andre Przywara
  2011-07-26 22:34             ` H. Peter Anvin
@ 2011-07-27  4:14             ` Avi Kivity
  2011-07-27  6:21               ` Borislav Petkov
  1 sibling, 1 reply; 46+ messages in thread
From: Avi Kivity @ 2011-07-27  4:14 UTC (permalink / raw)
  To: Andre Przywara
  Cc: H. Peter Anvin, Borislav Petkov, Ingo Molnar, Thomas Gleixner,
	LKML, Pohlack, Martin

On 07/26/2011 10:42 PM, Andre Przywara wrote:
>
> There is no need to determine it by calculating, because it caused by 
> the special design of the BD L1 cache and thus fixed.
> And a calculation would be even more confusing:
>
> The L1I is virtually indexed, but physically tagged.
> 64KB L1I cache, 64 Bytes per Cacheline = 1024 cache lines
> 1024 lines / 2 way associative = 512 indexes
> 64 Bytes per Cacheline (6 bits) + 512 indexes (9 bits) = bits [14:0]
> virtual and physical addresses are the same for bits [11:0], which 
> leaves the remaining 14:12 susceptible for aliasing.
>
> So bit 12 comes from PAGESIZE and yes, the 14 could be derived from 
> the CPUID cache info, but I don't see much value in breaking this down 
> this way.
> But I agree that there should be some comment in the patch which at 
> least notes that bits [14:12] are due to the L1I design, maybe we can 
> copy a nicer version of the above math in the commit message for 
> reference.
>

If among the 12,432.8 cpuid leaves exposed by the cpu we had a bit that 
said L1I was shared, and another that said it was virtually indexed, and 
others describing the cache size, cache line size, and number of ways, 
then we could perform the arithmetic at runtime, yes?

That means that if the caches grow or increase their associativity, then 
we don't need to patch the kernel again.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-27  4:14             ` Avi Kivity
@ 2011-07-27  6:21               ` Borislav Petkov
  2011-07-27  6:59                 ` Ingo Molnar
  0 siblings, 1 reply; 46+ messages in thread
From: Borislav Petkov @ 2011-07-27  6:21 UTC (permalink / raw)
  To: Avi Kivity
  Cc: Przywara, Andre, H. Peter Anvin, Borislav Petkov, Ingo Molnar,
	Thomas Gleixner, LKML, Pohlack, Martin

On Wed, Jul 27, 2011 at 12:14:26AM -0400, Avi Kivity wrote:
> On 07/26/2011 10:42 PM, Andre Przywara wrote:
> >
> > There is no need to determine it by calculating, because it caused by 
> > the special design of the BD L1 cache and thus fixed.
> > And a calculation would be even more confusing:
> >
> > The L1I is virtually indexed, but physically tagged.
> > 64KB L1I cache, 64 Bytes per Cacheline = 1024 cache lines
> > 1024 lines / 2 way associative = 512 indexes
> > 64 Bytes per Cacheline (6 bits) + 512 indexes (9 bits) = bits [14:0]
> > virtual and physical addresses are the same for bits [11:0], which 
> > leaves the remaining 14:12 susceptible for aliasing.
> >
> > So bit 12 comes from PAGESIZE and yes, the 14 could be derived from 
> > the CPUID cache info, but I don't see much value in breaking this down 
> > this way.
> > But I agree that there should be some comment in the patch which at 
> > least notes that bits [14:12] are due to the L1I design, maybe we can 
> > copy a nicer version of the above math in the commit message for 
> > reference.
> >
> 
> If among the 12,432.8 cpuid leaves exposed by the cpu we had a bit that 
> said L1I was shared, and another that said it was virtually indexed, and 
> others describing the cache size, cache line size, and number of ways, 
> then we could perform the arithmetic at runtime, yes?
> 
> That means that if the caches grow or increase their associativity, then 
> we don't need to patch the kernel again.

Yeah, actually the idea is to patch the kernel only this one time and
never ever be needing to do this for future CPUs, for this matter.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-27  6:21               ` Borislav Petkov
@ 2011-07-27  6:59                 ` Ingo Molnar
  2011-07-27  9:30                   ` Avi Kivity
  0 siblings, 1 reply; 46+ messages in thread
From: Ingo Molnar @ 2011-07-27  6:59 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Avi Kivity, Przywara, Andre, H. Peter Anvin, Thomas Gleixner,
	LKML, Pohlack, Martin


* Borislav Petkov <bp@amd64.org> wrote:

> On Wed, Jul 27, 2011 at 12:14:26AM -0400, Avi Kivity wrote:
> > On 07/26/2011 10:42 PM, Andre Przywara wrote:
> > >
> > > There is no need to determine it by calculating, because it caused by 
> > > the special design of the BD L1 cache and thus fixed.
> > > And a calculation would be even more confusing:
> > >
> > > The L1I is virtually indexed, but physically tagged.
> > > 64KB L1I cache, 64 Bytes per Cacheline = 1024 cache lines
> > > 1024 lines / 2 way associative = 512 indexes
> > > 64 Bytes per Cacheline (6 bits) + 512 indexes (9 bits) = bits [14:0]
> > > virtual and physical addresses are the same for bits [11:0], which 
> > > leaves the remaining 14:12 susceptible for aliasing.
> > >
> > > So bit 12 comes from PAGESIZE and yes, the 14 could be derived from 
> > > the CPUID cache info, but I don't see much value in breaking this down 
> > > this way.
> > > But I agree that there should be some comment in the patch which at 
> > > least notes that bits [14:12] are due to the L1I design, maybe we can 
> > > copy a nicer version of the above math in the commit message for 
> > > reference.
> > >
> > 
> > If among the 12,432.8 cpuid leaves exposed by the cpu we had a bit that 
> > said L1I was shared, and another that said it was virtually indexed, and 
> > others describing the cache size, cache line size, and number of ways, 
> > then we could perform the arithmetic at runtime, yes?
> > 
> > That means that if the caches grow or increase their associativity, then 
> > we don't need to patch the kernel again.
> 
> Yeah, actually the idea is to patch the kernel only this one time 
> and never ever be needing to do this for future CPUs, for this 
> matter.

But the check you have added turns this workaround on for all family 
0x15 CPUs. If a family 0x15 CPU is built with a larger or more 
associative cache, one that does not need the workaround, we would 
still turn on the workaround.

Or is it impossible that family 0x15 CPUs will ever be fixed?

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-27  6:59                 ` Ingo Molnar
@ 2011-07-27  9:30                   ` Avi Kivity
  2011-07-27 15:37                     ` Borislav Petkov
  0 siblings, 1 reply; 46+ messages in thread
From: Avi Kivity @ 2011-07-27  9:30 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Borislav Petkov, Przywara, Andre, H. Peter Anvin,
	Thomas Gleixner, LKML, Pohlack, Martin

On 07/27/2011 09:59 AM, Ingo Molnar wrote:
> >
> >  Yeah, actually the idea is to patch the kernel only this one time
> >  and never ever be needing to do this for future CPUs, for this
> >  matter.
>
> But the check you have added turns this workaround on for all family
> 0x15 CPUs. If a family 0x15 CPU is built with a larger or more
> associative cache, one that does not need the workaround, we would
> still turn on the workaround.

Similarly, if family 0x21 CPUs have a similar arrangement (with 
identical or different cache sizes and associativity) the hardcoded 
check will fail.

-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-27  9:30                   ` Avi Kivity
@ 2011-07-27 15:37                     ` Borislav Petkov
  2011-07-27 15:45                       ` Avi Kivity
  0 siblings, 1 reply; 46+ messages in thread
From: Borislav Petkov @ 2011-07-27 15:37 UTC (permalink / raw)
  To: Avi Kivity
  Cc: Ingo Molnar, Borislav Petkov, Przywara, Andre, H. Peter Anvin,
	Thomas Gleixner, LKML, Pohlack, Martin

On Wed, Jul 27, 2011 at 05:30:56AM -0400, Avi Kivity wrote:
> On 07/27/2011 09:59 AM, Ingo Molnar wrote:
> > >
> > >  Yeah, actually the idea is to patch the kernel only this one time
> > >  and never ever be needing to do this for future CPUs, for this
> > >  matter.
> >
> > But the check you have added turns this workaround on for all family
> > 0x15 CPUs. If a family 0x15 CPU is built with a larger or more
> > associative cache, one that does not need the workaround, we would
> > still turn on the workaround.
> 
> Similarly, if family 0x21 CPUs have a similar arrangement (with 
> identical or different cache sizes and associativity) the hardcoded 
> check will fail.

So, in short, I$ design is the same for all F15h processors so a family
0x15 check suffices here.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-27 15:37                     ` Borislav Petkov
@ 2011-07-27 15:45                       ` Avi Kivity
  2011-07-27 15:49                         ` Borislav Petkov
  0 siblings, 1 reply; 46+ messages in thread
From: Avi Kivity @ 2011-07-27 15:45 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Ingo Molnar, Przywara, Andre, H. Peter Anvin, Thomas Gleixner,
	LKML, Pohlack, Martin

On 07/27/2011 06:37 PM, Borislav Petkov wrote:
> On Wed, Jul 27, 2011 at 05:30:56AM -0400, Avi Kivity wrote:
> >  On 07/27/2011 09:59 AM, Ingo Molnar wrote:
> >  >  >
> >  >  >   Yeah, actually the idea is to patch the kernel only this one time
> >  >  >   and never ever be needing to do this for future CPUs, for this
> >  >  >   matter.
> >  >
> >  >  But the check you have added turns this workaround on for all family
> >  >  0x15 CPUs. If a family 0x15 CPU is built with a larger or more
> >  >  associative cache, one that does not need the workaround, we would
> >  >  still turn on the workaround.
> >
> >  Similarly, if family 0x21 CPUs have a similar arrangement (with
> >  identical or different cache sizes and associativity) the hardcoded
> >  check will fail.
>
> So, in short, I$ design is the same for all F15h processors so a family
> 0x15 check suffices here.

What about F21h processors?  I see you're not checking them.

Is there any reason the check should not be made based on topology?

-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-27 15:45                       ` Avi Kivity
@ 2011-07-27 15:49                         ` Borislav Petkov
  2011-07-27 15:57                           ` Avi Kivity
  0 siblings, 1 reply; 46+ messages in thread
From: Borislav Petkov @ 2011-07-27 15:49 UTC (permalink / raw)
  To: Avi Kivity
  Cc: Borislav Petkov, Ingo Molnar, Przywara, Andre, H. Peter Anvin,
	Thomas Gleixner, LKML, Pohlack, Martin

On Wed, Jul 27, 2011 at 11:45:00AM -0400, Avi Kivity wrote:
> What about F21h processors?  I see you're not checking them.
> 
> Is there any reason the check should not be made based on topology?

Because this patch addresses a performance improvement for the F15h
microarchitecture - that doesn't have anything to do with cache
topology.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-27 15:49                         ` Borislav Petkov
@ 2011-07-27 15:57                           ` Avi Kivity
  2011-07-27 16:42                             ` Borislav Petkov
  0 siblings, 1 reply; 46+ messages in thread
From: Avi Kivity @ 2011-07-27 15:57 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Ingo Molnar, Przywara, Andre, H. Peter Anvin, Thomas Gleixner,
	LKML, Pohlack, Martin

On 07/27/2011 06:49 PM, Borislav Petkov wrote:
> On Wed, Jul 27, 2011 at 11:45:00AM -0400, Avi Kivity wrote:
> >  What about F21h processors?  I see you're not checking them.
> >
> >  Is there any reason the check should not be made based on topology?
>
> Because this patch addresses a performance improvement for the F15h
> microarchitecture - that doesn't have anything to do with cache
> topology.

Okay.

Out of curiosity, what's the performance impact if the workaround is not 
enabled?

-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-27 15:57                           ` Avi Kivity
@ 2011-07-27 16:42                             ` Borislav Petkov
  0 siblings, 0 replies; 46+ messages in thread
From: Borislav Petkov @ 2011-07-27 16:42 UTC (permalink / raw)
  To: Avi Kivity
  Cc: Borislav Petkov, Ingo Molnar, Przywara, Andre, H. Peter Anvin,
	Thomas Gleixner, LKML, Pohlack, Martin

On Wed, Jul 27, 2011 at 11:57:45AM -0400, Avi Kivity wrote:
> Out of curiosity, what's the performance impact if the workaround is
> not enabled?

Up to 3% for a CPU-intensive style benchmark, and it can vary highly in
a microbenchmark depending on workload and compiler.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-24 17:39     ` Linus Torvalds
  2011-07-24 18:12       ` Ingo Molnar
  2011-07-24 18:23       ` Borislav Petkov
@ 2011-07-27 17:10       ` Borislav Petkov
  2011-07-27 17:16         ` H. Peter Anvin
  2 siblings, 1 reply; 46+ messages in thread
From: Borislav Petkov @ 2011-07-27 17:10 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Borislav Petkov, H. Peter Anvin, Ingo Molnar, Thomas Gleixner,
	LKML, Przywara, Andre, Pohlack, Martin

On Sun, Jul 24, 2011 at 01:39:25PM -0400, Linus Torvalds wrote:
> > Yeah, I like the BITS() thing - will change. I actually have a similar
> > macro GENMASK(o, hi) in <drivers/edac/amd64_edac.h> - I should move it
> > to <linux/bitops.h> and rename it to BITS().
> 
> So it may be that BITS() is much too generic a name, and will cause
> problems. A quick "git grep -w BITS" certainly finds a fair number of
> hits. So I don't think it's usable as-is, it was meant more as
> pseudo-code.

How about something like the following instead - it should take care
of all your bitmask generating needs. There are also a couple of
GENMASK/BITMASK identical definitions around the tree which can be
unified while I'm at it too.

diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index a3ef66a..b1970e3 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -6,6 +6,19 @@
 #define BIT(nr)                        (1UL << (nr))
 #define BIT_MASK(nr)           (1UL << ((nr) % BITS_PER_LONG))
 #define BIT_WORD(nr)           ((nr) / BITS_PER_LONG)
+
+/*
+ * Create a contiguous bitmask starting at bit position @lo and ending at
+ * position @hi. For example
+ *
+ * GENMASK_ULL(21, 39) gives us the 64bit vector 0x000000ffffe00000.
+ */
+#define _GENMASK_T(cast, type, lo, hi) \
+       (((cast)(1##type << ((hi) - (lo) + 1)) - 1) << (lo))
+#define GENMASK(lo, hi)                _GENMASK_T(unsigned, U, lo, hi)
+#define GENMASK_UL(lo, hi)     _GENMASK_T(unsigned long, UL, lo, hi)
+#define GENMASK_ULL(lo, hi)    _GENMASK_T(unsigned long long, ULL, lo, hi)
+
 #define BITS_PER_BYTE          8
 #define BITS_TO_LONGS(nr)      DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
 #endif

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-27 17:10       ` Borislav Petkov
@ 2011-07-27 17:16         ` H. Peter Anvin
  2011-07-28 13:44           ` Borislav Petkov
  0 siblings, 1 reply; 46+ messages in thread
From: H. Peter Anvin @ 2011-07-27 17:16 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Linus Torvalds, Ingo Molnar, Thomas Gleixner, LKML, Przywara,
	Andre, Pohlack, Martin

On 07/27/2011 10:10 AM, Borislav Petkov wrote:
> 
> How about something like the following instead - it should take care
> of all your bitmask generating needs. There are also a couple of
> GENMASK/BITMASK identical definitions around the tree which can be
> unified while I'm at it too.
> 
> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> index a3ef66a..b1970e3 100644
> --- a/include/linux/bitops.h
> +++ b/include/linux/bitops.h
> @@ -6,6 +6,19 @@
>  #define BIT(nr)                        (1UL << (nr))
>  #define BIT_MASK(nr)           (1UL << ((nr) % BITS_PER_LONG))
>  #define BIT_WORD(nr)           ((nr) / BITS_PER_LONG)
> +
> +/*
> + * Create a contiguous bitmask starting at bit position @lo and ending at
> + * position @hi. For example
> + *
> + * GENMASK_ULL(21, 39) gives us the 64bit vector 0x000000ffffe00000.
> + */
> +#define _GENMASK_T(cast, type, lo, hi) \
> +       (((cast)(1##type << ((hi) - (lo) + 1)) - 1) << (lo))
> +#define GENMASK(lo, hi)                _GENMASK_T(unsigned, U, lo, hi)
> +#define GENMASK_UL(lo, hi)     _GENMASK_T(unsigned long, UL, lo, hi)
> +#define GENMASK_ULL(lo, hi)    _GENMASK_T(unsigned long long, ULL, lo, hi)
> +

These really need to be usable from assembly language, too (in which
case you of course need to not have the cast and suffix), so it probably
should be defined in <linux/const.h> with the other constant macros.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-27 17:16         ` H. Peter Anvin
@ 2011-07-28 13:44           ` Borislav Petkov
  2011-07-28 14:02             ` H. Peter Anvin
  0 siblings, 1 reply; 46+ messages in thread
From: Borislav Petkov @ 2011-07-28 13:44 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Borislav Petkov, Linus Torvalds, Ingo Molnar, Thomas Gleixner,
	LKML, Przywara, Andre, Pohlack, Martin

On Wed, Jul 27, 2011 at 01:16:04PM -0400, H. Peter Anvin wrote:
> > + * Create a contiguous bitmask starting at bit position @lo and ending at
> > + * position @hi. For example
> > + *
> > + * GENMASK_ULL(21, 39) gives us the 64bit vector 0x000000ffffe00000.
> > + */
> > +#define _GENMASK_T(cast, type, lo, hi) \
> > +       (((cast)(1##type << ((hi) - (lo) + 1)) - 1) << (lo))
> > +#define GENMASK(lo, hi)                _GENMASK_T(unsigned, U, lo, hi)
> > +#define GENMASK_UL(lo, hi)     _GENMASK_T(unsigned long, UL, lo, hi)
> > +#define GENMASK_ULL(lo, hi)    _GENMASK_T(unsigned long long, ULL, lo, hi)
> > +
> 
> These really need to be usable from assembly language, too (in which
> case you of course need to not have the cast and suffix), so it probably
> should be defined in <linux/const.h> with the other constant macros.

How about that:

#define _GENMASK_T(cast, type, lo, hi) \
	        ((_AT(cast, (_AC(1,type) << ((hi) - (lo) + 1))) - 1) << (lo))
#define GENMASK(lo, hi)         _GENMASK_T(unsigned, U, lo, hi)
#define GENMASK_UL(lo, hi)      _GENMASK_T(unsigned long, UL, lo, hi)
#define GENMASK_ULL(lo, hi)     _GENMASK_T(unsigned long long, ULL, lo, hi)

outside of __KERNEL__ scope?


-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-28 13:44           ` Borislav Petkov
@ 2011-07-28 14:02             ` H. Peter Anvin
  2011-07-28 14:13               ` Borislav Petkov
  0 siblings, 1 reply; 46+ messages in thread
From: H. Peter Anvin @ 2011-07-28 14:02 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Linus Torvalds, Ingo Molnar, Thomas Gleixner, LKML, Przywara,
	Andre, Pohlack, Martin

On 07/28/2011 06:44 AM, Borislav Petkov wrote:
> On Wed, Jul 27, 2011 at 01:16:04PM -0400, H. Peter Anvin wrote:
>>> + * Create a contiguous bitmask starting at bit position @lo and ending at
>>> + * position @hi. For example
>>> + *
>>> + * GENMASK_ULL(21, 39) gives us the 64bit vector 0x000000ffffe00000.
>>> + */
>>> +#define _GENMASK_T(cast, type, lo, hi) \
>>> +       (((cast)(1##type << ((hi) - (lo) + 1)) - 1) << (lo))
>>> +#define GENMASK(lo, hi)                _GENMASK_T(unsigned, U, lo, hi)
>>> +#define GENMASK_UL(lo, hi)     _GENMASK_T(unsigned long, UL, lo, hi)
>>> +#define GENMASK_ULL(lo, hi)    _GENMASK_T(unsigned long long, ULL, lo, hi)
>>> +
>>
>> These really need to be usable from assembly language, too (in which
>> case you of course need to not have the cast and suffix), so it probably
>> should be defined in <linux/const.h> with the other constant macros.
> 
> How about that:
> 
> #define _GENMASK_T(cast, type, lo, hi) \
> 	        ((_AT(cast, (_AC(1,type) << ((hi) - (lo) + 1))) - 1) << (lo))
> #define GENMASK(lo, hi)         _GENMASK_T(unsigned, U, lo, hi)
> #define GENMASK_UL(lo, hi)      _GENMASK_T(unsigned long, UL, lo, hi)
> #define GENMASK_ULL(lo, hi)     _GENMASK_T(unsigned long long, ULL, lo, hi)
> 
> outside of __KERNEL__ scope?
> 

Then everything needs another underscore.

	-hpa


-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-28 14:02             ` H. Peter Anvin
@ 2011-07-28 14:13               ` Borislav Petkov
  2011-07-28 14:18                 ` H. Peter Anvin
  0 siblings, 1 reply; 46+ messages in thread
From: Borislav Petkov @ 2011-07-28 14:13 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Borislav Petkov, Linus Torvalds, Ingo Molnar, Thomas Gleixner,
	LKML, Przywara, Andre, Pohlack, Martin

On Thu, Jul 28, 2011 at 10:02:46AM -0400, H. Peter Anvin wrote:
> On 07/28/2011 06:44 AM, Borislav Petkov wrote:
> > On Wed, Jul 27, 2011 at 01:16:04PM -0400, H. Peter Anvin wrote:
> >>> + * Create a contiguous bitmask starting at bit position @lo and ending at
> >>> + * position @hi. For example
> >>> + *
> >>> + * GENMASK_ULL(21, 39) gives us the 64bit vector 0x000000ffffe00000.
> >>> + */
> >>> +#define _GENMASK_T(cast, type, lo, hi) \
> >>> +       (((cast)(1##type << ((hi) - (lo) + 1)) - 1) << (lo))
> >>> +#define GENMASK(lo, hi)                _GENMASK_T(unsigned, U, lo, hi)
> >>> +#define GENMASK_UL(lo, hi)     _GENMASK_T(unsigned long, UL, lo, hi)
> >>> +#define GENMASK_ULL(lo, hi)    _GENMASK_T(unsigned long long, ULL, lo, hi)
> >>> +
> >>
> >> These really need to be usable from assembly language, too (in which
> >> case you of course need to not have the cast and suffix), so it probably
> >> should be defined in <linux/const.h> with the other constant macros.
> > 
> > How about that:
> > 
> > #define _GENMASK_T(cast, type, lo, hi) \
> > 	        ((_AT(cast, (_AC(1,type) << ((hi) - (lo) + 1))) - 1) << (lo))
> > #define GENMASK(lo, hi)         _GENMASK_T(unsigned, U, lo, hi)
> > #define GENMASK_UL(lo, hi)      _GENMASK_T(unsigned long, UL, lo, hi)
> > #define GENMASK_ULL(lo, hi)     _GENMASK_T(unsigned long long, ULL, lo, hi)
> > 
> > outside of __KERNEL__ scope?
> > 
> 
> Then everything needs another underscore.

You want them to be _GENMASK{,_UL,_ULL}? Why? Because they're "calling"
macros with a single underscore? Actually, I've always wondered on what
our exact rules on underscores are. I can deduce the nesting level of
macros based on number of underscores but what else?

Thanks.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-28 14:13               ` Borislav Petkov
@ 2011-07-28 14:18                 ` H. Peter Anvin
  2011-07-28 14:35                   ` Borislav Petkov
  0 siblings, 1 reply; 46+ messages in thread
From: H. Peter Anvin @ 2011-07-28 14:18 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Linus Torvalds, Ingo Molnar, Thomas Gleixner, LKML, Przywara,
	Andre, Pohlack, Martin

On 07/28/2011 07:13 AM, Borislav Petkov wrote:
> 
> You want them to be _GENMASK{,_UL,_ULL}? Why? Because they're "calling"
> macros with a single underscore? Actually, I've always wondered on what
> our exact rules on underscores are. I can deduce the nesting level of
> macros based on number of underscores but what else?
> 

Internal symbols used in userspace-visible headers must be from the
namespace:

	^_[A-Z_].*

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] x86, AMD: Correct F15h IC aliasing issue
  2011-07-28 14:18                 ` H. Peter Anvin
@ 2011-07-28 14:35                   ` Borislav Petkov
  0 siblings, 0 replies; 46+ messages in thread
From: Borislav Petkov @ 2011-07-28 14:35 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Borislav Petkov, Linus Torvalds, Ingo Molnar, Thomas Gleixner,
	LKML, Przywara, Andre, Pohlack, Martin

On Thu, Jul 28, 2011 at 10:18:45AM -0400, H. Peter Anvin wrote:
> On 07/28/2011 07:13 AM, Borislav Petkov wrote:
> > 
> > You want them to be _GENMASK{,_UL,_ULL}? Why? Because they're "calling"
> > macros with a single underscore? Actually, I've always wondered on what
> > our exact rules on underscores are. I can deduce the nesting level of
> > macros based on number of underscores but what else?
> > 
> 
> Internal symbols used in userspace-visible headers must be from the
> namespace:
> 
> 	^_[A-Z_].*

Ah, thanks.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

^ permalink raw reply	[flat|nested] 46+ messages in thread

end of thread, other threads:[~2011-07-28 14:36 UTC | newest]

Thread overview: 46+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-22 13:15 [PATCH] x86, AMD: Correct F15h IC aliasing issue Borislav Petkov
2011-07-24 11:13 ` Ingo Molnar
2011-07-24 13:40   ` Borislav Petkov
2011-07-24 13:47     ` Ingo Molnar
2011-07-24 16:16     ` Andrew Morton
2011-07-26 18:33     ` Borislav Petkov
2011-07-24 16:04 ` Linus Torvalds
2011-07-24 17:22   ` Borislav Petkov
2011-07-24 17:39     ` Linus Torvalds
2011-07-24 18:12       ` Ingo Molnar
2011-07-24 18:23       ` Borislav Petkov
2011-07-24 18:30         ` Ingo Molnar
2011-07-24 19:07           ` Borislav Petkov
2011-07-24 20:44             ` Ingo Molnar
2011-07-25 20:00               ` Borislav Petkov
2011-07-25 20:06                 ` Ingo Molnar
2011-07-25 21:53                   ` Borislav Petkov
2011-07-26  5:58                     ` Ray Lee
2011-07-26 17:28                       ` Borislav Petkov
2011-07-26 18:34                         ` Ingo Molnar
2011-07-26 18:39                           ` Borislav Petkov
2011-07-26 18:47                             ` Ingo Molnar
2011-07-26 19:33                               ` Borislav Petkov
2011-07-27 17:10       ` Borislav Petkov
2011-07-27 17:16         ` H. Peter Anvin
2011-07-28 13:44           ` Borislav Petkov
2011-07-28 14:02             ` H. Peter Anvin
2011-07-28 14:13               ` Borislav Petkov
2011-07-28 14:18                 ` H. Peter Anvin
2011-07-28 14:35                   ` Borislav Petkov
2011-07-26 17:59 ` Avi Kivity
2011-07-26 18:13   ` Borislav Petkov
2011-07-26 18:16     ` H. Peter Anvin
2011-07-26 18:37       ` Borislav Petkov
2011-07-26 18:38         ` H. Peter Anvin
2011-07-26 19:42           ` Andre Przywara
2011-07-26 22:34             ` H. Peter Anvin
2011-07-27  4:14             ` Avi Kivity
2011-07-27  6:21               ` Borislav Petkov
2011-07-27  6:59                 ` Ingo Molnar
2011-07-27  9:30                   ` Avi Kivity
2011-07-27 15:37                     ` Borislav Petkov
2011-07-27 15:45                       ` Avi Kivity
2011-07-27 15:49                         ` Borislav Petkov
2011-07-27 15:57                           ` Avi Kivity
2011-07-27 16:42                             ` Borislav Petkov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.