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 14.50, Andrew Cooper ha escrit:
> On 30/09/15 13:35, Jan Beulich wrote:
>>>>> On 30.09.15 at 14:19, <roger.pau@citrix.com> wrote:
>>> El 30/09/15 a les 13.54, Jan Beulich ha escrit:
>>>>>>> On 30.09.15 at 13:37, <roger.pau@citrix.com> wrote:
>>>>>     /*
>>>>>      * 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.
>> Fundamentally, checks done here should include everything that
>> would otherwise lead to the domain getting killed due to failed
>> VMENTRY. I.e. both sets below may need extending.
>>
>>> 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?
>> I wouldn't check ESP at all. As to the two DPLs to, I don't think you
>> could launch a guest with these disagreeing.
>>
>>> 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.
>> Agreed, plus RIP being canonical.
> 
> LMA is a read-only bit with inconsistent semantics between Intel and AMD.
> 
> In particular, on Intel, LMA is not visible until LME has been set,
> which means that the typical setup of
> 
> mov $MSR_EFER, %ecx
> rdmsr
> bts $_EFER_LME, %eax
> wrmsr
> 
> Causes Xen to observe LME but not LMA when intercepting the wrmsr.
> 
> I think all you need to check is CR0.PG, CR4.PAE and EFER.LME (and guest
> X86_FEATURE_LM).  LMA will then leak through in subsequent rdmsr's

Although you don't need to set LMA in real hardware, AFAICT you need to
set it when using this interface, because hvm_update_guest_efer doesn't
set LMA if LME is enabled, which causes the vmentry to fail.

Roger.