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

Re: S3 resume issue in xstate_init

  • To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Tue, 17 Aug 2021 15:59:18 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=G0JyphNTTv/wgVjq2nBMZmFDueXKP14PKfVU1yUXoOo=; b=Rrcy20qCtGQ7lUnIsL+TXwPWiVHDiuDtwk/WJfrM/LMYpdCyMfEbBnVDsogsfQy3Hi3TK5I03bfuArCumvQjDZQsk0SYiTA6R3b7Yia+VWvZ+o9Z5NRvalyFvSvTNlwPlPNMCPlr6xxUQ+/u9xIs4aPmmuiWIm40RBzAk7I7MCnButyRY/dFMd97S78PoLlgTGeBIkEpzpEh3RKpFm248Q1OZBNbe9vefzq4IoIC4/voYB8uHgiqDc0TlQYC7aNs8LJBXusQ4LGXsMo9rN7V/HMSWIZI2MyFSxNT5el52unD+ZcfIuwaHi/DrBFTYvrcjfTayUze2NUF/VcI97MKlg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=aoJ+4rHsJFLocWUENOZJCcIs2P0sXg1mkt/6Pp8qKjtEnxSkgDrqmekVUeBH1zcVIiVX5Tw8V6M+YNgltyqKXG3U4jbEtBtnYAwHIwzDOi76KZ3cqiTrIRvgExSz6y5TgAtrIFJhI6czv9Y+U5kSiojQyEILM9mrbo2OiVfJrLuCV/cP5nVKDmTPdUitdIeUmAZvhAmO/1d4BiI0L56W/xVdcA32X/JwkbKprDw4baHmoOi6NMZOwG+ysjQyut+w65Vub4XnSBrFn6W26pC7jIld9mDy/aXxHJweLFSMyHgeFBZtCcHlGx/PngFXOLMOfyqmW4wPAPTx9tzup34k4w==
  • Authentication-results: invisiblethingslab.com; dkim=none (message not signed) header.d=none;invisiblethingslab.com; dmarc=none action=none header.from=suse.com;
  • Cc: xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Tue, 17 Aug 2021 13:59:27 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 17.08.2021 15:29, Andrew Cooper wrote:
> On 17/08/2021 14:21, Jan Beulich wrote:
>> On 17.08.2021 15:06, Andrew Cooper wrote:
>>> On 17/08/2021 13:53, Andrew Cooper wrote:
>>>> On 17/08/2021 13:21, Jan Beulich wrote:
>>>>>  I guess an easy fix would be to write 0 to
>>>>> this_cpu(xcr0) directly early in xstate_init(), maybe in an "else"
>>>>> to the early "if ( bsp )".
>>>>> I'm not sure though this would be a good fix longer term, as there
>>>>> might easily be other similar issues elsewhere. IOW we may need to
>>>>> see whether per-CPU data initialization wouldn't want changing.
>>>> We've got other registers too, like MSR_TSC_AUX, but I don't think we
>>>> want to be doing anything as drastic as changing how the initialisation
>>>> works.
>>>> The S3 path needs to explicitly write every register we do lazy context
>>>> switching of.
>>> Actually no - that's a dumb suggestion because the APs don't know
>>> better, and we don't want for_each_cpu() loops running from the BSP.
>>> Perhaps we want the cpu_down() logic to explicitly invalidate their
>>> lazily cached values?
>> I'd rather do this on the cpu_up() path (no point clobbering what may
>> get further clobbered, and then perhaps not to a value of our liking),
>> yet then we can really avoid doing this from a notifier and instead do
>> it early enough in xstate_init() (taking care of XSS at the same time).
> What we actually want to do is read the hardware register into the
> cached location.

Would you mind explaining why? I did consider doing so, but didn't
see the point when the first thing we do is write the register with
xfeature_mask (which can't be zero, and which hence would always get
written through to the register when the cached value is zero).

>  %xcr0 is possibly the only lazy register we also do
> extra sanity checks on.

I don't think we do?

(All of this is independent of my "x86/xstate: replace xsave_cntxt_size
and drop XCNTXT_MASK" I think, despite its reworking of the logic, and
which I'm still hoping to get reviewed at some point.)




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