From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Xu Subject: Re: [kvm-unit-tests PATCH 0/3] check multiple init for vm/smp Date: Tue, 25 Oct 2016 19:56:47 +0800 Message-ID: <20161025115647.GB13839@pxdev.xzpeter.org> References: <1477386030-13955-1-git-send-email-peterx@redhat.com> <20161025101247.t2v6td4rwex7ipj3@kamzik.brq.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Cc: kvm@vger.kernel.org, pbonzini@redhat.com, rkrcmar@redhat.com To: Andrew Jones Return-path: Received: from mx1.redhat.com ([209.132.183.28]:38654 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753454AbcJYL44 (ORCPT ); Tue, 25 Oct 2016 07:56:56 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (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 6B3B580F91 for ; Tue, 25 Oct 2016 11:56:56 +0000 (UTC) Content-Disposition: inline In-Reply-To: <20161025101247.t2v6td4rwex7ipj3@kamzik.brq.redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Tue, Oct 25, 2016 at 12:12:47PM +0200, Andrew Jones wrote: > On Tue, Oct 25, 2016 at 05:00:27PM +0800, Peter Xu wrote: > > The first two patches check whether there are multiple init for > > vm/smp, assertion fail will be triggered if so. > > > > Patch 3 is to cleanup existing setup_igt() when smp_init() is called > > (suggested by Drew). > > > > Peter Xu (3): > > x86: vm: add assert to avoid multiple vm init > > x86: smp: assert to avoid multiple init of smp > > tests: don't call setup_idt() if with smp_init() > > > > lib/x86/smp.c | 1 + > > lib/x86/vm.c | 1 + > > x86/apic.c | 1 - > > x86/hyperv_stimer.c | 1 - > > x86/hyperv_synic.c | 1 - > > x86/ioapic.c | 1 - > > x86/tscdeadline_latency.c | 1 - > > 7 files changed, 2 insertions(+), 5 deletions(-) > > > > -- > > 2.7.4 > > > > I actually looked at the code this time around and think that only > patch 1/3 is needed. While it would be pointless to call smp_init > multiple times, it doesn't hurt (that means 2/3 is OK, but not > necessary). And, now I see 3/3 is just a cleanup. No assert would > fire without it. So it's not necessary either. > > Actually, wrt to setup_idt, I'd say the only problems with it is > that there's the 'if (idt_initialized) return' and that it isn't > named something like setup_idt_defaults. Isn't it conceivable that > a unit test may want to revert its handlers back to the defaults > after installing custom handlers by simply calling this again? Yeah, that's possible. Maybe we can let anyone who needs this first to introduce the setup_idt_defaults(). For this series: I have no strong willingness to push this in, and yes all three patches are not essential at all. I just picked them out from vt-d unit test series, in case any of us would like to pick it. Thanks, -- peterx