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

["Jan Beulich" <JBeulich@suse.com>]

>>> On 09.09.14 at 08:43, <chao.p.peng@linux.intel.com> 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@linux.intel.com>
>> > ---
>> >  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).

Jan