|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] Ping: [PATCH v3] passthrough: give XEN_DOMCTL_test_assign_device more sane semantics
>>> On 16.08.17 at 14:20, <JBeulich@xxxxxxxx> wrote:
> So far callers of the libxc interface passed in a domain ID which was
> then ignored in the hypervisor. Instead, make the hypervisor honor it
> (accepting DOMID_INVALID to obtain original behavior), allowing to
> query whether a device can be assigned to a particular domain.
>
> Drop XSM's test_assign_{,dt}device hooks as no longer being
> individually useful.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> v3: Drop test-assign XSM hooks.
> v2: Alter the semantics to check whether the device can be assigned to
> the passed in domain.
>
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -391,11 +391,15 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe
>
> switch ( op->cmd )
> {
> - case XEN_DOMCTL_createdomain:
> case XEN_DOMCTL_test_assign_device:
> + if ( op->domain == DOMID_INVALID )
> + {
> + case XEN_DOMCTL_createdomain:
> case XEN_DOMCTL_gdbsx_guestmemio:
> - d = NULL;
> - break;
> + d = NULL;
> + break;
> + }
> + /* fall through */
> default:
> d = rcu_lock_domain_by_id(op->domain);
> if ( !d && op->cmd != XEN_DOMCTL_getdomaininfo )
> --- a/xen/drivers/passthrough/device_tree.c
> +++ b/xen/drivers/passthrough/device_tree.c
> @@ -143,12 +143,15 @@ int iommu_do_dt_domctl(struct xen_domctl
> switch ( domctl->cmd )
> {
> case XEN_DOMCTL_assign_device:
> + ASSERT(d);
> + /* fall through */
> + case XEN_DOMCTL_test_assign_device:
> ret = -ENODEV;
> if ( domctl->u.assign_device.dev != XEN_DOMCTL_DEV_DT )
> break;
>
> ret = -EINVAL;
> - if ( d->is_dying || domctl->u.assign_device.flags )
> + if ( (d && d->is_dying) || domctl->u.assign_device.flags )
> break;
>
> ret = dt_find_node_by_gpath(domctl->u.assign_device.u.dt.path,
> @@ -161,6 +164,17 @@ int iommu_do_dt_domctl(struct xen_domctl
> if ( ret )
> break;
>
> + if ( domctl->cmd == XEN_DOMCTL_test_assign_device )
> + {
> + if ( iommu_dt_device_is_assigned(dev) )
> + {
> + printk(XENLOG_G_ERR "%s already assigned.\n",
> + dt_node_full_name(dev));
> + ret = -EINVAL;
> + }
> + break;
> + }
> +
> ret = iommu_assign_dt_device(d, dev);
>
> if ( ret )
> @@ -194,33 +208,6 @@ int iommu_do_dt_domctl(struct xen_domctl
> dt_node_full_name(dev), d->domain_id, ret);
> break;
>
> - case XEN_DOMCTL_test_assign_device:
> - ret = -ENODEV;
> - if ( domctl->u.assign_device.dev != XEN_DOMCTL_DEV_DT )
> - break;
> -
> - ret = -EINVAL;
> - if ( domctl->u.assign_device.flags )
> - break;
> -
> - ret = dt_find_node_by_gpath(domctl->u.assign_device.u.dt.path,
> - domctl->u.assign_device.u.dt.size,
> - &dev);
> - if ( ret )
> - break;
> -
> - ret = xsm_test_assign_dtdevice(XSM_HOOK, dt_node_full_name(dev));
> - if ( ret )
> - break;
> -
> - if ( iommu_dt_device_is_assigned(dev) )
> - {
> - printk(XENLOG_G_ERR "%s already assigned.\n",
> - dt_node_full_name(dev));
> - ret = -EINVAL;
> - }
> - break;
> -
> default:
> ret = -ENOSYS;
> break;
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -1583,35 +1583,10 @@ int iommu_do_pci_domctl(
> }
> break;
>
> - case XEN_DOMCTL_test_assign_device:
> - ret = -ENODEV;
> - if ( domctl->u.assign_device.dev != XEN_DOMCTL_DEV_PCI )
> - break;
> -
> - ret = -EINVAL;
> - if ( domctl->u.assign_device.flags )
> - break;
> -
> - machine_sbdf = domctl->u.assign_device.u.pci.machine_sbdf;
> -
> - ret = xsm_test_assign_device(XSM_HOOK, machine_sbdf);
> - if ( ret )
> - break;
> -
> - seg = machine_sbdf >> 16;
> - bus = PCI_BUS(machine_sbdf);
> - devfn = PCI_DEVFN2(machine_sbdf);
> -
> - if ( device_assigned(seg, bus, devfn) )
> - {
> - printk(XENLOG_G_INFO
> - "%04x:%02x:%02x.%u already assigned, or non-existent\n",
> - seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
> - ret = -EINVAL;
> - }
> - break;
> -
> case XEN_DOMCTL_assign_device:
> + ASSERT(d);
> + /* fall through */
> + case XEN_DOMCTL_test_assign_device:
> /* Don't support self-assignment of devices. */
> if ( d == current->domain )
> {
> @@ -1625,7 +1600,9 @@ int iommu_do_pci_domctl(
>
> ret = -EINVAL;
> flags = domctl->u.assign_device.flags;
> - if ( d->is_dying || (flags & ~XEN_DOMCTL_DEV_RDM_RELAXED) )
> + if ( domctl->cmd == XEN_DOMCTL_assign_device
> + ? d->is_dying || (flags & ~XEN_DOMCTL_DEV_RDM_RELAXED)
> + : flags )
> break;
>
> machine_sbdf = domctl->u.assign_device.u.pci.machine_sbdf;
> @@ -1638,8 +1615,20 @@ int iommu_do_pci_domctl(
> bus = PCI_BUS(machine_sbdf);
> devfn = PCI_DEVFN2(machine_sbdf);
>
> - ret = device_assigned(seg, bus, devfn) ?:
> - assign_device(d, seg, bus, devfn, flags);
> + ret = device_assigned(seg, bus, devfn);
> + if ( domctl->cmd == XEN_DOMCTL_test_assign_device )
> + {
> + if ( ret )
> + {
> + printk(XENLOG_G_INFO
> + "%04x:%02x:%02x.%u already assigned, or
> non-existent\n",
> + seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
> + ret = -EINVAL;
> + }
> + break;
> + }
> + if ( !ret )
> + ret = assign_device(d, seg, bus, devfn, flags);
> if ( ret == -ERESTART )
> ret = hypercall_create_continuation(__HYPERVISOR_domctl,
> "h", u_domctl);
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -506,7 +506,11 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_sendt
>
> /* Assign a device to a guest. Sets up IOMMU structures. */
> /* XEN_DOMCTL_assign_device */
> -/* XEN_DOMCTL_test_assign_device */
> +/*
> + * XEN_DOMCTL_test_assign_device: Pass DOMID_INVALID to find out whether
> the
> + * given device is assigned to any DomU at all. Pass a specific domain ID
> to
> + * find out whether the given device can be assigned to that domain.
> + */
> /*
> * XEN_DOMCTL_deassign_device: The behavior of this DOMCTL differs
> * between the different type of device:
> --- a/xen/include/xsm/dummy.h
> +++ b/xen/include/xsm/dummy.h
> @@ -337,12 +337,6 @@ static XSM_INLINE int xsm_get_device_gro
> return xsm_default_action(action, current->domain, NULL);
> }
>
> -static XSM_INLINE int xsm_test_assign_device(XSM_DEFAULT_ARG uint32_t
> machine_bdf)
> -{
> - XSM_ASSERT_ACTION(XSM_HOOK);
> - return xsm_default_action(action, current->domain, NULL);
> -}
> -
> static XSM_INLINE int xsm_assign_device(XSM_DEFAULT_ARG struct domain *d,
> uint32_t machine_bdf)
> {
> XSM_ASSERT_ACTION(XSM_HOOK);
> @@ -358,12 +352,6 @@ static XSM_INLINE int xsm_deassign_devic
> #endif /* HAS_PASSTHROUGH && HAS_PCI */
>
> #if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_DEVICE_TREE)
> -static XSM_INLINE int xsm_test_assign_dtdevice(XSM_DEFAULT_ARG const char
> *dtpath)
> -{
> - XSM_ASSERT_ACTION(XSM_HOOK);
> - return xsm_default_action(action, current->domain, NULL);
> -}
> -
> static XSM_INLINE int xsm_assign_dtdevice(XSM_DEFAULT_ARG struct domain *d,
> const char *dtpath)
> {
> --- a/xen/include/xsm/xsm.h
> +++ b/xen/include/xsm/xsm.h
> @@ -109,13 +109,11 @@ struct xsm_operations {
>
> #if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_PCI)
> int (*get_device_group) (uint32_t machine_bdf);
> - int (*test_assign_device) (uint32_t machine_bdf);
> int (*assign_device) (struct domain *d, uint32_t machine_bdf);
> int (*deassign_device) (struct domain *d, uint32_t machine_bdf);
> #endif
>
> #if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_DEVICE_TREE)
> - int (*test_assign_dtdevice) (const char *dtpath);
> int (*assign_dtdevice) (struct domain *d, const char *dtpath);
> int (*deassign_dtdevice) (struct domain *d, const char *dtpath);
> #endif
> @@ -465,11 +463,6 @@ static inline int xsm_get_device_group(x
> return xsm_ops->get_device_group(machine_bdf);
> }
>
> -static inline int xsm_test_assign_device(xsm_default_t def, uint32_t
> machine_bdf)
> -{
> - return xsm_ops->test_assign_device(machine_bdf);
> -}
> -
> static inline int xsm_assign_device(xsm_default_t def, struct domain *d,
> uint32_t machine_bdf)
> {
> return xsm_ops->assign_device(d, machine_bdf);
> @@ -488,12 +481,6 @@ static inline int xsm_assign_dtdevice(xs
> return xsm_ops->assign_dtdevice(d, dtpath);
> }
>
> -static inline int xsm_test_assign_dtdevice(xsm_default_t def,
> - const char *dtpath)
> -{
> - return xsm_ops->test_assign_dtdevice(dtpath);
> -}
> -
> static inline int xsm_deassign_dtdevice(xsm_default_t def, struct domain
> *d,
> const char *dtpath)
> {
> --- a/xen/xsm/dummy.c
> +++ b/xen/xsm/dummy.c
> @@ -91,13 +91,11 @@ void __init xsm_fixup_ops (struct xsm_op
>
> #if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_PCI)
> set_to_dummy_if_null(ops, get_device_group);
> - set_to_dummy_if_null(ops, test_assign_device);
> set_to_dummy_if_null(ops, assign_device);
> set_to_dummy_if_null(ops, deassign_device);
> #endif
>
> #if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_DEVICE_TREE)
> - set_to_dummy_if_null(ops, test_assign_dtdevice);
> set_to_dummy_if_null(ops, assign_dtdevice);
> set_to_dummy_if_null(ops, deassign_dtdevice);
> #endif
> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -1278,6 +1281,9 @@ static int flask_assign_device(struct do
> int rc = -EPERM;
> struct avc_audit_data ad;
>
> + if ( !d )
> + return flask_test_assign_device(machine_bdf);
> +
> rc = current_has_perm(d, SECCLASS_RESOURCE, RESOURCE__ADD);
> if ( rc )
> return rc;
> @@ -1333,6 +1339,9 @@ static int flask_assign_dtdevice(struct
> int rc = -EPERM;
> struct avc_audit_data ad;
>
> + if ( !d )
> + return flask_test_assign_dtdevice(dtpath);
> +
> rc = current_has_perm(d, SECCLASS_RESOURCE, RESOURCE__ADD);
> if ( rc )
> return rc;
> @@ -1780,13 +1789,11 @@ static struct xsm_operations flask_ops =
>
> #if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_PCI)
> .get_device_group = flask_get_device_group,
> - .test_assign_device = flask_test_assign_device,
> .assign_device = flask_assign_device,
> .deassign_device = flask_deassign_device,
> #endif
>
> #if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_DEVICE_TREE)
> - .test_assign_dtdevice = flask_test_assign_dtdevice,
> .assign_dtdevice = flask_assign_dtdevice,
> .deassign_dtdevice = flask_deassign_dtdevice,
> #endif
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |