Re: [Xen-devel] [PATCH 2/3] x86: don't ignore foreigndom input on various MMUEXT ops

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.11.14 at 13:43, <dunlapg@xxxxxxxxx> wrote:
> On Thu, Nov 20, 2014 at 10:12 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>> Instead properly fail requests that shouldn't be issued on foreign
>> domains or - for MMUEXT_{CLEAR,COPY}_PAGE - extend the existing
>> operation to work that way.
> 
> I take it this is for 4.6?

Not really, as said in the cover letter.

> I've looked through it and everything looks OK.
> 
> But I agree with Tim, that having so many different changes all at the
> same time makes the patch hard to review.
> 
> In particular, I'd rather start with a patch to get rid of "okay"
> entirely; then make MMUEXT_{CLEAR,COPY}_PAGE use foreingndom instead
> of current; then have a patch which returns -EPERM for the other ones;
> then a patch to get rid of spage in MMUEXT_[UN]MARK_SUPER.

Yes, that's how I would have done it following Tim's comments if
there wasn't the desire for backporting - that'll become quite a bit
more involved if I do the cleanup patch removing "okay" first. And
doing the -EPERM one first would mean that the backport needs to
carefully add properly setting "okay". Splitting the clear/copy page
change from the -EPERM one doesn't make much sense to me at
all considering the title (and hence purpose) of the patch.

> Regarding MMUEXT_{CLEAR,COPY}_PAGE: This is effectively changing the
> interface.  Are we sure there are no callers which just expect them to
> work on current, and don't set foreigndom properly?

As much or as little as "all of the sudden" returning -EPERM in such
a case for other sub-ops. They never were meant to be used that
way, and the original omission of those checks shouldn't preclude us
from adding them.

Jan


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