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
> - 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
>>
