Re: [Xen-devel] [PATCH 1/2] xen/arm: add support for vm_assist hypercall

[Juergen Gross <jgross@xxxxxxxx>]

On 20/05/16 17:33, Jan Beulich wrote:
>>>> On 20.05.16 at 17:08, <JGross@xxxxxxxx> wrote:
>> On 20/05/16 16:51, Jan Beulich wrote:
>>>>>> On 20.05.16 at 16:42, <JGross@xxxxxxxx> wrote:
>>>> On 20/05/16 16:34, Jan Beulich wrote:
>>>>>>>> On 20.05.16 at 15:22, <JGross@xxxxxxxx> wrote:
>>>>>> --- a/xen/common/domain.c
>>>>>> +++ b/xen/common/domain.c
>>>>>> @@ -1408,7 +1408,6 @@ long do_vcpu_op(int cmd, unsigned int vcpuid, 
>>>> XEN_GUEST_HANDLE_PARAM(void) arg)
>>>>>>      return rc;
>>>>>>  }
>>>>>>  
>>>>>> -#ifdef VM_ASSIST_VALID
>>>>>>  long vm_assist(struct domain *p, unsigned int cmd, unsigned int type,
>>>>>>                 unsigned long valid)
>>>>>>  {
>>>>>> @@ -1427,7 +1426,6 @@ long vm_assist(struct domain *p, unsigned int cmd, 
>>>>>> unsigned int type,
>>>>>>  
>>>>>>      return -ENOSYS;
>>>>>>  }
>>>>>> -#endif
>>>>>>  
>>>>>>  struct pirq *pirq_get_info(struct domain *d, int pirq)
>>>>>>  {
>>>>>> --- a/xen/common/kernel.c
>>>>>> +++ b/xen/common/kernel.c
>>>>>> @@ -441,12 +441,10 @@ DO(nmi_op)(unsigned int cmd, 
>>>> XEN_GUEST_HANDLE_PARAM(void) arg)
>>>>>>      return rc;
>>>>>>  }
>>>>>>  
>>>>>> -#ifdef VM_ASSIST_VALID
>>>>>>  DO(vm_assist)(unsigned int cmd, unsigned int type)
>>>>>>  {
>>>>>>      return vm_assist(current->domain, cmd, type, VM_ASSIST_VALID);
>>>>>>  }
>>>>>> -#endif
>>>>>
>>>>> Removing these #ifdef-s is neither necessary for this patch (at least
>>>>> afaict) nor desirable (after all they had got added so that an arch
>>>>> doesn't get this code compiled for no reason).
>>>>
>>>> Removing is not necessary, right.
>>>>
>>>> OTOH there is no arch left needing those #ifdef-s to be in place. Or do
>>>> you think we should guard each single functionality in xen/common by
>>>> such means? I don't think so. In this case keeping the #ifdef-s would be
>>>> for historical reasons only.
>>>
>>> No, I don't want to go overboard with this. But we added these
>>> not so long ago, so I see no reason why they should now be
>>> removed again, just to maybe have them added in a couple of
>>> years again.
>>
>> Hmm, those #ifdef-s where needed for ARM, as on ARM there just was no
>> flag to be set via vm_assist hypercall. The new flag added in the next
>> patch is suitable for all architectures, so there would be no reason
>> for not supporting vm_assist in a new to be supported architecture.
> 
> Hmm, you have a point here. Otoh new architectures should
> probably assume that behavior even without having explicitly
> asked for it.

I don't think so, but you are the maintainer. In case there are no
objections by other maintainers I'll leave the #ifdef-s in place.


Juergen


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