[PATCH 0/5] Fix MISRA C 2012 violations

[PATCH 0/5] Fix MISRA C 2012 violations

[Xenia Ragiadakou]

Resolve MISRA C 2012 Rule 8.4 warnings.

Xenia Ragiadakou (5):
  xen/common: page_alloc: Fix MISRA C 2012 Rule 8.7 violation
  xen/common: vm_event: Fix MISRA C 2012 Rule 8.7 violation
  xen/drivers: iommu: Fix MISRA C 2012 Rule 8.7 violation
  xen/sched: credit: Fix MISRA C 2012 Rule 8.7 violation
  xen/arm64: traps: Fix MISRA C 2012 Rule 8.4 violations

 xen/arch/arm/arm64/traps.c             | 1 +
 xen/arch/arm/include/asm/arm64/traps.h | 2 ++
 xen/common/page_alloc.c                | 4 ++--
 xen/common/sched/credit.c              | 2 +-
 xen/common/vm_event.c                  | 2 +-
 xen/drivers/passthrough/iommu.c        | 2 +-
 6 files changed, 8 insertions(+), 5 deletions(-)

-- 
2.34.1

[PATCH 1/5] xen/common: page_alloc: Fix MISRA C 2012 Rule 8.7 violation

[Xenia Ragiadakou]

The variables page_offlined_list and page_broken_list are referenced only
in page_alloc.c.
Change their linkage from external to internal by adding the storage-class
specifier static to their definitions.

This patch aims to resolve indirectly a MISRA C 2012 Rule 8.4 violation warning.

Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---
 xen/common/page_alloc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 000ae6b972..fe0e15429a 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -235,9 +235,9 @@ static unsigned int dma_bitsize;
 integer_param("dma_bits", dma_bitsize);
 
 /* Offlined page list, protected by heap_lock. */
-PAGE_LIST_HEAD(page_offlined_list);
+static PAGE_LIST_HEAD(page_offlined_list);
 /* Broken page list, protected by heap_lock. */
-PAGE_LIST_HEAD(page_broken_list);
+static PAGE_LIST_HEAD(page_broken_list);
 
 /*************************
  * BOOT-TIME ALLOCATOR
-- 
2.34.1

[PATCH 2/5] xen/common: vm_event: Fix MISRA C 2012 Rule 8.7 violation

[Xenia Ragiadakou]

The function vm_event_wake() is referenced only in vm_event.c.
Change the linkage of the function from external to internal by adding
the storage-class specifier static to the function definition.

This patch aims to resolve indirectly a MISRA C 2012 Rule 8.4 violation warning.

Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---
 xen/common/vm_event.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
index 0b99a6ea72..ecf49c38a9 100644
--- a/xen/common/vm_event.c
+++ b/xen/common/vm_event.c
@@ -173,7 +173,7 @@ static void vm_event_wake_queued(struct domain *d, struct vm_event_domain *ved)
  * call vm_event_wake() again, ensuring that any blocked vCPUs will get
  * unpaused once all the queued vCPUs have made it through.
  */
