Re: [Xen-devel] [PATCH v2 07/15] x86: implement set value flow for MBA

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

On 17-08-31 09:30:47, Roger Pau Monn� wrote:
> On Thu, Aug 31, 2017 at 10:20:10AM +0800, Yi Sun wrote:
> > On 17-08-30 09:31:04, Roger Pau Monn� wrote:
> > > On Thu, Aug 24, 2017 at 09:14:41AM +0800, Yi Sun wrote:
> > > > diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
> > > > index 4a0c982..ce82975 100644
> > > > --- 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_val_type type);
> > > > +
> > > > +    /*
> > > > +     * check_val is used to check if input val fulfills SDM 
> > > > requirement.
> > > > +     * Change it to valid value if SDM allows.
> > > 
> > > I'm not really sure it's a good idea to change the value to a valid
> > > one, IMHO you should just check and print an error if the value is
> > > invalid (and return false of course).
> > > 
> > Per SDM, the HW has ability to automatically change the input value to what 
> > it
> > wants. E.g:
> >   Linear mode: HW wants the input value be 10/20/30/.../90. But user inputs 
> > 15.
> >                Then, HW can automatically change it to 10.
> 
> This seems like a very, very bad idea IMHO. What if I input 15 (which
> is invalid), but I would rather prefer 20 instead of 10?
> 
> I would very much prefer an error, so I can get to chose a valid value
> myself instead of the hardware deciding on my back.
> 
> > Even user inputs a value that does not fulfill HW requirement, HW can 
> > handle it.
> > So, we do not need return error to user. Otherwise, user needs to know 
> > details
> > of MBA.
> > 
> > But the issue here is how we get the actual value and show it to user. 
> > There are
> > two ways to do that:
> > 1. When setting value, check and change it to valid one and save it to our 
> > cache.
> 
> Instead of performing those checks manually, why not simply write the
> value and read it back to see the one the hardware has actually
> chosen (and cache it)?
> 
So, I guess you prefer optioin 2, right? If so, do we need check if input value
is valid? I see above comment that you prefer to return an error and leave the
decision to user.

> Is there any risk in writing an invalid value?

If input value is no more than the thrtl_max got through CPUID, it is fine that
HW can handle it.

> 
> > 2. When getting value, call rdmsr to read the actual value back from HW.
> > 

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