[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] x86, amd: Disable way access filter on Piledriver CPUs
>>> On 17.12.12 at 18:49, Mats Petersson <mats.petersson@xxxxxxxxxx> wrote: > On 17/12/12 17:08, Jan Beulich wrote: >> The Way Access Filter in recent AMD CPUs may hurt the performance of >> some workloads, caused by aliasing issues in the L1 cache. >> This patch disables it on the affected CPUs. >> >> The issue is similar to that one of last year: >> http://lkml.indiana.edu/hypermail/linux/kernel/1107.3/00041.html >> This new patch does not replace the old one, we just need another >> quirk for newer CPUs. >> >> The performance penalty without the patch depends on the >> circumstances, but is a bit less than the last year's 3%. >> >> The workloads affected would be those that access code from the same >> physical page under different virtual addresses, so different >> processes using the same libraries with ASLR or multiple instances of >> PIE-binaries. The code needs to be accessed simultaneously from both >> cores of the same compute unit. >> >> More details can be found here: >> http://developer.amd.com/Assets/SharedL1InstructionCacheonAMD15hCPU.pdf >> >> CPUs affected are anything with the core known as Piledriver. >> That includes the new parts of the AMD A-Series (aka Trinity) and the >> just released new CPUs of the FX-Series (aka Vishera). >> The model numbering is a bit odd here: FX CPUs have model 2, >> A-Series has model 10h, with possible extensions to 1Fh. Hence the >> range of model ids. >> >> Signed-off-by: Andre Przywara <osp@xxxxxxxxx> >> >> Add and use MSR_AMD64_IC_CFG. Update the value whenever it is found to >> not have all bits set, rather than just when it's zero. >> >> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx> >> >> --- a/xen/arch/x86/cpu/amd.c >> +++ b/xen/arch/x86/cpu/amd.c >> @@ -448,6 +448,14 @@ static void __devinit init_amd(struct cp >> } >> } >> >> + /* >> + * The way access filter has a performance penalty on some workloads. >> + * Disable it on the affected CPUs. >> + */ >> + if (c->x86 == 0x15 && c->x86_model >= 0x02 && c->x86_model < 0x20 && >> + !rdmsr_safe(MSR_AMD64_IC_CFG, value) && (value & 0x1e) != 0x1e) >> + wrmsr_safe(MSR_AMD64_IC_CFG, value | 0x1e); > Would it not be better to simply write 0x1e, rather than only write it > when it's not 0x1e? It's a one-off operation, but the extra readmsr > seems unnecessary to me. Possibly, but I guess you checked the original Linux commit that this was derived from, and saw that they do it yet a little more oddly (writing the disabling value only when the original value had all 4 bits clear. Fact is that the way it is now, we avoid an MSR write to a register we can't even read (and I think that is desirable in any case; the checking of the value is pretty benign then). Jan >> + >> amd_get_topology(c); >> >> /* Pointless to use MWAIT on Family10 as it does not deep sleep. */ >> --- a/xen/include/asm-x86/msr-index.h >> +++ b/xen/include/asm-x86/msr-index.h >> @@ -211,6 +211,7 @@ >> >> /* AMD64 MSRs */ >> #define MSR_AMD64_NB_CFG 0xc001001f >> +#define MSR_AMD64_IC_CFG 0xc0011021 >> #define MSR_AMD64_DC_CFG 0xc0011022 >> #define AMD64_NB_CFG_CF8_EXT_ENABLE_BIT 46 >> >> >> >> > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@xxxxxxxxxxxxx > http://lists.xen.org/xen-devel _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |