[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 3/5] x86/xstate: Rework xstate_ctxt_size() as xstate_uncompressed_size()
- To: Jan Beulich <jbeulich@xxxxxxxx>
- From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
- Date: Tue, 4 May 2021 13:15:18 +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=GlMIyk+8wBM13t2FCxK4lt6KVBCkfBaWOQiPDpVj+Jw=; b=SU827rx/1RGEb/KSWV24AyJcuLiTTd+0UPsZ7bPzhgc6Tt0dXI459kQSTMTVUm1P0QNNzCupKrMswIrioPjGWxuCJ+RQDFx6NrF3O6yU42siq/fmo2jke8i8+rytaCuID5KclIXFIUc6af7vZdfpLNSYhZnatpPzaqXD9DCEa13/6dtn2KoaiYn68hSq57asvP4tbyWpQcW4ZKwlqSVquOWzBr4BJpx+9yW5u3iauSMayIuhuysxAytNuTDXz+aeH4pZnmAPCe27whUAwrXFCW4AcFTN7sX7I0i71R5eOg24Z3vYCtMq7bUF8+HNokxTyQy7Y626pY7VXQGDcf5G8g==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Vfq+1/3VrX/BaGc0kMJcHB23vPC8wQ3KIflJT/7XD3MzgvlUI+WDzOsHjf6tYjvfoBzVgtRWeVDrtsS2Yp7QQJLexGnGWcH+HwRsiPP2lDY5YG6W0syE0VQttrnj1YJzoGoa9qErIgHNrQ4lkQqdPCt2vu2vsGwHOQu+ataoK9Q8IKuHGtcriln+BDPFwM7afQ8+7uYMVNaWXBYqfmcoqhutYtDjdT9mRgYnLnSv9CXU3s3KZZtz6f/fiJNlXOUR4pS30pX+JwD60N7fP7GHRoSfQ5kHtbIWP+KddMezdO9X9+D8S+nkMQKQUGuZJVjv21cKj1nwpWb/EikgFvKSEg==
- Authentication-results: esa2.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
- Cc: Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
- Delivery-date: Tue, 04 May 2021 12:15:37 +0000
- Ironport-hdrordr: A9a23:86CKHarzoV4YdMZU8tork4waV5rAeYIsi2QD101hICF9WMqeis yogbAnxQb54Qx8ZFgMkc2NUZPufVry7phwiLN+AZ6DW03ctHKsPMVe6+LZsl7dMgnf0sIY6q t6aah5D7TLYGRSqcrh+gG3H5IB7bC8gcWVrNzTxXtsUg1mApsIh2wSNi+hHkJ7XwVAD5Yifa DshPZvnSaqengcc62AZkUtYu6rnbz2va79bQVDLxAq7xTmt0LO1JfKVyKV2RoTSFp0sNMfzV Q=
- Ironport-sdr: ujbbvgy4WMIDfX7QDtVq8Cl+HsMXiwoo24DmkUb3vMLyTnPOvcGe2Sgr2KXFsIfdn2LP8+KnM/ vwOcdg7uHJtRwfI6t6kzYgso/AMDZzGAbp+BqqcaISHcZy48SBnbTZKBaoxkwNCjco60ciLdLw rXIELmvI5iYF3HVuwR40JFc1JYO26XY63whNKnAMXcb2lj7KzjVTlhrrgnCkyC+hXDaceAwUW0 lOK9iRnGob4QDqRIB5LAlWuMKu+JZgmGG94Z5C3bWDZ7/cy70r0XhCwRiLUQYO2/RntcIX154D 87U=
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 04/05/2021 13:08, Jan Beulich wrote:
> On 03.05.2021 20:17, Andrew Cooper wrote:
>> On 03/05/2021 16:39, Andrew Cooper wrote:
>>> We're soon going to need a compressed helper of the same form.
>>>
>>> The size of the uncompressed image is a strictly a property of the highest
>>> user state. This can be calculated trivially with xstate_offsets/sizes, and
>>> is much faster than a CPUID instruction in the first place, let alone the
>>> two
>>> XCR0 writes surrounding it.
>>>
>>> Retain the cross-check with hardware in debug builds, but forgo it normal
>>> builds. In particular, this means that the migration paths don't need to
>>> mess
>>> with XCR0 just to sanity check the buffer size.
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> The Qemu smoketests have actually found a bug here.
>>
>> https://gitlab.com/xen-project/patchew/xen/-/jobs/1232118510/artifacts/file/smoke.serial
>>
>> We call into xstate_uncompressed_size() from
>> hvm_register_CPU_save_and_restore() so the previous "xcr0 == 0" path was
>> critical to Xen not exploding on non-xsave platforms.
>>
>> This is straight up buggy - we shouldn't be registering xsave handlers
>> on non-xsave platforms, but the calculation is also wrong (in the safe
>> directly at least) when we use compressed formats. Yet another
>> unexpected surprise for the todo list.
> I don't view this as buggy at all - it was an implementation choice.
> Perhaps not the best one, but still correct afaict. Then again I'm
> afraid I don't understand "in the safe directly at least", so I may
> well be overlooking something. Will wait for your updated patch ...
For now, it is a patch 2.5/5 which just puts a cpu_has_xsave guard
around the registration. Everything to do with xsave record processing
is unnecessary overhead on a non-xsave platform.
I don't intend to alter patch 3 as a consequence.
~Andrew
|
