Re: [Xen-devel] [PATCH v15 01/11] multicall: add no preemption ability between two calls

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.09.14 at 12:51, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 09/09/14 11:39, Jan Beulich wrote:
>>>>> On 09.09.14 at 08:43, <chao.p.peng@xxxxxxxxxxxxxxx> wrote:
>>> On Fri, Sep 05, 2014 at 11:46:20AM +0100, Andrew Cooper wrote:
>>>> On 05/09/14 09:37, Chao Peng wrote:
>>>>> Add a flag to indicate if the execution can be preempted between two
>>>>> calls. If not specified, stay preemptable.
>>>>>
>>>>> Signed-off-by: Chao Peng <chao.p.peng@xxxxxxxxxxxxxxx>
>>>>> ---
>>>>>  xen/common/multicall.c   |    5 ++++-
>>>>>  xen/include/public/xen.h |    4 ++++
>>>>>  2 files changed, 8 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/xen/common/multicall.c b/xen/common/multicall.c
>>>>> index fa9d910..83b96eb 100644
>>>>> --- a/xen/common/multicall.c
>>>>> +++ b/xen/common/multicall.c
>>>>> @@ -40,6 +40,7 @@ do_multicall(
>>>>>      struct mc_state *mcs = &current->mc_state;
>>>>>      uint32_t         i;
>>>>>      int              rc = 0;
>>>>> +    bool_t           preemptable = 0;
>>>>>  
>>>>>      if ( unlikely(__test_and_set_bit(_MCSF_in_multicall, &mcs->flags)) )
>>>>>      {
>>>>> @@ -52,7 +53,7 @@ do_multicall(
>>>>>  
>>>>>      for ( i = 0; !rc && i < nr_calls; i++ )
>>>>>      {
>>>>> -        if ( i && hypercall_preempt_check() )
>>>>> +        if ( preemptable && hypercall_preempt_check() )
>>>>>              goto preempted;
>>>>>  
>>>>>          if ( unlikely(__copy_from_guest(&mcs->call, call_list, 1)) )
>>>>> @@ -61,6 +62,8 @@ do_multicall(
>>>>>              break;
>>>>>          }
>>>>>  
>>>>> +        preemptable = mcs->call.flags & MC_NO_PREEMPT;
>>>>> +
>>>> Please consider what would happen if a malicious guest set NO_PREEMPT on
>>>> every multicall entry.
>>> OK, I see. My direct purpose here is to support batch operations for
>>> XENPF_resource_op added in next patch. Recall what Jan suggested in v14
>>> comments, we have 3 possible ways to support XENPF_resource_op batch:
>>> 1) Add a field in the xenpf_resource_op to indicate the iteration;
>>> 2) Fiddle multicall mechanism, just like this patch;
>>> 3) Add a brand new hypercall.
>>>
>>> So perhaps I will give up option 2) before I can see any improvement
>>> here. While option 3) is aggressive, so I'd go option 1) through I also
>>> don't quite like it (Totally because the iteration is transparent for user).
>> The I suppose you didn't really understand Andrew's comment: I
>> don't think he was suggesting to drop the approach, but instead
>> to implement it properly (read: securely).
> 
> That is certainly one part of it.
> 
> However, there is the other open question (dropped from this context) of
> how to deal with a multicall which has NO_PREEMT set, which itself
> preempts, and I don't have a good answer for this.

The pretty natural answer to this is - the specific handler knows
best what to do.

Jan


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