[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH for-4.11] x86/pv: Unconditionally hide EFER.SVME from PV guests

On 07/05/2018 09:00, Jan Beulich wrote:
>>>> On 07.05.18 at 09:30, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 07/05/2018 08:03, Jan Beulich wrote:
>>>>>> On 04.05.18 at 19:28, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>> --- a/xen/arch/x86/pv/emul-priv-op.c
>>>> +++ b/xen/arch/x86/pv/emul-priv-op.c
>>>> @@ -867,7 +867,9 @@ static int read_msr(unsigned int reg, uint64_t *val,
>>>>          return X86EMUL_OKAY;
>>>>      case MSR_EFER:
>>>> -        *val = read_efer();
>>>> +        /* Hide unknown bits, and unconditionally hide SVME from guests. 
>>>> */
>>>> +        *val = read_efer() & EFER_KNOWN_MASK & ~EFER_SVME;
>>>> +        /* Hide the 64-bit features from 32-bit guests. */
>>>>          if ( is_pv_32bit_domain(currd) )
>>>>              *val &= ~(EFER_LME | EFER_LMA | EFER_LMSLE);
>>> Wouldn't it be better then to also move the LMSLE hiding up?
>> Actually, on second consideration, LMSLE shouldn't be hidden.  If LMSLE
>> is actually active, then segment descriptors behave differently whether
>> the guest knows about it or not.
> Actually - the placement shouldn't matter at all: I think it can't be set 
> anyway,
> as we would only possibly allow HVM guests to set it, i.e. the EFER value
> loaded during #VMEXIT won't ever have the bit set at present. And even the
> HVM side code uniformly forces cpu_has_lmsl to false right now.

On 3rd consideration, the current placement is correct.  LMSLE, if set,
only has any effect on data segment when running with a long mode %cs. 
Therefore, the clobber is in the right place.  Were we ever to enable it
in Xen, this code would be correct.

On the HVM side, yes - we still have it disabled for migration reasons. 
That said, I think we should consider dropping it entirely.  Even AMD
think it is an unused feature.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.