Re: [XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 30 Oct 2023, Jan Beulich wrote:
> On 30.10.2023 10:11, Nicola Vetrini wrote:
> > --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> > +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> > @@ -163,6 +163,15 @@ Therefore the absence of prior declarations is safe."
> >  -config=MC3R1.R8.4,reports+={safe, "first_area(any_loc(file(gcov)))"}
> >  -doc_end
> >  
> > +-doc_begin="Recognize the occurrence of current_stack_pointer as a 
> > declaration."
> > +-file_tag+={asm_defns, "^xen/arch/x86/include/asm/asm_defns\\.h$"}
> > +-config=MC3R1.R8.4,declarations+={safe, 
> > "loc(file(asm_defns))&&^current_stack_pointer$"}
> > +-doc_end
> > +
> > +-doc_begin="asmlinkage is a marker to indicate that the function is only 
> > used from asm modules."
> > +-config=MC3R1.R8.4,declarations+={safe,"loc(text(^.*asmlinkage.*$, 
> > -1..0))"}
> > +-doc_end
> 
> In the longer run asmlinkage will want using for functions used either way
> between C and assembly (i.e. C->asm calls as well as asm->C ones). I'd like
> to ask that the text please allow for that (e.g. s/from/to interface with/).
> 
> > --- a/xen/arch/x86/hvm/svm/intr.c
> > +++ b/xen/arch/x86/hvm/svm/intr.c
> > @@ -123,7 +123,7 @@ static void svm_enable_intr_window(struct vcpu *v, 
> > struct hvm_intack intack)
> >          vmcb, general1_intercepts | GENERAL1_INTERCEPT_VINTR);
> >  }
> >  
> > -void svm_intr_assist(void)
> > +asmlinkage void svm_intr_assist(void)
> 
> Nit (here and below): Attributes, unless impossible for some specific
> reason, should always go between type and identifier. Not all our code
> is conforming to that, but I think a majority is, and hence you should
> be able to find ample examples (taking e.g. __init).

Hi Jan, in general I agree with this principle but I noticed that this
is not the way Linux uses asmlinkage, a couple of examples:

asmlinkage void do_page_fault(struct pt_regs *regs);
asmlinkage const sys_call_ptr_t sys_call_table[];

Should we go our way or follow Linux on this in terms of code style?