[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2 1/4] x86/vpmu: Move vpmu_do_cpuid() handling into {pv, hvm}_cpuid()



On 06/12/16 11:11, Andrew Cooper wrote:
> On 05/12/16 20:59, Boris Ostrovsky wrote:
>> On 12/05/2016 01:24 PM, Andrew Cooper wrote:
>>> @@ -3516,6 +3516,17 @@ void hvm_cpuid(unsigned int input, unsigned int 
>>> *eax, unsigned int *ebx,
>>>              if ( !(hvm_pae_enabled(v) || hvm_long_mode_enabled(v)) )
>>>                  *edx &= ~cpufeat_mask(X86_FEATURE_PSE36);
>>>          }
>>> +
>>> +        if ( vpmu_enabled(v) &&
>>> +             vpmu_is_set(vcpu_vpmu(v), VPMU_CPU_HAS_DS) )
>>> +        {
>>> +            *edx |= cpufeat_mask(X86_FEATURE_DS);
>>> +            if ( cpu_has(&current_cpu_data, X86_FEATURE_DTES64) )
>>> +                *ecx |= cpufeat_mask(X86_FEATURE_DTES64);
>>> +            if ( cpu_has(&current_cpu_data, X86_FEATURE_DSCPL) )
>>> +                *ecx |= cpufeat_mask(X86_FEATURE_DSCPL);
>>> +        }
>>> +
>>>          break;
>>>  
>>>      case 0x7:
>>> @@ -3646,6 +3657,18 @@ void hvm_cpuid(unsigned int input, unsigned int 
>>> *eax, unsigned int *ebx,
>>>          }
>>>          break;
>>>  
>>> +    case 0x0000000a: /* Architectural Performance Monitor Features (Intel) 
>>> */
>>> +        if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL || 
>>> !vpmu_enabled(v) )
>>> +        {
>>> +            *eax = *ebx = *ecx = *edx = 0;
>>> +            break;
>>> +        }
>>> +
>>> +        /* Report at most version 3 since that's all we currently emulate 
>>> */
>>> +        if ( (*eax & 0xff) > 3 )
>>> +            *eax = (*eax & ~0xff) | 3;
>>> +        break;
>> Both this and Debug Store checks are the same for both HVM and PV. Can
>> they be factored out?
> The purpose of this patch series is to untangle the current call tree to
> make it easier to finally merge the PV and HVM paths into a single
> guest_cpuid().
>
> Yes, this does add a bit of duplication in the short timer, but allows
> for easier movement to the longterm goal.
>
>> (and then perhaps version update can gain back PMU_VERSION_MASK macro)
> That involves moving a whole load of Intel internals with generic names
> into vpmu.h, which is why I chose not to do it.  The end result in
> guest_cpuid() won't use it.

Boris: Any further comment, or is my explanation ok?  Strictly speaking
the patch isn't blocked on your review, but I'd prefer not to use that
technicality if you are unhappy with it.

~Andrew

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.