> -----Original Message----- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: 08 August 2018 11:43 > To: Paul Durrant <paul.durr...@citrix.com> > Cc: Andrew Cooper <andrew.coop...@citrix.com>; George Dunlap > <george.dun...@citrix.com>; Ian Jackson <ian.jack...@citrix.com>; Wei Liu > <wei.l...@citrix.com>; Stefano Stabellini <sstabell...@kernel.org>; xen- > devel <xen-devel@lists.xenproject.org>; Konrad Rzeszutek Wilk > <konrad.w...@oracle.com>; Tim (Xen.org) <t...@xen.org> > 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.durr...@citrix.com> wrote: > >> From: Jan Beulich [mailto:jbeul...@suse.com] > >> Sent: 08 August 2018 11:30 > >> > >> >>> On 08.08.18 at 11:00, <paul.durr...@citrix.com> 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. Paul > Jan > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel