From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [kvm-unit-tests PATCH] x86: apic: APIC ID tests Date: Thu, 14 Jul 2016 11:50:49 +0200 Message-ID: References: <20160707172453.15642-1-rkrcmar@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , kvm@vger.kernel.org Return-path: Received: from mx1.redhat.com ([209.132.183.28]:59330 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750865AbcGNJux (ORCPT ); Thu, 14 Jul 2016 05:50:53 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7D6CAA0B4B for ; Thu, 14 Jul 2016 09:50:52 +0000 (UTC) In-Reply-To: <20160707172453.15642-1-rkrcmar@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 07/07/2016 19:24, Radim Kr=C4=8Dm=C3=A1=C5=99 wrote: > KVM commit 000000000000 ("KVM: x86: reset xAPIC ID") fixed xAPIC ID > value after reset. >=20 > QEMU commit 5232d00a041c ("target-i386: Implement CPUID[0xB] (Extende= d > Topology Enumeration)") added initial x2APIC to CPUID. >=20 > KVM commit 000000000000 ("KVM: x86: use hardware-compatible format fo= r > APIC ID register") changed internal format of APIC ID register, so ma= ke > sure that guest-visible APIC ID was not been affected. >=20 > Signed-off-by: Radim Kr=C4=8Dm=C3=A1=C5=99 > --- > I'll send v2 with hashes when KVM patches are merged. >=20 > Should I remove the hunk that depends on QEMU version? > (It introduces a FAIL.) No, it's always possible to revert it locally if desired. Applied with fixed hashes, thanks. Paolo > lib/x86/apic.c | 9 +++++++++ > lib/x86/apic.h | 1 + > x86/apic.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++= +++++++++ > 3 files changed, 67 insertions(+) >=20 > diff --git a/lib/x86/apic.c b/lib/x86/apic.c > index 2ceba395e0dc..a9ed28d67727 100644 > --- a/lib/x86/apic.c > +++ b/lib/x86/apic.c > @@ -1,6 +1,7 @@ > #include "libcflat.h" > #include "apic.h" > #include "msr.h" > +#include "processor.h" > =20 > static void *g_apic =3D (void *)0xfee00000; > static void *g_ioapic =3D (void *)0xfec00000; > @@ -129,6 +130,14 @@ int enable_x2apic(void) > } > } > =20 > +void reset_apic(void) > +{ > + u64 disabled =3D rdmsr(MSR_IA32_APICBASE) & ~(APIC_EN | APIC_EXT= D); > + wrmsr(MSR_IA32_APICBASE, disabled); > + apic_ops =3D &xapic_ops; > + wrmsr(MSR_IA32_APICBASE, disabled | APIC_EN); > +} > + > u32 ioapic_read_reg(unsigned reg) > { > *(volatile u32 *)g_ioapic =3D reg; > diff --git a/lib/x86/apic.h b/lib/x86/apic.h > index 2d0504c2088d..dbd6c9b6e7e4 100644 > --- a/lib/x86/apic.h > +++ b/lib/x86/apic.h > @@ -37,5 +37,6 @@ void apic_icr_write(uint32_t val, uint32_t dest); > uint32_t apic_id(void); > =20 > int enable_x2apic(void); > +void reset_apic(void); > =20 > #endif > diff --git a/x86/apic.c b/x86/apic.c > index 8b08a950a0c7..5fc83c681ce8 100644 > --- a/x86/apic.c > +++ b/x86/apic.c > @@ -136,6 +136,62 @@ static void test_apicbase(void) > report_prefix_pop(); > } > =20 > +static void do_write_apic_id(void *id) > +{ > + apic_write(APIC_ID, *(u32 *)id); > +} > + > +static void __test_apic_id(void * unused) > +{ > + u32 id, newid; > + u8 initial_xapic_id =3D cpuid(1).b >> 24; > + u32 initial_x2apic_id =3D cpuid(0xb).d; > + bool x2apic_mode =3D rdmsr(MSR_IA32_APICBASE) & APIC_EXTD; > + > + if (x2apic_mode) > + reset_apic(); > + > + id =3D apic_id(); > + report("xapic id matches cpuid", initial_xapic_id =3D=3D id); > + > + newid =3D (id + 1) << 24; > + report("writeable xapic id", > + !test_for_exception(GP_VECTOR, do_write_apic_id, &newid)= && > + id + 1 =3D=3D apic_id()); > + > + if (!enable_x2apic()) > + goto out; > + > + report("non-writeable x2apic id", > + test_for_exception(GP_VECTOR, do_write_apic_id, &newid))= ; > + report("sane x2apic id", initial_xapic_id =3D=3D (apic_id() & 0x= ff)); > + > + /* old QEMUs do not set initial x2APIC ID */ > + report("x2apic id matches cpuid", > + initial_xapic_id =3D=3D (initial_x2apic_id & 0xff) && > + initial_x2apic_id =3D=3D apic_id()); > + > +out: > + reset_apic(); > + > + report("correct xapic id after reset", initial_xapic_id =3D=3D a= pic_id()); > + > + /* old KVMs do not reset xAPIC ID */ > + if (id !=3D apic_id()) > + apic_write(APIC_ID, id << 24); > + > + if (x2apic_mode) > + enable_x2apic(); > +} > + > +static void test_apic_id(void) > +{ > + if (cpu_count() < 2) > + return; > + > + on_cpu(1, __test_apic_id, NULL); > +} > + > static int ipi_count; > =20 > static void self_ipi_isr(isr_regs_t *regs) > @@ -303,6 +359,7 @@ int main() > test_lapic_existence(); > =20 > mask_pic_interrupts(); > + test_apic_id(); > test_enable_x2apic(); > test_apicbase(); > =20 >=20