[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 1/1] x86/AMD: Fix nested svm crash due to assertion in __virt_to_maddr
On 04.07.13 23:48, Tim Deegan wrote: > At 20:42 +0100 on 04 Jul (1372970576), Andrew Cooper wrote: >> On 04/07/13 20:36, suravee.suthikulpanit@xxxxxxx wrote: >>> From: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx> >>> >>> 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 <suravee.suthikulpanit@xxxxxxx> >>> --- >>> 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. > > IIUC this is a physical address, so paddr_t is the correct type. Right. > Also, it might be nicer to call these svm_vm{save,load}_by_paddr() or similar > to make it clear what they do. I agree. But why did the assertion never trigger when called from elsewhere in svm_vm{load,save}? >> 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? > > I think the virtual mapping is ok from that point of view -- it's mapped > with map_domain_page_global(). I worry that we might run out of mapping > slots if we keep a lot of these permanent mappings around, though. The number of mappings = number of guest hypervisors * number of virtual cpus per guest hypervisor number of guest hypervisors = number of domains excluding all domains where nestedhvm is not used even when turned on Christoph _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |