| Message ID | a1b5c3b273145c35535fed3647bf850d9ae5db7f.1698829473.git.nicola.vetrini@bugseng.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix or deviate various instances of missing declarations | expand |
On Wed, 1 Nov 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. > > Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> > --- > 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 > Changes in v6: > - conditioned asmlinkage definition to make it overridable; > - move the pseudo-attribute after the return type > --- > 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 | 5 +++++ > 11 files changed, 30 insertions(+), 10 deletions(-) > > diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl > index fa56e5c00a27..06619ecf7e8d 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 to interface with asm modules." > +-config=MC3R1.R8.4,declarations+={safe,"loc(text(^(?s).*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. Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> If Julien prefers a different wording I could modify on commit as needed > * - 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..4805c5567213 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) > +void asmlinkage 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..fc7658d67d4e 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) > +void asmlinkage 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..cb8abe7a0066 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) > +void asmlinkage 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) > +void asmlinkage 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..8beeaab1517b 100644 > --- a/xen/arch/x86/hvm/vmx/intr.c > +++ b/xen/arch/x86/hvm/vmx/intr.c > @@ -224,7 +224,7 @@ void vmx_sync_exit_bitmap(struct vcpu *v) > } > } > > -void vmx_intr_assist(void) > +void asmlinkage vmx_intr_assist(void) > { > struct hvm_intack intack; > struct vcpu *v = current; > diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c > index 1edc7f1e919f..f5739b3aa5a1 100644 > --- a/xen/arch/x86/hvm/vmx/vmx.c > +++ b/xen/arch/x86/hvm/vmx/vmx.c > @@ -4035,7 +4035,7 @@ static void undo_nmis_unblocked_by_iret(void) > guest_info | VMX_INTR_SHADOW_NMI); > } > > -void vmx_vmexit_handler(struct cpu_user_regs *regs) > +void asmlinkage vmx_vmexit_handler(struct cpu_user_regs *regs) > { > unsigned long exit_qualification, exit_reason, idtv_info, intr_info = 0; > unsigned int vector = 0; > @@ -4787,7 +4787,7 @@ static void lbr_fixup(void) > } > > /* Returns false if the vmentry has to be restarted */ > -bool vmx_vmenter_helper(const struct cpu_user_regs *regs) > +bool asmlinkage vmx_vmenter_helper(const struct cpu_user_regs *regs) > { > struct vcpu *curr = current; > struct domain *currd = curr->domain; > diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c > index 16b0ef82b6c8..a28803987af6 100644 > --- a/xen/arch/x86/hvm/vmx/vvmx.c > +++ b/xen/arch/x86/hvm/vmx/vvmx.c > @@ -1490,7 +1490,7 @@ static void nvmx_eptp_update(void) > __vmwrite(EPT_POINTER, get_shadow_eptp(curr)); > } > > -void nvmx_switch_guest(void) > +void asmlinkage nvmx_switch_guest(void) > { > struct vcpu *v = current; > struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v); > diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c > index e1356f696aba..6393467b06fd 100644 > --- a/xen/arch/x86/traps.c > +++ b/xen/arch/x86/traps.c > @@ -2265,7 +2265,7 @@ void asm_domain_crash_synchronous(unsigned long addr) > } > > #ifdef CONFIG_DEBUG > -void check_ist_exit(const struct cpu_user_regs *regs, bool ist_exit) > +void asmlinkage check_ist_exit(const struct cpu_user_regs *regs, bool ist_exit) > { > const unsigned int ist_mask = > (1U << X86_EXC_NMI) | (1U << X86_EXC_DB) | > diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c > index e03e80813e36..668605e5bc67 100644 > --- a/xen/arch/x86/x86_64/traps.c > +++ b/xen/arch/x86/x86_64/traps.c > @@ -266,7 +266,7 @@ void show_page_walk(unsigned long addr) > l1_table_offset(addr), l1e_get_intpte(l1e), pfn); > } > > -void do_double_fault(struct cpu_user_regs *regs) > +void asmlinkage do_double_fault(struct cpu_user_regs *regs) > { > unsigned int cpu; > unsigned long crs[8]; > diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h > index dd99e573083f..94e2d6080d93 100644 > --- a/xen/include/xen/compiler.h > +++ b/xen/include/xen/compiler.h > @@ -159,6 +159,11 @@ > # define ASM_FLAG_OUT(yes, no) no > #endif > > +/* Mark a function or variable as being used only to interface with asm */ > +#ifndef asmlinkage > +#define asmlinkage > +#endif > + > /* > * NB: we need to disable the gcc-compat warnings for clang in some places or > * else it will complain with: "'break' is bound to loop, GCC binds it to > -- > 2.34.1 >
Hi Stefano, On 01/11/2023 23:10, Stefano Stabellini wrote: > On Wed, 1 Nov 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. >> >> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> >> --- >> 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 >> Changes in v6: >> - conditioned asmlinkage definition to make it overridable; >> - move the pseudo-attribute after the return type >> --- >> 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 | 5 +++++ >> 11 files changed, 30 insertions(+), 10 deletions(-) >> >> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl >> index fa56e5c00a27..06619ecf7e8d 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 to interface with asm modules." >> +-config=MC3R1.R8.4,declarations+={safe,"loc(text(^(?s).*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. > > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > > If Julien prefers a different wording I could modify on commit as needed I think my comment on the previous version was misunderstood. I have asked to replace all SAF-1 with asmlinkage and I thought you agreed with that. This is a similar situation to octal-ok. I don't think it is right to have multiple ways to tag a deviation. I don't particular want to see more added, and I would like the current ones to be gone. So what's the plan to remove SAF-1-safe? At minimum, the deviation should be extremely clear that there **must** be no new use of SAF-1-safe. Something like: "- Functions and variables used only by asm modules are either marked with the `asmlinkage` macro. Existing code may use SAF-1-safe textual deviation (see safe.json) but new code should not use it. " Obviously this is not my preference. Cheers,
Hi Julien, On 2023-11-02 00:42, Julien Grall wrote: > Hi Stefano, > > On 01/11/2023 23:10, Stefano Stabellini wrote: >> On Wed, 1 Nov 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. >>> >>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> >>> --- >>> 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 >>> Changes in v6: >>> - conditioned asmlinkage definition to make it overridable; >>> - move the pseudo-attribute after the return type >>> --- >>> 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 | 5 +++++ >>> 11 files changed, 30 insertions(+), 10 deletions(-) >>> >>> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl >>> b/automation/eclair_analysis/ECLAIR/deviations.ecl >>> index fa56e5c00a27..06619ecf7e8d 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 to interface with asm modules." >>> +-config=MC3R1.R8.4,declarations+={safe,"loc(text(^(?s).*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. >> >> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> >> >> If Julien prefers a different wording I could modify on commit as >> needed > > I think my comment on the previous version was misunderstood. I have > asked to replace all SAF-1 with asmlinkage and I thought you agreed > with that. > > This is a similar situation to octal-ok. I don't think it is right to > have multiple ways to tag a deviation. > > I don't particular want to see more added, and I would like the current > ones to be gone. So what's the plan to remove SAF-1-safe? > > At minimum, the deviation should be extremely clear that there **must** > be no new use of SAF-1-safe. Something like: > > "- Functions and variables used only by asm modules are either marked > with the `asmlinkage` macro. Existing code may use SAF-1-safe textual > deviation (see safe.json) but new code should not use it. > " > > Obviously this is not my preference. > > Cheers, I was thinking about doing a separate patch to replace the remaining SAF comments and change the deviation message (not yet submitted, but it's essentially ready), so that the commits would look more coherent. Is that ok for you? Thanks,
On 02.11.2023 00:10, Stefano Stabellini wrote: > On Wed, 1 Nov 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. >> >> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> >> --- >> 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 >> Changes in v6: >> - conditioned asmlinkage definition to make it overridable; >> - move the pseudo-attribute after the return type >> --- >> 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 | 5 +++++ >> 11 files changed, 30 insertions(+), 10 deletions(-) >> >> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl >> index fa56e5c00a27..06619ecf7e8d 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 to interface with asm modules." >> +-config=MC3R1.R8.4,declarations+={safe,"loc(text(^(?s).*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. > > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> Acked-by: Jan Beulich <jbeulich@suse.com> with ... > If Julien prefers a different wording I could modify on commit as needed ... wording clarified along the lines of his subsequent reply. Yet better would be, as also suggested by him, to accompany this with a 2nd patch replacing the now deprecated SAF-* comments. Jan
On 2023-11-02 14:27, Jan Beulich wrote: > On 02.11.2023 00:10, Stefano Stabellini wrote: >> On Wed, 1 Nov 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. >>> >>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> >>> --- >>> 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 >>> Changes in v6: >>> - conditioned asmlinkage definition to make it overridable; >>> - move the pseudo-attribute after the return type >>> --- >>> 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 | 5 +++++ >>> 11 files changed, 30 insertions(+), 10 deletions(-) >>> >>> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl >>> b/automation/eclair_analysis/ECLAIR/deviations.ecl >>> index fa56e5c00a27..06619ecf7e8d 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 to interface with asm modules." >>> +-config=MC3R1.R8.4,declarations+={safe,"loc(text(^(?s).*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. >> >> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > > Acked-by: Jan Beulich <jbeulich@suse.com> > with ... > >> If Julien prefers a different wording I could modify on commit as >> needed > > ... wording clarified along the lines of his subsequent reply. Yet > better > would be, as also suggested by him, to accompany this with a 2nd patch > replacing the now deprecated SAF-* comments. > > Jan Yes, as replied to Julien in this thread: a second patch will be submitted shortly.
diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl index fa56e5c00a27..06619ecf7e8d 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 to interface with asm modules." +-config=MC3R1.R8.4,declarations+={safe,"loc(text(^(?s).*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..4805c5567213 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) +void asmlinkage 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..fc7658d67d4e 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) +void asmlinkage 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..cb8abe7a0066 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) +void asmlinkage 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) +void asmlinkage 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..8beeaab1517b 100644 --- a/xen/arch/x86/hvm/vmx/intr.c +++ b/xen/arch/x86/hvm/vmx/intr.c @@ -224,7 +224,7 @@ void vmx_sync_exit_bitmap(struct vcpu *v) } } -void vmx_intr_assist(void) +void asmlinkage vmx_intr_assist(void) { struct hvm_intack intack; struct vcpu *v = current; diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index 1edc7f1e919f..f5739b3aa5a1 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -4035,7 +4035,7 @@ static void undo_nmis_unblocked_by_iret(void) guest_info | VMX_INTR_SHADOW_NMI); } -void vmx_vmexit_handler(struct cpu_user_regs *regs) +void asmlinkage vmx_vmexit_handler(struct cpu_user_regs *regs) { unsigned long exit_qualification, exit_reason, idtv_info, intr_info = 0; unsigned int vector = 0; @@ -4787,7 +4787,7 @@ static void lbr_fixup(void) } /* Returns false if the vmentry has to be restarted */ -bool vmx_vmenter_helper(const struct cpu_user_regs *regs) +bool asmlinkage vmx_vmenter_helper(const struct cpu_user_regs *regs) { struct vcpu *curr = current; struct domain *currd = curr->domain; diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c index 16b0ef82b6c8..a28803987af6 100644 --- a/xen/arch/x86/hvm/vmx/vvmx.c +++ b/xen/arch/x86/hvm/vmx/vvmx.c @@ -1490,7 +1490,7 @@ static void nvmx_eptp_update(void) __vmwrite(EPT_POINTER, get_shadow_eptp(curr)); } -void nvmx_switch_guest(void) +void asmlinkage nvmx_switch_guest(void) { struct vcpu *v = current; struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v); diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c index e1356f696aba..6393467b06fd 100644 --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -2265,7 +2265,7 @@ void asm_domain_crash_synchronous(unsigned long addr) } #ifdef CONFIG_DEBUG -void check_ist_exit(const struct cpu_user_regs *regs, bool ist_exit) +void asmlinkage check_ist_exit(const struct cpu_user_regs *regs, bool ist_exit) { const unsigned int ist_mask = (1U << X86_EXC_NMI) | (1U << X86_EXC_DB) | diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c index e03e80813e36..668605e5bc67 100644 --- a/xen/arch/x86/x86_64/traps.c +++ b/xen/arch/x86/x86_64/traps.c @@ -266,7 +266,7 @@ void show_page_walk(unsigned long addr) l1_table_offset(addr), l1e_get_intpte(l1e), pfn); } -void do_double_fault(struct cpu_user_regs *regs) +void asmlinkage do_double_fault(struct cpu_user_regs *regs) { unsigned int cpu; unsigned long crs[8]; diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h index dd99e573083f..94e2d6080d93 100644 --- a/xen/include/xen/compiler.h +++ b/xen/include/xen/compiler.h @@ -159,6 +159,11 @@ # define ASM_FLAG_OUT(yes, no) no #endif +/* Mark a function or variable as being used only to interface with asm */ +#ifndef asmlinkage +#define asmlinkage +#endif + /* * NB: we need to disable the gcc-compat warnings for clang in some places or * else it will complain with: "'break' is bound to loop, GCC binds it to
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. Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> --- 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 Changes in v6: - conditioned asmlinkage definition to make it overridable; - move the pseudo-attribute after the return type --- 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 | 5 +++++ 11 files changed, 30 insertions(+), 10 deletions(-)