[PATCH 3/3] xen/MISRA: Remove nonstandard inline keywords

Andrew Cooper posted 3 patches 10 months ago
[PATCH 3/3] xen/MISRA: Remove nonstandard inline keywords
Posted by Andrew Cooper 10 months ago
The differences between inline, __inline and __inline__ keywords are a
vestigial remnant of older C standards, and in Xen we use inline almost
exclusively.

Replace __inline and __inline__ with regular inline, and remove their
exceptions from the MISRA configuration.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Roberto Bagnara <roberto.bagnara@bugseng.com>
CC: Nicola Vetrini <nicola.vetrini@bugseng.com>
CC: Simone Ballarin <simone.ballarin@bugseng.com>

I'm entirely guessing at the Eclair configuration.
---
 .../eclair_analysis/ECLAIR/toolchain.ecl      |  6 +++---
 docs/misra/C-language-toolchain.rst           |  2 +-
 xen/arch/x86/include/asm/apic.h               | 20 +++++++++----------
 xen/include/acpi/cpufreq/cpufreq.h            |  4 ++--
 xen/include/xen/bitops.h                      |  4 ++--
 xen/include/xen/compiler.h                    |  7 +++----
 6 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/automation/eclair_analysis/ECLAIR/toolchain.ecl b/automation/eclair_analysis/ECLAIR/toolchain.ecl
index e6cd289b5e92..71a1e2cce029 100644
--- a/automation/eclair_analysis/ECLAIR/toolchain.ecl
+++ b/automation/eclair_analysis/ECLAIR/toolchain.ecl
@@ -15,7 +15,7 @@
     _Static_assert: see Section \"2.1 C Language\" of "GCC_MANUAL".
     asm, __asm__: see Sections \"6.48 Alternate Keywords\" and \"6.47 How to Use Inline Assembly Language in C Code\" of "GCC_MANUAL".
     __volatile__: see Sections \"6.48 Alternate Keywords\" and \"6.47.2.1 Volatile\" of "GCC_MANUAL".
-    __const__, __inline__, __inline: see Section \"6.48 Alternate Keywords\" of "GCC_MANUAL".
+    __const__ : see Section \"6.48 Alternate Keywords\" of "GCC_MANUAL".
     typeof, __typeof__: see Section \"6.7 Referring to a Type with typeof\" of "GCC_MANUAL".
     __alignof__, __alignof: see Sections \"6.48 Alternate Keywords\" and \"6.44 Determining the Alignment of Functions, Types or Variables\" of "GCC_MANUAL".
     __attribute__: see Section \"6.39 Attribute Syntax\" of "GCC_MANUAL".
@@ -23,8 +23,8 @@
     __builtin_va_arg: non-documented GCC extension.
     __builtin_offsetof: see Section \"6.53 Support for offsetof\" of "GCC_MANUAL".
 "
--config=STD.tokenext,behavior+={c99, GCC_ARM64, "^(_Static_assert|asm|__asm__|__volatile__|__const__|__inline__|typeof|__typeof__|__alignof__|__attribute__|__builtin_types_compatible_p|__builtin_va_arg|__builtin_offsetof)$"}
--config=STD.tokenext,behavior+={c99, GCC_X86_64, "^(_Static_assert|asm|__asm__|__volatile__|__const__|__inline__|__inline|typeof|__typeof__|__alignof__|__alignof|__attribute__|__builtin_types_compatible_p|__builtin_va_arg|__builtin_offsetof)$"}
+-config=STD.tokenext,behavior+={c99, GCC_ARM64, "^(_Static_assert|asm|__asm__|__volatile__|__const__|typeof|__typeof__|__alignof__|__attribute__|__builtin_types_compatible_p|__builtin_va_arg|__builtin_offsetof)$"}
+-config=STD.tokenext,behavior+={c99, GCC_X86_64, "^(_Static_assert|asm|__asm__|__volatile__|__const__|typeof|__typeof__|__alignof__|__alignof|__attribute__|__builtin_types_compatible_p|__builtin_va_arg|__builtin_offsetof)$"}
 -doc_end
 
 -doc_begin="Non-documented GCC extension."
diff --git a/docs/misra/C-language-toolchain.rst b/docs/misra/C-language-toolchain.rst
index 2866cb191b1a..b7c2000992ac 100644
--- a/docs/misra/C-language-toolchain.rst
+++ b/docs/misra/C-language-toolchain.rst
@@ -84,7 +84,7 @@ The table columns are as follows:
           see Sections "6.48 Alternate Keywords" and "6.47 How to Use Inline Assembly Language in C Code" of GCC_MANUAL.
        __volatile__:
           see Sections "6.48 Alternate Keywords" and "6.47.2.1 Volatile" of GCC_MANUAL.
-       __const__, __inline__, __inline:
+       __const__:
           see Section "6.48 Alternate Keywords" of GCC_MANUAL.
        typeof, __typeof__:
           see Section "6.7 Referring to a Type with typeof" of GCC_MANUAL.
diff --git a/xen/arch/x86/include/asm/apic.h b/xen/arch/x86/include/asm/apic.h
index 486d689478b2..b20fae7ebc6a 100644
--- a/xen/arch/x86/include/asm/apic.h
+++ b/xen/arch/x86/include/asm/apic.h
@@ -49,12 +49,12 @@ const struct genapic *apic_x2apic_probe(void);
  * Basic functions accessing APICs.
  */
 
