Re: [Xen-devel] [PATCH v6 08/16] x86: implement set value flow for MBA

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.10.17 at 06:33, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
> On 17-10-11 07:38:52, Jan Beulich wrote:
>> >>> On 08.10.17 at 09:23, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
>> > --- a/xen/arch/x86/psr.c
>> > +++ b/xen/arch/x86/psr.c
>> > @@ -138,6 +138,12 @@ static const struct feat_props {
>> >  
>> >      /* write_msr is used to write out feature MSR register. */
>> >      void (*write_msr)(unsigned int cos, uint32_t val, enum psr_type type);
>> > +
>> > +    /*
>> > +     * check_val is used to check if input val fulfills SDM requirement.
>> > +     * Change it to valid value if SDM allows.
>> > +     */
>> > +    bool (*check_val)(const struct feat_node *feat, unsigned long *val);
>> 
>> I'm pretty sure I've said so before - "check" to me implies all r/o
>> inputs. Perhaps sanitize_val() or even just sanitize()?
>> 
>> And why unsigned long when the only caller has a uint32_t in its
>> hands?
>> 
> To be compatible with cat_check_cbm (old name is 'psr_check_cbm' in L2 
> series),
> the last parameter type is 'unsigned long'. We have discussed it in L2 patch 
> set
> v9, patch 10.

Iirc (without checking the old thread) this was for calculations to
be done as unsigned long ones. If that's the only aspect here,
then imo this is not a valid reason for the hook's parameter type
to be unsigned long *.

Jan


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