[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: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Tue, 12 Apr 2022 14:17:16 +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=I0bbBtWWSE9c+Jrhd3uPxpdOIXBy9jgJsEvB3lTQxl8=; b=Ez5psYSacCmjgrffmy6KDyi/ejbiitsppTvFFx3Mu+uS1J8sNR+OtpOqOcxb6hyZQ2yCYB/Sc0ONMeFYwWZxChgZd6e+SXxFxbepUekzWRLsxIFC7suizpKBq8mfqcB7jCB12j/ZpbOxRWNLQmyVxvE7nQ/dujTBvsAmKAg0fApPBZQs52TwEifekN2xte4C+uLeOmbMtwKbA9nt9AlBQikxcjS0eI91JQ/1Hj7vvTtzJc6wIjIRit65l/5ksaWhRVadwhDGELhqYErhVgNnfSb/q0dbvyGYBDiEvsXyAjGkhJfa+fVZ0Lw1FJvcJ0Vx14cbebfLDVNFKbdurwuT+g==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=k84pVLPmP1E1Is+tFPCtAdWH9m86B/RQToi+ZEIcuozsCQNhdan5ycNUcFRhBd9giO4vPn4yQZ0pDnOB+EZ3XD5n2sQcKdUDiEfY7+GoocHRgt+FP75LtDyPLXnrEGoFtpl7KTA5wqjpQ9xLRbYR1hPvNyz3jYHSM7/wvru5lg9rsTxPZ/WAA7pbhbQ8Jm3IEBmniD/8ySD00EouhAILDUxWYXPJlewLd4az2hlh7zHQHF3N66bS1WrWRpsjAF2QBLUoBhg06XBExkyRONkDzoZWmCrQTiXlV8VVYlrue6wGtRYE4Y8dmY0N91Yp2tUMsbbqdSPjZJE/HhYE4fy08g==
  • 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>, Kevin Tian <kevin.tian@xxxxxxxxx>, Paul Durrant <paul@xxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Delivery-date: Tue, 12 Apr 2022 12:17:38 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 12.04.2022 13:05, Roger Pau Monné wrote:
> On Mon, Apr 11, 2022 at 11:35:54AM +0200, 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.
> 
> I would explicitly say that none of the code in the locked region
> touches any data in the domain_iommu struct, so taking the
> mapping_lock is unneeded.

But that would limit what the mapping_lock protects more than it actually
does: The entire page tables hanging off of the root table are also
protected by that lock. It's just that for a pdev, after having
installed identity mappings, the root doesn't hang off of hd. But in
principle - i.e. if the per-device mappings weren't static once created -
the lock would be the one to hold whenever any of these page tables was
modified.

> Long term we might wish to implemented a per-device lock that could be
> used here, instead of relying on the pcidevs lock.

Well, I would want to avoid this unless a need arises to not hold the
pcidevs lock here. Or, of course, if a need arose to dynamically alter
these page tables.

>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks.

Jan




 


Rackspace

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