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

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

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.


--- 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.


Xen-devel mailing list



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