diff mbox

[v9,0/7] KVM: VMX: Add Posted Interrupt supporting

Message ID A9667DDFB95DB7438FA9D7D576C3D87E09A075E0@SHSMSX101.ccr.corp.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Zhang, Yang Z April 11, 2013, 1:03 a.m. UTC 
Gleb Natapov wrote on 2013-04-10:
> On Wed, Apr 10, 2013 at 09:22:50PM +0800, Yang Zhang wrote:
>> From: Yang Zhang <yang.z.zhang@Intel.com>
>> 
>> The follwoing patches are adding the Posted Interrupt supporting to KVM:
>> The first patch enables the feature 'acknowledge interrupt on vmexit'.Since
>> it is required by Posted interrupt, we need to enable it firstly.
>> 
>> And the subsequent patches are adding the posted interrupt supporting:
>> Posted Interrupt allows APIC interrupts to inject into guest directly
>> without any vmexit.
>> 
>> - When delivering a interrupt to guest, if target vcpu is running,
>>   update Posted-interrupt requests bitmap and send a notification event
>>   to the vcpu. Then the vcpu will handle this interrupt automatically,
>>   without any software involvemnt.
>> - If target vcpu is not running or there already a notification event
>>   pending in the vcpu, do nothing. The interrupt will be handled by
>>   next vm entry
>> Changes from v8 to v9:
>> * Add tracing in PI case when deliver interrupt.
>> * Scan ioapic when updating SPIV register.
> Do not see it at the patch series. Have I missed it?
The change is in forth patch:

And I found recalculate_apic_map() doesn't track the enable/disable apic by software approach. So make_scan_ioapic_request in recalculate_apic_map() is not enough.
We also should force rescan ioapic when apic state is changed via software approach(update spiv reg).

> 
>> * Rebase on top of KVM upstream + RTC eoi tracking patch.
>> 
>> Changes from v7 to v8:
>> * Remove unused memeber 'on' from struct pi_desc.
>> * Register a dummy function to sync_pir_to_irr is apicv is disabled.
>> * Minor fixup.
>> * Rebase on top of KVM upstream + RTC eoi tracking patch.
>> 
>> Yang Zhang (7):
>>   KVM: VMX: Enable acknowledge interupt on vmexit
>>   KVM: VMX: Register a new IPI for posted interrupt
>>   KVM: VMX: Check the posted interrupt capability
>>   KVM: Call common update function when ioapic entry changed.
>>   KVM: Set TMR when programming ioapic entry
>>   KVM: VMX: Add the algorithm of deliver posted interrupt
>>   KVM: VMX: Use posted interrupt to deliver virtual interrupt
>>  arch/ia64/kvm/lapic.h              |    6 -
>>  arch/x86/include/asm/entry_arch.h  |    4 +
>>  arch/x86/include/asm/hardirq.h     |    3 +
>>  arch/x86/include/asm/hw_irq.h      |    1 +
>>  arch/x86/include/asm/irq_vectors.h |    5 +
>>  arch/x86/include/asm/kvm_host.h    |    3 + arch/x86/include/asm/vmx.h
>>          |    4 + arch/x86/kernel/entry_64.S         |    5 +
>>  arch/x86/kernel/irq.c              |   22 ++++
>>  arch/x86/kernel/irqinit.c          |    4 + arch/x86/kvm/lapic.c      
>>          |   66 ++++++++---- arch/x86/kvm/lapic.h               |    7
>>  ++ arch/x86/kvm/svm.c                 |   12 ++ arch/x86/kvm/vmx.c    
>>              |  207 +++++++++++++++++++++++++++++++-----
>>  arch/x86/kvm/x86.c                 |   19 +++-
>>  include/linux/kvm_host.h           |    4 +- virt/kvm/ioapic.c        
>>           |   32 ++++-- virt/kvm/ioapic.h                  |    7 +-
>>  virt/kvm/irq_comm.c                |    4 +- virt/kvm/kvm_main.c      
>>           |    5 +- 20 files changed, 341 insertions(+), 79 deletions(-)
> 
> --
> 			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


Best regards,
Yang


--
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

Comments

Gleb Natapov April 11, 2013, 5:44 a.m. UTC | #1 
On Thu, Apr 11, 2013 at 01:03:30AM +0000, Zhang, Yang Z wrote:
> Gleb Natapov wrote on 2013-04-10:
> > On Wed, Apr 10, 2013 at 09:22:50PM +0800, Yang Zhang wrote:
> >> From: Yang Zhang <yang.z.zhang@Intel.com>
> >> 
> >> The follwoing patches are adding the Posted Interrupt supporting to KVM:
> >> The first patch enables the feature 'acknowledge interrupt on vmexit'.Since
> >> it is required by Posted interrupt, we need to enable it firstly.
> >> 
> >> And the subsequent patches are adding the posted interrupt supporting:
> >> Posted Interrupt allows APIC interrupts to inject into guest directly
> >> without any vmexit.
> >> 
> >> - When delivering a interrupt to guest, if target vcpu is running,
> >>   update Posted-interrupt requests bitmap and send a notification event
> >>   to the vcpu. Then the vcpu will handle this interrupt automatically,
> >>   without any software involvemnt.
> >> - If target vcpu is not running or there already a notification event
> >>   pending in the vcpu, do nothing. The interrupt will be handled by
> >>   next vm entry
> >> Changes from v8 to v9:
> >> * Add tracing in PI case when deliver interrupt.
> >> * Scan ioapic when updating SPIV register.
> > Do not see it at the patch series. Have I missed it?
> The change is in forth patch:
> 
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 6796218..4ccdc94 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -134,11 +134,7 @@ static inline void apic_set_spiv(struct kvm_lapic *apic, u32 val)
>  			static_key_slow_inc(&apic_sw_disabled.key);
>  	}
>  	apic_set_reg(apic, APIC_SPIV, val);
> -}
> -
> -static inline int apic_enabled(struct kvm_lapic *apic)
> -{
> -	return kvm_apic_sw_enabled(apic) &&	kvm_apic_hw_enabled(apic);
> +	kvm_make_request(KVM_REQ_SCAN_IOAPIC, apic->vcpu);
>  }
> 
OK, see it now. Thanks.

