From mboxrd@z Thu Jan 1 00:00:00 1970 From: Suravee Suthikulanit Subject: Re: [PATCH v4 3/4] amd-iommu: disable iommu_hap_pt_share with AMD IOMMUs Date: Fri, 8 Aug 2014 10:36:51 -0500 Message-ID: <53E4EE93.3020607@amd.com> References: <1401876381-42977-1-git-send-email-roger.pau@citrix.com> <1401876381-42977-4-git-send-email-roger.pau@citrix.com> <53CE9FAC.8070008@amd.com> <53CEA1ED.1050604@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------010001040602080008030902" Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XFmEY-0006VT-BW for xen-devel@lists.xenproject.org; Fri, 08 Aug 2014 15:37:34 +0000 In-Reply-To: <53CEA1ED.1050604@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: =?UTF-8?B?Um9nZXIgUGF1IE1vbm7DqQ==?= , "xen-devel@lists.xenproject.org" Cc: Jan Beulich , Xiantao Zhang List-Id: xen-devel@lists.xenproject.org --------------010001040602080008030902 Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: quoted-printable On 7/22/2014 12:39 PM, Roger Pau Monn=C3=A9 wrote: > On 22/07/14 19:30, Suravee Suthikulpanit wrote: >> Roger, >> >> I am not quite sure why you would disable "iommu_hap_pt_share" for AMD >> IOMMU. The current implementation assumes that the p2m can be shared. >> >> Also, I feel that simply just set iommu_hap_pt_share =3D 0 (while stil= l >> having several places in the AMD iommu drivers and p2m-pt.c assuming >> that it can be shared) seems a bit messy. > > According to the comment in p2m.h, AMD IOMMU only supports bit 52 to bi= t > 58 in the pte to be 0, otherwise the hw generates page faults. > > If we want to support doing IO to devices behind an IOMMU from page > types different than p2m_ram_rw the p2m tables cannot be shared, becaus= e > the bits from 52 to 58 will indeed be different than 0, and will > generate page faults. > > Roger. > As you have mentioned, they cannot be shared due to the 52 and 58 bits.=20 However, what I was trying to say is that, besides just simply set the=20 flag to 0, we probably should remove existing logic in various places=20 that assumes that AMD IOMMU can have share_p2m_table=3D1. If you are agree, the attachment is the patch that should do that. I have tested device-passthrough w/ the amd-iommu: disable=20 iommu_hap_pt_share with AMD IOMMUs, and my patch and it is working. Acked-by Suravee Suthikulpanit Thanks, Suravee --------------010001040602080008030902 Content-Type: text/x-patch; name="iommu-removal-of-share_p2m_table-from-AMD-IOMMU.patch" Content-Disposition: attachment; filename="iommu-removal-of-share_p2m_table-from-AMD-IOMMU.patch" Content-Transfer-Encoding: quoted-printable >>From f28838679867fbbc3be6286556eed7f908eea559 Mon Sep 17 00:00:00 2001 From: Suravee Suthikulpanit Date: Fri, 8 Aug 2014 07:26:15 -0500 Subject: [PATCH] iommu: Removal of share_p2m_table from AMD IOMMU This patch removes existing logics which assumes iommu_hap_pt_share is enabled for AMD IOMMU. Signed-off-by Suravee Suthikulpanit Cd: Roger Pau Monn=C3=A9 Cc: Xiantao Zhang Cc: Jan Beulich --- NOTES: This patch depends on the "amd-iommu: disable iommu_hap_pt_share w= ith AMD IOMMUs" patch from Roger Pau Monne . xen/arch/x86/mm/p2m-pt.c | 23 ++++++-------------- xen/drivers/passthrough/amd/iommu_map.c | 33 -----------------------= ------ xen/drivers/passthrough/amd/pci_amd_iommu.c | 4 ---- xen/drivers/passthrough/iommu.c | 2 +- 4 files changed, 8 insertions(+), 54 deletions(-) diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c index 085ab6f..6bec0e9 100644 --- a/xen/arch/x86/mm/p2m-pt.c +++ b/xen/arch/x86/mm/p2m-pt.c @@ -36,7 +36,6 @@ #include #include #include -#include =20 #include "mm-locks.h" =20 @@ -653,22 +652,14 @@ p2m_pt_set_entry(struct p2m_domain *p2m, unsigned l= ong gfn, mfn_t mfn, =20 if ( iommu_enabled && need_iommu(p2m->domain) ) { - if ( iommu_hap_pt_share ) - { - if ( old_mfn && (old_mfn !=3D mfn_x(mfn)) ) - amd_iommu_flush_pages(p2m->domain, gfn, page_order); - } + unsigned int flags =3D p2m_get_iommu_flags(p2mt); + + if ( flags !=3D 0 ) + for ( i =3D 0; i < (1UL << page_order); i++ ) + iommu_map_page(p2m->domain, gfn+i, mfn_x(mfn)+i, flags); else - { - unsigned int flags =3D p2m_get_iommu_flags(p2mt); - - if ( flags !=3D 0 ) - for ( i =3D 0; i < (1UL << page_order); i++ ) - iommu_map_page(p2m->domain, gfn+i, mfn_x(mfn)+i, fla= gs); - else - for ( int i =3D 0; i < (1UL << page_order); i++ ) - iommu_unmap_page(p2m->domain, gfn+i); - } + for ( int i =3D 0; i < (1UL << page_order); i++ ) + iommu_unmap_page(p2m->domain, gfn+i); } =20 out: diff --git a/xen/drivers/passthrough/amd/iommu_map.c b/xen/drivers/passth= rough/amd/iommu_map.c index a8c60ec..2808c31 100644 --- a/xen/drivers/passthrough/amd/iommu_map.c +++ b/xen/drivers/passthrough/amd/iommu_map.c @@ -640,9 +640,6 @@ int amd_iommu_map_page(struct domain *d, unsigned lon= g gfn, unsigned long mfn, =20 BUG_ON( !hd->arch.root_table ); =20 - if ( iommu_use_hap_pt(d) ) - return 0; - memset(pt_mfn, 0, sizeof(pt_mfn)); =20 spin_lock(&hd->arch.mapping_lock); @@ -718,9 +715,6 @@ int amd_iommu_unmap_page(struct domain *d, unsigned l= ong gfn) =20 BUG_ON( !hd->arch.root_table ); =20 - if ( iommu_use_hap_pt(d) ) - return 0; - memset(pt_mfn, 0, sizeof(pt_mfn)); =20 spin_lock(&hd->arch.mapping_lock); @@ -777,30 +771,3 @@ int amd_iommu_reserve_domain_unity_map(struct domain= *domain, } return 0; } - -/* Share p2m table with iommu. */ -void amd_iommu_share_p2m(struct domain *d) -{ - struct hvm_iommu *hd =3D domain_hvm_iommu(d); - struct page_info *p2m_table; - mfn_t pgd_mfn; - - ASSERT( is_hvm_domain(d) && d->arch.hvm_domain.hap_enabled ); - - if ( !iommu_use_hap_pt(d) ) - return; - - pgd_mfn =3D pagetable_get_mfn(p2m_get_pagetable(p2m_get_hostp2m(d)))= ; - p2m_table =3D mfn_to_page(mfn_x(pgd_mfn)); - - if ( hd->arch.root_table !=3D p2m_table ) - { - free_amd_iommu_pgtable(hd->arch.root_table); - hd->arch.root_table =3D p2m_table; - - /* When sharing p2m with iommu, paging mode =3D 4 */ - hd->arch.paging_mode =3D IOMMU_PAGING_MODE_LEVEL_4; - AMD_IOMMU_DEBUG("Share p2m table with iommu: p2m table =3D %#lx\= n", - mfn_x(pgd_mfn)); - } -} diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/pa= ssthrough/amd/pci_amd_iommu.c index 0b301b3..c893dea 100644 --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c @@ -453,9 +453,6 @@ static void deallocate_iommu_page_tables(struct domai= n *d) { struct hvm_iommu *hd =3D domain_hvm_iommu(d); =20 - if ( iommu_use_hap_pt(d) ) - return; - spin_lock(&hd->arch.mapping_lock); if ( hd->arch.root_table ) { @@ -619,7 +616,6 @@ const struct iommu_ops amd_iommu_ops =3D { .setup_hpet_msi =3D amd_setup_hpet_msi, .suspend =3D amd_iommu_suspend, .resume =3D amd_iommu_resume, - .share_p2m =3D amd_iommu_share_p2m, .crash_shutdown =3D amd_iommu_suspend, .dump_p2m_table =3D amd_dump_p2m_table, }; diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/io= mmu.c index cc12735..5d3299a 100644 --- a/xen/drivers/passthrough/iommu.c +++ b/xen/drivers/passthrough/iommu.c @@ -332,7 +332,7 @@ void iommu_share_p2m_table(struct domain* d) { const struct iommu_ops *ops =3D iommu_get_ops(); =20 - if ( iommu_enabled && is_hvm_domain(d) ) + if ( iommu_enabled && is_hvm_domain(d) && ops->share_p2m) ops->share_p2m(d); } =20 --=20 1.9.0 --------------010001040602080008030902 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --------------010001040602080008030902--