Re: [Xen-devel] [PATCH v3 07/32] xen/x86: fix arch_set_info_guest for HVM guests

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.07.15 at 11:59, <roger.pau@xxxxxxxxxx> wrote:
> Just to recap and to make sure I got all the points:
> 
>  - vCPU initialization from the toolstack (BSP initialization) is going 
> to be done using XEN_DOMCTL_sethvmcontext.
>  - XEN_DOMCTL_{get/set}vcpucontext is going to return EOPNOTSUPP for 
> HVM guests.
>  - A new vcpu_guest_contatext (vcpu_hvm_context?) is going to be 
> introduced together with a a new VCPUOP_initialize hypercall 
> (VCPUOP_hvm_initialize?).

Since the current one is called VCPUOP_initialise, why not just
VCPUOP_initialize? Having "hvm" in the name when it's meant to
also be used for PVH may end up being confusing.

> The following is a layout proposal for the new vcpu_context:
> 
> struct vcpu_hvm_context {
> /* 32bit fields of the structure will be used. */
> #define _VCPUHVM_MODE_32B              0
> #define VCPUHVM_MODE_32B               (1<<_VCPUHVM_MODE_32B)
> /* 64bit fields of the structure will be used. */
> #define _VCPUHVM_MODE_64B              1
> #define VCPUHVM_MODE_64B               (1<<_VCPUHVM_MODE_64B)

16-bit mode?

> #define _VCPUHVM_online                2
> #define VCPUHVM_online                 (1<<_VCPUHVM_online )

What is this needed for?

>     uint32_t flags;                         /* VCPUHVM_* flags.   */
>     struct cpu_hvm_regs user_regs;          /* CPU registers.     */
> };
> 
> #if defined(__GNUC__) && !defined(__STRICT_ANSI__)
> /* Anonymous union includes both 32- and 64-bit names (e.g., ebp/rbp). */
> # define __DECL_REG(n64, n32) union {          \
>         uint64_t n64;                          \
>         uint32_t n32;                          \
>     }
> #else
> /* Non-gcc sources must always use the proper 64-bit name (e.g., rbp). */
> #define __DECL_REG(n64, n32) uint64_t n64
> #endif
> 
> #define __DECL_GP_REG(n) __DECL_REG(r##n, e##n)
> 
> struct cpu_hvm_regs {
>     /* General purpose registers. */
>     __DECL_GP_REG(bx);
>     __DECL_GP_REG(cx);
>     __DECL_GP_REG(dx);
>     __DECL_GP_REG(si);
>     __DECL_GP_REG(di);
>     __DECL_GP_REG(bp);
>     __DECL_GP_REG(ax);
>     __DECL_GP_REG(ip);
>     __DECL_GP_REG(sp);

Please put GPRs in their natural order - no need to mimic the order
in any existing structure.

>     __DECL_GP_REG(flags);

r8-r11, selector and descriptor registers, pseudo descriptor registers.
Or else for all of them their default state would need to be spelled out.


>     /* Control registers.         */
>     uint64_t cr[8];
>     /* Valid on amd64 only.       */

Fields valid/useful in one mode only should probably be put in
union-ized sub-structures.

Jan

>     uint64_t efer;
> 
>     /* Debug registers.           */
>     uint64_t db[8];  
> };
> 
> #undef __DECL_GP_REG
> #undef __DECL_REG
> 
> Of course the APs will be allowed to start in any mode they wish, 
> regardless of the mode the guest is currently running on.
> 
> Roger.




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