On 08/08/18 11:43, Jan Beulich wrote: >>>> 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); > ^^^ > >>>> @@ -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?
On a tangent, why does a gnttab version of 0 exist at all? Why don't we explicitly initialise it to 1 in the hypervisor? We've had plenty of XSAs to do with mishandling of a gnttab version of 0. Why not fix the problem at its source, and simplify the gnttab code while we are at it. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
