On Thu, Jul 16, 2020 at 12:25:40PM +0200, Jan Beulich wrote: > On 16.07.2020 12:14, Roger Pau Monné wrote: > > On Wed, Jul 15, 2020 at 12:03:57PM +0200, Jan Beulich wrote: > >> Instead of checking inside the hook whether any non-coherent IOMMUs are > >> present, simply install the hook only when this is the case. > >> > >> To prove that there are no other references to the now dynamically > >> updated ops structure (and hence that its updating happens early > >> enough), make it static and rename it at the same time. > >> > >> Note that this change implies that sync_cache() shouldn't be called > >> directly unless there are unusual circumstances, like is the case in > >> alloc_pgtable_maddr(), which gets invoked too early for iommu_ops to > >> be set already (and therefore we also need to be careful there to > >> avoid accessing vtd_ops later on, as it lives in .init). > >> > >> Signed-off-by: Jan Beulich <jbeul...@suse.com> > > > > I think this is slightly better than what we currently have: > > > > Reviewed-by: Roger Pau Monné <roger....@citrix.com> > > Thanks. > > > I would however prefer if we also added a check to assert that > > alloc_pgtable_maddr is never called before iommu_alloc. > > It would be quite odd for this to happen - what point would > there be to allocate a table to hang off of an IOMMU when > no IOMMU was found at all so far? Furthermore, such a > restriction could either be viewed to not suffice afaict (as > a subsequent iommu_alloc() may in principle fine a non- > coherent IOMMU), or to be pointless (until a non-coherent > IOMMU was found and allocated any table for, it doesn't > really matter whether we sync cache). In the end, whether to > sync cache in alloc_pgtable_maddr() doesn't really depend on > any global property, but only on the property / properties > of the IOMMU(s) the table is going to be made visible to.
Right, I think I was indeed overly paranoid. I was mostly worried about iommu_alloc calling alloc_pgtable_maddr before checking whether the IOOMMU is incoherent, but this is not likely to happen. Thanks.
