[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] kvm: migrate vPMU state
From: |
Gleb Natapov |
Subject: |
Re: [Qemu-devel] [PATCH] kvm: migrate vPMU state |
Date: |
Thu, 25 Jul 2013 13:14:12 +0300 |
On Wed, Jul 24, 2013 at 05:01:15PM +0200, Andreas Färber wrote:
> Am 24.07.2013 16:37, schrieb Paolo Bonzini:
> > This requires kernel 3.10 but it is otherwise quite simple to do.
> > The kernel pays attention to MSRs writes that are host initiated, and
> > disables all side effects of the PMU registers (e.g. the global status
> > MSR can be written and global overflow control MSR does not clear bits
> > in the global status MSR).
> >
> > Only two bits are interesting. First, the number of general-purpose
> > counters must be fetched from CPUID so that we do not read non-existent
> > MSRs. It need not be part of the migration stream.
> >
> > Second, to avoid any possible side effects during the setting of MSRs
> > I stop the PMU while setting the counters and event selector MSRs.
> > Stopping the PMU snapshots the counters and ensures that no strange
> > races can happen if the counters were saved close to their overflow
> > value.
> >
> > Signed-off-by: Paolo Bonzini <address@hidden>
> > ---
> > target-i386/cpu.h | 23 +++++++++++++
> > target-i386/kvm.c | 93
> > ++++++++++++++++++++++++++++++++++++++++++++++++---
> > target-i386/machine.c | 44 ++++++++++++++++++++++++
> > 3 files changed, 155 insertions(+), 5 deletions(-)
> >
> > diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> > index 058c57f..522eed4 100644
> > --- a/target-i386/cpu.h
> > +++ b/target-i386/cpu.h
> > @@ -304,6 +304,8 @@
> > #define MSR_TSC_ADJUST 0x0000003b
> > #define MSR_IA32_TSCDEADLINE 0x6e0
> >
> > +#define MSR_P6_PERFCTR0 0xc1
> > +
> > #define MSR_MTRRcap 0xfe
> > #define MSR_MTRRcap_VCNT 8
> > #define MSR_MTRRcap_FIXRANGE_SUPPORT (1 << 8)
> > @@ -317,6 +319,8 @@
> > #define MSR_MCG_STATUS 0x17a
> > #define MSR_MCG_CTL 0x17b
> >
> > +#define MSR_P6_EVNTSEL0 0x186
> > +
> > #define MSR_IA32_PERF_STATUS 0x198
> >
> > #define MSR_IA32_MISC_ENABLE 0x1a0
> > @@ -342,6 +346,14 @@
> >
> > #define MSR_MTRRdefType 0x2ff
> >
> > +#define MSR_CORE_PERF_FIXED_CTR0 0x309
> > +#define MSR_CORE_PERF_FIXED_CTR1 0x30a
> > +#define MSR_CORE_PERF_FIXED_CTR2 0x30b
> > +#define MSR_CORE_PERF_FIXED_CTR_CTRL 0x38d
> > +#define MSR_CORE_PERF_GLOBAL_STATUS 0x38e
> > +#define MSR_CORE_PERF_GLOBAL_CTRL 0x38f
> > +#define MSR_CORE_PERF_GLOBAL_OVF_CTRL 0x390
> > +
> > #define MSR_MC0_CTL 0x400
> > #define MSR_MC0_STATUS 0x401
> > #define MSR_MC0_ADDR 0x402
> [snip]
>
> We have a mix of lines with tab indentation and with space indentation
> here - should new sections be using spaces to satisfy checkpatch.pl?
>
> New msr_* fields would be candidates for X86CPU, but there's already one
> in CPUX86State. :/
>
Is this NAK? Should I expect another version?
--
Gleb.