On 30/04/2024 15:13, Anthony PERARD wrote:
> On Wed, Feb 07, 2024 at 05:39:55PM +0000, Alejandro Vallejo wrote:
>> diff --git a/tools/include/xenguest.h b/tools/include/xenguest.h
>> index e01f494b772a..4e9078fdee4d 100644
>> --- a/tools/include/xenguest.h
>> +++ b/tools/include/xenguest.h
>> @@ -784,7 +784,13 @@ xen_pfn_t *xc_map_m2p(xc_interface *xch,
>> unsigned long *mfn0);
>>
>> #if defined(__i386__) || defined(__x86_64__)
>> -typedef struct xc_cpu_policy xc_cpu_policy_t;
>> +#include <xen/lib/x86/cpu-policy.h>
>
> I don't think it's a good idea to expose "cpu-policy.h" to "xenguest.h",
> and it's going to break the build of every tools outside of xen
> repository that are using xenguest.h. xenguest.h is installed on a
> system, but cpu-policy.h isn't.
>
>> +typedef struct xc_cpu_policy {
>> + struct cpu_policy policy;
>> + xen_cpuid_leaf_t leaves[CPUID_MAX_SERIALISED_LEAVES];
>> + xen_msr_entry_t msrs[MSR_MAX_SERIALISED_ENTRIES];
>> +} xc_cpu_policy_t;
>
> Would using an accessor be an option to access `leaves` and `msrs` for
> "xen-cpuid" tool? That would avoid the need to expose the "cpu-policy.h"
> headers.
>
> With accessors, we might not need to expose xc_cpu_policy_serialise() to
> the world anymore, and it could be internal to libxenguest, probably, I
> haven't check.
>
> Thanks,
>
That would work out for xen-cpuid.
I'm on the fence in the general case because I think it'd be
advantageous to let clients (i.e: xl) manipulate directly the
deserialized form of the policy. That would allow flipping features on
or off a heck of lot easier. We could create per-feature accessors, but
then we'll end up with _a lot_ of them.
I guess we'll get there when we get there. The xen-cpuid case definitely
doesn't warrant it. I'll give this a go.
Cheers,
Alejandro
