> @@ -692,8 +732,10 @@ static void svm_ctxt_switch_to(struct vc
> static void svm_do_resume(struct vcpu *v)
> {
> bool_t debug_state = v->domain->debugger_attached;
> -
> - if ( unlikely(v->arch.hvm_vcpu.debug_state_latch != debug_state) )
> + bool_t guestmode = nestedhvm_vcpu_in_guestmode(v);
> +
> + if ( !guestmode &&
> + unlikely(v->arch.hvm_vcpu.debug_state_latch != debug_state) )
> {
> uint32_t mask = (1U << TRAP_debug) | (1U << TRAP_int3);
> v->arch.hvm_vcpu.debug_state_latch = debug_state;
> @@ -712,11 +754,14 @@ static void svm_do_resume(struct vcpu *v
> hvm_asid_flush_vcpu(v);
> }
>
> - /* Reflect the vlapic's TPR in the hardware vtpr */
> - v->arch.hvm_svm.vmcb->vintr.fields.tpr =
> - (vlapic_get_reg(vcpu_vlapic(v), APIC_TASKPRI) & 0xFF) >> 4;
> -
> - hvm_do_resume(v);
> + if ( !guestmode )
> + {
> + /* Reflect the vlapic's TPR in the hardware vtpr */
> + v->arch.hvm_svm.vmcb->vintr.fields.tpr =
> + (vlapic_get_reg(vcpu_vlapic(v), APIC_TASKPRI) & 0xFF) >> 4;
> +
> + hvm_do_resume(v);
> + }
Can you explain why we shouldn't sync the vTPR and the vlapic state when
the guest is in nested mode?
[...]
> + } else {
> + /* host shadow paging + guest shadow paging. */
> + host_vmcb->np_enable = 0;
> + host_vmcb->h_cr3 = 0x0;
> +
> +#if 0
> + host_vmcb->cr3 = v->shadow_shadow_table;
> +
> + /* hvm_set_cr3() below sets v->arch.hvm_vcpu.guest_cr[3] for us. */
> + rc = hvm_set_cr3(ns_vmcb->cr3);
> + if (rc != X86EMUL_OKAY)
> + gdprintk(XENLOG_ERR, "hvm_set_cr3 failed, rc: %u\n", rc);
> +#endif
> + }
> +
Please remove this.
[...]
> +static void
> +svm_vmexit_do_vmload(struct vmcb_struct *vmcb,
> + struct cpu_user_regs *regs,
> + struct vcpu *v, uint64_t vmcbaddr)
> +{
> + int ret;
> + unsigned int inst_len;
> + struct vmcb_struct *tmp_vmcb;
> +
> + if ( (inst_len = __get_instruction_length(v, INSTR_VMLOAD)) == 0 )
> + return;
> +
> + /* tmp_vmcb can't be a local variable on the stack because
> + * the machine stops with a sudden freeze.
> + */
A VMCB structure is large, and Xen stacks are small. :) xmalloc()ing it
on every call seems a bit fragile though - and silently returning if the
xmalloc() fails is wrong. Is this something that could be allocated
once at VCPU creation time?
> + tmp_vmcb = xmalloc(struct vmcb_struct);
> + if (tmp_vmcb == NULL)
> + return;
> +
[...]
> @@ -1623,11 +2895,25 @@ asmlinkage void svm_vmexit_handler(struc
>
> case VMEXIT_MONITOR:
> case VMEXIT_MWAIT:
> + hvm_inject_exception(TRAP_invalid_op, HVM_DELIVER_NO_ERROR_CODE, 0);
> + break;
> +
> case VMEXIT_VMRUN:
> + svm_vmexit_do_vmrun(regs, v,
> + regs->eax);
> + break;
svm_vmexit_do_vmrun() and the other routines called below don't seem to
check that nested HVM is enabled; that seems like it would be bad if a
non-nesting guest tries to execute VMRUN &c. Or have you changed the
intercept bits somehow so that never happens?
Cheers,
Tim.
> case VMEXIT_VMLOAD:
> + svm_vmexit_do_vmload(vmcb, regs, v, regs->eax);
> + break;
> case VMEXIT_VMSAVE:
> + svm_vmexit_do_vmsave(vmcb, regs, v, regs->eax);
> + break;
> case VMEXIT_STGI:
> + svm_vmexit_do_stgi(regs, v);
> + break;
> case VMEXIT_CLGI:
> + svm_vmexit_do_clgi(regs, v);
> + break;
> case VMEXIT_SKINIT:
> hvm_inject_exception(TRAP_invalid_op, HVM_DELIVER_NO_ERROR_CODE, 0);
> break;
--
Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Principal Software Engineer, XenServer Engineering
Citrix Systems UK Ltd. (Company #02937203, SL9 0BG)
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|