Re: [PATCH v3 07/13] libs/guest: obtain a compatible cpu policy from two input ones

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 30/04/2021 16:52, Roger Pau Monne wrote:
> Introduce a helper to obtain a compatible cpu policy based on two
> input cpu policies. Currently this is done by and'ing all CPUID
> feature leaves and MSR entries, except for MSR_ARCH_CAPABILITIES which
> has the RSBA bit or'ed.
>
> The _AC macro is pulled from libxl_internal.h into xen-tools/libs.h
> since it's required in order to use the msr-index.h header.
>
> Note there's no need to place this helper in libx86, since the
> calculation of a compatible policy shouldn't be done from the
> hypervisor.
>
> No callers of the interface introduced.
>
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> ---
> Changes since v2:
>  - Add some comments.
>  - Remove stray double semicolon.
>  - AND all 0x7 subleaves (except 0.EAX).
>  - Explicitly handle MSR indexes in a switch statement.
>  - Error out when an unhandled MSR is found.
>  - Add handling of leaf 0x80000021.
>
> Changes since v1:
>  - Only AND the feature parts of cpuid.
>  - Use a binary search to find the matching leaves and msr entries.
>  - Remove default case from MSR level function.
> ---
>  tools/include/xen-tools/libs.h    |   5 ++
>  tools/include/xenctrl.h           |   4 +
>  tools/libs/guest/xg_cpuid_x86.c   | 137 ++++++++++++++++++++++++++++++
>  tools/libs/light/libxl_internal.h |   2 -

This *needs* to be in libx86.  I don't particularly mind if you start
with it behind #ifdef __XEN__ (I'm still sure we'll need it in the
hypervisor), but this, more than just about anything else, needs to be
covered by the unit tests.

Next, you need to follow the same structure as Xen's cpuid.c for
deriving policies.  You can't just loop through the two serialised forms
like this.

To start with, you want to calculate the min of a/b->max_leaf, then loop
over that pulling information sideways from a/b.

For MSRs, all but MSR_INTEL_PLATFORM_INFO are CPUID qualified, so need
to look like:

if ( out.cpuid.feat.arch_caps )
    out.msr.arch_caps.raw =
        ((a.msr.arch_caps.raw ^ INV_MASK) &
         (b.msr.arch_caps.raw ^ INV_MASK)) ^ INV_MASK;

Where INV_MASK is the mask of arch caps bits which want inverted
polarity.  (Name subject to change - perhap ARCH_CAPS_POLARITY_MASK ?)


I'm sure I had some work starting this somewhere.  I'll see if I can
locate it.

~Andrew