From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Chen, Tiejun" Subject: Re: [v2][PATCH 1/1] xen:vtd: missing RMRR mapping while share EPT Date: Thu, 24 Jul 2014 17:39:24 +0800 Message-ID: <53D0D44C.2090606@intel.com> References: <1406191841-5258-1-git-send-email-tiejun.chen@intel.com> <53D0CA69.6090500@citrix.com> <53D0CBDF.4010606@intel.com> <53D0CDA9.7020306@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <53D0CDA9.7020306@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: Andrew Cooper , JBeulich@suse.com, yang.z.zhang@intel.com, kevin.tian@intel.com Cc: xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 2014/7/24 17:11, Andrew Cooper wrote: > On 24/07/14 10:03, Chen, Tiejun wrote: >> On 2014/7/24 16:57, Andrew Cooper wrote: >>> On 24/07/14 09:50, Tiejun Chen wrote: >>>> intel_iommu_map_page() does nothing if VT-d shares EPT page table. >>>> So rmrr_identity_mapping() never create RMRR mapping but in some >>>> cases like some GFX drivers it still need to access RMRR. >>>> >>>> Here we will create those RMRR mappings even in shared EPT case. >>>> >>>> Signed-off-by: Tiejun Chen >>>> --- >>>> xen/drivers/passthrough/vtd/iommu.c | 18 ++++++++++++++++-- >>>> 1 file changed, 16 insertions(+), 2 deletions(-) >>>> >>>> v2: >>>> >>>> * Fix coding style. >>>> * Still need to abide intel_iommu_map_page(), so we should do nothing >>>> if dom0 and iommu supports pass thru. >>>> >>>> diff --git a/xen/drivers/passthrough/vtd/iommu.c >>>> b/xen/drivers/passthrough/vtd/iommu.c >>>> index 042b882..aca79db 100644 >>>> --- a/xen/drivers/passthrough/vtd/iommu.c >>>> +++ b/xen/drivers/passthrough/vtd/iommu.c >>>> @@ -41,6 +41,7 @@ >>>> #include "extern.h" >>>> #include "vtd.h" >>>> #include "../ats.h" >>>> +#include "../../../arch/x86/mm/mm-locks.h" >>> >>> >> >> iommu.c:38:29: fatal error: asm/mm/mm-locks.h: No such file or directory >> #include >> ^ >> compilation terminated. >> >> Tiejun > > Hmm - my mistake. The lack of easy access to this header file goes to > emphasise that it is private to arch/x86/mm, and there are indeed no > current users outside of that part of the tree. > > Would it not be better have some public p2m_set_identity() function in > p2m.c ? > Sounds good so what about this? Subject: [PATCH 1/2] xen:p2m: introduce a public p2m_set_identity Often p2m_set_entry() always needs to hold p2m_lock but p2m_lock() is not easy included for some common occasions. So here introduce an new public p2m_set_identity holding this lock. Signed-off-by: Tiejun Chen --- xen/arch/x86/mm/p2m.c | 13 +++++++++++++ xen/include/asm-x86/p2m.h | 3 +++ 2 files changed, 16 insertions(+) diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c index 642ec28..d0cc586 100644 --- a/xen/arch/x86/mm/p2m.c +++ b/xen/arch/x86/mm/p2m.c @@ -385,6 +385,19 @@ int p2m_set_entry(struct p2m_domain *p2m, unsigned long gfn, mfn_t mfn, return rc; } +/* Just hold p2m_lock then public. */ +int p2m_set_identity(struct p2m_domain *p2m, unsigned long gfn, mfn_t mfn, + unsigned int page_order, p2m_type_t p2mt, p2m_access_t p2ma) +{ + int rc = 0; + + p2m_lock(p2m); + rc = p2m_set_entry(p2m, gfn, mfn, page_order, p2mt, p2ma); + p2m_unlock(p2m); + + return rc; +} + struct page_info *p2m_alloc_ptp(struct p2m_domain *p2m, unsigned long type) { struct page_info *pg; diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h index 0ddbadb..169bcd7 100644 --- a/xen/include/asm-x86/p2m.h +++ b/xen/include/asm-x86/p2m.h @@ -625,6 +625,9 @@ void p2m_free_ptp(struct p2m_domain *p2m, struct page_info *pg); int p2m_set_entry(struct p2m_domain *p2m, unsigned long gfn, mfn_t mfn, unsigned int page_order, p2m_type_t p2mt, p2m_access_t p2ma); +int p2m_set_identity(struct p2m_domain *p2m, unsigned long gfn, mfn_t mfn, + unsigned int page_order, p2m_type_t p2mt, p2m_access_t p2ma); + /* Set up function pointers for PT implementation: only for use by p2m code */ extern void p2m_pt_init(struct p2m_domain *p2m); -- 1.9.1 Thanks Tiejun