Re: [Xen-devel] [PATCH 6/6] x86/emul: dedup hvmemul_cpuid() and pv_emul_cpuid()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.11.18 at 15:23, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 09/11/2018 17:16, Andrew Cooper wrote:
>>
>>> However, one
>>> issue already might be that in order for bits in a (sub)leaf above
>>> (guest) limits to come out all clear, it is guest_cpuid() which cuts
>>> things off. Neither cpuid_featureset_to_policy() nor its inverse
>>> nor sanitise_featureset() look to zap fields above limits, in case
>>> they've been previously set to non-zero values. Am I overlooking
>>> something?
>> No - that is an aspect I'd overlooked, because the
>> DOMCTL_set_cpuid_policy work (which does this correctly) hasn't gone in yet.
>>
>> I think I'll tweak recalculate_misc() to zero out beyond the max_subleaf
>> settings, because the intention was always that a flat cpuid_policy was
>> safe to use in this manner.  I think there is an existing corner case
>> when trying to level basic.max_leaf to < 7, or extd.max_leaf to < 0x8000007.
> 
> Actually, I remember now that I tried fixing this once before.  It's not
> possible without DOMCTL_set_cpu_policy because the current API with
> DOMCTL_set_cpuid provides leaves piecewise and there is no point at
> which Xen can safely zero the out-of-range leaves without loosing
> information later if max_leaf is increased.

There could be such a point if an arch call was added at the point
where d->creation_finished gets set the first time.

Looking at the domctl side flow I'm wondering what the domain
pausing is good for there, now that we don't allow changes once
d->creation_finished is set. Was not dropping this an oversight
of 3d0cab7b5d?

> The content of cpuid_policy will be safe for the emulator to use,
> because it will be bounded by real hardware support.

Except that it voids all respective uses of vcpu_has_*() - just
cpu_has_* would then be sufficient. And overall behavior is
inconsistent between bit-wise leveling and full leaf hiding.

>  As levelling like
> this is an extreme corner case which doesn't work at the moment for
> other reasons, I'd like to take this series now and fix up the behaviour
> later, to reduce the dependencies in the remaining CPUID/MSR work - its
> already complicated enough.

Hmm, okay, as long as a respective remark gets added at least
to the commit message, I could live with that.

Jan



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