Re: [PATCH 10/17] PVH xen: introduce vmx_pvh.c and pvh.c

[Mukesh Rathor <mukesh.rathor@oracle.com>]

On Thu, 25 Apr 2013 18:16:28 -0700
Mukesh Rathor <mukesh.rathor@oracle.com> wrote:

> On Thu, 25 Apr 2013 09:36:56 +0100
> "Jan Beulich" <JBeulich@suse.com> wrote:
> 
> > >>> On 25.04.13 at 02:57, Mukesh Rathor <mukesh.rathor@oracle.com>
> > >>> wrote:
> > >> > +         */
> > >> > +        case GNTTABOP_map_grant_ref:
> > >> > +        case GNTTABOP_unmap_grant_ref:
> > >> > +        case GNTTABOP_setup_table:
> > >> > +        case GNTTABOP_copy:
> > >> > +        case GNTTABOP_query_size:
> > >> > +        case GNTTABOP_set_version:
> > >> > +            return do_grant_table_op(cmd, uop, count);
> > >> > +    }
> > >> > +    return -ENOSYS;
> > >> > +}
> > >> 
> > >> As said before - I object to this sort of white listing. A PVH
> > >> guest ought to be permitted to issue any hypercall, with the sole
> > >> exception of MMU and very few other ones. So if anything,
> > >> specific hypercall functions should be black listed.
> > > 
> > > Well, like I said before, these are verified/tested with PVH
> > > currently, and during the early stages we need to do whatever to
> > > catch things as bugs come in. I can make it DEBUG only if that
> > > makes it easier for you? I'd rather see a post here saying they
> > > got ENOSYS than saying they got weird crash/hang/etc...
> > 
> > Then this patch series really ought to continue to be RFC, and
> > I start questioning why I'm spending hours reviewing it. The
> > number of hacks you need clearly should be limited - to me it is
> > unacceptable to scatter half done code all over the tree. I had
> > the same problem when I did the 32-on-64 support, and iirc I
> > got things into largely hack free state before even posting the
> > first full, non-RFC series.
> 
> I really appreciate your time reviewing it. Given the size of the
> feature and that I'm the only one working on it, the only way I know
> is to do it in steps, and that sometimes requires temporary code.
> 
> I'll ifdef DEBUG the above code.

Acutally, on a second thought, would you be OK if I just added
return -ENOSYS to the do_grant_table_op() for calls that are not in
above list?

thx,
m