Xen project Mailing List

[Xen-devel] [PATCH] domctl: improve device assignment structure layout and use

To: "xen-devel" <xen-devel@xxxxxxxxxxxxxxxxxxxx>

From: "Jan Beulich" <JBeulich@xxxxxxxx>

Date: Fri, 09 Jun 2017 07:47:12 -0600

Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wei.liu2@xxxxxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>, Tim Deegan <tim@xxxxxxx>, Julien Grall <julien.grall@xxxxxxx>

Delivery-date: Fri, 09 Jun 2017 13:47:32 +0000

List-id: Xen developer discussion <xen-devel.lists.xen.org>

Avoid needless gaps. Make flags field mandatory for all three operations (and rename it to fit the intended future purpose of possibly holding more than just one flag). Also correct a typo in a related domctl.h comment. Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx> --- The domain ID treatment of test_assign_device is at least strange: Callers pass in a value, but do_domctl() as well as the actual handler code ignore them. I think we should increase flexibility here by making e.g. DOMID_INVALID a wild card to obtain current behavior, while allowing individual domain IDs to be passed in to inquire whether the device is assigned to that specific domain. --- a/tools/libxc/xc_domain.c +++ b/tools/libxc/xc_domain.c @@ -1488,7 +1488,7 @@ int xc_assign_device( xc_interface *xch, uint32_t domid, uint32_t machine_sbdf, - uint32_t flag) + uint32_t flags) { DECLARE_DOMCTL; @@ -1496,7 +1496,7 @@ int xc_assign_device( domctl.domain = domid; domctl.u.assign_device.dev = XEN_DOMCTL_DEV_PCI; domctl.u.assign_device.u.pci.machine_sbdf = machine_sbdf; - domctl.u.assign_device.flag = flag; + domctl.u.assign_device.flags = flags; return do_domctl(xch, &domctl); } @@ -1547,6 +1547,7 @@ int xc_test_assign_device( domctl.domain = domid; domctl.u.assign_device.dev = XEN_DOMCTL_DEV_PCI; domctl.u.assign_device.u.pci.machine_sbdf = machine_sbdf; + domctl.u.assign_device.flags = 0; return do_domctl(xch, &domctl); } @@ -1562,6 +1563,7 @@ int xc_deassign_device( domctl.domain = domid; domctl.u.assign_device.dev = XEN_DOMCTL_DEV_PCI; domctl.u.assign_device.u.pci.machine_sbdf = machine_sbdf; + domctl.u.assign_device.flags = 0; return do_domctl(xch, &domctl); } @@ -1588,7 +1590,7 @@ int xc_assign_dt_device( * DT doesn't own any RDM so actually DT has nothing to do * for any flag and here just fix that as 0. */ - domctl.u.assign_device.flag = 0; + domctl.u.assign_device.flags = 0; set_xen_guest_handle(domctl.u.assign_device.u.dt.path, path); rc = do_domctl(xch, &domctl); @@ -1617,6 +1619,7 @@ int xc_test_assign_dt_device( domctl.u.assign_device.dev = XEN_DOMCTL_DEV_DT; domctl.u.assign_device.u.dt.size = size; set_xen_guest_handle(domctl.u.assign_device.u.dt.path, path); + domctl.u.assign_device.flags = 0; rc = do_domctl(xch, &domctl); @@ -1644,6 +1646,7 @@ int xc_deassign_dt_device( domctl.u.assign_device.dev = XEN_DOMCTL_DEV_DT; domctl.u.assign_device.u.dt.size = size; set_xen_guest_handle(domctl.u.assign_device.u.dt.path, path); + domctl.u.assign_device.flags = 0; rc = do_domctl(xch, &domctl); --- a/xen/drivers/passthrough/device_tree.c +++ b/xen/drivers/passthrough/device_tree.c @@ -147,11 +147,9 @@ int iommu_do_dt_domctl(struct xen_domctl if ( domctl->u.assign_device.dev != XEN_DOMCTL_DEV_DT ) break; - if ( unlikely(d->is_dying) ) - { - ret = -EINVAL; + ret = -EINVAL; + if ( d->is_dying || 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, @@ -176,6 +174,10 @@ int iommu_do_dt_domctl(struct xen_domctl 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); @@ -197,6 +199,10 @@ int iommu_do_dt_domctl(struct xen_domctl 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); --- a/xen/drivers/passthrough/pci.c +++ b/xen/drivers/passthrough/pci.c @@ -1540,12 +1540,13 @@ int iommu_do_pci_domctl( { u16 seg; u8 bus, devfn; - u32 flag; int ret = 0; uint32_t machine_sbdf; switch ( domctl->cmd ) { + unsigned int flags; + case XEN_DOMCTL_get_device_group: { u32 max_sdevs; @@ -1583,6 +1584,10 @@ int iommu_do_pci_domctl( 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); @@ -1614,11 +1619,10 @@ int iommu_do_pci_domctl( if ( domctl->u.assign_device.dev != XEN_DOMCTL_DEV_PCI ) break; - if ( unlikely(d->is_dying) ) - { - ret = -EINVAL; + ret = -EINVAL; + flags = domctl->u.assign_device.flags; + if ( d->is_dying || (flags & ~XEN_DOMCTL_DEV_RDM_RELAXED) ) break; - } machine_sbdf = domctl->u.assign_device.u.pci.machine_sbdf; @@ -1629,15 +1633,9 @@ int iommu_do_pci_domctl( seg = machine_sbdf >> 16; bus = PCI_BUS(machine_sbdf); devfn = PCI_DEVFN2(machine_sbdf); - flag = domctl->u.assign_device.flag; - if ( flag & ~XEN_DOMCTL_DEV_RDM_RELAXED ) - { - ret = -EINVAL; - break; - } ret = device_assigned(seg, bus, devfn) ?: - assign_device(d, seg, bus, devfn, flag); + assign_device(d, seg, bus, devfn, flags); if ( ret == -ERESTART ) ret = hypercall_create_continuation(__HYPERVISOR_domctl, "h", u_domctl); @@ -1661,6 +1659,10 @@ int iommu_do_pci_domctl( 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_deassign_device(XSM_HOOK, d, machine_sbdf); --- a/xen/include/public/domctl.h +++ b/xen/include/public/domctl.h @@ -511,14 +511,17 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_sendt * XEN_DOMCTL_deassign_device: The behavior of this DOMCTL differs * between the different type of device: * - PCI device (XEN_DOMCTL_DEV_PCI) will be reassigned to DOM0 - * - DT device (XEN_DOMCTL_DT_PCI) will left unassigned. DOM0 + * - DT device (XEN_DOMCTL_DEV_DT) will left unassigned. DOM0 * will have to call XEN_DOMCTL_assign_device in order to use the * device. */ #define XEN_DOMCTL_DEV_PCI 0 #define XEN_DOMCTL_DEV_DT 1 struct xen_domctl_assign_device { + /* IN */ uint32_t dev; /* XEN_DOMCTL_DEV_* */ + uint32_t flags; +#define XEN_DOMCTL_DEV_RDM_RELAXED 1 /* assign only */ union { struct { uint32_t machine_sbdf; /* machine PCI ID of assigned device */ @@ -528,9 +531,6 @@ struct xen_domctl_assign_device { XEN_GUEST_HANDLE_64(char) path; /* path to the device tree node */ } dt; } u; - /* IN */ -#define XEN_DOMCTL_DEV_RDM_RELAXED 1 - uint32_t flag; /* flag of assigned device */ }; typedef struct xen_domctl_assign_device xen_domctl_assign_device_t; DEFINE_XEN_GUEST_HANDLE(xen_domctl_assign_device_t);

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

©2013 Xen Project, A Linux Foundation Collaborative Project. All Rights Reserved.
Linux Foundation is a registered trademark of The Linux Foundation.
Xen Project is a trademark of The Linux Foundation.