Re: [Xen-devel] [PATCH v16 06/13] x86/hvm: Introduce hvm_save_mtrr_msr_one func

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.08.18 at 11:20, <aisaila@xxxxxxxxxxxxxxx> wrote:
> --- a/xen/arch/x86/hvm/mtrr.c
> +++ b/xen/arch/x86/hvm/mtrr.c
> @@ -718,52 +718,56 @@ int hvm_set_mem_pinned_cacheattr(struct domain *d, 
> uint64_t gfn_start,
>      return 0;
>  }
>  
> -static int hvm_save_mtrr_msr(struct domain *d, hvm_domain_context_t *h)
> +static int hvm_save_mtrr_msr_one(struct vcpu *v, hvm_domain_context_t *h)
>  {
> -    struct vcpu *v;
> +    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 |
> +                             MASK_INSR(mtrr_state->fixed_enabled,
> +                                       MTRRdefType_FE) |
> +                             MASK_INSR(mtrr_state->enabled, MTRRdefType_E),
> +        .msr_mtrr_cap      = mtrr_state->mtrr_cap,
> +    };
> +    unsigned int i;
>  
> -    /* save mtrr&pat */
> -    for_each_vcpu(d, v)
> +    if ( MASK_EXTR(hw_mtrr.msr_mtrr_cap, MTRRcap_VCNT) >
> +         (ARRAY_SIZE(hw_mtrr.msr_mtrr_var) / 2) )
>      {
> -        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 |
> -                                 MASK_INSR(mtrr_state->fixed_enabled,
> -                                           MTRRdefType_FE) |
> -                                 MASK_INSR(mtrr_state->enabled, 
> MTRRdefType_E),
> -            .msr_mtrr_cap      = mtrr_state->mtrr_cap,
> -        };
> -        unsigned int i;
> +        dprintk(XENLOG_G_ERR,
> +                "HVM save: %pv: too many (%lu) variable range MTRRs\n",
> +                v, MASK_EXTR(hw_mtrr.msr_mtrr_cap, MTRRcap_VCNT));
> +        return -EINVAL;
> +    }
>  
> -        if ( MASK_EXTR(hw_mtrr.msr_mtrr_cap, MTRRcap_VCNT) >
> -             (ARRAY_SIZE(hw_mtrr.msr_mtrr_var) / 2) )
> -        {
> -            dprintk(XENLOG_G_ERR,
> -                    "HVM save: %pv: too many (%lu) variable range MTRRs\n",
> -                    v, MASK_EXTR(hw_mtrr.msr_mtrr_cap, MTRRcap_VCNT));
> -            return -EINVAL;
> -        }
> +    hvm_get_guest_pat(v, &hw_mtrr.msr_pat_cr);
>  
> -        hvm_get_guest_pat(v, &hw_mtrr.msr_pat_cr);
> +    for ( i = 0; i < MASK_EXTR(hw_mtrr.msr_mtrr_cap, MTRRcap_VCNT); i++ )
> +    {
> +        hw_mtrr.msr_mtrr_var[i * 2] = mtrr_state->var_ranges->base;
> +        hw_mtrr.msr_mtrr_var[i * 2 + 1] = mtrr_state->var_ranges->mask;
> +    }
>  
> -        for ( i = 0; i < MASK_EXTR(hw_mtrr.msr_mtrr_cap, MTRRcap_VCNT); i++ )
> -        {
> -            /* save physbase */
> -            hw_mtrr.msr_mtrr_var[i*2] =
> -                ((uint64_t*)mtrr_state->var_ranges)[i*2];
> -            /* save physmask */
> -            hw_mtrr.msr_mtrr_var[i*2+1] =
> -                ((uint64_t*)mtrr_state->var_ranges)[i*2+1];
> -        }
> +    BUILD_BUG_ON(sizeof(hw_mtrr.msr_mtrr_fixed) !=
> +                 sizeof(mtrr_state->fixed_ranges));
>  
> -        for ( i = 0; i < NUM_FIXED_MSR; i++ )
> -            hw_mtrr.msr_mtrr_fixed[i] =
> -                ((uint64_t*)mtrr_state->fixed_ranges)[i];
> +    memcpy(hw_mtrr.msr_mtrr_fixed, mtrr_state->fixed_ranges, 
> sizeof(hw_mtrr.msr_mtrr_fixed));

Long line.

> -        if ( hvm_save_entry(MTRR, v->vcpu_id, h, &hw_mtrr) != 0 )
> -            return 1;
> +    return hvm_save_entry(MTRR, v->vcpu_id, h, &hw_mtrr);
> +}
> +
> +static int hvm_save_mtrr_msr(struct domain *d, hvm_domain_context_t *h)
> +{
> +    struct vcpu *v;
> +    int err = 0;
> +
> +    /* save mtrr&pat */
> +    for_each_vcpu(d, v)
> +    {
> +       err = hvm_save_mtrr_msr_one(v, h);
> +       if ( err )
> +           break;
>      }
> -    return 0;
> +    return err;
>  }

Please, once again, take the opportunity to add the missing blank line
ahead of the function's main (only) return statement. With this (which
can be done while committing, should no other need for a v17 arise)
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan


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