Re: [Xen-devel] [PATCH v21 1/2] common: add a new mappable resource type: XENMEM_resource_grant_table

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Xen-devel [mailto:xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx] On Behalf
> Of Paul Durrant
> Sent: 08 August 2018 12:33
> To: 'Jan Beulich' <JBeulich@xxxxxxxx>
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>; Wei Liu
> <wei.liu2@xxxxxxxxxx>; Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Tim
> (Xen.org) <tim@xxxxxxx>; George Dunlap <George.Dunlap@xxxxxxxxxx>; Ian
> Jackson <Ian.Jackson@xxxxxxxxxx>; xen-devel <xen-
> devel@xxxxxxxxxxxxxxxxxxxx>
> Subject: Re: [Xen-devel] [PATCH v21 1/2] common: add a new mappable
> resource type: XENMEM_resource_grant_table
> 
> > -----Original Message-----
> > From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> > Sent: 08 August 2018 11:43
> > To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> > Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; George Dunlap
> > <George.Dunlap@xxxxxxxxxx>; Ian Jackson <Ian.Jackson@xxxxxxxxxx>; Wei
> Liu
> > <wei.liu2@xxxxxxxxxx>; Stefano Stabellini <sstabellini@xxxxxxxxxx>; xen-
> > devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>; Konrad Rzeszutek Wilk
> > <konrad.wilk@xxxxxxxxxx>; Tim (Xen.org) <tim@xxxxxxx>
> > Subject: RE: [PATCH v21 1/2] common: add a new mappable resource type:
> > XENMEM_resource_grant_table
> >
> > >>> On 08.08.18 at 12:38, <Paul.Durrant@xxxxxxxxxx> wrote:
> > >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> > >> Sent: 08 August 2018 11:30
> > >>
> > >> >>> On 08.08.18 at 11:00, <paul.durrant@xxxxxxxxxx> wrote:
> > >> > @@ -3860,6 +3866,47 @@ int mem_sharing_gref_to_gfn(struct
> > >> grant_table *gt, grant_ref_t ref,
> > >> >  }
> > >> >  #endif
> > >> >
> > >> > +/* caller must hold write lock */
> > >> > +static int gnttab_get_status_frame_mfn(struct domain *d,
> > >> > +                                       unsigned long idx, mfn_t *mfn)
> > >> > +{
> > >> > +    struct grant_table *gt = d->grant_table;
> > >>
> > >> const?
> > >>
> > >
> > > IIRC that didn't work because gnttab_grow_table() modifies the content.
> >
> > But you don't pass gt to the function:
> >
> > >> > +    ASSERT(gt->gt_version == 2);
> > >> > +
> > >> > +    if ( idx >= nr_status_frames(gt) )
> > >> > +    {
> > >> > +        unsigned long nr = status_to_grant_frames(idx + 1);
> > >> > +
> > >> > +        if ( nr <= gt->max_grant_frames )
> > >> > +            gnttab_grow_table(d, nr);
> >
> > ^^^
> 
> I know, I was just remembering that the compiler complained when I tried
> this before. My memory could be wrong so I'll try it again.
> 
> >
> > >> > @@ -3906,6 +3943,38 @@ int gnttab_map_frame(struct domain *d,
> > >> unsigned long idx, gfn_t gfn,
> > >> >      return rc;
> > >> >  }
> > >> >
> > >> > +int gnttab_get_shared_frame(struct domain *d, unsigned long idx,
> > >> > +                            mfn_t *mfn)
> > >> > +{
> > >> > +    struct grant_table *gt = d->grant_table;
> > >> > +    int rc;
> > >> > +
> > >> > +    grant_write_lock(gt);
> > >> > +
> > >> > +    if ( gt->gt_version == 0 )
> > >> > +        gt->gt_version = 1;
> > >>
> > >> Since you've moved this here instead of dropping it, what requirement
> > >> have you found for this to be set (other than the ASSERT() you put in
> > >> gnttab_get_shared_frame_mfn()?
> > >>
> > >
> > > The code in patch #2 is executed before the grant table version is set. I
> > > could alternatively have libxl explicitly set the version to 1 before 
> > > trying
> > > to seed the table.
> >
> > But that's not my point. What's wrong with leaving it at zero?
> >
> 
> I'm not particularly happy calling gnttab_grow_table() with version left at 0
> but I can try it and see if it breaks.

Actually, no. There is nowhere else that leaves it at 0 and I find that I can't 
set the version explicitly from the toolstack as gnttab_set_version doesn't 
take a domid as a parameter. I'll leave the version setting as-is.

  Paul

> 
>   Paul
> 
> > Jan
> >
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel