Re: [Xen-devel] [PATCH v4 01/12] x86/mm: allow a privileged PV domain to map guest mfns

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@xxxxxxxxxx]
> Sent: 07 September 2017 12:02
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Andrew Cooper
> <Andrew.Cooper3@xxxxxxxxxx>; Jan Beulich <jbeulich@xxxxxxxx>; Wei Liu
> <wei.liu2@xxxxxxxxxx>
> Subject: Re: [Xen-devel] [PATCH v4 01/12] x86/mm: allow a privileged PV
> domain to map guest mfns
> 
> On Tue, Sep 05, 2017 at 12:37:05PM +0100, Paul Durrant wrote:
> > In the case where a PV domain is mapping guest resources then it needs
> make
> > the HYPERVISOR_mmu_update call using DOMID_SELF, rather than the
> guest
> > domid, so that the passed in gmfn values are correctly treated as mfns
> > rather than gfns present in the guest p2m.
> >
> > This patch removes a check which currently disallows mapping of a page
> when
> > the owner of the page tables matches the domain passed to
> > HYPERVISOR_mmu_update, but that domain is not the real owner of the
> page.
> > The check was introduced by patch d3c6a215ca9 ("x86: Clean up
> > get_page_from_l1e() to correctly distinguish between owner-of-pte and
> > owner-of-data-page in all cases") but it's not clear why it was needed.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> > ---
> > Cc: Jan Beulich <jbeulich@xxxxxxxx>
> > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > ---
> >  xen/arch/x86/mm.c | 13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> >
> > diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> > index c94f1e5406..bd8aeac59e 100644
> > --- a/xen/arch/x86/mm.c
> > +++ b/xen/arch/x86/mm.c
> > @@ -1024,12 +1024,15 @@ get_page_from_l1e(
> >                     (real_pg_owner != dom_cow) ) )
> >      {
> >          /*
> > -         * Let privileged domains transfer the right to map their target
> > -         * domain's pages. This is used to allow stub-domain pvfb export to
> > -         * dom0, until pvfb supports granted mappings. At that time this
> > -         * minor hack can go away.
> > +         * If the real page owner is not the domain specified in the
> > +         * hypercall then establish that the specified domain has
> > +         * mapping privilege over the page owner.
> > +         * This is used to allow stub-domain pvfb export to dom0. It is
> > +         * also used to allow a privileged PV domain to map mfns using
> > +         * DOMID_SELF, which is needed for mapping guest resources such
> > +         * grant table frames.
> >           */
> > -        if ( (real_pg_owner == NULL) || (pg_owner == l1e_owner) ||
> > +        if ( (real_pg_owner == NULL) ||
> 
> I still can't quite figure out if it is safe to remove the check.
> 
> Looking at the rest of the series, you already have the foreign domid to
> hand when you call the get_resource hypercall. What is wrong with using
> that directly? Why do you need DOMID_SELF in the first place?

Because the page is not in the foreign domain's p2m... that's kind of the 
entire point of the resource mapping API!

  Paul

> 
> >               xsm_priv_mapping(XSM_TARGET, pg_owner, real_pg_owner) )
> >          {
> >              gdprintk(XENLOG_WARNING,
> > --
> > 2.11.0
> >
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@xxxxxxxxxxxxx
> > https://lists.xen.org/xen-devel

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