Such helpers is just a wrapper to the existing
x86_cpu_policies_are_compatible function. This requires building
policy.c from libx86 on user land also.

No user of the interface introduced.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Changes since v1:
 - Initialize err.
 - Explicitly name parameters as host and guest.
---
 tools/include/xenctrl.h         |  4 ++++
 tools/libs/guest/Makefile       |  2 +-
 tools/libs/guest/xg_cpuid_x86.c | 19 +++++++++++++++++++
 3 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index 9a6d1b126d8..5f699c09509 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -2619,6 +2619,10 @@ int xc_cpu_policy_update_cpuid(xc_interface *xch, xc_cpu_policy_t policy,
 int xc_cpu_policy_update_msrs(xc_interface *xch, xc_cpu_policy_t policy,
                               const xen_msr_entry_t *msrs, uint32_t nr);
 
+/* Compatibility calculations. */
+bool xc_cpu_policy_is_compatible(xc_interface *xch, const xc_cpu_policy_t host,
+                                 const xc_cpu_policy_t guest);
+
 int xc_get_cpu_levelling_caps(xc_interface *xch, uint32_t *caps);
 int xc_get_cpu_featureset(xc_interface *xch, uint32_t index,
                           uint32_t *nr_features, uint32_t *featureset);
diff --git a/tools/libs/guest/Makefile b/tools/libs/guest/Makefile
index 604e1695d6a..6d2a1d5bbc0 100644
--- a/tools/libs/guest/Makefile
+++ b/tools/libs/guest/Makefile
@@ -40,7 +40,7 @@ $(patsubst %.c,%.opic,$(ELF_SRCS-y)): CFLAGS += -Wno-pointer-sign
 ifeq ($(CONFIG_X86),y) # Add libx86 to the build
 vpath %.c ../../../xen/lib/x86
 
-SRCS-y                 += cpuid.c msr.c
+SRCS-y                 += cpuid.c msr.c policy.c
 endif
 
 # new domain builder
diff --git a/tools/libs/guest/xg_cpuid_x86.c b/tools/libs/guest/xg_cpuid_x86.c
index 26b09322dfa..bd2f31dd87f 100644
--- a/tools/libs/guest/xg_cpuid_x86.c
+++ b/tools/libs/guest/xg_cpuid_x86.c
@@ -925,3 +925,22 @@ int xc_cpu_policy_update_msrs(xc_interface *xch, xc_cpu_policy_t policy,
 
     return rc;
 }
+
+bool xc_cpu_policy_is_compatible(xc_interface *xch, const xc_cpu_policy_t host,
+                                 const xc_cpu_policy_t guest)
+{
+    struct cpu_policy_errors err = INIT_CPU_POLICY_ERRORS;
+    struct cpu_policy h = { &host->cpuid, &host->msr };
+    struct cpu_policy g = { &guest->cpuid, &guest->msr };
+    int rc = x86_cpu_policies_are_compatible(&h, &g, &err);
+
+    if ( !rc )
+        return true;
+
+    if ( err.leaf != -1 )
+        ERROR("Leaf %#x subleaf %#x is not compatible", err.leaf, err.subleaf);
+    if ( err.msr != -1 )
+        ERROR("MSR index %#x is not compatible", err.msr);
+
+    return false;
+}
-- 
2.30.1

On 13.04.2021 16:01, Roger Pau Monne wrote:
> --- a/tools/libs/guest/xg_cpuid_x86.c
> +++ b/tools/libs/guest/xg_cpuid_x86.c
> @@ -925,3 +925,22 @@ int xc_cpu_policy_update_msrs(xc_interface *xch, xc_cpu_policy_t policy,
>  
>      return rc;
>  }
> +
> +bool xc_cpu_policy_is_compatible(xc_interface *xch, const xc_cpu_policy_t host,
> +                                 const xc_cpu_policy_t guest)
> +{
> +    struct cpu_policy_errors err = INIT_CPU_POLICY_ERRORS;
> +    struct cpu_policy h = { &host->cpuid, &host->msr };
> +    struct cpu_policy g = { &guest->cpuid, &guest->msr };
> +    int rc = x86_cpu_policies_are_compatible(&h, &g, &err);
> +
> +    if ( !rc )
> +        return true;
> +
> +    if ( err.leaf != -1 )
> +        ERROR("Leaf %#x subleaf %#x is not compatible", err.leaf, err.subleaf);
> +    if ( err.msr != -1 )
> +        ERROR("MSR index %#x is not compatible", err.msr);

Personally I'm against making assumptions like these ones about what
(in this case) INIT_CPU_POLICY_ERRORS actually expands to (i.e. three
times -1). I can see how alternatives to this are quickly going to
get ugly, so I'll leave it to others to judge.

Jan

On Wed, Apr 14, 2021 at 03:36:54PM +0200, Jan Beulich wrote:
> On 13.04.2021 16:01, Roger Pau Monne wrote:
> > --- a/tools/libs/guest/xg_cpuid_x86.c
> > +++ b/tools/libs/guest/xg_cpuid_x86.c
> > @@ -925,3 +925,22 @@ int xc_cpu_policy_update_msrs(xc_interface *xch, xc_cpu_policy_t policy,
> >  
> >      return rc;
> >  }
> > +
> > +bool xc_cpu_policy_is_compatible(xc_interface *xch, const xc_cpu_policy_t host,
> > +                                 const xc_cpu_policy_t guest)
> > +{
> > +    struct cpu_policy_errors err = INIT_CPU_POLICY_ERRORS;
> > +    struct cpu_policy h = { &host->cpuid, &host->msr };
> > +    struct cpu_policy g = { &guest->cpuid, &guest->msr };
> > +    int rc = x86_cpu_policies_are_compatible(&h, &g, &err);
> > +
> > +    if ( !rc )
> > +        return true;
> > +
> > +    if ( err.leaf != -1 )
> > +        ERROR("Leaf %#x subleaf %#x is not compatible", err.leaf, err.subleaf);
> > +    if ( err.msr != -1 )
> > +        ERROR("MSR index %#x is not compatible", err.msr);
> 
> Personally I'm against making assumptions like these ones about what
> (in this case) INIT_CPU_POLICY_ERRORS actually expands to (i.e. three
> times -1). I can see how alternatives to this are quickly going to
> get ugly, so I'll leave it to others to judge.

Would you like me to define a separate POLICY_ERROR? ie:

#define INIT_CPU_POLICY_ERROR -1
#define INIT_CPU_POLICY_ERRORS { INIT_CPU_POLICY_ERROR, \
                                 INIT_CPU_POLICY_ERROR, \
                                 INIT_CPU_POLICY_ERROR }

We already have a bunch of open coded -1 checks anyway, but might
prevent new ones from appearing.

Thanks, Roger.

On 22.04.2021 10:22, Roger Pau Monné wrote:
> On Wed, Apr 14, 2021 at 03:36:54PM +0200, Jan Beulich wrote:
>> On 13.04.2021 16:01, Roger Pau Monne wrote:
>>> --- a/tools/libs/guest/xg_cpuid_x86.c
>>> +++ b/tools/libs/guest/xg_cpuid_x86.c
>>> @@ -925,3 +925,22 @@ int xc_cpu_policy_update_msrs(xc_interface *xch, xc_cpu_policy_t policy,
>>>  
>>>      return rc;
>>>  }
>>> +
>>> +bool xc_cpu_policy_is_compatible(xc_interface *xch, const xc_cpu_policy_t host,
>>> +                                 const xc_cpu_policy_t guest)
>>> +{
>>> +    struct cpu_policy_errors err = INIT_CPU_POLICY_ERRORS;
>>> +    struct cpu_policy h = { &host->cpuid, &host->msr };
>>> +    struct cpu_policy g = { &guest->cpuid, &guest->msr };
>>> +    int rc = x86_cpu_policies_are_compatible(&h, &g, &err);
>>> +
>>> +    if ( !rc )
>>> +        return true;
>>> +
>>> +    if ( err.leaf != -1 )
>>> +        ERROR("Leaf %#x subleaf %#x is not compatible", err.leaf, err.subleaf);
>>> +    if ( err.msr != -1 )
>>> +        ERROR("MSR index %#x is not compatible", err.msr);
>>
>> Personally I'm against making assumptions like these ones about what
>> (in this case) INIT_CPU_POLICY_ERRORS actually expands to (i.e. three
>> times -1). I can see how alternatives to this are quickly going to
>> get ugly, so I'll leave it to others to judge.
> 
> Would you like me to define a separate POLICY_ERROR? ie:
> 
> #define INIT_CPU_POLICY_ERROR -1
> #define INIT_CPU_POLICY_ERRORS { INIT_CPU_POLICY_ERROR, \
>                                  INIT_CPU_POLICY_ERROR, \
>                                  INIT_CPU_POLICY_ERROR }

I would prefer this; I'm not sure (nit: properly parenthesized)
-1 is  the value to use though, considering the fields are unsigned.
I wonder what Andrew thinks, as he did introduce all of this.

> We already have a bunch of open coded -1 checks anyway, but might
> prevent new ones from appearing.

Indeed.

Jan