[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH 03/10] xen: pci: introduce ats_list_lock
- To: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
- From: Jan Beulich <jbeulich@xxxxxxxx>
- Date: Fri, 27 Jan 2023 09:13:18 +0100
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.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=x9Vhe70Om2MBmgaGBCi2JO16S9NIgYIWitcatAN8/Q8=; b=jsKCv/y7QzH7AglA3NtSHGS7gQojpW1tdN17nEbMS4KQJ3EzxoZrHtTpTKaXjPsukHwfZhx9bLJLhlsfNybvWM8W0aQi6mJ604W4MAzMEvQoNp1ejrhjVwRRqmuYXILLQPtDrXnrtEI/rQkx54ftCEsdSVznOZVO/nUUp7LbAgXA+UqyKrrt0YRJ5tAEigzX5cwjuD8YYDMGYdhTdugvEJI3aERSe1TIYZGDzLmV6sfO1b1vEg4Gxnazso8K3/L9nJ08EhH4pHmb7siN6xq9AWAFHON06VglFXXpmtOvol/QoEnseO91885d2zSrlUiBY45E4xHVO4IymxVU+Yfofw==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Uqps4NbFksyLnoHc298MdU8OEcyTiZ2L5bqcy9Ok5o2I3kW+JLwLhjbs863WnBnhV4and/AbTPNYPNdzCTQ0KfUom8tiLt0M1cB9foUpMh3LBWAlLyOTuOwwSHI0zBdyaV/ZyzlE38CYeF+P5VVnmddYw+kSEn0mstuaGFaDleQOvbHPq+cpfXPd/BRpApkteqfUPk6aHs5td+nlcGW50zCKktD6SM49BT5bvSg9WKGfWPcMh/8FpPm9Qsh0he+PZaVdDkKpOqaYGUxe030p77yhX8XDV32vd5zhtKjFf79EWAe9QBHW/jkLHJ1UXH936M8u7gPnPy96Rf2XFtGbfQ==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
- Cc: "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, 27 Jan 2023 08:13:40 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
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.
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.
Jan
|
