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

[Mukesh Rathor <mukesh.rathor@oracle.com>]

On Thu, 25 Jul 2013 14:47:55 +0100
Tim Deegan <tim@xen.org> wrote:

> At 18:59 -0700 on 23 Jul (1374605957), Mukesh Rathor wrote:
....
> > + * Set vmcs fields in support of vcpu_op -> VCPUOP_initialise
> > hcall.  Called
> > require the
> > + *       guest to have same hidden values as the default values
> > loaded in the
> > + *       vmcs in pvh_construct_vmcs(), ie, the GDT the vcpu is
> > coming up on
> > + *       should be something like following,
> > + *       (from 64bit linux, CS:0x10 DS/SS:0x18) :
> > + *
> > + *           ffff88007f704000:  0000000000000000 00cf9b000000ffff
> > + *           ffff88007f704010:  00af9b000000ffff 00cf93000000ffff
> > + *           ffff88007f704020:  00cffb000000ffff 00cff3000000ffff
> 
> What a bizarre interface!  Why does this operation not load the hidden
> parts from the GDT/LDT that the caller supplied?

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). 

> 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.

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
 *
 * 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
 *
 */
int vmx_pvh_set_vcpu_info(struct vcpu *v, struct vcpu_guest_context *ctxtp)


Hope that helps.

thanks
mukesh