>>> On 04.09.15 at 14:08, <roger....@citrix.com> wrote: > --- a/xen/arch/x86/hvm/svm/svm.c > +++ b/xen/arch/x86/hvm/svm/svm.c > @@ -1035,6 +1035,7 @@ static void noreturn svm_do_resume(struct vcpu *v) > struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb; > bool_t debug_state = v->domain->debugger_attached; > bool_t vcpu_guestmode = 0; > + struct vlapic *vlapic = vcpu_vlapic(v); > > if ( nestedhvm_enabled(v->domain) && nestedhvm_vcpu_in_guestmode(v) ) > vcpu_guestmode = 1; > @@ -1058,14 +1059,14 @@ static void noreturn svm_do_resume(struct vcpu *v) > hvm_asid_flush_vcpu(v); > } > > - if ( !vcpu_guestmode ) > + if ( !vcpu_guestmode && !vlapic_hw_disabled(vlapic) ) > { > vintr_t intr; > > /* Reflect the vlapic's TPR in the hardware vtpr */ > intr = vmcb_get_vintr(vmcb); > intr.fields.tpr = > - (vlapic_get_reg(vcpu_vlapic(v), APIC_TASKPRI) & 0xFF) >> 4; > + (vlapic_get_reg(vlapic, APIC_TASKPRI) & 0xFF) >> 4; > vmcb_set_vintr(vmcb, intr); > } > > @@ -2294,6 +2295,7 @@ void svm_vmexit_handler(struct cpu_user_regs *regs) > int inst_len, rc; > vintr_t intr; > bool_t vcpu_guestmode = 0; > + struct vlapic *vlapic = vcpu_vlapic(v); > > hvm_invalidate_regs_fields(regs); > > @@ -2311,11 +2313,11 @@ void svm_vmexit_handler(struct cpu_user_regs *regs) > * NB. We need to preserve the low bits of the TPR to make checked builds > * of Windows work, even though they don't actually do anything. > */ > - if ( !vcpu_guestmode ) { > + if ( !vcpu_guestmode && !vlapic_hw_disabled(vlapic) ) { > intr = vmcb_get_vintr(vmcb); > - vlapic_set_reg(vcpu_vlapic(v), APIC_TASKPRI, > + vlapic_set_reg(vlapic, APIC_TASKPRI, > ((intr.fields.tpr & 0x0F) << 4) | > - (vlapic_get_reg(vcpu_vlapic(v), APIC_TASKPRI) & 0x0F)); > + (vlapic_get_reg(vlapic, APIC_TASKPRI) & 0x0F)); > } > > exit_reason = vmcb->exitcode; > @@ -2697,14 +2699,14 @@ void svm_vmexit_handler(struct cpu_user_regs *regs) > } > > out: > - if ( vcpu_guestmode ) > + if ( vcpu_guestmode || vlapic_hw_disabled(vlapic) ) > /* Don't clobber TPR of the nested guest. */
The comment is now stale. Also - aren't all the changes to this file (and perhaps othersfurther down) bug fixes in their own right? > @@ -1042,8 +1045,7 @@ void vlapic_tdt_msr_set(struct vlapic *vlapic, uint64_t > value) > uint64_t guest_tsc; > struct vcpu *v = vlapic_vcpu(vlapic); > > - /* may need to exclude some other conditions like vlapic->hw.disabled */ > - if ( !vlapic_lvtt_tdt(vlapic) ) > + if ( !vlapic_lvtt_tdt(vlapic) || vlapic_hw_disabled(vlapic) ) > { > HVM_DBG_LOG(DBG_LEVEL_VLAPIC_TIMER, "ignore tsc deadline msr write"); The newly added condition needless triggers the HVM_DBG_LOG(). Please separate it. > @@ -1328,7 +1339,10 @@ static int lapic_load_hidden(struct domain *d, > hvm_domain_context_t *h) > uint16_t vcpuid; > struct vcpu *v; > struct vlapic *s; > - > + > + if ( !has_vlapic(d) ) > + return 0; > + > /* Which vlapic to load? */ > vcpuid = hvm_load_instance(h); > if ( vcpuid >= d->max_vcpus || (v = d->vcpu[vcpuid]) == NULL ) > @@ -1360,7 +1374,10 @@ static int lapic_load_regs(struct domain *d, > hvm_domain_context_t *h) > uint16_t vcpuid; > struct vcpu *v; > struct vlapic *s; > - > + > + if ( !has_vlapic(d) ) > + return 0; I agree that the save side should return zero in that case, but aren't load attempts invalid (and hence warrant an error return)? > @@ -1399,7 +1416,7 @@ int vlapic_init(struct vcpu *v) > > HVM_DBG_LOG(DBG_LEVEL_VLAPIC, "%d", v->vcpu_id); > > - if ( is_pvh_vcpu(v) ) > + if ( is_pvh_vcpu(v) || !has_vlapic(v->domain) ) Isn't the latter alone sufficient? > @@ -1452,6 +1469,9 @@ void vlapic_destroy(struct vcpu *v) > { > struct vlapic *vlapic = vcpu_vlapic(v); > > + if ( !has_vlapic(vlapic_domain(vlapic)) ) I think v->domain would be better here. > --- a/xen/arch/x86/hvm/vmsi.c > +++ b/xen/arch/x86/hvm/vmsi.c > @@ -482,6 +482,9 @@ found: > > void msixtbl_init(struct domain *d) > { > + if ( !has_vlapic(d) ) > + return; > + > INIT_LIST_HEAD(&d->arch.hvm_domain.msixtbl_list); > spin_lock_init(&d->arch.hvm_domain.msixtbl_list_lock); Don't you also need to add guards to msixtbl_pt_{,un}register()? > --- a/xen/arch/x86/hvm/vmx/vmcs.c > +++ b/xen/arch/x86/hvm/vmx/vmcs.c > @@ -1002,7 +1002,7 @@ static int construct_vmcs(struct vcpu *v) > ~(SECONDARY_EXEC_ENABLE_VM_FUNCTIONS | > SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS); > > - if ( is_pvh_domain(d) ) > + if ( is_pvh_domain(d) || !has_vlapic(d) ) See above. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel