[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 08/18] IOMMU/x86: support freeing of pagetables


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Fri, 3 Dec 2021 10:38:54 +0100
  • 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=pkBAKchbWbpbkskL+7dSQZsjum5Q2M8NmmaKhbouStU=; b=GYrDyty0WF3EJajxx5xuXlwzLRuGlF3V6e3UpWp1a6o1I7ubKVZSnz/0EsfU/eZBQdP3YDn72PclmotBhUXbeXO9KTfioABgCAMrNFC2HZSceyb1wYSKxqBRe49gtjCuclPoQtruQEPSsKsRqdrXjYHb9q+Fsbf+00XeQ69+y4kgKeBdoThPRIwQBGTH0pw87ymzc5421+VU2Jqmh317RcAF7+0RLUcw4aRW9PInqLWxJdeb0vyN8g7ms9vl6H9C0oYsEp5SGBBP9KNfOHsoMaLX70jQxvqR4T7D18QPsLLuuC4bwvxCx/L/BQ0LPygWcq1bKWeN6JjXm2PYnFDuvA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=CvnbX1atvOPhKVwTQ+kiH8pxcjHfar+ImpD5d7bnV5+ZkseA9i7KDQxk0T0/G+azqZZE+2DRL9y9/kOXAuDdgUi2QQUZ7o8OIlkaLh0tWEV07NIdGBHwGYW8mKQHZ/rnKl3plQGWuU4WWS/B9wl5ipxujKmTumdATBhdo0LHnsFk5so1dj56vEwoLUyzsmj55PSKavzOQlcFjshBbD/+acTov6SBcRMmy7SbQWtVuyU96RsrxSWmNslC/WBuY6vnuZxOuGtEgMtDi1FMfctwsG+1Jha6gnGES27eHdKEzf/zG8pARdjbt42MWAHTGHaEMmj+V/BMGv8E9sXRQ8IoMg==
  • Authentication-results: esa3.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, "Andrew Cooper" <andrew.cooper3@xxxxxxxxxx>, Paul Durrant <paul@xxxxxxx>, Wei Liu <wl@xxxxxxx>
  • Delivery-date: Fri, 03 Dec 2021 09:39:43 +0000
  • Ironport-data: A9a23:FHTVBaqADlvp2Xfz8t4taPWiMkteBmL1YhIvgKrLsJaIsI4StFCzt garIBmAbKyINGWnf4hzOYjj9EwG6p6By9RjTgduqSg0RCND+JuZCYyVIHmrMnLJJKUvbq7GA +byyDXkBJppJpMJjk71atANlZT4vE2xbuKU5NTsY0idfic5Dnd+4f5fs7Rh2Ncx24DjW1jlV e7a+KUzBnf0g1aYDUpMg06zgEsHUCPa4W5wUvQWPJinjXeG/5UnJMt3yZKZdhMUdrJ8DO+iL 9sv+Znilo/vE7XBPfv++lrzWhVirrc/pmFigFIOM0SpqkAqSiDfTs/XnRfTAKtao2zhojx/9 DlCnYK2bRwsJo3OpPUiUQRdOQwmbZJ9x5aSdBBTseTLp6HHW37lwvEoB0AqJ4wIvO1wBAmi9 9RBdmpLNErawbvrnvTrEYGAhex6RCXvFJkYtXx6iynQEN4tQIzZQrWM7thdtNs1rp0fR66BN 5tHAdZpRDXhZRR/HGYZM4o7krupjFP/aiQHjU3A8MLb5ECMlVcsgdABKuH9YtWXQe1Fk0Deo XjJl0zmBjkKOdrZziCKmlqujOLSmSLwWKoJCaa1sPVthTW71mEVTREbS1a/if24kVKlHcJSL VQO/SgjprR081akJuQRRDXh/iTC5ERFHYMNTatqs2lh15Y4/S6HAEwfSyZhauA3i80rQBYHj g+2pOzAUGkHXKKudVqR8bKdrDWXMCcTLHMfaSJscTbp8+UPs6lo0EuRE48L/Lqdy4SsRGqum 2ziQD0W3u1L1aY2O7OHEUcrat5GjrzAVUYL6wreRQpJBSspNdf+N+REBbU2hMuszbp1rHHd7 BDoeODEtYji6K1hcgTWG43h+5nzuJ643MX02wIHInXY323FF4SfVY5R+ipiA0xiL9wJfzTkC GeK51gBuMALYyP0Nv4tC25UNyjM5fO7fTgCfqqLBuein7ArLFPXlM2QTRD4M5/RfLgEzvhkZ MbznTeEBncGE6V3pAdatM9GuYLHMhsWnDuJLbiilkzP+ePHOBa9FOdUWHPTP7tRxP7V/23oH yN3apLiJ+N3C7alPEE6MOc7cDg3EJTMLcys9pEMKLfcelEO9aNII6a5/I7NsrdNxsx9vuzJ4 mu8Sglfzl/+jmfAMgKEdjZob7aHYHq1hS9T0fUEMQn61n49T5yo6atDJZI7caN+rL5ozOJuT ulDcMKFW6wdRjPC8jUbTJ/8sI09K0j72VPQZ3KoMGolYpptZw3V4du4LAHhwzYDU3isvswkr rz+ig6CGcgfRx5vBdr9Ye60yw/jpmAUne9/BhOaItRadEj23pJtLij90q0+L80WcE2RzTqGz QeGRxwfoLCV8YMy9dDIg4GCrpuoTLQiThYLQTGD4O/vZyfA/2elzYtRa8qyfGjQBDHu5aGvR eRJ1PWgYvcJq0lH7thnGLFxwKNgu9a2/+1Gzh5pFWngZkiwDu8yOWGP2MRCu/EfxrJdvgfqC EuD9sMDZOeMMcLhVlUQOBAkfqKI0vRNwmve6vE8IUPb4i5r/eXYDRUObkfU0CENfqFoNI4Fw Ps6vJ9E4gOyvRMmL9Kag30G7G+LNHEBD/0qu5xy7FUHUeb3JoWuuaDhNxI=
  • Ironport-hdrordr: A9a23:aNRfyKOYMkWuEsBcT13155DYdb4zR+YMi2TDiHoedfUFSKOlfp 6V8MjztSWVtN4QMEtQ/+xoHJPwPE80kqQFnbX5XI3SJjUO3VHIEGgM1/qG/9SNIVybygcZ79 YeT0EcMqyBMbEZt7eD3ODQKb9Jq7PrgcPY55as854ud3AQV0gJ1XYJNu/xKDwOeOApP+tfKH LKjfA32QZINE5nI/iTNz0gZazuttfLnJXpbVovAAMm0hCHiXeN5KThGxaV8x8CW3cXqI1Svl Ttokjc3OGOovu7whjT2yv66IlXosLozp9mCNaXgsYYBz3wgkKDZZhnWZeFoDcpydvfpGoCoZ 3pmVMNLs5z43TeciWcpgbs4RDp1HIU53rr2Taj8DPeiP28YAh/J9tKhIpffBecwVEnpstA3K VC2H/cn4ZLDDvb9R6Np+TgZlVPrA6ZsHAimekcgzh0So0FcoJcqoQZ4Qd8DIoAJiTn84oqed MeTf003MwmM29yUkqp+1WGmLeXLzAO91a9MwY/U/WuontrdCsT9Tpe+CQd9k1wva7VBaM0od gsCZ4Y4Y2mePVmG56VNN1xM/dfNVa9NS4kEFjiamgPR5t3cU4klfbMkcIIDaeRCcU18Kc=
  • Ironport-sdr: LQu59hm8dc4Syty5TuTLB+HGMn8aK0deJDzAE+2itFVTR8GX9wv0i2MNbzqubARhT3CL98JXb5 tzIMz5wFBJzkDmIVChdyyMwx3FmUo/Kw3fWimfpbo7/klyQGGyZR5IA4AIGseG289Bix8QFFTY ICgUL98k1ZAzmN1ZznN1D1mIbhExXV5Ov6q/9G785gPR/UPup++dCIifs6hGPDXgdikojN94T2 UbBr7xV9BDLpaojEOsc8e8iHfkl6TPj3212Y4E2VnbWFq7AvHOfzAkFdjPRt1mEJqwm8ZtOTrT 8LNjuNQb1PbeyyOfacyoXhYk
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Fri, Dec 03, 2021 at 09:30:00AM +0100, Roger Pau Monné wrote:
> On Thu, Dec 02, 2021 at 05:10:38PM +0100, Jan Beulich wrote:
> > On 02.12.2021 17:03, Roger Pau Monné wrote:
> > > On Fri, Sep 24, 2021 at 11:48:21AM +0200, Jan Beulich wrote:
> > >> For vendor specific code to support superpages we need to be able to
> > >> deal with a superpage mapping replacing an intermediate page table (or
> > >> hierarchy thereof). Consequently an iommu_alloc_pgtable() counterpart is
> > >> needed to free individual page tables while a domain is still alive.
> > >> Since the freeing needs to be deferred until after a suitable IOTLB
> > >> flush was performed, released page tables get queued for processing by a
> > >> tasklet.
> > >>
> > >> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> > >> ---
> > >> I was considering whether to use a softirq-taklet instead. This would
> > >> have the benefit of avoiding extra scheduling operations, but come with
> > >> the risk of the freeing happening prematurely because of a
> > >> process_pending_softirqs() somewhere.
> > > 
> > > Another approach that comes to mind (maybe you already thought of it
> > > and discarded) would be to perform the freeing after the flush in
> > > iommu_iotlb_flush{_all} while keeping the per pPCU lists.
> > 
> > This was my initial plan, but I couldn't convince myself that the first
> > flush to happen would be _the_ one associated with the to-be-freed
> > page tables. ISTR (vaguely though) actually having found an example to
> > the contrary.
> 
> I see. If we keep the list per pCPU I'm not sure we could have an
> IOMMU  flush not related to the to-be-freed pages, as we cannot have
> interleaved IOMMU operations on the same pCPU.
> 
> Also, if we strictly add the pages to the freeing list once unhooked
> from the IOMMU page tables it should be safe to flush and then free
> them, as they would be no references remaining anymore.

Replying to my last paragraph: there are different types of flushes,
and they have different scopes, so just adding the pages to be freed
to a random list and expecting any flush to remove them from the IOMMU
cache is not correct.

I still think the first paragraph is accurate, as we shouldn't have
interleaving IOMMU operations on the same pCPU, so a flush on a pCPU
should always clear the entries that have been freed as a result of
the ongoing operation on that pCPU, and those operations should be
sequential.

Thanks, Roger.



 


Rackspace

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