On 06/10/2023 19:57, Andrew Cooper wrote:
On 06/10/2023 9:26 am, Nicola Vetrini wrote:
The COUNT_LEAVES macro is introduced to avoid using an essentially
boolean value in a subtraction.
No functional change.
Signed-off-by: Nicola Vetrini <nicola.vetr...@bugseng.com>
---
xen/include/xen/lib/x86/cpu-policy.h | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/xen/include/xen/lib/x86/cpu-policy.h
b/xen/include/xen/lib/x86/cpu-policy.h
index bab3eecda6c1..700993cc67e8 100644
--- a/xen/include/xen/lib/x86/cpu-policy.h
+++ b/xen/include/xen/lib/x86/cpu-policy.h
@@ -95,17 +95,18 @@ const char *x86_cpuid_vendor_to_str(unsigned int
vendor);
#define CPUID_GUEST_NR_EXTD MAX(CPUID_GUEST_NR_EXTD_INTEL, \
CPUID_GUEST_NR_EXTD_AMD)
+#define COUNT_LEAVES(X) ((X) - ((X) ? 1 : 0))
/*
* Maximum number of leaves a struct cpu_policy turns into when
serialised for
* interaction with the toolstack. (Sum of all leaves in each union,
less the
* entries in basic which sub-unions hang off of.)
*/
-#define CPUID_MAX_SERIALISED_LEAVES \
- (CPUID_GUEST_NR_BASIC + \
- CPUID_GUEST_NR_FEAT - !!CPUID_GUEST_NR_FEAT + \
- CPUID_GUEST_NR_CACHE - !!CPUID_GUEST_NR_CACHE + \
- CPUID_GUEST_NR_TOPO - !!CPUID_GUEST_NR_TOPO + \
- CPUID_GUEST_NR_XSTATE - !!CPUID_GUEST_NR_XSTATE + \
+#define CPUID_MAX_SERIALISED_LEAVES \
+ (CPUID_GUEST_NR_BASIC + \
+ COUNT_LEAVES(CPUID_GUEST_NR_FEAT) + \
+ COUNT_LEAVES(CPUID_GUEST_NR_CACHE) + \
+ COUNT_LEAVES(CPUID_GUEST_NR_TOPO) + \
+ COUNT_LEAVES(CPUID_GUEST_NR_XSTATE) + \
CPUID_GUEST_NR_EXTD + 2 /* hv_limit and hv2_limit */ )
This may not have been a MISRA-approved calculation, but encapsulating
it like this breaks any ability to follow what's going on.
CPUID data in x86 is mostly a sparse 1-D array (BASIC, EXTD, HV
blocks),
but a couple of elements in the BASIC array have arrays themselves.
The struct is laid out for O(1) access, so you can't just say
sizeof(struct) / sizeof(element). The BASIC array has elements (0x4,
0x7, 0xb, 0xd) which hold no data because there's actually an array
elsewhere containing all the data.
So logically, it's:
(BASIC + (FEAT - 1) + (CACHE - 1) + (TOPO - 1) + (XSTATE - 1)) + EXTD +
2
And in practice I'd far rather express it with a plain -1 than a -
!!NR_, if the latter isn't an option.
Presumably MISRA would be happy with that?
If so, I can submit a patch. There's also a typo in that the comment
that wants fixing.
~Andrew
Yes, that should be fine. I'll be happy to test that.
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
