[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/8] IOMMU/x86: drop locking from quarantine_init() hooks
- To: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>
- From: Jan Beulich <jbeulich@xxxxxxxx>
- Date: Tue, 12 Apr 2022 15:14:52 +0200
- 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=mrXLvynpQtfJ1BHM6+6H2L3/h6HhE7FyYSNJwGux3i4=; b=Xy14g7X4QKFBAYFOZZ4gnZrKAx0ysK7MHaYDI0QKolDGxZ0wdAd7cdVt7EjVGAOoddYDPVyozoZpPCPSDKTO16EVrL2UeFl+eXWBRiU0EaWzx0JNEESar6kFdp4SABpQ4PQTIryTT4wTI1kOxGr1qB4hNdJWrBI5MAsbESovrvKDhE6qI2DAQ+KEPi2kz35gRf77Q2IdX1h979D1HiHiivAmPxIeTQBtv0Sd5cZvgmb5AYd61uIEBRNMffTqg3mNEUcDfspHG34kk4ftgNJ7kjyoBkKA9q8615n8fPUVx8W1k2dcdx+bcUNyF/+cNcv/1CnAU0r+rNIkmaJIljxcFA==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Y1s1JEu/Rh3TnvmNo/Nlpx/6FLizba+HKhOW9LrR0wOAHGrGfvX5czviY3Lo6nEjBsl2kWOyLB25ca73IInR0zN6O7FpqbH8l/tD2DNZB9fJebTkfWKqIrZ+teuGJmwSBE6QTJRhWHsSbmybNc7FLqD0b1DQKEK8Ks5goL5D2INsjcDjzAIMXZCjhrmWg4kJYgWSSEc/sNwXyWNqm3E73AELJqMYdfF7MvieCR1nj4wXG8FNGpEmW6L/rWTYGz8DjMXHYAF5iWJD6LNKIuS99crHFmtbnf5ROjSDGbiO9oHq7GsHLwp67Ce3RdKLZSfFurOHOBoyC6Kd0YfpuK+KYg==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
- Cc: Kevin Tian <kevin.tian@xxxxxxxxx>, Paul Durrant <paul@xxxxxxx>, Roger Pau Monne <roger.pau@xxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
- Delivery-date: Tue, 12 Apr 2022 13:15:02 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 11.04.2022 12:01, Andrew Cooper wrote:
> On 11/04/2022 10:35, Jan Beulich wrote:
>> Prior extension of these functions to enable per-device quarantine page
>> tables already didn't add more locking there, but merely left in place
>> what had been there before. But really locking is unnecessary here:
>> We're running with pcidevs_lock held (i.e. multiple invocations of the
>> same function [or their teardown equivalents] are impossible, and hence
>> there are no "local" races), while all consuming of the data being
>> populated here can't race anyway due to happening sequentially
>> afterwards. See also the comment in struct arch_pci_dev.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> It is only legitimate to drop these calls if you delete the mapping_lock
> entirely. Otherwise you're breaking the semantics of mapping_lock.
>
> Your argument of "well this is already guarded by the pci lock" means
> that these are uncontended lock/unlock operations, and therefore not
> interesting to drop in the first place.
>
> This patch is specifically setting us up for an XSA in the future when
> the behaviour of the the PCI lock changes, the fix for which will be
> revert this patch.
Further to my earlier reply, may I remind you that changes to the PCI
lock won't go unnoticed here, as there are respective ASSERT()s in
place.
Jan
|