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

Re: [PATCH] SVM: avoid VMSAVE in ctxt-switch-to

  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Mon, 19 Oct 2020 16:52:36 +0100
  • Authentication-results: esa2.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Mon, 19 Oct 2020 15:52:45 +0000
  • Ironport-sdr: 6NVJcIB61YKqhRjmvbljT1RA1LC67gi89xaDcbMmJ+aWEYpjdv0N3iSsx7YiE6ZsFvmS490tDS bRY1PX2iw/oe2EH+zcl5ZLo52+h8FxqcQ0o0pt3ofHazw8Iv+opCxXhFmE627RQZK49eGaX0og 5GPv1lZ0Acn5FvnzpaDgTpnN6gwU5LjIXrn9cezZ52x4KTucKlbrqCFi5f+bYKoCCNHpGgcnGh /ozKuP+IGtVB396KlXDZMB5r0TcVjvx8kaxFxg3wZn+g0kNUV0vAnoCFkxF+KKuXHFYBthOQtt fOo=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 19/10/2020 16:47, Jan Beulich wrote:
> On 19.10.2020 17:22, Andrew Cooper wrote:
>> On 19/10/2020 16:02, Jan Beulich wrote:
>>> On 19.10.2020 16:30, Andrew Cooper wrote:
>>>> On 19/10/2020 15:21, Jan Beulich wrote:
>>>>> On 19.10.2020 16:10, Andrew Cooper wrote:
>>>>>> On 19/10/2020 14:40, Jan Beulich wrote:
>>>>>>> Of the state saved by the insn and reloaded by the corresponding VMLOAD
>>>>>>> - TR, syscall, and sysenter state are invariant while having Xen's state
>>>>>>>   loaded,
>>>>>>> - FS, GS, and LDTR are not used by Xen and get suitably set in PV
>>>>>>>   context switch code.
>>>>>> I think it would be helpful to state that Xen's state is suitably cached
>>>>>> in _svm_cpu_up(), as this does now introduce an ordering dependency
>>>>>> during boot.
>>>>> I've added a sentence.
>>>>>> Is it possibly worth putting an assert checking the TR selector?  That
>>>>>> ought to be good enough to catch stray init reordering problems.
>>>>> How would checking just the TR selector help? If other pieces of TR
>>>>> or syscall/sysenter were out of sync, we'd be hosed, too.
>>>> They're far less likely to move relative to tr, than everything relative
>>>> to hvm_up().
>>>>> I'm also not sure what exactly you mean to do for such an assertion:
>>>>> Merely check the host VMCB field against TSS_SELECTOR, or do an
>>>>> actual STR to be closer to what the VMSAVE actually did?
>>>> ASSERT(str() == TSS_SELECTOR);
>>> Oh, that's odd. How would this help with the VMCB?
>> It wont.
>> We're not checking the behaviour of the VMSAVE instruction.  We just
>> want to sanity check that %tr is already configured.
> TR not already being configured is out of question in a function
> involved in context switching, don't you think? I thought you're
> worried about the VMCB not being set up correctly? Or are you in
> the end asking for the suggested assertion to go into
> _svm_cpu_up(), yet I didn't understand it that way?

I meant in _svm_cpu_up().  It is only at at __init time where the new
implicit dependency is created.

>> This version is far more simple than checking VMCB.trsel, which will
>> require a map_domain_page().
> In the general case yes, but in the most common case (PV also
> enabled) we have a mapping already (host_vmcb_va).

Still rather more invasive than I was hoping for a quick sanity check
that ought never to fire.




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