[PATCH v2] xen/arm: domain: Fix MISRA C 2012 Rule 8.7 violation

[PATCH v2] xen/arm: domain: Fix MISRA C 2012 Rule 8.7 violation

[Xenia Ragiadakou]

The function idle_loop() is referenced only in domain.c.
Change its linkage from external to internal by adding the storage-class
specifier static to its definitions.

Add the function as a 'fake' input operand to the inline assembly statement,
to make the compiler aware that the function is used.
Fake means that the function is not actually used as an operand by the asm code.
That is because there is not a suitable gcc arm32 asm constraint for labels.

Declare return_to_new_vcpu32() and return_to_new_vcpu64() that are also
referenced by this inline asm statement.

Also, this patch resolves indirectly a MISRA C 2012 Rule 8.4 violation warning.

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

Changes in v2:
- remove the 'used' attribute and pass the function as input operand to
the inline asm statement
- declare return_to_new_vcpu32() and return_to_new_vcpu64()

 xen/arch/arm/domain.c              | 2 +-
 xen/arch/arm/include/asm/current.h | 5 ++++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 2f8eaab7b5..780b6bcfaa 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -63,7 +63,7 @@ static void do_idle(void)
     rcu_idle_exit(cpu);
 }
 
-void idle_loop(void)
+static void idle_loop(void)
 {
     unsigned int cpu = smp_processor_id();
 
diff --git a/xen/arch/arm/include/asm/current.h b/xen/arch/arm/include/asm/current.h
index 73e81458e5..225e00af71 100644
--- a/xen/arch/arm/include/asm/current.h
+++ b/xen/arch/arm/include/asm/current.h
@@ -44,8 +44,11 @@ static inline struct cpu_info *get_cpu_info(void)
 
 #define guest_cpu_user_regs() (&get_cpu_info()->guest_cpu_user_regs)
 
+extern void return_to_new_vcpu32(void);
+extern void return_to_new_vcpu64(void);
+
 #define switch_stack_and_jump(stack, fn) do {                           \
-    asm volatile ("mov sp,%0; b " STR(fn) : : "r" (stack) : "memory" ); \
+    asm volatile ("mov sp,%0; b " STR(fn) : : "r" (stack), "X" (fn) : "memory" ); \
     unreachable();                                                      \
 } while ( false )
 
-- 
2.34.1

Re: [PATCH v2] xen/arm: domain: Fix MISRA C 2012 Rule 8.7 violation

[Jan Beulich]

On 28.07.2022 09:57, Xenia Ragiadakou wrote:
> --- a/xen/arch/arm/include/asm/current.h
> +++ b/xen/arch/arm/include/asm/current.h
> @@ -44,8 +44,11 @@ static inline struct cpu_info *get_cpu_info(void)
>  
>  #define guest_cpu_user_regs() (&get_cpu_info()->guest_cpu_user_regs)
>  
> +extern void return_to_new_vcpu32(void);
> +extern void return_to_new_vcpu64(void);

While ultimately it's the Arm maintainers to judge, may I suggest that
these be put in arm/domain.c to limit visibility?

Jan

Re: [PATCH v2] xen/arm: domain: Fix MISRA C 2012 Rule 8.7 violation

[Xenia Ragiadakou]

Hi Jan,

On 7/28/22 12:26, Jan Beulich wrote:
> On 28.07.2022 09:57, Xenia Ragiadakou wrote:
>> --- a/xen/arch/arm/include/asm/current.h
>> +++ b/xen/arch/arm/include/asm/current.h
>> @@ -44,8 +44,11 @@ static inline struct cpu_info *get_cpu_info(void)
>>   
>>   #define guest_cpu_user_regs() (&get_cpu_info()->guest_cpu_user_regs)
>>   
>> +extern void return_to_new_vcpu32(void);
>> +extern void return_to_new_vcpu64(void);
> 
> While ultimately it's the Arm maintainers to judge, may I suggest that
> these be put in arm/domain.c to limit visibility?

I agree with you. Will fix and resend.

-- 
Xenia

Re: [PATCH v2] xen/arm: domain: Fix MISRA C 2012 Rule 8.7 violation

[Julien Grall]

Hi,

On 28/07/2022 10:26, Jan Beulich wrote:
> On 28.07.2022 09:57, Xenia Ragiadakou wrote:
>> --- a/xen/arch/arm/include/asm/current.h
>> +++ b/xen/arch/arm/include/asm/current.h
>> @@ -44,8 +44,11 @@ static inline struct cpu_info *get_cpu_info(void)
>>   
>>   #define guest_cpu_user_regs() (&get_cpu_info()->guest_cpu_user_regs)
>>   
>> +extern void return_to_new_vcpu32(void);
>> +extern void return_to_new_vcpu64(void);
> 
> While ultimately it's the Arm maintainers to judge, may I suggest that
> these be put in arm/domain.c to limit visibility?

In general, I am not in favor of declaring prototype outside of headers. 
That said, I would be okay with it for the two prototypes because:
   1) they are prototypes for assembly functions
   2) declaring in current.h sounds wrong. A better place would be 
domain.h but this would not reduce the visibility too much
   3) this is unlikely to be used by other part of Xen


Cheers,

-- 
Julien Grall

Re: [PATCH v2] xen/arm: domain: Fix MISRA C 2012 Rule 8.7 violation

[Xenia Ragiadakou]

On 7/28/22 16:05, Julien Grall wrote:
> Hi,
> 
> On 28/07/2022 10:26, Jan Beulich wrote:
>> On 28.07.2022 09:57, Xenia Ragiadakou wrote:
>>> --- a/xen/arch/arm/include/asm/current.h
>>> +++ b/xen/arch/arm/include/asm/current.h
>>> @@ -44,8 +44,11 @@ static inline struct cpu_info *get_cpu_info(void)
>>>   #define guest_cpu_user_regs() (&get_cpu_info()->guest_cpu_user_regs)
>>> +extern void return_to_new_vcpu32(void);
>>> +extern void return_to_new_vcpu64(void);
>>
>> While ultimately it's the Arm maintainers to judge, may I suggest that
>> these be put in arm/domain.c to limit visibility?
> 
> In general, I am not in favor of declaring prototype outside of headers. 
> That said, I would be okay with it for the two prototypes because:
>    1) they are prototypes for assembly functions
>    2) declaring in current.h sounds wrong. A better place would be 
> domain.h but this would not reduce the visibility too much
>    3) this is unlikely to be used by other part of Xen

What I will ask is irrelevant to the placement but relevant to the 
declaration itself. I should also have declared them noreturn, right?

-- 
Xenia