Re: [Xen-devel] [V10 PATCH 09/23] PVH xen: introduce pvh_set_vcpu_info() and vmx_pvh_set_vcpu_info()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.07.13 at 03:59, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> wrote:
> +int vmx_pvh_set_vcpu_info(struct vcpu *v, struct vcpu_guest_context *ctxtp)

Minimally

int vmx_pvh_set_vcpu_info(struct vcpu *v, const struct vcpu_guest_context 
*ctxtp)

And you should overall try to constify function parameters wherever
possible.

> +{
> +    if ( v->vcpu_id == 0 )
> +        return 0;
> +
> +    if ( !(ctxtp->flags & VGCF_in_kernel) )
> +        return -EINVAL;
> +
> +    vmx_vmcs_enter(v);
> +    __vmwrite(GUEST_GDTR_BASE, ctxtp->gdt.pvh.addr);
> +    __vmwrite(GUEST_GDTR_LIMIT, ctxtp->gdt.pvh.limit);
> +    __vmwrite(GUEST_LDTR_BASE, ctxtp->ldt_base);
> +    __vmwrite(GUEST_LDTR_LIMIT, ctxtp->ldt_ents);
> +
> +    __vmwrite(GUEST_FS_BASE, ctxtp->fs_base);
> +    __vmwrite(GUEST_GS_BASE, ctxtp->gs_base_kernel);
> +
> +    __vmwrite(GUEST_CS_SELECTOR, ctxtp->user_regs.cs);
> +    __vmwrite(GUEST_SS_SELECTOR, ctxtp->user_regs.ss);
> +    __vmwrite(GUEST_ES_SELECTOR, ctxtp->user_regs.es);
> +    __vmwrite(GUEST_DS_SELECTOR, ctxtp->user_regs.ds);
> +    __vmwrite(GUEST_FS_SELECTOR, ctxtp->user_regs.fs);
> +    __vmwrite(GUEST_GS_SELECTOR, ctxtp->user_regs.gs);
> +
> +    if ( vmx_add_guest_msr(MSR_SHADOW_GS_BASE) )
> +    {
> +        vmx_vmcs_exit(v);
> +        return -EINVAL;
> +    }
> +    vmx_write_guest_msr(MSR_SHADOW_GS_BASE, ctxtp->gs_base_user);
> +
> +    vmx_vmcs_exit(v);
> +    return 0;
> +}

So despite my earlier comments you still neither use nor check the
implied / defaulted to hidden parts of the segment registers vs the
destined descriptor table entries. I'm not going to ack the patch
without this being done at least in an #ifndef NDEBUG code section,
or without a code comment explaining why this cannot be done.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel