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

[Jan Beulich <jbeulich@xxxxxxxx>]

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).

> --- a/xen/include/xen/compiler.h
> +++ b/xen/include/xen/compiler.h
> @@ -159,6 +159,9 @@
>  # define ASM_FLAG_OUT(yes, no) no
>  #endif
>  
> +/* Mark a function or variable as used only from asm */
> +#define asmlinkage

I appreciate this being an immediately "natural" place, but considering
what we know from Linux I think we ought to allow for arch overrides here
right away. For that I'm afraid compiler.h isn't best; it may still be
okay as long as at least an #ifndef is put around it. Imo, however, this
ought to go into xen/linkage.h, as is being introduced by "common:
assembly entry point type/size annotations". It's somewhat a shame that
this and the rest of that series has missed 4.18 ...

Jan