| Message ID | 1501003615-15274-14-git-send-email-olekstysh@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, all. Any comments? On Tue, Jul 25, 2017 at 8:26 PM, Oleksandr Tyshchenko <olekstysh@gmail.com> wrote: > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > > 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@epam.com> > CC: Jan Beulich <jbeulich@suse.com> > CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > > --- > Changes in v1: > - > > Changes in v2: > - > > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > --- > 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 >
Hi. Gentle reminder. On Mon, Aug 21, 2017 at 7:44 PM, Oleksandr Tyshchenko <olekstysh@gmail.com> wrote: > Hi, all. > > Any comments? > > On Tue, Jul 25, 2017 at 8:26 PM, Oleksandr Tyshchenko > <olekstysh@gmail.com> wrote: >> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> >> >> 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@epam.com> >> CC: Jan Beulich <jbeulich@suse.com> >> CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> >> >> --- >> Changes in v1: >> - >> >> Changes in v2: >> - >> >> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> >> --- >> 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
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; }