[XEN PATCH 0/7] Fix or deviate various instances of missing declarations

[XEN PATCH 0/7] Fix or deviate various instances of missing declarations

[Nicola Vetrini]

The patches in this series aim to fix or deviate various instances where a
function or variable do not have a declaration visible when such entity is
defined (in violation of MISRA C:2012 Rule 8.4).
An exception listed under docs/misra/rules.rst allows asm-only functions and
variables to be exempted, while the other instances are either changed
(e.g., making them static) or a missing header inclusion is added.

Some of the patches in this series are potential candidates for bug fixes, or
just general improvements that may be suited for inclusion in the next rc.

Nicola Vetrini (7):
  xen: add declarations for variables where needed
  x86: add deviations for variables only used in asm code
  x86: add deviation comments for  asm-only functions
  x86/grant: switch included header to make declarations visible
  x86/vm_event: add missing include for hvm_vm_event_do_resume
  xen/console: make function static inline
  x86/mem_access: make function static

 xen/arch/arm/include/asm/setup.h           |  3 +++
 xen/arch/arm/include/asm/smp.h             |  3 +++
 xen/arch/x86/cpu/mcheck/mce.c              |  6 +++---
 xen/arch/x86/hvm/grant_table.c             |  3 +--
 xen/arch/x86/hvm/svm/intr.c                |  1 +
 xen/arch/x86/hvm/svm/nestedsvm.c           |  1 +
 xen/arch/x86/hvm/svm/svm.c                 |  2 ++
 xen/arch/x86/hvm/vm_event.c                |  1 +
 xen/arch/x86/include/asm/asm_defns.h       |  1 +
 xen/arch/x86/include/asm/hvm/grant_table.h |  2 ++
 xen/arch/x86/include/asm/setup.h           |  3 +++
 xen/arch/x86/irq.c                         |  2 +-
 xen/arch/x86/mm/mem_access.c               |  2 +-
 xen/arch/x86/platform_hypercall.c          |  3 ---
 xen/arch/x86/setup.c                       |  1 +
 xen/arch/x86/traps.c                       |  1 +
 xen/arch/x86/x86_64/traps.c                |  1 +
 xen/common/symbols.c                       | 17 -----------------
 xen/include/xen/consoled.h                 |  2 +-
 xen/include/xen/hypercall.h                |  3 +++
 xen/include/xen/symbols.h                  | 18 ++++++++++++++++++
 21 files changed, 48 insertions(+), 28 deletions(-)

--
2.34.1

[XEN PATCH 1/7] xen: add declarations for variables where needed

[Nicola Vetrini]

Some variables with external linkage used in C code do not have
a visible declaration where they are defined. Providing such
declaration also resolves violations of MISRA C:2012 Rule 8.4.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 xen/arch/arm/include/asm/setup.h  |  3 +++
 xen/arch/arm/include/asm/smp.h    |  3 +++
 xen/arch/x86/cpu/mcheck/mce.c     |  6 +++---
 xen/arch/x86/include/asm/setup.h  |  3 +++
 xen/arch/x86/irq.c                |  2 +-
 xen/arch/x86/platform_hypercall.c |  3 ---
 xen/common/symbols.c              | 17 -----------------
 xen/include/xen/hypercall.h       |  3 +++
 xen/include/xen/symbols.h         | 18 ++++++++++++++++++
 9 files changed, 34 insertions(+), 24 deletions(-)

diff --git a/xen/arch/arm/include/asm/setup.h b/xen/arch/arm/include/asm/setup.h
index b8866c20f462..8806a74b216d 100644
--- a/xen/arch/arm/include/asm/setup.h
+++ b/xen/arch/arm/include/asm/setup.h
@@ -183,9 +183,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/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index 6141b7eb9cf1..e855f958030d 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -1682,13 +1682,13 @@ 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 +1702,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/include/asm/setup.h b/xen/arch/x86/include/asm/setup.h
index dfdd9e555149..3c27fe915ed4 100644
--- a/xen/arch/x86/include/asm/setup.h
+++ b/xen/arch/x86/include/asm/setup.h
@@ -13,8 +13,11 @@ extern char __2M_rwdata_start[], __2M_rwdata_end[];
 extern unsigned long xenheap_initial_phys_start;
 extern uint64_t boot_tsc_stamp;
 
+extern char cpu0_stack[STACK_SIZE];
 extern void *stack_start;
 
+extern unsigned long cr4_pv32_mask;
+
 void early_cpu_init(void);
 void early_time_init(void);
 
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 6abfd8162120..604dba94b052 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/arch/x86/platform_hypercall.c b/xen/arch/x86/platform_hypercall.c
index 9469de9045c7..e4dbec73d784 100644
--- a/xen/arch/x86/platform_hypercall.c
+++ b/xen/arch/x86/platform_hypercall.c
@@ -36,9 +36,6 @@
 #include "cpu/mtrr/mtrr.h"
 #include <xsm/xsm.h>
 
