From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Chen, Tiejun" Subject: Re: [v3][PATCH 03/16] xen/vtd: create RMRR mapping Date: Thu, 11 Jun 2015 17:31:56 +0800 Message-ID: <5579558C.30709@intel.com> References: <1433985325-16676-1-git-send-email-tiejun.chen@intel.com> <1433985325-16676-4-git-send-email-tiejun.chen@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: "Tian, Kevin" , "jbeulich@suse.com" , "tim@xen.org" , "andrew.cooper3@citrix.com" , "Zhang, Yang Z" , "wei.liu2@citrix.com" , "ian.campbell@citrix.com" , "Ian.Jackson@eu.citrix.com" , "stefano.stabellini@citrix.com" Cc: "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org On 2015/6/11 17:14, Tian, Kevin wrote: >> From: Chen, Tiejun >> Sent: Thursday, June 11, 2015 9:15 AM >> >> RMRR reserved regions must be setup in the pfn space with an identity >> mapping to reported mfn. However existing code has problem to setup >> correct mapping when VT-d shares EPT page table, so lead to problem >> when assigning devices (e.g GPU) with RMRR reported. So instead, this >> patch aims to setup identity mapping in p2m layer, regardless of >> whether EPT is shared or not. And we still keep creating VT-d table. >> >> Signed-off-by: Tiejun Chen >> --- >> xen/arch/x86/mm/p2m.c | 10 ++++++++-- >> xen/drivers/passthrough/vtd/iommu.c | 3 +-- >> 2 files changed, 9 insertions(+), 4 deletions(-) >> >> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c >> index a6db236..c7198a5 100644 >> --- a/xen/arch/x86/mm/p2m.c >> +++ b/xen/arch/x86/mm/p2m.c >> @@ -927,10 +927,16 @@ int set_identity_p2m_entry(struct domain *d, unsigned long >> gfn, >> } >> >> gfn_unlock(p2m, gfn, 0); >> - return ret; >> } >> + else >> + ret = 0; >> >> - return 0; >> + if( ret == 0 ) >> + { >> + ret = iommu_map_page(d, gfn, gfn, IOMMUF_readable|IOMMUF_writable); >> + } >> + >> + return ret; > > p2m_set_entry will setup IOMMU pages already. You don't need > another explicit iommu map here. Right. > >> } >> >> /* Returns: 0 for success, -errno for failure */ >> diff --git a/xen/drivers/passthrough/vtd/iommu.c >> b/xen/drivers/passthrough/vtd/iommu.c >> index 6a37624..31ce1af 100644 >> --- a/xen/drivers/passthrough/vtd/iommu.c >> +++ b/xen/drivers/passthrough/vtd/iommu.c >> @@ -1856,8 +1856,7 @@ static int rmrr_identity_mapping(struct domain *d, bool_t map, >> >> while ( base_pfn < end_pfn ) >> { >> - int err = intel_iommu_map_page(d, base_pfn, base_pfn, >> - IOMMUF_readable|IOMMUF_writable); >> + int err = set_identity_p2m_entry(d, base_pfn, p2m_access_rw); >> >> if ( err ) >> return err; > > Tim has another comment to replace earlier unmap with Yes, I knew this. > guest_physmap_remove_page() which will call iommu > unmap internally. Please include this change too. > But, guest_physmap_remove_page() | + p2m_remove_page() | + iommu_unmap_page() | + p2m_set_entry(p2m, gfn, _mfn(INVALID_MFN), xxx) I think this already remove these pages both on ept/vt-d sides, right? Or I'm misunderstand what you guys mean? Thanks Tiejun