Re: [Xen-devel] [PATCH v1] x86/msr: add Raw and Host domain policies

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 16/02/18 16:00, Sergey Dyasli wrote:
> On Fri, 2018-02-16 at 11:38 +0000, Andrew Cooper wrote:
>> On 16/02/18 11:31, Sergey Dyasli wrote:
>>> On Fri, 2018-02-16 at 04:06 -0700, Jan Beulich wrote:
>>>>>>> On 16.02.18 at 11:33, <sergey.dyasli@xxxxxxxxxx> wrote:
>>>>> On Thu, 2018-02-15 at 06:33 -0700, Jan Beulich wrote:
>>>>>>>>> On 08.02.18 at 11:23, <sergey.dyasli@xxxxxxxxxx> wrote:
>>>>>>>         uint64_t val;
>>>>>>> +       int rc;
>>>>>>>  
>>>>>>> -       if (rdmsr_safe(MSR_INTEL_PLATFORM_INFO, val) ||
>>>>>>> +       if ((rc = rdmsr_safe(MSR_INTEL_PLATFORM_INFO, val)) == 0)
>>>>>>> +       {
>>>>>>> +               dp->plaform_info.available = true;
>>>>>>> +               if (val & MSR_PLATFORM_INFO_CPUID_FAULTING)
>>>>>>> +                       dp->plaform_info.cpuid_faulting = true;
>>>>>>> +       }
>>>>>>> +
>>>>>>> +       if (rc ||
>>>>>>>             !(val & MSR_PLATFORM_INFO_CPUID_FAULTING) ||
>>>>>>>             rdmsr_safe(MSR_INTEL_MISC_FEATURES_ENABLES,
>>>>>>>                        this_cpu(msr_misc_features)))
>>>>>> Below here we have
>>>>>>
>>>>>>          setup_clear_cpu_cap(X86_FEATURE_CPUID_FAULTING);
>>>>>>
>>>>>> Shouldn't this be reflected in the host policy?
>>>>> I guess the correct thing to do for now for host_msr_domain_policy is:
>>>>>
>>>>>     dp->plaform_info.cpuid_faulting = cpu_has_cpuid_faulting;
>>>>>     
>>>>> Looking at the code, calculate_pv_max_policy() will be simplified with
>>>>> the above change: pv_max_msr_domain_policy will become a copy of host
>>>>> policy.
>>>>>
>>>>> This actually brings a question: what to do about per-pCPU MSRs in the
>>>>> context of MSR policy?
>>>> How does per-pCPU-ness of an MSR affect the policy? Are you
>>>> thinking of CPUs with different capabilities? We assume all CPUs
>>>> are identical in many other places.
>>> Yes, CPUs are assumed to be identical. But currently Xen checks
>>> the presence of MISC_FEATURES_ENABLES (which is a per-pCPU msr)
>>> on the boot CPU, and it affects X86_FEATURE_CPUID_FAULTING. Which
>>> in it's turn affects the presence of MISC_FEATURES_ENABLES for PV vCPUs.
>>>
>>> So the actual question is: where to store the availability of
>>> MISC_FEATURES_ENABLES (and possibly other per-pCPU msrs in the future)
>>> and is it even needed to do so?
>> Store it in one single host policy.
> And where do you propose to actually store it? Currently there are
> two distinct structures: msr_domain_policy and msr_vcpu_policy.

msr_domain_policy, like this patch does.

By and large, I expect the "read only feature style" MSRs to all be in
the domain policy, and the "read/write make stuff happen" MSRs to all be
in the vcpu policy.

We can revisit this if/when we find a counterexample.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel