Re: [Xen-devel] [PATCH v4 12/24] x86: refactor psr: set value: implement write msr flow.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 11.01.17 at 07:22, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
> On 17-01-10 08:15:15, Jan Beulich wrote:
>> >>> On 14.12.16 at 05:07, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
>> > --- a/xen/arch/x86/psr.c
>> > +++ b/xen/arch/x86/psr.c
>> > @@ -186,6 +186,9 @@ struct feat_ops {
>> >      unsigned int (*exceeds_cos_max)(const uint64_t val[],
>> >                                      const struct feat_node *feat,
>> >                                      unsigned int cos);
>> > +    /* write_msr is used to write out feature MSR register. */
>> > +    int (*write_msr)(unsigned int cos, const uint64_t val[],
>> > +                     struct feat_node *feat);
>> 
>> Looks like this function again returns number-of-values, yet this time
>> without a comment saying so. While you don't need to replicate
>> that description multiple time, please at least has a brief reference.
>> That said, with the type checks moved out I think this return value
>> model won't be needed anymore - the caller, having checked the
>> type, could then simply call the get-num-val (or however it was
>> named) hook to know how many array entries to skip.
>> 
> For write msr, we may need iterate the whole feature list to write values for
> every feature if the input value is not same as old on the COS ID. So, I 
> prefer
> to keep current return value, the number-of-values handled. That would be 
> clear
> and easy to implement. Of course, we can call get_cos_num to get the returen
> value or define a macro to replace the digit. How do you think?

Well, my general reservation here is that this way you require about
half a dozen functions to all return the same value. If the value
changes (or if somebody clones the set), there's the risk of one not
getting properly updated. Therefore I'd much prefer for just one
function to return the count. And I'm relatively certain that with the
type checks moved out, this will actually end up being the more
natural way.

Jan


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