-void vm_event_wake(struct domain *d, struct vm_event_domain *ved)
+static void vm_event_wake(struct domain *d, struct vm_event_domain *ved)
 {
     if ( !list_empty(&ved->wq.list) )
         vm_event_wake_queued(d, ved);
-- 
2.34.1

[PATCH 3/5] xen/drivers: iommu: Fix MISRA C 2012 Rule 8.7 violation

[Xenia Ragiadakou]

The variable iommu_crash_disable is referenced only in one translation unit.
Change its linkage from external to internal by adding the storage-class
specifier static to its definition.

This patch aims to resolve indirectly a MISRA C 2012 Rule 8.4 violation warning.

Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---
 xen/drivers/passthrough/iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 75df3aa8dd..77f64e6174 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -29,7 +29,7 @@ bool_t __initdata iommu_enable = 1;
 bool_t __read_mostly iommu_enabled;
 bool_t __read_mostly force_iommu;
 bool_t __read_mostly iommu_verbose;
-bool_t __read_mostly iommu_crash_disable;
+static bool_t __read_mostly iommu_crash_disable;
 
 #define IOMMU_quarantine_none         0 /* aka false */
 #define IOMMU_quarantine_basic        1 /* aka true */
-- 
2.34.1

[PATCH 4/5] xen/sched: credit: Fix MISRA C 2012 Rule 8.7 violation

[Xenia Ragiadakou]

The per-cpu variable last_tickle_cpu is referenced only in credit.c.
Change its linkage from external to internal by adding the storage-class
specifier static to its definitions.

This patch aims to resolve indirectly a MISRA C 2012 Rule 8.4 violation warning.

Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---
 xen/common/sched/credit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/common/sched/credit.c b/xen/common/sched/credit.c
index 4d3bd8cba6..47945c2834 100644
--- a/xen/common/sched/credit.c
+++ b/xen/common/sched/credit.c
@@ -348,7 +348,7 @@ static void burn_credits(struct csched_unit *svc, s_time_t now)
 static bool __read_mostly opt_tickle_one_idle = true;
 boolean_param("tickle_one_idle_cpu", opt_tickle_one_idle);
 
-DEFINE_PER_CPU(unsigned int, last_tickle_cpu);
+static DEFINE_PER_CPU(unsigned int, last_tickle_cpu);
 
 static inline void __runq_tickle(const struct csched_unit *new)
 {
-- 
2.34.1

[PATCH 5/5] xen/arm64: traps: Fix MISRA C 2012 Rule 8.4 violations

[Xenia Ragiadakou]

Add a function prototype for do_bad_mode() in <asm/arm64/traps.h> and include
header <asm/traps.h> in traps.c, so that the declarations of the functions
do_bad_mode() and finalize_instr_emulation(), which have external linkage,
are visible before the function definitions.

Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---
 xen/arch/arm/arm64/traps.c             | 1 +
 xen/arch/arm/include/asm/arm64/traps.h | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/xen/arch/arm/arm64/traps.c b/xen/arch/arm/arm64/traps.c
index 3f8858acec..a995ad7c2c 100644
--- a/xen/arch/arm/arm64/traps.c
+++ b/xen/arch/arm/arm64/traps.c
@@ -22,6 +22,7 @@
 #include <asm/hsr.h>
 #include <asm/system.h>
 #include <asm/processor.h>
+#include <asm/traps.h>
 
 #include <public/xen.h>
 
diff --git a/xen/arch/arm/include/asm/arm64/traps.h b/xen/arch/arm/include/asm/arm64/traps.h
index 2379b578cb..a347cb13d6 100644
--- a/xen/arch/arm/include/asm/arm64/traps.h
+++ b/xen/arch/arm/include/asm/arm64/traps.h
@@ -6,6 +6,8 @@ void inject_undef64_exception(struct cpu_user_regs *regs, int instr_len);
 void do_sysreg(struct cpu_user_regs *regs,
                const union hsr hsr);
 
+void do_bad_mode(struct cpu_user_regs *regs, int reason);
+
 #endif /* __ASM_ARM64_TRAPS__ */
 /*
  * Local variables:
-- 
2.34.1

Re: [PATCH 1/5] xen/common: page_alloc: Fix MISRA C 2012 Rule 8.7 violation

[Jan Beulich]

On 26.06.2022 23:11, Xenia Ragiadakou wrote:
> The variables page_offlined_list and page_broken_list are referenced only
> in page_alloc.c.
> Change their linkage from external to internal by adding the storage-class
> specifier static to their definitions.
> 
> This patch aims to resolve indirectly a MISRA C 2012 Rule 8.4 violation warning.
> 
> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH 2/5] xen/common: vm_event: Fix MISRA C 2012 Rule 8.7 violation

[Jan Beulich]

On 26.06.2022 23:11, Xenia Ragiadakou wrote:
> The function vm_event_wake() is referenced only in vm_event.c.
> Change the linkage of the function from external to internal by adding
> the storage-class specifier static to the function definition.
> 
> This patch aims to resolve indirectly a MISRA C 2012 Rule 8.4 violation warning.

Actually also for patch 1 - this is slightly confusing, as the title
talks about 8.7. At first I suspected a typo. May I suggest to add
"also" (which I think could be easily done while committing)?

> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH 3/5] xen/drivers: iommu: Fix MISRA C 2012 Rule 8.7 violation

[Jan Beulich]

On 26.06.2022 23:11, Xenia Ragiadakou wrote:
> The variable iommu_crash_disable is referenced only in one translation unit.
> Change its linkage from external to internal by adding the storage-class
> specifier static to its definition.
> 
> This patch aims to resolve indirectly a MISRA C 2012 Rule 8.4 violation warning.
> 
> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
(with the same remark as on patch 2)

Re: [PATCH 4/5] xen/sched: credit: Fix MISRA C 2012 Rule 8.7 violation

[Jan Beulich]

On 26.06.2022 23:11, Xenia Ragiadakou wrote:
> The per-cpu variable last_tickle_cpu is referenced only in credit.c.
> Change its linkage from external to internal by adding the storage-class
> specifier static to its definitions.
> 
> This patch aims to resolve indirectly a MISRA C 2012 Rule 8.4 violation warning.
> 
> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
(again with the same remark as on patch 2)

Re: [PATCH 2/5] xen/common: vm_event: Fix MISRA C 2012 Rule 8.7 violation

[xenia]

Hi Jan,

On 6/27/22 10:11, Jan Beulich wrote:

> On 26.06.2022 23:11, Xenia Ragiadakou wrote:
>> The function vm_event_wake() is referenced only in vm_event.c.
>> Change the linkage of the function from external to internal by adding
>> the storage-class specifier static to the function definition.
>>
>> This patch aims to resolve indirectly a MISRA C 2012 Rule 8.4 violation warning.
> Actually also for patch 1 - this is slightly confusing, as the title
> talks about 8.7. At first I suspected a typo. May I suggest to add
> "also" (which I think could be easily done while committing)?


This is actually a violation of MISRA C 2012 Rule 8.7 (Advisory), which 
states that functions referenced in only one translation unit should not 
be defined with external linkage.
This violation triggers a MISRA C 2012 Rule 8.4 violation warning, 
because the function is defined with external linkage without a visible 
declaration at the point of definition.
I thought that this does not make it a violation of MISRA C 2012 Rule 8.4.

>> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>

Re: [PATCH 2/5] xen/common: vm_event: Fix MISRA C 2012 Rule 8.7 violation

[Jan Beulich]

On 27.06.2022 09:54, xenia wrote:
> On 6/27/22 10:11, Jan Beulich wrote:
>> On 26.06.2022 23:11, Xenia Ragiadakou wrote:
>>> The function vm_event_wake() is referenced only in vm_event.c.
>>> Change the linkage of the function from external to internal by adding
>>> the storage-class specifier static to the function definition.
>>>
>>> This patch aims to resolve indirectly a MISRA C 2012 Rule 8.4 violation warning.
>> Actually also for patch 1 - this is slightly confusing, as the title
>> talks about 8.7. At first I suspected a typo. May I suggest to add
>> "also" (which I think could be easily done while committing)?
> 
> 
> This is actually a violation of MISRA C 2012 Rule 8.7 (Advisory), which 
> states that functions referenced in only one translation unit should not 
> be defined with external linkage.
> This violation triggers a MISRA C 2012 Rule 8.4 violation warning, 
> because the function is defined with external linkage without a visible 
> declaration at the point of definition.
> I thought that this does not make it a violation of MISRA C 2012 Rule 8.4.

I think this is a violation of both rules. It would be a violation of
only 8.7 if the function had a declaration, but still wasn't used
outside its defining CU.

Jan

Re: [PATCH 2/5] xen/common: vm_event: Fix MISRA C 2012 Rule 8.7 violation

[xenia]

Hi Jan,

On 6/27/22 11:43, Jan Beulich wrote:
> On 27.06.2022 09:54, xenia wrote:
>> On 6/27/22 10:11, Jan Beulich wrote:
>>> On 26.06.2022 23:11, Xenia Ragiadakou wrote:
>>>> The function vm_event_wake() is referenced only in vm_event.c.
>>>> Change the linkage of the function from external to internal by adding
>>>> the storage-class specifier static to the function definition.
>>>>
>>>> This patch aims to resolve indirectly a MISRA C 2012 Rule 8.4 violation warning.
>>> Actually also for patch 1 - this is slightly confusing, as the title
>>> talks about 8.7. At first I suspected a typo. May I suggest to add
>>> "also" (which I think could be easily done while committing)?
>>
>> This is actually a violation of MISRA C 2012 Rule 8.7 (Advisory), which
>> states that functions referenced in only one translation unit should not
>> be defined with external linkage.
>> This violation triggers a MISRA C 2012 Rule 8.4 violation warning,
>> because the function is defined with external linkage without a visible
>> declaration at the point of definition.
>> I thought that this does not make it a violation of MISRA C 2012 Rule 8.4.
> I think this is a violation of both rules. It would be a violation of
> only 8.7 if the function had a declaration, but still wasn't used
> outside its defining CU.

So you are suggesting to change the line
"This patch aims to resolve indirectly a MISRA C 2012 Rule 8.4 violation 
warning."
to
"Also, this patch aims to resolve indirectly a MISRA C 2012 Rule 8.4 
violation warning."

I will wait one more day in case there is some input for the last patch, 
and I will send a v2 with the above changed.

Xenia

Re: [PATCH 5/5] xen/arm64: traps: Fix MISRA C 2012 Rule 8.4 violations

[Bertrand Marquis]

Hi Xenia,

> On 26 Jun 2022, at 22:11, Xenia Ragiadakou <burzalodowa@gmail.com> wrote:
> 
> Add a function prototype for do_bad_mode() in <asm/arm64/traps.h> and include
> header <asm/traps.h> in traps.c, so that the declarations of the functions
> do_bad_mode() and finalize_instr_emulation(), which have external linkage,
> are visible before the function definitions.
> 
> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>

Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand

> ---
> xen/arch/arm/arm64/traps.c             | 1 +
> xen/arch/arm/include/asm/arm64/traps.h | 2 ++
> 2 files changed, 3 insertions(+)
> 
> diff --git a/xen/arch/arm/arm64/traps.c b/xen/arch/arm/arm64/traps.c
> index 3f8858acec..a995ad7c2c 100644
> --- a/xen/arch/arm/arm64/traps.c
> +++ b/xen/arch/arm/arm64/traps.c
> @@ -22,6 +22,7 @@
> #include <asm/hsr.h>
> #include <asm/system.h>
> #include <asm/processor.h>
> +#include <asm/traps.h>
> 
> #include <public/xen.h>
> 
> diff --git a/xen/arch/arm/include/asm/arm64/traps.h b/xen/arch/arm/include/asm/arm64/traps.h
> index 2379b578cb..a347cb13d6 100644
> --- a/xen/arch/arm/include/asm/arm64/traps.h
> +++ b/xen/arch/arm/include/asm/arm64/traps.h
> @@ -6,6 +6,8 @@ void inject_undef64_exception(struct cpu_user_regs *regs, int instr_len);
> void do_sysreg(struct cpu_user_regs *regs,
>                const union hsr hsr);
> 
> +void do_bad_mode(struct cpu_user_regs *regs, int reason);
> +
> #endif /* __ASM_ARM64_TRAPS__ */
> /*
>  * Local variables:
> -- 
> 2.34.1
> 

Re: [PATCH 2/5] xen/common: vm_event: Fix MISRA C 2012 Rule 8.7 violation

[Stefano Stabellini]

On Mon, 27 Jun 2022, Xenia Ragiadakou wrote:
> The function vm_event_wake() is referenced only in vm_event.c.
> Change the linkage of the function from external to internal by adding
> the storage-class specifier static to the function definition.
> 
> This patch aims to resolve indirectly a MISRA C 2012 Rule 8.4 violation warning.
> 
> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>

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


> ---
>  xen/common/vm_event.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
> index 0b99a6ea72..ecf49c38a9 100644
> --- a/xen/common/vm_event.c
> +++ b/xen/common/vm_event.c
> @@ -173,7 +173,7 @@ static void vm_event_wake_queued(struct domain *d, struct vm_event_domain *ved)
>   * call vm_event_wake() again, ensuring that any blocked vCPUs will get
>   * unpaused once all the queued vCPUs have made it through.
>   */
> -void vm_event_wake(struct domain *d, struct vm_event_domain *ved)
> +static void vm_event_wake(struct domain *d, struct vm_event_domain *ved)
>  {
>      if ( !list_empty(&ved->wq.list) )
>          vm_event_wake_queued(d, ved);
> -- 
> 2.34.1
> 
> 

Re: [PATCH 4/5] xen/sched: credit: Fix MISRA C 2012 Rule 8.7 violation

[Stefano Stabellini]

On Mon, 27 Jun 2022, Xenia Ragiadakou wrote:
> The per-cpu variable last_tickle_cpu is referenced only in credit.c.
> Change its linkage from external to internal by adding the storage-class
> specifier static to its definitions.
> 
> This patch aims to resolve indirectly a MISRA C 2012 Rule 8.4 violation warning.
> 
> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>

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


> ---
>  xen/common/sched/credit.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/common/sched/credit.c b/xen/common/sched/credit.c
> index 4d3bd8cba6..47945c2834 100644
> --- a/xen/common/sched/credit.c
> +++ b/xen/common/sched/credit.c
> @@ -348,7 +348,7 @@ static void burn_credits(struct csched_unit *svc, s_time_t now)
>  static bool __read_mostly opt_tickle_one_idle = true;
>  boolean_param("tickle_one_idle_cpu", opt_tickle_one_idle);
>  
> -DEFINE_PER_CPU(unsigned int, last_tickle_cpu);
> +static DEFINE_PER_CPU(unsigned int, last_tickle_cpu);
>  
>  static inline void __runq_tickle(const struct csched_unit *new)
>  {
> -- 
> 2.34.1
> 
>