Re: [PATCH RESEND v5 07/24] x86: refactor psr: implement get value flow.

[Yi Sun <yi.y.sun@linux.intel.com>]

Hi,

Thanks for reviewing! I agree with your comments except below one. Could you
please check my response?

On 17-01-31 15:29:34, Konrad Rzeszutek Wilk wrote:
> On Thu, Jan 19, 2017 at 02:01:09PM +0800, Yi Sun wrote:
> > +int psr_get_val(struct domain *d, unsigned int socket,
> > +                uint64_t *val, enum cbm_type type)
> >  {
> > -    return 0;
> > +    const struct psr_socket_info *info = get_socket_info(socket);
> > +    unsigned int cos = d->arch.psr_cos_ids[socket];
> > +    const struct feat_node *feat;
> > +    enum psr_feat_type feat_type;
> > +
> > +    if ( IS_ERR(info) )
> > +        return PTR_ERR(info);
> > +
> > +    feat_type = psr_cbm_type_to_feat_type(type);
> > +    list_for_each_entry(feat, &info->feat_list, list)
> > +    {
> > +        if ( feat->feature != feat_type )
> > +            continue;
> > +
> > +        if ( feat->ops.get_val(feat, cos, type, val) )
> > +            /* Found */
> 
> No need. The 'psr_get_info' does not have this.
> 
> > +            return 0;
> > +    }
> > +
> > +    return -ENOENT;
> 
> This function looks quite similar to 'psr_get_info'.
> 
> Perhaps it may make sense to have an common one that has an
> extra argument (whether to call get_val or get_feat_info)?
> 
> And then psr_get_val and psr_get_info can call in this common
> code with this extra argument attached?
>
Yes, the both functions are almost same. But I feel not good to combine them to
one function. I think it makes the interface not explicit. As there are only 3
interfaces exposed by psr.c, I want to keep current implementation. Is that
acceptable to you?

Thanks,
Sun Yi

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