-static __inline void apic_mem_write(unsigned long reg, u32 v)
+static inline void apic_mem_write(unsigned long reg, u32 v)
 {
 	*((volatile u32 *)(APIC_BASE+reg)) = v;
 }
 
-static __inline u32 apic_mem_read(unsigned long reg)
+static inline u32 apic_mem_read(unsigned long reg)
 {
 	return *((volatile u32 *)(APIC_BASE+reg));
 }
@@ -63,7 +63,7 @@ static __inline u32 apic_mem_read(unsigned long reg)
  * access the 64-bit ICR register.
  */
 
-static __inline void apic_wrmsr(unsigned long reg, uint64_t msr_content)
+static inline void apic_wrmsr(unsigned long reg, uint64_t msr_content)
 {
     if (reg == APIC_DFR || reg == APIC_ID || reg == APIC_LDR ||
         reg == APIC_LVR)
@@ -72,7 +72,7 @@ static __inline void apic_wrmsr(unsigned long reg, uint64_t msr_content)
     wrmsrl(MSR_X2APIC_FIRST + (reg >> 4), msr_content);
 }
 
-static __inline uint64_t apic_rdmsr(unsigned long reg)
+static inline uint64_t apic_rdmsr(unsigned long reg)
 {
     uint64_t msr_content;
 
@@ -83,7 +83,7 @@ static __inline uint64_t apic_rdmsr(unsigned long reg)
     return msr_content;
 }
 
-static __inline void apic_write(unsigned long reg, u32 v)
+static inline void apic_write(unsigned long reg, u32 v)
 {
 
     if ( x2apic_enabled )
@@ -92,7 +92,7 @@ static __inline void apic_write(unsigned long reg, u32 v)
         apic_mem_write(reg, v);
 }
 
-static __inline u32 apic_read(unsigned long reg)
+static inline u32 apic_read(unsigned long reg)
 {
     if ( x2apic_enabled )
         return apic_rdmsr(reg);
@@ -100,7 +100,7 @@ static __inline u32 apic_read(unsigned long reg)
         return apic_mem_read(reg);
 }
 
-static __inline u64 apic_icr_read(void)
+static inline u64 apic_icr_read(void)
 {
     u32 lo, hi;
 
@@ -115,7 +115,7 @@ static __inline u64 apic_icr_read(void)
     return ((u64)lo) | (((u64)hi) << 32);
 }
 
-static __inline void apic_icr_write(u32 low, u32 dest)
+static inline void apic_icr_write(u32 low, u32 dest)
 {
     if ( x2apic_enabled )
         apic_wrmsr(APIC_ICR, low | ((uint64_t)dest << 32));
@@ -126,13 +126,13 @@ static __inline void apic_icr_write(u32 low, u32 dest)
     }
 }
 
-static __inline bool apic_isr_read(uint8_t vector)
+static inline bool apic_isr_read(uint8_t vector)
 {
     return (apic_read(APIC_ISR + ((vector & ~0x1f) >> 1)) >>
             (vector & 0x1f)) & 1;
 }
 
