Re: [Xen-devel] [PATCH v9] x86/altp2m: support for setting restrictions for an array of pages

[Petre Ovidiu PIRCALABU <ppircalabu@xxxxxxxxxxxxxxx>]

On Mi, 2017-12-13 at 01:47 -0700, Jan Beulich wrote:
> >
> > >
> > > >
> > > > On 13.12.17 at 08:12, <ppircalabu@xxxxxxxxxxxxxxx> wrote:
> > @@ -4619,6 +4623,38 @@ static int do_altp2m_op(
> >                                      a.u.set_mem_access.view);
> >          break;
> >
> > +    case HVMOP_altp2m_set_mem_access_multi:
> > +        if ( a.u.set_mem_access_multi.pad ||
> > +             ( a.u.set_mem_access_multi.nr &&
> > +               a.u.set_mem_access_multi.opaque >=
> > a.u.set_mem_access_multi.nr ) )
> Why not simply >, which would avoid the need to check nr against
> zero? Also if the more involved form really needs to stay for some
> reason, then please remove the stray blanks inside the inner
> parentheses. No matter which route is chosen, I guess this could
> be taken care of while committing. Apart from this the patch looks
> okay now to me, but as indicated before I'm not really wanting to
> ack it; Tamas - having looked at this some more after the earlier
> discussion I guess my main issue is with the existence of
> XEN_ALTP2M_mixed. If there was no mode where external tools
> could compete with in-guest altp2m accesses (other than that
> allowed by XEN_ALTP2M_limited) I think I'd be fine giving my ack
> following in particular George's earlier line of arguments.
>
> Jan
>
>
> ________________________
> This email was scanned by Bitdefender

I have added the extra check because the case in which "opaque" and
"nr" are equal but not zero should be invalid (p2m_set_mem_access_multi
- the last iteration cannot be preemted). Although it's an additional
condition to check, it is not part of a performance critical path and
in my opinion, having the input parameters properly defined is worth
the trade-off.

If we accept this case as valid, although highly unlikely, (opaque ==
nr and not 0), calling p2m_set_mem_access_multi will do nothing except
locking/unlocking p2m (case in which the performance penalty will be
significantly greater).

Many thanks,
Petre

________________________
This email was scanned by Bitdefender
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel