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

[Paolo Bonzini <pbonzini@redhat.com>]

On 4/15/25 13:55, Philippe Mathieu-Daudé wrote:
> On 15/4/25 13:51, Paolo Bonzini wrote:
>> On Tue, Apr 15, 2025 at 1:51 PM CLEMENT MATHIEU--DRIF
>> <clement.mathieu--drif@eviden.com> wrote:
>>> On 15/04/2025 11:30 am, Paolo Bonzini 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 4/15/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-- 
>>>>> drif@eviden.com>
>>>>
>>>> Please use a separate lock instead of the BQL.
>>>
>>> Hi Paolo,
>>>
>>> We need this particular lock because some of the functions we call
>>> require the bql to be held.
>>
>> What functions do you need?
>>
>>> Is it a problem?
>>
>> It depends on the function. :)
> 
> memory_region_set_enabled()
>    -> memory_region_transaction_begin()
>       -> assert(bql_locked())

Oh, I found Yi Liu's reply that came a little before mine.

Yeah, then I guess this is unavoidable (short of adding locks to all of 
memory.c---which would be a good thing but...).  But please mention this 
in the comment that you are adding; this:

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

describes the code but does not explain it.

Paolo