Re: [PATCH 3/5] x86: add domctl cmd to set/get CDP code/data CBM

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 06/09/15 08:15, He Chen wrote:
> On Wed, Sep 02, 2015 at 12:59:07PM +0100, Andrew Cooper wrote:
>> On 02/09/15 09:28, He Chen wrote:
>>> CDP extends CAT and provides the capacity to control L3 code & data
>>> cache. With CDP, one COS correspond to two CMBs(code & data). cbm_type
>>> is added to support distinguish different CBM operation. Besides, new
>>> domctl cmds are introdunced to support set/get CDP CBM. Some CAT
>>> functions to operation CBMs are extended to support CDP.
>>>
>>> Signed-off-by: He Chen <he.chen@linux.intel.com>
>>> ---
>>>   xen/arch/x86/domctl.c       |  33 +++++++++-
>>>   xen/arch/x86/psr.c          | 142 ++++++++++++++++++++++++++++++++------------
>>>   xen/include/asm-x86/psr.h   |  12 +++-
>>>   xen/include/public/domctl.h |   4 ++
>>>   4 files changed, 150 insertions(+), 41 deletions(-)
>>>
>>> diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
>>> index 26596dd..8e92d24 100644
>>> --- a/xen/arch/x86/psr.c
>>> +++ b/xen/arch/x86/psr.c
>>> +static int pick_avail_cos(struct psr_cat_cbm *map, int cos_max, int old_cos)
>>> +{
>>> +    int cos;
>>> +
>>> +    /* If old cos is referred only by the domain, then use it. */
>>> +    if ( map[old_cos].ref == 1 )
>>> +        return old_cos;
>>> +
>>> +    /* Then we pick an unused one, never pick 0 */
>>> +    for ( cos = 1; cos <= cos_max; cos++ )
>>> +        if ( map[cos].ref == 0 )
>>> +            return cos;
>>> +
>>> +    return -EOVERFLOW;
>> ENOENT surely, or use EOVERFLOW consistently.
>>
> I am not sure I got your point here. pick_avail_cos is to get an unused
> COS, if succeed, it returns a positive number which means COS, but
> when fail, it should return a negative number to indicate an error.
>
> As far as I know, ENOENT is 2 and EOVERFLOW is 75, if I return ENOENT
> directly, the function which call pick_avail_cos could not tell the
> value is a valid COS or an error number.
>
> Would you mind explaining in more detail for me and thanks.

You are mixing at matching use of ENOENT and EOVERFLOW for the same kind 
of failure from different subroutines.  Please be consistent (and negative).

~Andrew