On 09/10/2024 10:32 am, Borislav Petkov wrote:
On Wed, Oct 09, 2024 at 09:34:37AM +0200, Peter Zijlstra wrote:
You need ifdeffery either way around, either directly like this or for that macro. This is simple and straight forward.
Nothing in this file full of macros is simple. In any case, I would've done this as the ifdeffery is shorter and the macro is simpler. We have this coding pattern in a lot of headers, abstracting 32-bit vs 64-bit machine details, and it is a very common and familiar one:
/*
- In 32bit mode, the memory operand must be a %cs reference. The data
- segments may not be usable (vm86 mode), and the stack segment may not be
- flat (ESPFIX32).
*/ #ifdef CONFIG_X86_64 #define VERW_ARG "verw mds_verw_sel(%rip)" #else /* CONFIG_X86_32 */ #define VERW_ARG "verw %cs:mds_verw_sel" #endif
/*
- Macro to execute VERW instruction that mitigate transient data sampling
- attacks such as MDS. On affected systems a microcode update overloaded VERW
- instruction to also clear the CPU buffers. VERW clobbers CFLAGS.ZF.
- Note: Only the memory operand variant of VERW clears the CPU buffers.
*/ .macro CLEAR_CPU_BUFFERS ALTERNATIVE "", VERW_ARG, X86_FEATURE_CLEAR_CPU_BUF .endm
I'll bite. Why do you think this form is is better?
You've now got VERW_ARG leaking across the whole kernel, and a layer of obfuscatio^W indirection in CLEAR_CPU_BUFFERS.
Admittedly, when I wrote this fragment as a suggestion[1], the 32bit comment was in the main comment because there really is no need for it to be separate.
But abstracting away VERW_ARG like this hampers legibility/clarity, rather than improving it IMO.
~Andrew
[1] https://lore.kernel.org/lkml/5703f2d8-7ca0-4f01-a954-c0eb1de930b4@citrix.com...