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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 24/03/16 17:11, Jan Beulich wrote:
>>>> 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.

Ah ok - I will tweak it.

~Andrew

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