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

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

> -----Original Message-----
> From: Ross Lagerwall [mailto:ross.lagerwall@citrix.com]
> Sent: 20 October 2017 10:37
> To: Paul Durrant <Paul.Durrant@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>;
> George Dunlap <George.Dunlap@citrix.com>; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Tim
> (Xen.org) <tim@xen.org>; Jan Beulich <jbeulich@suse.com>
> Subject: Re: [Xen-devel] [PATCH v1 2/5] xen: Provide
> XEN_DMOP_add_to_physmap
> 
> On 10/20/2017 10:15 AM, Paul Durrant wrote:
> >> -----Original Message-----
> snip>> 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.
> >
> xatp.size is only used for XENMAPSPACE_gmfn_range which is not
> supported
> by this interface. size gets set to 0 by the C99 designated initializer.
> 
> Based on your other comments, would it make sense to instead use
> XENMAPSPACE_gmfn_range and have the caller set the size?

Yes... my eyes had read XENMAPSPACE_gmfn_range in the first place, hence my confusion over the size parameter.

> 
> As it is currently, QEMU does only populate VRAM one page at a time
> (using xen_xc_domain_add_to_physmap)

Ouch, yes, I'd forgotten that.

> so it is already slow but it could
> be improved.

Indeed. I think we should shoot for a better semantic given that it's a new op.

  Paul

> 
> --
> Ross Lagerwall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel