[PATCH 3/6] xen/x86: Refactor xen/lib/x86 so it can be linked in hvmloader

Alejandro Vallejo posted 6 patches 10 months ago
There is a newer version of this series
[PATCH 3/6] xen/x86: Refactor xen/lib/x86 so it can be linked in hvmloader
Posted by Alejandro Vallejo 10 months ago
A future patch will use these in hvmloader, which is freestanding, but
lacks the Xen code. The following changes fix the compilation errors.

* string.h => Remove memset() usages and bzero through assignments
* inttypes.h => Use stdint.h (it's what it should've been to begin with)
* errno.h => Use xen/errno.h instead

No functional change intended.

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
---
 xen/lib/x86/cpuid.c   | 12 ++++++------
 xen/lib/x86/private.h |  8 +++++---
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/xen/lib/x86/cpuid.c b/xen/lib/x86/cpuid.c
index eb7698dc73..4a138c3a11 100644
--- a/xen/lib/x86/cpuid.c
+++ b/xen/lib/x86/cpuid.c
@@ -5,8 +5,8 @@
 static void zero_leaves(struct cpuid_leaf *l,
                         unsigned int first, unsigned int last)
 {
-    if ( first <= last )
-        memset(&l[first], 0, sizeof(*l) * (last - first + 1));
+    for (l = &l[first]; first <= last; first++, l++ )
+        *l = (struct cpuid_leaf){};
 }
 
 unsigned int x86_cpuid_lookup_vendor(uint32_t ebx, uint32_t ecx, uint32_t edx)
@@ -244,7 +244,7 @@ void x86_cpu_policy_clear_out_of_range_leaves(struct cpu_policy *p)
                 ARRAY_SIZE(p->basic.raw) - 1);
 
     if ( p->basic.max_leaf < 4 )
-        memset(p->cache.raw, 0, sizeof(p->cache.raw));
+        p->cache = (typeof(p->cache)){};
     else
     {
         for ( i = 0; (i < ARRAY_SIZE(p->cache.raw) &&
@@ -255,13 +255,13 @@ void x86_cpu_policy_clear_out_of_range_leaves(struct cpu_policy *p)
     }
 
     if ( p->basic.max_leaf < 7 )
-        memset(p->feat.raw, 0, sizeof(p->feat.raw));
+        p->feat = (typeof(p->feat)){};
     else
         zero_leaves(p->feat.raw, p->feat.max_subleaf + 1,
                     ARRAY_SIZE(p->feat.raw) - 1);
 
     if ( p->basic.max_leaf < 0xb )
-        memset(p->topo.raw, 0, sizeof(p->topo.raw));
+        p->topo = (typeof(p->topo)){};
     else
     {
         for ( i = 0; (i < ARRAY_SIZE(p->topo.raw) &&
@@ -272,7 +272,7 @@ void x86_cpu_policy_clear_out_of_range_leaves(struct cpu_policy *p)
     }
 
     if ( p->basic.max_leaf < 0xd || !cpu_policy_xstates(p) )
-        memset(p->xstate.raw, 0, sizeof(p->xstate.raw));
+        p->xstate = (typeof(p->xstate)){};
     else
     {
         /* This logic will probably need adjusting when XCR0[63] gets used. */
diff --git a/xen/lib/x86/private.h b/xen/lib/x86/private.h
index 60bb82a400..4b8cb97e64 100644
--- a/xen/lib/x86/private.h
+++ b/xen/lib/x86/private.h
@@ -17,12 +17,14 @@
 
 #else
 
-#include <errno.h>
-#include <inttypes.h>
+#include <stdint.h>
 #include <stdbool.h>
 #include <stddef.h>
-#include <string.h>
 
+enum {
+#define XEN_ERRNO(ident, rc) ident = (rc),
+#include <xen/errno.h>
+};
 #include <xen/asm/msr-index.h>
 #include <xen/asm/x86-vendors.h>
 
-- 
2.34.1
Re: [PATCH 3/6] xen/x86: Refactor xen/lib/x86 so it can be linked in hvmloader
Posted by Jan Beulich 7 months, 2 weeks ago
On 09.01.2024 16:38, Alejandro Vallejo wrote:
> A future patch will use these in hvmloader, which is freestanding, but
> lacks the Xen code. The following changes fix the compilation errors.
> 
> * string.h => Remove memset() usages and bzero through assignments

But hvmloader does have memset(). It's just that it doesn't have string.h.
Yet it doesn't have e.g. stdint.h either.

> * inttypes.h => Use stdint.h (it's what it should've been to begin with)
> * errno.h => Use xen/errno.h instead
> 
> No functional change intended.
> 
> Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
> ---
>  xen/lib/x86/cpuid.c   | 12 ++++++------
>  xen/lib/x86/private.h |  8 +++++---
>  2 files changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/lib/x86/cpuid.c b/xen/lib/x86/cpuid.c
> index eb7698dc73..4a138c3a11 100644
> --- a/xen/lib/x86/cpuid.c
> +++ b/xen/lib/x86/cpuid.c
> @@ -5,8 +5,8 @@
>  static void zero_leaves(struct cpuid_leaf *l,
>                          unsigned int first, unsigned int last)
>  {
> -    if ( first <= last )
> -        memset(&l[first], 0, sizeof(*l) * (last - first + 1));
> +    for (l = &l[first]; first <= last; first++, l++ )
> +        *l = (struct cpuid_leaf){};
>  }

Even if memset() was to be replaced here, we already have two instances
of an EMPTY_LEAF #define. Those will want moving to a single header and
hen using here, I expect. Presumably code further down could then use it,
too.

> --- a/xen/lib/x86/private.h
> +++ b/xen/lib/x86/private.h
> @@ -17,12 +17,14 @@
>  
>  #else
>  
> -#include <errno.h>
> -#include <inttypes.h>
> +#include <stdint.h>
>  #include <stdbool.h>
>  #include <stddef.h>
> -#include <string.h>
>  
> +enum {
> +#define XEN_ERRNO(ident, rc) ident = (rc),
> +#include <xen/errno.h>
> +};
>  #include <xen/asm/msr-index.h>
>  #include <xen/asm/x86-vendors.h>

As to the comment at the top - we're in a block of code conditional upon
!Xen here already. Would there be anything wrong with simply recognizing
hvmloader here, too, and then including its util.h in place of string.h?

Jan