[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 03/18] PVH xen: create domctl_memory_mapping() function
On Fri, 31 May 2013 10:46:45 +0100 "Jan Beulich" <JBeulich@xxxxxxxx> wrote: > >>> On 25.05.13 at 03:25, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> > >>> wrote: > > Changes in V5: > > - Move iomem_access_permitted check to case statment from the > > function as current doesn't point to dom0 during construct_dom0. > > > > Changes in V6: > > - Move iomem_access_permitted back to domctl_memory_mapping() as > > it should be after the sanity and wrap checks of mfns/gfns. > > So you're undoing what you previously did? How can it work then? > Or is the whole patch perhaps no longer needed with Dom0 code > dropped for the time being? > > Otherwise, rather than undoing what you did in v5, I'd think you'd > want to keep the sanity/wrap checks in the caller as well, since if > you need the function to be split out just for Dom0 building, you > can assume the inputs to be sane. And if so, the XSM check could > remain in the caller too. That would also make sure there really is > no change in functionality: Well, thats where I had it originally, but one of the reviewers early on suggested moving it to the function, in case there's another caller in future. It's only for dom0, so I can drop the check ' !is_idle_vcpu(current) ' and add it during dom0 construction patch, or drop the whole patch and add it to construct dom0 patch series, now phase 1.5. Please lmk. > > +long domctl_memory_mapping(struct domain *d, unsigned long gfn, > > + unsigned long mfn, unsigned long > > nr_mfns, > > + bool_t add_map) > > +{ > > + unsigned long i; > > + long ret; > > + > > + if ( (mfn + nr_mfns - 1) < mfn || /* wrap? */ > > + ((mfn | (mfn + nr_mfns - 1)) >> (paddr_bits - > > PAGE_SHIFT)) || > > + (gfn + nr_mfns - 1) < gfn ) /* wrap? */ > > + return -EINVAL; > > + > > + /* caller construct_dom0() runs on idle vcpu */ > > + if ( !is_idle_vcpu(current) && > > This check is new, so this is not pure code motion. > > > + !iomem_access_permitted(current->domain, mfn, mfn + > > nr_mfns - 1) ) > > + return -EPERM; > > + > > + ret = xsm_iomem_permission(XSM_HOOK, d, mfn, mfn + nr_mfns - > > 1, add_map); > > And this was xsm_iomem_mapping() in the original code. What's going > on here? Bad merge! That's why I like to do things in small steps and keep patchsets small. Orig code had xsm_iomem_permission() at some point. thanks Mukesh _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |