Re: [PATCH] intel_iommu: Take the bql before registering a new address space

[Philippe Mathieu-Daudé]

On 15/4/25 08:18, CLEMENT MATHIEU--DRIF wrote:

Address space creation might end up being called without holding the
bql as it is exposed through the IOMMU ops.

Signed-off-by: Clement Mathieu--Drif <clement.mathieu--d...@eviden.com>
---
  hw/i386/intel_iommu.c | 10 ++++++++++
  1 file changed, 10 insertions(+)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index dffd7ee885..fea2220013 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -4216,6 +4216,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, 
PCIBus *bus,
      vtd_dev_as = g_hash_table_lookup(s->vtd_address_spaces, &key);
      if (!vtd_dev_as) {
          struct vtd_as_key *new_key = g_malloc(sizeof(*new_key));
+        bool take_bql = !bql_locked();

new_key->bus = bus;

          new_key->devfn = devfn;
@@ -4238,6 +4239,11 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, 
PCIBus *bus,
          vtd_dev_as->context_cache_entry.context_cache_gen = 0;
          vtd_dev_as->iova_tree = iova_tree_new();

+ /* Some functions in this branch require the bql, make sure we own it */

+        if (take_bql) {
+            bql_lock();
+        }
+
          memory_region_init(&vtd_dev_as->root, OBJECT(s), name, UINT64_MAX);
          address_space_init(&vtd_dev_as->as, &vtd_dev_as->root, "vtd-root");

@@ -4305,6 +4311,10 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, vtd_switch_address_space(vtd_dev_as);

Would it help clarifying to propagate this argument down?
vtd_switch_address_space(VTDAddressSpace *as, bool need_lock);

+ if (take_bql) {

+            bql_unlock();
+        }
+
          g_hash_table_insert(s->vtd_address_spaces, new_key, vtd_dev_as);
      }
      return vtd_dev_as;