Re: [Xen-devel] [PATCH 1/3] x86/vpmu: Calculate vpmu_enabled() based on vpmu_mode value

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 02/13/2017 07:50 AM, Jan Beulich wrote:
> > > > On 13.02.17 at 03:29, <boris.ostrovsky@xxxxxxxxxx> wrote:
> > vpmu_enabled() is currently reported based on VPMU_CONTEXT_ALLOCATED
> > bit of VPMU's flags. This presents a problem on Intel processors where
> > VPMU context is allocated lazily, during the first access to VPMU MSRs.
> > With such delayed allocation we, for example, cannot properly report
> > CPUID's leaf 0xa since it is likely that the leaf is queried by a
> > guest before the guest attempts ro read or write the MSR.
> >
> > Instead of keying vpmu_enabled() off the flags we should compute it
> > based on vpmu_mode.
> Doesn't this have its own downsides? What if the mode changes
> behind the back of a DomU? While certain mode changes are
> disallowed while there are active users, there's nothing preventing
> mode or features from changing between a DomU querying CPUID
> and enabling the vPMU.

As you said, the mode can't change after a domU VCPU has been 
initialized, i.e. before the VCPU started running.
Of course, this was broken, which is what patch 2 (not quite 
successfully) tried to fix. And I proposed in my reply to Andrew
to instead define vpmu_enabled() as !!vcpu_vpmu(v)->arch_vpmu_ops.
That should solve problems that both patches try to address.

-boris


> > --- a/xen/arch/x86/cpu/vpmu.c
> > +++ b/xen/arch/x86/cpu/vpmu.c
> > @@ -110,6 +110,12 @@ static void __init parse_vpmu_params(char *s)
> >      printk("VPMU: unknown flags: %s - vpmu disabled!\n", s);
> >  }
> >
> > +bool_t vpmu_enabled (const struct vcpu *v)
> Stray blank.
>
> Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel