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

[Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx>]

On 17-10-12 03:43:26, Jan Beulich wrote:
> >>> 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 *.
> 
Because below macros used in cat_check_cbm require the input addr to be unsigned
long, we define the last parameter of cat_check_cbm to be unsigned long.
    find_first_bit
    find_next_zero_bit
    find_next_bit

If you think the unsigned long is not appropriate for 'check_val', I think I
have to define a local variable in cat_check_cbm to do the convertion.

> Jan

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