Re: [Xen-devel] [PATCH v4 11/26] xen/x86: Improvements to in-hypervisor cpuid sanity checks

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.03.16 at 18:01, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 24/03/16 16:47, Jan Beulich wrote:
>>>>> On 23.03.16 at 17:36, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> +        if ( !is_pvh_domain(currd) )
>>>          {
>>> -            __clear_bit(X86_FEATURE_XSAVE % 32, &c);
>>> -            __clear_bit(X86_FEATURE_AVX % 32, &c);
>>> +            /*
>>> +             * Delete the PVH condition when HVMLite formally replaces PVH,
>>> +             * and HVM guests no longer enter a PV codepath.
>>> +             */
>>> +
>>> +            /* OSXSAVE cleared by pv_featureset.  Fast-forward CR4 back 
>>> in. */
>>> +            if ( (is_pv_domain(currd) && guest_kernel_mode(curr, regs) &&
>>> +                  (read_cr4() & X86_CR4_OSXSAVE)) ||
>>> +                 (curr->arch.pv_vcpu.ctrlreg[4] & X86_CR4_OSXSAVE) )
>>> +                c |= cpufeat_mask(X86_FEATURE_OSXSAVE);
>>>          }
>> The is_pv_domain() is now redundant with the is_pvh_domain()
>> earlier on, and it would likely end up confusing the reader if on
>> the right side of the || then ->arch.pv_vcpu is being referenced.
> 
> I specifically chose to order the code like this to make it easier to
> remove the is_pvh_domain() conditional in the future, without having to
> re-edit the PV path.
> 
> This layout matches the OSPKE version, and I would prefer to keep it
> this way unless you really insist on changing it.

Well, after removing is_pvh_domain() the is_pv_domain() still
won't be needed here, or would also be needed to guard the
curr->arch.pv_vcpu access. So yes, I insist on _some_ change
to make the whole thing consistent.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel