Re: [V7 PATCH 5/7] pvh: change xsm_add_to_physmap

[Mukesh Rathor <mukesh.rathor@oracle.com>]

On Thu, 20 Feb 2014 17:22:34 -0800
Mukesh Rathor <mukesh.rathor@oracle.com> wrote:

> On Thu, 20 Feb 2014 13:49:58 +0000
> Julien Grall <julien.grall@linaro.org> wrote:
> 
> > On 02/20/2014 02:22 AM, Mukesh Rathor wrote:
> > > On Wed, 12 Feb 2014 16:47:54 +0000
> > > Julien Grall <julien.grall@linaro.org> wrote:
> > > 
> > >> Hi Mukesh,
> > >>
> > >> On 12/17/2013 02:38 AM, Mukesh Rathor wrote:
> > >>> In preparation for the next patch, we update xsm_add_to_physmap
> > >>> to allow for checking of foreign domain. Thus, the current
> > >>> domain must have the right to update the mappings of target
> > >>> domain with pages from foreign domain.
> > >>>
> > >>> Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
> > >>
> > >> While I was playing with XSM on ARM, I have noticed that Daniel
> > >> De Graff has added xsm_map_gfmn_foreign few months ago (see
> > >> commit 0b201e6).
> > >>
> > >> Would it be suitable to use this XSM instead of extending
> > >> xsm_add_to_physmap?
> > >>
> > >> Regards,
> > >>
> > > 
> > > Not the same thing. add to physmap could be adding to a domain's
> > > physmap pages from a foreign domain.
> > 
> > Let assume you don't modify xsm_add_to_physmap, in this case:
> >    - xsm_add_to_physmap checks if the current domain is allowed to
> > modify the p2m of a given domain
> >    - xsm_map_gfmn_foreign checks if the given domain is allowed to
> > have foreign mapping from the foreign domain
> > 
> > Both XSM are distinct and should be used together. You don't care
> > that
> 
> I see, i thought you meant replace one with another. I am not a
> security expert, so just followed the suggestions. But looking at the
> code looks like above is the way to go, and I can just drop my
> xsm_add_to_physmap change patch (which btw doesn't check whether
> target has access to foreign mappings, so is prob not correct).
> Thanks for noticing.


BTW, in include/xsm/xsm.h, shouldn't 

static inline int xsm_map_gmfn_foreign (struct domain *d, struct domain *t)

be

static inline int xsm_map_gmfn_foreign (xsm_default_t def, struct domain *d, struct domain *t)

not sure how you were able to compile xsm enabled in arm???

thanks
Mukesh