[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v9 13/25] x86: refactor psr: L3 CAT: set value: implement write msr flow.



>>> On 16.03.17 at 12:08, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
> @@ -421,6 +425,18 @@ static bool cat_fits_cos_max(const uint32_t val[],
>  }
>  
>  /* L3 CAT ops */
> +static void l3_cat_write_msr(unsigned int cos, uint32_t val,
> +                             enum cbm_type type, struct feat_node *feat)

"type" is an unused parameter. Please remove it from the hook
and this function.

> +{
> +    if ( feat->cos_reg_val[cos] != val )
> +    {
> +        feat->cos_reg_val[cos] = val;
> +        wrmsrl(MSR_IA32_PSR_L3_MASK(cos), (uint64_t)val);

I don't see the need for the cast.

> +    }
> +
> +    return;
> +}

Stray "return".

> +struct cos_write_info
> +{
> +    unsigned int cos;
> +    struct feat_node *feature;
> +    uint32_t val;
> +    enum cbm_type type;

With the "type" parameter removed above, this looks to be an
unused field then too. After the removal of it, please re-order
fields to leave no holes.

> +static void do_write_psr_msr(void *data)
> +{
> +    struct cos_write_info *info = (struct cos_write_info *)data;

Unnecessary cast again.

> +    unsigned int cos            = info->cos;
> +    struct feat_node *feat      = info->feature;
> +
> +    if ( !feat )
> +        return;

You shouldn't even call this function when !feat.

> +    if ( cos > feat->ops.get_cos_max(feat) )
> +        return;
> +
> +    feat->ops.write_msr(cos, info->val, info->type, feat);
> +}
> +
>  static int write_psr_msr(unsigned int socket, unsigned int cos,
>                           uint32_t val, enum cbm_type type,

"type", according to the above comments, should then go away
here too as it seems.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.