RE: [PATCH] kvm/vmx: EPTP switching test
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
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
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
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.
-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.
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.
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.
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