On Wed, Aug 31, 2022, Uros Bizjak wrote: > On Wed, Aug 17, 2022 at 5:58 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > > > +PeterZ > > > > On Wed, Aug 17, 2022, Uros Bizjak wrote: > > > There is no need to declare vmread_error asmlinkage, its arguments > > > can be passed via registers for both, 32-bit and 64-bit targets. > > > Function argument registers are considered call-clobbered registers, > > > they are saved in the trampoline just before the function call and > > > restored afterwards. > > > > I'm officially confused. What's the purpose of asmlinkage when used in the kernel? > > Is it some historical wart that's no longer truly necessary and only causes pain? > > > > When I wrote this code, I thought that the intent was that it should be applied to > > any and all asm => C function calls. But that's obviously not required as there > > are multiple instances of asm code calling C functions without annotations of any > > kind. > > It is the other way around. As written in coding-style.rst: > > Large, non-trivial assembly functions should go in .S files, with corresponding > C prototypes defined in C header files. The C prototypes for assembly > functions should use ``asmlinkage``. > > So, prototypes for *assembly functions* should use asmlinkage. I gotta imagine that documentation is stale. I don't understand why asmlinkage would be a one-way thing. > That said, asmlinkage for i386 just switches ABI to the default > stack-passing ABI. However, we are calling assembly files, so the > argument handling in the callee is totally under our control and there > is no need to switch ABIs. It looks to me that besides syscalls, > asmlinkage is and should be used only for a large imported body of asm > functions that use standard stack-passing ABI (e.g. x86 crypto and > math-emu functions), otherwise it is just a burden to push and pop > registers to/from stack for no apparent benefit. Yeah, this is what I'm confused about. Unless there's something we're missing, we should update the docs to clarify when asmlinkage is actually needed. > > And vmread_error() isn't the only case where asmlinkage appears to be a burden, e.g. > > schedule_tail_wrapper() => schedule_tail() seems to exist purely to deal with the > > side affect of asmlinkage generating -regparm=0 on 32-bit kernels. > > schedule_tail is external to the x86 arch directory, and for some > reason marked asmlinkage. So, the call from asm must follow asmlinkage > ABI. Ahhh, it's a common helper that's called from assembly on other architectures. That makes sense. Thanks much!