> As you mentioned, since it will call apic_enabled() to check whether apic is enabled in vcpu_scan_ioapic. So we must ensure rescan ioapic when apic state changed.
> And I found recalculate_apic_map() doesn't track the enable/disable apic by software approach. So make_scan_ioapic_request in recalculate_apic_map() is not enough.
> We also should force rescan ioapic when apic state is changed via software approach(update spiv reg).
> 
10.4.7.2 Local APIC State After It Has Been Software Disabled says:

  Pending interrupts in the IRR and ISR registers are held and require
  masking or handling by the CPU.

My understanding is that we should treat software disabled APIC as a
valid target for an interrupt. vcpu_scan_ioapic() should check
kvm_apic_hw_enabled() only.

--
			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
Zhang, Yang Z April 11, 2013, 6:05 a.m. UTC | #2 
Gleb Natapov wrote on 2013-04-11:
> On Thu, Apr 11, 2013 at 01:03:30AM +0000, Zhang, Yang Z wrote:
>> Gleb Natapov wrote on 2013-04-10:
>>> On Wed, Apr 10, 2013 at 09:22:50PM +0800, Yang Zhang wrote:
>>>> From: Yang Zhang <yang.z.zhang@Intel.com>
>>>> 
>>>> The follwoing patches are adding the Posted Interrupt supporting to KVM:
>>>> The first patch enables the feature 'acknowledge interrupt on vmexit'.Since
>>>> it is required by Posted interrupt, we need to enable it firstly.
>>>> 
>>>> And the subsequent patches are adding the posted interrupt supporting:
>>>> Posted Interrupt allows APIC interrupts to inject into guest directly
>>>> without any vmexit.
>>>> 
>>>> - When delivering a interrupt to guest, if target vcpu is running,
>>>>   update Posted-interrupt requests bitmap and send a notification
>>>>   event to the vcpu. Then the vcpu will handle this interrupt
>>>>   automatically, without any software involvemnt. - If target vcpu is
>>>>   not running or there already a notification event pending in the
>>>>   vcpu, do nothing. The interrupt will be handled by next vm entry
>>>> Changes from v8 to v9:
>>>> * Add tracing in PI case when deliver interrupt.
>>>> * Scan ioapic when updating SPIV register.
>>> Do not see it at the patch series. Have I missed it?
>> The change is in forth patch:
>> 
>> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
>> index 6796218..4ccdc94 100644
>> --- a/arch/x86/kvm/lapic.c
>> +++ b/arch/x86/kvm/lapic.c
>> @@ -134,11 +134,7 @@ static inline void apic_set_spiv(struct kvm_lapic *apic,
> u32 val)
>>  			static_key_slow_inc(&apic_sw_disabled.key);
>>  	}
>>  	apic_set_reg(apic, APIC_SPIV, val);
>> -}
>> -
>> -static inline int apic_enabled(struct kvm_lapic *apic)
>> -{
>> -	return kvm_apic_sw_enabled(apic) &&	kvm_apic_hw_enabled(apic);
>> +	kvm_make_request(KVM_REQ_SCAN_IOAPIC, apic->vcpu);
>>  }
> OK, see it now. Thanks.
> 
>> As you mentioned, since it will call apic_enabled() to check whether apic is
> enabled in vcpu_scan_ioapic. So we must ensure rescan ioapic when apic state
> changed.
>> And I found recalculate_apic_map() doesn't track the enable/disable apic by
> software approach. So make_scan_ioapic_request in recalculate_apic_map() is
> not enough.
>> We also should force rescan ioapic when apic state is changed via
>> software approach(update spiv reg).
>> 
> 10.4.7.2 Local APIC State After It Has Been Software Disabled says:
> 
>   Pending interrupts in the IRR and ISR registers are held and require
>   masking or handling by the CPU.
> My understanding is that we should treat software disabled APIC as a
> valid target for an interrupt. vcpu_scan_ioapic() should check
> kvm_apic_hw_enabled() only.
Indeed. kvm_apic_hw_enabled() is the right one.

Best regards,
Yang


--
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
diff mbox

Patch

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 6796218..4ccdc94 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -134,11 +134,7 @@  static inline void apic_set_spiv(struct kvm_lapic *apic, u32 val)
 			static_key_slow_inc(&apic_sw_disabled.key);
 	}
 	apic_set_reg(apic, APIC_SPIV, val);
-}
-
-static inline int apic_enabled(struct kvm_lapic *apic)
-{
-	return kvm_apic_sw_enabled(apic) &&	kvm_apic_hw_enabled(apic);
+	kvm_make_request(KVM_REQ_SCAN_IOAPIC, apic->vcpu);
 }

As you mentioned, since it will call apic_enabled() to check whether apic is enabled in vcpu_scan_ioapic. So we must ensure rescan ioapic when apic state changed.