On Thu, Jun 16, 2016 at 06:12:11PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilb...@redhat.com>
>
> Fill the bits between 51..number-of-physical-address-bits in the
> MTRR_PHYSMASKn variable range mtrr masks so that they're consistent
> in the migration stream irrespective of the physical address space
> of the source VM in a migration.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
> Suggested-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> include/hw/i386/pc.h | 5 +++++
> target-i386/cpu.c | 1 +
> target-i386/cpu.h | 3 +++
> target-i386/kvm.c | 25 ++++++++++++++++++++++++-
> 4 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 49566c8..3883d04 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -362,6 +362,11 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t
> *);
> .driver = TYPE_X86_CPU,\
> .property = "cpuid-0xb",\
> .value = "off",\
> + },\
> + {\
> + .driver = TYPE_X86_CPU,\
> + .property = "fill-mtrr-mask",\
> + .value = "off",\
> },
>
> #define PC_COMPAT_2_5 \
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index f9302f9..4111240 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -3272,6 +3272,7 @@ static Property x86_cpu_properties[] = {
> DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
> DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
> DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),
> + DEFINE_PROP_BOOL("fill-mtrr-mask", X86CPU, fill_mtrr_mask, true),
This is necessary only when phys_bits is higher on the
destination, right?
Should we really default this to true? I would like to enable
this hack only when really necessary. Except when using host's
phys_bits (phys-bits=0), is there any valid reason to expect
higher phys-bits on the destination?
> DEFINE_PROP_UINT32("level", X86CPU, env.cpuid_level, 0),
> DEFINE_PROP_UINT32("xlevel", X86CPU, env.cpuid_xlevel, 0),
> DEFINE_PROP_UINT32("xlevel2", X86CPU, env.cpuid_xlevel2, 0),
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 61c06b7..13d8501 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -1181,6 +1181,9 @@ struct X86CPU {
> /* Compatibility bits for old machine types: */
> bool enable_cpuid_0xb;
>
> + /* if true fill the top bits of the MTRR_PHYSMASKn variable range */
> + bool fill_mtrr_mask;
> +
> /* in order to simplify APIC support, we leave this pointer to the
> user */
> struct DeviceState *apic_state;
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index d95d06b..9b5158e 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -1934,6 +1934,7 @@ static int kvm_get_msrs(X86CPU *cpu)
> CPUX86State *env = &cpu->env;
> struct kvm_msr_entry *msrs = cpu->kvm_msr_buf->entries;
> int ret, i;
> + uint64_t mtrr_top_bits;
>
> kvm_msr_buf_reset(cpu);
>
> @@ -2083,6 +2084,27 @@ static int kvm_get_msrs(X86CPU *cpu)
> }
>
> assert(ret == cpu->kvm_msr_buf->nmsrs);
> + /*
> + * MTRR masks: Each mask consists of 5 parts
> + * a 10..0: must be zero
> + * b 11 : valid bit
> + * c n-1.12: actual mask bits
> + * d 51..n: reserved must be zero
> + * e 63.52: reserved must be zero
> + *
> + * 'n' is the number of physical bits supported by the CPU and is
> + * apparently always <= 52. We know our 'n' but don't know what
> + * the destinations 'n' is; it might be smaller, in which case
> + * it masks (c) on loading. It might be larger, in which case
> + * we fill 'd' so that d..c is consistent irrespetive of the 'n'
> + * we're migrating to.
> + */
> + if (cpu->fill_mtrr_mask) {
> + mtrr_top_bits = BIT_RANGE(51, cpu_x86_guest_phys_address_bits(env));
> + } else {
> + mtrr_top_bits = 0;
> + }
> +
> for (i = 0; i < ret; i++) {
> uint32_t index = msrs[i].index;
> switch (index) {
> @@ -2278,7 +2300,8 @@ static int kvm_get_msrs(X86CPU *cpu)
> break;
> case MSR_MTRRphysBase(0) ... MSR_MTRRphysMask(MSR_MTRRcap_VCNT - 1):
> if (index & 1) {
> - env->mtrr_var[MSR_MTRRphysIndex(index)].mask = msrs[i].data;
> + env->mtrr_var[MSR_MTRRphysIndex(index)].mask = msrs[i].data |
> + mtrr_top_bits;
> } else {
> env->mtrr_var[MSR_MTRRphysIndex(index)].base = msrs[i].data;
> }
> --
> 2.7.4
>
--
Eduardo
