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

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

El 29/09/15 a les 12.48, Jan Beulich ha escrit:
>>>> On 29.09.15 at 12:37, <andrew.cooper3@citrix.com> wrote:
>> On 29/09/15 11:33, Jan Beulich wrote:
>>>>>> On 29.09.15 at 12:25, <andrew.cooper3@citrix.com> wrote:
>>>> On 29/09/15 11:07, Jan Beulich wrote:
>>>>>>>> On 29.09.15 at 12:00, <andrew.cooper3@citrix.com> wrote:
>>>>>> Therefore, we are back to the question of whether to provide all segment
>>>>>> registers, or specify a flat layout without specific selector values.  I
>>>>>> would prefer the former to the latter.
>>>>> If we don't go the CS+SS only route, then yes, I'd too prefer
>>>>> completing the set (I would probably agree with not adding FS
>>>>> and GS, and even recommend against it in the 64-bit variant,
>>>>> but I do insist on ES in that case).
>>>> I would still err on the CS/SS/DS/ES side given a straight choice.  It
>>>> offers more flexibility for rarer usecases.
>>> Okay, all four of them then for 32-bit, and just CS and SS for 64-bit?
>>
>> Is SS needed for 64bit?  It is expected to be NUL just like DS and ES.
> 
> Indeed, we should be able to get away without it. And for CS all
> we'd need would be a selector.

Ok thanks, so we seem to have a consensus. Before posting a new 
revision, does the following vcpu_hvm_context look fine to both of you:

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.
     */
    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;

    /*
     * Setting the CS attributes field is allowed in order to
     * start in compatibility mode.
     */
    uint16_t cs_ar;
};

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;
};

Thanks, Roger.