[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




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.