RE: [PATCH] kvm/vmx: EPTP switching test

2015-11-17 Thread Wang, Wei W
On 17/11/2015 18:18,  Paolo Bonzini wrote:
> On 17/11/2015 02:45, Zhang, Yang Z wrote:
> > We have a different version in hand which is using separate EPTP.
> 
> Can you say in advance what you are using EPTP switching for?  Offlist if
> necessary.

Hi Paolo,

We are using EPTP switching for a protected inter-VM communication design, as 
shown in the slides (#23) here: 
http://events.linuxfoundation.org/sites/events/files/slides/Jun_Nakajima_NFV_KVM%202015_final.pdf


Best,
Wei
N�r��yb�X��ǧv�^�)޺{.n�+h����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf

RE: A question about vring operation

2015-08-27 Thread Wang, Wei W

On 24/07/2015 21:36,  Michael S. Tsirkin wrote:
On Thu, Aug 27, 2015 at 01:18:47AM +, Wang, Wei W wrote:
 Hi all,
 
  
 
 I have a question about the vring_avail:
 
 It only includes an idx (equivalent to the ring tail), which is used 
 by the frontend (virtio_net) to fill bufs. The backend (e.g. 
 vhost_net) maintains the ring head (last_avail_idx) by itself. The 
 frontend checks if the ring is full or empty via a counter (vq-num_free).
 
 My question is why can’t we include the ring head in the  vring_avail 
 struct, so that the vq-num_free is not needed, and the backend can 
 directly use it without maintaining its own copy?
 
  
 
 Thanks,
 
 Wei
 
  
 
I'm not sure I understand your proposal, and what it would accomplish. Write a 
patch, that'll make it easier to discuss.
Also copy all relevant mailing lists, not just kvm.

Thanks Michael. I haven’t got the patch ready yet. I am just wondering if it is 
better to have last_avail_idx in vhost_virtqueue moved into vring_avail.

In other regular ring operations, we usually use *head and *tail to judge if 
the ring is full or empty, but it seems vring_avail does not use this method in 
virtio_net.c (it uses a counter). 

Best,
Wei



RE: [PATCH] KVM: x86: reset RVI upon system reset

2014-11-05 Thread Wang, Wei W
On 05/11/2014 4:07, Tiejun Chen wrote:
  A bug was reported as follows: when running Windows 7 32-bit guests
  on qemu-kvm, sometimes the guests run into blue screen during
  reboot. The problem was that a guest's RVI was not cleared when it
  rebooted. This
  patch has fixed the problem.
 
  Signed-off-by: Wei Wang wei.w.w...@intel.com
  Signed-off-by: Yang Zhang yang.z.zh...@intel.com
  Tested-by: Rongrong Liu rongrongx@intel.com, Da Chun
  ng...@qq.com
  ---
 arch/x86/kvm/lapic.c |3 +++
 arch/x86/kvm/vmx.c   |   12 ++--
 2 files changed, 9 insertions(+), 6 deletions(-)
 
  diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index
  66dd173..6942742 100644
  --- a/arch/x86/kvm/lapic.c
  +++ b/arch/x86/kvm/lapic.c
  @@ -1712,6 +1712,9 @@ void kvm_apic_post_state_restore(struct
  kvm_vcpu *vcpu,
apic-isr_count = kvm_apic_vid_enabled(vcpu-kvm) ?
1 : count_vectors(apic-regs + 
  APIC_ISR);
apic-highest_isr_cache = -1;
  + if (kvm_x86_ops-hwapic_irr_update)
  + kvm_x86_ops-hwapic_irr_update(vcpu,
  + apic_find_highest_irr(apic));
 
  Could we pass 0 directly? Because looks we just need to clear RVI.
 
  kvm_x86_ops-hwapic_irr_update(vcpu, 0);
 
  I think this already makes sense based on your description.
 
  Thanks
  Tiejun
 
  No. This is a restore function, and we cannot assume that the callers
 always need to reset to the initial state.
 
 Okay. Maybe I'm confused by the following change.
 
 
  Wei
 
kvm_x86_ops-hwapic_isr_update(vcpu-kvm,
  apic_find_highest_isr(apic));
kvm_make_request(KVM_REQ_EVENT, vcpu);
kvm_rtc_eoi_tracking_restore_one(vcpu);
  diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index
  fe4d2f4..d632548 100644
  --- a/arch/x86/kvm/vmx.c
  +++ b/arch/x86/kvm/vmx.c
  @@ -7292,19 +7292,19 @@ static void vmx_set_rvi(int vector)
 static void vmx_hwapic_irr_update(struct kvm_vcpu *vcpu, int max_irr)
 {
if (max_irr == -1)
  + max_irr = 0;
  +
  + if (!is_guest_mode(vcpu)) {
  + vmx_set_rvi(max_irr);
return;
  + }
 
/*
 * If a vmexit is needed, vmx_check_nested_events handles it.
 */
  - if (is_guest_mode(vcpu)  nested_exit_on_intr(vcpu))
  + if ((is_guest_mode(vcpu)  nested_exit_on_intr(vcpu)) || max_irr
  ==
  +0)
 
 Its really not readable to modify max_irr as 0 and check that here, and
 especially when you read the original comment.
 
 So what about this?


I think both are ok.
If we zero max_irr in vmx_set_rvi(), we still need this check:
if ((is_guest_mode(vcpu)  nested_exit_on_intr(vcpu)) || max_irr == -1)

Let's see if Paolo has any comments, then send out a second version.

Wei
 
 diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index
 0cd99d8..bc4558b 100644
 --- a/arch/x86/kvm/vmx.c
 +++ b/arch/x86/kvm/vmx.c
 @@ -7280,6 +7280,9 @@ static void vmx_set_rvi(int vector)
  u16 status;
  u8 old;
 
 +   if (vector == -1)
 +   vector = 0;
 +
  status = vmcs_read16(GUEST_INTR_STATUS);
  old = (u8)status  0xff;
  if ((u8)vector != old) {
 @@ -7291,9 +7294,6 @@ static void vmx_set_rvi(int vector)
 
   static void vmx_hwapic_irr_update(struct kvm_vcpu *vcpu, int max_irr)
   {
 -   if (max_irr == -1)
 -   return;
 -
  /*
   * If a vmexit is needed, vmx_check_nested_events handles it.
   */
 @@ -7305,6 +7305,9 @@ static void vmx_hwapic_irr_update(struct
 kvm_vcpu *vcpu, int max_irr)
  return;
  }
 
 +   if (max_irr == -1)
 +   return;
 +
  /*
   * Fall back to pre-APICv interrupt injection since L2
   * is run without virtual interrupt delivery.
 
 
 Thanks
 Tiejun
 
return;
 
  - if (!is_guest_mode(vcpu)) {
  - vmx_set_rvi(max_irr);
  - return;
  - }
  -
/*
 * Fall back to pre-APICv interrupt injection since L2
 * is run without virtual interrupt delivery.
 
 
 
--
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] KVM: x86: reset RVI upon system reset

2014-11-04 Thread Wang, Wei W
On 05/11/2014 2:14, Tiejun Chen wrote:
  A bug was reported as follows: when running Windows 7 32-bit guests on
  qemu-kvm, sometimes the guests run into blue screen during reboot. The
  problem was that a guest's RVI was not cleared when it rebooted. This
 patch has fixed the problem.
 
  Signed-off-by: Wei Wang wei.w.w...@intel.com
  Signed-off-by: Yang Zhang yang.z.zh...@intel.com
  Tested-by: Rongrong Liu rongrongx@intel.com, Da Chun
  ng...@qq.com
  ---
arch/x86/kvm/lapic.c |3 +++
arch/x86/kvm/vmx.c   |   12 ++--
2 files changed, 9 insertions(+), 6 deletions(-)
 
  diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index
  66dd173..6942742 100644
  --- a/arch/x86/kvm/lapic.c
  +++ b/arch/x86/kvm/lapic.c
  @@ -1712,6 +1712,9 @@ void kvm_apic_post_state_restore(struct
 kvm_vcpu *vcpu,
  apic-isr_count = kvm_apic_vid_enabled(vcpu-kvm) ?
  1 : count_vectors(apic-regs + APIC_ISR);
  apic-highest_isr_cache = -1;
  +   if (kvm_x86_ops-hwapic_irr_update)
  +   kvm_x86_ops-hwapic_irr_update(vcpu,
  +   apic_find_highest_irr(apic));
 
 Could we pass 0 directly? Because looks we just need to clear RVI.
 
 kvm_x86_ops-hwapic_irr_update(vcpu, 0);
 
 I think this already makes sense based on your description.
 
 Thanks
 Tiejun

No. This is a restore function, and we cannot assume that the callers always 
need to reset to the initial state.

Wei
 
  kvm_x86_ops-hwapic_isr_update(vcpu-kvm,
 apic_find_highest_isr(apic));
  kvm_make_request(KVM_REQ_EVENT, vcpu);
  kvm_rtc_eoi_tracking_restore_one(vcpu);
  diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index
  fe4d2f4..d632548 100644
  --- a/arch/x86/kvm/vmx.c
  +++ b/arch/x86/kvm/vmx.c
  @@ -7292,19 +7292,19 @@ static void vmx_set_rvi(int vector)
static void vmx_hwapic_irr_update(struct kvm_vcpu *vcpu, int max_irr)
{
  if (max_irr == -1)
  +   max_irr = 0;
  +
  +   if (!is_guest_mode(vcpu)) {
  +   vmx_set_rvi(max_irr);
  return;
  +   }
 
  /*
   * If a vmexit is needed, vmx_check_nested_events handles it.
   */
  -   if (is_guest_mode(vcpu)  nested_exit_on_intr(vcpu))
  +   if ((is_guest_mode(vcpu)  nested_exit_on_intr(vcpu)) || max_irr
 ==
  +0)
  return;
 
  -   if (!is_guest_mode(vcpu)) {
  -   vmx_set_rvi(max_irr);
  -   return;
  -   }
  -
  /*
   * Fall back to pre-APICv interrupt injection since L2
   * is run without virtual interrupt delivery.
 
--
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 v2] KVM: x86: keep eoi exit bitmap accurate before loading it.

2014-09-03 Thread Wang, Wei W
 -Original Message-
 From: kvm-ow...@vger.kernel.org [mailto:kvm-ow...@vger.kernel.org] On
 Behalf Of Wang, Wei W
 Sent: Friday, August 29, 2014 8:59 AM
 To: Paolo Bonzini; Zhang, Yang Z; kvm@vger.kernel.org
 Cc: alex.william...@redhat.com
 Subject: RE: [PATCH v2] KVM: x86: keep eoi exit bitmap accurate before
 loading it.
 
 I think we can think about it for another couple of days and see if any corner
 case is not covered.
 
 Wei

Hi Paolo, we have not found any corner case. Do you still have any concerns? If 
not, I can email out the two patches with your suggested commit messages. 
If anyone find a bug in the future, we can also get back to solve it.

Wei
 
  -Original Message-
  From: Paolo Bonzini [mailto:pbonz...@redhat.com]
  Sent: Thursday, August 28, 2014 7:01 PM
  To: Wang, Wei W; Zhang, Yang Z; kvm@vger.kernel.org
  Cc: alex.william...@redhat.com
  Subject: Re: [PATCH v2] KVM: x86: keep eoi exit bitmap accurate before
  loading it.
 
  Il 28/08/2014 12:14, Wang, Wei W ha scritto:
   We will do some more tests on it to make sure there are no problems.
 
  No, I don't think there are any easily-detected practical problems
  with the patch.  But I'm not sure I understand all the theoretical
  problems and whether possible races are benign.  These would be really
  hard to debug, unless you get a bug that is 100% reproducible.
 
  Paolo
 --
 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
--
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 v2] KVM: x86: keep eoi exit bitmap accurate before loading it.

2014-08-28 Thread Wang, Wei W
We will do some more tests on it to make sure there are no problems.
Wei

-Original Message-
From: Paolo Bonzini [mailto:paolo.bonz...@gmail.com] On Behalf Of Paolo Bonzini
Sent: Thursday, August 28, 2014 4:44 PM
To: Zhang, Yang Z; Wang, Wei W; kvm@vger.kernel.org
Cc: alex.william...@redhat.com
Subject: Re: [PATCH v2] KVM: x86: keep eoi exit bitmap accurate before loading 
it.

Il 28/08/2014 08:17, Zhang, Yang Z ha scritto:
  Program edge-triggered MSI for vector 123
  Interrupt comes in, ISR[123]=1
  Mask MSI
  Program level-triggered IOAPIC interrupt for vector 123
 
 You cannot assign the vector 123 to another trigger mode interrupt 
 before previous IRQ handler finished (means issue EOI). If there is an 
 interrupt comes from the IOAPIC entry immediately after you reprogram 
 the entry, it will update the TMR to 1. Since we are still in previous 
 IRQ handler, it will get confused to see the TMR becomes 1.

Yeah, that could be confusing to real hardware as well.  Still, I'm a bit 
nervous at the possibility of races introduced by these patches...

I wouldn't mind a second review.

Paolo
--
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 v2] KVM: x86: keep eoi exit bitmap accurate before loading it.

2014-08-28 Thread Wang, Wei W
I think we can think about it for another couple of days and see if any corner 
case is not covered.
 
Wei

 -Original Message-
 From: Paolo Bonzini [mailto:pbonz...@redhat.com]
 Sent: Thursday, August 28, 2014 7:01 PM
 To: Wang, Wei W; Zhang, Yang Z; kvm@vger.kernel.org
 Cc: alex.william...@redhat.com
 Subject: Re: [PATCH v2] KVM: x86: keep eoi exit bitmap accurate before
 loading it.
 
 Il 28/08/2014 12:14, Wang, Wei W ha scritto:
  We will do some more tests on it to make sure there are no problems.
 
 No, I don't think there are any easily-detected practical problems with the
 patch.  But I'm not sure I understand all the theoretical problems and
 whether possible races are benign.  These would be really hard to debug,
 unless you get a bug that is 100% reproducible.
 
 Paolo
--
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] KVM: x86: keep EOI exit bitmap accurate before loading it.

2014-08-26 Thread Wang, Wei W
Hi Alex,
Could you have a test if the two patches have solved the bug?

Thanks,
Wei

-Original Message-
From: Wang, Wei W 
Sent: Wednesday, August 27, 2014 12:28 AM
To: kvm@vger.kernel.org
Cc: pbonz...@redhat.com; alex.william...@redhat.com; Wang, Wei W; Zhang, Yang Z
Subject: [PATCH] KVM: x86: keep EOI exit bitmap accurate before loading it.

Guest may mask the IOAPIC entry before issue EOI. In such case, EOI will not be 
intercepted by hypervisor, since the corrensponding bit in eoi exit bitmap is 
not set after the masking of IOAPIC entry.

The solution here is to OR EOI_exit_bitmap with tmr.

Tested-by: Rongrong Liu rongrongx@intel.com
Signed-off-by: Yang Zhang yang.z.zh...@intel.com
Signed-off-by: Wei Wang wei.w.w...@intel.com
---
 arch/x86/kvm/lapic.c |9 +
 arch/x86/kvm/lapic.h |2 ++
 arch/x86/kvm/x86.c   |1 +
 virt/kvm/ioapic.c|6 +++---
 4 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index 93c2e93..759d24e 
100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -533,6 +533,15 @@ void kvm_apic_update_tmr(struct kvm_vcpu *vcpu, u32 *tmr)
}
 }
 
+void kvm_apic_update_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap,
+   u32 *tmr)
+{
+   u32 i;
+
+   for (i = 0; i  8; i++)
+   *((u32 *)eoi_exit_bitmap + i) |= tmr[i]; }
+
 static void apic_update_ppr(struct kvm_lapic *apic)  {
u32 tpr, isrv, ppr, old_ppr;
diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h index 6a11845..eda7be7 
100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -55,6 +55,8 @@ void kvm_apic_set_version(struct kvm_vcpu *vcpu);
 
 void kvm_apic_update_tmr(struct kvm_vcpu *vcpu, u32 *tmr);  void 
kvm_apic_update_irr(struct kvm_vcpu *vcpu, u32 *pir);
+void kvm_apic_update_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap,
+   u32 *tmr);
 int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest);  int 
kvm_apic_match_logical_addr(struct kvm_lapic *apic, u8 mda);  int 
kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq, diff --git 
a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index d401684..4042bc0 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5992,6 +5992,7 @@ static void vcpu_scan_ioapic(struct kvm_vcpu *vcpu)
 
kvm_ioapic_scan_entry(vcpu, eoi_exit_bitmap, tmr);
kvm_apic_update_tmr(vcpu, tmr);
+   kvm_apic_update_eoi_exitmap(vcpu, eoi_exit_bitmap, tmr);
kvm_x86_ops-load_eoi_exitmap(vcpu, eoi_exit_bitmap);  }
 
diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c index e8ce34c..ea5f697 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -254,9 +254,9 @@ void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 
*eoi_exit_bitmap,
spin_lock(ioapic-lock);
for (index = 0; index  IOAPIC_NUM_PINS; index++) {
e = ioapic-redirtbl[index];
-   if (e-fields.trig_mode == IOAPIC_LEVEL_TRIG ||
-   kvm_irq_has_notifier(ioapic-kvm, KVM_IRQCHIP_IOAPIC, 
index) ||
-   index == RTC_GSI) {
+   if (!e-fields.mask  e-fields.trig_mode == IOAPIC_LEVEL_TRIG
+   || kvm_irq_has_notifier(ioapic-kvm, KVM_IRQCHIP_IOAPIC,
+   index) || index == RTC_GSI) {
if (kvm_apic_match_dest(vcpu, NULL, 0,
e-fields.dest_id, e-fields.dest_mode)) {
__set_bit(e-fields.vector,
--
1.7.1

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