[XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions
As stated in rules.rst, functions used only in asm modules are allowed to have no prior declaration visible when being defined, hence these functions are marked with an 'asmlinkage' macro, which is then deviated for MISRA C:2012 Rule 8.4. 'current_stack_pointer' in x86/asm_defns is a declaration, not a definition, and is thus marked as safe for ECLAIR. Signed-off-by: Nicola Vetrini --- Changes in v3: - added SAF deviations for vmx counterparts to svm functions. Changes in v5: - drop SAF deviations in favour of the pseudo-attribute asmlinkage --- automation/eclair_analysis/ECLAIR/deviations.ecl | 9 + docs/misra/deviations.rst| 6 ++ xen/arch/x86/hvm/svm/intr.c | 2 +- xen/arch/x86/hvm/svm/nestedsvm.c | 2 +- xen/arch/x86/hvm/svm/svm.c | 4 ++-- xen/arch/x86/hvm/vmx/intr.c | 2 +- xen/arch/x86/hvm/vmx/vmx.c | 4 ++-- xen/arch/x86/hvm/vmx/vvmx.c | 2 +- xen/arch/x86/traps.c | 2 +- xen/arch/x86/x86_64/traps.c | 2 +- xen/include/xen/compiler.h | 3 +++ 11 files changed, 28 insertions(+), 10 deletions(-) diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl index fa56e5c00a27..28369174458d 100644 --- 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 + -doc_begin="The following variables are compiled in multiple translation units belonging to different executables and therefore are safe." -config=MC3R1.R8.6,declarations+={safe, "name(current_stack_pointer||bsearch||sort)"} diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst index 8511a189253b..d468da2f5ce9 100644 --- a/docs/misra/deviations.rst +++ b/docs/misra/deviations.rst @@ -133,6 +133,12 @@ Deviations related to MISRA C:2012 Rules: configuration. Therefore, the absence of prior declarations is safe. - Tagged as `safe` for ECLAIR. + * - R8.4 + - Functions and variables used only by asm modules are either marked with + the `asmlinkage` macro or with a SAF-1-safe textual deviation + (see safe.json). + - Tagged as `safe` for ECLAIR. + * - R8.6 - The following variables are compiled in multiple translation units belonging to different executables and therefore are safe. diff --git a/xen/arch/x86/hvm/svm/intr.c b/xen/arch/x86/hvm/svm/intr.c index 192e17ebbfbb..34e5ff886e47 100644 --- 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) { struct vcpu *v = current; struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb; diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c index a09b6abaaeaf..222dfbe28781 100644 --- a/xen/arch/x86/hvm/svm/nestedsvm.c +++ b/xen/arch/x86/hvm/svm/nestedsvm.c @@ -1441,7 +1441,7 @@ nestedsvm_vcpu_vmexit(struct vcpu *v, struct cpu_user_regs *regs, } /* VCPU switch */ -void nsvm_vcpu_switch(void) +asmlinkage void nsvm_vcpu_switch(void) { struct cpu_user_regs *regs = guest_cpu_user_regs(); struct vcpu *v = current; diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c index 24c417ca7199..c1d83fe8af70 100644 --- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -1056,7 +1056,7 @@ static void noreturn cf_check svm_do_resume(void) reset_stack_and_jump(svm_asm_do_resume); } -void svm_vmenter_helper(void) +asmlinkage void svm_vmenter_helper(void) { const struct cpu_user_regs *regs = guest_cpu_user_regs(); struct vcpu *curr = current; @@ -2586,7 +2586,7 @@ const struct hvm_function_table * __init start_svm(void) return &svm_function_table; } -void svm_vmexit_handler(void) +asmlinkage void svm_vmexit_handler(void) { struct cpu_user_regs *regs = guest_cpu_user_regs(); uint64_t exit_reason; diff --git a/xen/arch/x86/hvm/vmx/intr.c b/xen/arch/x86/hvm/vmx/intr.c index fd719c4c01d2..4f2ac1db3e83 100644 --- a/xen/arch/x86/hvm/vmx/intr.c +++ b/xen/arch/x86/hvm/vmx/intr.c @@ -224,7 +224,7 @@ v
Re: [XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions
Hi Nicola, On 30/10/2023 09:11, Nicola Vetrini wrote: As stated in rules.rst, functions used only in asm modules are allowed to have no prior declaration visible when being defined, hence these functions are marked with an 'asmlinkage' macro, which is then deviated for MISRA C:2012 Rule 8.4. AFAIU, this is a replacement of SAF-1. If so, I would like a consistent way to address Rule 8.4. So can you write a patch to replace all the use of SAF-1 with asmlinkage and remove SAF-1? Cheers, -- Julien Grall
Re: [XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions
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
Re: [XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions
On Mon, 30 Oct 2023, Julien Grall wrote: > Hi Nicola, > > On 30/10/2023 09:11, Nicola Vetrini wrote: > > As stated in rules.rst, functions used only in asm modules > > are allowed to have no prior declaration visible when being > > defined, hence these functions are marked with an > > 'asmlinkage' macro, which is then deviated for MISRA C:2012 > > Rule 8.4. > > AFAIU, this is a replacement of SAF-1. If so, I would like a consistent way to > address Rule 8.4. So can you write a patch to replace all the use of SAF-1 > with asmlinkage and remove SAF-1? +1
Re: [XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions
On Mon, 30 Oct 2023, Nicola Vetrini wrote: > As stated in rules.rst, functions used only in asm modules > are allowed to have no prior declaration visible when being > defined, hence these functions are marked with an > 'asmlinkage' macro, which is then deviated for MISRA C:2012 > Rule 8.4. > > 'current_stack_pointer' in x86/asm_defns is a declaration, not a definition, > and is thus marked as safe for ECLAIR. > > Signed-off-by: Nicola Vetrini Just wanted to say that this patch looks OK and I don't have any further coments on top of Jan's
Re: [XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions
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?
Re: [XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions
On 31.10.2023 00:02, Stefano Stabellini wrote: > 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? Linux isn't very consistent in attribute placement anyway, and doing it "randomly" relies on the compiler guys never going to tighten what attributes mean dependent on their placement (iirc gcc doc has text to the effect of this possibly changing at any time). Aiui part of the reason why parsing is more relaxed than it should be is that certain attributes cannot be placed unambiguously at their nominally dedicated place. So my personal view on your question is that we should go our more consistent way. Jan
Re: [XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions
On 2023-10-30 23:54, Stefano Stabellini wrote: On Mon, 30 Oct 2023, Julien Grall wrote: Hi Nicola, On 30/10/2023 09:11, Nicola Vetrini wrote: > As stated in rules.rst, functions used only in asm modules > are allowed to have no prior declaration visible when being > defined, hence these functions are marked with an > 'asmlinkage' macro, which is then deviated for MISRA C:2012 > Rule 8.4. AFAIU, this is a replacement of SAF-1. If so, I would like a consistent way to address Rule 8.4. So can you write a patch to replace all the use of SAF-1 with asmlinkage and remove SAF-1? +1 Ok, no problem -- Nicola Vetrini, BSc Software Engineer, BUGSENG srl (https://bugseng.com)
Re: [XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions
On 2023-10-31 08:50, Jan Beulich wrote: On 31.10.2023 00:02, Stefano Stabellini wrote: 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/). Will do --- 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? Linux isn't very consistent in attribute placement anyway, and doing it "randomly" relies on the compiler guys never going to tighten what attributes mean dependent on their placement (iirc gcc doc has text to the effect of this possibly changing at any time). Aiui part of the reason why parsing is more relaxed than it should be is that certain attributes cannot be placed unambiguously at their nominally dedicated place. So my personal view on your question is that we should go our more consistent way. Jan Ok. -- Nicola Vetrini, BSc Software Engineer, BUGSENG srl (https://bugseng.com)
Re: [XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions
On 2023-10-30 16:12, 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). --- 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 An #ifndef around what, exactly? Anyway, making (part of) this series wait for approval until the other has been accepted into 4.19 (for which I have no specific timeframe) does not seem that desirable to me. -- Nicola Vetrini, BSc Software Engineer, BUGSENG srl (https://bugseng.com)
Re: [XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions
On 31.10.2023 09:22, Nicola Vetrini wrote: > On 2023-10-30 16:12, 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). >> >>> --- 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 ... > > An #ifndef around what, exactly? The #define. That way at least an arch's config.h can override it. > Anyway, making (part of) this series > wait for approval > until the other has been accepted into 4.19 (for which I have no > specific timeframe) > does not seem that desirable to me. It's not ideal, but you or anyone else are free to help that other work make progress. Jan
Re: [XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions
On Tue, 31 Oct 2023, Jan Beulich wrote: > On 31.10.2023 09:22, Nicola Vetrini wrote: > > On 2023-10-30 16:12, 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). > >> > >>> --- 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 ... > > > > An #ifndef around what, exactly? > > The #define. That way at least an arch's config.h can override it. I think the #ifdef is the way to go for now to reduce dependencies between series > > Anyway, making (part of) this series > > wait for approval > > until the other has been accepted into 4.19 (for which I have no > > specific timeframe) > > does not seem that desirable to me. > > It's not ideal, but you or anyone else are free to help that other work > make progress.