Messages in this thread | | | Date | Tue, 17 Mar 2015 11:00:46 +0100 | From | Quentin Casasnovas <> | Subject | Re: [PATCH RFC 0/2] x86/fpu: avoid "xstate_fault" in xsave_user/xrestore_user |
| |
On Tue, Mar 17, 2015 at 10:47:50AM +0100, Borislav Petkov wrote: > On Mon, Mar 16, 2015 at 11:37:44PM +0100, Quentin Casasnovas wrote: > > ... > > > __user_insn("btl [var2], %0 \n\t", > > , /* no outputs, no need for dummy arg */ > > SINGLE_ARG("r" (var1), [var2] "r" (var2)), /* two inputs */ > > "cc"); > > So this becomes pretty unreadable IMO. And we shouldn't go nuts with > optimizing this and sacrifice readability a lot. > > TBH, I'd much prefer: > > if (static_cpu_has_safe(X86_FEATURE_XSAVEOPT)) { > check_insn(XSAVEOPT, ...); ^ > return; > } > > if (static_cpu_has_safe(X86_FEATURE_XSAVES)) { > check_insn(XSAVES); > return; > } > > check_insn(XSAVE, ...) > > which is pretty clear. >
Fair point, but AFAIUI we can't do check_insn(XSAVES) alone as of today, and the "..." in your "check_isns(XSAVEOPT, ...)" code above would still need to contain the outputs operands.
My suggestion was to rework (check|user)_insn() so it can accept zero to N inputs, outputs or clobbers to make it generic enough so the snippet of code you've written becomes valid, and maybe move those macro where they can be used for other sub-systems?
Am I missing something?
Quentin
| |