Re: [PATCH 3/5]KVM:x86, apicv: enable virtual interrupt delivery for VMX

2012-09-16 Thread Avi Kivity
On 09/14/2012 05:17 PM, Li, Jiongxi wrote:
 
  -static void apic_send_ipi(struct kvm_lapic *apic)
  +/*
  + * this interface assumes a trap-like exit, which has already
  +finished
  + * desired side effect including vISR and vPPR update.
  + */
  +void kvm_apic_set_eoi(struct kvm_vcpu *vcpu, int vector) {
  +  struct kvm_lapic *apic = vcpu-arch.apic;
  +  int trigger_mode;
  +
  +  if (apic_test_and_clear_vector(vector, apic-regs + APIC_TMR))
  +  trigger_mode = IOAPIC_LEVEL_TRIG;
  +  else
  +  trigger_mode = IOAPIC_EDGE_TRIG;
  +
  +  if (!(apic_get_reg(apic, APIC_SPIV)  APIC_SPIV_DIRECTED_EOI))
  +  kvm_ioapic_update_eoi(apic-vcpu-kvm, vector, trigger_mode);
  +  kvm_make_request(KVM_REQ_EVENT, apic-vcpu); }
  +EXPORT_SYMBOL_GPL(kvm_apic_set_eoi);
 
 What's the difference between this and apic_set_eoi()?

 In kvm_apic_set_eoi, We can use the vector directly from exit_qualification 
 while there is EOI-induced VMExit, and doesn't need to do the 'clear isr', 
 'update ppr' things which are handled by hardware.

The name needs to reflect this.  For most functions we can add _traplike
to the name, but here the hardware has done even more things for us, so
we can call it _accelerated.  Please add a comment detailing the
differences.

 
   /*
* If nested=1, nested virtualization is supported, i.e., guests may use
* VMX and be a hypervisor for its own guests. If nested=0, guests
  may not @@ -430,6 +433,8 @@ struct vcpu_vmx {
 
 bool rdtscp_enabled;
 
  +  u64 eoi_exit_bitmap[4];
  +
 
 Unused?
 This is used in PATCH 4/5

Then move it there please.

 
  +  if (enable_apicv_vid) {
  +  vmcs_write64(EOI_EXIT_BITMAP0, 0);
  +  vmcs_write64(EOI_EXIT_BITMAP1, 0);
  +  vmcs_write64(EOI_EXIT_BITMAP2, 0);
  +  vmcs_write64(EOI_EXIT_BITMAP3, 0);
  +
  +  vmcs_write16(GUEST_INTR_STATUS, 0);
 
 Need to update GUEST_INTR_STATUS after live migration (or perhaps also
 when enabling the APIC?)
 After live migration, GUEST_INTR_STATUS will be updated in VMEntry. 
 'kvm_x86_ops-update_irq(vcpu)' function does that.

Ok.


-- 
error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 3/5]KVM:x86, apicv: enable virtual interrupt delivery for VMX

2012-09-14 Thread Li, Jiongxi
Sorry for the late response

 -Original Message-
 From: Avi Kivity [mailto:a...@redhat.com]
 Sent: Friday, September 07, 2012 12:30 AM
 To: Li, Jiongxi
 Cc: kvm@vger.kernel.org
 Subject: Re: [PATCH 3/5]KVM:x86, apicv: enable virtual interrupt delivery for
 VMX
 
 On 09/05/2012 08:41 AM, Li, Jiongxi wrote:
  - before returning to guest, RVI should be updated if any pending IRRs
 
 process pending interrupts does that for you, so you only need this with
 KVM_SET_APIC.
 
  - EOI exit bitmap controls whether an EOI write should cause VM-Exit.
if set, a trap-like induced EOI VM-Exit is triggered. Keep all the
bitmaps cleared for now, which should be enough to allow a MSI based
device passthrough
 
 What about level-triggered interrupts, or interrupts which have ack notifiers
 set?
 
We will merge the EOI exit bitmap part patch
 
  -static void apic_send_ipi(struct kvm_lapic *apic)
  +/*
  + * this interface assumes a trap-like exit, which has already
  +finished
  + * desired side effect including vISR and vPPR update.
  + */
  +void kvm_apic_set_eoi(struct kvm_vcpu *vcpu, int vector) {
  +   struct kvm_lapic *apic = vcpu-arch.apic;
  +   int trigger_mode;
  +
  +   if (apic_test_and_clear_vector(vector, apic-regs + APIC_TMR))
  +   trigger_mode = IOAPIC_LEVEL_TRIG;
  +   else
  +   trigger_mode = IOAPIC_EDGE_TRIG;
  +
  +   if (!(apic_get_reg(apic, APIC_SPIV)  APIC_SPIV_DIRECTED_EOI))
  +   kvm_ioapic_update_eoi(apic-vcpu-kvm, vector, trigger_mode);
  +   kvm_make_request(KVM_REQ_EVENT, apic-vcpu); }
  +EXPORT_SYMBOL_GPL(kvm_apic_set_eoi);
 
 What's the difference between this and apic_set_eoi()?
