From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: [PATCH 1/3 v2] Add Directed EOI support to APIC emulation Date: Mon, 29 Jun 2009 12:52:28 +0300 Message-ID: <20090629095228.GV20289@redhat.com> References: <1246191331-31085-1-git-send-email-gleb@redhat.com> <1246191331-31085-2-git-send-email-gleb@redhat.com> <4A4886E4.3070907@redhat.com> <20090629092937.GT20289@redhat.com> <4A488E0C.7080207@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org To: Avi Kivity Return-path: Received: from mx2.redhat.com ([66.187.237.31]:43621 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752991AbZF2Jw1 (ORCPT ); Mon, 29 Jun 2009 05:52:27 -0400 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n5T9qVUk025783 for ; Mon, 29 Jun 2009 05:52:31 -0400 Content-Disposition: inline In-Reply-To: <4A488E0C.7080207@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Mon, Jun 29, 2009 at 12:49:00PM +0300, Avi Kivity wrote: > On 06/29/2009 12:29 PM, Gleb Natapov wrote: >> On Mon, Jun 29, 2009 at 12:18:28PM +0300, Avi Kivity wrote: >> >>> On 06/28/2009 03:15 PM, Gleb Natapov wrote: >>> >>>> Directed EOI is specified by x2APIC, but is available even when lapic is >>>> in xAPIC mode. >>>> >>>> #define APIC_LVT_NUM 6 >>>> /* 14 is the version for Xeon and Pentium 8.4.8*/ >>>> -#define APIC_VERSION (0x14UL | ((APIC_LVT_NUM - 1)<< 16)) >>>> +#define APIC_VERSION (0x14UL | ((APIC_LVT_NUM - 1)<< 16) | \ >>>> + APIC_LVR_DIRECTED_EOI) >>>> >>>> >>> Better make that depend on the x2apic cpuid bit. >>> >>> >> Are you sure. It looks like this feature is independent from x2APIC. It just specified >> by the same spec. >> > > We're changing something that the guests sees. Suppose the guest has a > bug in directed EOI, just upgrading kvm will cause it to trigger. If we > make it dependent on x2apic (or something else that needs to be selected > by the user), we maintain compatibility. > Yes, I thought about something else (not x2apic). But yet another command line switch look like overkill. >>>> case APIC_SPIV: >>>> - apic_set_reg(apic, APIC_SPIV, val& 0x3ff); >>>> + apic_set_reg(apic, APIC_SPIV, val& 0xfff); >>>> if (!(val& APIC_SPIV_APIC_ENABLED)) { >>>> int i; >>>> u32 lvt_val; >>>> >>>> >>> Confused, you're adding bits 10 and 11 while APIC_SPIV_DIRECTED_EOI is >>> bit 12? >>> >> For well behaved guests it doesn't matter :) And Intel keep changing >> what reserved bits are in this register. Older doc says bit 9 is a Focus >> Processor bit, x2APIC doc says bit 9 is registered. So what should we do >> for bit 9? >> > > Let's make it a separate patch in case something blows. I think you > need to allow bit 9 even if x2apic retroactively reserves it. > OK. -- Gleb.