lkml.org 
[lkml]   [2015]   [Jun]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 08/15] KVM: MTRR: introduce fixed_mtrr_segment table


On 06/01/2015 05:25 PM, Paolo Bonzini wrote:
>
>
> On 30/05/2015 12:59, Xiao Guangrong wrote:
>> This table summarizes the information of fixed MTRRs and introduce some APIs
>> to abstract its operation which helps us to clean up the code and will be
>> used in later patches
>>
>> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
>> ---
>> arch/x86/kvm/mtrr.c | 191 ++++++++++++++++++++++++++++++++++++++--------------
>> 1 file changed, 142 insertions(+), 49 deletions(-)
>>
>> diff --git a/arch/x86/kvm/mtrr.c b/arch/x86/kvm/mtrr.c
>> index d3c06d2..888441e 100644
>> --- a/arch/x86/kvm/mtrr.c
>> +++ b/arch/x86/kvm/mtrr.c
>> @@ -102,12 +102,126 @@ bool kvm_mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data)
>> }
>> EXPORT_SYMBOL_GPL(kvm_mtrr_valid);
>>
>> +struct fixed_mtrr_segment {
>> + u64 start;
>> + u64 end;
>> +
>> + /*
>> + * unit corresponds to the MSR entry in this segment, the size
>> + * of unit is covered in one MSR.
>> + */
>> + u64 unit_size;
>> +
>> + /* a range is covered in one memory cache type. */
>> + u64 range_size;
>> +
>> + /* the start position in kvm_mtrr.fixed_ranges[]. */
>> + int range_start;
>> +};
>> +
>> +static struct fixed_mtrr_segment fixed_seg_table[] = {
>> + /* MSR_MTRRfix64K_00000, 1 unit. 64K fixed mtrr. */
>> + {
>> + .start = 0x0,
>> + .end = 0x80000,
>> + .unit_size = 0x80000,
>
> Unit size is always range size * 8.

Good catch, will drop this.

>
>> + .range_size = 1ULL << 16,
>
> Perhaps 64 * 1024 (and so on below) is clearer, because it matches the
> name of the MSR?

Yes, it's better.

>
>> + .range_start = 0,
>> + },
>> +
>> + /*
>> + * MSR_MTRRfix16K_80000 ... MSR_MTRRfix16K_A0000, 2 units,
>> + * 16K fixed mtrr.
>> + */
>> + {
>> + .start = 0x80000,
>> + .end = 0xc0000,
>> + .unit_size = (0xc0000 - 0x80000) / 2,
>> + .range_size = 1ULL << 14,
>> + .range_start = 8,
>> + },
>> +
>> + /*
>> + * MSR_MTRRfix4K_C0000 ... MSR_MTRRfix4K_F8000, 8 units,
>> + * 4K fixed mtrr.
>> + */
>> + {
>> + .start = 0xc0000,
>> + .end = 0x100000,
>> + .unit_size = (0x100000 - 0xc0000) / 8,
>> + .range_size = 1ULL << 12,
>> + .range_start = 24,
>> + }
>> +};
>> +
>> +static int fixed_msr_to_range_index(u32 msr)
>> +{
>> + int seg, unit;
>> +
>> + if (!fixed_msr_to_seg_unit(msr, &seg, &unit))
>> + return -1;
>> +
>> + fixed_mtrr_seg_unit_range_index(seg, unit);
>
> This looks wrong.

Oops, should be "return fixed_mtrr_seg_unit_range_index(seg, unit);" :(

>
>> + return 0;
>> +}
>> +
>> static void update_mtrr(struct kvm_vcpu *vcpu, u32 msr)
>> {
>> struct kvm_mtrr *mtrr_state = &vcpu->arch.mtrr_state;
>> gfn_t start, end, mask;
>> int index;
>> - bool is_fixed = true;
>>
>> if (msr == MSR_IA32_CR_PAT || !tdp_enabled ||
>> !kvm_arch_has_noncoherent_dma(vcpu->kvm))
>> @@ -116,32 +230,19 @@ static void update_mtrr(struct kvm_vcpu *vcpu, u32 msr)
>> if (!mtrr_state->mtrr_enabled && msr != MSR_MTRRdefType)
>> return;
>>
>> + if (fixed_msr_to_range(msr, &start, &end)) {
>> + if (!mtrr_state->fixed_mtrr_enabled)
>> + return;
>> + goto do_zap;
>> + }
>> +
>> switch (msr) {
>
> Please move defType handling in an "if" above "if
> (fixed_msr_to_range(msr, &start, &end))". Then you don't need the goto.
>

Yes, indeed. Will do it in the next version.


\
 
 \ /
  Last update: 2015-06-03 05:01    [W:0.122 / U:0.048 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site