|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 13/13] [RFC] iommu: AMD-Vi: Squash map_pages/unmap_pages with map_page/unmap_page
Hi, all.
Any comments?
On Tue, Jul 25, 2017 at 8:26 PM, Oleksandr Tyshchenko
<olekstysh@xxxxxxxxx> wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
>
> Reduce the scope of the TODO by squashing single-page stuff with
> multi-page one. Next target is to use large pages whenever possible
> in the case that hardware supports them.
>
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> CC: Jan Beulich <jbeulich@xxxxxxxx>
> CC: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
>
> ---
> Changes in v1:
> -
>
> Changes in v2:
> -
>
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> ---
> xen/drivers/passthrough/amd/iommu_map.c | 250
> ++++++++++++++++----------------
> 1 file changed, 121 insertions(+), 129 deletions(-)
>
> diff --git a/xen/drivers/passthrough/amd/iommu_map.c
> b/xen/drivers/passthrough/amd/iommu_map.c
> index ea3a728..22d0cc6 100644
> --- a/xen/drivers/passthrough/amd/iommu_map.c
> +++ b/xen/drivers/passthrough/amd/iommu_map.c
> @@ -631,188 +631,180 @@ static int update_paging_mode(struct domain *d,
> unsigned long gfn)
> return 0;
> }
>
> -static int __must_check amd_iommu_map_page(struct domain *d, unsigned long
> gfn,
> - unsigned long mfn,
> - unsigned int flags)
> +/*
> + * TODO: Optimize by using large pages whenever possible in the case
> + * that hardware supports them.
> + */
> +int __must_check amd_iommu_map_pages(struct domain *d, unsigned long gfn,
> + unsigned long mfn,
> + unsigned int order,
> + unsigned int flags)
> {
> - bool_t need_flush = 0;
> struct domain_iommu *hd = dom_iommu(d);
> int rc;
> - unsigned long pt_mfn[7];
> - unsigned int merge_level;
> + unsigned long orig_gfn = gfn;
> + unsigned long i;
>
> if ( iommu_use_hap_pt(d) )
> return 0;
>
> - memset(pt_mfn, 0, sizeof(pt_mfn));
> -
> spin_lock(&hd->arch.mapping_lock);
> -
> rc = amd_iommu_alloc_root(hd);
> + spin_unlock(&hd->arch.mapping_lock);
> if ( rc )
> {
> - spin_unlock(&hd->arch.mapping_lock);
> AMD_IOMMU_DEBUG("Root table alloc failed, gfn = %lx\n", gfn);
> domain_crash(d);
> return rc;
> }
>
> - /* Since HVM domain is initialized with 2 level IO page table,
> - * we might need a deeper page table for lager gfn now */
> - if ( is_hvm_domain(d) )
> + for ( i = 0; i < (1UL << order); i++, gfn++, mfn++ )
> {
> - if ( update_paging_mode(d, gfn) )
> + bool_t need_flush = 0;
> + unsigned long pt_mfn[7];
> + unsigned int merge_level;
> +
> + memset(pt_mfn, 0, sizeof(pt_mfn));
> +
> + spin_lock(&hd->arch.mapping_lock);
> +
> + /* Since HVM domain is initialized with 2 level IO page table,
> + * we might need a deeper page table for lager gfn now */
> + if ( is_hvm_domain(d) )
> + {
> + if ( update_paging_mode(d, gfn) )
> + {
> + spin_unlock(&hd->arch.mapping_lock);
> + AMD_IOMMU_DEBUG("Update page mode failed gfn = %lx\n", gfn);
> + domain_crash(d);
> + rc = -EFAULT;
> + goto err;
> + }
> + }
> +
> + if ( iommu_pde_from_gfn(d, gfn, pt_mfn) || (pt_mfn[1] == 0) )
> {
> spin_unlock(&hd->arch.mapping_lock);
> - AMD_IOMMU_DEBUG("Update page mode failed gfn = %lx\n", gfn);
> + AMD_IOMMU_DEBUG("Invalid IO pagetable entry gfn = %lx\n", gfn);
> domain_crash(d);
> - return -EFAULT;
> + rc = -EFAULT;
> + goto err;
> }
> - }
>
> - if ( iommu_pde_from_gfn(d, gfn, pt_mfn) || (pt_mfn[1] == 0) )
> - {
> - spin_unlock(&hd->arch.mapping_lock);
> - AMD_IOMMU_DEBUG("Invalid IO pagetable entry gfn = %lx\n", gfn);
> - domain_crash(d);
> - return -EFAULT;
> - }
> + /* Install 4k mapping first */
> + need_flush = set_iommu_pte_present(pt_mfn[1], gfn, mfn,
> + IOMMU_PAGING_MODE_LEVEL_1,
> + !!(flags & IOMMUF_writable),
> + !!(flags & IOMMUF_readable));
>
> - /* Install 4k mapping first */
> - need_flush = set_iommu_pte_present(pt_mfn[1], gfn, mfn,
> - IOMMU_PAGING_MODE_LEVEL_1,
> - !!(flags & IOMMUF_writable),
> - !!(flags & IOMMUF_readable));
> + /* Do not increase pde count if io mapping has not been changed */
> + if ( !need_flush )
> + {
> + spin_unlock(&hd->arch.mapping_lock);
> + continue;
> + }
>
> - /* Do not increase pde count if io mapping has not been changed */
> - if ( !need_flush )
> - goto out;
> + /* 4K mapping for PV guests never changes,
> + * no need to flush if we trust non-present bits */
> + if ( is_hvm_domain(d) )
> + amd_iommu_flush_pages(d, gfn, 0);
>
> - /* 4K mapping for PV guests never changes,
> - * no need to flush if we trust non-present bits */
> - if ( is_hvm_domain(d) )
> - amd_iommu_flush_pages(d, gfn, 0);
> -
> - for ( merge_level = IOMMU_PAGING_MODE_LEVEL_2;
> - merge_level <= hd->arch.paging_mode; merge_level++ )
> - {
> - if ( pt_mfn[merge_level] == 0 )
> - break;
> - if ( !iommu_update_pde_count(d, pt_mfn[merge_level],
> - gfn, mfn, merge_level) )
> - break;
> -
> - if ( iommu_merge_pages(d, pt_mfn[merge_level], gfn,
> - flags, merge_level) )
> + for ( merge_level = IOMMU_PAGING_MODE_LEVEL_2;
> + merge_level <= hd->arch.paging_mode; merge_level++ )
> {
> - spin_unlock(&hd->arch.mapping_lock);
> - AMD_IOMMU_DEBUG("Merge iommu page failed at level %d, "
> - "gfn = %lx mfn = %lx\n", merge_level, gfn, mfn);
> - domain_crash(d);
> - return -EFAULT;
> + if ( pt_mfn[merge_level] == 0 )
> + break;
> + if ( !iommu_update_pde_count(d, pt_mfn[merge_level],
> + gfn, mfn, merge_level) )
> + break;
> +
> + if ( iommu_merge_pages(d, pt_mfn[merge_level], gfn,
> + flags, merge_level) )
> + {
> + spin_unlock(&hd->arch.mapping_lock);
> + AMD_IOMMU_DEBUG("Merge iommu page failed at level %d, "
> + "gfn = %lx mfn = %lx\n", merge_level, gfn,
> mfn);
> + domain_crash(d);
> + rc = -EFAULT;
> + goto err;
> + }
> +
> + /* Deallocate lower level page table */
> + free_amd_iommu_pgtable(mfn_to_page(pt_mfn[merge_level - 1]));
> }
>
> - /* Deallocate lower level page table */
> - free_amd_iommu_pgtable(mfn_to_page(pt_mfn[merge_level - 1]));
> + spin_unlock(&hd->arch.mapping_lock);
> }
>
> -out:
> - spin_unlock(&hd->arch.mapping_lock);
> return 0;
> +
> +err:
> + while ( i-- )
> + /* If statement to satisfy __must_check. */
> + if ( amd_iommu_unmap_pages(d, orig_gfn + i, 0) )
> + continue;
> +
> + return rc;
> }
>
> -static int __must_check amd_iommu_unmap_page(struct domain *d,
> - unsigned long gfn)
> +int __must_check amd_iommu_unmap_pages(struct domain *d,
> + unsigned long gfn,
> + unsigned int order)
> {
> - unsigned long pt_mfn[7];
> struct domain_iommu *hd = dom_iommu(d);
> + int rt = 0;
> + unsigned long i;
>
> if ( iommu_use_hap_pt(d) )
> return 0;
>
> - memset(pt_mfn, 0, sizeof(pt_mfn));
> -
> - spin_lock(&hd->arch.mapping_lock);
> -
> if ( !hd->arch.root_table )
> - {
> - spin_unlock(&hd->arch.mapping_lock);
> return 0;
> - }
>
> - /* Since HVM domain is initialized with 2 level IO page table,
> - * we might need a deeper page table for lager gfn now */
> - if ( is_hvm_domain(d) )
> + for ( i = 0; i < (1UL << order); i++, gfn++ )
> {
> - int rc = update_paging_mode(d, gfn);
> + unsigned long pt_mfn[7];
>
> - if ( rc )
> - {
> - spin_unlock(&hd->arch.mapping_lock);
> - AMD_IOMMU_DEBUG("Update page mode failed gfn = %lx\n", gfn);
> - if ( rc != -EADDRNOTAVAIL )
> - domain_crash(d);
> - return rc;
> - }
> - }
> + memset(pt_mfn, 0, sizeof(pt_mfn));
>
> - if ( iommu_pde_from_gfn(d, gfn, pt_mfn) || (pt_mfn[1] == 0) )
> - {
> - spin_unlock(&hd->arch.mapping_lock);
> - AMD_IOMMU_DEBUG("Invalid IO pagetable entry gfn = %lx\n", gfn);
> - domain_crash(d);
> - return -EFAULT;
> - }
> -
> - /* mark PTE as 'page not present' */
> - clear_iommu_pte_present(pt_mfn[1], gfn);
> - spin_unlock(&hd->arch.mapping_lock);
> + spin_lock(&hd->arch.mapping_lock);
>
> - amd_iommu_flush_pages(d, gfn, 0);
> -
> - return 0;
> -}
> -
> -/* TODO: Optimize by squashing map_pages/unmap_pages with
> map_page/unmap_page */
> -int __must_check amd_iommu_map_pages(struct domain *d, unsigned long gfn,
> - unsigned long mfn, unsigned int order,
> - unsigned int flags)
> -{
> - unsigned long i;
> - int rc = 0;
> -
> - for ( i = 0; i < (1UL << order); i++ )
> - {
> - rc = amd_iommu_map_page(d, gfn + i, mfn + i, flags);
> - if ( unlikely(rc) )
> + /* Since HVM domain is initialized with 2 level IO page table,
> + * we might need a deeper page table for lager gfn now */
> + if ( is_hvm_domain(d) )
> {
> - while ( i-- )
> - /* If statement to satisfy __must_check. */
> - if ( amd_iommu_unmap_page(d, gfn + i) )
> - continue;
> + int rc = update_paging_mode(d, gfn);
>
> - break;
> + if ( rc )
> + {
> + spin_unlock(&hd->arch.mapping_lock);
> + AMD_IOMMU_DEBUG("Update page mode failed gfn = %lx\n", gfn);
> + if ( rc != -EADDRNOTAVAIL )
> + domain_crash(d);
> + if ( !rt )
> + rt = rc;
> + continue;
> + }
> }
> - }
> -
> - return rc;
> -}
>
> -int __must_check amd_iommu_unmap_pages(struct domain *d, unsigned long gfn,
> - unsigned int order)
> -{
> - unsigned long i;
> - int rc = 0;
> + if ( iommu_pde_from_gfn(d, gfn, pt_mfn) || (pt_mfn[1] == 0) )
> + {
> + spin_unlock(&hd->arch.mapping_lock);
> + AMD_IOMMU_DEBUG("Invalid IO pagetable entry gfn = %lx\n", gfn);
> + domain_crash(d);
> + if ( !rt )
> + rt = -EFAULT;
> + continue;
> + }
>
> - for ( i = 0; i < (1UL << order); i++ )
> - {
> - int ret = amd_iommu_unmap_page(d, gfn + i);
> + /* mark PTE as 'page not present' */
> + clear_iommu_pte_present(pt_mfn[1], gfn);
> + spin_unlock(&hd->arch.mapping_lock);
>
> - if ( !rc )
> - rc = ret;
> + amd_iommu_flush_pages(d, gfn, 0);
> }
>
> - return rc;
> + return rt;
> }
>
> int amd_iommu_reserve_domain_unity_map(struct domain *domain,
> @@ -831,7 +823,7 @@ int amd_iommu_reserve_domain_unity_map(struct domain
> *domain,
> gfn = phys_addr >> PAGE_SHIFT;
> for ( i = 0; i < npages; i++ )
> {
> - rt = amd_iommu_map_page(domain, gfn +i, gfn +i, flags);
> + rt = amd_iommu_map_pages(domain, gfn +i, gfn +i, flags, 0);
> if ( rt != 0 )
> return rt;
> }
> --
> 2.7.4
>
--
Regards,
Oleksandr Tyshchenko
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |