Re: [Xen-devel] [v5][PATCH 1/2] xen:x86:mm:p2m: introduce set_identity_p2m_entry

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

On 2014/7/29 18:27, Jan Beulich wrote:
> > > > On 29.07.14 at 12:18, <tiejun.chen@xxxxxxxxx> wrote:
> > On 2014/7/29 17:53, Jan Beulich wrote:
> > > > > > On 29.07.14 at 11:11, <tiejun.chen@xxxxxxxxx> wrote:
> > > > On 2014/7/29 16:19, Jan Beulich wrote:
> > > > > ? Of course it may still be necessary to also inspect the obtained p2mt
> > > > > and a.
> > > > Are you saying this?
> > > >
> > > >         if ( !p2m_is_valid(p2mt) ||
> > > >              !mfn_valid(mfn) ||
> > > >              (a != p2m_access_rw) )
> > > I'm afraid that's not enough context to know whether what you
> > > mean to do is sufficient. Plus !p2m_is_valid() is too weak. You
> > > simply need to properly think through what should happen if you
> > > find a valid mapping, but any of the tuple (mfn, p2mt, a) don't
> > > match what you intend to be there.
> > Actually as I understand we can create these mapping only in one case of
> > !mfn_valid(mfn). For others scenarios we just return with that  warning
> > message no matter what that tuple is explicitly. So here I try to
> > understand why you're saying we need check more by show this condition
> > combination.
> Perhaps, but with the exception that at least if the entire tuple
> matches you should return success (and not print anything).
> There might be further cases where an existing mapping would
> be good enough (like a being p2m_access_rwx), but perhaps
> there's not much point in trying to deal with them without explicit
> need.
I think the following cases should be enough:

#1: !mfn_valid(mfn)

We can create those mapping safely.

#2: mfn_x(mfn) == gfn && p2mt == p2m_mmio_direct && a == p2m_access_rw

We already have these matched mappings.

#3: Others

Return with that waring message: "Cannot identity map d%d:%lx, already 
mapped to %lx but mismatch.\n"
So what about this?

@@ -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 but 
mismatch.\n",
+               d->domain_id, gfn, mfn_x(mfn));
+
+    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)
 {


Thanks
Tiejun

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