-static __inline u32 get_apic_id(void) /* Get the physical APIC id */
+static inline u32 get_apic_id(void)
 {
     u32 id = apic_read(APIC_ID);
     return x2apic_enabled ? id : GET_xAPIC_ID(id);
diff --git a/xen/include/acpi/cpufreq/cpufreq.h b/xen/include/acpi/cpufreq/cpufreq.h
index b0c860f0ec21..3456d4c95f98 100644
--- a/xen/include/acpi/cpufreq/cpufreq.h
+++ b/xen/include/acpi/cpufreq/cpufreq.h
@@ -148,7 +148,7 @@ extern int cpufreq_driver_getavg(unsigned int cpu, unsigned int flag);
 extern int cpufreq_update_turbo(int cpuid, int new_state);
 extern int cpufreq_get_turbo_status(int cpuid);
 
-static __inline__ int 
+static inline int
 __cpufreq_governor(struct cpufreq_policy *policy, unsigned int event)
 {
     return policy->governor->governor(policy, event);
@@ -179,7 +179,7 @@ extern struct cpufreq_driver cpufreq_driver;
 
 int cpufreq_register_driver(const struct cpufreq_driver *);
 
-static __inline__
+static inline
 void cpufreq_verify_within_limits(struct cpufreq_policy *policy,
                                   unsigned int min, unsigned int max)
 {
diff --git a/xen/include/xen/bitops.h b/xen/include/xen/bitops.h
index edd6817d5356..a88d45475c40 100644
--- a/xen/include/xen/bitops.h
+++ b/xen/include/xen/bitops.h
@@ -127,7 +127,7 @@ static inline int generic_fls64(__u64 x)
 # endif
 #endif
 
-static __inline__ int get_bitmask_order(unsigned int count)
+static inline int get_bitmask_order(unsigned int count)
 {
     int order;
     
@@ -135,7 +135,7 @@ static __inline__ int get_bitmask_order(unsigned int count)
     return order;   /* We could be slightly more clever with -1 here... */
 }
 
-static __inline__ int get_count_order(unsigned int count)
+static inline int get_count_order(unsigned int count)
 {
     int order;
 
diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h
index 04b8bc18df0e..16d554f2a593 100644
--- a/xen/include/xen/compiler.h
+++ b/xen/include/xen/compiler.h
@@ -20,9 +20,8 @@
 #define likely(x)     __builtin_expect(!!(x),1)
 #define unlikely(x)   __builtin_expect(!!(x),0)
 
-#define inline        __inline__
-#define always_inline __inline__ __attribute__ ((__always_inline__))
-#define gnu_inline    __inline__ __attribute__ ((__gnu_inline__))
+#define always_inline inline __attribute__((__always_inline__))
+#define gnu_inline    inline __attribute__((__gnu_inline__))
 #define noinline      __attribute__((__noinline__))
 
 #define noreturn      __attribute__((__noreturn__))
@@ -83,7 +82,7 @@
  * inline functions not expanded inline get placed in .init.text.
  */
 #include <xen/init.h>
-#define __inline__ __inline__ __init
+#define inline inline __init
 #endif
 
 #define __attribute_pure__  __attribute__((__pure__))
-- 
2.30.2


Re: [PATCH 3/3] xen/MISRA: Remove nonstandard inline keywords
Posted by Stefano Stabellini 10 months ago
On Wed, 22 Nov 2023, Andrew Cooper wrote:
> The differences between inline, __inline and __inline__ keywords are a
> vestigial remnant of older C standards, and in Xen we use inline almost
> exclusively.
> 
> Replace __inline and __inline__ with regular inline, and remove their
> exceptions from the MISRA configuration.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> diff --git a/docs/misra/C-language-toolchain.rst b/docs/misra/C-language-toolchain.rst
> index 2866cb191b1a..b7c2000992ac 100644
> --- a/docs/misra/C-language-toolchain.rst
> +++ b/docs/misra/C-language-toolchain.rst
> @@ -84,7 +84,7 @@ The table columns are as follows:
>            see Sections "6.48 Alternate Keywords" and "6.47 How to Use Inline Assembly Language in C Code" of GCC_MANUAL.
>         __volatile__:
>            see Sections "6.48 Alternate Keywords" and "6.47.2.1 Volatile" of GCC_MANUAL.
> -       __const__, __inline__, __inline:
> +       __const__:
>            see Section "6.48 Alternate Keywords" of GCC_MANUAL.
>         typeof, __typeof__:
>            see Section "6.7 Referring to a Type with typeof" of GCC_MANUAL.

Asking the Bugseng guys as well, do we need to add to
C-language-toolchain.rst:
inline __attribute__((__always_inline__))
inline __attribute__((__gnu_inline__))

Given that the problem was also present before this patch:

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


> diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h
> index 04b8bc18df0e..16d554f2a593 100644
> --- a/xen/include/xen/compiler.h
> +++ b/xen/include/xen/compiler.h
> @@ -20,9 +20,8 @@
>  #define likely(x)     __builtin_expect(!!(x),1)
>  #define unlikely(x)   __builtin_expect(!!(x),0)
>  
> -#define inline        __inline__
> -#define always_inline __inline__ __attribute__ ((__always_inline__))
> -#define gnu_inline    __inline__ __attribute__ ((__gnu_inline__))
> +#define always_inline inline __attribute__((__always_inline__))
> +#define gnu_inline    inline __attribute__((__gnu_inline__))
>  #define noinline      __attribute__((__noinline__))
>  
>  #define noreturn      __attribute__((__noreturn__))

This is where they are used.


> @@ -83,7 +82,7 @@
>   * inline functions not expanded inline get placed in .init.text.
>   */
>  #include <xen/init.h>
> -#define __inline__ __inline__ __init
> +#define inline inline __init
>  #endif
>  
>  #define __attribute_pure__  __attribute__((__pure__))
> -- 
> 2.30.2
> 
>
Re: [PATCH 3/3] xen/MISRA: Remove nonstandard inline keywords
Posted by Andrew Cooper 10 months ago
On 22/11/2023 10:13 pm, Stefano Stabellini wrote:
> On Wed, 22 Nov 2023, Andrew Cooper wrote:
>> The differences between inline, __inline and __inline__ keywords are a
>> vestigial remnant of older C standards, and in Xen we use inline almost
>> exclusively.
>>
>> Replace __inline and __inline__ with regular inline, and remove their
>> exceptions from the MISRA configuration.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>
>> diff --git a/docs/misra/C-language-toolchain.rst b/docs/misra/C-language-toolchain.rst
>> index 2866cb191b1a..b7c2000992ac 100644
>> --- a/docs/misra/C-language-toolchain.rst
>> +++ b/docs/misra/C-language-toolchain.rst
>> @@ -84,7 +84,7 @@ The table columns are as follows:
>>            see Sections "6.48 Alternate Keywords" and "6.47 How to Use Inline Assembly Language in C Code" of GCC_MANUAL.
>>         __volatile__:
>>            see Sections "6.48 Alternate Keywords" and "6.47.2.1 Volatile" of GCC_MANUAL.
>> -       __const__, __inline__, __inline:
>> +       __const__:
>>            see Section "6.48 Alternate Keywords" of GCC_MANUAL.
>>         typeof, __typeof__:
>>            see Section "6.7 Referring to a Type with typeof" of GCC_MANUAL.
> Asking the Bugseng guys as well, do we need to add to
> C-language-toolchain.rst:
> inline __attribute__((__always_inline__))
> inline __attribute__((__gnu_inline__))

__attribute__ itself is in the list of permitted non-standard tokens, in
both files.

However, neither file has anything concerning the parameter(s) to the
__attribute__, and we do use an awful lot of them.

If they want discussing, then that's going to be a lot of work.

> Given that the problem was also present before this patch:
>
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

Thanks.
Re: [PATCH 3/3] xen/MISRA: Remove nonstandard inline keywords
Posted by Nicola Vetrini 10 months ago
On 2023-11-22 23:20, Andrew Cooper wrote:
> On 22/11/2023 10:13 pm, Stefano Stabellini wrote:
>> On Wed, 22 Nov 2023, Andrew Cooper wrote:
>>> The differences between inline, __inline and __inline__ keywords are 
>>> a
>>> vestigial remnant of older C standards, and in Xen we use inline 
>>> almost
>>> exclusively.
>>> 
>>> Replace __inline and __inline__ with regular inline, and remove their
>>> exceptions from the MISRA configuration.
>>> 
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> 
>>> diff --git a/docs/misra/C-language-toolchain.rst 
>>> b/docs/misra/C-language-toolchain.rst
>>> index 2866cb191b1a..b7c2000992ac 100644
>>> --- a/docs/misra/C-language-toolchain.rst
>>> +++ b/docs/misra/C-language-toolchain.rst
>>> @@ -84,7 +84,7 @@ The table columns are as follows:
>>>            see Sections "6.48 Alternate Keywords" and "6.47 How to 
>>> Use Inline Assembly Language in C Code" of GCC_MANUAL.
>>>         __volatile__:
>>>            see Sections "6.48 Alternate Keywords" and "6.47.2.1 
>>> Volatile" of GCC_MANUAL.
>>> -       __const__, __inline__, __inline:
>>> +       __const__:
>>>            see Section "6.48 Alternate Keywords" of GCC_MANUAL.
>>>         typeof, __typeof__:
>>>            see Section "6.7 Referring to a Type with typeof" of 
>>> GCC_MANUAL.
>> Asking the Bugseng guys as well, do we need to add to
>> C-language-toolchain.rst:
>> inline __attribute__((__always_inline__))
>> inline __attribute__((__gnu_inline__))
> 
> __attribute__ itself is in the list of permitted non-standard tokens, 
> in
> both files.
> 
> However, neither file has anything concerning the parameter(s) to the
> __attribute__, and we do use an awful lot of them.
> 
> If they want discussing, then that's going to be a lot of work.
> 

Just __attribute__ is fine, since we point to
Section "6.39 Attribute Syntax" of GCC_MANUAL.
which describes the syntax for the token and gives pointers to other 
relevant sections of the manual.

>> Given that the problem was also present before this patch:
>> 
>> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> 
> Thanks.

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
Re: [PATCH 3/3] xen/MISRA: Remove nonstandard inline keywords
Posted by Simone Ballarin 10 months ago
On 22/11/23 15:27, Andrew Cooper wrote:
> The differences between inline, __inline and __inline__ keywords are a
> vestigial remnant of older C standards, and in Xen we use inline almost
> exclusively.
> 
> Replace __inline and __inline__ with regular inline, and remove their
> exceptions from the MISRA configuration.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> --- > CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Wei Liu <wl@xen.org>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Roberto Bagnara <roberto.bagnara@bugseng.com>
> CC: Nicola Vetrini <nicola.vetrini@bugseng.com>
> CC: Simone Ballarin <simone.ballarin@bugseng.com>
> 
> I'm entirely guessing at the Eclair configuration.
> ---
>   .../eclair_analysis/ECLAIR/toolchain.ecl      |  6 +++---
>   docs/misra/C-language-toolchain.rst           |  2 +-
>   xen/arch/x86/include/asm/apic.h               | 20 +++++++++----------
>   xen/include/acpi/cpufreq/cpufreq.h            |  4 ++--
>   xen/include/xen/bitops.h                      |  4 ++--
>   xen/include/xen/compiler.h                    |  7 +++----
>   6 files changed, 21 insertions(+), 22 deletions(-)
> 
> diff --git a/automation/eclair_analysis/ECLAIR/toolchain.ecl b/automation/eclair_analysis/ECLAIR/toolchain.ecl
> index e6cd289b5e92..71a1e2cce029 100644
> --- a/automation/eclair_analysis/ECLAIR/toolchain.ecl
> +++ b/automation/eclair_analysis/ECLAIR/toolchain.ecl
> @@ -15,7 +15,7 @@
>       _Static_assert: see Section \"2.1 C Language\" of "GCC_MANUAL".
>       asm, __asm__: see Sections \"6.48 Alternate Keywords\" and \"6.47 How to Use Inline Assembly Language in C Code\" of "GCC_MANUAL".
>       __volatile__: see Sections \"6.48 Alternate Keywords\" and \"6.47.2.1 Volatile\" of "GCC_MANUAL".
> -    __const__, __inline__, __inline: see Section \"6.48 Alternate Keywords\" of "GCC_MANUAL".
> +    __const__ : see Section \"6.48 Alternate Keywords\" of "GCC_MANUAL".
>       typeof, __typeof__: see Section \"6.7 Referring to a Type with typeof\" of "GCC_MANUAL".
>       __alignof__, __alignof: see Sections \"6.48 Alternate Keywords\" and \"6.44 Determining the Alignment of Functions, Types or Variables\" of "GCC_MANUAL".
>       __attribute__: see Section \"6.39 Attribute Syntax\" of "GCC_MANUAL".
> @@ -23,8 +23,8 @@
>       __builtin_va_arg: non-documented GCC extension.
>       __builtin_offsetof: see Section \"6.53 Support for offsetof\" of "GCC_MANUAL".
>   "
> --config=STD.tokenext,behavior+={c99, GCC_ARM64, "^(_Static_assert|asm|__asm__|__volatile__|__const__|__inline__|typeof|__typeof__|__alignof__|__attribute__|__builtin_types_compatible_p|__builtin_va_arg|__builtin_offsetof)$"}
> --config=STD.tokenext,behavior+={c99, GCC_X86_64, "^(_Static_assert|asm|__asm__|__volatile__|__const__|__inline__|__inline|typeof|__typeof__|__alignof__|__alignof|__attribute__|__builtin_types_compatible_p|__builtin_va_arg|__builtin_offsetof)$"}
> +-config=STD.tokenext,behavior+={c99, GCC_ARM64, "^(_Static_assert|asm|__asm__|__volatile__|__const__|typeof|__typeof__|__alignof__|__attribute__|__builtin_types_compatible_p|__builtin_va_arg|__builtin_offsetof)$"}
> +-config=STD.tokenext,behavior+={c99, GCC_X86_64, "^(_Static_assert|asm|__asm__|__volatile__|__const__|typeof|__typeof__|__alignof__|__alignof|__attribute__|__builtin_types_compatible_p|__builtin_va_arg|__builtin_offsetof)$"}
>   -doc_end
>   
>   -doc_begin="Non-documented GCC extension."
> diff --git a/docs/misra/C-language-toolchain.rst b/docs/misra/C-language-toolchain.rst
> index 2866cb191b1a..b7c2000992ac 100644
> --- a/docs/misra/C-language-toolchain.rst
> +++ b/docs/misra/C-language-toolchain.rst
> @@ -84,7 +84,7 @@ The table columns are as follows:
>             see Sections "6.48 Alternate Keywords" and "6.47 How to Use Inline Assembly Language in C Code" of GCC_MANUAL.
>          __volatile__:
>             see Sections "6.48 Alternate Keywords" and "6.47.2.1 Volatile" of GCC_MANUAL.
> -       __const__, __inline__, __inline:
> +       __const__:
>             see Section "6.48 Alternate Keywords" of GCC_MANUAL.
>          typeof, __typeof__:
>             see Section "6.7 Referring to a Type with typeof" of GCC_MANUAL.
> diff --git a/xen/arch/x86/include/asm/apic.h b/xen/arch/x86/include/asm/apic.h
> index 486d689478b2..b20fae7ebc6a 100644
> --- a/xen/arch/x86/include/asm/apic.h
> +++ b/xen/arch/x86/include/asm/apic.h
> @@ -49,12 +49,12 @@ const struct genapic *apic_x2apic_probe(void);
>    * Basic functions accessing APICs.
>    */
>   
> -static __inline void apic_mem_write(unsigned long reg, u32 v)
> +static inline void apic_mem_write(unsigned long reg, u32 v)
>   {
>   	*((volatile u32 *)(APIC_BASE+reg)) = v;
>   }
>   
> -static __inline u32 apic_mem_read(unsigned long reg)
> +static inline u32 apic_mem_read(unsigned long reg)
>   {
>   	return *((volatile u32 *)(APIC_BASE+reg));
>   }
> @@ -63,7 +63,7 @@ static __inline u32 apic_mem_read(unsigned long reg)
>    * access the 64-bit ICR register.
>    */
>   
> -static __inline void apic_wrmsr(unsigned long reg, uint64_t msr_content)
> +static inline void apic_wrmsr(unsigned long reg, uint64_t msr_content)
>   {
>       if (reg == APIC_DFR || reg == APIC_ID || reg == APIC_LDR ||
>           reg == APIC_LVR)
> @@ -72,7 +72,7 @@ static __inline void apic_wrmsr(unsigned long reg, uint64_t msr_content)
>       wrmsrl(MSR_X2APIC_FIRST + (reg >> 4), msr_content);
>   }
>   
> -static __inline uint64_t apic_rdmsr(unsigned long reg)
> +static inline uint64_t apic_rdmsr(unsigned long reg)
>   {
>       uint64_t msr_content;
>   
> @@ -83,7 +83,7 @@ static __inline uint64_t apic_rdmsr(unsigned long reg)
>       return msr_content;
>   }
>   
> -static __inline void apic_write(unsigned long reg, u32 v)
> +static inline void apic_write(unsigned long reg, u32 v)
>   {
>   
>       if ( x2apic_enabled )
> @@ -92,7 +92,7 @@ static __inline void apic_write(unsigned long reg, u32 v)
>           apic_mem_write(reg, v);
>   }
>   
> -static __inline u32 apic_read(unsigned long reg)
> +static inline u32 apic_read(unsigned long reg)
>   {
>       if ( x2apic_enabled )
>           return apic_rdmsr(reg);
> @@ -100,7 +100,7 @@ static __inline u32 apic_read(unsigned long reg)
>           return apic_mem_read(reg);
>   }
>   
> -static __inline u64 apic_icr_read(void)
> +static inline u64 apic_icr_read(void)
>   {
>       u32 lo, hi;
>   
> @@ -115,7 +115,7 @@ static __inline u64 apic_icr_read(void)
>       return ((u64)lo) | (((u64)hi) << 32);
>   }
>   
> -static __inline void apic_icr_write(u32 low, u32 dest)
> +static inline void apic_icr_write(u32 low, u32 dest)
>   {
>       if ( x2apic_enabled )
>           apic_wrmsr(APIC_ICR, low | ((uint64_t)dest << 32));
> @@ -126,13 +126,13 @@ static __inline void apic_icr_write(u32 low, u32 dest)
>       }
>   }
>   
> -static __inline bool apic_isr_read(uint8_t vector)
> +static inline bool apic_isr_read(uint8_t vector)
>   {
>       return (apic_read(APIC_ISR + ((vector & ~0x1f) >> 1)) >>
>               (vector & 0x1f)) & 1;
>   }
>   
> -static __inline u32 get_apic_id(void) /* Get the physical APIC id */
> +static inline u32 get_apic_id(void)
>   {
>       u32 id = apic_read(APIC_ID);
>       return x2apic_enabled ? id : GET_xAPIC_ID(id);
> diff --git a/xen/include/acpi/cpufreq/cpufreq.h b/xen/include/acpi/cpufreq/cpufreq.h
> index b0c860f0ec21..3456d4c95f98 100644
> --- a/xen/include/acpi/cpufreq/cpufreq.h
> +++ b/xen/include/acpi/cpufreq/cpufreq.h
> @@ -148,7 +148,7 @@ extern int cpufreq_driver_getavg(unsigned int cpu, unsigned int flag);
>   extern int cpufreq_update_turbo(int cpuid, int new_state);
>   extern int cpufreq_get_turbo_status(int cpuid);
>   
> -static __inline__ int
> +static inline int
>   __cpufreq_governor(struct cpufreq_policy *policy, unsigned int event)
>   {
>       return policy->governor->governor(policy, event);
> @@ -179,7 +179,7 @@ extern struct cpufreq_driver cpufreq_driver;
>   
>   int cpufreq_register_driver(const struct cpufreq_driver *);
>   
> -static __inline__
> +static inline
>   void cpufreq_verify_within_limits(struct cpufreq_policy *policy,
>                                     unsigned int min, unsigned int max)
>   {
> diff --git a/xen/include/xen/bitops.h b/xen/include/xen/bitops.h
> index edd6817d5356..a88d45475c40 100644
> --- a/xen/include/xen/bitops.h
> +++ b/xen/include/xen/bitops.h
> @@ -127,7 +127,7 @@ static inline int generic_fls64(__u64 x)
>   # endif
>   #endif
>   
> -static __inline__ int get_bitmask_order(unsigned int count)
> +static inline int get_bitmask_order(unsigned int count)
>   {
>       int order;
>       
> @@ -135,7 +135,7 @@ static __inline__ int get_bitmask_order(unsigned int count)
>       return order;   /* We could be slightly more clever with -1 here... */
>   }
>   
> -static __inline__ int get_count_order(unsigned int count)
> +static inline int get_count_order(unsigned int count)
>   {
>       int order;
>   
> diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h
> index 04b8bc18df0e..16d554f2a593 100644
> --- a/xen/include/xen/compiler.h
> +++ b/xen/include/xen/compiler.h
> @@ -20,9 +20,8 @@
>   #define likely(x)     __builtin_expect(!!(x),1)
>   #define unlikely(x)   __builtin_expect(!!(x),0)
>   
> -#define inline        __inline__
> -#define always_inline __inline__ __attribute__ ((__always_inline__))
> -#define gnu_inline    __inline__ __attribute__ ((__gnu_inline__))
> +#define always_inline inline __attribute__((__always_inline__))
> +#define gnu_inline    inline __attribute__((__gnu_inline__))
>   #define noinline      __attribute__((__noinline__))
>   
>   #define noreturn      __attribute__((__noreturn__))
> @@ -83,7 +82,7 @@
>    * inline functions not expanded inline get placed in .init.text.
>    */
>   #include <xen/init.h>
> -#define __inline__ __inline__ __init
> +#define inline inline __init
>   #endif
>   
>   #define __attribute_pure__  __attribute__((__pure__))

Reviewed-by: Simone Ballarin <simone.ballarin@bugseng.com>

-- 
Simone Ballarin, M.Sc.

Field Application Engineer, BUGSENG (https://bugseng.com)


Re: [PATCH 3/3] xen/MISRA: Remove nonstandard inline keywords
Posted by Nicola Vetrini 10 months ago
On 2023-11-22 15:27, Andrew Cooper wrote:
> The differences between inline, __inline and __inline__ keywords are a
> vestigial remnant of older C standards, and in Xen we use inline almost
> exclusively.
> 
> Replace __inline and __inline__ with regular inline, and remove their
> exceptions from the MISRA configuration.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Wei Liu <wl@xen.org>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Roberto Bagnara <roberto.bagnara@bugseng.com>
> CC: Nicola Vetrini <nicola.vetrini@bugseng.com>
> CC: Simone Ballarin <simone.ballarin@bugseng.com>
> 
> I'm entirely guessing at the Eclair configuration.
> ---

The configuration changes are ok. One observation below.

>  .../eclair_analysis/ECLAIR/toolchain.ecl      |  6 +++---
>  docs/misra/C-language-toolchain.rst           |  2 +-
>  xen/arch/x86/include/asm/apic.h               | 20 +++++++++----------
>  xen/include/acpi/cpufreq/cpufreq.h            |  4 ++--
>  xen/include/xen/bitops.h                      |  4 ++--
>  xen/include/xen/compiler.h                    |  7 +++----
>  6 files changed, 21 insertions(+), 22 deletions(-)
> 
> diff --git a/automation/eclair_analysis/ECLAIR/toolchain.ecl 
> b/automation/eclair_analysis/ECLAIR/toolchain.ecl
> index e6cd289b5e92..71a1e2cce029 100644
> --- a/automation/eclair_analysis/ECLAIR/toolchain.ecl
> +++ b/automation/eclair_analysis/ECLAIR/toolchain.ecl
> @@ -15,7 +15,7 @@
>      _Static_assert: see Section \"2.1 C Language\" of "GCC_MANUAL".
>      asm, __asm__: see Sections \"6.48 Alternate Keywords\" and \"6.47 
> How to Use Inline Assembly Language in C Code\" of "GCC_MANUAL".
>      __volatile__: see Sections \"6.48 Alternate Keywords\" and 
> \"6.47.2.1 Volatile\" of "GCC_MANUAL".
> -    __const__, __inline__, __inline: see Section \"6.48 Alternate 
> Keywords\" of "GCC_MANUAL".
> +    __const__ : see Section \"6.48 Alternate Keywords\" of 
> "GCC_MANUAL".
>      typeof, __typeof__: see Section \"6.7 Referring to a Type with 
> typeof\" of "GCC_MANUAL".
>      __alignof__, __alignof: see Sections \"6.48 Alternate Keywords\" 
> and \"6.44 Determining the Alignment of Functions, Types or Variables\" 
> of "GCC_MANUAL".
>      __attribute__: see Section \"6.39 Attribute Syntax\" of 
> "GCC_MANUAL".
> @@ -23,8 +23,8 @@
>      __builtin_va_arg: non-documented GCC extension.
>      __builtin_offsetof: see Section \"6.53 Support for offsetof\" of 
> "GCC_MANUAL".
>  "
> --config=STD.tokenext,behavior+={c99, GCC_ARM64, 
> "^(_Static_assert|asm|__asm__|__volatile__|__const__|__inline__|typeof|__typeof__|__alignof__|__attribute__|__builtin_types_compatible_p|__builtin_va_arg|__builtin_offsetof)$"}
> --config=STD.tokenext,behavior+={c99, GCC_X86_64, 
> "^(_Static_assert|asm|__asm__|__volatile__|__const__|__inline__|__inline|typeof|__typeof__|__alignof__|__alignof|__attribute__|__builtin_types_compatible_p|__builtin_va_arg|__builtin_offsetof)$"}
> +-config=STD.tokenext,behavior+={c99, GCC_ARM64, 
> "^(_Static_assert|asm|__asm__|__volatile__|__const__|typeof|__typeof__|__alignof__|__attribute__|__builtin_types_compatible_p|__builtin_va_arg|__builtin_offsetof)$"}
> +-config=STD.tokenext,behavior+={c99, GCC_X86_64, 
> "^(_Static_assert|asm|__asm__|__volatile__|__const__|typeof|__typeof__|__alignof__|__alignof|__attribute__|__builtin_types_compatible_p|__builtin_va_arg|__builtin_offsetof)$"}
>  -doc_end
> 
>  -doc_begin="Non-documented GCC extension."
> diff --git a/docs/misra/C-language-toolchain.rst 
> b/docs/misra/C-language-toolchain.rst
> index 2866cb191b1a..b7c2000992ac 100644
> --- a/docs/misra/C-language-toolchain.rst
> +++ b/docs/misra/C-language-toolchain.rst
> @@ -84,7 +84,7 @@ The table columns are as follows:
>            see Sections "6.48 Alternate Keywords" and "6.47 How to Use 
> Inline Assembly Language in C Code" of GCC_MANUAL.
>         __volatile__:
>            see Sections "6.48 Alternate Keywords" and "6.47.2.1 
> Volatile" of GCC_MANUAL.
> -       __const__, __inline__, __inline:
> +       __const__:
>            see Section "6.48 Alternate Keywords" of GCC_MANUAL.
>         typeof, __typeof__:
>            see Section "6.7 Referring to a Type with typeof" of 
> GCC_MANUAL.

> diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h
> index 04b8bc18df0e..16d554f2a593 100644
> --- a/xen/include/xen/compiler.h
> +++ b/xen/include/xen/compiler.h
> @@ -20,9 +20,8 @@
>  #define likely(x)     __builtin_expect(!!(x),1)
>  #define unlikely(x)   __builtin_expect(!!(x),0)
> 
> -#define inline        __inline__
> -#define always_inline __inline__ __attribute__ ((__always_inline__))
> -#define gnu_inline    __inline__ __attribute__ ((__gnu_inline__))
> +#define always_inline inline __attribute__((__always_inline__))
> +#define gnu_inline    inline __attribute__((__gnu_inline__))
>  #define noinline      __attribute__((__noinline__))
> 
>  #define noreturn      __attribute__((__noreturn__))
> @@ -83,7 +82,7 @@
>   * inline functions not expanded inline get placed in .init.text.
>   */
>  #include <xen/init.h>
> -#define __inline__ __inline__ __init
> +#define inline inline __init

The violation of Rule 20.4 (A macro shall not be defined with the same 
name as a keyword) is still present due to this macro.


>  #endif
> 
>  #define __attribute_pure__  __attribute__((__pure__))

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

Re: [PATCH 3/3] xen/MISRA: Remove nonstandard inline keywords
Posted by Andrew Cooper 10 months ago
On 22/11/2023 4:39 pm, Nicola Vetrini wrote:
> On 2023-11-22 15:27, Andrew Cooper wrote:
>> The differences between inline, __inline and __inline__ keywords are a
>> vestigial remnant of older C standards, and in Xen we use inline almost
>> exclusively.
>>
>> Replace __inline and __inline__ with regular inline, and remove their
>> exceptions from the MISRA configuration.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Roger Pau Monné <roger.pau@citrix.com>
>> CC: Wei Liu <wl@xen.org>
>> CC: Stefano Stabellini <sstabellini@kernel.org>
>> CC: Roberto Bagnara <roberto.bagnara@bugseng.com>
>> CC: Nicola Vetrini <nicola.vetrini@bugseng.com>
>> CC: Simone Ballarin <simone.ballarin@bugseng.com>
>>
>> I'm entirely guessing at the Eclair configuration.
>> ---
>
> The configuration changes are ok. One observation below.

Thanks.  Can I take that as an Ack/Reviewed-by ?

>> diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h
>> index 04b8bc18df0e..16d554f2a593 100644
>> --- a/xen/include/xen/compiler.h
>> +++ b/xen/include/xen/compiler.h
>> @@ -83,7 +82,7 @@
>>   * inline functions not expanded inline get placed in .init.text.
>>   */
>>  #include <xen/init.h>
>> -#define __inline__ __inline__ __init
>> +#define inline inline __init
>
> The violation of Rule 20.4 (A macro shall not be defined with the same
> name as a keyword) is still present due to this macro.

I was expecting this to come up.

There's a comment half out of context above, but to expand on it, we
have a feature in the build system where if you say obj-y += foo.init.o
then it gets compiled as normal and then all symbols checked for being
in the relevant .init sections.  It's a safeguard around init-only code
ending up in the runtime image (which is good for other goals of safety).

This particular define is necessary to cause out-of-lined static inlines
to end up in the right section, without having to invent a new
__inline_or_init macro and rewriting half the header files in the project.

I think it's going to need a local deviation.  It's deliberate, and all
we're doing is using the inline keyword to hook in an extra __section()
attribute.

~Andrew

Re: [PATCH 3/3] xen/MISRA: Remove nonstandard inline keywords
Posted by Nicola Vetrini 10 months ago
On 2023-11-22 17:46, Andrew Cooper wrote:
> On 22/11/2023 4:39 pm, Nicola Vetrini wrote:
>> On 2023-11-22 15:27, Andrew Cooper wrote:
>>> The differences between inline, __inline and __inline__ keywords are 
>>> a
>>> vestigial remnant of older C standards, and in Xen we use inline 
>>> almost
>>> exclusively.
>>> 
>>> Replace __inline and __inline__ with regular inline, and remove their
>>> exceptions from the MISRA configuration.
>>> 
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> ---
>>> CC: Jan Beulich <JBeulich@suse.com>
>>> CC: Roger Pau Monné <roger.pau@citrix.com>
>>> CC: Wei Liu <wl@xen.org>
>>> CC: Stefano Stabellini <sstabellini@kernel.org>
>>> CC: Roberto Bagnara <roberto.bagnara@bugseng.com>
>>> CC: Nicola Vetrini <nicola.vetrini@bugseng.com>
>>> CC: Simone Ballarin <simone.ballarin@bugseng.com>
>>> 
>>> I'm entirely guessing at the Eclair configuration.
>>> ---
>> 
>> The configuration changes are ok. One observation below.
> 
> Thanks.  Can I take that as an Ack/Reviewed-by ?
> 

I see that Simone already gave one; that should suffice.

>>> diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h
>>> index 04b8bc18df0e..16d554f2a593 100644
>>> --- a/xen/include/xen/compiler.h
>>> +++ b/xen/include/xen/compiler.h
>>> @@ -83,7 +82,7 @@
>>>   * inline functions not expanded inline get placed in .init.text.
>>>   */
>>>  #include <xen/init.h>
>>> -#define __inline__ __inline__ __init
>>> +#define inline inline __init
>> 
>> The violation of Rule 20.4 (A macro shall not be defined with the same
>> name as a keyword) is still present due to this macro.
> 
> I was expecting this to come up.
> 
> There's a comment half out of context above, but to expand on it, we
> have a feature in the build system where if you say obj-y += foo.init.o
> then it gets compiled as normal and then all symbols checked for being
> in the relevant .init sections.  It's a safeguard around init-only code
> ending up in the runtime image (which is good for other goals of 
> safety).
> 
> This particular define is necessary to cause out-of-lined static 
> inlines
> to end up in the right section, without having to invent a new
> __inline_or_init macro and rewriting half the header files in the 
> project.
> 
> I think it's going to need a local deviation.  It's deliberate, and all
> we're doing is using the inline keyword to hook in an extra __section()
> attribute.
> 
> ~Andrew

That's fair. I also agree that an exception for this use of inline can 
be made.
-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)

Re: [PATCH 3/3] xen/MISRA: Remove nonstandard inline keywords
Posted by Andrew Cooper 10 months ago
On 22/11/2023 2:27 pm, Andrew Cooper wrote:
> The differences between inline, __inline and __inline__ keywords are a
> vestigial remnant of older C standards, and in Xen we use inline almost
> exclusively.
>
> Replace __inline and __inline__ with regular inline, and remove their
> exceptions from the MISRA configuration.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Wei Liu <wl@xen.org>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Roberto Bagnara <roberto.bagnara@bugseng.com>
> CC: Nicola Vetrini <nicola.vetrini@bugseng.com>
> CC: Simone Ballarin <simone.ballarin@bugseng.com>
>
> I'm entirely guessing at the Eclair configuration.

https://gitlab.com/xen-project/people/andyhhp/xen/-/jobs/5596360097 is
Eclair running on this change, and it came back green

But I'll have to defer to bugsend to judge whether it was correctly
configured.

~Andrew