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

[Tim Deegan <tim@xxxxxxx>]

At 17:58 -0700 on 25 Jul (1374775120), Mukesh Rathor wrote:
> Pl see: http://lists.xen.org/archives/html/xen-devel/2013-07/msg01489.html
> 
> That was an option discussed with Jan, walking and reading the GDT
> entries from the gdtaddr the guest provided to load the hidden
> parts. But, I agree with him, that for the initial cpu boot we can
> restrict the ABI to say: 0 base addr, ~0 limit, and "read/write,
> accessed" default attributes for the hidden part (64bit guest). 

I see, thanks for the link.

> > If this _is_ to be the interface:
> >   - this comment should be somewhere in the interface headers (and
> > docs) so that OS authors can find it; and
> >   - it should specify the _exact_ constraints that the guest must
> >     follow in constructing its tables.
> 
> I can add a comment to VCPUOP_initialise:
> 
> diff --git a/xen/include/public/vcpu.h b/xen/include/public/vcpu.h
> index e888daf..03baae2 100644
> --- a/xen/include/public/vcpu.h
> +++ b/xen/include/public/vcpu.h
> @@ -43,6 +43,8 @@
>   * 
>   * @extra_arg == pointer to vcpu_guest_context structure containing initial
>   *               state for the VCPU.
> + *
> + * PVH: for constraints, please see prolog of vmx_pvh_set_vcpu_info().
>   */
>  #define VCPUOP_initialise            0
>  
> 
> Or I can explicitly add the details here:
> 
>   * @extra_arg == pointer to vcpu_guest_context structure containing initial
>   *               state for the VCPU.
> + *
> + * PVH constraints: The vcpu GDT must adhere to following default values.
> + *   For 64 bit guest:
> + *      CS: base:0 limit:fffff Type:0xb DPL:0  P:1  AVL:0  L:1  D:0 G:1
> + *                ie, GDT entry: 00af9b000000ffff
> + *      Others: base: 0 limit:fffff Type:3 DPL:0  P:1  AVL:0  L:0  B:1 G:1
> + *                i,e GDT entry: 00cf93000000ffff
> + *              (except GS_BASE and FS_BASE that are guest provided).
>   */
>  #define VCPUOP_initialise            0
> 
> 
> Please lmk which is better.

The second, please.  Having the details in the interface is much
friendlier. 

> I can also redo the function prolog to explicitly provide entry details:
> 
> /*
>  * Set vmcs fields in support of vcpu_op -> VCPUOP_initialise hcall.  Called
>  * from arch_set_info_guest() which sets the (PVH relevant) non-vmcs fields.
>  *
>  * (NOTE: In case of VMCS, loading a selector doesn't cause the hidden fields
>  *        to be automatically loaded.)
>  *
>  * Guest Constraints: 
>  *       We load selectors here but not the hidden parts, except for 
>  *       GS_BASE and FS_BASE. This means we require the guest to have same 
>  *       hidden values as the default values loaded in the vmcs (in 
>  *       pvh_construct_vmcs()). 
>  *       For 64 bit guest:
>  *         CS: base:0 limit:fffff Type:0xb DPL:0  P:1  AVL:0  L:1  D:0 G:1
>  *             ie, GDT entry: 00af9b000000ffff
>  *         Others: base: 0 limit:fffff Type:3 DPL:0  P:1  AVL:0  L:0  B:1 G:1
>  *             i,e GDT entry: 00cf93000000ffff

Yes, that's helpful -- maybe a link back to the interface heder here too
so if anyone changes it they'll know to change both.

>  * In case of linux:
>  *     The boot vcpu calls this to set context for the non boot smp vcpu. The
>  *     call comes from cpu_initialize_context().  (boot vcpu 0 context is
>  *     set by the tools via do_domctl -> vcpu_initialise). 
>  *     For 64bit, GDT for bringup vcpu looks like (CS:0x10 DS/SS:0x18) :
>  *
>  *           ffff88007f704000:  0000000000000000 00cf9b000000ffff
>  *           ffff88007f704010:  00af9b000000ffff 00cf93000000ffff
>  *           ffff88007f704020:  00cffb000000ffff 00cff3000000ffff

I'm not sure that's as much use -- it certainly won't be kept in line with
any changes in the linux code.

Cheers,

Tim.

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