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

From: "Jan Beulich" <JBeulich@suse.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Kevin Tian <kevin.tian@intel.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Luwei Kang <luwei.kang@intel.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Tim Deegan <tim@xen.org>,
	xen-devel@lists.xen.org, Jun Nakajima <jun.nakajima@intel.com>,
	Wei Liu <wei.liu2@citrix.com>
Subject: Re: [PATCH RESEND v1 5/7] x86: Implement Intel Processor Trace context switch
Date: Thu, 03 May 2018 01:27:52 -0600	[thread overview]
Message-ID: <5AEAB9F802000078001C0588@prv1-mh.provo.novell.com> (raw)
In-Reply-To: <86d05029-599e-8fa4-d98c-a340e08297f5@citrix.com>

>>> On 02.05.18 at 18:51, <andrew.cooper3@citrix.com> 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@citrix.com> 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@intel.com>
>>>>>> 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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel