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

Re: [PATCH] x86/xstate: reset cached register values on resume

  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Thu, 26 Aug 2021 09:50:40 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.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=1m+2iFF8RpNK/aFR1R4C/j7e36kWbDGgwOOxsM+KnqY=; b=LfQ06uuG7O84Pk/lU0FkONlsdrstwy0lLJDJiLy04/H301AEnxbmKq0RsspRmXOHh6OyB9Z7d9k3Lcl7pnXC5FTiIJVq218WOy0YkRE2MxRzedS9B6WoYk/jW4x5znTb0cjRphQHURopsObo7rME86j+TtBLmjpbW1PsIcZ+apkrWowxnbeI7BvnW8pQA8HWtHd4WOVwET+YGwO6Kzt0LbuUhZgpgI/8kUMHBIK672jmQT8SKVroAkOvHCzeDELb/MzxP8E0kjUPKDduFwxG28AYAkRV0NMTUzF3Wn8I7JM5T3Sk99VnKRnQJCSM0/m3wHrj25CbEDgJ/t1m9k9d+w==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ELos0GsJg3O1ZQow1lRht89pHhHI+QmPKk16p3KxtTWvIHZjcRcFXVJgL5+OMnq6v3vNAB3nKRyz/971vLNqe52MdS0lGXAdAEbJziLZm/gtzdz+WL+gbQtBg4mo4ZIpcRWSkg1pWvUPVah05dUO//Ihs1BDEVbwNRQkgwMBvPRHoPwKetmGjSI3wyN/GkdL3M3Kfk5EFNW+9B8OMCUHMH8zLPzSBUvn7izVYZcuj6G6EABe2nL0pLSwjY10Dgrw3N3MO1+PVgiss2GF2ck/j9fH9Y198/Nqu6S6BzUo67U0whcI5vP/m8uXdNKwGIJL2dqi4YmKzOFn5d2P/w1Oag==
  • Authentication-results: esa1.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Thu, 26 Aug 2021 08:50:58 +0000
  • Ironport-hdrordr: A9a23:+ZnEW6CREPBiXQXlHelo55DYdb4zR+YMi2TDt3oddfU1SL38qy nKpp4mPHDP5wr5NEtPpTniAtjjfZq/z/5ICOAqVN/PYOCPggCVxepZnOjfKlPbehEX9oRmpN 1dm6oVMqyMMbCt5/yKnDVRELwbsaa6GLjDv5a785/0JzsaE52J6W1Ce2GmO3wzfiZqL7wjGq GR48JWzgDQAkj+PqyAdx84t/GonayzqK7b
  • Ironport-sdr: 0hYusKMiWHPBDhU7eTj8JXxvRSMmEUpx0sHXyu6Ter46AmrLxJ0jvtYTN+Jg2wjgaT862jpL7s zc2HcedQVZY9o/taJhRyx/G3GTjzRkSVLUT6k117nv3GzJFwY+jbvI1ExdGMg3U3cq1PYUmUU8 +4Dt1JcKDKjR3X6qFkb4hO1/UZoV1HKKVRIV1702RPv+dzzVOBgRjb/coM7i84c3J96ev3GD/p ZwNlfI26JgwPaS3u65SgRRNgmPjzHf7KgsjSn1B3LqUx6PA3eQH+TaZVgUeh3ikK3XmBplL5Hi LRalfEVC0yWEq3nk2dZffui9
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 26/08/2021 08:40, Jan Beulich wrote:
> On 25.08.2021 18:49, Andrew Cooper wrote:
>> On 25/08/2021 16:02, Jan Beulich wrote:
>>> On 24.08.2021 23:11, Andrew Cooper wrote:
>>>  If
>>> the register started out non-zero (the default on AMD iirc, as it's
>>> not really masks there) but the first value to be written was zero,
>>> we'd skip the write.
>> There is cpuidmask_defaults which does get filled from the MSRs on boot.
>> AMD are real CPUID settings, while Intel is an and-mask.  i.e. they're
>> both non-zero (unless someone does something silly with the command line
>> arguments, and I don't expect Xen to be happy booting if the leaves all
>> read 0).
> Surely not all of them together, but I don't think it's completely
> unreasonable for one (say the XSAVE one, if e.g. XSAVE is to be turned
> off altogether for guests) to be all zero.
>> Each early_init_*() has an explicit ctxt_switch_levelling(NULL) call
>> which, given non-zero content in cpuidmask_defaults should latch each
>> one appropriately in the the per-cpu variable.
> Well, as you say - provided the individual fields are all non-zero.

The MSRs only exist on CPUs which have non-zero features in the relevant

The XSAVE and Therm registers could plausibly be 0.  Dom0 is first to
boot and won't expect to have XSAVE hidden, but we do zero all of leaf 6
in recalculate_misc()

There are certainly corner cases here to improve, but I think all
registers will latch on the first early_init_*(), except for Therm on
AMD which will latch on the first context switch from a PV guest back to

>>>> cpu/common.c:120:static DEFINE_PER_CPU(uint64_t, msr_misc_features);
>>> Almost the same here - we only initialize the variable on the BSP
>>> afaics.
>> No - way way way worse, I think.
>> For all APs, we write 0 or MSR_MISC_FEATURES_CPUID_FAULTING into
>> MSR_INTEL_MISC_FEATURES_ENABLES, which amongst other things turns off
>> Fast String Enable.
> Urgh, indeed. Prior to 6e2fdc0f8902 there was a read-modify-write
> operation. With the introduction of the cache variable this went
> away, while the cache gets filled for BSP only.

Yeah - I really screwed up on that one...  It was also part of the PV
Shim work done in a hurry in the lead up to Spectre/Meltdown.


Most of the content wants to be identical on all cores, so we do want to
fix up AP values with the BSP value if they differ, but we also want a
software cache covering at least the CPUID_FAULTING bit so we don't have
a unnecessary MSR read on the context switch path.

I'll try to do something better.




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