Re: [v2][PATCH 1/1] xen:vtd: missing RMRR mapping while share EPT

["Chen, Tiejun" <tiejun.chen@intel.com>]

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 <tiejun.chen@intel.com>
>>>> ---
>>>>    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"
>>>
>>> <asm/mm/mm-locks.h>
>>
>> iommu.c:38:29: fatal error: asm/mm/mm-locks.h: No such file or directory
>>   #include <asm/mm/mm-locks.h>
>>                               ^
>> 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 <tiejun.chen@intel.com>
---
  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