On 15/04/2025 8:53 am, Philippe Mathieu-Daudé wrote:
> Caution: External email. Do not open attachments or click links, unless
> this email comes from a known sender and you know the content is safe.
>
>
> 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 <[email protected]>
>> ---
>> 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);
Hi phil, vtd_switch_address_space already does the same kind of check
>
>>
>> + if (take_bql) {
>> + bql_unlock();
>> + }
>> +
>> g_hash_table_insert(s->vtd_address_spaces, new_key,
>> vtd_dev_as);
>> }
>> return vtd_dev_as;
>
