From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: [PATCH kvm-unit-tests v2] pmu: fixes for Sandy Bridge hosts Date: Mon, 3 Jun 2013 09:41:14 +0300 Message-ID: <20130603064114.GV4725@redhat.com> References: <1369936038-19242-1-git-send-email-pbonzini@redhat.com> <20130602153341.GK24773@redhat.com> <51AC3929.50605@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org To: Paolo Bonzini Return-path: Received: from mx1.redhat.com ([209.132.183.28]:21385 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751134Ab3FCGlQ (ORCPT ); Mon, 3 Jun 2013 02:41:16 -0400 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r536fG5H026934 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 3 Jun 2013 02:41:16 -0400 Content-Disposition: inline In-Reply-To: <51AC3929.50605@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Mon, Jun 03, 2013 at 08:35:21AM +0200, Paolo Bonzini wrote: > Il 02/06/2013 17:33, Gleb Natapov ha scritto: > > On Thu, May 30, 2013 at 07:47:18PM +0200, Paolo Bonzini wrote: > >> This patch includes two fixes for SB: > >> > >> * the 3rd fixed counter ("ref cpu cycles") can sometimes report > >> less than the number of iterations > >> > >> * there is an 8th counter which causes out of bounds accesses > >> to gp_event or check_counters_many's cnt array > >> > >> There is still a bug in KVM, because the "pmu all counters-0" > >> test fails. (It passes if you use any 6 of the 8 gp counters, > >> fails if you use 7 or 8). > >> > > Changelog? > > Changelog is simply that the patch applies. :( > Just say so, I tried to see how they are different and failed. > v1 was hand-edited and I did not regenerate it after testing the edit: > > -@@ -395,6 +396,14 @@ int main(int ac, char **av) > +@@ -395,6 +396,10 @@ int main(int ac, char **av) > Yeah, hard to spot :) > Paolo > > >> Signed-off-by: Paolo Bonzini > >> --- > >> x86/pmu.c | 21 +++++++++++++++------ > >> 1 file changed, 15 insertions(+), 6 deletions(-) > >> > >> diff --git a/x86/pmu.c b/x86/pmu.c > >> index 2c46f31..dca753a 100644 > >> --- a/x86/pmu.c > >> +++ b/x86/pmu.c > >> @@ -88,9 +88,10 @@ struct pmu_event { > >> }, fixed_events[] = { > >> {"fixed 1", MSR_CORE_PERF_FIXED_CTR0, 10*N, 10.2*N}, > >> {"fixed 2", MSR_CORE_PERF_FIXED_CTR0 + 1, 1*N, 30*N}, > >> - {"fixed 3", MSR_CORE_PERF_FIXED_CTR0 + 2, 1*N, 30*N} > >> + {"fixed 3", MSR_CORE_PERF_FIXED_CTR0 + 2, 0.1*N, 30*N} > >> }; > >> > >> +static int num_counters; > >> static int tests, failures; > >> > >> char *buf; > >> @@ -237,7 +238,7 @@ static void check_gp_counter(struct pmu_event *evt) > >> }; > >> int i; > >> > >> - for (i = 0; i < eax.split.num_counters; i++, cnt.ctr++) { > >> + for (i = 0; i < num_counters; i++, cnt.ctr++) { > >> cnt.count = 0; > >> measure(&cnt, 1); > >> report(evt->name, i, verify_event(cnt.count, evt)); > >> @@ -276,7 +277,7 @@ static void check_counters_many(void) > >> pmu_counter_t cnt[10]; > >> int i, n; > >> > >> - for (i = 0, n = 0; n < eax.split.num_counters; i++) { > >> + for (i = 0, n = 0; n < num_counters; i++) { > >> if (ebx.full & (1 << i)) > >> continue; > >> > >> @@ -316,10 +317,10 @@ static void check_counter_overflow(void) > >> /* clear status before test */ > >> wrmsr(MSR_CORE_PERF_GLOBAL_OVF_CTRL, rdmsr(MSR_CORE_PERF_GLOBAL_STATUS)); > >> > >> - for (i = 0; i < eax.split.num_counters + 1; i++, cnt.ctr++) { > >> + for (i = 0; i < num_counters + 1; i++, cnt.ctr++) { > >> uint64_t status; > >> int idx; > >> - if (i == eax.split.num_counters) > >> + if (i == num_counters) > >> cnt.ctr = fixed_events[0].unit_sel; > >> if (i % 2) > >> cnt.config |= EVNTSEL_INT; > >> @@ -355,7 +356,7 @@ static void check_rdpmc(void) > >> uint64_t val = 0x1f3456789ull; > >> int i; > >> > >> - for (i = 0; i < eax.split.num_counters; i++) { > >> + for (i = 0; i < num_counters; i++) { > >> uint64_t x = (val & 0xffffffff) | > >> ((1ull << (eax.split.bit_width - 32)) - 1) << 32; > >> wrmsr(MSR_IA32_PERFCTR0 + i, val); > >> @@ -395,6 +396,10 @@ int main(int ac, char **av) > >> printf("Fixed counters: %d\n", edx.split.num_counters_fixed); > >> printf("Fixed counter width: %d\n", edx.split.bit_width_fixed); > >> > >> + num_counters = eax.split.num_counters; > >> + if (num_counters > ARRAY_SIZE(gp_events)) > >> + num_counters = ARRAY_SIZE(gp_events); > >> + > >> apic_write(APIC_LVTPC, PC_VECTOR); > >> > >> check_gp_counters(); > >> -- > >> 1.8.2.1 > >> > >> -- > >> To unsubscribe from this list: send the line "unsubscribe kvm" in > >> the body of a message to majordomo@vger.kernel.org > >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > -- > > Gleb. > > -- > > To unsubscribe from this list: send the line "unsubscribe kvm" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- Gleb.