[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 4/9] x86emul: support CMPccXADD
- To: Jan Beulich <jbeulich@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
- From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
- Date: Thu, 6 Apr 2023 20:28:33 +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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=5TTQKdhsyUGU4bjLTQMm7FDlgUpl6toPSRU4SGT/cwo=; b=gnsZxey7SuyabkwBr5ZWiDKHnSylpUIQLtxTOjQJp9T90th4Z4rMsErhPR3AfRjk9v3WwqLEFMMCqQ6v21aNr5IroPpnzFtiBihuesyEBHlO/nNqlD3f53Pw3my1SUnHynHHhnRVVKRpmg1FbqAxuKrDf5X3BFrHCigAK/y9628VqfY4OPhyvwbVGqPUJLAqJEseRMM7IBdrPq4Km3u6L/uEhRKaKU2/6uQkm+g6rMqT86o26ixZw2ZLYZrZQfT93KCZm0zvK+m2RCH0KUAB0ELWtRICutg5EWaMdgmPbyG4TBaP1yqQSMa7wTp8Vj/7+iBeN+TJOkdfXldZeNWZMQ==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=RnxF715zXNCxTFOsqJDp/sz9voXh6/aWCSn2BiI+gWgJzMTyOWxY119jV4zEJxLwNU/hCXefI6TvHw510maiSM4xaDlpXlJl3xYFLRV0exYPSDB33gkf6maHhfGTCoI2oq6DaugwHJZteApY8FGRsRGV0DcCAusBuKyORw7IQzkNXmWVypq8sSeINAe3c3+72ljN4VbZfyK2bY5tzeywacZk3sNZt0kHfU1ObDPckEM+IkknxVcC41P6cVUvMmcrxna2GtpCqOVb9WDLFgfeZhnsUHgSs604k0oU8mX0+hAn4qk6jKugRVtfT6oZU877b0W5VwkB3258WdDGujkkrA==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
- Cc: Wei Liu <wl@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>
- Delivery-date: Thu, 06 Apr 2023 19:29:16 +0000
- Ironport-data: A9a23:llOviqOtrDpn/cXvrR2IlsFynXyQoLVcMsEvi/4bfWQNrUoi0mZWz GJOUDvVPP2JYTHyeY13bITj9k0E7J6Dx4JmTwto+SlhQUwRpJueD7x1DKtS0wC6dZSfER09v 63yTvGacajYm1eF/k/F3oDJ9CU6jufQAOKnUoYoAwgpLSd8UiAtlBl/rOAwh49skLCRDhiE/ Nj/uKUzAnf8s9JPGj9SuvLrRC9H5qyo42tE5gJmP5ingXeF/5UrJMNHTU2OByOQrrl8RoaSW +vFxbelyWLVlz9F5gSNy+uTnuUiG9Y+DCDW4pZkc/HKbitq/0Te5p0TJvsEAXq7vh3S9zxHJ HehgrTrIeshFvWkdO3wyHC0GQkmVUFN0OevzXRSLaV/ZqAJGpfh66wGMa04AWEX0rxPB3tUz NI5ExEUS0+JnPju7oPrSeY506zPLOGzVG8ekldJ6GiBSNMZG9XESaiM4sJE1jAtgMwIBezZe 8cSdTtoalLHfgFLPVAUTpk5mY9EhFGmK2Ee9A3T+PdxujCIpOBy+OGF3N79U9qGX8hK2G2fo XrL5T/RCRAGLt2PjzGC9xpAg8eWxXiqB9xPTefQGvhCx1qo53ZLATEqTnjhg8OktVPkAdB1J BlBksYphe1onKCxdfH/VRClpH+PvjYHRsFdVeY97Wml2qfSpgqUGGUAZjpAc8A98t87QyQw0 V2ElM+vAiZg2JWKTVqN+7HSqim9URX5NkcHbC4ACA4aud/qpdhrigqVF444VqmoktfyBDf8h SiQqzQzjKkSishN0Lin+VfAgHSnoZ2hohMJ2zg7l1mNtmtRDLNJraTzgbQHxZ6s9Lqkc2Q=
- Ironport-hdrordr: A9a23:Zm3ewqieYkNIr1U+fYtkwF6w53BQX6J23DAbv31ZSRFFG/FwyP rCoB1L73XJYWgqM03IwerwSpVohEmsg6KdkrNhSItKPTOW9ldASbsP0WKM+UyfJ8STzJ8n6U 4kSdkMNDSSNyk6sS+Z2njILz9I+rDun87Y5pa9vg4dNT2Gc5sB0+46MHfqLqQffngEOXNTLu vg2iMznUveRZ1hVLXGOpBqZZm4mzT+ruOlXTc2QzI34gyHjTel85/9CQWV0y0fXTRG3Ks4/X KAtwDi/K2sv8ihzBXRzXXe4v1t6b7c4+oGKN2Hj8AULjn2qgKwf4RnRpWJoTAyp4iUmTAXue iJjwYrOsxy73/LXmWtuhvrxizpzToo4W+K8y7+vVLT5eDpTjczC85MnrtDdArIzkI8sNZ3wM twrgakXtdsfEv9dOuU3amGazha0m6P5VYym+8aiHJSFaMYdb9qtIQauHhYFZ8RdRiKorzORI NVbf301bJzSxe3fnrZtm5gzJiHRXIoBCqLRUAEp4i8zyVWtGoR9TpI+OUv2lM7sL4tQZhN4O rJdo5ykqtVc8MQZaVhQM8cXMqMDHDXSx6kChPNHb3eLtBWB5vxke+q3Fx13pD2RHUw9upppH 0VaiIGiYYwE3ieQvFmkqc7smGffI16NQ6dj/22rKIJzoEUf4CbeRFqkjgV4oydSsUkc4nmsr 6ISeVr6t/YXC3T8NVyrlTDs287EwhTbCXj0uxLFm5m5Pi7cbECntarOcruGA==
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 04/04/2023 3:52 pm, Jan Beulich wrote:
> Unconditionally wire this through the ->rmw() hook. Since x86_emul_rmw()
> now wants to construct and invoke a stub, make stub_exn available to it
> via a new field in the emulator state structure.
IMO, patch 5 should be re-ordered with this, because it removes one
incidental change that's not really related to CMPccXADD.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> # SDE: -grr or -srf
The ISE makes a point of noting that CMPccXADD is implicitly locked,
like XCHG. (Unlike XCHG, there isn't a valid reg/reg encoding.)
Right now, the xchg emulation overrides lock_prefix, but I have a
feeling that's stale now with the rmw() hook in place. But it is
dubious that we let xchg fall back to a non-atomic exchange if the rmw()
hook is missing.
Either way, I think it would be nice to clean that up so we don't have
differences in the handling of instructions which the ISE at least
claims are similar.
Tangentially, what about the RAO instructions?
> --- a/tools/tests/x86_emulator/x86-emulate.h
> +++ b/tools/tests/x86_emulator/x86-emulate.h
> @@ -934,6 +935,8 @@ decode_0f38(struct x86_emulate_state *s,
> ctxt->opcode |= MASK_INSR(s->vex.pfx, X86EMUL_OPC_PFX_MASK);
> break;
>
> + case X86EMUL_OPC_VEX_66(0, 0xe0)
> + ... X86EMUL_OPC_VEX_66(0, 0xef): /* cmp<cc>xadd */
I know the style is a little mixed in the emulator, but
+ case X86EMUL_OPC_VEX_66(0, 0xe0) ...
+ X86EMUL_OPC_VEX_66(0, 0xef): /* cmp<cc>xadd */
is more consistent with Xen style (because it's somewhat of a binary
operator), and more readable IMO.
> --- a/xen/include/public/arch-x86/cpufeatureset.h
> +++ b/xen/include/public/arch-x86/cpufeatureset.h
> @@ -278,6 +278,7 @@ XEN_CPUFEATURE(SSBD, 9*32+31) /
> /* Intel-defined CPU features, CPUID level 0x00000007:1.eax, word 10 */
> XEN_CPUFEATURE(AVX_VNNI, 10*32+ 4) /*A AVX-VNNI Instructions */
> XEN_CPUFEATURE(AVX512_BF16, 10*32+ 5) /*A AVX512 BFloat16 Instructions */
> +XEN_CPUFEATURE(CMPCCXADD, 10*32+ 7) /*A CMPccXADD Instructions */
Given the non-triviality of this instruction, I'd prefer to keep this
"a" until we've tried it on real hardware.
~Andrew
|
