| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <556C2526.6090901@redhat.com>
Date: Mon, 01 Jun 2015 11:25:58 +0200
From: Paolo Bonzini <pbonzini@...hat.com>
To: Xiao Guangrong <guangrong.xiao@...ux.intel.com>
CC: gleb@...nel.org, mtosatti@...hat.com, kvm@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 08/15] KVM: MTRR: introduce fixed_mtrr_segment table
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@...ux.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.
> + .range_size = 1ULL << 16,
Perhaps 64 * 1024 (and so on below) is clearer, because it matches the
name of the MSR?
> + .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.
> + 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.
Paolo
> - 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 = mtrr_state->var_ranges[index].base & PAGE_MASK;
> mask = mtrr_state->var_ranges[index].mask & PAGE_MASK;
> @@ -150,38 +251,33 @@ static void update_mtrr(struct kvm_vcpu *vcpu, u32 msr)
> end = ((start & mask) | ~mask) + 1;
> }
>
> - if (is_fixed && !mtrr_state->fixed_mtrr_enabled)
> - return;
> -
> +do_zap:
> kvm_zap_gfn_range(vcpu->kvm, gpa_to_gfn(start), gpa_to_gfn(end));
> }
>
> int kvm_mtrr_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> {
> - u64 *p = (u64 *)&vcpu->arch.mtrr_state.fixed_ranges;
> + int index;
>
> if (!kvm_mtrr_valid(vcpu, msr, data))
> return 1;
>
> - if (msr == MSR_MTRRdefType)
> + index = fixed_msr_to_range_index(msr);
> + if (index >= 0)
> + *(u64 *)&vcpu->arch.mtrr_state.fixed_ranges[index] = data;
> + else if (msr == MSR_MTRRdefType)
> vcpu->arch.mtrr_state.deftype = data;
> - else if (msr == MSR_MTRRfix64K_00000)
> - p[0] = data;
> - else if (msr == MSR_MTRRfix16K_80000 || msr == MSR_MTRRfix16K_A0000)
> - p[1 + msr - MSR_MTRRfix16K_80000] = data;
> - else if (msr >= MSR_MTRRfix4K_C0000 && msr <= MSR_MTRRfix4K_F8000)
> - p[3 + msr - MSR_MTRRfix4K_C0000] = data;
> else if (msr == MSR_IA32_CR_PAT)
> vcpu->arch.pat = data;
> else { /* Variable MTRRs */
> - int idx, is_mtrr_mask;
> + int is_mtrr_mask;
>
> - idx = (msr - 0x200) / 2;
> - is_mtrr_mask = msr - 0x200 - 2 * idx;
> + index = (msr - 0x200) / 2;
> + is_mtrr_mask = msr - 0x200 - 2 * index;
> if (!is_mtrr_mask)
> - vcpu->arch.mtrr_state.var_ranges[idx].base = data;
> + vcpu->arch.mtrr_state.var_ranges[index].base = data;
> else
> - vcpu->arch.mtrr_state.var_ranges[idx].mask = data;
> + vcpu->arch.mtrr_state.var_ranges[index].mask = data;
> }
>
> update_mtrr(vcpu, msr);
> @@ -190,7 +286,7 @@ int kvm_mtrr_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
>
> int kvm_mtrr_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
> {
> - u64 *p = (u64 *)&vcpu->arch.mtrr_state.fixed_ranges;
> + int index;
>
> /* MSR_MTRRcap is a readonly MSR. */
> if (msr == MSR_MTRRcap) {
> @@ -207,25 +303,22 @@ int kvm_mtrr_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
> if (!msr_mtrr_valid(msr))
> return 1;
>
> - if (msr == MSR_MTRRdefType)
> + index = fixed_msr_to_range_index(msr);
> + if (index >= 0)
> + *pdata = *(u64 *)&vcpu->arch.mtrr_state.fixed_ranges[index];
> + else if (msr == MSR_MTRRdefType)
> *pdata = vcpu->arch.mtrr_state.deftype;
> - else if (msr == MSR_MTRRfix64K_00000)
> - *pdata = p[0];
> - else if (msr == MSR_MTRRfix16K_80000 || msr == MSR_MTRRfix16K_A0000)
> - *pdata = p[1 + msr - MSR_MTRRfix16K_80000];
> - else if (msr >= MSR_MTRRfix4K_C0000 && msr <= MSR_MTRRfix4K_F8000)
> - *pdata = p[3 + msr - MSR_MTRRfix4K_C0000];
> else if (msr == MSR_IA32_CR_PAT)
> *pdata = vcpu->arch.pat;
> else { /* Variable MTRRs */
> - int idx, is_mtrr_mask;
> + int is_mtrr_mask;
>
> - idx = (msr - 0x200) / 2;
> - is_mtrr_mask = msr - 0x200 - 2 * idx;
> + index = (msr - 0x200) / 2;
> + is_mtrr_mask = msr - 0x200 - 2 * index;
> if (!is_mtrr_mask)
> - *pdata = vcpu->arch.mtrr_state.var_ranges[idx].base;
> + *pdata = vcpu->arch.mtrr_state.var_ranges[index].base;
> else
> - *pdata = vcpu->arch.mtrr_state.var_ranges[idx].mask;
> + *pdata = vcpu->arch.mtrr_state.var_ranges[index].mask;
> }
>
> return 0;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists