Re: [Xen-devel] [PATCH v3] x86/altp2m: Added xc_altp2m_set_mem_access_multi()

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 09.10.2017 10:23, Jan Beulich wrote:
> > > > On 06.10.17 at 18:07, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> > On 10/06/2017 06:34 PM, Jan Beulich wrote:
> > > > > > On 05.10.17 at 17:42, <ppircalabu@xxxxxxxxxxxxxxx> wrote:
> > > > @@ -4451,6 +4453,7 @@ static int do_altp2m_op(
> > > >       case HVMOP_altp2m_destroy_p2m:
> > > >       case HVMOP_altp2m_switch_p2m:
> > > >       case HVMOP_altp2m_set_mem_access:
> > > > +    case HVMOP_altp2m_set_mem_access_multi:
> > > Was it agreed that this, just like others (many wrongly, I think) is
> > > supposed to be invokable by the affected domain itself? Its non-
> > > altp2m counterpart is a DM_PRIV operation. If the one here is
> > > meant to be different, I think the commit message should say why.
> > In the absence of an answer from the designers of altp2m, we've chosen
> > to remain consistent with HVMOP_altp2m_set_mem_access - since that is
> > allowed to be invoked by the domain itself, this operation is also
> > allowed to do that.
> >
> > Back in March, I've sent a DOMCTL version:
> >
> > https://patchwork.kernel.org/patch/9633615/
> >
> > and a HVMOP version (minus the compat part):
> >
> > https://patchwork.kernel.org/patch/9612799/
> >
> > It has been discussed, and an authoritative answer on the design of this
> > was sought out, but despite several kind reminders during this time, it
> > never came. At this point, the least modification to the initial design
> > appears to be to keep the new operation as a HVMOP. This is an important
> > optimization, and the waiting period for objections has surely been
> > reasonable.
> Okay, this is (sort of) fine, but especially when there was no
> feedback the decision taken (and its reason) should be recorded
> in the commit message. As stated above as well as earlier, I
> strongly think that the altp2m permissions are too lax right now
> (and hence the patch here widens the problem, but I can agree
> that making it match set_mem_access is not unreasonable).
Of course. We'll update the commit message. Thanks for the review!


Razvan

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