Re: [Xen-devel] [PATCH v2 3/6] hvm/mtrr: use the hardware number of variable ranges for Dom0

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.05.18 at 15:41, <roger.pau@xxxxxxxxxx> wrote:
> On Wed, May 16, 2018 at 06:01:00AM -0600, Jan Beulich wrote:
>> --- unstable.orig/xen/arch/x86/hvm/mtrr.c
>> +++ unstable/xen/arch/x86/hvm/mtrr.c
>> @@ -676,22 +676,22 @@ int hvm_set_mem_pinned_cacheattr(struct
>>  
>>  static int hvm_save_mtrr_msr(struct domain *d, hvm_domain_context_t *h)
>>  {
>> -    int i;
>>      struct vcpu *v;
>> -    struct hvm_hw_mtrr hw_mtrr;
>> -    struct mtrr_state *mtrr_state;
>> +
>>      /* save mtrr&pat */
>>      for_each_vcpu(d, v)
>>      {
>> -        mtrr_state = &v->arch.hvm_vcpu.mtrr;
>> +        const struct mtrr_state *mtrr_state = &v->arch.hvm_vcpu.mtrr;
>> +        struct hvm_hw_mtrr hw_mtrr = {
>> +            .msr_mtrr_def_type = mtrr_state->def_type |
>> +                                 (mtrr_state->enabled << 10),
>> +            .msr_mtrr_cap      = mtrr_state->mtrr_cap,
>> +        };
>> +        unsigned int i;
>>  
>>          hvm_get_guest_pat(v, &hw_mtrr.msr_pat_cr);
>>  
>> -        hw_mtrr.msr_mtrr_def_type = mtrr_state->def_type
>> -                                | (mtrr_state->enabled << 10);
>> -        hw_mtrr.msr_mtrr_cap = mtrr_state->mtrr_cap;
>> -
>> -        for ( i = 0; i < MTRR_VCNT; i++ )
>> +        for ( i = 0; i < (uint8_t)hw_mtrr.msr_mtrr_cap; i++ )
> 
> I've changed this to use MASK_EXTR instead (and switched the cases
> below).

Thanks, that was the intention anyway.

>> @@ -729,6 +729,14 @@ static int hvm_load_mtrr_msr(struct doma
>>      if ( hvm_load_entry(MTRR, h, &hw_mtrr) != 0 )
>>          return -EINVAL;
>>  
>> +    if ( (uint8_t)hw_mtrr.msr_mtrr_cap > MTRR_VCNT )
> 
> And here I would compare against the VCNT in mtrr_state->mtrr_cat
> instead of MTRR_VCNT if you agree.

No, I don't agree: We're _defining_ the number of MTRRs for this vCPU
here. There's no difference if such a record is loaded only once, or if all
such records agree in count. But I don't see why we should refuse
loading a record with a count higher than that provided by an earlier
record, but no larger than MTRR_VCNT.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel