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

[Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>]

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.

> Part of my CPUID work will be cleaning up some of these warts in the
> detection logic.

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