[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/2] VT-d: install sync_cache hook on demand


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Thu, 16 Jul 2020 12:14:09 +0200
  • Authentication-results: esa4.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Kevin Tian <kevin.tian@xxxxxxxxx>
  • Delivery-date: Thu, 16 Jul 2020 10:14:21 +0000
  • Ironport-sdr: cHUKvwbhjDaAptNNP9/325Js1K18w6caGyK74KVjdm+uT/0y6xo817C1iL2InG2RnfRvc+wG4F sp2o+W7JrkGVXK/1eb2gFeGnxfsyZqhzy1sxo1dDG603v/CkEc6bMuLGSii+wVsybPy5DVJPPa /CLllbFOmox41UoMJyFCJT4ZoZ2hJZtEi6Y3kZODZSBYvE+v3XJ7jHsyK0Aj2fbnrn5TSYmQSk bZEN6KqQZrRwnSEULuX/0ULTCX3OXjuQLAgcwzOVNyOx0PX3pH4zya8aqlrIH9f8ZyoFbEj6jf C7Q=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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 <jbeulich@xxxxxxxx>

I think this is slightly better than what we currently have:

Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

I would however prefer if we also added a check to assert that
alloc_pgtable_maddr is never called before iommu_alloc. We could maybe
poison the .sync_cache field, and then either set to NULL or to
sync_cache in iommu_alloc?

Maybe I'm just overly paranoid with this.

Thanks.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.