[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 12/18 V2]:PVH xen: support invalid op, return PVH features etc...
On Fri, Mar 15, 2013 at 05:48:44PM -0700, Mukesh Rathor wrote: > The biggest change in this patch is in traps.c to allow forced invalid > op for PVH guest. Also, enable hypercall page init for PVH guest also. > Finally, set guest type to PVH if PV with HAP is created. > > Changes in V2: > - Fix emulate_forced_invalid_op() to use proper copy function, and inject PF > in case it fails. > - remove extraneous PVH check in STI/CLI ops en emulate_privileged_op(). > - Make assert a debug ASSERT in show_registers(). > - debug.c: keep get_gfn() locked and move put_gfn closer to it. > > Signed-off-by: Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> > --- > xen/arch/x86/debug.c | 9 ++++----- > xen/arch/x86/traps.c | 43 > +++++++++++++++++++++++++++++++++++++------ > xen/arch/x86/x86_64/traps.c | 5 +++-- > xen/common/domain.c | 9 +++++++++ > xen/common/domctl.c | 4 ++++ > xen/common/kernel.c | 6 +++++- > 6 files changed, 62 insertions(+), 14 deletions(-) > > diff --git a/xen/arch/x86/debug.c b/xen/arch/x86/debug.c > index 502edbc..abe538f 100644 > --- a/xen/arch/x86/debug.c > +++ b/xen/arch/x86/debug.c > @@ -59,7 +59,9 @@ dbg_hvm_va2mfn(dbgva_t vaddr, struct domain *dp, int toaddr, > return INVALID_MFN; > } > > - mfn = mfn_x(get_gfn(dp, *gfn, &gfntype)); > + mfn = mfn_x(get_gfn_query(dp, *gfn, &gfntype)); > + put_gfn(dp, *gfn); > + > if ( p2m_is_readonly(gfntype) && toaddr ) > { > DBGP2("kdb:p2m_is_readonly: gfntype:%x\n", gfntype); > @@ -158,7 +160,7 @@ dbg_rw_guest_mem(dbgva_t addr, dbgbyte_t *buf, int len, > struct domain *dp, > > pagecnt = min_t(long, PAGE_SIZE - (addr & ~PAGE_MASK), len); > > - mfn = (is_hvm_domain(dp) > + mfn = (is_hvm_or_pvh_domain(dp) > ? dbg_hvm_va2mfn(addr, dp, toaddr, &gfn) > : dbg_pv_va2mfn(addr, dp, pgd3)); > if ( mfn == INVALID_MFN ) > @@ -178,9 +180,6 @@ dbg_rw_guest_mem(dbgva_t addr, dbgbyte_t *buf, int len, > struct domain *dp, > } > > unmap_domain_page(va); > - if ( gfn != INVALID_GFN ) > - put_gfn(dp, gfn); > - > addr += pagecnt; > buf += pagecnt; > len -= pagecnt; > diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c > index ab54f82..14656c1 100644 > --- a/xen/arch/x86/traps.c > +++ b/xen/arch/x86/traps.c > @@ -459,6 +459,10 @@ static void instruction_done( > struct cpu_user_regs *regs, unsigned long eip, unsigned int bpmatch) > { > regs->eip = eip; > + > + if ( is_pvh_vcpu(current) ) > + return; Can it be above the 'regs->eip = eip' ? > + > regs->eflags &= ~X86_EFLAGS_RF; > if ( bpmatch || (regs->eflags & X86_EFLAGS_TF) ) > { > @@ -475,6 +479,10 @@ static unsigned int check_guest_io_breakpoint(struct > vcpu *v, > unsigned int width, i, match = 0; > unsigned long start; > > + if ( is_pvh_vcpu(v) ) { > + /* for pvh, ctrlreg field is not implemented/used unless we need to > */ > + return 0; > + } > if ( !(v->arch.debugreg[5]) || > !(v->arch.pv_vcpu.ctrlreg[4] & X86_CR4_DE) ) > return 0; > @@ -908,14 +916,18 @@ static int emulate_invalid_rdtscp(struct cpu_user_regs > *regs) > unsigned long emulate_forced_invalid_op(struct cpu_user_regs *regs) > { > char sig[5], instr[2]; > - unsigned long eip, rc; > + unsigned long eip, rc, addr; > > eip = regs->eip; > > /* Check for forced emulation signature: ud2 ; .ascii "xen". */ > - if ( (rc = copy_from_user(sig, (char *)eip, sizeof(sig))) != 0 ) > + if ( (rc = raw_copy_from_guest(sig, (char *)eip, sizeof(sig))) != 0 ) > { > - propagate_page_fault(eip + sizeof(sig) - rc, 0); > + addr = eip + sizeof(sig) - rc; > + if ( is_pvh_vcpu(current) ) > + return addr; > + > + propagate_page_fault(addr, 0); > return EXCRET_fault_fixed; > } > if ( memcmp(sig, "\xf\xbxen", sizeof(sig)) ) > @@ -923,9 +935,13 @@ unsigned long emulate_forced_invalid_op(struct > cpu_user_regs *regs) > eip += sizeof(sig); > > /* We only emulate CPUID. */ > - if ( ( rc = copy_from_user(instr, (char *)eip, sizeof(instr))) != 0 ) > + if ( ( rc = raw_copy_from_guest(instr, (char *)eip, sizeof(instr))) != 0 > ) > { > - propagate_page_fault(eip + sizeof(instr) - rc, 0); > + addr = eip + sizeof(instr) - rc; > + if ( is_pvh_vcpu(current) ) > + return addr; > + > + propagate_page_fault(addr, 0); > return EXCRET_fault_fixed; > } > if ( memcmp(instr, "\xf\xa2", sizeof(instr)) ) > @@ -1068,6 +1084,10 @@ void propagate_page_fault(unsigned long addr, u16 > error_code) > struct vcpu *v = current; > struct trap_bounce *tb = &v->arch.pv_vcpu.trap_bounce; > > + /* PVH should not get here. ctrlreg is not implemented amongst other > + * things */ > + ASSERT( !is_pvh_vcpu(v) ); > + > v->arch.pv_vcpu.ctrlreg[2] = addr; > arch_set_cr2(v, addr); > > @@ -1453,6 +1473,9 @@ static int read_descriptor(unsigned int sel, > { > struct desc_struct desc; > > + if ( is_pvh_vcpu(v) ) > + return hvm_pvh_read_descriptor(sel, v, regs, base, limit, ar); Ah, that is why you are using such weird arguments. And it looks like emulate_privileged_op really needs it as single variables. Perhaps then you can just add a comment in hvm_pvh_read_descriptor saying that the reason you are shifting is b/c the caller (emulate_privileged_op) expects it to be in this format. > + > if ( !vm86_mode(regs) ) > { > if ( sel < 4) > @@ -1571,6 +1594,11 @@ static int guest_io_okay( > int user_mode = !(v->arch.flags & TF_kernel_mode); > #define TOGGLE_MODE() if ( user_mode ) toggle_guest_mode(v) > > + /* for PVH we check this in vmexit for EXIT_REASON_IO_INSTRUCTION > + * and so don't need to check again here. */ > + if (is_pvh_vcpu(v)) Odd syntax. > + return 1; > + > if ( !vm86_mode(regs) && > (v->arch.pv_vcpu.iopl >= (guest_kernel_mode(v, regs) ? 1 : 3)) ) > return 1; > @@ -1816,7 +1844,7 @@ static inline uint64_t guest_misc_enable(uint64_t val) > _ptr = (unsigned int)_ptr; \ > if ( (limit) < sizeof(_x) - 1 || (eip) > (limit) - (sizeof(_x) - 1) ) \ > goto fail; \ > - if ( (_rc = copy_from_user(&_x, (type *)_ptr, sizeof(_x))) != 0 ) \ > + if ( (_rc = raw_copy_from_guest(&_x, (type *)_ptr, sizeof(_x))) != 0 ) \ > { \ > propagate_page_fault(_ptr + sizeof(_x) - _rc, 0); \ > goto skip; \ > @@ -3245,6 +3273,9 @@ void do_device_not_available(struct cpu_user_regs *regs) > > BUG_ON(!guest_mode(regs)); > > + /* PVH should not get here. ctrlreg is not implemented */ > + ASSERT( !is_pvh_vcpu(curr) ); > + > vcpu_restore_fpu_lazy(curr); > > if ( curr->arch.pv_vcpu.ctrlreg[0] & X86_CR0_TS ) > diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c > index d2f7209..47ec2ff 100644 > --- a/xen/arch/x86/x86_64/traps.c > +++ b/xen/arch/x86/x86_64/traps.c > @@ -147,7 +147,7 @@ void vcpu_show_registers(const struct vcpu *v) > unsigned long crs[8]; > > /* No need to handle HVM for now. */ .. and PVH .. > - if ( is_hvm_vcpu(v) ) > + if ( is_hvm_or_pvh_vcpu(v) ) > return; > > crs[0] = v->arch.pv_vcpu.ctrlreg[0]; > @@ -440,6 +440,7 @@ static long register_guest_callback(struct > callback_register *reg) > long ret = 0; > struct vcpu *v = current; > > + ASSERT( !is_pvh_vcpu(v) ); > if ( !is_canonical_address(reg->address) ) > return -EINVAL; > > @@ -620,7 +621,7 @@ static void hypercall_page_initialise_ring3_kernel(void > *hypercall_page) > void hypercall_page_initialise(struct domain *d, void *hypercall_page) > { > memset(hypercall_page, 0xCC, PAGE_SIZE); > - if ( is_hvm_domain(d) ) > + if ( is_hvm_or_pvh_domain(d) ) > hvm_hypercall_page_initialise(d, hypercall_page); > else if ( !is_pv_32bit_domain(d) ) > hypercall_page_initialise_ring3_kernel(hypercall_page); > diff --git a/xen/common/domain.c b/xen/common/domain.c > index b6f10b7..aac6699 100644 > --- a/xen/common/domain.c > +++ b/xen/common/domain.c > @@ -232,6 +232,15 @@ struct domain *domain_create( > > if ( domcr_flags & DOMCRF_hvm ) > d->guest_type = hvm_guest; > + else if ( domcr_flags & DOMCRF_pvh ) { > + d->guest_type = pvh_guest; > + if ( !(domcr_flags & DOMCRF_hap) ) { > + printk("PVH guest must have HAP on\n"); Ahem. XENLOG_ERR > + goto fail; > + } else > + printk("PVH guest. Please note it is experimental. domid:%d\n", > + domid); XENLOG_INFO or XENLOG_DEBUG > + } > > if ( domid == 0 ) > { > diff --git a/xen/common/domctl.c b/xen/common/domctl.c > index c98e99c..ab615f1 100644 > --- a/xen/common/domctl.c > +++ b/xen/common/domctl.c > @@ -149,6 +149,8 @@ void getdomaininfo(struct domain *d, struct > xen_domctl_getdomaininfo *info) > > if ( is_hvm_domain(d) ) > info->flags |= XEN_DOMINF_hvm_guest; > + else if ( is_pvh_domain(d) ) > + info->flags |= XEN_DOMINF_pvh_guest; > > xsm_security_domaininfo(d, info); > > @@ -400,6 +402,8 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) > u_domctl) > domcr_flags = 0; > if ( op->u.createdomain.flags & XEN_DOMCTL_CDF_hvm_guest ) > domcr_flags |= DOMCRF_hvm; > + else if ( op->u.createdomain.flags & XEN_DOMCTL_CDF_hap ) > + domcr_flags |= DOMCRF_pvh; /* PV with HAP is a PVH guest */ <scratches his head> So if the user sets: 'hap' in their guest config we automatically set domcr_flags to DOMCRF_hvm | DOMCRF_pvh right? Then in domain_create we do this check: if (domcr_flags & DOMCRF_pvh ) { d->guest_type = pvh_guest; and we force an HVM guest with 'hap=1' in the guest config to be a pvh_guest ? > if ( op->u.createdomain.flags & XEN_DOMCTL_CDF_hap ) > domcr_flags |= DOMCRF_hap; > if ( op->u.createdomain.flags & XEN_DOMCTL_CDF_s3_integrity ) > diff --git a/xen/common/kernel.c b/xen/common/kernel.c > index 72fb905..3bba758 100644 > --- a/xen/common/kernel.c > +++ b/xen/common/kernel.c > @@ -289,7 +289,11 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) > arg) > if ( current->domain == dom0 ) > fi.submap |= 1U << XENFEAT_dom0; > #ifdef CONFIG_X86 > - if ( !is_hvm_vcpu(current) ) > + if ( is_pvh_vcpu(current) ) > + fi.submap |= (1U << XENFEAT_hvm_safe_pvclock) | > + (1U << XENFEAT_supervisor_mode_kernel) | > + (1U << XENFEAT_hvm_callback_vector); > + else if ( !is_hvm_vcpu(current) ) > fi.submap |= (1U << XENFEAT_mmu_pt_update_preserve_ad) | > (1U << XENFEAT_highmem_assist) | > (1U << XENFEAT_gnttab_map_avail_bits); > -- > 1.7.2.3 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@xxxxxxxxxxxxx > http://lists.xen.org/xen-devel > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |