>>> On 08.08.18 at 12:46, <andrew.coop...@citrix.com> wrote: > 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: >>>>> +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?
Fair question, which unfortunately I can't answer. > 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. I don't mind, unless a reason for the seemingly strange behavior can be found. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
