> -----Original Message----- > From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On Behalf > Of Paul Durrant > Sent: 08 August 2018 12:33 > To: 'Jan Beulich' <jbeul...@suse.com> > Cc: Stefano Stabellini <sstabell...@kernel.org>; Wei Liu > <wei.l...@citrix.com>; Andrew Cooper <andrew.coop...@citrix.com>; Tim > (Xen.org) <t...@xen.org>; George Dunlap <george.dun...@citrix.com>; Ian > Jackson <ian.jack...@citrix.com>; xen-devel <xen- > de...@lists.xenproject.org> > 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: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.
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@lists.xenproject.org > https://lists.xenproject.org/mailman/listinfo/xen-devel _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
