Re: [PATCH 8/9] KVM: MMU: fix MTRR update

[Xiao Guangrong <guangrong.xiao@linux.intel.com>]

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.