Re: [PATCH v3 20/28] x86/domctl: Update PV domain cpumasks when setting cpuid policy

["Jan Beulich" <JBeulich@suse.com>]

>>> On 22.03.16 at 17:37, <andrew.cooper3@citrix.com> wrote:
> On 21/03/16 17:06, Jan Beulich wrote:
>>>>> On 15.03.16 at 16:35, <andrew.cooper3@citrix.com> wrote:
>>> +            switch ( boot_cpu_data.x86_vendor )
>>> +            {
>>> +            case X86_VENDOR_INTEL:
>>> +                mask &= ((uint64_t)edx << 32) | ecx;
>>> +                break;
>>> +
>>> +            case X86_VENDOR_AMD:
>>> +                mask &= ((uint64_t)ecx << 32) | edx;
>>> +
>>> +                /* Fast-forward bits - Must be set. */
>>> +                if (ecx & cpufeat_mask(X86_FEATURE_XSAVE))
>> Missing blanks inside parens. Also I think the comment should be
>> expanded to include the fact that the need to do this here but
>> not in the Intel case was empirically(?) determined
> 
> It was empirically determined that AMD did have to behave like this.
> 
>>  - just in case
>> something isn't quite right with this on some future hardware,
>> and readers (possibly including ourselves) wonder.
> 
> Intel and AMD masking MSRs are fundamentally different, and function
> differently.
> 
> Intel MSRs are documented strictly an & mask, and experimentally, are
> applied before OSXSAVE/APIC are fast-forwarded from hardware.
> 
> AMD MSRs are documented strictly as an override, with a caution to avoid
> setting bits which aren't actually supported in the hardware, and
> experimentally need the fast-forward bits set if fast-forwarding is to
> occur.
> 
> I suppose I could split this up and move this logic into
> cpu/{amd,intel}.c as appropriate, and call a vendor specific function to
> appropriately translate a guest cpuid value into a mask value.  However,
> a lot of common logic would end up needing to be duplicated.

I'm certainly against code duplication. And just to re-iterate - all
I have been asking for was to write some of what you've said in
a source comment.

Jan


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