On Tue, Oct 04, 2016 at 04:38:16PM +0200, Thomas Gleixner wrote: > On Tue, 4 Oct 2016, Prarit Bhargava wrote: > > On 10/04/2016 06:58 AM, Thomas Gleixner wrote: > > > While it is the right thing to initialize the package map in that case, it > > > still papers over a robustness issue in the uncore code, which needs to be > > > fixed first. > > > > I will include a separate patch with an error check for pkg == 0xffff in the > > uncore code. > > 0xffff? That won't help. The id returned is -1 if the entry is not > initialized. And aside of that just patching that particular place is not > helping as the uncore code and also rapl is relying on the package map > being populated. > > So we need a sanity check in the initialization code which prevents any of > this being executed. I still need to test this, but how about something like this? thanks, jirka --- diff --git a/arch/x86/events/intel/rapl.c b/arch/x86/events/intel/rapl.c index 28865938aadf..61d087a2f25d 100644 --- a/arch/x86/events/intel/rapl.c +++ b/arch/x86/events/intel/rapl.c @@ -598,8 +598,13 @@ static int rapl_cpu_online(unsigned int cpu) static int rapl_cpu_prepare(unsigned int cpu) { - struct rapl_pmu *pmu = cpu_to_rapl_pmu(cpu); + struct rapl_pmu *pmu; + int pkg = topology_logical_package_id(cpu); + + if (WARN_ON(pkg == -1)) + return -EINVAL; + pmu = cpu_to_rapl_pmu(cpu); if (pmu) return 0; @@ -613,7 +618,7 @@ static int rapl_cpu_prepare(unsigned int cpu) pmu->timer_interval = ms_to_ktime(rapl_timer_ms); pmu->cpu = -1; rapl_hrtimer_init(pmu); - rapl_pmus->pmus[topology_logical_package_id(cpu)] = pmu; + rapl_pmus->pmus[pkg] = pmu; return 0; } diff --git a/arch/x86/events/intel/uncore.c b/arch/x86/events/intel/uncore.c index 463dc7a5a6c3..3f657fbbf787 100644 --- a/arch/x86/events/intel/uncore.c +++ b/arch/x86/events/intel/uncore.c @@ -1352,7 +1352,7 @@ static int __init intel_uncore_init(void) { const struct x86_cpu_id *id; struct intel_uncore_init_fun *uncore_init; - int pret = 0, cret = 0, ret; + int pret = 0, cret = 0, ret, cpu; id = x86_match_cpu(intel_uncore_match); if (!id) @@ -1363,6 +1363,13 @@ static int __init intel_uncore_init(void) max_packages = topology_max_packages(); + for_each_present_cpu(cpu) { + int pkg = topology_logical_package_id(cpu); + + if (WARN_ON(pkg == -1)) + return -EINVAL; + } + uncore_init = (struct intel_uncore_init_fun *)id->driver_data; if (uncore_init->pci_init) { pret = uncore_init->pci_init();