Messages in this thread Patch in this message | | | Date | Mon, 7 Dec 2009 13:21:33 +0100 | From | Borislav Petkov <> | Subject | [PATCH] x86, msr: add support for non-contiguous cpumasks |
| |
Hi Peter,
can you please take a look at the following patch which fixes a problem with non-contiguous cpumasks submitted to rd/wrmsr_on_cpus() (see URL in the commit message below) and let me know whether this approach looks ok. If yes, I could submit it with my patchqueue along with the rest of amd64_edac updates for it'll be very convenient if we could get it in now so that it could be backported to .32.
Thanks.
--- The current rd/wrmsr_on_cpus helpers assume that the supplied cpumasks are contiguous. However, there are machines out there like some K8 multinode Opterons which have a non-contiguous core enumeration on each node (e.g. cores 0,2 on node 0 instead of 0,1), see http://www.gossamer-threads.com/lists/linux/kernel/1160268.
This patch fixes out-of-bounds writes (see URL above) by making sure we stay inbounds by using an msr->cpu field which tells us which CPU's MSRs to access.
Additionally, this patch adds msrs_{alloc,free} helpers for preparing the array of MSRs to access.
Cc: H. Peter Anvin <hpa@zytor.com> Cc: Mauro Carvalho Chehab <mchehab@redhat.com> Cc: Aristeu Rozanski <aris@redhat.com> Cc: Randy Dunlap <randy.dunlap@oracle.com> Cc: Doug Thompson <dougthompson@xmission.com> Not-yet-signed-off-by: Borislav Petkov <borislav.petkov@amd.com> --- arch/x86/include/asm/msr.h | 11 ++++++ arch/x86/lib/msr.c | 81 +++++++++++++++++++++++++++++++++++++------ drivers/edac/amd64_edac.c | 17 ++++----- 3 files changed, 87 insertions(+), 22 deletions(-)
diff --git a/arch/x86/include/asm/msr.h b/arch/x86/include/asm/msr.h index 9a00219..4f528f8 100644 --- a/arch/x86/include/asm/msr.h +++ b/arch/x86/include/asm/msr.h @@ -18,6 +18,7 @@ #include <asm/cpumask.h> struct msr { + int cpu; union { struct { u32 l; @@ -247,6 +248,8 @@ do { \ #ifdef CONFIG_SMP int rdmsr_on_cpu(unsigned int cpu, u32 msr_no, u32 *l, u32 *h); int wrmsr_on_cpu(unsigned int cpu, u32 msr_no, u32 l, u32 h); +struct msr *msrs_alloc(const struct cpumask *mask); +void msrs_free(struct msr *msrs); void rdmsr_on_cpus(const struct cpumask *mask, u32 msr_no, struct msr *msrs); void wrmsr_on_cpus(const struct cpumask *mask, u32 msr_no, struct msr *msrs); int rdmsr_safe_on_cpu(unsigned int cpu, u32 msr_no, u32 *l, u32 *h); @@ -264,6 +267,14 @@ static inline int wrmsr_on_cpu(unsigned int cpu, u32 msr_no, u32 l, u32 h) wrmsr(msr_no, l, h); return 0; } +struct msr *msrs_alloc(const struct cpumask *mask) +{ + return kzalloc(sizeof(struct msr), GFP_KERNEL); +} +void msrs_free(struct msr *msrs) +{ + kfree(msrs); +} static inline void rdmsr_on_cpus(const cpumask_t *m, u32 msr_no, struct msr *msrs) { diff --git a/arch/x86/lib/msr.c b/arch/x86/lib/msr.c index 41628b1..e2213de 100644 --- a/arch/x86/lib/msr.c +++ b/arch/x86/lib/msr.c @@ -7,20 +7,40 @@ struct msr_info { u32 msr_no; struct msr reg; struct msr *msrs; - int off; + unsigned msrs_len; int err; }; +static inline struct msr *scroll_to_cpu(struct msr_info *rv, int this_cpu, + const char *caller) +{ + int i = 0; + + if (!rv->msrs) + return &rv->reg; + + while (i < rv->msrs_len && + rv->msrs[i].cpu != this_cpu) + i++; + + if (rv->msrs[i].cpu != this_cpu) { + pr_err("%s: called on a wrong cpu (%d vs %d)?\n", + caller, rv->msrs[i].cpu, this_cpu); + return NULL; + } + + return &rv->msrs[i]; +} + static void __rdmsr_on_cpu(void *info) { struct msr_info *rv = info; struct msr *reg; int this_cpu = raw_smp_processor_id(); - if (rv->msrs) - reg = &rv->msrs[this_cpu - rv->off]; - else - reg = &rv->reg; + reg = scroll_to_cpu(rv, this_cpu, __func__); + if (!reg) + return; rdmsr(rv->msr_no, reg->l, reg->h); } @@ -31,10 +51,9 @@ static void __wrmsr_on_cpu(void *info) struct msr *reg; int this_cpu = raw_smp_processor_id(); - if (rv->msrs) - reg = &rv->msrs[this_cpu - rv->off]; - else - reg = &rv->reg; + reg = scroll_to_cpu(rv, this_cpu, __func__); + if (!reg) + return; wrmsr(rv->msr_no, reg->l, reg->h); } @@ -80,9 +99,9 @@ static void __rwmsr_on_cpus(const struct cpumask *mask, u32 msr_no, memset(&rv, 0, sizeof(rv)); - rv.off = cpumask_first(mask); - rv.msrs = msrs; - rv.msr_no = msr_no; + rv.msr_no = msr_no; + rv.msrs = msrs; + rv.msrs_len = cpumask_weight(mask); this_cpu = get_cpu(); @@ -120,6 +139,44 @@ void wrmsr_on_cpus(const struct cpumask *mask, u32 msr_no, struct msr *msrs) } EXPORT_SYMBOL(wrmsr_on_cpus); +/* + * Allocate enough msr structs for the supplied cpumask. Also, take care of + * non-contigious bitmasks. + */ +struct msr *msrs_alloc(const struct cpumask *mask) +{ + struct msr *msrs; + int i, cpu = -1; + unsigned msrs_len; + + if (cpumask_empty(mask)) { + pr_warning("%s: empty cpumask!\n", __func__); + return NULL; + } + + msrs_len = cpumask_weight(mask); + + msrs = kzalloc(sizeof(struct msr) * msrs_len, GFP_KERNEL); + if (!msrs) { + pr_warning("%s: error allocating msrs\n", __func__); + return NULL; + } + + for (i = 0; i < msrs_len; i++) { + cpu = cpumask_next(cpu, mask); + msrs[i].cpu = cpu; + } + + return msrs; +} +EXPORT_SYMBOL(msrs_alloc); + +void msrs_free(struct msr *msrs) +{ + kfree(msrs); +} +EXPORT_SYMBOL(msrs_free); + /* These "safe" variants are slower and should be used when the target MSR may not actually exist. */ static void __rdmsr_safe_on_cpu(void *info) diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c index 533f5ff..784c968 100644 --- a/drivers/edac/amd64_edac.c +++ b/drivers/edac/amd64_edac.c @@ -2507,10 +2507,8 @@ static bool amd64_nb_mce_bank_enabled_on_node(int nid) get_cpus_on_this_dct_cpumask(mask, nid); - msrs = kzalloc(sizeof(struct msr) * cpumask_weight(mask), GFP_KERNEL); + msrs = msrs_alloc(mask); if (!msrs) { - amd64_printk(KERN_WARNING, "%s: error allocating msrs\n", - __func__); free_cpumask_var(mask); return false; } @@ -2532,7 +2530,7 @@ static bool amd64_nb_mce_bank_enabled_on_node(int nid) ret = true; out: - kfree(msrs); + msrs_free(msrs); free_cpumask_var(mask); return ret; } @@ -2551,17 +2549,16 @@ static int amd64_toggle_ecc_err_reporting(struct amd64_pvt *pvt, bool on) get_cpus_on_this_dct_cpumask(cmask, pvt->mc_node_id); - msrs = kzalloc(sizeof(struct msr) * cpumask_weight(cmask), GFP_KERNEL); - if (!msrs) { - amd64_printk(KERN_WARNING, "%s: error allocating msrs\n", - __func__); + msrs = msrs_alloc(cmask); + if (!msrs) return -ENOMEM; - } rdmsr_on_cpus(cmask, MSR_IA32_MCG_CTL, msrs); for_each_cpu(cpu, cmask) { + WARN_ON(cpu != msrs[idx].cpu); + if (on) { if (msrs[idx].l & K8_MSR_MCGCTL_NBE) pvt->flags.ecc_report = 1; @@ -2578,7 +2575,7 @@ static int amd64_toggle_ecc_err_reporting(struct amd64_pvt *pvt, bool on) } wrmsr_on_cpus(cmask, MSR_IA32_MCG_CTL, msrs); - kfree(msrs); + msrs_free(msrs); free_cpumask_var(cmask); return 0; -- 1.6.5.3
-- Regards/Gruss, Boris.
Operating | Advanced Micro Devices GmbH System | Karl-Hammerschmidt-Str. 34, 85609 Dornach b. München, Germany Research | Geschäftsführer: Andrew Bowd, Thomas M. McCoy, Giuliano Meroni Center | Sitz: Dornach, Gemeinde Aschheim, Landkreis München (OSRC) | Registergericht München, HRB Nr. 43632 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |