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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.04.13 at 03:58, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> wrote:
> On Thu, 25 Apr 2013 18:16:28 -0700
> Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> wrote:
> 
>> On Thu, 25 Apr 2013 09:36:56 +0100
>> "Jan Beulich" <JBeulich@xxxxxxxx> wrote:
>> 
>> > >>> On 25.04.13 at 02:57, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>
>> > >>> 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?

On a first glance this would be at least as bogus as adding a
frontend stub.

Jan


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