[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v12 15/23] x86: refactor psr: CDP: implement set value callback function.



On 17-06-30 03:33:17, Jan Beulich wrote:
> >>> Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx> 06/30/17 10:56 AM >>>
> >On 17-06-30 15:22:56, Yi Sun wrote:
> >> On 17-06-30 00:42:22, Jan Beulich wrote:
> >> > >          {
> >> > >              val[i] = new_val;
> >> > > -            return 0;
> >> > > +            ret = 0;
> >> > >          }
> >> > 
> >> > Wouldn't it be better to return -EINVAL in a to be added else branch here
> >> > and ...
> >> > 
> >After reading codes again, I think we cannot return -EINVAL in else branch 
> >here.
> >E.g. for CDP, user wants to set CODE. Then, the 'type' is CODE. At the first
> >iteration, the props->type[0] is DATA which does not match 'type'. But we 
> >cannot
> >return error here. We should iterate next 'type[]'.
> 
> But that's why you're adding the second check in the if(). If neither of the 
> two
> conditions are true, this is an error, isn't it? On the converse, why would 
> you
> return 0 if the first loop iteration is fine but the second isn't.
> 
The second check in the if() is for the case to set both DATA and CODE at same
time. The above example I wrote is for the case that user just wants to set CODE
but not DATA.

Under such case, the input 'feat_type' is CDP so that the second check is always
false.

The input 'type' is CODE. The props->type[0] is DATA and props->type[1] is CODE.
In the first iteration, the props->type[0] is DATA so that it does not match
'type' and the second check is false too. If we use else branch here, it will
enter the branch and return -EVINVAL. But this is not we want, right? We hope
the second iteration should be executed to set CODE.
 
> Jan

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.