[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] xen: Work around Clang-IAS macro \@ expansion bug
- To: Jan Beulich <jbeulich@xxxxxxxx>
- From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
- Date: Fri, 24 Feb 2023 12:15:27 +0000
- 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=4U8NCPg4+c5YLYKjwqjmKVf47QAqrWwrWpmb9THUcTc=; b=LGsObn6Dh/aJzmI/hNrwDhvjBexZG3R8Kwadotjv81muShNsOxTeElvwCsTfZNUp1sQQ+mC2NQn5An5Br4IZAjx4k7AG91FiFqoRuw2gaGD2WQ3ebY8p/roHIeSJXjBD7vA+8QDXUxKY3X1pNtEvQks3w+msQ8Z0/t5Ou5SlujW0siaLVGXCHtd/hlc4LSH5f1EssWtsbpBnD2cfEs1CdEuayQp7TZmd/RR3KqxT0A+HDrsWMWVhMAjKBpxQUj3dGqtDR5rtEV/fdS0FCsJ10XFSCIir2eSsXrXeDJWHbL1Ta9z9dKhzNH8uldWsJHoAGxj8qup37HVFJQ87TofWgQ==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=hE1JrfethE1OtGyi2KOBU38qVUFHsTzZAuUy8G5MEAttsLJf0N+e7vfmUbHlQ/q9jrL4vbIcMqgqseuCYqvmtcCN0xKEWVPZVgkkWrvu3PWiwDK0cxDOJSGIIZsseAqacMna8A/U6XunNuAX6gY2WNAAyUrMiVpnTP3C6XF51GIoPUqr+mcVuwYtKL+HesH8md6z0okQGkHC71GK7Z7t/zHIKHKY2Cn/TvC5UKMNdQZCR3Xu3d15OVBcn0YWTOkaX1F9hGKGYq+fQf7ZxQJCc6wbbTbAbzK2MJPQnNncMX/VeAGrVpzqQVKg1nz0+ZkJ8wT/paJmFEBGiTjM4mdfcw==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
- Cc: Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
- Delivery-date: Fri, 24 Feb 2023 12:15:53 +0000
- Ironport-data: A9a23:lNSzI6g07dgkfcGysgSrgihxX161QhEKZh0ujC45NGQN5FlHY01je htvXm7VPP2OMWKjfY10PoXk8EIHu8fWndBgGVBsqns3Rnwb9cadCdqndUqhZCn6wu8v7q5Ex 55HNoSfdpBcolv0/ErF3m3J9CEkvU2wbuOgTrWCYmYpHlUMpB4J0XpLg/Q+jpNjne+3CgaMv cKai8DEMRqu1iUc3lg8sspvkzsy+qWt0N8klgZmP6sT5gWCzyN94K83fsldEVOpGuG4IcbiL wrz5OnR1n/U+R4rFuSknt7TGqHdauePVeQmoiM+t5mK2nCulARrukoIHKN0hXNsoyeIh7hMJ OBl7vRcf+uL0prkw4zxWzEAe8130DYvFLXveRBTuuTLp6HKnueFL1yDwyjaMKVBktubD12i+ tQEFHciPxmEgd7r+5GGE/YrieF5durkadZ3VnFIlVk1DN4AaLWaGuDgw48d2z09wMdTAfzZe swVLyJ1awjNaAFOPVFRD48imOCvhT/0dDgwRFC9/PJrpTSMilEhluGzYLI5efTTLSlRtm+eq njL4CLSBRYCOcbE4TGE7mitlqnEmiaTtIc6ReLiraY72QL7Kmo7ISczCBjj/N6FiWX9YfVCE x0e/CUhlP1nnKCsZpynN/Gim1aGtBMBX9tbE8Uh9RqAjKHT5m6xGWwsXjNHLts8u6ceVTEsk 1OEgd7tLThuq6GOD2KQ8K+OqjG/MjRTKnUNDRLoViMA6tjn5Ys13hTGS486FLbv14OlXzbt3 zqNsS4ywa0JitIG3Lm6+laBhC+wop/OTUg+4QC/sn+Z0z6VrbWNP+SAgWU3J94ZRGpFZjFtZ EQ5pvU=
- Ironport-hdrordr: A9a23:IhJlF6muiCffZDh7ZA7wv1rCmALpDfIX3DAbv31ZSRFFG/Fw9v re5cjzsCWetN9/YhAdcLy7VpVoOEmwyXct2+Us1NSZLW/bURWTXeJfBOLZqlWOJ8SZzJ856U 5OSdkbNDSaNzhHZKjBkWuF+whK+rO6GLrCv5a480tQ
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 24/02/2023 7:14 am, Jan Beulich wrote:
> On 23.02.2023 21:36, Andrew Cooper wrote:
>> https://github.com/llvm/llvm-project/issues/60792
>>
>> It turns out that Clang-IAS does not expand \@ uniquely in a translaition
>> unit, and the XSA-426 change tickles this bug:
>>
>> <instantiation>:4:1: error: invalid symbol redefinition
>> .L1_fill_rsb_loop:
>> ^
>> make[3]: *** [Rules.mk:247: arch/x86/acpi/cpu_idle.o] Error 1
>>
>> Extend DO_OVERWRITE_RSB with an optional parameter so C callers can mix %= in
>> too, which Clang does seem to expand properly.
>>
>> Fixes: 63305e5392ec ("x86/spec-ctrl: Mitigate Cross-Thread Return Address
>> Predictions")
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> A little hesitantly
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
Thanks.
>
>> --- a/xen/arch/x86/include/asm/spec_ctrl.h
>> +++ b/xen/arch/x86/include/asm/spec_ctrl.h
>> @@ -83,7 +83,7 @@ static always_inline void spec_ctrl_new_guest_context(void)
>> wrmsrl(MSR_PRED_CMD, PRED_CMD_IBPB);
>>
>> /* (ab)use alternative_input() to specify clobbers. */
>> - alternative_input("", "DO_OVERWRITE_RSB", X86_BUG_IBPB_NO_RET,
>> + alternative_input("", "DO_OVERWRITE_RSB xu=%=", X86_BUG_IBPB_NO_RET,
> Especially with there possibly appearing more cases where we need to
> add such, wrapping the extra parameter in a C macro continues to
> seem better to me, for having a minimal level of documentation (I
> has CLANG in the suggested name for exactly this purpose) right at
> the place of use. The way you have it you force readers to go look
> up the assembler macro and read through the commentary there in order
> to find any explanation for the oddity.
I'm not massively happy with the patch in this form, but it is less bad
than splitting the name out.
We do not separate out parameters elsewhere. Doing so adds cognitive
complexity to following the C, because now instead of having 2 places to
look at to figure out what's going on, you have 3.
Even a name like CLANG_EXTRA_UNIQUE is only meaningful to you and me.
Everyone else has to go and find the two other places to understand
what's going on.
Finally, and most importantly, despite having moved to a 2-char macro
parameter name, there's still not a meaningful identifier here in C
that's shorter.
I do understand the want to try and make this more obvious, but adding
cognitive complexity and code volume isn't a good way of improving things.
~Andrew
|
