Re: [Xen-devel] [PATCH RESEND v1 5/7] x86: Implement Intel Processor Trace context switch

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 02.05.18 at 18:51, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 02/05/18 17:15, Wei Liu wrote:
>> On Wed, May 02, 2018 at 09:43:33AM -0600, Jan Beulich wrote:
>>>>>> On 02.05.18 at 17:19, <wei.liu2@xxxxxxxxxx> wrote:
>>>> On Fri, Apr 27, 2018 at 08:53:30AM +0000, Kang, Luwei wrote:
>>>>>>> Load/Store Intel processor trace register in context switch.
>>>>>>> MSR IA32_RTIT_CTL is loaded/stored automatically from VMCS.
>>>>>>> When Intel PT is supported in guest, we need load/restore PT MSRs only
>>>>>>> when PT is enabled in guest.
>>>>>>>
>>>>>>> Signed-off-by: Luwei Kang <luwei.kang@xxxxxxxxx>
>>>>>> Is there a reason to not use xsaves/xrstors when they are available?
>>>>>>
>>>>> There have two method to implement context switch(manual and 
>>>>> xsave/xrstors).
>>>>> The first method is more directly and also won't have any performance 
>>>> overhead if intel PT is disabled.
>>>>> If use xsave/xrstors we need to check if it available and whether PT is 
>>>> supported  in XSS (CPUID.0D(ecx=1).ecx).
>>>>> I will think about this scenario and may make an independent patch to 
>>>>> enable 
> 
>>>> it. 
>>>>
>>>> Fair enough.
>>>>
>>>> I figure Xen doesn't understand xsaves (among other xsave* features) at
>>>> the moment so a dedicated series to enable that is required.
>>> I wouldn't be surprised if there were bugs, but if you look at xstate.c 
> you'll
>>> find a number of references to XSAVES.
>> What I meant was in xstate.c:xstate_init:
>>
>>    /* Mask out features not currently understood by Xen. */
>>     eax &= (cpufeat_mask(X86_FEATURE_XSAVEOPT) |
>>             cpufeat_mask(X86_FEATURE_XSAVEC) |
>>             cpufeat_mask(X86_FEATURE_XGETBV1) |
>>             cpufeat_mask(X86_FEATURE_XSAVES));
>>
>> Also PT is missing in state component definitions in x86-defns.h.
> 
> PT is the first defined XSS state, and was introduced at the same time
> as XSAVES.  Xen's xsave logic should be updated to use XSAVES/XRSTORS if
> available (which, other than its privileged nature, works identically to
> XSAVEC AFAICT).

We do that already (except that we don't "if available" but "if necessary",
i.e. if any state requiring XSAVES is to be saved/restored).

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel