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

["Roger Pau Monné" <roger.pau@citrix.com>]

El 24/07/15 a les 13.11, Andrew Cooper ha escrit:
> On 24/07/15 10:59, Roger Pau Monné wrote:
>> 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)
>> #define _VCPUHVM_online                2
>> #define VCPUHVM_online                 (1<<_VCPUHVM_online )
>>      uint32_t flags;                         /* VCPUHVM_* flags.   */
>>      struct cpu_hvm_regs user_regs;          /* CPU registers.     */
>> };
> 
> To avoid making the 32bit (and optionally 16bit) massive as a side
> effect of 64bit, can I suggest
> 
> uint32_t flags;
> union {
>     cpu_hvm32_regs;
>     cpu_hvm64_regs;
> };
> 
> Which allows hvm32_regs to be a far smaller structure.

But what's the benefit of this? vcpu_hvm_context is going to be equally
large, and that's what we pass as the hypercall argument. This is
mimicking the format of the structure that's already used on ARM, so I
would rather keep it as is.

>>
>> #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);
>>      __DECL_GP_REG(flags);
>>
>>      /* Control registers.         */
>>      uint64_t cr[8];
>>      /* Valid on amd64 only.       */
>>      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.
> 
> To start straight in 64bit, you need gdtr and cs as a minimum
> 
> With that in mind, room for each of the task registers, and segment
> selectors.

I was planning to say that on both 32 and 64 bits we start with a flat
GDT (like we do for the BSP). But we can also arrange for this to be set
by the user. So we would need to add the following fields:

     uint{16/32/64}_t gdtr;
     uint16_t ss, es, ds, fs, gs;
     uint16_t tr;

Roger.