Re: [Xen-devel] [RFC][v2][PATCH 07/14] xen/passthrough: extend hypercall to support rdm reservation policy

[Julien Grall <julien.grall@xxxxxxxxxx>]

Hi,

On 25/05/2015 12:50, Chen, Tiejun wrote:
> On 2015/5/25 18:02, Julien Grall wrote:
> > On 25/05/2015 04:09, Chen, Tiejun wrote:
> > > On 2015/5/22 18:33, Julien Grall wrote:
> > > > Hi,
> > > >
> > > > On 22/05/2015 10:35, Tiejun Chen wrote:
> > > > > diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> > > > > index 0c0ea4a..203c80e 100644
> > > > > --- a/xen/include/public/domctl.h
> > > > > +++ b/xen/include/public/domctl.h
> > > > > @@ -499,6 +499,11 @@ struct xen_domctl_assign_device {
> > > > >               XEN_GUEST_HANDLE_64(char) path; /* path to the device
> > > > > tree node */
> > > > >           } dt;
> > > > >       } u;
> > > > > +    /* IN */
> > > > > +#define XEN_DOMCTL_DEV_NO_RDM           0
> > > > > +#define XEN_DOMCTL_DEV_RDM_RELAXED      1
> > > > > +#define XEN_DOMCTL_DEV_RDM_STRICT       2
> > > > > +    uint32_t  flag;   /* flag of assigned device */
> > > > You don't plumb this value for DT neither in the toolstack (see
> > > > xc_assign_dt_device) and Xen. Please add a comment saying it's only
> > > > used
> > > I think we should do this,
> > >
> > > @@ -1801,6 +1801,8 @@ int xc_assign_dt_device(
> > >
> > >       domctl.u.assign_device.dev = XEN_DOMCTL_DEV_DT;
> > >       domctl.u.assign_device.u.dt.size = size;
> > > +    /* DT doesn't own any RDM. */
> > > +    domctl.u.assign_device.flag = XEN_DOMCTL_DEV_NO_RDM;
> > >       set_xen_guest_handle(domctl.u.assign_device.u.dt.path, path);
> > >
> > >       rc = do_domctl(xch, &domctl);
> > I would be fine with plumbing in drivers/passthrough/device_tree.c and a
> > check that the value is not different.
> Are you saying something like this?
>
> @@ -149,6 +149,14 @@ int iommu_do_dt_domctl(struct xen_domctl *domctl,
> struct domain *d,
>           if ( domctl->u.assign_device.dev != XEN_DOMCTL_DEV_DT )
>               break;
>
> +        if ( domctl->u.assign_device.dev == XEN_DOMCTL_DEV_NO_RDM )
wrong field here. Other than that it looks good to me.

> +        {
> +            printk(XENLOG_G_ERR "XEN_DOMCTL_assign_device: assign \"%s\""
> +                   " to dom%u failed (%d) since we don't support RDM.\n",
> +                   dt_node_full_name(dev), d->domain_id, ret);
> +            break;
> +        }
> +
>           if ( unlikely(d->is_dying) )
>           {
>               ret = -EINVAL;
Regards,

--
Julien Grall

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