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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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.

Jan


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