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

Re: [PATCH v2 05/18] IOMMU: have iommu_{,un}map() split requests into largest possible chunks


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Tue, 30 Nov 2021 16:24:40 +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=ISLsqZxJvVYtJW0BVuapftjJk/ha01IJN2Cj/DfsBbU=; b=I06RFNakPMmadqA4QH/yDaHLdmvqwmHLTA5w8dfIJMhw9ueIlNL8gfQeeLNADFl3DgHA9WbIzle80cicpV7FvaU6Gz5rytOM6gyJpiYtoRBUQN2oE2D73InveTlvDM6XieRLipSFsaVyFp3nOadKDQKzB2yGieg40nMhiTBm5BhYloAwZ8FIs9FSJvfgb0RI8OyGI9/O1PRYZFRDeobfqW1+Mqyh5T00xw4+rjriPvkk0JaEqker0xIAIRXZRZIbhxSTJrp6YltuXwt/R8+YfCBXibAPFwJ4vgKkL1rORDNhMbc9FNXoyZjgp3ux1+FcRwZVbpDeamXoWV3yxpafLQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=A9VyUbBmNYgOfzkkC5qJv4wYvFc54NQfdUkBXXa0FiTxskoGB81ckzL4bflk/ROv150BCTYvoUZnYcCqJSUsNVP79XARV5Lw2KcYbxxr4sMb9IfyCJfMASzcUUu1dVQer+uKpU/ga9VBM2OgX/lulgC46fhIs4DBh96ANMJPSc6NcNKnWMT/wvlKh4Lrm/4YLg1cjYUfbjYn6WKANVnDosLdUj8FU0V7ylXGgoeYzuacPq635g77nTVEbXHeCletG9vRY3cdwGzD1eEU5FvXqID6ArsOp9WtulgV2QTYaSP+lb92P1PEO4GBVfjHn2CRPzmxq6y/TrdBw3zLFyrnaA==
  • Authentication-results: esa6.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>
  • Delivery-date: Tue, 30 Nov 2021 15:25:15 +0000
  • Ironport-data: A9a23:sEPzqazDq7VlBWfZgqR6t+fuwSrEfRIJ4+MujC+fZmUNrF6WrkUBy mBJCzvXO/3ZZWahfY93aNvj8B9Tu5PXy9djTVE/ryAxQypGp/SeCIXCJC8cHc8zwu4v7q5Dx 59DAjUVBJlsFhcwnvopW1TYhSEUOZugH9IQM8aZfHAuLeNYYH1500s6wrVh2tQAbeWRWGthh /uj+6UzB3f9s9JEGjp8B3Wr8U4HUFza4Vv0j3RmDRx5lAa2e0o9VfrzEZqZPXrgKrS4K8bhL wr1IBNVyUuCl/slIovNfr8W6STmSJaKVeSFoiI+t6RPHnGuD8H9u0o2HKN0VKtZt9mGt81Jz upcmq7hczgOb6rxiuYkdjIAED4raMWq+JefSZS+mcmazkmAeHrw2fR+SkoxOOX0+M4uXzsIr 6ZBbmlQMFbT3Ipaw5riIgVort4kI8TxepsWp1lrzC3DDOZgSpfGK0nPzYIIjGhs3pESdRrYT 9MidD59TgzrWS1wYg0nUbFjld33mECqJlW0r3rK/PFqsgA/1jdZyLHwNPLFd9rMQt9a9m6Iq 2SD82nnDxUyMN2E1SHD4n+qnvXIny7wRMQVDrLQ3vxgjUCXx2cTIAYLTlb9qv684nNSQPoGd RZSoHB36/Fvqgr7FbERQiFUvlaZhhgjSvViQtcFz1CK97jW2iOyXmsbG2sphMMdiOc6Qjkj1 1msltzvBCByvLD9dU9x5ot4vhvpZ3FLcDZqiTssCFJcvoK9+N1bYgfnF447SMaIYsvJ9SYcK txghAw3nP0tgMECzM1XFniX0mv39vAlouPYjzg7v15JDCslNOZJhKTysDA3CMqsyq7CFTFtW 1BexqCjABgmV83lqcB0aLxl8EuVz/iEKibAplVkAoMs8T+gk1b6I9sAumgnfxw3aJpeEdMMX KM1kVgKjHO0FCH3BZKbnqrrU5h6pUQePYqNug/ogipmPcEqKV7vENBGbk+MxWH9+HXAYolkU ap3hf2EVC5AYYw+lWLeb75EjdcDm3BvrUuOFMuT50n2jtKjiIu9FO5t3K2mNbtisstpYWz9r r5iCid940kFDbClPHCIqdV7wJJjBSFTOK0aYvd/L4arCgFnBHsgG7nWx7YgcJZihKNbiqHD+ XTVZ6OS4AaXaaTvJVrYZ3Z9RqnoWJoj/3s3MTZ1ZQSj2mQ5YJbp56AaLsNlcb4i/e1l7Ph1U /haJJnQXqUREmzKq2YHcJ3wjI1+bxD31wiACDWoPWokdJl6Sg2XptK9Jlnz9DMDBzacvNclp +HyzRvSRJcOHlwwDMvfZP+14Um2uHwRxLB7U0fSe4EBc0Tw6ol6bSf2i6Zvcc0LLBzCwBqc1 hqXXkhE9bWc/ddt/YCQ166eroqvH+9vJWZgHjHWveSsKC3X3mu/2oscAuyGSi/QCTHv86K4a OQLk/ylaK8bnExHupZXGqpwyf5s/MPmorJXw1g2HHjPaFj3WLpsLmPfgJtKv6xJgLRYpRG3S gSE/dwDYeeFP8bsEVgwIgs5b7vciaFIy2eKtfllcl/n4CJX/aacVRQANhaBvyVRMb9pPd532 uwmosMXt1SyhxdC3gxqVcyIG7Bg9kA9bpg=
  • Ironport-hdrordr: A9a23:MYoZ1q/Vvbn2hzAxfbFuk+E8db1zdoMgy1knxilNoENuHfBwxv rDoB1E73LJYVYqOU3Jmbi7Sc69qFfnhORICOgqTMyftWzd1ldAQ7sSj7cKrweQfhEWs9QtqJ uIEJIOduEYb2IK9PoSiTPQe71LoKjlgdGVbKXlvg9QpGlRGt5dBmxCe2Cm+yNNNW177c1TLu vh2iMLnUvqRV0nKuCAQlUVVenKoNPG0LrgfB49HhYirC2Dlymh5rLWGwWRmk52aUID/Z4StU z+1yDp7KSqtP+2jjfaym/o9pxT3P/s0MFKCsCggtUcbh/slgGrToJ8XKDqhkF+nMifrHIR1P XcqRYpOMp+r1vXY2GOuBPonzLt1T4/gkWSv2OwsD/Gm4jUVTg6A81OicZyaR3C8Xctu9l6ze Ziw3+Zn4A/N2KPoA3No/zzEz16nEu9pnQv1cQJiWZEbIcYYLhN6aQC4UJuFosaFi6S0vFpLA BXNrCd2B9qSyLYU5iA1VMfguBEH05DUitue3Jy+/B8iFNt7TVEJ0hx/r1pop5PzuN4d3B+3Z W2Dk1frsA7ciYnV9MMOA4/e7rENoXse2OEDIvAGyWuKEk4U0i93qIfpo9Fo92XRA==
  • Ironport-sdr: TvPflXJMeJpYzGDSf/yH0RKmktlsvS66hbb3G29pJVpJ+D0KKz9Y0+VVsGPt2efnRr20g4gXwE JKSrNc+yLL6yHo/0ALeuAYVBUqY59KIGXbVVExJxyLv9ZhrzuN3KK0G+AaaD18RfjP3CoqOfmQ R1NWyIFmJn6syW3V4X+2iycThEJPld0ptEGcUh201KZTVdL/JJNwdoPZbDfBb9j2usQrV+cqfS OMSXej5yvPcGg4nI4y4bPAJB/qznR3n/HQtU9bBpMFx5hd7RkofpKXZY4Us9uKoja81Nb0u2k5 BGqS9UtC7cnkecbCPRxVmEAK
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Fri, Sep 24, 2021 at 11:45:57AM +0200, Jan Beulich wrote:
> Introduce a helper function to determine the largest possible mapping
> that allows covering a request (or the next part of it that is left to
> be processed).
> 
> In order to not add yet more recurring dfn_add() / mfn_add() to the two
> callers of the new helper, also introduce local variables holding the
> values presently operated on.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -260,12 +260,38 @@ void iommu_domain_destroy(struct domain
>      arch_iommu_domain_destroy(d);
>  }
>  
> -int iommu_map(struct domain *d, dfn_t dfn, mfn_t mfn,
> +static unsigned int mapping_order(const struct domain_iommu *hd,
> +                                  dfn_t dfn, mfn_t mfn, unsigned long nr)
> +{
> +    unsigned long res = dfn_x(dfn) | mfn_x(mfn);
> +    unsigned long sizes = hd->platform_ops->page_sizes;
> +    unsigned int bit = find_first_set_bit(sizes), order = 0;
> +
> +    ASSERT(bit == PAGE_SHIFT);
> +
> +    while ( (sizes = (sizes >> bit) & ~1) )
> +    {
> +        unsigned long mask;
> +
> +        bit = find_first_set_bit(sizes);
> +        mask = (1UL << bit) - 1;
> +        if ( nr <= mask || (res & mask) )
> +            break;
> +        order += bit;
> +        nr >>= bit;
> +        res >>= bit;
> +    }
> +
> +    return order;
> +}

This looks like it could be used in other places, I would at least
consider using it in pvh_populate_memory_range where we also need to
figure out the maximum order given an address and a number of pages.

Do you think you could place it in a more generic file and also use
more generic parameters (ie: unsigned long gfn and mfn)?

> +
> +int iommu_map(struct domain *d, dfn_t dfn0, mfn_t mfn0,
>                unsigned long page_count, unsigned int flags,
>                unsigned int *flush_flags)
>  {
>      const struct domain_iommu *hd = dom_iommu(d);
>      unsigned long i;
> +    unsigned int order;
>      int rc = 0;
>  
>      if ( !is_iommu_enabled(d) )
> @@ -273,10 +299,16 @@ int iommu_map(struct domain *d, dfn_t df
>  
>      ASSERT(!IOMMUF_order(flags));
>  
> -    for ( i = 0; i < page_count; i++ )
> +    for ( i = 0; i < page_count; i += 1UL << order )
>      {
> -        rc = iommu_call(hd->platform_ops, map_page, d, dfn_add(dfn, i),
> -                        mfn_add(mfn, i), flags, flush_flags);
> +        dfn_t dfn = dfn_add(dfn0, i);
> +        mfn_t mfn = mfn_add(mfn0, i);
> +        unsigned long j;
> +
> +        order = mapping_order(hd, dfn, mfn, page_count - i);
> +
> +        rc = iommu_call(hd->platform_ops, map_page, d, dfn, mfn,
> +                        flags | IOMMUF_order(order), flush_flags);
>  
>          if ( likely(!rc) )
>              continue;
> @@ -284,14 +316,18 @@ int iommu_map(struct domain *d, dfn_t df
>          if ( !d->is_shutting_down && printk_ratelimit() )
>              printk(XENLOG_ERR
>                     "d%d: IOMMU mapping dfn %"PRI_dfn" to mfn %"PRI_mfn" 
> failed: %d\n",
> -                   d->domain_id, dfn_x(dfn_add(dfn, i)),
> -                   mfn_x(mfn_add(mfn, i)), rc);
> +                   d->domain_id, dfn_x(dfn), mfn_x(mfn), rc);
> +
> +        for ( j = 0; j < i; j += 1UL << order )
> +        {
> +            dfn = dfn_add(dfn0, j);
> +            order = mapping_order(hd, dfn, _mfn(0), i - j);
>  
> -        while ( i-- )
>              /* if statement to satisfy __must_check */
> -            if ( iommu_call(hd->platform_ops, unmap_page, d, dfn_add(dfn, i),
> -                            0, flush_flags) )
> +            if ( iommu_call(hd->platform_ops, unmap_page, d, dfn, order,
> +                            flush_flags) )
>                  continue;
> +        }

Why you need this unmap loop here, can't you just use iommu_unmap?

Thanks, Roger.



 


Rackspace

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