All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Zhang, Yang Z" <yang.z.zhang@intel.com>
To: Marcelo Tosatti <mtosatti@redhat.com>
Cc: "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"gleb@redhat.com" <gleb@redhat.com>,
	"Shan, Haitao" <haitao.shan@intel.com>,
	"Zhang, Xiantao" <xiantao.zhang@intel.com>,
	"Nakajima, Jun" <jun.nakajima@intel.com>,
	"Anvin, H Peter" <h.peter.anvin@intel.com>
Subject: RE: [PATCH 2/2] x86, apicv: Add Posted Interrupt supporting
Date: Fri, 25 Jan 2013 00:40:21 +0000	[thread overview]
Message-ID: <A9667DDFB95DB7438FA9D7D576C3D87E31AB93@SHSMSX101.ccr.corp.intel.com> (raw)
In-Reply-To: <20130124234339.GA18719@amt.cnet>

Marcelo Tosatti wrote on 2013-01-25:
> On Thu, Dec 13, 2012 at 03:29:40PM +0800, Yang Zhang wrote:
>> From: Yang Zhang <yang.z.zhang@Intel.com>
>> 
>> 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.
>> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
>> ---
>>  arch/x86/include/asm/entry_arch.h  |    1 +
>>  arch/x86/include/asm/hw_irq.h      |    1 + arch/x86/include/asm/irq.h
>>          |    1 + arch/x86/include/asm/irq_vectors.h |    4 +
>>  arch/x86/include/asm/kvm_host.h    |    3 + arch/x86/include/asm/vmx.h
>>          |    4 + arch/x86/kernel/entry_64.S         |    2 +
>>  arch/x86/kernel/irq.c              |   25 +++++++
>>  arch/x86/kernel/irqinit.c          |    2 + arch/x86/kvm/lapic.c      
>>          |   16 +++- arch/x86/kvm/lapic.h               |    1 +
>>  arch/x86/kvm/vmx.c                 |  133
>>  +++++++++++++++++++++++++++++++++--- 12 files changed, 180
>>  insertions(+), 13 deletions(-)
>> diff --git a/arch/x86/include/asm/entry_arch.h
>> b/arch/x86/include/asm/entry_arch.h index 40afa00..7b0a29e 100644 ---
>> a/arch/x86/include/asm/entry_arch.h +++
>> b/arch/x86/include/asm/entry_arch.h @@ -18,6 +18,7 @@
>> BUILD_INTERRUPT(reboot_interrupt,REBOOT_VECTOR)
>>  #endif
>>  
>>  BUILD_INTERRUPT(x86_platform_ipi, X86_PLATFORM_IPI_VECTOR)
>> +BUILD_INTERRUPT(posted_intr_ipi, POSTED_INTR_VECTOR)
>> 
>>  /*
>>   * every pentium local APIC has two 'local interrupts', with a
>> diff --git a/arch/x86/include/asm/hw_irq.h b/arch/x86/include/asm/hw_irq.h
>> index eb92a6e..ee61af3 100644
>> --- a/arch/x86/include/asm/hw_irq.h
>> +++ b/arch/x86/include/asm/hw_irq.h
>> @@ -28,6 +28,7 @@
>>  /* Interrupt handlers registered during init_IRQ */ extern void
>>  apic_timer_interrupt(void); extern void x86_platform_ipi(void);
>>  +extern void posted_intr_ipi(void); extern void error_interrupt(void);
>>  extern void irq_work_interrupt(void);
>> diff --git a/arch/x86/include/asm/irq.h b/arch/x86/include/asm/irq.h
>> index ba870bb..cff9933 100644
>> --- a/arch/x86/include/asm/irq.h
>> +++ b/arch/x86/include/asm/irq.h
>> @@ -30,6 +30,7 @@ extern void irq_force_complete_move(int);
>>  #endif
>>  
>>  extern void (*x86_platform_ipi_callback)(void); +extern void
>>  (*posted_intr_callback)(void); extern void native_init_IRQ(void);
>>  extern bool handle_irq(unsigned irq, struct pt_regs *regs);
>> diff --git a/arch/x86/include/asm/irq_vectors.h
>> b/arch/x86/include/asm/irq_vectors.h index 1508e51..8f2e383 100644 ---
>> a/arch/x86/include/asm/irq_vectors.h +++
>> b/arch/x86/include/asm/irq_vectors.h @@ -102,6 +102,10 @@
>>   */
>>  #define X86_PLATFORM_IPI_VECTOR		0xf7
>> +#ifdef CONFIG_HAVE_KVM
>> +#define POSTED_INTR_VECTOR 		0xf2
>> +#endif
>> +
>>  /*
>>   * IRQ work vector:
>>   */
>> diff --git a/arch/x86/include/asm/kvm_host.h
>> b/arch/x86/include/asm/kvm_host.h index 7e26d1a..82423a8 100644 ---
>> a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h
>> @@ -700,6 +700,9 @@ struct kvm_x86_ops {
>>  	int (*has_virtual_interrupt_delivery)(struct kvm_vcpu *vcpu);
>>  	void (*update_irq)(struct kvm_vcpu *vcpu);
>>  	void (*update_eoi_exitmap)(struct kvm_vcpu *vcpu, int vector, bool set);
>> +	int (*has_posted_interrupt)(struct kvm_vcpu *vcpu);
>> +	int (*send_nv)(struct kvm_vcpu *vcpu, int vector);
>> +	void (*update_irr)(struct kvm_vcpu *vcpu);
>>  	int (*set_tss_addr)(struct kvm *kvm, unsigned int addr);
>>  	int (*get_tdp_level)(void);
>>  	u64 (*get_mt_mask)(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio);
>> diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
>> index 1003341..7b9e1d0 100644
>> --- a/arch/x86/include/asm/vmx.h
>> +++ b/arch/x86/include/asm/vmx.h
>> @@ -152,6 +152,7 @@
>>  #define PIN_BASED_EXT_INTR_MASK                 0x00000001
>>  #define PIN_BASED_NMI_EXITING                   0x00000008
>>  #define PIN_BASED_VIRTUAL_NMIS                  0x00000020
>> +#define PIN_BASED_POSTED_INTR                   0x00000080
>> 
>>  #define VM_EXIT_SAVE_DEBUG_CONTROLS             0x00000002 #define
>>  VM_EXIT_HOST_ADDR_SPACE_SIZE            0x00000200 @@ -174,6 +175,7 @@
>>  /* VMCS Encodings */ enum vmcs_field { 	VIRTUAL_PROCESSOR_ID          
>>   = 0x00000000, +	POSTED_INTR_NV                  = 0x00000002,
>>  	GUEST_ES_SELECTOR               = 0x00000800, 	GUEST_CS_SELECTOR     
>>           = 0x00000802, 	GUEST_SS_SELECTOR               = 0x00000804,
>>  @@ -208,6 +210,8 @@ enum vmcs_field { 	VIRTUAL_APIC_PAGE_ADDR_HIGH    
>>  = 0x00002013, 	APIC_ACCESS_ADDR		= 0x00002014,
>>  	APIC_ACCESS_ADDR_HIGH		= 0x00002015,
>> +	POSTED_INTR_DESC_ADDR           = 0x00002016,
>> +	POSTED_INTR_DESC_ADDR_HIGH      = 0x00002017,
>>  	EPT_POINTER                     = 0x0000201a,
>>  	EPT_POINTER_HIGH                = 0x0000201b,
>>  	EOI_EXIT_BITMAP0                = 0x0000201c,
>> diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
>> index b51b2c7..d06eea1 100644
>> --- a/arch/x86/kernel/entry_64.S
>> +++ b/arch/x86/kernel/entry_64.S
>> @@ -1160,6 +1160,8 @@ apicinterrupt LOCAL_TIMER_VECTOR \
>>  	apic_timer_interrupt smp_apic_timer_interrupt
>>  apicinterrupt X86_PLATFORM_IPI_VECTOR \
>>  	x86_platform_ipi smp_x86_platform_ipi
>> +apicinterrupt POSTED_INTR_VECTOR \
>> +	posted_intr_ipi smp_posted_intr_ipi
>> 
>>  apicinterrupt THRESHOLD_APIC_VECTOR \
>>  	threshold_interrupt smp_threshold_interrupt
>> diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
>> index e4595f1..781d324 100644
>> --- a/arch/x86/kernel/irq.c
>> +++ b/arch/x86/kernel/irq.c
>> @@ -22,6 +22,9 @@ atomic_t irq_err_count;
>> 
>>  /* Function pointer for generic interrupt vector handling */
>>  void (*x86_platform_ipi_callback)(void) = NULL;
>> +/* Function pointer for posted interrupt vector handling */
>> +void (*posted_intr_callback)(void) = NULL;
>> +EXPORT_SYMBOL_GPL(posted_intr_callback);
>> 
>>  /*
>>   * 'what should we do if we get a hw irq event on an illegal vector'.
>> @@ -228,6 +231,28 @@ void smp_x86_platform_ipi(struct pt_regs *regs)
>>  	set_irq_regs(old_regs);
>>  }
>> +/*
>> + * Handler for POSTED_INTERRUPT_VECTOR.
>> + */
>> +void smp_posted_intr_ipi(struct pt_regs *regs)
>> +{
>> +	struct pt_regs *old_regs = set_irq_regs(regs);
>> +
>> +	ack_APIC_irq();
>> +
>> +	irq_enter();
>> +
>> +	exit_idle();
>> +
>> +	if (posted_intr_callback)
>> +		posted_intr_callback();
>> +
>> +	irq_exit();
>> +
>> +	set_irq_regs(old_regs);
>> +}
>> +
>> +
>>  EXPORT_SYMBOL_GPL(vector_used_by_percpu_irq);
>>  
>>  #ifdef CONFIG_HOTPLUG_CPU
>> diff --git a/arch/x86/kernel/irqinit.c b/arch/x86/kernel/irqinit.c
>> index 6e03b0d..d15ca4f 100644
>> --- a/arch/x86/kernel/irqinit.c
>> +++ b/arch/x86/kernel/irqinit.c
>> @@ -205,6 +205,8 @@ static void __init apic_intr_init(void)
>> 
>>  	/* IPI for X86 platform specific use */
>>  	alloc_intr_gate(X86_PLATFORM_IPI_VECTOR, x86_platform_ipi);
>> +	/* IPI for posted interrupt use */
>> +	alloc_intr_gate(POSTED_INTR_VECTOR, posted_intr_ipi);
>> 
>>  	/* IPI vectors for APIC spurious and error interrupts */
>>  	alloc_intr_gate(SPURIOUS_APIC_VECTOR, spurious_interrupt);
>> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
>> index 2109a6a..d660b9d 100644
>> --- a/arch/x86/kvm/lapic.c
>> +++ b/arch/x86/kvm/lapic.c
>> @@ -350,6 +350,7 @@ static inline int apic_find_highest_irr(struct kvm_lapic
> *apic)
>>  	if (!apic->irr_pending)
>>  		return -1;
>> +	kvm_x86_ops->update_irr(apic->vcpu);
>>  	result = apic_search_irr(apic);
>>  	ASSERT(result == -1 || result >= 16);
>> @@ -725,18 +726,25 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int
> delivery_mode,
>>  		if (trig_mode) {
>>  			apic_debug("level trig mode for vector %d", vector);
>>  			apic_set_vector(vector, apic->regs + APIC_TMR);
>> -		} else
>> +		} else {
>>  			apic_clear_vector(vector, apic->regs + APIC_TMR);
>> -
>> +			if (kvm_x86_ops->has_posted_interrupt(vcpu)) {
>> +				result = 1;
>> +				apic->irr_pending = true;
>> +				kvm_x86_ops->send_nv(vcpu, vector);
>> +				goto out;
>> +			}
> 
> Hi,
> 
> Steps 4, 5 and 6 of section 29.6 are executed in both VMX root/non-root
> modes, or only non-root mode?
SDM doesn't tell. But we don't need know this in software level.

>
>
> If only non-root mode, there is a problem if target vcpu<->pcpu vm-exits
> before receiving and acking the interrupt. In that case PIR set bits are
> not transferred to VIRR.
>
> It would be necessary to read notification bit on VM-exit and, if set,
> do PIR->VIRR transfer in software. The downside, is lack of an atomic
In current implementation, it will sync PIR to VIRR before vmentry.

> (VIRR |= PIR; PIR = 0) in software. So it would require synchronization
> to KVM APIC injection (which ATM relies on atomic test_and_set of IRR).


Best regards,
Yang



  reply	other threads:[~2013-01-25  0:40 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-12-13  7:29 [PATCH 0/2] KVM: Add posted interrupt supporting Yang Zhang
2012-12-13  7:29 ` [PATCH 1/2] x86: Enable ack interrupt on vmexit Yang Zhang
2012-12-13  7:51   ` Gleb Natapov
2012-12-13  7:54     ` Zhang, Yang Z
2012-12-13  7:58       ` Gleb Natapov
2012-12-13  8:03         ` Zhang, Yang Z
2012-12-13  8:05           ` Gleb Natapov
2012-12-13  8:19             ` Zhang, Yang Z
2012-12-13  8:22               ` Gleb Natapov
2012-12-13  8:23                 ` Zhang, Yang Z
2012-12-16 13:26                 ` Zhang, Yang Z
2012-12-18  9:11                   ` Gleb Natapov
2012-12-13  7:29 ` [PATCH 2/2] x86, apicv: Add Posted Interrupt supporting Yang Zhang
2013-01-22 22:59   ` Marcelo Tosatti
2013-01-23  5:09     ` Zhang, Yang Z
2013-01-24 23:43   ` Marcelo Tosatti
2013-01-25  0:40     ` Zhang, Yang Z [this message]
2013-01-30 23:03       ` Marcelo Tosatti
2013-01-30 23:57         ` Marcelo Tosatti
2013-01-31  7:35         ` Gleb Natapov
2013-01-31  9:43         ` Gleb Natapov
2013-01-31 13:32           ` Marcelo Tosatti
2013-01-31 13:38             ` Gleb Natapov
2013-01-31 13:44               ` Marcelo Tosatti
2013-01-31 13:55                 ` Gleb Natapov
2013-02-04  0:57                   ` Marcelo Tosatti
2013-02-04  9:10                     ` Zhang, Yang Z
2013-02-04  9:55                     ` Gleb Natapov
2013-02-04 14:43                       ` Marcelo Tosatti
2013-02-04 17:13                         ` Gleb Natapov
2013-02-04 19:59                           ` Marcelo Tosatti
2013-02-04 20:47                             ` Marcelo Tosatti
2013-02-05  5:57                               ` Zhang, Yang Z
2013-02-05  8:00                                 ` Gleb Natapov
2013-02-05 10:35                                   ` Zhang, Yang Z
2013-02-05 10:54                                     ` Gleb Natapov
2013-02-05 10:58                                       ` Zhang, Yang Z
2013-02-05 11:16                                         ` Gleb Natapov
2013-02-05 13:26                                           ` Zhang, Yang Z
2013-02-05 13:29                                             ` Gleb Natapov
2013-02-05 13:40                                               ` Zhang, Yang Z
2013-02-05 13:43                                                 ` Gleb Natapov
2013-02-07  1:23                                                 ` Marcelo Tosatti
2013-02-05  7:32                               ` Gleb Natapov
2013-02-06 22:49                                 ` Marcelo Tosatti
2013-02-07  0:24                                   ` Marcelo Tosatti
2013-02-07 13:52                                     ` Gleb Natapov
2013-02-08  2:07                                       ` Marcelo Tosatti
2013-02-08 12:18                                         ` Gleb Natapov
2013-02-07 14:01                                   ` Gleb Natapov
2013-02-07 21:49                                     ` Marcelo Tosatti
2013-02-08 12:28                                       ` Gleb Natapov
2013-02-08 13:46                                         ` Marcelo Tosatti
2013-01-31  9:37   ` Gleb Natapov
2013-02-04  9:11     ` Zhang, Yang Z

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=A9667DDFB95DB7438FA9D7D576C3D87E31AB93@SHSMSX101.ccr.corp.intel.com \
    --to=yang.z.zhang@intel.com \
    --cc=gleb@redhat.com \
    --cc=h.peter.anvin@intel.com \
    --cc=haitao.shan@intel.com \
    --cc=jun.nakajima@intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=mtosatti@redhat.com \
    --cc=xiantao.zhang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.