[PATCH] x86/cpuid: Change cpuid() from a macro to a static inline

Andrew Cooper posted 1 patch 3 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20240116115838.560473-1-andrew.cooper3@citrix.com
xen/arch/x86/include/asm/processor.h | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
[PATCH] x86/cpuid: Change cpuid() from a macro to a static inline
Posted by Andrew Cooper 3 months, 1 week ago
Fixes MISRA XXX

No functional change.

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: Julien Grall <julien@xen.org>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
CC: Bertrand Marquis <bertrand.marquis@arm.com>
CC: Michal Orzel <michal.orzel@amd.com>
CC: Roberto Bagnara <roberto.bagnara@bugseng.com>
CC: Federico Serafini <federico.serafini@bugseng.com>
CC: consulting@bugseng.com <consulting@bugseng.com>

Can someone please remind me which MISRA rule is the one about macros aliasing
identifiers?
---
 xen/arch/x86/include/asm/processor.h | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/include/asm/processor.h b/xen/arch/x86/include/asm/processor.h
index ff62b080afbf..b227cdee8ef3 100644
--- a/xen/arch/x86/include/asm/processor.h
+++ b/xen/arch/x86/include/asm/processor.h
@@ -126,14 +126,6 @@ static inline int cpu_nr_siblings(unsigned int cpu)
     return cpu_data[cpu].x86_num_siblings;
 }
 
-/*
- * Generic CPUID function
- * clear %ecx since some cpus (Cyrix MII) do not set or clear %ecx
- * resulting in stale register contents being returned.
- */
-#define cpuid(leaf, eax, ebx, ecx, edx)          \
-        cpuid_count(leaf, 0, eax, ebx, ecx, edx)
-
 /* Some CPUID calls want 'count' to be placed in ecx */
 static inline void cpuid_count(
     unsigned int op,
@@ -148,6 +140,21 @@ static inline void cpuid_count(
           : "0" (op), "c" (count) );
 }
 
+/*
+ * Generic CPUID function
+ * clear %ecx since some cpus (Cyrix MII) do not set or clear %ecx
+ * resulting in stale register contents being returned.
+ */
+static inline void cpuid(
+    unsigned int leaf,
+    unsigned int *eax,
+    unsigned int *ebx,
+    unsigned int *ecx,
+    unsigned int *edx)
+{
+    cpuid_count(leaf, 0, eax, ebx, ecx, edx);
+}
+
 /*
  * CPUID functions returning a single datum
  */

base-commit: f3f6c500e2dbd23af77c207e2cf4b496fffa1b0d
-- 
2.30.2


Re: [PATCH] x86/cpuid: Change cpuid() from a macro to a static inline
Posted by Jan Beulich 3 months, 1 week ago
On 16.01.2024 12:58, Andrew Cooper wrote:
> Fixes MISRA XXX

Rule 5.5 if I'm not mistaken; had to look it up for the patch sent
earlier in the day. As to "fixes" - when it's not an actual bug, I had
(successfully) asked the bugseng guys to avoid that term, and instead
use "addresses" or "eliminates a ... violation" or some such.

> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

Jan
Re: [PATCH] x86/cpuid: Change cpuid() from a macro to a static inline
Posted by Federico Serafini 3 months, 1 week ago
On 16/01/24 14:02, Jan Beulich wrote:
> On 16.01.2024 12:58, Andrew Cooper wrote:
>> Fixes MISRA XXX
> 
> Rule 5.5 if I'm not mistaken; had to look it up for the patch sent
> earlier in the day. As to "fixes" - when it's not an actual bug, I had
> (successfully) asked the bugseng guys to avoid that term, and instead
> use "addresses" or "eliminates a ... violation" or some such.
> 
>> No functional change.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
> Jan

I confirm that it is Rule 5.5.

I would like to point out that although the patch fixes violations of
Rule 5.5, it introduces new violations of Rule 5.3 "An identifier 
declared in an inner scope shall not hide an identifier declared in an 
outer scope": cpuid is used also as an identifier for some formal 
arguments (the pipeline does not fail because Rule 5.3 is not tagged
as "clean" and the introduction of new violations does not cause
a failure).
A solution could be to rename the function adding a prefix or a suffix
to its name.

-- 
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)
Re: [PATCH] x86/cpuid: Change cpuid() from a macro to a static inline
Posted by Andrew Cooper 3 months, 1 week ago
On 16/01/2024 1:02 pm, Jan Beulich wrote:
> On 16.01.2024 12:58, Andrew Cooper wrote:
>> Fixes MISRA XXX
> Rule 5.5 if I'm not mistaken; had to look it up for the patch sent
> earlier in the day. As to "fixes" - when it's not an actual bug, I had
> (successfully) asked the bugseng guys to avoid that term, and instead
> use "addresses" or "eliminates a ... violation" or some such.

Ok.

>
>> No functional change.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Thanks.

~Andrew