Re: [Xen-devel] [PATCH 1/1] xen:vtd: missing RMRR mapping while share EPT

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

On 2014/7/23 23:42, Jan Beulich wrote:
> > > > On 23.07.14 at 11:35, <tiejun.chen@xxxxxxxxx> 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.
> Technically this is fine, but there are mechanical issues:
>
> > @@ -1867,7 +1869,19 @@ static int rmrr_identity_mapping(struct domain *d,
> >
> >       while ( base_pfn < end_pfn )
> >       {
> > -        if ( intel_iommu_map_page(d, base_pfn, base_pfn,
> > +        if ( iommu_use_hap_pt(d) ) {
> > +            dprintk(XENLOG_DEBUG VTDPREFIX,
> > +                    "Set RMRR mapping: pfn:0x%lx mfn:0x%lx.\n",
> > +                    base_pfn, mfn_x(_mfn(base_pfn)));
> Do we really need this message, even more so not at guest level?
Its useful to debug as I think, but if you insist on this point, I'm 
fine to remove this as well.
> And if you're convinced we need it, please use %#lx instead of
> 0x%lx.
Okay.

> > +            p2m_lock(p2m);
> > +            if ( p2m_set_entry(p2m, base_pfn, _mfn(base_pfn), PAGE_ORDER_4K,
> > +                    p2m_mmio_direct, p2m_access_rw) ) {
> > +                p2m_unlock(p2m);
> > +                return -1;
> > +            }
> > +            p2m_unlock(p2m);
> > +        }
> > +        else if ( intel_iommu_map_page(d, base_pfn, base_pfn,
> >                                     IOMMUF_readable|IOMMUF_writable) )
> >               return -1;
> >           base_pfn++;
> Apart from the above there are several coding style issues here.
Are you saying this thing?

if ()
{
}

So what about this?


@@ -1867,7 +1869,21 @@ static int rmrr_identity_mapping(struct domain *d,

     while ( base_pfn < end_pfn )
     {
-        if ( intel_iommu_map_page(d, base_pfn, base_pfn,
+        if ( iommu_use_hap_pt(d) )
+        {
+            dprintk(XENLOG_DEBUG VTDPREFIX,
+                    "Set RMRR mapping: pfn:%#lx mfn:%#lx.\n",
+                    base_pfn, mfn_x(_mfn(base_pfn)));
+            p2m_lock(p2m);
+            if ( p2m_set_entry(p2m, base_pfn, _mfn(base_pfn), 
PAGE_ORDER_4K,
+                    p2m_mmio_direct, p2m_access_rw) )
+            {
+                p2m_unlock(p2m);
+                return -1;
+            }
+            p2m_unlock(p2m);
+        }
+        else if ( intel_iommu_map_page(d, base_pfn, base_pfn,
                                   IOMMUF_readable|IOMMUF_writable) )
             return -1;
         base_pfn++;

Thanks
Tiejun

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel