On Wed, Apr 14, 2021 at 03:49:02PM +0200, Jan Beulich wrote:
> On 13.04.2021 16:01, Roger Pau Monne wrote:
> > @@ -944,3 +945,130 @@ bool xc_cpu_policy_is_compatible(xc_interface *xch,
> > const xc_cpu_policy_t host,
> >
> > return false;
> > }
> > +
> > +static uint64_t level_msr(unsigned int index, uint64_t val1, uint64_t val2)
> > +{
> > + uint64_t val = val1 & val2;;
>
> For arbitrary MSRs this isn't going to do any good. If only very
> specific MSRs are assumed to make it here, I think this wants
> commenting on.
I've added: "MSRs passed to level_msr are expected to be bitmaps of
features"
> > + xen_cpuid_leaf_t *out)
> > +{
> > + *out = (xen_cpuid_leaf_t){ };
> > +
> > + switch ( l1->leaf )
> > + {
> > + case 0x1:
> > + case 0x80000001:
> > + out->c = l1->c & l2->c;
> > + out->d = l1->d & l2->d;
> > + return true;
> > +
> > + case 0xd:
> > + if ( l1->subleaf != 1 )
> > + break;
> > + out->a = l1->a & l2->a;
> > + return true;
>
> Could you explain your thinking behind this (a code comment would
> likely help)? You effectively discard everything except subleaf 1
> by returning false in that case, don't you?
Yes, the intent is to only level the features bitfield found in
subleaf 1.
I was planning for level_leaf so far in this series to deal with the
feature leaves part of the featureset only. I guess you would also
like to leverage other parts of the xstate leaf, like the max_size or
the supported bits in xss_{low,high}?
> > + case 0x7:
> > + switch ( l1->subleaf )
> > + {
> > + case 0:
> > + out->b = l1->b & l2->b;
> > + out->c = l1->c & l2->c;
> > + out->d = l1->d & l2->d;
> > + return true;
> > +
> > + case 1:
> > + out->a = l1->a & l2->a;
> > + return true;
> > + }
> > + break;
>
> Can we perhaps assume all subleaves here are going to hold flags,
> and hence and both sides together without regard to what subleaf
> we're actually dealing with (subleaf 1 remaining special as to
I think you meant subleaf 0 EAX (the max subleaf value)?
subleaf 1 EAX is just a normal bitfield AFAIK.
> EAX of course)? This would avoid having to remember to make yet
> another mechanical change when enabling a new subleaf.
Sure, seems like a fine approach since leaf 7 will only contain
feature bitmaps.
> > + case 0x80000007:
> > + out->d = l1->d & l2->d;
> > + return true;
> > +
> > + case 0x80000008:
> > + out->b = l1->b & l2->b;
> > + return true;
> > + }
> > +
> > + return false;
> > +}
>
> Considering your LFENCE-always-serializing patch, I assume
> whichever ends up going in last will take care of adding handling
> of that leaf here?
Indeed.
Thanks, Roger.
