* [XEN PATCH][for-4.19 v5 1/8] xen: modify or add declarations for variables where needed
2023-10-30 9:11 [XEN PATCH][for-4.19 v5 0/8] Fix or deviate various instances of missing declarations Nicola Vetrini
@ 2023-10-30 9:11 ` Nicola Vetrini
2023-10-30 9:11 ` [XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions Nicola Vetrini
` (6 subsequent siblings)
7 siblings, 0 replies; 20+ messages in thread
From: Nicola Vetrini @ 2023-10-30 9:11 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder,
consulting, jbeulich, andrew.cooper3, roger.pau, Nicola Vetrini,
Julien Grall, Bertrand Marquis, Volodymyr Babchuk, George Dunlap,
Wei Liu
Some variables with external linkage used in C code do not have
a visible declaration where they are defined. Other variables
can be made static, thereby eliminating the need for a declaration.
Doing so also resolves violations of MISRA C:2012 Rule 8.4.
Fix typo s/mcinfo_dumpped/mcinfo_dumped/ while making
the variable static.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
Jan's ack is for the x86 part, but no other concerns have been
raised for the arm files.
Changes in v2:
- make xenpf_lock static on ARM
Changes in v3:
- moved back code from symbols.h to symbols.c
- dropped two declarations, now deviated
Changes in v4:
- revise commit message
---
xen/arch/arm/include/asm/setup.h | 3 +++
xen/arch/arm/include/asm/smp.h | 3 +++
xen/arch/arm/platform_hypercall.c | 2 +-
xen/arch/x86/cpu/mcheck/mce.c | 7 ++++---
xen/arch/x86/irq.c | 2 +-
xen/include/xen/symbols.h | 1 +
6 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/xen/arch/arm/include/asm/setup.h b/xen/arch/arm/include/asm/setup.h
index 98af6f55f5a0..2a2d6114f2eb 100644
--- a/xen/arch/arm/include/asm/setup.h
+++ b/xen/arch/arm/include/asm/setup.h
@@ -184,9 +184,12 @@ int map_range_to_domain(const struct dt_device_node *dev,
extern lpae_t boot_pgtable[XEN_PT_LPAE_ENTRIES];
#ifdef CONFIG_ARM_64
+extern lpae_t boot_first[XEN_PT_LPAE_ENTRIES];
extern lpae_t boot_first_id[XEN_PT_LPAE_ENTRIES];
#endif
+extern lpae_t boot_second[XEN_PT_LPAE_ENTRIES];
extern lpae_t boot_second_id[XEN_PT_LPAE_ENTRIES];
+extern lpae_t boot_third[XEN_PT_LPAE_ENTRIES * XEN_NR_ENTRIES(2)];
extern lpae_t boot_third_id[XEN_PT_LPAE_ENTRIES];
/* Find where Xen will be residing at runtime and return a PT entry */
diff --git a/xen/arch/arm/include/asm/smp.h b/xen/arch/arm/include/asm/smp.h
index 4fabdf5310d8..28bf24a01d95 100644
--- a/xen/arch/arm/include/asm/smp.h
+++ b/xen/arch/arm/include/asm/smp.h
@@ -6,6 +6,9 @@
#include <asm/current.h>
#endif
+extern struct init_info init_data;
+extern unsigned long smp_up_cpu;
+
DECLARE_PER_CPU(cpumask_var_t, cpu_sibling_mask);
DECLARE_PER_CPU(cpumask_var_t, cpu_core_mask);
diff --git a/xen/arch/arm/platform_hypercall.c b/xen/arch/arm/platform_hypercall.c
index 743687a30390..fde4bc3e5809 100644
--- a/xen/arch/arm/platform_hypercall.c
+++ b/xen/arch/arm/platform_hypercall.c
@@ -17,7 +17,7 @@
#include <asm/current.h>
#include <asm/event.h>
-DEFINE_SPINLOCK(xenpf_lock);
+static DEFINE_SPINLOCK(xenpf_lock);
long do_platform_op(XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op)
{
diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index 6141b7eb9cf1..779a458cd88f 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -1682,13 +1682,14 @@ long do_mca(XEN_GUEST_HANDLE_PARAM(xen_mc_t) u_xen_mc)
return ret;
}
-int mcinfo_dumpped;
+static int mcinfo_dumped;
+
static int cf_check x86_mcinfo_dump_panic(mctelem_cookie_t mctc)
{
struct mc_info *mcip = mctelem_dataptr(mctc);
x86_mcinfo_dump(mcip);
- mcinfo_dumpped++;
+ mcinfo_dumped++;
return 0;
}
@@ -1702,7 +1703,7 @@ static void mc_panic_dump(void)
for_each_online_cpu(cpu)
mctelem_process_deferred(cpu, x86_mcinfo_dump_panic,
mctelem_has_deferred_lmce(cpu));
- dprintk(XENLOG_ERR, "End dump mc_info, %x mcinfo dumped\n", mcinfo_dumpped);
+ dprintk(XENLOG_ERR, "End dump mc_info, %x mcinfo dumped\n", mcinfo_dumped);
}
void mc_panic(const char *s)
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index f42ad539dcd5..ecd67f7f8416 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -43,7 +43,7 @@ int __read_mostly opt_irq_vector_map = OPT_IRQ_VECTOR_MAP_DEFAULT;
static unsigned char __read_mostly irq_max_guests;
integer_param("irq-max-guests", irq_max_guests);
-vmask_t global_used_vector_map;
+static vmask_t global_used_vector_map;
struct irq_desc __read_mostly *irq_desc = NULL;
diff --git a/xen/include/xen/symbols.h b/xen/include/xen/symbols.h
index 20bbb28ef226..1b2863663aa0 100644
--- a/xen/include/xen/symbols.h
+++ b/xen/include/xen/symbols.h
@@ -33,4 +33,5 @@ struct symbol_offset {
uint32_t stream; /* .. in the compressed stream.*/
uint32_t addr; /* .. and in the fixed size address array. */
};
+
#endif /*_XEN_SYMBOLS_H*/
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions
2023-10-30 9:11 [XEN PATCH][for-4.19 v5 0/8] Fix or deviate various instances of missing declarations Nicola Vetrini
2023-10-30 9:11 ` [XEN PATCH][for-4.19 v5 1/8] xen: modify or add declarations for variables where needed Nicola Vetrini
@ 2023-10-30 9:11 ` Nicola Vetrini
2023-10-30 11:29 ` Julien Grall
` (2 more replies)
2023-10-30 9:11 ` [XEN PATCH][for-4.19 v5 3/8] x86: add asmlinkage macro to variables only used in asm code Nicola Vetrini
` (5 subsequent siblings)
7 siblings, 3 replies; 20+ messages in thread
From: Nicola Vetrini @ 2023-10-30 9:11 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder,
consulting, jbeulich, andrew.cooper3, roger.pau, Nicola Vetrini,
Simone Ballarin, Doug Goldstein, George Dunlap, Julien Grall,
Wei Liu, Jun Nakajima, Kevin Tian
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 <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
---
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 @@ void vmx_sync_exit_bitmap(struct vcpu *v)
}
}
-void vmx_intr_assist(void)
+asmlinkage void 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..af7e4e997c75 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)
+asmlinkage void 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)
+asmlinkage bool 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..f234057b88ae 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)
+asmlinkage void 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..b516da7b80f0 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)
+asmlinkage void 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..ab5959daa887 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)
+asmlinkage void 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..39d696176f3d 100644
--- 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
+
/*
* 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
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions
2023-10-30 9:11 ` [XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions Nicola Vetrini
@ 2023-10-30 11:29 ` Julien Grall
2023-10-30 22:54 ` Stefano Stabellini
2023-10-30 15:12 ` Jan Beulich
2023-10-30 22:55 ` Stefano Stabellini
2 siblings, 1 reply; 20+ messages in thread
From: Julien Grall @ 2023-10-30 11:29 UTC (permalink / raw)
To: Nicola Vetrini, xen-devel
Cc: sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder,
consulting, jbeulich, andrew.cooper3, roger.pau, Simone Ballarin,
Doug Goldstein, George Dunlap, Wei Liu, Jun Nakajima, Kevin Tian
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions
2023-10-30 11:29 ` Julien Grall
@ 2023-10-30 22:54 ` Stefano Stabellini
2023-10-31 8:15 ` Nicola Vetrini
0 siblings, 1 reply; 20+ messages in thread
From: Stefano Stabellini @ 2023-10-30 22:54 UTC (permalink / raw)
To: Julien Grall
Cc: Nicola Vetrini, xen-devel, sstabellini, michal.orzel,
xenia.ragiadakou, ayan.kumar.halder, consulting, jbeulich,
andrew.cooper3, roger.pau, Simone Ballarin, Doug Goldstein,
George Dunlap, Wei Liu, Jun Nakajima, Kevin Tian
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions
2023-10-30 22:54 ` Stefano Stabellini
@ 2023-10-31 8:15 ` Nicola Vetrini
0 siblings, 0 replies; 20+ messages in thread
From: Nicola Vetrini @ 2023-10-31 8:15 UTC (permalink / raw)
To: Stefano Stabellini
Cc: Julien Grall, xen-devel, michal.orzel, xenia.ragiadakou,
ayan.kumar.halder, consulting, jbeulich, andrew.cooper3,
roger.pau, Simone Ballarin, Doug Goldstein, George Dunlap,
Wei Liu, Jun Nakajima, Kevin Tian
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)
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions
2023-10-30 9:11 ` [XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions Nicola Vetrini
2023-10-30 11:29 ` Julien Grall
@ 2023-10-30 15:12 ` Jan Beulich
2023-10-30 23:02 ` Stefano Stabellini
2023-10-31 8:22 ` Nicola Vetrini
2023-10-30 22:55 ` Stefano Stabellini
2 siblings, 2 replies; 20+ messages in thread
From: Jan Beulich @ 2023-10-30 15:12 UTC (permalink / raw)
To: Nicola Vetrini
Cc: sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder,
consulting, andrew.cooper3, roger.pau, Simone Ballarin,
Doug Goldstein, George Dunlap, Julien Grall, Wei Liu,
Jun Nakajima, Kevin Tian, xen-devel
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
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions
2023-10-30 15:12 ` Jan Beulich
@ 2023-10-30 23:02 ` Stefano Stabellini
2023-10-31 7:50 ` Jan Beulich
2023-10-31 8:22 ` Nicola Vetrini
1 sibling, 1 reply; 20+ messages in thread
From: Stefano Stabellini @ 2023-10-30 23:02 UTC (permalink / raw)
To: Jan Beulich
Cc: Nicola Vetrini, sstabellini, michal.orzel, xenia.ragiadakou,
ayan.kumar.halder, consulting, andrew.cooper3, roger.pau,
Simone Ballarin, Doug Goldstein, George Dunlap, Julien Grall,
Wei Liu, Jun Nakajima, Kevin Tian, xen-devel
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?
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions
2023-10-30 23:02 ` Stefano Stabellini
@ 2023-10-31 7:50 ` Jan Beulich
2023-10-31 8:18 ` Nicola Vetrini
0 siblings, 1 reply; 20+ messages in thread
From: Jan Beulich @ 2023-10-31 7:50 UTC (permalink / raw)
To: Stefano Stabellini
Cc: Nicola Vetrini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder,
consulting, andrew.cooper3, roger.pau, Simone Ballarin,
Doug Goldstein, George Dunlap, Julien Grall, Wei Liu,
Jun Nakajima, Kevin Tian, xen-devel
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
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions
2023-10-31 7:50 ` Jan Beulich
@ 2023-10-31 8:18 ` Nicola Vetrini
0 siblings, 0 replies; 20+ messages in thread
From: Nicola Vetrini @ 2023-10-31 8:18 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, michal.orzel, xenia.ragiadakou,
ayan.kumar.halder, consulting, andrew.cooper3, roger.pau,
Simone Ballarin, Doug Goldstein, George Dunlap, Julien Grall,
Wei Liu, Jun Nakajima, Kevin Tian, xen-devel
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)
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions
2023-10-30 15:12 ` Jan Beulich
2023-10-30 23:02 ` Stefano Stabellini
@ 2023-10-31 8:22 ` Nicola Vetrini
2023-10-31 8:26 ` Jan Beulich
1 sibling, 1 reply; 20+ messages in thread
From: Nicola Vetrini @ 2023-10-31 8:22 UTC (permalink / raw)
To: Jan Beulich
Cc: sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder,
consulting, andrew.cooper3, roger.pau, Simone Ballarin,
Doug Goldstein, George Dunlap, Julien Grall, Wei Liu,
Jun Nakajima, Kevin Tian, xen-devel
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)
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions
2023-10-31 8:22 ` Nicola Vetrini
@ 2023-10-31 8:26 ` Jan Beulich
2023-10-31 22:56 ` Stefano Stabellini
0 siblings, 1 reply; 20+ messages in thread
From: Jan Beulich @ 2023-10-31 8:26 UTC (permalink / raw)
To: Nicola Vetrini
Cc: sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder,
consulting, andrew.cooper3, roger.pau, Simone Ballarin,
Doug Goldstein, George Dunlap, Julien Grall, Wei Liu,
Jun Nakajima, Kevin Tian, xen-devel
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
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions
2023-10-31 8:26 ` Jan Beulich
@ 2023-10-31 22:56 ` Stefano Stabellini
0 siblings, 0 replies; 20+ messages in thread
From: Stefano Stabellini @ 2023-10-31 22:56 UTC (permalink / raw)
To: Jan Beulich
Cc: Nicola Vetrini, sstabellini, michal.orzel, xenia.ragiadakou,
ayan.kumar.halder, consulting, andrew.cooper3, roger.pau,
Simone Ballarin, Doug Goldstein, George Dunlap, Julien Grall,
Wei Liu, Jun Nakajima, Kevin Tian, xen-devel
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.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions
2023-10-30 9:11 ` [XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions Nicola Vetrini
2023-10-30 11:29 ` Julien Grall
2023-10-30 15:12 ` Jan Beulich
@ 2023-10-30 22:55 ` Stefano Stabellini
2 siblings, 0 replies; 20+ messages in thread
From: Stefano Stabellini @ 2023-10-30 22:55 UTC (permalink / raw)
To: Nicola Vetrini
Cc: xen-devel, sstabellini, michal.orzel, xenia.ragiadakou,
ayan.kumar.halder, consulting, jbeulich, andrew.cooper3,
roger.pau, Simone Ballarin, Doug Goldstein, George Dunlap,
Julien Grall, Wei Liu, Jun Nakajima, Kevin Tian
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 <nicola.vetrini@bugseng.com>
Just wanted to say that this patch looks OK and I don't have any further
coments on top of Jan's
^ permalink raw reply [flat|nested] 20+ messages in thread
* [XEN PATCH][for-4.19 v5 3/8] x86: add asmlinkage macro to variables only used in asm code
2023-10-30 9:11 [XEN PATCH][for-4.19 v5 0/8] Fix or deviate various instances of missing declarations Nicola Vetrini
2023-10-30 9:11 ` [XEN PATCH][for-4.19 v5 1/8] xen: modify or add declarations for variables where needed Nicola Vetrini
2023-10-30 9:11 ` [XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions Nicola Vetrini
@ 2023-10-30 9:11 ` Nicola Vetrini
2023-10-30 9:11 ` [XEN PATCH][for-4.19 v5 4/8] x86/grant: switch included header to make declarations visible Nicola Vetrini
` (4 subsequent siblings)
7 siblings, 0 replies; 20+ messages in thread
From: Nicola Vetrini @ 2023-10-30 9:11 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder,
consulting, jbeulich, andrew.cooper3, roger.pau, Nicola Vetrini,
Wei Liu
To avoid a violation of MISRA C:2012 Rule 8.4, as permitted
by docs/misra/rules.rst.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
Changes in v3:
- Edited commit message
- Add two new variables
Changes in v5:
- Mark current_stack_pointer as a declaration.
- Use asmlinkage instead of SAF.
---
xen/arch/x86/setup.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index a3d3f797bb1e..272fd6f2ad3c 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -75,7 +75,8 @@ static bool __initdata opt_invpcid = true;
boolean_param("invpcid", opt_invpcid);
bool __read_mostly use_invpcid;
-unsigned long __read_mostly cr4_pv32_mask;
+/* Only used in asm code and within this source file */
+asmlinkage unsigned long __read_mostly cr4_pv32_mask;
/* **** Linux config option: propagated to domain0. */
/* "acpi=off": Sisables both ACPI table parsing and interpreter. */
@@ -146,14 +147,15 @@ cpumask_t __read_mostly cpu_present_map;
unsigned long __read_mostly xen_phys_start;
-char __section(".init.bss.stack_aligned") __aligned(STACK_SIZE)
+/* Only used in asm code and within this source file */
+asmlinkage char __section(".init.bss.stack_aligned") __aligned(STACK_SIZE)
cpu0_stack[STACK_SIZE];
/* Used by the BSP/AP paths to find the higher half stack mapping to use. */
void *stack_start = cpu0_stack + STACK_SIZE - sizeof(struct cpu_info);
/* Used by the boot asm to stash the relocated multiboot info pointer. */
-unsigned int __initdata multiboot_ptr;
+asmlinkage unsigned int __initdata multiboot_ptr;
struct cpuinfo_x86 __read_mostly boot_cpu_data = { 0, 0, 0, 0, -1 };
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [XEN PATCH][for-4.19 v5 4/8] x86/grant: switch included header to make declarations visible
2023-10-30 9:11 [XEN PATCH][for-4.19 v5 0/8] Fix or deviate various instances of missing declarations Nicola Vetrini
` (2 preceding siblings ...)
2023-10-30 9:11 ` [XEN PATCH][for-4.19 v5 3/8] x86: add asmlinkage macro to variables only used in asm code Nicola Vetrini
@ 2023-10-30 9:11 ` Nicola Vetrini
2023-10-30 9:11 ` [XEN PATCH][for-4.19 v5 5/8] x86/vm_event: add missing include for hvm_vm_event_do_resume Nicola Vetrini
` (3 subsequent siblings)
7 siblings, 0 replies; 20+ messages in thread
From: Nicola Vetrini @ 2023-10-30 9:11 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder,
consulting, jbeulich, andrew.cooper3, roger.pau, Nicola Vetrini,
Wei Liu
The declarations for {create,replace}_grant_p2m_mapping are
not visible when these functions are defined, therefore the right
header needs to be included to allow them to be visible.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
Changes in v3:
- asm/paging.h can be replaced with mm-frame.h, because just the
definition of mfn_t is needed.
---
xen/arch/x86/hvm/grant_table.c | 3 +--
xen/arch/x86/include/asm/hvm/grant_table.h | 2 ++
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/hvm/grant_table.c b/xen/arch/x86/hvm/grant_table.c
index 30d51d54a949..afe449d8882c 100644
--- a/xen/arch/x86/hvm/grant_table.c
+++ b/xen/arch/x86/hvm/grant_table.c
@@ -9,8 +9,7 @@
#include <xen/types.h>
-#include <public/grant_table.h>
-
+#include <asm/hvm/grant_table.h>
#include <asm/p2m.h>
int create_grant_p2m_mapping(uint64_t addr, mfn_t frame,
diff --git a/xen/arch/x86/include/asm/hvm/grant_table.h b/xen/arch/x86/include/asm/hvm/grant_table.h
index 33c1da1a25f3..01e23f79b8cf 100644
--- a/xen/arch/x86/include/asm/hvm/grant_table.h
+++ b/xen/arch/x86/include/asm/hvm/grant_table.h
@@ -10,6 +10,8 @@
#ifndef __X86_HVM_GRANT_TABLE_H__
#define __X86_HVM_GRANT_TABLE_H__
+#include <xen/mm-frame.h>
+
#ifdef CONFIG_HVM
int create_grant_p2m_mapping(uint64_t addr, mfn_t frame,
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [XEN PATCH][for-4.19 v5 5/8] x86/vm_event: add missing include for hvm_vm_event_do_resume
2023-10-30 9:11 [XEN PATCH][for-4.19 v5 0/8] Fix or deviate various instances of missing declarations Nicola Vetrini
` (3 preceding siblings ...)
2023-10-30 9:11 ` [XEN PATCH][for-4.19 v5 4/8] x86/grant: switch included header to make declarations visible Nicola Vetrini
@ 2023-10-30 9:11 ` Nicola Vetrini
2023-10-30 9:11 ` [XEN PATCH][for-4.19 v5 6/8] xen/console: remove stub definition in consoled.h Nicola Vetrini
` (2 subsequent siblings)
7 siblings, 0 replies; 20+ messages in thread
From: Nicola Vetrini @ 2023-10-30 9:11 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder,
consulting, jbeulich, andrew.cooper3, roger.pau, Nicola Vetrini,
Tamas K Lengyel, Alexandru Isaila, Petre Pircalabu, Wei Liu
The missing header makes the declaration visible when the function
is defined, thereby fixing a violation of MISRA C:2012 Rule 8.4.
Fixes: 1366a0e76db6 ("x86/vm_event: add hvm/vm_event.{h,c}")
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
---
xen/arch/x86/hvm/vm_event.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/xen/arch/x86/hvm/vm_event.c b/xen/arch/x86/hvm/vm_event.c
index 3b064bcfade5..c1af230e7aed 100644
--- a/xen/arch/x86/hvm/vm_event.c
+++ b/xen/arch/x86/hvm/vm_event.c
@@ -24,6 +24,7 @@
#include <xen/vm_event.h>
#include <asm/hvm/emulate.h>
#include <asm/hvm/support.h>
+#include <asm/hvm/vm_event.h>
#include <asm/vm_event.h>
static void hvm_vm_event_set_registers(const struct vcpu *v)
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [XEN PATCH][for-4.19 v5 6/8] xen/console: remove stub definition in consoled.h
2023-10-30 9:11 [XEN PATCH][for-4.19 v5 0/8] Fix or deviate various instances of missing declarations Nicola Vetrini
` (4 preceding siblings ...)
2023-10-30 9:11 ` [XEN PATCH][for-4.19 v5 5/8] x86/vm_event: add missing include for hvm_vm_event_do_resume Nicola Vetrini
@ 2023-10-30 9:11 ` Nicola Vetrini
2023-10-30 9:11 ` [XEN PATCH][for-4.19 v5 7/8] x86/mem_access: make function static Nicola Vetrini
2023-10-30 9:11 ` [XEN PATCH][for-4.19 v5 8/8] docs/misra: exclude three more files Nicola Vetrini
7 siblings, 0 replies; 20+ messages in thread
From: Nicola Vetrini @ 2023-10-30 9:11 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder,
consulting, jbeulich, andrew.cooper3, roger.pau, Nicola Vetrini,
George Dunlap, Julien Grall, Wei Liu
The stub definition of 'consoled_guest_tx' can be removed, since its
its single caller uses the implementation built with PV_SHIM enabled.
Fixes: 5ef49f185c2d ("x86/pv-shim: shadow PV console's page for L2 DomU")
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
xen/include/xen/consoled.h | 7 -------
1 file changed, 7 deletions(-)
diff --git a/xen/include/xen/consoled.h b/xen/include/xen/consoled.h
index fd5d220a8aca..2b30516b3a0a 100644
--- a/xen/include/xen/consoled.h
+++ b/xen/include/xen/consoled.h
@@ -3,18 +3,11 @@
#include <public/io/console.h>
-#ifdef CONFIG_PV_SHIM
-
void consoled_set_ring_addr(struct xencons_interface *ring);
struct xencons_interface *consoled_get_ring_addr(void);
size_t consoled_guest_rx(void);
size_t consoled_guest_tx(char c);
-#else
-
-size_t consoled_guest_tx(char c) { return 0; }
-
-#endif /* !CONFIG_PV_SHIM */
#endif /* __XEN_CONSOLED_H__ */
/*
* Local variables:
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [XEN PATCH][for-4.19 v5 7/8] x86/mem_access: make function static
2023-10-30 9:11 [XEN PATCH][for-4.19 v5 0/8] Fix or deviate various instances of missing declarations Nicola Vetrini
` (5 preceding siblings ...)
2023-10-30 9:11 ` [XEN PATCH][for-4.19 v5 6/8] xen/console: remove stub definition in consoled.h Nicola Vetrini
@ 2023-10-30 9:11 ` Nicola Vetrini
2023-10-30 9:11 ` [XEN PATCH][for-4.19 v5 8/8] docs/misra: exclude three more files Nicola Vetrini
7 siblings, 0 replies; 20+ messages in thread
From: Nicola Vetrini @ 2023-10-30 9:11 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder,
consulting, jbeulich, andrew.cooper3, roger.pau, Nicola Vetrini,
Tamas K Lengyel, Alexandru Isaila, Petre Pircalabu, George Dunlap,
Wei Liu
The function is used only within this file, and therefore can be static.
No functional change.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
---
Changes in v3:
- style fix
---
xen/arch/x86/mm/mem_access.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c
index 3449e0ee85ff..60a0cce68aa3 100644
--- a/xen/arch/x86/mm/mem_access.c
+++ b/xen/arch/x86/mm/mem_access.c
@@ -249,9 +249,9 @@ bool p2m_mem_access_check(paddr_t gpa, unsigned long gla,
return (p2ma != p2m_access_n2rwx);
}
-int p2m_set_altp2m_mem_access(struct domain *d, struct p2m_domain *hp2m,
- struct p2m_domain *ap2m, p2m_access_t a,
- gfn_t gfn)
+static int p2m_set_altp2m_mem_access(struct domain *d, struct p2m_domain *hp2m,
+ struct p2m_domain *ap2m, p2m_access_t a,
+ gfn_t gfn)
{
mfn_t mfn;
p2m_type_t t;
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [XEN PATCH][for-4.19 v5 8/8] docs/misra: exclude three more files
2023-10-30 9:11 [XEN PATCH][for-4.19 v5 0/8] Fix or deviate various instances of missing declarations Nicola Vetrini
` (6 preceding siblings ...)
2023-10-30 9:11 ` [XEN PATCH][for-4.19 v5 7/8] x86/mem_access: make function static Nicola Vetrini
@ 2023-10-30 9:11 ` Nicola Vetrini
7 siblings, 0 replies; 20+ messages in thread
From: Nicola Vetrini @ 2023-10-30 9:11 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder,
consulting, jbeulich, andrew.cooper3, roger.pau, Nicola Vetrini,
George Dunlap, Julien Grall, Wei Liu
These files should not conform to MISRA guidelines at the moment,
therefore they are added to the exclusion list.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
These exclusions are automatically picked up by ECLAIR's automation
to hide reports originating from these files.
Changes in v4:
- Fixed typo
---
docs/misra/exclude-list.json | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/docs/misra/exclude-list.json b/docs/misra/exclude-list.json
index 575ed22a7f67..b858a0baa106 100644
--- a/docs/misra/exclude-list.json
+++ b/docs/misra/exclude-list.json
@@ -145,6 +145,10 @@
"rel_path": "common/zstd/*",
"comment": "Imported from Linux, ignore for now"
},
+ {
+ "rel_path": "common/symbols-dummy.c",
+ "comment": "The resulting code is not included in the final Xen binary, ignore for now"
+ },
{
"rel_path": "crypto/*",
"comment": "Origin is external and documented in crypto/README.source"
@@ -189,6 +193,14 @@
"rel_path": "include/acpi/acpixf.h",
"comment": "Imported from Linux, ignore for now"
},
+ {
+ "rel_path": "include/acpi/acexcep.h",
+ "comment": "Imported from Linux, ignore for now"
+ },
+ {
+ "rel_path": "include/acpi/acglobal.h",
+ "comment": "Imported from Linux, ignore for now"
+ },
{
"rel_path": "include/xen/acpi.h",
"comment": "Imported from Linux, ignore for now"
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread