On 04/07/18 10:01, Jan Beulich wrote:
>>>> On 03.07.18 at 22:55, <andrew.coop...@citrix.com> wrote:
>> --- a/xen/common/libx86/cpuid.c
>> +++ b/xen/common/libx86/cpuid.c
>> @@ -34,6 +34,100 @@ const uint32_t *x86_cpuid_lookup_deep_deps(uint32_t
>> feature)
>> }
>>
>> /*
>> + * Copy a single cpuid_leaf into a provided xen_cpuid_leaf_t buffer,
>> + * performing boundary checking against the buffer size.
>> + */
>> +static int copy_leaf_to_buffer(uint32_t leaf, uint32_t subleaf,
>> + const struct cpuid_leaf *data,
>> + cpuid_leaf_buffer_t leaves,
>> + uint32_t *curr_entry, const uint32_t
>> nr_entries)
>> +{
>> + const xen_cpuid_leaf_t val = {
>> + leaf, subleaf, data->a, data->b, data->c, data->d,
>> + };
>> +
>> + if ( *curr_entry == nr_entries )
>> + return -ENOBUFS;
>> +
>> + if ( copy_to_buffer_offset(leaves, *curr_entry, &val, 1) )
>> + return -EFAULT;
>> +
>> + ++*curr_entry;
> Following on from what Wei has said - you don't mean to have a way
> here then to indicate to a higher up caller how many slots would have
> been needed?
I don't understand your query. An individual build has a compile-time
static maximum number of leaves, and this number can be obtained in the
usual way by making a hypercall with a NULL guest handle.
The external representation must not encode this number, as it will
change build to build, hardware to hardware, and in such times as we
gain a load of new features in microcode.
>
>> +int x86_cpuid_copy_to_buffer(const struct cpuid_policy *p,
>> + cpuid_leaf_buffer_t leaves,
>> + uint32_t *nr_entries_p)
>> +{
>> + const uint32_t nr_entries = *nr_entries_p;
>> + uint32_t curr_entry = 0, leaf, subleaf;
>> +
>> +#define COPY_LEAF(l, s, data) \
>> + ({ int ret; \
>> + if ( (ret = copy_leaf_to_buffer( \
>> + l, s, data, leaves, &curr_entry, nr_entries)) ) \
>> + return ret; \
>> + })
>> +
>> + /* Basic leaves. */
>> + for ( leaf = 0; leaf <= MIN(p->basic.max_leaf,
>> + ARRAY_SIZE(p->basic.raw) - 1); ++leaf )
>> + {
>> + switch ( leaf )
>> + {
>> + case 0x4:
>> + for ( subleaf = 0; subleaf < ARRAY_SIZE(p->cache.raw);
>> ++subleaf )
>> + COPY_LEAF(leaf, subleaf, &p->cache.raw[subleaf]);
>> + break;
>> +
>> + case 0x7:
>> + for ( subleaf = 0;
>> + subleaf <= MIN(p->feat.max_subleaf,
>> + ARRAY_SIZE(p->feat.raw) - 1); ++subleaf )
>> + COPY_LEAF(leaf, subleaf, &p->feat.raw[subleaf]);
>> + break;
>> +
>> + case 0xb:
>> + for ( subleaf = 0; subleaf < ARRAY_SIZE(p->topo.raw); ++subleaf
>> )
>> + COPY_LEAF(leaf, subleaf, &p->topo.raw[subleaf]);
>> + break;
>> +
>> + case 0xd:
>> + for ( subleaf = 0; subleaf < ARRAY_SIZE(p->xstate.raw);
>> ++subleaf )
>> + COPY_LEAF(leaf, subleaf, &p->xstate.raw[subleaf]);
>> + break;
>> +
>> + default:
>> + COPY_LEAF(leaf, XEN_CPUID_NO_SUBLEAF, &p->basic.raw[leaf]);
>> + break;
>> + }
>> + }
>> +
>> + COPY_LEAF(0x40000000, XEN_CPUID_NO_SUBLEAF,
>> + &(struct cpuid_leaf){ p->hv_limit });
>> + COPY_LEAF(0x40000100, XEN_CPUID_NO_SUBLEAF,
>> + &(struct cpuid_leaf){ p->hv2_limit });
> Is it a good idea to produce wrong (zero) EBX, ECX, and EDX values here?
The handling of these leaves currently problematic, and this patch is
bug-compatible with how DOMCTL_set_cpuid currently behaves (See
update_domain_cpuid_info()).
Annoyingly, I need this marshalling series implemented before I can fix
the hypervisor leaves to use the "new" CPUID infrastructure; the main
complication being because of the dynamic location of the Xen leaves.
Eventually, the interface will be that Xen leaves live at 0x40000000 and
the toolstack can manipulate a subset of the information by providing
leaves in the usual manor. To enable viridian, the toolstack writes
HyperV's signature at 0x40000000, and Xen's at 0x40000100. This also
allows for a mechanism to hide the Xen CPUID leaves by writing a 0 max leaf.
Amongst other things, this will allow sensible control of the Viridian
features without having to squeeze more bits into the HVMPARAM.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
