Re: [PATCH v1 2/5] xen: Provide XEN_DMOP_add_to_physmap

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
> From: Xen-devel [mailto:xen-devel-bounces@lists.xen.org] On Behalf Of
> Paul Durrant
> Sent: 20 October 2017 10:16
> To: Ross Lagerwall <ross.lagerwall@citrix.com>; Xen-devel <xen-
> devel@lists.xen.org>
> Cc: Stefano Stabellini <sstabellini@kernel.org>; Wei Liu
> <wei.liu2@citrix.com>; Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>;
> Andrew Cooper <Andrew.Cooper3@citrix.com>; Tim (Xen.org)
> <tim@xen.org>; George Dunlap <George.Dunlap@citrix.com>; Ross
> Lagerwall <ross.lagerwall@citrix.com>; Jan Beulich <jbeulich@suse.com>; Ian
> Jackson <Ian.Jackson@citrix.com>
> Subject: Re: [Xen-devel] [PATCH v1 2/5] xen: Provide
> XEN_DMOP_add_to_physmap
> 
> > -----Original Message-----
> > From: Xen-devel [mailto:xen-devel-bounces@lists.xen.org] On Behalf Of
> > Ross Lagerwall
> > Sent: 18 October 2017 15:04
> > To: Xen-devel <xen-devel@lists.xen.org>
> > Cc: Stefano Stabellini <sstabellini@kernel.org>; Wei Liu
> > <wei.liu2@citrix.com>; Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>;
> > George Dunlap <George.Dunlap@citrix.com>; Andrew Cooper
> > <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Tim
> > (Xen.org) <tim@xen.org>; Ross Lagerwall <ross.lagerwall@citrix.com>; Jan
> > Beulich <jbeulich@suse.com>
> > Subject: [Xen-devel] [PATCH v1 2/5] xen: Provide
> > XEN_DMOP_add_to_physmap
> >
> > Provide XEN_DMOP_add_to_physmap, a limited version of
> > XENMEM_add_to_physmap to allow a deprivileged QEMU to move VRAM
> > when a
> > guest programs its BAR. It is equivalent to XENMEM_add_to_physmap with
> > space == XENMAPSPACE_gmfn.
> >
> > Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> > ---
> >  xen/arch/x86/hvm/dm.c          | 17 +++++++++++++++++
> >  xen/include/public/hvm/dm_op.h | 14 ++++++++++++++
> >  xen/include/xlat.lst           |  1 +
> >  3 files changed, 32 insertions(+)
> >
> > diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c
> > index 32ade95..432a863 100644
> > --- a/xen/arch/x86/hvm/dm.c
> > +++ b/xen/arch/x86/hvm/dm.c
> > @@ -640,6 +640,22 @@ static int dm_op(const struct dmop_args *op_args)
> >          break;
> >      }
> >
> > +    case XEN_DMOP_add_to_physmap:
> > +    {
> > +        const struct xen_dm_op_add_to_physmap *data =
> > +            &op.u.add_to_physmap;
> > +        struct xen_add_to_physmap xatp = {
> > +            .domid = op_args->domid,
> > +            .space = XENMAPSPACE_gmfn,
> > +            .idx = data->idx,
> > +            .gpfn = data->gpfn,
> > +        };
> > +
> 
> Where does xatp.size get set? Looks like you're missing a parameter.
> 
>   Paul
> 
> > +        rc = xenmem_add_to_physmap(d, &xatp,
> > +                                   XENMEM_add_to_physmap >>
> MEMOP_EXTENT_SHIFT);

... Also looking at this slightly odd argument, assuming that the additional size parameter could be arbitrarily large, you're going to need to handle continuations.

  Paul

> > +        break;
> > +    }
> > +
> >      default:
> >          rc = -EOPNOTSUPP;
> >          break;
> > @@ -669,6 +685,7 @@ CHECK_dm_op_set_mem_type;
> >  CHECK_dm_op_inject_event;
> >  CHECK_dm_op_inject_msi;
> >  CHECK_dm_op_remote_shutdown;
> > +CHECK_dm_op_add_to_physmap;
> >
> >  int compat_dm_op(domid_t domid,
> >                   unsigned int nr_bufs,
> > diff --git a/xen/include/public/hvm/dm_op.h
> > b/xen/include/public/hvm/dm_op.h
> > index e173085..88aace7 100644
> > --- a/xen/include/public/hvm/dm_op.h
> > +++ b/xen/include/public/hvm/dm_op.h
> > @@ -368,6 +368,19 @@ struct xen_dm_op_remote_shutdown {
> >                             /* (Other reason values are not blocked) */
> >  };
> >
> > +/*
> > + * XEN_DMOP_add_to_physmap : Sets the GPFN at which a particular
> page
> > appears
> > + *                           in the specified guest's pseudophysical address
> > + *                           space. Identical to XENMEM_add_to_physmap with
> > + *                           space == XENMAPSPACE_gmfn.
> > + */
> > +#define XEN_DMOP_add_to_physmap 17
> > +
> > +struct xen_dm_op_add_to_physmap {
> > +    uint64_aligned_t idx;  /* Index into GMFN space. */
> > +    uint64_aligned_t gpfn; /* GPFN in domid where the GMFN should
> > appear. */
> > +};
> > +
> >  struct xen_dm_op {
> >      uint32_t op;
> >      uint32_t pad;
> > @@ -389,6 +402,7 @@ struct xen_dm_op {
> >          struct xen_dm_op_map_mem_type_to_ioreq_server
> >                  map_mem_type_to_ioreq_server;
> >          struct xen_dm_op_remote_shutdown remote_shutdown;
> > +        struct xen_dm_op_add_to_physmap add_to_physmap;
> >      } u;
> >  };
> >
> > diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst
> > index 4346cbe..d40bac6 100644
> > --- a/xen/include/xlat.lst
> > +++ b/xen/include/xlat.lst
> > @@ -57,6 +57,7 @@
> >  ?	grant_entry_v2			grant_table.h
> >  ?	gnttab_swap_grant_ref		grant_table.h
> >  !	dm_op_buf			hvm/dm_op.h
> > +?	dm_op_add_to_physmap		hvm/dm_op.h
> >  ?	dm_op_create_ioreq_server	hvm/dm_op.h
> >  ?	dm_op_destroy_ioreq_server	hvm/dm_op.h
> >  ?	dm_op_get_ioreq_server_info	hvm/dm_op.h
> > --
> > 2.9.5
> >
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > https://lists.xen.org/xen-devel
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel