Re: [Xen-devel] [PATCH v2 04/13] libx86: Share struct cpuid_policy with userspace

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.07.18 at 22:03, <andrew.cooper3@xxxxxxxxxx> wrote:
> --- a/tools/include/xen-tools/libs.h
> +++ b/tools/include/xen-tools/libs.h
> @@ -13,4 +13,8 @@
>  #define ARRAY_SIZE(a) (sizeof(a) / sizeof(*a))
>  #endif
>  
> +#ifndef MAX
> +#define MAX(x, y) ((x) > (y) ? (x) : (y))
> +#endif

I find asymmetries like this odd: There should then also be MIN() imo.

> +static inline void cpuid_featureset_to_policy(
> +    const uint32_t fs[FEATURESET_NR_ENTRIES], struct cpuid_policy *p)
> +{
> +    p->basic._1d  = fs[FEATURESET_1d];
> +    p->basic._1c  = fs[FEATURESET_1c];
> +    p->extd.e1d   = fs[FEATURESET_e1d];
> +    p->extd.e1c   = fs[FEATURESET_e1c];
> +    p->xstate.Da1 = fs[FEATURESET_Da1];
> +    p->feat._7b0  = fs[FEATURESET_7b0];
> +    p->feat._7c0  = fs[FEATURESET_7c0];
> +    p->extd.e7d   = fs[FEATURESET_e7d];
> +    p->extd.e8b   = fs[FEATURESET_e8b];
> +    p->feat._7d0  = fs[FEATURESET_7d0];
> +}

I realize this is only code movement, but since you didn't answer the
question raised on the Intel Process Trace thread (v2 03/10) yet, I'll
raise it here again: Shouldn't other fields of p be set to zero here?

Irrespective of both items (i.e. with or without them addressed)
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel