From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 1/1] x86/AMD: Fix nested svm crash due to assertion in __virt_to_maddr Date: Thu, 4 Jul 2013 20:42:56 +0100 Message-ID: <51D5D040.4070501@citrix.com> References: <1372966572-2703-1-git-send-email-suravee.suthikulpanit@amd.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1372966572-2703-1-git-send-email-suravee.suthikulpanit@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: suravee.suthikulpanit@amd.com Cc: chegger@amazon.de, JBeulich@suse.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 04/07/13 20:36, suravee.suthikulpanit@amd.com wrote: > From: Suravee Suthikulpanit > > Fix assertion in __virt_to_maddr when starting nested SVM guest > in debug mode. Investigation has shown that svm_vmsave/svm_vmload > make use of __pa() with invalid address. > > Signed-off-by: Suravee Suthikulpanit > --- > xen/arch/x86/hvm/svm/svm.c | 4 ++-- > xen/include/asm-x86/hvm/svm/svm.h | 14 ++++++++++++++ > 2 files changed, 16 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c > index acd2d49..944569a 100644 > --- a/xen/arch/x86/hvm/svm/svm.c > +++ b/xen/arch/x86/hvm/svm/svm.c > @@ -1809,7 +1809,7 @@ svm_vmexit_do_vmload(struct vmcb_struct *vmcb, > goto inject; > } > > - svm_vmload(nv->nv_vvmcx); > + nestedsvm_vmload(nv->nv_vvmcxaddr); > /* State in L1 VMCB is stale now */ > v->arch.hvm_svm.vmcb_in_sync = 0; > > @@ -1845,7 +1845,7 @@ svm_vmexit_do_vmsave(struct vmcb_struct *vmcb, > goto inject; > } > > - svm_vmsave(nv->nv_vvmcx); > + nestedsvm_vmsave(nv->nv_vvmcxaddr); > > __update_guest_eip(regs, inst_len); > return; > diff --git a/xen/include/asm-x86/hvm/svm/svm.h b/xen/include/asm-x86/hvm/svm/svm.h > index 64e7e25..909e8a1 100644 > --- a/xen/include/asm-x86/hvm/svm/svm.h > +++ b/xen/include/asm-x86/hvm/svm/svm.h > @@ -55,6 +55,20 @@ static inline void svm_vmsave(void *vmcb) > : : "a" (__pa(vmcb)) : "memory" ); > } > > +static inline void nestedsvm_vmload(uint64_t vmcb) unsigned long if this is actually an address. But more importantly, if virt_to_maddr() fails an assertion because the virtual address is not a persistent mapping, what is going to happen when the virtual mapping (potentially) changes while the vvmcx is in use? ~Andrew > +{ > + asm volatile ( > + ".byte 0x0f,0x01,0xda" /* vmload */ > + : : "a" (vmcb) : "memory" ); > +} > + > +static inline void nestedsvm_vmsave(uint64_t vmcb) > +{ > + asm volatile ( > + ".byte 0x0f,0x01,0xdb" /* vmsave */ > + : : "a" (vmcb) : "memory" ); > +} > + > static inline void svm_invlpga(unsigned long vaddr, uint32_t asid) > { > asm volatile (