Date: Thu, 5 Nov 2009 14:52:04 +0100 From: Attilio Rao <attilio@freebsd.org> To: Giovanni Trematerra <giovanni.trematerra@gmail.com> Cc: des@des.no, FreeBSD Current <freebsd-current@freebsd.org> Subject: Re: [PATCH] AMD Opteron Rev. E hack Message-ID: <3bbf2fe10911050552k3f9396feu8f4b9ed7a8dda81b@mail.gmail.com> In-Reply-To: <4e6cba830911050302k56bed35aj5ca9fa16379ab325@mail.gmail.com> References: <4e6cba830911050302k56bed35aj5ca9fa16379ab325@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
2009/11/5 Giovanni Trematerra <giovanni.trematerra@gmail.com>:
> Hi,
> I have a quick and dirty patch to address the problem as discussed on
> commit r198868 in svn-src-head@
> I introduced BROKEN_OPTERON_E kernel option for i386/amd64 arch.
> The patch isn't tested yet, I only successfully compiled on i386.
> Can you let me know if the patch is on the right direction to resolve the issue?
> style(9) tips are welcomed.
> diff -r 75d35d8e7fe1 sys/amd64/amd64/identcpu.c
> --- a/sys/amd64/amd64/identcpu.c Thu Nov 05 11:18:35 2009 +0100
> +++ b/sys/amd64/amd64/identcpu.c Thu Nov 05 12:42:35 2009 +0100
> @@ -404,6 +404,10 @@
>
> if (cpu_vendor_id == CPU_VENDOR_AMD)
> print_AMD_info();
> +#if defined(BROKEN_OPTERON_E)
> + else
> + printf("BROKEN_OPTERON_E option in your kernel is useless with y
> our CPU\n");
> +#endif
> }
>
> void
> @@ -620,10 +624,17 @@
> */
> if (CPUID_TO_FAMILY(cpu_id) == 0xf && CPUID_TO_MODEL(cpu_id) >= 0x20 &&
> CPUID_TO_MODEL(cpu_id) <= 0x3f) {
> +#if !defined(BROKEN_OPTERON_E)
> printf("WARNING: This architecture revision has known SMP "
> "hardware bugs which may cause random instability\n");
> - printf("WARNING: For details see: "
> - "http://bugzilla.kernel.org/show_bug.cgi?id=11305\n");
> +#else
> + printf("WARNING: options BROKEN_OPTERON_E is in your kernel. "
> + "Expect performance penalties\n");
> + else
> +
> + printf("WARNING: options BROKEN_OPTERON_E is useless with your C
> PU."
> + "Expect performance penalties\n");
> +#endif
> }
> }
I would leave the whole logic within print_AMD_info() and not pollute
external code and I would not print a string if the fix is in.
> diff -r 75d35d8e7fe1 sys/amd64/include/atomic.h
> --- a/sys/amd64/include/atomic.h Thu Nov 05 11:18:35 2009 +0100
> +++ b/sys/amd64/include/atomic.h Thu Nov 05 12:42:35 2009 +0100
> @@ -36,6 +36,14 @@
> #define wmb() __asm __volatile("sfence;" : : : "memory")
> #define rmb() __asm __volatile("lfence;" : : : "memory")
>
> +#include "opt_cpu.h"
> +
> +#if defined(BROKEN_OPTERON_E) && (defined(SMP) || !defined(_KERNEL))
> + #define OPTERON_E_HACK() rmb()
> +#else
> + #define OPTERON_E_HACK()
> +#endif
> +
> /*
> * Various simple operations on memory, each of which is atomic in the
> * presence of interrupts and multiple processors.
> @@ -147,6 +155,8 @@
> "m" (*dst) /* 4 */
> : "memory");
>
> + OPTERON_E_HACK();
> +
> return (res);
> }
You need to override the whole barrier IMHO and not add this new stub.
Attilio
--
Peace can only be achieved by understanding - A. Einstein
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3bbf2fe10911050552k3f9396feu8f4b9ed7a8dda81b>