In kvm_apic_set_eoi, We can use the vector directly from exit_qualification 
while there is EOI-induced VMExit, and doesn't need to do the 'clear isr', 
'update ppr' things which are handled by hardware.
 
  +
  + static void apic_send_ipi(struct kvm_lapic *apic)
 
 Extra space added.
OK.

 
   /*
* If nested=1, nested virtualization is supported, i.e., guests may use
* VMX and be a hypervisor for its own guests. If nested=0, guests
  may not @@ -430,6 +433,8 @@ struct vcpu_vmx {
 
  bool rdtscp_enabled;
 
  +   u64 eoi_exit_bitmap[4];
  +
 
 Unused?
This is used in PATCH 4/5

 
 
  @@ -3876,6 +3894,15 @@ static int vmx_vcpu_setup(struct vcpu_vmx
 *vmx)
  vmx_secondary_exec_control(vmx));
  }
 
  +   if (enable_apicv_vid) {
  +   vmcs_write64(EOI_EXIT_BITMAP0, 0);
  +   vmcs_write64(EOI_EXIT_BITMAP1, 0);
  +   vmcs_write64(EOI_EXIT_BITMAP2, 0);
  +   vmcs_write64(EOI_EXIT_BITMAP3, 0);
  +
  +   vmcs_write16(GUEST_INTR_STATUS, 0);
 
 Need to update GUEST_INTR_STATUS after live migration (or perhaps also
 when enabling the APIC?)
After live migration, GUEST_INTR_STATUS will be updated in VMEntry. 
'kvm_x86_ops-update_irq(vcpu)' function does that.

 
 
 
 --
 error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/5]KVM:x86, apicv: enable virtual interrupt delivery for VMX

2012-09-06 Thread Avi Kivity
On 09/05/2012 08:41 AM, Li, Jiongxi wrote:
 - before returning to guest, RVI should be updated if any pending IRRs

process pending interrupts does that for you, so you only need this with
KVM_SET_APIC.

 - EOI exit bitmap controls whether an EOI write should cause VM-Exit.
   if set, a trap-like induced EOI VM-Exit is triggered. Keep all the
   bitmaps cleared for now, which should be enough to allow a MSI based
   device passthrough

What about level-triggered interrupts, or interrupts which have ack
notifiers set?

  
 -static void apic_send_ipi(struct kvm_lapic *apic)
 +/*
 + * this interface assumes a trap-like exit, which has already finished
 + * desired side effect including vISR and vPPR update.
 + */
 +void kvm_apic_set_eoi(struct kvm_vcpu *vcpu, int vector)
 +{
 + struct kvm_lapic *apic = vcpu-arch.apic;
 + int trigger_mode;
 +
 + if (apic_test_and_clear_vector(vector, apic-regs + APIC_TMR))
 + trigger_mode = IOAPIC_LEVEL_TRIG;
 + else
 + trigger_mode = IOAPIC_EDGE_TRIG;
 +
 + if (!(apic_get_reg(apic, APIC_SPIV)  APIC_SPIV_DIRECTED_EOI))
 + kvm_ioapic_update_eoi(apic-vcpu-kvm, vector, trigger_mode);
 + kvm_make_request(KVM_REQ_EVENT, apic-vcpu);
 +}
 +EXPORT_SYMBOL_GPL(kvm_apic_set_eoi);

