From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wen Congyang Subject: Re: [RFC Patch v3 18/18] x86/hvm: Always set pending event injection when loading VMC[BS] state. Date: Thu, 11 Sep 2014 14:10:03 +0800 Message-ID: <54113CBB.1030105@cn.fujitsu.com> References: <1409908261-18682-1-git-send-email-wency@cn.fujitsu.com> <1409908261-18682-19-git-send-email-wency@cn.fujitsu.com> <54106910.1080207@amd.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <54106910.1080207@amd.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Aravind Gopalakrishnan , xen devel Cc: Ian Campbell , Ian Jackson , Jiang Yunhong , Dong Eddie , Tim Deegan , Jun Nakajima , Yang Hongyang , Suravee Suthikulpanit , Lai Jiangshan List-Id: xen-devel@lists.xenproject.org On 09/10/2014 11:06 PM, Aravind Gopalakrishnan wrote: > On 9/5/2014 4:11 AM, Wen Congyang wrote: >> In colo mode, secondary vm is running, so VM_ENTRY_INTR_INFO may >> valid before restoring vmcs. If there is no pending event after >> restoring vm, we should clear it. >> >> Signed-off-by: Wen Congyang >> >> Also clear pending software exceptions. >> Copy the fix to SVM as well. >> >> Signed-off-by: Tim Deegan >> >> vmx part: >> Cc: Jun Nakajima >> Acked-by: Kevin Tian >> >> svm part: >> Cc: Aravind Gopalakrishnan >> Cc: Suravee Suthikulpanit >> --- >> xen/arch/x86/hvm/svm/svm.c | 15 ++++++++------- >> xen/arch/x86/hvm/vmx/vmx.c | 25 ++++++++++++------------- >> 2 files changed, 20 insertions(+), 20 deletions(-) >> >> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c >> index b5188e6..053e511 100644 >> --- a/xen/arch/x86/hvm/svm/svm.c >> +++ b/xen/arch/x86/hvm/svm/svm.c >> @@ -321,16 +321,17 @@ static int svm_vmcb_restore(struct vcpu *v, struct hvm_hw_cpu *c) >> vmcb_set_h_cr3(vmcb, pagetable_get_paddr(p2m_get_pagetable(p2m))); >> } >> - if ( c->pending_valid ) >> + if ( c->pending_valid && >> + hvm_event_needs_reinjection(c->pending_type, c->pending_vector) ) >> { >> gdprintk(XENLOG_INFO, "Re-injecting %#"PRIx32", %#"PRIx32"\n", >> c->pending_event, c->error_code); >> - >> - if ( hvm_event_needs_reinjection(c->pending_type, c->pending_vector) ) >> - { >> - vmcb->eventinj.bytes = c->pending_event; >> - vmcb->eventinj.fields.errorcode = c->error_code; >> - } >> + vmcb->eventinj.bytes = c->pending_event; >> + vmcb->eventinj.fields.errorcode = c->error_code; >> + } >> + else >> + { >> + vmcb->eventinj.bytes = 0; >> } >> vmcb->cleanbits.bytes = 0; >> > > Hi, > You mention that this 'fix' is just copied for svm. But you have not seen the problem of "VM_ENTRY_INTR_INFO may be valid" (whose svm equivalent is "vmcb->eventinj.bytes is valid"). > My concern is that we should test colo mode for svm first, since, if the problem is never really seen on svm, then fix may not be _necessary_ Agree. > > At this point, my problems are with test setups. I can help testing scenarios, but as Wen had mentioned, 'colo testing' might be the way to test. > So, if I can get some pointers to how I can reproduce the issue at hand, then it would be very helpful. Currently, only COLO can trigger this problem, and COLO is not ready now. So I think this bugfix is not very important now. If the COLO is ready, and I will give you the way to trigger this problem. > > (Tim had mentioned we could try to simulate it by running a guest that takes lot of faults and save-restore another guest over it. However, > I am not having much luck following this route. I got a hvm guest to continuously take SW exceptions on all vcpus and tried to save-restore. > I can't see vmcb->eventinj.bytes containing any valid info.) I don't understand this way.. If we can trigger this problem by this way, we can verify this patch now. Thanks Wen Congyang > > Thanks, > -Aravind. > . >