[PATCH 3/3] x86/ucode: Create a real type for loading_state

Andrew Cooper posted 3 patches 1 day, 3 hours ago
[PATCH 3/3] x86/ucode: Create a real type for loading_state
Posted by Andrew Cooper 1 day, 3 hours ago
Using typeof() in wait_for_state()/set_state() unnecesserily cryptic, and more
verbose than using a proper type.

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>
---
 xen/arch/x86/cpu/microcode/core.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c
index 7aaf62839f56..fe47c3a6c18d 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -70,12 +70,13 @@ static unsigned int nr_cores;
  *  - LOADING_ENTER: all CPUs have called in. Initiate ucode loading.
  *  - LOADING_EXIT: ucode loading is done or aborted.
  */
-static enum {
+typedef enum {
     LOADING_PREPARE,
     LOADING_CALLIN,
     LOADING_ENTER,
     LOADING_EXIT,
-} loading_state;
+} loading_state_t;
+static loading_state_t loading_state;
 
 struct patch_with_flags {
     unsigned int flags;
@@ -237,9 +238,9 @@ static bool cf_check wait_cpu_callout(unsigned int nr)
     return atomic_read(&cpu_out) >= nr;
 }
 
-static bool wait_for_state(typeof(loading_state) state)
+static bool wait_for_state(loading_state_t state)
 {
-    typeof(loading_state) cur_state;
+    loading_state_t cur_state;
 
     while ( (cur_state = ACCESS_ONCE(loading_state)) != state )
     {
@@ -251,7 +252,7 @@ static bool wait_for_state(typeof(loading_state) state)
     return true;
 }
 
-static void set_state(typeof(loading_state) state)
+static void set_state(loading_state_t state)
 {
     ACCESS_ONCE(loading_state) = state;
 }
-- 
2.39.5


Re: [PATCH 3/3] x86/ucode: Create a real type for loading_state
Posted by Jan Beulich 17 hours ago
On 17.11.2025 23:21, Andrew Cooper wrote:
> Using typeof() in wait_for_state()/set_state() unnecesserily cryptic, and more
> verbose than using a proper type.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

I don't strictly mind the change, so
Acked-by: Jan Beulich <jbeulich@suse.com>
but I think the use of typeof() also has its benefits:

> @@ -237,9 +238,9 @@ static bool cf_check wait_cpu_callout(unsigned int nr)
>      return atomic_read(&cpu_out) >= nr;
>  }
>  
> -static bool wait_for_state(typeof(loading_state) state)
> +static bool wait_for_state(loading_state_t state)
>  {
> -    typeof(loading_state) cur_state;
> +    loading_state_t cur_state;
>  
>      while ( (cur_state = ACCESS_ONCE(loading_state)) != state )
>      {

Even if the type of loading_state changed, no type mismatches would result
here. Or in other words, a type adjustment there would not entail code
changes here and ...

> @@ -251,7 +252,7 @@ static bool wait_for_state(typeof(loading_state) state)
>      return true;
>  }
>  
> -static void set_state(typeof(loading_state) state)
> +static void set_state(loading_state_t state)
>  {
>      ACCESS_ONCE(loading_state) = state;
>  }

... here (which could easily be forgotten as the compiler might not flag
such mismatches).

Thing of course is that the type of loading_state is pretty unlikely to
change.

Jan