Re: [PATCH v2 4/8] x86: dynamically attach/detach CQM service for a guest

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

On 25/11/13 03:26, Xu, Dongxiao wrote:
>> -----Original Message-----
>> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
>> Sent: Thursday, November 21, 2013 8:50 PM
>> To: Xu, Dongxiao
>> Cc: xen-devel@lists.xen.org
>> Subject: Re: [Xen-devel] [PATCH v2 4/8] x86: dynamically attach/detach CQM
>> service for a guest
>>
>> On 21/11/13 07:20, dongxiao.xu@intel.com wrote:
>>> From: Dongxiao Xu <dongxiao.xu@intel.com>
>>>
>>> Add hypervisor side support for dynamically attach and detach CQM
>>> services for a certain guest.
>>>
>>> Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
>>> Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
>>> ---
>>>  xen/arch/x86/domctl.c       |   40
>> ++++++++++++++++++++++++++++++++++++++++
>>>  xen/include/public/domctl.h |   14 ++++++++++++++
>>>  2 files changed, 54 insertions(+)
>>>
>>> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
>>> index f7e4586..5ef21f9 100644
>>> --- a/xen/arch/x86/domctl.c
>>> +++ b/xen/arch/x86/domctl.c
>>> @@ -35,6 +35,7 @@
>>>  #include <asm/mem_sharing.h>
>>>  #include <asm/xstate.h>
>>>  #include <asm/debugger.h>
>>> +#include <asm/pqos.h>
>>>
>>>  static int gdbsx_guest_mem_io(
>>>      domid_t domid, struct xen_domctl_gdbsx_memio *iop)
>>> @@ -1223,6 +1224,45 @@ long arch_do_domctl(
>>>      }
>>>      break;
>>>
>>> +    case XEN_DOMCTL_attach_pqos:
>>> +    {
>>> +        if ( domctl->u.qos_res.flags & XEN_DOMCTL_ADF_pqos_cqm )
>>> +        {
>>> +            if ( !system_supports_cqm() )
>>> +                ret = -ENODEV;
>>> +            else if ( d->arch.pqos_cqm_rmid > 0 )
>>> +                ret = -EINVAL;
>> EEXISTS perhaps?
> EEXIST stands for "File exists" while EINVAL stands for "Invalid argument ".
> Here I used EINVAL because if the domain is already CQM enabled, then the "domid" parameter in "xl pqos-attach cqm domid" should be an invalid argument.
> Do you think it is OK?

I am looking for some way to distinguish "you passed an invalid flag"
(which is appropriately EINVAL), from "you are trying to attach to a
guest which already has qpos attached".

Personally, I would think that EEXISTS is fine from a hypercall, as Xen
will never have files to report about.  On the other hand, there appear
to be 0 current uses of it in Xen.

~Andrew