Hi, On 11/05/17 12:12, Jan Beulich wrote:
On 12.05.17 at 04:42, <xiong.y.zh...@intel.com> wrote:Commit 6d774a951696 ("x86/ioreq server: synchronously reset outstanding p2m_ioreq_server entries when an ioreq server unmaps") introduced p2m_finish_type_change(), which was meant to synchronously finish a previously initiated type change over a gpfn range. It did this by calling get_entry(), checking if it was the appropriate type, and then calling set_entry().Unfortunately, a previous commit (1679e0df3df6 "x86/ioreq server: asynchronously reset outstanding p2m_ioreq_server entries") modified get_entry() to always return the new type after the type change, meaning that p2m_finish_type_change() never changed any entries. Which means when an ioreq server was detached and then re-attached (as happens in XenGT on reboot) the re-attach failed. Fix this by using the existing p2m-specific recalculation logic instead of doing a read-check-write loop. Fix: 'commit 6d774a951696 ("x86/ioreq server: synchronously reset outstanding p2m_ioreq_server entries when an ioreq server unmaps")' Signed-off-by: Xiong Zhang <xiong.y.zh...@intel.com> Signed-off-by: Yu Zhang <yu.c.zh...@linux.intel.com> Reviewed-by: George Dunlap <george.dun...@ctrix.com>Reviewed-by: Jan Beulich <jbeul...@suse.com> albeit I'm not overly happy with ...+int p2m_finish_type_change(struct domain *d, + gfn_t first_gfn, unsigned long max_nr) { struct p2m_domain *p2m = p2m_get_hostp2m(d); - p2m_type_t t; unsigned long gfn = gfn_x(first_gfn); unsigned long last_gfn = gfn + max_nr - 1; - - ASSERT(ot != nt); - ASSERT(p2m_is_changeable(ot) && p2m_is_changeable(nt)); + int rc = 0; p2m_lock(p2m); last_gfn = min(last_gfn, p2m->max_mapped_pfn); while ( gfn <= last_gfn ) { - get_gfn_query_unlocked(d, gfn, &t); - - if ( t == ot ) - p2m_change_type_one(d, gfn, t, nt); + rc = p2m->recalc(p2m, gfn); + /* + * ept->recalc could return 0/1/-ENOMEM. pt->recalc could return + * 0/-ENOMEM/-ENOENT, -ENOENT isn't an error as we are looping + * gfn here. + */ + if ( rc == -ENOENT)
NIT: space missing before )
+ rc = 0; + else if ( rc < 0 ) + { + gdprintk(XENLOG_ERR, "p2m->recalc failed! Dom%d gfn=%lx\n", + d->domain_id, gfn);... a message being logged here. Also I'm sure it was pointed out before that if this is meant for 4.9 (which I assume it is) you should have Cc-ed Julien (now added).
Xiong Zhang, can you confirm this is meant for Xen 4.9? Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
