> -----Original Message----- > From: Tian, Kevin [mailto:kevin.t...@intel.com] > Sent: 07 August 2018 03:56 > To: Paul Durrant <paul.durr...@citrix.com>; xen-devel@lists.xenproject.org > Cc: Stefano Stabellini <sstabell...@kernel.org>; Julien Grall > <julien.gr...@arm.com>; Andrew Cooper <andrew.coop...@citrix.com>; > Ian Jackson <ian.jack...@citrix.com>; Jan Beulich <jbeul...@suse.com>; > Konrad Rzeszutek Wilk <konrad.w...@oracle.com>; Tim (Xen.org) > <t...@xen.org>; Nakajima, Jun <jun.nakaj...@intel.com>; George Dunlap > <george.dun...@citrix.com> > Subject: RE: [PATCH v5 05/15] iommu: don't domain_crash() inside > iommu_map/unmap_page() > > > From: Paul Durrant [mailto:paul.durr...@citrix.com] > > Sent: Saturday, August 4, 2018 1:22 AM > > > > Turn iommu_map/unmap_page() into straightforward wrappers that check > > the > > iommu_iotlb_flush is also changed.
Indeed it is. I'll call it out. > > > existence of the relevant iommu_op and call through to it. This makes > them > > usable by PV IOMMU code to be delivered in future patches. > > Leave the decision on whether to invoke domain_crash() up to the caller. > > This has the added benefit that the (module/line number) message that > > domain_crash() spits out will be more indicative of where the problem lies. > > > > NOTE: This patch includes one bit of clean-up in set_identity_p2m_entry() > > replacing use of p2m->domain with the domain pointer passed into the > > function. > > > > Signed-off-by: Paul Durrant <paul.durr...@citrix.com> > > Reviewed-by: George Dunlap <george.dun...@eu.citrix.com> > > Reviewed-by: Wei Liu <wei.l...@citrix.com> > > Reviewed-by: Kevin Tian <kevin.t...@intel.com>, with one small comment: Thanks. > > > > > if ( need_iommu(p2m->domain) && > > (lpae_valid(orig_pte) || lpae_valid(*entry)) ) > > + { > > rc = iommu_iotlb_flush(p2m->domain, _bfn(gfn_x(sgfn)), > > 1UL << page_order); > > + if ( unlikely(rc) && !is_hardware_domain(p2m->domain) ) > > + domain_crash(p2m->domain); > > + } > > to reduce duplication, does it make sense to introduce a wrapper > like domain_crash_nd ('nd' indicate !is_hardware_domain, and > becomes a nop for hardware domain)? Then it becomes: > > if ( unlikely(rc) ) > domain_crash_nd(p2m->domain); That's a bigger change and I'd like to defer to the other maintainers as to whether they think it is a good idea. I'm happy to change this in v6 if anyone gives me a +1. Paul > > Thanks > Kevin _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel