Re: [Xen-devel] RFC: PVH set vcpu info context in vmcs....

[Tim Deegan <tim@xxxxxxx>]

Hi, 

At 18:45 -0700 on 12 Aug (1376333113), Mukesh Rathor wrote:
> I just decided to create a new thread... Here's the code I wanted
> to propose.
> 
> /*
>  * Set vmcs fields in support of vcpu_op -> VCPUOP_initialise hcall used
>  * to initialise a secondary vcpu prior to boot. Called from 
>  * arch_set_info_guest() which sets the (PVH relevant) non-vmcs fields.
>  *
>  * In case of linux:
>  *     The call comes from cpu_initialize_context().  (boot vcpu 0 context is
>  *     set by the tools via do_domctl -> vcpu_initialise).
>  *
>  * PVH 32bitfixme: this function needs to be modified for 32bit guest. 
>  */
> int vmx_pvh_set_vcpu_info(struct vcpu *v, struct vcpu_guest_context *ctxtp)
> {
>     int rc;
> 
>     if ( v->vcpu_id == 0 )
>         return 0;
> 
>     if ( !(ctxtp->flags & VGCF_in_kernel) )
>         return -EINVAL;

Is that a new restriction on PVH?  Should there be a matching check of
the CS RPL or is it safe not to?

>     if ( ctxtp->ldt_base || ctxtp->ldt_ents ||
>          (ctxtp->user_regs.cs & 4) 

I think you could support LDTs by either taking the LDT range supplied
(with a bit of sanity checking) or asking for a descriptor and using
hvm_load_segment_selector(x86_seg_ldtr).

> || ctxtp->user_regs.ss || 
>          ctxtp->user_regs.es || ctxtp->user_regs.ds )
>         return -EINVAL;
> 
>     if ( ctxtp->user_regs.cs == 0 )
>         return -EINVAL;
> 
>     vmx_vmcs_enter(v);
>     __vmwrite(GUEST_GDTR_BASE, ctxtp->gdt.pvh.addr);
>     __vmwrite(GUEST_GDTR_LIMIT, ctxtp->gdt.pvh.limit);
> 
>     __vmwrite(GUEST_FS_BASE, ctxtp->fs_base);
>     __vmwrite(GUEST_GS_BASE, ctxtp->gs_base_kernel);
> 
>     /* IA-32e: ss/es/ds are ignored, we load cs only. */ 
>     __vmwrite(GUEST_CS_SELECTOR, ctxtp->user_regs.cs);
>     if ( (rc = hvm_load_segment_selector(x86_seg_cs, ctxtp->user_regs.cs)) ) 
>         return rc;

This looks good to me, thank you.

>     __vmwrite(GUEST_FS_SELECTOR, ctxtp->user_regs.fs);
>     __vmwrite(GUEST_GS_SELECTOR, ctxtp->user_regs.gs);

Or we could load these segments from their selectors too, and then
either check that the supplied bases match or mandate that they be zero.
I'm happy enough with this interface, though.  Either way it would be
nice to document which of GDT[fs].base and fs_base is actually loaded
(and likewise for gs_base_*).

Tim.

>     if ( (rc = vmx_add_guest_msr(MSR_SHADOW_GS_BASE)) )
>     {
>         vmx_vmcs_exit(v);
>         return rc;
>     }
>     vmx_write_guest_msr(MSR_SHADOW_GS_BASE, ctxtp->gs_base_user);
> 
>     vmx_vmcs_exit(v);
>     return 0;
> }

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