Re: [Xen-devel] [v7][RFC][PATCH 08/13] xen/x86/p2m: set p2m_access_n for reserved device memory mapping

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

On 2014/10/29 17:20, Jan Beulich wrote:
> > > > On 29.10.14 at 09:20, <tiejun.chen@xxxxxxxxx> wrote:
> > @@ -686,8 +686,19 @@ guest_physmap_add_entry(struct domain *d, unsigned long 
> > gfn,
> >        /* Now, actually do the two-way mapping */
> >        if ( mfn_valid(_mfn(mfn)) )
> >        {
> > -        rc = p2m_set_entry(p2m, gfn, _mfn(mfn), page_order, t,
> > -                           p2m->default_access);
> > +        rc = 0;
> > +        if ( !is_hardware_domain(d) )
> > +        {
> > +            rc = 
> > iommu_get_reserved_device_memory(p2m_check_reserved_device_memory,
> > +                                                  &gfn);
> > +            if ( rc < 0 )
> > +                printk("Domain %d can't can't check reserved device memory.\n",
> > +                       d->domain_id);
> > +        }
> > +
> > +        /* We need to set reserved device memory as p2m_access_n. */
> > +        a =  ( rc == 1 ) ? p2m_access_n : p2m->default_access;
> > +        rc = p2m_set_entry(p2m, gfn, _mfn(mfn), page_order, t, a);
> >            if ( rc )
> >                goto out; /* Failed to update p2m, bail without updating m2p. */
> Getting closer. Just set a to p2m->default_access before the if(),
> and overwrite it when rc == 1 inside the if(). And properly handle
> the error case (just logging a message - which btw lacks a proper
> XENLOG_G_* prefix - doesn't seem enough to me).
Please check the follows:

@@ -686,8 +686,22 @@ guest_physmap_add_entry(struct domain *d, unsigned 
long gfn,
     /* Now, actually do the two-way mapping */
     if ( mfn_valid(_mfn(mfn)) )
     {
-        rc = p2m_set_entry(p2m, gfn, _mfn(mfn), page_order, t,
-                           p2m->default_access);
+        rc = 0;
+        a =  p2m->default_access;
+        if ( !is_hardware_domain(d) )
+        {
+            rc = 
iommu_get_reserved_device_memory(p2m_check_reserved_device_memory,
+                                                  &gfn);
+            /* We need to set reserved device memory as p2m_access_n. */
+            if ( rc == 1 )
+                a = p2m_access_n;
+            else if ( rc < 0 )
+                printk(XENLOG_WARNING
+                       "Domain %d can't check reserved device memory.\n",
+                       d->domain_id);
+        }
+
+        rc = p2m_set_entry(p2m, gfn, _mfn(mfn), page_order, t, a);
         if ( rc )
             goto out; /* Failed to update p2m, bail without updating 
m2p. */

> But then again this code may change altogether if you avoid
> populating the reserved regions in the first place.
Are you saying this scenario?

#1 Here we first set these ranges as p2m_access_n
#2 We reset them as 1:1 RMRR mapping with p2m_access_rw somewhere
#3 Someone may try to populate these ranges again

Thanks
Tiejun

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