What's the difference between this and apic_set_eoi()?

 +
 + static void apic_send_ipi(struct kvm_lapic *apic)

Extra space added.

  /*
   * If nested=1, nested virtualization is supported, i.e., guests may use
   * VMX and be a hypervisor for its own guests. If nested=0, guests may not
 @@ -430,6 +433,8 @@ struct vcpu_vmx {
  
   bool rdtscp_enabled;
  
 + u64 eoi_exit_bitmap[4];
 +

Unused?

  
 @@ -3876,6 +3894,15 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx)
   vmx_secondary_exec_control(vmx));
   }
  
 + if (enable_apicv_vid) {
 + vmcs_write64(EOI_EXIT_BITMAP0, 0);
 + vmcs_write64(EOI_EXIT_BITMAP1, 0);
 + vmcs_write64(EOI_EXIT_BITMAP2, 0);
 + vmcs_write64(EOI_EXIT_BITMAP3, 0);
 +
 + vmcs_write16(GUEST_INTR_STATUS, 0);

Need to update GUEST_INTR_STATUS after live migration (or perhaps also
when enabling the APIC?)



-- 
error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/5]KVM:x86, apicv: enable virtual interrupt delivery for VMX

2012-09-04 Thread Li, Jiongxi
- before returning to guest, RVI should be updated if any pending IRRs
- EOI exit bitmap controls whether an EOI write should cause VM-Exit.
  if set, a trap-like induced EOI VM-Exit is triggered. Keep all the
  bitmaps cleared for now, which should be enough to allow a MSI based
  device passthrough

Signed-off-by: Kevin Tian kevin.t...@intel.com
Signed-off-by: Jiongxi Li jiongxi...@intel.com
---
 arch/x86/include/asm/vmx.h |   11 
 arch/x86/kvm/lapic.c   |   22 +++-
 arch/x86/kvm/lapic.h   |1 +
 arch/x86/kvm/vmx.c |   62 ++-
 4 files changed, 93 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index 4a8193e..b1eca96 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -60,6 +60,7 @@
 #define SECONDARY_EXEC_WBINVD_EXITING  0x0040
 #define SECONDARY_EXEC_UNRESTRICTED_GUEST  0x0080
 #define SECONDARY_EXEC_APIC_REGISTER_VIRT   0x0100
+#define SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY0x0200
 #define SECONDARY_EXEC_PAUSE_LOOP_EXITING  0x0400
 #define SECONDARY_EXEC_ENABLE_INVPCID  0x1000
 
@@ -97,6 +98,7 @@ enum vmcs_field {
GUEST_GS_SELECTOR   = 0x080a,
GUEST_LDTR_SELECTOR = 0x080c,
GUEST_TR_SELECTOR   = 0x080e,
+   GUEST_INTR_STATUS   = 0x0810,
HOST_ES_SELECTOR= 0x0c00,
HOST_CS_SELECTOR= 0x0c02,
HOST_SS_SELECTOR= 0x0c04,
@@ -124,6 +126,14 @@ enum vmcs_field {
APIC_ACCESS_ADDR_HIGH   = 0x2015,
EPT_POINTER = 0x201a,
EPT_POINTER_HIGH= 0x201b,
+   EOI_EXIT_BITMAP0= 0x201c,
+   EOI_EXIT_BITMAP0_HIGH   = 0x201d,
+   EOI_EXIT_BITMAP1= 0x201e,
+   EOI_EXIT_BITMAP1_HIGH   = 0x201f,
+   EOI_EXIT_BITMAP2= 0x2020,
+   EOI_EXIT_BITMAP2_HIGH   = 0x2021,
+   EOI_EXIT_BITMAP3= 0x2022,
+   EOI_EXIT_BITMAP3_HIGH   = 0x2023,
GUEST_PHYSICAL_ADDRESS  = 0x2400,
GUEST_PHYSICAL_ADDRESS_HIGH = 0x2401,
VMCS_LINK_POINTER   = 0x2800,
@@ -279,6 +289,7 @@ enum vmcs_field {
 #define EXIT_REASON_MCE_DURING_VMENTRY  41
 #define EXIT_REASON_TPR_BELOW_THRESHOLD 43
 #define EXIT_REASON_APIC_ACCESS 44
+#define EXIT_REASON_EOI_INDUCED 45
 #define EXIT_REASON_EPT_VIOLATION   48
 #define EXIT_REASON_EPT_MISCONFIG   49
 #define EXIT_REASON_WBINVD 54
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index c47f3d3..d203501 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -604,7 +604,27 @@ static int apic_set_eoi(struct kvm_lapic *apic)
return vector;
 }
 
-static void apic_send_ipi(struct kvm_lapic *apic)
+/*
+ * this interface assumes a trap-like exit, which has already finished
+ * desired side effect including vISR and vPPR update.
+ */
+void kvm_apic_set_eoi(struct kvm_vcpu *vcpu, int vector)
+{
+   struct kvm_lapic *apic = vcpu-arch.apic;
+   int trigger_mode;
+
+   if (apic_test_and_clear_vector(vector, apic-regs + APIC_TMR))
+   trigger_mode = IOAPIC_LEVEL_TRIG;
+   else
+   trigger_mode = IOAPIC_EDGE_TRIG;
+
+   if (!(apic_get_reg(apic, APIC_SPIV)  APIC_SPIV_DIRECTED_EOI))
+   kvm_ioapic_update_eoi(apic-vcpu-kvm, vector, trigger_mode);
+   kvm_make_request(KVM_REQ_EVENT, apic-vcpu);
+}
+EXPORT_SYMBOL_GPL(kvm_apic_set_eoi);
+
+ static void apic_send_ipi(struct kvm_lapic *apic)
 {
u32 icr_low = apic_get_reg(apic, APIC_ICR);
u32 icr_high = apic_get_reg(apic, APIC_ICR2);
diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
index 4e3b435..585337f 100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -60,6 +60,7 @@ u64 kvm_get_lapic_tscdeadline_msr(struct kvm_vcpu *vcpu);
 void kvm_set_lapic_tscdeadline_msr(struct kvm_vcpu *vcpu, u64 data);
 
 int kvm_apic_write_nodecode(struct kvm_vcpu *vcpu, u32 offset);
+void kvm_apic_set_eoi(struct kvm_vcpu *vcpu, int vector);
 
 void kvm_lapic_set_vapic_addr(struct kvm_vcpu *vcpu, gpa_t vapic_addr);
 void kvm_lapic_sync_from_vapic(struct kvm_vcpu *vcpu);
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 4a26d04..424a09d 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -86,6 +86,9 @@ module_param(fasteoi, bool, S_IRUGO);
 static bool __read_mostly enable_apicv_reg = 0;
 module_param(enable_apicv_reg, bool, S_IRUGO);
 
+static bool __read_mostly enable_apicv_vid = 0;
+module_param(enable_apicv_vid, bool, S_IRUGO);
+
 /*
  * If nested=1, nested virtualization is supported, i.e., guests may use
  * VMX and be a hypervisor for its own guests. If nested=0, guests may not
@@