Message-ID: <554AC6A2.9030509@linux.intel.com>
Date: Thu, 07 May 2015 09:57:54 +0800
From: Xiao Guangrong <guangrong.xiao@linux.intel.com>
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0
MIME-Version: 1.0
To: David Matlack <dmatlack@google.com>
CC: Paolo Bonzini <pbonzini@redhat.com>, Gleb Natapov <gleb@kernel.org>,
        Marcelo Tosatti <mtosatti@redhat.com>, kvm list <kvm@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 8/9] KVM: MMU: fix MTRR update
References: <1430389490-24602-1-git-send-email-guangrong.xiao@linux.intel.com> <1430389490-24602-9-git-send-email-guangrong.xiao@linux.intel.com> <CALzav=ePH1_s8iDEjFNhXMJWfemKcFHbjgS_CGu46Jyon-QRTA@mail.gmail.com>
In-Reply-To: <CALzav=ePH1_s8iDEjFNhXMJWfemKcFHbjgS_CGu46Jyon-QRTA@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2714
Lines: 73


Hi David,

Thanks for your review.

On 05/07/2015 05:36 AM, David Matlack wrote:

>> +static void vmx_set_msr_mtrr(struct kvm_vcpu *vcpu, u32 msr)
>> +{
>> +       struct mtrr_state_type *mtrr_state = &vcpu->arch.mtrr_state;
>> +       unsigned char mtrr_enabled = mtrr_state->enabled;
>> +       gfn_t start, end, mask;
>> +       int index;
>> +       bool is_fixed = true;
>> +
>> +       if (msr == MSR_IA32_CR_PAT || !enable_ept ||
>> +             !kvm_arch_has_noncoherent_dma(vcpu->kvm))
>> +               return;
>> +
>> +       if (!(mtrr_enabled & 0x2) && msr != MSR_MTRRdefType)
>> +               return;
>> +
>> +       switch (msr) {
>> +       case MSR_MTRRfix64K_00000:
>> +               start = 0x0;
>> +               end = 0x80000;
>> +               break;
>> +       case MSR_MTRRfix16K_80000:
>> +               start = 0x80000;
>> +               end = 0xa0000;
>> +               break;
>> +       case MSR_MTRRfix16K_A0000:
>> +               start = 0xa0000;
>> +               end = 0xc0000;
>> +               break;
>> +       case MSR_MTRRfix4K_C0000 ... MSR_MTRRfix4K_F8000:
>> +               index = msr - MSR_MTRRfix4K_C0000;
>> +               start = 0xc0000 + index * (32 << 10);
>> +               end = start + (32 << 10);
>> +               break;
>> +       case MSR_MTRRdefType:
>> +               is_fixed = false;
>> +               start = 0x0;
>> +               end = ~0ULL;
>> +               break;
>> +       default:
>> +               /* variable range MTRRs. */
>> +               is_fixed = false;
>> +               index = (msr - 0x200) / 2;
>> +               start = (((u64)mtrr_state->var_ranges[index].base_hi) << 32) +
>> +                      (mtrr_state->var_ranges[index].base_lo & PAGE_MASK);
>> +               mask = (((u64)mtrr_state->var_ranges[index].mask_hi) << 32) +
>> +                      (mtrr_state->var_ranges[index].mask_lo & PAGE_MASK);
>> +               mask |= ~0ULL << cpuid_maxphyaddr(vcpu);
>> +
>> +               end = ((start & mask) | ~mask) + 1;
>> +       }
>> +
>> +       if (is_fixed && !(mtrr_enabled & 0x1))
>> +               return;
>
> For variable range MTRRs, I think you want to break out here if the valid flag
> (bit 11 of the mask MTRR) is not set.

We should update these MTRRs whenever the valid bit is changed. If here we see
valid bit is zero, guest is disabling MTRR for that range so that we need to
drop cache type for that range we previously set.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/