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

Re: [PATCH v5 03/15] IOMMU/x86: support freeing of pagetables


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Tue, 31 May 2022 18:25:49 +0200
  • 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=EVAW9MHPBvthcSE5X9oVg97dafPQVum65wpdyBvxjw8=; b=U0WIQMbI/Gs6EO98mpZlxCrMCRwXoxJV62+YTXtw4YzGtXwvzGuwvg/CR6I2Fto0VKxMP+ZWS1oomUurV8gAOQgjeZ6MrCj6VeDxUWLV9aQK+6v/NrAx1tPOC3JjAmRJSLkYrz2BQIa2giz18E6mkd6nBNVZlS4tncTo89KhvID4Y1Oo2/kYyxgspDtBKzfN9vPGuWiOjxrgBsY3PbUEoWefXNMLwNPmiIS8pVBKvwiOUltZgHfxR60AIWzPAa9elqEgGYYvC0ws6pSOcaWfeZSKzo7kwUaRO8jnhm2J9tNTgl+GEW3RPr+rI36WMthKhSz5TjOCDwu0lPWQLak/gg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=UywujExXW2Bu9/UdeXRRU0W8D0rrAeeqn6U+kJXRr+oeki4hITmUSDs4HKMcBYOPC299q8/v4YqyIS9M26whsibgGDf/5wIxrGrqE31a15+qdoqKT1743GihW/WFPGiz5sv7Olm/bSj0X5pKcSpfgLwytO9mPUdVVD+qfekR2hXOQ2WyXSe9lmPBB8Zn85bPi/bAY/+ncGfeE6UuzOCGkK1Adnk0vuK8mmJSNQj6vzEcms3FQRh4ypyTamuxE7fzpGraV9RYzrY5synsw1BaA6QcB8jHenN0MxiCZwVqmwpcGqNCRLtrhPqnT7/6xt3qcME9uiYOYvgFSyS35v9Jhg==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Paul Durrant <paul@xxxxxxx>
  • Delivery-date: Tue, 31 May 2022 16:26:06 +0000
  • Ironport-data: A9a23:zFmOga7QtKAWKnaPHMpCrAxRtE/GchMFZxGqfqrLsTDasY5as4F+v jFOUWGBaPaCYmXwe4p/PIux8RtVu5PUmINlSwBt+CA3Hi5G8cbLO4+Ufxz6V8+wwmwvb67FA +E2MISowBUcFyeEzvuVGuG96yE6j8lkf5KkYAL+EnkZqTRMFWFw0HqPp8Zj2tQy2YXiWlvX0 T/Pi5a31GGNimYc3l08s8pvmDs31BglkGpF1rCWTakjUG72zxH5PrpGTU2CByKQrr1vNvy7X 47+IISRpQs1yfuP5uSNyd4XemVSKlLb0JPnZnB+A8BOiTAazsA+PzpS2FPxpi67hh3Q9+2dx umhurSxbFwCYKH1hd1edDB8DxAiYpFEofjYdC3XXcy7lyUqclPK6tA2UAQaGNNd/ex6R2ZT6 fYfNTYBKAiZgP67y666Te8qgdk/KM7sP8UUvXQIITPxVK56B8ycBfqRo4MJtNszrpkm8fL2f c0WZCApdB3dSxZOJk0WGNQ1m+LAanzXLGcF8w3I+fRfD277nCtx9aryOtXuPcXRX8kSu32en EjI8DGsav0dHJnFodafyVqujOLSmSLwWKoJCaa1sPVthTW71mEVTREbS1a/if24kVKlHcJSL VQO/SgjprR081akJvHlVgC8iG6JuFgbQdU4LgEhwASEy66R7wPHAGEBFm5FcIZ+6J5wQiE23 FiUmd+vHSZorLCeVXOa8PGTsC+2Pi8Wa2QFYEfoUDc43jUqm6lr5jqnczqpOPTdYgHdcd0o/ w23kQ==
  • Ironport-hdrordr: A9a23:zYBax6kbUjzWdYgFWpx/Z97oGhvpDfOwimdD5ihNYBxZY6Wkfp +V8cjzhCWftN9OYhodcLC7V5Voj0mskKKdxbNhRYtKPTOWwVdASbsP0WKM+V3d8kHFh41gPO JbAtND4b7LfCRHZKTBkW6F+r8bqbHokZxAx92uqUuFJTsaFp2IhD0JbjpzfHcGJjWvUvECZe ChD4d81kydUEVSSv7+KmgOXuDFqdGOvJX6YSQeDxpizAWVlzun5JPzDhDdh34lInty6IZn1V KAvx3y562lvf3+4hjA11XL55ATvNf60NNMCOGFl8BQADTxjQSDYphnRtS5zXkIidDqzGxvvM jHoh8mMcg2w3TNflutqR+o4AXk2CZG0Q6W9XaoxV/Y5eDpTjMzDMRMwahDdAHC1kYmtNZglI pWwmOwrfNsfF/9tRW4w+KNewBhl0Kyr3Znu/UUlWZjXYwXb6IUhZAD/XlSDIwLEEvBmc0a+d FVfY/hDcttABKnhyizhBgu/DXsZAV4Iv6+eDlMhiTPuAIm30yQzCMjtb4idzk7hdAAoqJ/lp r525RT5c5zp/AtHNNA7cc6ML+K4z/2MGXx2Fz7GyWWKIg3f1TwlrXQ3JIZoMmXRb1g9upApH 2GaiISiVIP
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Fri, May 27, 2022 at 01:13:09PM +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-tasklet 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.
> ---
> v5: Fix CPU_UP_PREPARE for BIGMEM. Schedule tasklet in CPU_DOWN_FAILED
>     when list is not empty. Skip all processing in CPU_DEAD when list is
>     empty.
> v4: Change type of iommu_queue_free_pgtable()'s 1st parameter. Re-base.
> v3: Call process_pending_softirqs() from free_queued_pgtables().
> 
> --- a/xen/arch/x86/include/asm/iommu.h
> +++ b/xen/arch/x86/include/asm/iommu.h
> @@ -147,6 +147,7 @@ void iommu_free_domid(domid_t domid, uns
>  int __must_check iommu_free_pgtables(struct domain *d);
>  struct domain_iommu;
>  struct page_info *__must_check iommu_alloc_pgtable(struct domain_iommu *hd);
> +void iommu_queue_free_pgtable(struct domain_iommu *hd, struct page_info *pg);
>  
>  #endif /* !__ARCH_X86_IOMMU_H__ */
>  /*
> --- a/xen/drivers/passthrough/x86/iommu.c
> +++ b/xen/drivers/passthrough/x86/iommu.c
> @@ -12,6 +12,7 @@
>   * this program; If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> +#include <xen/cpu.h>
>  #include <xen/sched.h>
>  #include <xen/iocap.h>
>  #include <xen/iommu.h>
> @@ -566,6 +567,98 @@ struct page_info *iommu_alloc_pgtable(st
>      return pg;
>  }
>  
> +/*
> + * Intermediate page tables which get replaced by large pages may only be
> + * freed after a suitable IOTLB flush. Hence such pages get queued on a
> + * per-CPU list, with a per-CPU tasklet processing the list on the assumption
> + * that the necessary IOTLB flush will have occurred by the time tasklets get
> + * to run. (List and tasklet being per-CPU has the benefit of accesses not
> + * requiring any locking.)
> + */
> +static DEFINE_PER_CPU(struct page_list_head, free_pgt_list);
> +static DEFINE_PER_CPU(struct tasklet, free_pgt_tasklet);
> +
> +static void free_queued_pgtables(void *arg)
> +{
> +    struct page_list_head *list = arg;
> +    struct page_info *pg;
> +    unsigned int done = 0;
> +
> +    while ( (pg = page_list_remove_head(list)) )
> +    {
> +        free_domheap_page(pg);
> +
> +        /* Granularity of checking somewhat arbitrary. */
> +        if ( !(++done & 0x1ff) )
> +             process_pending_softirqs();

Hm, I'm wondering whether we really want to process pending softirqs
here.

Such processing will prevent the watchdog from triggering, which we
likely want in production builds.  OTOH in debug builds we should make
sure that free_queued_pgtables() doesn't take longer than a watchdog
window, or else it's likely to cause issues to guests scheduled on
this same pCPU (and calling process_pending_softirqs() will just mask
it).

Thanks, Roger.



 


Rackspace

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