On Tue, Apr 15, 2025 at 10:16 AM CLEMENT MATHIEU--DRIF <clement.mathieu--d...@eviden.com> wrote: > On 15/04/2025 2:33 pm, Stefan Hajnoczi wrote: > > On Tue, Apr 15, 2025 at 03:11:00AM -0400, Michael S. Tsirkin wrote: > >> On Tue, Apr 15, 2025 at 06:18:08AM +0000, 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> > >> > >> > >> Reviewed-by: Michael S. Tsirkin <m...@redhat.com> > >> > >> Stefan, want to pick this one up, too? > > > > Not yet, it may need to wait until after the release: > > - Discussion is still ongoing. > > - Is this a regression in 10.0 or a long-standing issue? > > It's a long standing issue
Thanks for confirming. Let's not worry about the 10.0 release. There's time to come to a consensus and the fixes can be included in stable releases afterwards. Stefan > > > - Who is affected and what is the impact? > > > > There are still a few hours left before -rc4 is tagged. I will merge it > > if consensus is reached and the missing information becomes clear. > > > > Thanks, > > Stefan > > > >> > >> > >>> --- > >>> 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); > >>> > >>> + if (take_bql) { > >>> + bql_unlock(); > >>> + } > >>> + > >>> g_hash_table_insert(s->vtd_address_spaces, new_key, vtd_dev_as); > >>> } > >>> return vtd_dev_as; > >>> -- > >>> 2.49.0 > >>