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 30/09/15 a les 13.54, Jan Beulich ha escrit:
>>>> On 30.09.15 at 13:37, <roger.pau@citrix.com> wrote:
>> This is what I currently have prototyped according to the comments, it 
>> should allow starting the vCPU in all possible modes AFAICT.
> 
> Looks okay, one more comment:
> 
>> 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).
> 
> I'd leave out mentioning TR here (or else it'll be odd not to mention
> e.g. LDTR too). Perhaps just "..., so the cached parts of the segment
> registers get set to match that environment"?

That sounds fine. I'm going to update the patch and the FreeBSD part in
order to test it. Since we also spoke about adding sanity checks, I
wonder whether I should add those checks now, or leave them for a later
patch. IMHO those checks are only useful for developers.

For VCPU_HVM_MODE_32B:
 - rIP within CS limit.
 - Check that CS.DPL == SS.DPL.
 - rSP within SS limit.

TBH I don't think we should enforce the last two checks, starting with
an invalid stack should be fine as long as the user knows it. Maybe
print a warning/debug message in this case?

For VCPU_HVM_MODE_64B:
 - Check that cr0 has paging enabled.
 - Check that cr4 has pae enabled.
 - Check that efer has the LMA/LME bits set.

Those should be always enforced for long mode.

Roger.