On Tuesday 16 June 2009 20:05:29 Marcelo Tosatti wrote:
> Do not allow invalid MTRR/PAT values in set_msr_mtrr.
>
> Please review carefully.
>
> Signed-off-by: Marcelo Tosatti <[email protected]>
>
Looks fine to me.
Is it necessary to check reserved bit of MSR_MTRRdefType and variable MTRRs as
well? Maybe like this:
if (msr == MSR_MTRRdefType) {
return valid_mtrr_type(data & ~0xc00ull);
}
And variable ones can be:
#define MTRR_VALID_MASK(v, msr) (~(rsvd_bits(cpuid_max_physaddr(v)) | ((msr %
2) << 11)))
return valid_mtrr_type(data & MTRR_VALID_MASK(vcpu, msr)))
(rsvd_bits() is in mmu.c, both untested)
Maybe we can put cpuid_max_physaddr as a field in vcpu struct?
--
regards
Yang, Sheng
>
> Index: kvm/arch/x86/kvm/x86.c
> ===================================================================
> --- kvm.orig/arch/x86/kvm/x86.c
> +++ kvm/arch/x86/kvm/x86.c
> @@ -722,11 +722,53 @@ static bool msr_mtrr_valid(unsigned msr)
> return false;
> }
>
> +static unsigned mtrr_types[] = {0, 1, 4, 5, 6};
> +static unsigned pat_types[] = {0, 1, 4, 5, 6, 7};
> +
> +static bool valid_mt(unsigned type, int len, unsigned array[len])
> +{
> + int i;
> +
> + for (i = 0; i < len; i++)
> + if (type == array[i])
> + return true;
> +
> + return false;
> +}
> +
> +#define valid_pat_type(a) valid_mt(a, ARRAY_SIZE(pat_types), pat_types)
> +#define valid_mtrr_type(a) valid_mt(a, ARRAY_SIZE(mtrr_types), mtrr_types)
> +
> +static bool mtrr_valid(u32 msr, u64 data)
> +{
> + int i;
> +
> + if (!msr_mtrr_valid(msr))
> + return false;
> +
> + if (msr == MSR_IA32_CR_PAT) {
> + for (i = 0; i < 8; i++)
> + if (!valid_pat_type((data >> (i * 8)) & 0xff))
> + return false;
> + return true;
> + } else if (msr == MSR_MTRRdefType) {
> + return valid_mtrr_type(data & 0xff);
> + } else if (msr >= MSR_MTRRfix64K_00000 && msr <= MSR_MTRRfix4K_F8000) {
> + for (i = 0; i < 8 ; i++)
> + if (!valid_mtrr_type((data >> (i * 8)) & 0xff))
> + return false;
> + return true;
> + }
> +
> + /* variable MTRRs, physmaskn have bits 0-10 reserved */
> + return valid_mtrr_type(data & 0xff);
> +}
> +
> static int set_msr_mtrr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> {
> u64 *p = (u64 *)&vcpu->arch.mtrr_state.fixed_ranges;
>
> - if (!msr_mtrr_valid(msr))
> + if (!mtrr_valid(msr, data))
> return 1;
>
> if (msr == MSR_MTRRdefType) {
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
