Re: [PATCH v6 24/29] xen/x86: allow HVM guests to use hypercalls to bring up vCPUs

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 30/09/15 12:37, Roger Pau Monné wrote:
> El 30/09/15 a les 12.03, Jan Beulich ha escrit:
>>>>> On 29.09.15 at 18:49, <roger.pau@citrix.com> wrote:
>>> El 29/09/15 a les 18.20, Jan Beulich ha escrit:
>>>>>>> On 29.09.15 at 18:01, <roger.pau@citrix.com> wrote:
>>>>> Yes, I should add back all the registers here, so it looks like:
>>>>>
>>>>>      uint32_t cs_base;
>>>>>      uint32_t ds_base;
>>>>>      uint32_t ss_base;
>>>>>      uint32_t es_base;
>>>>>      uint32_t cs_limit;
>>>>>      uint32_t ds_limit;
>>>>>      uint32_t ss_limit;
>>>>>      uint32_t es_limit;
>>>>>      uint16_t cs_ar;
>>>>>      uint16_t ds_ar;
>>>>>      uint16_t ss_ar;
>>>>>      uint16_t es_ar;
>>>> Or specify that compat mode entry is via using 32-bit register state.
>>> Yes, that should also work. If that's the preference then I guess we 
>>> can remove all the selectors stuff from the 64bit structure.
>> That's my preference (with the EFER comment then accordingly
>> updated). Andrew?
> This is what I currently have prototyped according to the comments, it 
> should allow starting the vCPU in all possible modes AFAICT.
>
> struct vcpu_hvm_x86_32 {
>     uint32_t eax;
>     uint32_t ecx;
>     uint32_t edx;
>     uint32_t ebx;
>     uint32_t esp;
>     uint32_t ebp;
>     uint32_t esi;
>     uint32_t edi;
>     uint32_t eip;
>     uint32_t eflags;
>
>     uint32_t cr0;
>     uint32_t cr3;
>     uint32_t cr4;
>
>     /*
>      * EFER should only be used to set the NXE bit (if required)
>      * when starting a vCPU in 32bit mode with paging enabled or
>      * to set the LME/LMA bits in order to start the vCPU in
>      * compatibility mode.
>      */
>     uint64_t efer;
>
>     uint32_t cs_base;
>     uint32_t ds_base;
>     uint32_t ss_base;
>     uint32_t es_base;
>     uint32_t tr_base;
>     uint32_t cs_limit;
>     uint32_t ds_limit;
>     uint32_t ss_limit;
>     uint32_t es_limit;
>     uint32_t tr_limit;
>     uint16_t cs_ar;
>     uint16_t ds_ar;
>     uint16_t ss_ar;
>     uint16_t es_ar;
>     uint16_t tr_ar;
> };
>
> struct vcpu_hvm_x86_64 {
>     uint64_t rax;
>     uint64_t rcx;
>     uint64_t rdx;
>     uint64_t rbx;
>     uint64_t rsp;
>     uint64_t rbp;
>     uint64_t rsi;
>     uint64_t rdi;
>     uint64_t rip;
>     uint64_t rflags;
>
>     uint64_t cr0;
>     uint64_t cr3;
>     uint64_t cr4;
>     uint64_t efer;
>
>     /*
>      * Using VCPU_HVM_MODE_64B implies that the vCPU is launched
>      * directly in long mode, so the type of the cached part
>      * of the TR register is set to describe a 64-bit TSS (Busy).
>      * The cached part of the CS register will also have the L bit
>      * set (64-bit code segment).
>      *
>      * If the user wants to launch the vCPU in compatibility mode
>      * the 32-bit structure should be used instead.
>      */
> };
>
> struct vcpu_hvm_context {
> #define VCPU_HVM_MODE_32B 0  /* 32bit fields of the structure will be used. */
> #define VCPU_HVM_MODE_64B 1  /* 64bit fields of the structure will be used. */
>     uint32_t mode;
>
>     /* CPU registers. */
>     union {
>         struct vcpu_hvm_x86_32 x86_32;
>         struct vcpu_hvm_x86_64 x86_64;
>     } cpu_regs;
> };
>
> Roger.

Looks good to me.

~Andrew