From 672101b54269d7a69c2ccaa345ea396dcca63238 Mon Sep 17 00:00:00 2001 From: Jan Beulich Date: Fri, 14 Aug 2015 12:36:26 +0200 Subject: [PATCH] x86/HVM: honor p2m_ram_ro in hvm_map_guest_frame_rw() ... and its callers. While all non-nested users are made fully honor the semantics of that type, doing so in the nested case seemed insane (if doable at all, considering VMCS shadowing), and hence there the respective operations are simply made fail. One case not (yet) taken care of is that of a page getting transitioned to this type after a mapping got established. Signed-off-by: Jan Beulich Acked-by: Boris Ostrovsky Reviewed-by: Andrew Cooper Acked-by: Kevin Tian Release-acked-by: Wei Liu --- xen/arch/x86/hvm/hvm.c | 39 ++++++++++++++++---------- xen/arch/x86/hvm/svm/nestedsvm.c | 16 +++++++++-- xen/arch/x86/hvm/vmx/vmx.c | 13 +++++---- xen/arch/x86/hvm/vmx/vvmx.c | 48 ++++++++++++++++++++++---------- xen/include/asm-x86/hvm/hvm.h | 3 +- 5 files changed, 81 insertions(+), 38 deletions(-) diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 707ad86d11..615fa89083 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -3666,8 +3666,8 @@ int hvm_virtual_to_linear_addr( /* On non-NULL return, we leave this function holding an additional * ref on the underlying mfn, if any */ -static void *__hvm_map_guest_frame(unsigned long gfn, bool_t writable, - bool_t permanent) +static void *_hvm_map_guest_frame(unsigned long gfn, bool_t permanent, + bool_t *writable) { void *map; p2m_type_t p2mt; @@ -3690,7 +3690,12 @@ static void *__hvm_map_guest_frame(unsigned long gfn, bool_t writable, } if ( writable ) - paging_mark_dirty(d, page_to_mfn(page)); + { + if ( !p2m_is_discard_write(p2mt) ) + paging_mark_dirty(d, page_to_mfn(page)); + else + *writable = 0; + } if ( !permanent ) return __map_domain_page(page); @@ -3702,14 +3707,16 @@ static void *__hvm_map_guest_frame(unsigned long gfn, bool_t writable, return map; } -void *hvm_map_guest_frame_rw(unsigned long gfn, bool_t permanent) +void *hvm_map_guest_frame_rw(unsigned long gfn, bool_t permanent, + bool_t *writable) { - return __hvm_map_guest_frame(gfn, 1, permanent); + *writable = 1; + return _hvm_map_guest_frame(gfn, permanent, writable); } void *hvm_map_guest_frame_ro(unsigned long gfn, bool_t permanent) { - return __hvm_map_guest_frame(gfn, 0, permanent); + return _hvm_map_guest_frame(gfn, permanent, NULL); } void hvm_unmap_guest_frame(void *p, bool_t permanent) @@ -3729,7 +3736,7 @@ void hvm_unmap_guest_frame(void *p, bool_t permanent) put_page(mfn_to_page(mfn)); } -static void *hvm_map_entry(unsigned long va) +static void *hvm_map_entry(unsigned long va, bool_t *writable) { unsigned long gfn; uint32_t pfec; @@ -3752,7 +3759,7 @@ static void *hvm_map_entry(unsigned long va) if ( (pfec == PFEC_page_paged) || (pfec == PFEC_page_shared) ) goto fail; - v = hvm_map_guest_frame_rw(gfn, 0); + v = hvm_map_guest_frame_rw(gfn, 0, writable); if ( v == NULL ) goto fail; @@ -3774,6 +3781,7 @@ static int hvm_load_segment_selector( struct segment_register desctab, cs, segr; struct desc_struct *pdesc, desc; u8 dpl, rpl, cpl; + bool_t writable; int fault_type = TRAP_invalid_tss; struct cpu_user_regs *regs = guest_cpu_user_regs(); struct vcpu *v = current; @@ -3810,7 +3818,7 @@ static int hvm_load_segment_selector( if ( ((sel & 0xfff8) + 7) > desctab.limit ) goto fail; - pdesc = hvm_map_entry(desctab.base + (sel & 0xfff8)); + pdesc = hvm_map_entry(desctab.base + (sel & 0xfff8), &writable); if ( pdesc == NULL ) goto hvm_map_fail; @@ -3869,6 +3877,7 @@ static int hvm_load_segment_selector( break; } } while ( !(desc.b & 0x100) && /* Ensure Accessed flag is set */ + writable && /* except if we are to discard writes */ (cmpxchg(&pdesc->b, desc.b, desc.b | 0x100) != desc.b) ); /* Force the Accessed flag in our local copy. */ @@ -3906,6 +3915,7 @@ void hvm_task_switch( struct cpu_user_regs *regs = guest_cpu_user_regs(); struct segment_register gdt, tr, prev_tr, segr; struct desc_struct *optss_desc = NULL, *nptss_desc = NULL, tss_desc; + bool_t otd_writable, ntd_writable; unsigned long eflags; int exn_raised, rc; struct { @@ -3932,11 +3942,12 @@ void hvm_task_switch( goto out; } - optss_desc = hvm_map_entry(gdt.base + (prev_tr.sel & 0xfff8)); + optss_desc = hvm_map_entry(gdt.base + (prev_tr.sel & 0xfff8), + &otd_writable); if ( optss_desc == NULL ) goto out; - nptss_desc = hvm_map_entry(gdt.base + (tss_sel & 0xfff8)); + nptss_desc = hvm_map_entry(gdt.base + (tss_sel & 0xfff8), &ntd_writable); if ( nptss_desc == NULL ) goto out; @@ -4068,11 +4079,11 @@ void hvm_task_switch( v->arch.hvm_vcpu.guest_cr[0] |= X86_CR0_TS; hvm_update_guest_cr(v, 0); - if ( (taskswitch_reason == TSW_iret) || - (taskswitch_reason == TSW_jmp) ) + if ( (taskswitch_reason == TSW_iret || + taskswitch_reason == TSW_jmp) && otd_writable ) clear_bit(41, optss_desc); /* clear B flag of old task */ - if ( taskswitch_reason != TSW_iret ) + if ( taskswitch_reason != TSW_iret && ntd_writable ) set_bit(41, nptss_desc); /* set B flag of new task */ if ( errcode >= 0 ) diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c index dd927f7450..46f253200d 100644 --- a/xen/arch/x86/hvm/svm/nestedsvm.c +++ b/xen/arch/x86/hvm/svm/nestedsvm.c @@ -74,10 +74,20 @@ int nestedsvm_vmcb_map(struct vcpu *v, uint64_t vmcbaddr) nv->nv_vvmcxaddr = VMCX_EADDR; } - if (nv->nv_vvmcx == NULL) { - nv->nv_vvmcx = hvm_map_guest_frame_rw(vmcbaddr >> PAGE_SHIFT, 1); - if (nv->nv_vvmcx == NULL) + if ( !nv->nv_vvmcx ) + { + bool_t writable; + void *vvmcx = hvm_map_guest_frame_rw(paddr_to_pfn(vmcbaddr), 1, + &writable); + + if ( !vvmcx ) return 0; + if ( !writable ) + { + hvm_unmap_guest_frame(vvmcx, 1); + return 0; + } + nv->nv_vvmcx = vvmcx; nv->nv_vvmcxaddr = vmcbaddr; } diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index c32d86323d..999defeaa3 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -1853,14 +1853,17 @@ static int vmx_vcpu_emulate_vmfunc(struct cpu_user_regs *regs) static bool_t vmx_vcpu_emulate_ve(struct vcpu *v) { - bool_t rc = 0; - ve_info_t *veinfo = gfn_x(vcpu_altp2m(v).veinfo_gfn) != INVALID_GFN ? - hvm_map_guest_frame_rw(gfn_x(vcpu_altp2m(v).veinfo_gfn), 0) : NULL; + bool_t rc = 0, writable; + unsigned long gfn = gfn_x(vcpu_altp2m(v).veinfo_gfn); + ve_info_t *veinfo; - if ( !veinfo ) + if ( gfn == INVALID_GFN ) return 0; - if ( veinfo->semaphore != 0 ) + veinfo = hvm_map_guest_frame_rw(gfn, 0, &writable); + if ( !veinfo ) + return 0; + if ( !writable || veinfo->semaphore != 0 ) goto out; rc = 1; diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c index 34b05be1da..cb6f9b820e 100644 --- a/xen/arch/x86/hvm/vmx/vvmx.c +++ b/xen/arch/x86/hvm/vmx/vvmx.c @@ -1618,10 +1618,23 @@ int nvmx_handle_vmptrld(struct cpu_user_regs *regs) if ( nvcpu->nv_vvmcxaddr == VMCX_EADDR ) { - nvcpu->nv_vvmcx = hvm_map_guest_frame_rw(gpa >> PAGE_SHIFT, 1); - if ( nvcpu->nv_vvmcx ) - nvcpu->nv_vvmcxaddr = gpa; - if ( !nvcpu->nv_vvmcx || + bool_t writable; + void *vvmcx = hvm_map_guest_frame_rw(paddr_to_pfn(gpa), 1, &writable); + + if ( vvmcx ) + { + if ( writable ) + { + nvcpu->nv_vvmcx = vvmcx; + nvcpu->nv_vvmcxaddr = gpa; + } + else + { + hvm_unmap_guest_frame(vvmcx, 1); + vvmcx = NULL; + } + } + if ( !vvmcx || !map_io_bitmap_all(v) || !_map_msr_bitmap(v) ) { @@ -1675,13 +1688,10 @@ int nvmx_handle_vmclear(struct cpu_user_regs *regs) if ( rc != X86EMUL_OKAY ) return rc; + BUILD_BUG_ON(X86EMUL_OKAY != VMSUCCEED); /* rc = VMSUCCEED; */ if ( gpa & 0xfff ) - { - vmreturn(regs, VMFAIL_INVALID); - return X86EMUL_OKAY; - } - - if ( gpa == nvcpu->nv_vvmcxaddr ) + rc = VMFAIL_INVALID; + else if ( gpa == nvcpu->nv_vvmcxaddr ) { if ( cpu_has_vmx_vmcs_shadowing ) nvmx_clear_vmcs_pointer(v, nvcpu->nv_vvmcx); @@ -1692,14 +1702,22 @@ int nvmx_handle_vmclear(struct cpu_user_regs *regs) else { /* Even if this VMCS isn't the current one, we must clear it. */ - vvmcs = hvm_map_guest_frame_rw(gpa >> PAGE_SHIFT, 0); + bool_t writable; + + vvmcs = hvm_map_guest_frame_rw(paddr_to_pfn(gpa), 0, &writable); if ( vvmcs ) - clear_vvmcs_launched(&nvmx->launched_list, - domain_page_map_to_mfn(vvmcs)); - hvm_unmap_guest_frame(vvmcs, 0); + { + if ( writable ) + clear_vvmcs_launched(&nvmx->launched_list, + domain_page_map_to_mfn(vvmcs)); + else + rc = VMFAIL_VALID; + hvm_unmap_guest_frame(vvmcs, 0); + } } - vmreturn(regs, VMSUCCEED); + vmreturn(regs, rc); + return X86EMUL_OKAY; } diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h index 3cac64faf5..68b216cdb5 100644 --- a/xen/include/asm-x86/hvm/hvm.h +++ b/xen/include/asm-x86/hvm/hvm.h @@ -440,7 +440,8 @@ int hvm_virtual_to_linear_addr( unsigned int addr_size, unsigned long *linear_addr); -void *hvm_map_guest_frame_rw(unsigned long gfn, bool_t permanent); +void *hvm_map_guest_frame_rw(unsigned long gfn, bool_t permanent, + bool_t *writable); void *hvm_map_guest_frame_ro(unsigned long gfn, bool_t permanent); void hvm_unmap_guest_frame(void *p, bool_t permanent); -- 2.39.5