-/* Declarations for items shared with the compat mode handler. */
-extern spinlock_t xenpf_lock;
-
 #define RESOURCE_ACCESS_MAX_ENTRIES 3
 struct resource_access {
     unsigned int nr_done;
diff --git a/xen/common/symbols.c b/xen/common/symbols.c
index 691e61792506..7c3514c65f2e 100644
--- a/xen/common/symbols.c
+++ b/xen/common/symbols.c
@@ -21,23 +21,6 @@
 #include <xen/guest_access.h>
 #include <xen/errno.h>
 
-#ifdef SYMBOLS_ORIGIN
-extern const unsigned int symbols_offsets[];
-#define symbols_address(n) (SYMBOLS_ORIGIN + symbols_offsets[n])
-#else
-extern const unsigned long symbols_addresses[];
-#define symbols_address(n) symbols_addresses[n]
-#endif
-extern const unsigned int symbols_num_syms;
-extern const u8 symbols_names[];
-
-extern const struct symbol_offset symbols_sorted_offsets[];
-
-extern const u8 symbols_token_table[];
-extern const u16 symbols_token_index[];
-
-extern const unsigned int symbols_markers[];
-
 /* expand a compressed symbol data into the resulting uncompressed string,
    given the offset to where the symbol is in the compressed stream */
 static unsigned int symbols_expand_symbol(unsigned int off, char *result)
diff --git a/xen/include/xen/hypercall.h b/xen/include/xen/hypercall.h
index f307dfb59760..12de5a69b5b1 100644
--- a/xen/include/xen/hypercall.h
+++ b/xen/include/xen/hypercall.h
@@ -24,6 +24,9 @@
 /* Needs to be after asm/hypercall.h. */
 #include <xen/hypercall-defs.h>
 
+/* Declarations for items shared with the compat mode handler. */
+extern spinlock_t xenpf_lock;
+
 extern long
 arch_do_domctl(
     struct xen_domctl *domctl, struct domain *d,
diff --git a/xen/include/xen/symbols.h b/xen/include/xen/symbols.h
index 20bbb28ef226..92540409265e 100644
--- a/xen/include/xen/symbols.h
+++ b/xen/include/xen/symbols.h
@@ -33,4 +33,22 @@ struct symbol_offset {
     uint32_t stream; /* .. in the compressed stream.*/
     uint32_t addr;   /* .. and in the fixed size address array. */
 };
+
+#ifdef SYMBOLS_ORIGIN
+extern const unsigned int symbols_offsets[];
+#define symbols_address(n) (SYMBOLS_ORIGIN + symbols_offsets[n])
+#else
+extern const unsigned long symbols_addresses[];
+#define symbols_address(n) symbols_addresses[n]
+#endif
+extern const unsigned int symbols_num_syms;
+extern const u8 symbols_names[];
+
+extern const struct symbol_offset symbols_sorted_offsets[];
+
+extern const u8 symbols_token_table[];
+extern const u16 symbols_token_index[];
+
+extern const unsigned int symbols_markers[];
+
 #endif /*_XEN_SYMBOLS_H*/
-- 
2.34.1

[XEN PATCH 2/7] x86: add deviations for variables only used in asm code

[Nicola Vetrini]

These variables are only used by asm code, and therefore
the lack of a declaration is justified by the corresponding
SAF comment.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 xen/arch/x86/include/asm/asm_defns.h | 1 +
 xen/arch/x86/setup.c                 | 1 +
 2 files changed, 2 insertions(+)

diff --git a/xen/arch/x86/include/asm/asm_defns.h b/xen/arch/x86/include/asm/asm_defns.h
index baaaccb26e17..a2516de7749b 100644
--- a/xen/arch/x86/include/asm/asm_defns.h
+++ b/xen/arch/x86/include/asm/asm_defns.h
@@ -31,6 +31,7 @@ asm ( "\t.equ CONFIG_INDIRECT_THUNK, "
  * gets set up by the containing function.
  */
 #ifdef CONFIG_FRAME_POINTER
+/* SAF-1-safe */
 register unsigned long current_stack_pointer asm("rsp");
 # define ASM_CALL_CONSTRAINT , "+r" (current_stack_pointer)
 #else
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 08ba1f95d635..7e2979f419af 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -153,6 +153,7 @@ char __section(".init.bss.stack_aligned") __aligned(STACK_SIZE)
 void *stack_start = cpu0_stack + STACK_SIZE - sizeof(struct cpu_info);
 
 /* Used by the boot asm to stash the relocated multiboot info pointer. */
+/* SAF-1-safe */
 unsigned int __initdata multiboot_ptr;
 
 struct cpuinfo_x86 __read_mostly boot_cpu_data = { 0, 0, 0, 0, -1 };
-- 
2.34.1

[XEN PATCH 3/7] x86: add deviation comments for asm-only functions

[Nicola Vetrini]

As stated in rules.rst, functions used only in asm code
are allowed to have no prior declaration visible when being
defined, hence these functions are deviated.
This also fixes violations of MISRA C:2012 Rule 8.4.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 xen/arch/x86/hvm/svm/intr.c      | 1 +
 xen/arch/x86/hvm/svm/nestedsvm.c | 1 +
 xen/arch/x86/hvm/svm/svm.c       | 2 ++
 xen/arch/x86/traps.c             | 1 +
 xen/arch/x86/x86_64/traps.c      | 1 +
 5 files changed, 6 insertions(+)

diff --git a/xen/arch/x86/hvm/svm/intr.c b/xen/arch/x86/hvm/svm/intr.c
index 192e17ebbfbb..bd9dc560bbc6 100644
--- a/xen/arch/x86/hvm/svm/intr.c
+++ b/xen/arch/x86/hvm/svm/intr.c
@@ -123,6 +123,7 @@ static void svm_enable_intr_window(struct vcpu *v, struct hvm_intack intack)
         vmcb, general1_intercepts | GENERAL1_INTERCEPT_VINTR);
 }
 
+/* SAF-1-safe */
 void svm_intr_assist(void)
 {
     struct vcpu *v = current;
diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index a09b6abaaeaf..c80d59e0728e 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -1441,6 +1441,7 @@ nestedsvm_vcpu_vmexit(struct vcpu *v, struct cpu_user_regs *regs,
 }
 
 /* VCPU switch */
+/* SAF-1-safe */
 void nsvm_vcpu_switch(void)
 {
     struct cpu_user_regs *regs = guest_cpu_user_regs();
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index beb076ea8d62..b9fabd45a119 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1044,6 +1044,7 @@ static void noreturn cf_check svm_do_resume(void)
     reset_stack_and_jump(svm_asm_do_resume);
 }
 
+/* SAF-1-safe */
 void svm_vmenter_helper(void)
 {
     const struct cpu_user_regs *regs = guest_cpu_user_regs();
@@ -2574,6 +2575,7 @@ const struct hvm_function_table * __init start_svm(void)
     return &svm_function_table;
 }
 
+/* SAF-1-safe */
 void svm_vmexit_handler(void)
 {
     struct cpu_user_regs *regs = guest_cpu_user_regs();
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 0a005f088bca..f27ddb728b2c 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -2260,6 +2260,7 @@ void asm_domain_crash_synchronous(unsigned long addr)
 }
 
 #ifdef CONFIG_DEBUG
+/* SAF-1-safe */
 void check_ist_exit(const struct cpu_user_regs *regs, bool ist_exit)
 {
     const unsigned int ist_mask =
diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
index f4d17b483032..bcb7559b21c3 100644
--- a/xen/arch/x86/x86_64/traps.c
+++ b/xen/arch/x86/x86_64/traps.c
@@ -266,6 +266,7 @@ void show_page_walk(unsigned long addr)
            l1_table_offset(addr), l1e_get_intpte(l1e), pfn);
 }
 
+/* SAF-1-safe */
 void do_double_fault(struct cpu_user_regs *regs)
 {
     unsigned int cpu;
-- 
2.34.1

[XEN PATCH 4/7] x86/grant: switch included header to make declarations visible

[Nicola Vetrini]

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>
---
 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..576aeb50adf4 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 <asm/paging.h>
+
 #ifdef CONFIG_HVM
 
 int create_grant_p2m_mapping(uint64_t addr, mfn_t frame,
-- 
2.34.1

[XEN PATCH 5/7] x86/vm_event: add missing include for hvm_vm_event_do_resume

[Nicola Vetrini]

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

[XEN PATCH 6/7] xen/console: make function static inline

[Nicola Vetrini]

The definition of 'consoled_guest_tx' can be static inline,
thereby fixing a violation of MISRA C:2012 Rule 8.4.

Fixes: 5ef49f185c2d ("x86/pv-shim: shadow PV console's page for L2 DomU")
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 xen/include/xen/consoled.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/include/xen/consoled.h b/xen/include/xen/consoled.h
index fd5d220a8aca..e943d8d48f7b 100644
--- a/xen/include/xen/consoled.h
+++ b/xen/include/xen/consoled.h
@@ -12,7 +12,7 @@ size_t consoled_guest_tx(char c);
 
 #else
 
-size_t consoled_guest_tx(char c) { return 0; }
+static inline size_t consoled_guest_tx(char c) { return 0; }
 
 #endif /* !CONFIG_PV_SHIM */
 #endif /* __XEN_CONSOLED_H__ */
-- 
2.34.1

[XEN PATCH 7/7] x86/mem_access: make function static

[Nicola Vetrini]

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>
---
 xen/arch/x86/mm/mem_access.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c
index c472fa1ee58b..78633d335d72 100644
--- a/xen/arch/x86/mm/mem_access.c
+++ b/xen/arch/x86/mm/mem_access.c
@@ -249,7 +249,7 @@ 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,
+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)
 {
-- 
2.34.1

Re: [XEN PATCH 1/7] xen: add declarations for variables where needed

[Roger Pau Monné]

On Mon, Oct 02, 2023 at 09:49:44AM +0200, Nicola Vetrini wrote:
> Some variables with external linkage used in C code do not have
> a visible declaration where they are defined. Providing such
> declaration also resolves violations of MISRA C:2012 Rule 8.4.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> ---
>  xen/arch/arm/include/asm/setup.h  |  3 +++
>  xen/arch/arm/include/asm/smp.h    |  3 +++
>  xen/arch/x86/cpu/mcheck/mce.c     |  6 +++---
>  xen/arch/x86/include/asm/setup.h  |  3 +++
>  xen/arch/x86/irq.c                |  2 +-
>  xen/arch/x86/platform_hypercall.c |  3 ---
>  xen/common/symbols.c              | 17 -----------------
>  xen/include/xen/hypercall.h       |  3 +++
>  xen/include/xen/symbols.h         | 18 ++++++++++++++++++
>  9 files changed, 34 insertions(+), 24 deletions(-)
> 
> diff --git a/xen/arch/arm/include/asm/setup.h b/xen/arch/arm/include/asm/setup.h
> index b8866c20f462..8806a74b216d 100644
> --- a/xen/arch/arm/include/asm/setup.h
> +++ b/xen/arch/arm/include/asm/setup.h
> @@ -183,9 +183,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/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
> index 6141b7eb9cf1..e855f958030d 100644
> --- a/xen/arch/x86/cpu/mcheck/mce.c
> +++ b/xen/arch/x86/cpu/mcheck/mce.c
> @@ -1682,13 +1682,13 @@ 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 +1702,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/include/asm/setup.h b/xen/arch/x86/include/asm/setup.h
> index dfdd9e555149..3c27fe915ed4 100644
> --- a/xen/arch/x86/include/asm/setup.h
> +++ b/xen/arch/x86/include/asm/setup.h
> @@ -13,8 +13,11 @@ extern char __2M_rwdata_start[], __2M_rwdata_end[];
>  extern unsigned long xenheap_initial_phys_start;
>  extern uint64_t boot_tsc_stamp;
>  
> +extern char cpu0_stack[STACK_SIZE];
>  extern void *stack_start;
>  
> +extern unsigned long cr4_pv32_mask;

This one might better go in compat.h, albeit that would require it's
setting to be gated to CONFIG_PV32.  setup.h is IMO for init time
stuff.

> +
>  void early_cpu_init(void);
>  void early_time_init(void);
>  
> diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
> index 6abfd8162120..604dba94b052 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/arch/x86/platform_hypercall.c b/xen/arch/x86/platform_hypercall.c
> index 9469de9045c7..e4dbec73d784 100644
> --- a/xen/arch/x86/platform_hypercall.c
> +++ b/xen/arch/x86/platform_hypercall.c
> @@ -36,9 +36,6 @@
>  #include "cpu/mtrr/mtrr.h"
>  #include <xsm/xsm.h>
>  
> -/* Declarations for items shared with the compat mode handler. */
> -extern spinlock_t xenpf_lock;
> -
>  #define RESOURCE_ACCESS_MAX_ENTRIES 3
>  struct resource_access {
>      unsigned int nr_done;
> diff --git a/xen/common/symbols.c b/xen/common/symbols.c
> index 691e61792506..7c3514c65f2e 100644
> --- a/xen/common/symbols.c
> +++ b/xen/common/symbols.c
> @@ -21,23 +21,6 @@
>  #include <xen/guest_access.h>
>  #include <xen/errno.h>
>  
> -#ifdef SYMBOLS_ORIGIN
> -extern const unsigned int symbols_offsets[];
> -#define symbols_address(n) (SYMBOLS_ORIGIN + symbols_offsets[n])
> -#else
> -extern const unsigned long symbols_addresses[];
> -#define symbols_address(n) symbols_addresses[n]
> -#endif
> -extern const unsigned int symbols_num_syms;
> -extern const u8 symbols_names[];
> -
> -extern const struct symbol_offset symbols_sorted_offsets[];
> -
> -extern const u8 symbols_token_table[];
> -extern const u16 symbols_token_index[];
> -
> -extern const unsigned int symbols_markers[];
> -
>  /* expand a compressed symbol data into the resulting uncompressed string,
>     given the offset to where the symbol is in the compressed stream */
>  static unsigned int symbols_expand_symbol(unsigned int off, char *result)
> diff --git a/xen/include/xen/hypercall.h b/xen/include/xen/hypercall.h
> index f307dfb59760..12de5a69b5b1 100644
> --- a/xen/include/xen/hypercall.h
> +++ b/xen/include/xen/hypercall.h
> @@ -24,6 +24,9 @@
>  /* Needs to be after asm/hypercall.h. */
>  #include <xen/hypercall-defs.h>
>  
> +/* Declarations for items shared with the compat mode handler. */
> +extern spinlock_t xenpf_lock;

I'm confused about why this needs to be moved, AFAICT xenpf_lock is
only used in platform_hypercall.c, and the declaration is
unconditional, so there's no definition without declaration issue.

> +
>  extern long
>  arch_do_domctl(
>      struct xen_domctl *domctl, struct domain *d,
> diff --git a/xen/include/xen/symbols.h b/xen/include/xen/symbols.h
> index 20bbb28ef226..92540409265e 100644
> --- a/xen/include/xen/symbols.h
> +++ b/xen/include/xen/symbols.h
> @@ -33,4 +33,22 @@ struct symbol_offset {
>      uint32_t stream; /* .. in the compressed stream.*/
>      uint32_t addr;   /* .. and in the fixed size address array. */
>  };
> +
> +#ifdef SYMBOLS_ORIGIN
> +extern const unsigned int symbols_offsets[];
> +#define symbols_address(n) (SYMBOLS_ORIGIN + symbols_offsets[n])
> +#else
> +extern const unsigned long symbols_addresses[];
> +#define symbols_address(n) symbols_addresses[n]
> +#endif
> +extern const unsigned int symbols_num_syms;
> +extern const u8 symbols_names[];
> +
> +extern const struct symbol_offset symbols_sorted_offsets[];
> +
> +extern const u8 symbols_token_table[];
> +extern const u16 symbols_token_index[];
> +
> +extern const unsigned int symbols_markers[];
> +
>  #endif /*_XEN_SYMBOLS_H*/

This one is ugly, but I can't see a better way immediately.

Thanks, Roger.

Re: [XEN PATCH 2/7] x86: add deviations for variables only used in asm code

[Stefano Stabellini]

On Mon, 2 Oct 2023, Nicola Vetrini wrote:
> These variables are only used by asm code, and therefore
> the lack of a declaration is justified by the corresponding
> SAF comment.
>
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

Re: [XEN PATCH 3/7] x86: add deviation comments for asm-only functions

[Stefano Stabellini]

On Mon, 2 Oct 2023, Nicola Vetrini wrote:
> As stated in rules.rst, functions used only in asm code
> are allowed to have no prior declaration visible when being
> defined, hence these functions are deviated.
> This also fixes violations of MISRA C:2012 Rule 8.4.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

Re: [XEN PATCH 4/7] x86/grant: switch included header to make declarations visible

[Stefano Stabellini]

On Mon, 2 Oct 2023, Nicola Vetrini wrote:
> 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>
> ---
>  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>

This makes sense...


>  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..576aeb50adf4 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 <asm/paging.h>

... but I don't understand this one. It doesn't look like
asm/hvm/grant_table.h actually needs asm/paging.h ? Maybe it should be
included in xen/arch/x86/hvm/grant_table.c instead ?


>  #ifdef CONFIG_HVM
>  
>  int create_grant_p2m_mapping(uint64_t addr, mfn_t frame,
> -- 
> 2.34.1
> 

Re: [XEN PATCH 6/7] xen/console: make function static inline

[Stefano Stabellini]

On Mon, 2 Oct 2023, Nicola Vetrini wrote:
> The definition of 'consoled_guest_tx' can be static inline,
> thereby fixing a violation of MISRA C:2012 Rule 8.4.
> 
> 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 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/include/xen/consoled.h b/xen/include/xen/consoled.h
> index fd5d220a8aca..e943d8d48f7b 100644
> --- a/xen/include/xen/consoled.h
> +++ b/xen/include/xen/consoled.h
> @@ -12,7 +12,7 @@ size_t consoled_guest_tx(char c);
>  
>  #else
>  
> -size_t consoled_guest_tx(char c) { return 0; }
> +static inline size_t consoled_guest_tx(char c) { return 0; }
>  
>  #endif /* !CONFIG_PV_SHIM */
>  #endif /* __XEN_CONSOLED_H__ */
> -- 
> 2.34.1
> 

Re: [XEN PATCH 4/7] x86/grant: switch included header to make declarations visible

[Nicola Vetrini]

On 03/10/2023 00:42, Stefano Stabellini wrote:
> On Mon, 2 Oct 2023, Nicola Vetrini wrote:
>> 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>
>> ---
>>  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>
> 
> This makes sense...
> 
> 
>>  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..576aeb50adf4 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 <asm/paging.h>
> 
> ... but I don't understand this one. It doesn't look like
> asm/hvm/grant_table.h actually needs asm/paging.h ? Maybe it should be
> included in xen/arch/x86/hvm/grant_table.c instead ?
> 
> 
>>  #ifdef CONFIG_HVM
>> 
>>  int create_grant_p2m_mapping(uint64_t addr, mfn_t frame,
>> --
>> 2.34.1
>> 

See this thread [1] for more context. There was no response, so I went 
for the route that
made more sense to me. I guess you could say that only <xen/mm-frame.h> 
is actually needed
to get a definition of mfn_t, but I put <asm/paging.h> as in the 
<asm/grant_table.h> header.

[1] 
https://lore.kernel.org/xen-devel/a4b6710b66ed05292388ac6882b940ec@bugseng.com/

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)

Re: [XEN PATCH 1/7] xen: add declarations for variables where needed

[Nicola Vetrini]

On 02/10/2023 13:07, Roger Pau Monné wrote:
> On Mon, Oct 02, 2023 at 09:49:44AM +0200, Nicola Vetrini wrote:
>> Some variables with external linkage used in C code do not have
>> a visible declaration where they are defined. Providing such
>> declaration also resolves violations of MISRA C:2012 Rule 8.4.
>> 
>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
>> ---
>>  xen/arch/arm/include/asm/setup.h  |  3 +++
>>  xen/arch/arm/include/asm/smp.h    |  3 +++
>>  xen/arch/x86/cpu/mcheck/mce.c     |  6 +++---
>>  xen/arch/x86/include/asm/setup.h  |  3 +++
>>  xen/arch/x86/irq.c                |  2 +-
>>  xen/arch/x86/platform_hypercall.c |  3 ---
>>  xen/common/symbols.c              | 17 -----------------
>>  xen/include/xen/hypercall.h       |  3 +++
>>  xen/include/xen/symbols.h         | 18 ++++++++++++++++++
>>  9 files changed, 34 insertions(+), 24 deletions(-)
>> 

>> diff --git a/xen/arch/x86/include/asm/setup.h 
>> b/xen/arch/x86/include/asm/setup.h
>> index dfdd9e555149..3c27fe915ed4 100644
>> --- a/xen/arch/x86/include/asm/setup.h
>> +++ b/xen/arch/x86/include/asm/setup.h
>> @@ -13,8 +13,11 @@ extern char __2M_rwdata_start[], __2M_rwdata_end[];
>>  extern unsigned long xenheap_initial_phys_start;
>>  extern uint64_t boot_tsc_stamp;
>> 
>> +extern char cpu0_stack[STACK_SIZE];
>>  extern void *stack_start;
>> 
>> +extern unsigned long cr4_pv32_mask;
> 
> This one might better go in compat.h, albeit that would require it's
> setting to be gated to CONFIG_PV32.  setup.h is IMO for init time
> stuff.
> 

Ok

>> +
>>  void early_cpu_init(void);
>>  void early_time_init(void);
>> 
>> diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
>> index 6abfd8162120..604dba94b052 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/arch/x86/platform_hypercall.c 
>> b/xen/arch/x86/platform_hypercall.c
>> index 9469de9045c7..e4dbec73d784 100644
>> --- a/xen/arch/x86/platform_hypercall.c
>> +++ b/xen/arch/x86/platform_hypercall.c
>> @@ -36,9 +36,6 @@
>>  #include "cpu/mtrr/mtrr.h"
>>  #include <xsm/xsm.h>
>> 
>> -/* Declarations for items shared with the compat mode handler. */
>> -extern spinlock_t xenpf_lock;
>> -
>>  #define RESOURCE_ACCESS_MAX_ENTRIES 3
>>  struct resource_access {
>>      unsigned int nr_done;
>> diff --git a/xen/common/symbols.c b/xen/common/symbols.c
>> index 691e61792506..7c3514c65f2e 100644
>> --- a/xen/common/symbols.c
>> +++ b/xen/common/symbols.c
>> @@ -21,23 +21,6 @@
>>  #include <xen/guest_access.h>
>>  #include <xen/errno.h>
>> 
>> -#ifdef SYMBOLS_ORIGIN
>> -extern const unsigned int symbols_offsets[];
>> -#define symbols_address(n) (SYMBOLS_ORIGIN + symbols_offsets[n])
>> -#else
>> -extern const unsigned long symbols_addresses[];
>> -#define symbols_address(n) symbols_addresses[n]
>> -#endif
>> -extern const unsigned int symbols_num_syms;
>> -extern const u8 symbols_names[];
>> -
>> -extern const struct symbol_offset symbols_sorted_offsets[];
>> -
>> -extern const u8 symbols_token_table[];
>> -extern const u16 symbols_token_index[];
>> -
>> -extern const unsigned int symbols_markers[];
>> -
>>  /* expand a compressed symbol data into the resulting uncompressed 
>> string,
>>     given the offset to where the symbol is in the compressed stream 
>> */
>>  static unsigned int symbols_expand_symbol(unsigned int off, char 
>> *result)
>> diff --git a/xen/include/xen/hypercall.h b/xen/include/xen/hypercall.h
>> index f307dfb59760..12de5a69b5b1 100644
>> --- a/xen/include/xen/hypercall.h
>> +++ b/xen/include/xen/hypercall.h
>> @@ -24,6 +24,9 @@
>>  /* Needs to be after asm/hypercall.h. */
>>  #include <xen/hypercall-defs.h>
>> 
>> +/* Declarations for items shared with the compat mode handler. */
>> +extern spinlock_t xenpf_lock;
> 
> I'm confused about why this needs to be moved, AFAICT xenpf_lock is
> only used in platform_hypercall.c, and the declaration is
> unconditional, so there's no definition without declaration issue.
> 

The violation was on ARM code, because of a slight inconsistency:
xen/arch/arm/platform_hypercall.c:DEFINE_SPINLOCK(xenpf_lock);

xen/arch/x86/platform_hypercall.c:extern spinlock_t xenpf_lock;
xen/arch/x86/platform_hypercall.c:DEFINE_SPINLOCK(xenpf_lock);

therefore, by moving the extern declaration to the common header there 
should be no issue.

>> +
>>  extern long
>>  arch_do_domctl(
>>      struct xen_domctl *domctl, struct domain *d,
>> diff --git a/xen/include/xen/symbols.h b/xen/include/xen/symbols.h
>> index 20bbb28ef226..92540409265e 100644
>> --- a/xen/include/xen/symbols.h
>> +++ b/xen/include/xen/symbols.h
>> @@ -33,4 +33,22 @@ struct symbol_offset {
>>      uint32_t stream; /* .. in the compressed stream.*/
>>      uint32_t addr;   /* .. and in the fixed size address array. */
>>  };
>> +
>> +#ifdef SYMBOLS_ORIGIN
>> +extern const unsigned int symbols_offsets[];
>> +#define symbols_address(n) (SYMBOLS_ORIGIN + symbols_offsets[n])
>> +#else
>> +extern const unsigned long symbols_addresses[];
>> +#define symbols_address(n) symbols_addresses[n]
>> +#endif
>> +extern const unsigned int symbols_num_syms;
>> +extern const u8 symbols_names[];
>> +
>> +extern const struct symbol_offset symbols_sorted_offsets[];
>> +
>> +extern const u8 symbols_token_table[];
>> +extern const u16 symbols_token_index[];
>> +
>> +extern const unsigned int symbols_markers[];
>> +
>>  #endif /*_XEN_SYMBOLS_H*/
> 
> This one is ugly, but I can't see a better way immediately.
> 
> Thanks, Roger.

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)

Re: [XEN PATCH 1/7] xen: add declarations for variables where needed

[Roger Pau Monné]

On Tue, Oct 03, 2023 at 09:05:34AM +0200, Nicola Vetrini wrote:
> On 02/10/2023 13:07, Roger Pau Monné wrote:
> > On Mon, Oct 02, 2023 at 09:49:44AM +0200, Nicola Vetrini wrote:
> > > Some variables with external linkage used in C code do not have
> > > a visible declaration where they are defined. Providing such
> > > declaration also resolves violations of MISRA C:2012 Rule 8.4.
> > > 
> > > Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> > > ---
> > >  xen/arch/arm/include/asm/setup.h  |  3 +++
> > >  xen/arch/arm/include/asm/smp.h    |  3 +++
> > >  xen/arch/x86/cpu/mcheck/mce.c     |  6 +++---
> > >  xen/arch/x86/include/asm/setup.h  |  3 +++
> > >  xen/arch/x86/irq.c                |  2 +-
> > >  xen/arch/x86/platform_hypercall.c |  3 ---
> > >  xen/common/symbols.c              | 17 -----------------
> > >  xen/include/xen/hypercall.h       |  3 +++
> > >  xen/include/xen/symbols.h         | 18 ++++++++++++++++++
> > >  9 files changed, 34 insertions(+), 24 deletions(-)
> > > 
> 
> > > diff --git a/xen/arch/x86/include/asm/setup.h
> > > b/xen/arch/x86/include/asm/setup.h
> > > index dfdd9e555149..3c27fe915ed4 100644
> > > --- a/xen/arch/x86/include/asm/setup.h
> > > +++ b/xen/arch/x86/include/asm/setup.h
> > > @@ -13,8 +13,11 @@ extern char __2M_rwdata_start[], __2M_rwdata_end[];
> > >  extern unsigned long xenheap_initial_phys_start;
> > >  extern uint64_t boot_tsc_stamp;
> > > 
> > > +extern char cpu0_stack[STACK_SIZE];
> > >  extern void *stack_start;
> > > 
> > > +extern unsigned long cr4_pv32_mask;
> > 
> > This one might better go in compat.h, albeit that would require it's
> > setting to be gated to CONFIG_PV32.  setup.h is IMO for init time
> > stuff.
> > 
> 
> Ok
> 
> > > +
> > >  void early_cpu_init(void);
> > >  void early_time_init(void);
> > > 
> > > diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
> > > index 6abfd8162120..604dba94b052 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/arch/x86/platform_hypercall.c
> > > b/xen/arch/x86/platform_hypercall.c
> > > index 9469de9045c7..e4dbec73d784 100644
> > > --- a/xen/arch/x86/platform_hypercall.c
> > > +++ b/xen/arch/x86/platform_hypercall.c
> > > @@ -36,9 +36,6 @@
> > >  #include "cpu/mtrr/mtrr.h"
> > >  #include <xsm/xsm.h>
> > > 
> > > -/* Declarations for items shared with the compat mode handler. */
> > > -extern spinlock_t xenpf_lock;
> > > -
> > >  #define RESOURCE_ACCESS_MAX_ENTRIES 3
> > >  struct resource_access {
> > >      unsigned int nr_done;
> > > diff --git a/xen/common/symbols.c b/xen/common/symbols.c
> > > index 691e61792506..7c3514c65f2e 100644
> > > --- a/xen/common/symbols.c
> > > +++ b/xen/common/symbols.c
> > > @@ -21,23 +21,6 @@
> > >  #include <xen/guest_access.h>
> > >  #include <xen/errno.h>
> > > 
> > > -#ifdef SYMBOLS_ORIGIN
> > > -extern const unsigned int symbols_offsets[];
> > > -#define symbols_address(n) (SYMBOLS_ORIGIN + symbols_offsets[n])
> > > -#else
> > > -extern const unsigned long symbols_addresses[];
> > > -#define symbols_address(n) symbols_addresses[n]
> > > -#endif
> > > -extern const unsigned int symbols_num_syms;
> > > -extern const u8 symbols_names[];
> > > -
> > > -extern const struct symbol_offset symbols_sorted_offsets[];
> > > -
> > > -extern const u8 symbols_token_table[];
> > > -extern const u16 symbols_token_index[];
> > > -
> > > -extern const unsigned int symbols_markers[];
> > > -
> > >  /* expand a compressed symbol data into the resulting uncompressed
> > > string,
> > >     given the offset to where the symbol is in the compressed stream
> > > */
> > >  static unsigned int symbols_expand_symbol(unsigned int off, char
> > > *result)
> > > diff --git a/xen/include/xen/hypercall.h b/xen/include/xen/hypercall.h
> > > index f307dfb59760..12de5a69b5b1 100644
> > > --- a/xen/include/xen/hypercall.h
> > > +++ b/xen/include/xen/hypercall.h
> > > @@ -24,6 +24,9 @@
> > >  /* Needs to be after asm/hypercall.h. */
> > >  #include <xen/hypercall-defs.h>
> > > 
> > > +/* Declarations for items shared with the compat mode handler. */
> > > +extern spinlock_t xenpf_lock;
> > 
> > I'm confused about why this needs to be moved, AFAICT xenpf_lock is
> > only used in platform_hypercall.c, and the declaration is
> > unconditional, so there's no definition without declaration issue.
> > 
> 
> The violation was on ARM code, because of a slight inconsistency:
> xen/arch/arm/platform_hypercall.c:DEFINE_SPINLOCK(xenpf_lock);
> 
> xen/arch/x86/platform_hypercall.c:extern spinlock_t xenpf_lock;
> xen/arch/x86/platform_hypercall.c:DEFINE_SPINLOCK(xenpf_lock);
> 
> therefore, by moving the extern declaration to the common header there
> should be no issue.

Just make xenpf_lock static on arm, that would be a better solution.
The only need for the lock to be global is because of compat code, and
arm doesn't seem to need it.

Thanks, Roger.

Re: [XEN PATCH 1/7] xen: add declarations for variables where needed

[Nicola Vetrini]

On 03/10/2023 09:18, Roger Pau Monné wrote:
> On Tue, Oct 03, 2023 at 09:05:34AM +0200, Nicola Vetrini wrote:
>> On 02/10/2023 13:07, Roger Pau Monné wrote:
>> > On Mon, Oct 02, 2023 at 09:49:44AM +0200, Nicola Vetrini wrote:
>> > > Some variables with external linkage used in C code do not have
>> > > a visible declaration where they are defined. Providing such
>> > > declaration also resolves violations of MISRA C:2012 Rule 8.4.
>> > >
>> > > Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
>> > > ---
>> > >  xen/arch/arm/include/asm/setup.h  |  3 +++
>> > >  xen/arch/arm/include/asm/smp.h    |  3 +++
>> > >  xen/arch/x86/cpu/mcheck/mce.c     |  6 +++---
>> > >  xen/arch/x86/include/asm/setup.h  |  3 +++
>> > >  xen/arch/x86/irq.c                |  2 +-
>> > >  xen/arch/x86/platform_hypercall.c |  3 ---
>> > >  xen/common/symbols.c              | 17 -----------------
>> > >  xen/include/xen/hypercall.h       |  3 +++
>> > >  xen/include/xen/symbols.h         | 18 ++++++++++++++++++
>> > >  9 files changed, 34 insertions(+), 24 deletions(-)
>> > >

>> > > diff --git a/xen/include/xen/hypercall.h b/xen/include/xen/hypercall.h
>> > > index f307dfb59760..12de5a69b5b1 100644
>> > > --- a/xen/include/xen/hypercall.h
>> > > +++ b/xen/include/xen/hypercall.h
>> > > @@ -24,6 +24,9 @@
>> > >  /* Needs to be after asm/hypercall.h. */
>> > >  #include <xen/hypercall-defs.h>
>> > >
>> > > +/* Declarations for items shared with the compat mode handler. */
>> > > +extern spinlock_t xenpf_lock;
>> >
>> > I'm confused about why this needs to be moved, AFAICT xenpf_lock is
>> > only used in platform_hypercall.c, and the declaration is
>> > unconditional, so there's no definition without declaration issue.
>> >
>> 
>> The violation was on ARM code, because of a slight inconsistency:
>> xen/arch/arm/platform_hypercall.c:DEFINE_SPINLOCK(xenpf_lock);
>> 
>> xen/arch/x86/platform_hypercall.c:extern spinlock_t xenpf_lock;
>> xen/arch/x86/platform_hypercall.c:DEFINE_SPINLOCK(xenpf_lock);
>> 
>> therefore, by moving the extern declaration to the common header there
>> should be no issue.
> 
> Just make xenpf_lock static on arm, that would be a better solution.
> The only need for the lock to be global is because of compat code, and
> arm doesn't seem to need it.
> 
> Thanks, Roger.

I'll do that, thanks.

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)

Re: [XEN PATCH 4/7] x86/grant: switch included header to make declarations visible

[Stefano Stabellini]

On Tue, 3 Oct 2023, Nicola Vetrini wrote:
> On 03/10/2023 00:42, Stefano Stabellini wrote:
> > On Mon, 2 Oct 2023, Nicola Vetrini wrote:
> > > 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>
> > > ---
> > >  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>
> > 
> > This makes sense...
> > 
> > 
> > >  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..576aeb50adf4 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 <asm/paging.h>
> > 
> > ... but I don't understand this one. It doesn't look like
> > asm/hvm/grant_table.h actually needs asm/paging.h ? Maybe it should be
> > included in xen/arch/x86/hvm/grant_table.c instead ?
> > 
> > 
> > >  #ifdef CONFIG_HVM
> > > 
> > >  int create_grant_p2m_mapping(uint64_t addr, mfn_t frame,
> > > --
> > > 2.34.1
> > > 
> 
> See this thread [1] for more context. There was no response, so I went for the
> route that
> made more sense to me. I guess you could say that only <xen/mm-frame.h> is
> actually needed
> to get a definition of mfn_t, but I put <asm/paging.h> as in the
> <asm/grant_table.h> header.
> 
> [1]
> https://lore.kernel.org/xen-devel/a4b6710b66ed05292388ac6882b940ec@bugseng.com/

I didn't realize it was for mfn_t. In that case it makes sense.

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>