Re: [PATCH 1/3 v2] Add Directed EOI support to APIC emulation

[Gleb Natapov <gleb@redhat.com>]

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.