Re: [PATCH] xen: Fix XSM build following c/s 92942fd

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

On 09/02/16 17:05, Jan Beulich wrote:
>>>> On 09.02.16 at 17:21, <andrew.cooper3@citrix.com> wrote:
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> I'm sorry for the breakage / not noticing.
>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Tim Deegan <tim@xen.org>
>> CC: Ian Campbell <Ian.Campbell@citrix.com>
>> CC: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>>
>> Is this actually an appropraite fix?  Software relying on -ENOSYS for Xen
>> feature detection is going to break when running under an XSM hypervisor.
> That's a valid concern, and it's certainly not nice for XSM to need
> tweaking here at all. Perhaps ...
>
>> --- a/xen/xsm/flask/hooks.c
>> +++ b/xen/xsm/flask/hooks.c
>> @@ -1421,7 +1421,6 @@ static int flask_shadow_control(struct domain *d, uint32_t op)
>>          break;
>>      case XEN_DOMCTL_SHADOW_OP_ENABLE:
>>      case XEN_DOMCTL_SHADOW_OP_ENABLE_TEST:
>> -    case XEN_DOMCTL_SHADOW_OP_ENABLE_TRANSLATE:
>>      case XEN_DOMCTL_SHADOW_OP_GET_ALLOCATION:
>>      case XEN_DOMCTL_SHADOW_OP_SET_ALLOCATION:
>>          perm = SHADOW__ENABLE;
> ... rather than just removing the case it should be moved to a
> separate case yielding -ENOSYS or -EOPNOTSUPP (right now
> shadow_domctl() returns -EINVAL anyway afaics)? (This of
> course would mean that we can't completely suppress the
> XEN_DOMCTL_SHADOW_OP_ENABLE_TRANSLATE #define in
> public/domctl.h. We could limit it to __XEN__ though.)

The problem this creates is that we gain two locations prescribing the
authoritative set of supported actions, which is one too many.

The first question is whether blanket -EPERMs are ok.  This is the
current behaviour, and as such, this patch should probably be taken in
this form.  (i.e. fix the build and maintain the status quo).

If blanket -EPERMs are not ok, we are going to need some longer work to
make the XSM checks finer grained, and after the primary -ENOSYS checks.

~Andrew