[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: Jan Beulich <jbeulich@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>
  • Date: Mon, 11 Apr 2022 10:01:35 +0000
  • Accept-language: en-GB, en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.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=wzJSTetHpqccLFsLDjNzAZ2vuqpeH4DeQ/6f42uP5x8=; b=P0rwD1XBa48GKiubKU1e9EkyWhl/u63fflEIHk78+347xDU3+MuOc1xUw/lAwmiU7WUt3uKBU+4LT1/QdEoZo25E0o0ZuY075HZJSBY0UPGPZ46GtjGSquzYcN4qEwU8lQmGa/GddgBgoBv+2DrGo358/yzVKJ+8X/Xy73BqSqzO21+bHL/mLd9xJvhlz2XjKMyVartGBXlERnHDXvdgxg5N71lb+XLTiW9ot1X+mrEJ8YfIu1Wl5ez/4hH96vrXzyo5pugDy3yq5G6nnOwUGlh0iYo2/wQLakfIN1hnesOEe217Nm7VnZwRiDrsfaVwYPLgFAN0iSIPLqAPTs7Y+w==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=X9HtQoHkhDvljKqVbAoZNwD/zOxshwSRVaRally+UjqbmNGxCSuQ9WpfCRgMwqKequudj8tD/rUqK+lMOeSG/jiJdq43FKXuzLmTbcfTV4CJygtavk23KPz+CyS0hNBHRfSFRRuac+r17xUEnUn6FMxDByfiPPVOVDgH785sgi3hwO0ltZaa0ZYpavE2QAXo1uWq6jTRCb8rca+g0BAsmBgAWd75kAW2mLQ4vNu3O1wKDgkTky57llt9wdHSc9EYwoCaqoeT+ex9hHjH6oqk9x84uX36o9yapw9efIaVMjgM3yb0CbOj+mUvNkvSmAm6uGKCNq2EOAnmXueUqoVm0A==
  • Authentication-results: esa2.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: Kevin Tian <kevin.tian@xxxxxxxxx>, Paul Durrant <paul@xxxxxxx>, "Roger Pau Monne" <roger.pau@xxxxxxxxxx>
  • Delivery-date: Mon, 11 Apr 2022 10:01:54 +0000
  • Ironport-data: A9a23:39yw9awOi1f2G37Wc1p6t+cQxirEfRIJ4+MujC+fZmUNrF6WrkVVy 2sZXmDQaPvfMTPyedknaoSz9kwO7J7Tz4UySFY6+yAxQypGp/SeCIXCJC8cHc8zwu4v7q5Dx 59DAjUVBJlsFhcwnj/0bv656yMUOZigHtIQMsadUsxKbVIiGX9JZS5LwbZj2NY02IHhWmthh PupyyHhEA79s9JLGjp8B5Kr8HuDa9yr5Vv0FnRnDRx6lAe2e0s9VfrzFonoR5fMeaFGH/bSe gr25OrRElU1XfsaIojNfr7TKiXmS1NJVOSEoiI+t6OK2nCuqsGuu0qS2TV1hUp/0l20c95NJ NpltqWRaFYwMZT3seU5CAFpTWImI+p09+qSSZS/mZT7I0zudnLtx7NlDV0sPJ1e8eFyaY1M3 aVGcnZXNEnF3r/ohuLgIgVvrp1LwM3DFYUToHx/ixreCu4rW8vrSKTW/95Imjw3g6iiGN6AN 5dFMmc1MXwsZTVoGkYOBbgFhNuKjyT2SG1D73+eobootj27IAtZj+G2bYu9lsaxbcZYgEee4 H7H9mLRAxcGOdjZwj2Amlq0j/LLtTP2XsQVDrLQ3vxgjUCXx2cTIAYLTlb9qv684mauVtQaJ 0EK9y4Gqakp6FftXtT7Rwe/onOPolgbQdU4LgEhwFjTkOyOuV/fXzVaCG4aADA7iCMobT8T9 37YhtzQPhVulOaNeCzerqnTnzznbED5MlQ+TSMDSAIE5fzqr4cykg/DQ75fLUKlsjHmMWqum m7X9UDSk51W1JdWjPvjoTgrlhr2/vD0ohgJChI7t45Pxidwf8abaoOh8jA3Bt4Qfd/CHjFtU JXp8vVyDdzi77nQz0Rho81XRdlFAspp1xWG3DZS82EJrWjFxpJaVdk4DMtCDEloKN0YXjTif VXevwhcjLcKYif7M/YpON/oVJVwpUQFKTgCfqqKBuein7ArLFPXlM2QTRP4M5/RfLgEzvhkZ MbznTeEBncGE6V3pAdatM9GuYLHMhsWnDuJLbiilkzP+ePHOBa9FOdUWHPTP7tRxP7V/23oH yN3apLiJ+N3C7alPEE6MOc7cDg3EJTMLcyu8JwIK7Tbe2KL2ggJUpfs/F/oQKQ894x9nebU5 HCtHEhezVv0n3rcLguWLHtkbdvSsVxX8hrX4QRE0Y6U5kUe
  • Ironport-hdrordr: A9a23:7dyT2KzpsZmstuHp7u+fKrPxiOskLtp133Aq2lEZdPULSKOlfp GV8MjziyWYtN9IYgBcpTiBUJPwJE81bfZOkMcs1MSZLXXbUQyTXcBfBOrZsnLd8kjFmNK1up 0QCpSWZOeAbmSSyPyKmjVQcOxQgOVvkprY/ds2pk0FJWoBCsFdBkVCe32m+yVNNVN77PECZf 6hD7981lydkAMsH6OG7xc+Lor+juyOsKijTQ8NBhYh5gXLpyiv8qTGHx+R2Qpbey9TwJ85mF K10DDR1+GGibWW2xXc32jc49B9g9360OZOA8SKl4w8NijssAC1f45sMofy/gzd4dvfrWrCou O85CvIDP4DrU85uVvF+CcF7jOQlArGLUWSkWNwz0GT+vARDwhKdPapzbgpDCcxrXBQ4e2UmZ g7r15w/fBsfGL9tTW46N7SWx5wkE2o5XIkjO4IlnRaFZATcblLsOUkjQlo+Tg7bVbHAa0cYa FT5fvnlb1rmJKhHgfkl3gqxMbpUmU4Hx+ATERHssuJ0yJOlHQ8y0cD3sQQknoJ6Zp4EvB/lq j5G7UtkKsLQt4dbKp7CutEScyrCnbVSRaJNG6JO1zoGKwOJnqIoZ/q57c+4v2sZfUzvdYPsY WEVEkduX85ekroB8HL1JpX8grVSGH4RjjpwtE23ekxhlQ9fsucDcSuciFdryL7mYRtPiTyYY fHBK5r
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHYTYeuJ+is9Rj/YESzKfVlQCNZoKzqezGA
  • Thread-topic: [PATCH 1/8] IOMMU/x86: drop locking from quarantine_init() hooks

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.

~Andrew

 


Rackspace

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