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

Commit Message

Andrew Cooper Jan. 16, 2024, 11:58 a.m. UTC

Fixes MISRA XXX

No functional change.

Signed-off-by: Andrew Cooper <[e-mail]>
---
CC: Jan Beulich <[e-mail]>
CC: Roger Pau Monné <[e-mail]>
CC: Wei Liu <[e-mail]>
CC: Stefano Stabellini <[e-mail]>
CC: Julien Grall <[e-mail]>
CC: Volodymyr Babchuk <[e-mail]>
CC: Bertrand Marquis <[e-mail]>
CC: Michal Orzel <[e-mail]>
CC: Roberto Bagnara <[e-mail]>
CC: Federico Serafini <[e-mail]>
CC: [e-mail] <[e-mail]>

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


base-commit: f3f6c500e2dbd23af77c207e2cf4b496fffa1b0d

Comments

Jan Beulich Jan. 16, 2024, 1:02 p.m. UTC | #1

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 <[e-mail]>

Reviewed-by: Jan Beulich <[e-mail]>

Jan

Andrew Cooper Jan. 16, 2024, 1:46 p.m. UTC | #2

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 <[e-mail]>
> Reviewed-by: Jan Beulich <[e-mail]>

Thanks.

~Andrew

Federico Serafini Jan. 16, 2024, 2:10 p.m. UTC | #3

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 <[e-mail]>
> 
> Reviewed-by: Jan Beulich <[e-mail]>
> 
> 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.

Patch

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