[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH 03/10] xen: pci: introduce ats_list_lock
- To: Jan Beulich <jbeulich@xxxxxxxx>
- From: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
- Date: Fri, 17 Feb 2023 01:20:30 +0000
- Accept-language: en-US
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=epam.com; dmarc=pass action=none header.from=epam.com; dkim=pass header.d=epam.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=3fNDbS5wRCEYx1LoxfhMr/zCSSmLAzvIcCA6XdVAClM=; b=Bmo9T5NuvtQ5x1xcuYWHugkoxaFNhl1BSq+hM2ziJdZ8nP/kgzPouEkW9DuPQl3aV0DnljjFdNc0NF1reSf9/TWSsin4mlxF1qy9W/TkCsFPt0ZVBhSwFhGjnAyyHLBlDfgiWR1QpEyST1oweZqFWPTtpEXx4Y0ho9KextV3XowRVx2heq68Yo9kDM7jHvX6P1ktf27hQm9zkvSPgnuRYgttjYFcyuYcKkupCZHOmJ86v+5m8FHu2zn7s2CcGpK1z8bbfw+wnAaBa+hRQpdBkJeqAOyWi7vi41ZC6omSJfSMz7yo5tkne7oiI2oeGL+YxhG25xoa4CIRv126Jnzj8Q==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ROjgkqMMmuin4U/ptMVaxKP2RzZ/RAjPrFYHnc2pacIgf2CgDmyg7AkYKbxS3YBjBLxuDxO6O0COSS8km4xkmKPpGBCSFobcGBQK9e8lesU53ORfLJ+/zHZTH+jWU20DeR+gEXPAyndl9CVEiaGr513DRroOCF2q34LqVotTTsqenZl95pKlLu8wHPhhUJ6xvA4GStqKHWFauvGPOJxwI9c6uda4SwAVkR8nxaLknPS2k6kQaarR7X92Lw7JwEH4Ii/Zb4CjFXNUxyOFUjEat0CowubZLo4ETEfAzD7p7QlRnWEONJlo/z6PeGImBhMgnYWge2wnEn4xb2dNuz+fbw==
- Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Paul Durrant <paul@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Kevin Tian <kevin.tian@xxxxxxxxx>
- Delivery-date: Fri, 17 Feb 2023 01:21:17 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
- Thread-index: AQHYvUN+A/bxwqt+90CvUR3e9vnmcK6ySQOAgACK4gCAIIy7AA==
- Thread-topic: [RFC PATCH 03/10] xen: pci: introduce ats_list_lock
Hello Jan,
Jan Beulich <jbeulich@xxxxxxxx> writes:
> On 27.01.2023 00:56, Stefano Stabellini wrote:
>> On Wed, 31 Aug 2022, Volodymyr Babchuk wrote:
>>> --- a/xen/drivers/passthrough/pci.c
>>> +++ b/xen/drivers/passthrough/pci.c
>>> @@ -1641,6 +1641,7 @@ void iommu_dev_iotlb_flush_timeout(struct domain *d,
>>> struct pci_dev *pdev)
>>> {
>>> pcidevs_lock();
>>>
>>> + /* iommu->ats_list_lock is taken by the caller of this function */
>>
>> This is a locking inversion. In all other places we take pcidevs_lock
>> first, then ats_list_lock lock. For instance look at
>> xen/drivers/passthrough/pci.c:deassign_device that is called with
>> pcidevs_locked and then calls iommu_call(... reassign_device ...) which
>> ends up taking ats_list_lock.
>>
>> This is the only exception. I think we need to move the
>> spin_lock(ats_list_lock) from qinval.c to here.
>
> First question here is what the lock is meant to protect: Just the list,
> or also the ATS state change (i.e. serializing enable and disable against
> one another). In the latter case the lock also wants naming differently.
My intention was to protect list only. But I believe you are right and
we should protect the whole state. I'll rename the lock to ats_lock.
> Second question is who is to acquire the lock. Why isn't this done _in_
> {en,dis}able_ats_device() themselves? That would then allow to further
> reduce the locked range, because at least the pci_find_ext_capability()
> call and the final logging can occur without the lock held.
You are right, I'll extended {en,dis}able_ats_device() API to pass
pointer to the lock.
|
