Re: [PATCH v4 02/12] x86/mm: add HYPERVISOR_memory_op to acquire guest resources

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: 07 September 2017 12:36
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>; xen-devel@lists.xenproject.org; Andrew
> Cooper <Andrew.Cooper3@citrix.com>; Jan Beulich <jbeulich@suse.com>
> Subject: Re: [Xen-devel] [PATCH v4 02/12] x86/mm: add
> HYPERVISOR_memory_op to acquire guest resources
> 
> On Thu, Sep 07, 2017 at 12:18:25PM +0100, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: Wei Liu [mailto:wei.liu2@citrix.com]
> > > Sent: 07 September 2017 12:11
> > > To: Paul Durrant <Paul.Durrant@citrix.com>
> > > Cc: xen-devel@lists.xenproject.org; Andrew Cooper
> > > <Andrew.Cooper3@citrix.com>; Jan Beulich <jbeulich@suse.com>; Wei
> Liu
> > > <wei.liu2@citrix.com>
> > > Subject: Re: [Xen-devel] [PATCH v4 02/12] x86/mm: add
> > > HYPERVISOR_memory_op to acquire guest resources
> > >
> > > On Tue, Sep 05, 2017 at 12:37:06PM +0100, Paul Durrant wrote:
> > > [...]
> > > >
> > > > +static int xenmem_acquire_grant_table(struct domain *d,
> > > > +                                      unsigned long frame,
> > > > +                                      unsigned long nr_frames,
> > > > +                                      unsigned long mfn_list[])
> > > > +{
> > > > +    unsigned int i;
> > > > +
> > > > +    /*
> > > > +     * Iterate through the list backwards so that gnttab_get_frame() is
> > > > +     * first called for the highest numbered frame. This means that the
> > > > +     * out-of-bounds check will be done on the first iteration and, if
> > > > +     * the table needs to grow, it will only grow once.
> > > > +     */
> > > > +    i = nr_frames;
> > > > +    while ( i-- != 0 )
> > > > +    {
> > > > +        mfn_t mfn = gnttab_get_frame(d, frame + i);
> > > > +
> > >
> > > I think you should lock guest grant table first and use the _locked
> > > variant here to get a consistent view of guest grant table frames.
> >
> > Once the table has grown, is there any way they can change?
> >
> 
> Hmm... no.
> 
> I think you can leave the code as-is.
> 
> >
> > Ok, if you think it's necessary. (This is a tools-only hypercall and the ranges
> are supplied by privcmd, allocated in kernel).
> >
> 
> IMHO we should allow for use case for semi-trusted users of this
> hypercall in the future.

Ok, I'll send a v5.

Cheers,

  Paul

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