Re: [v6][PATCH 1/2] xen:x86:mm:p2m: introduce set_identity_p2m_entry

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

On 2014/8/1 6:29, Tian, Kevin wrote:
>> From: Chen, Tiejun
>> Sent: Tuesday, July 29, 2014 6:36 PM
>>
>> Its used conveniently to create RMRR mapping in shared EPT case.
>>
>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>> ---
>>   xen/arch/x86/mm/p2m.c     | 29 +++++++++++++++++++++++++++++
>>   xen/include/asm-x86/p2m.h |  3 +++
>>   2 files changed, 32 insertions(+)
>>
>> v6:
>>
>> * Refactor set_identity_p2m_entry to make sense
>>
>> v5:
>>
>> * Rename this function as set_identity_p2m_entry()
>> * Get mfn directly inside set_identity_p2m_entry()
>>
>> v4:
>>
>> * new patch to combine get and set together to create RMRR mapping.
>>
>> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
>> index 642ec28..06bed7a 100644
>> --- a/xen/arch/x86/mm/p2m.c
>> +++ b/xen/arch/x86/mm/p2m.c
>> @@ -858,6 +858,35 @@ int set_mmio_p2m_entry(struct domain *d, unsigned
>> long gfn, mfn_t mfn)
>>       return set_typed_p2m_entry(d, gfn, mfn, p2m_mmio_direct);
>>   }
>>
>> +int set_identity_p2m_entry(struct domain *d, unsigned long gfn)
>> +{
>> +    p2m_type_t p2mt;
>> +    p2m_access_t a;
>> +    mfn_t mfn;
>> +    struct p2m_domain *p2m = p2m_get_hostp2m(d);
>> +    int ret = -EBUSY;
>> +
>> +    gfn_lock(p2m, gfn, 0);
>> +
>> +    mfn = p2m->get_entry(p2m, gfn, &p2mt, &a, 0, NULL);
>> +
>> +    if ( !mfn_valid(mfn) )
>> +        ret = p2m_set_entry(p2m, gfn, _mfn(gfn), PAGE_ORDER_4K,
>> p2m_mmio_direct,
>> +                            p2m_access_rw);
>> +    else if ( mfn_x(mfn) == gfn &&
>> +              p2mt == p2m_mmio_direct &&
>> +              a == p2m_access_rw )
>> +        ret = 0;
>> +    else
>> +        printk(XENLOG_G_WARNING
>> +               "Cannot identity map d%d:%lx, already mapped to %lx.\n",
>> +               d->domain_id, gfn, mfn_x(mfn));
>
> what about !mfn_valid but the GFN has been used for emulated MMIOs? w/o
> a guest e820 view you can't avoid overlapping by just looking at mfn...

What about the follows based on this patch?

@@ -865,14 +865,26 @@ int set_identity_p2m_entry(struct domain *d, 
unsigned long gfn)
      mfn_t mfn;
      struct p2m_domain *p2m = p2m_get_hostp2m(d);
      int ret = -EBUSY;
+    u64 base_addr, end_addr;

      gfn_lock(p2m, gfn, 0);

      mfn = p2m->get_entry(p2m, gfn, &p2mt, &a, 0, NULL);

      if ( !mfn_valid(mfn) )
+    {
+        base_addr = gfn << PAGE_ORDER_4K;
+        end_addr = base_addr + PAGE_MASK;
+        if ( (!page_is_ram_type(paddr_to_pfn(base_addr), 
RAM_TYPE_RESERVED)) ||
+             (!page_is_ram_type(paddr_to_pfn(end_add), 
RAM_TYPE_RESERVED)) )
+        {
+            gfn_unlock(p2m, gfn, 0);
+            return ret;
+        }
+
          ret = p2m_set_entry(p2m, gfn, _mfn(gfn), PAGE_ORDER_4K, 
p2m_mmio_direct,
                              p2m_access_rw);
+    }
      else if ( mfn_x(mfn) == gfn &&
                p2mt == p2m_mmio_direct &&
                a == p2m_access_rw )

Note this chunk of codes just show what I want to do, without test.

Thanks
Tiejun

>
> Thanks
> Kevin
>
>> +
>> +    gfn_unlock(p2m, gfn, 0);
>> +
>> +    return ret;
>> +}
>> +
>>   /* Returns: 0 for success, -errno for failure */
>>   int clear_mmio_p2m_entry(struct domain *d, unsigned long gfn)
>>   {
>> diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
>> index 0ddbadb..d130f9a 100644
>> --- a/xen/include/asm-x86/p2m.h
>> +++ b/xen/include/asm-x86/p2m.h
>> @@ -536,6 +536,9 @@ int p2m_is_logdirty_range(struct p2m_domain *,
>> unsigned long start,
>>   int set_mmio_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn);
>>   int clear_mmio_p2m_entry(struct domain *d, unsigned long gfn);
>>
>> +/* Set identity addresses in the p2m table (for pass-through) */
>> +int set_identity_p2m_entry(struct domain *d, unsigned long gfn);
>> +
>>   /* Add foreign mapping to the guest's p2m table. */
>>   int p2m_add_foreign(struct domain *tdom, unsigned long fgfn,
>>                       unsigned long gpfn, domid_t foreign_domid);
>> --
>> 1.9.1
>
>