[Xen-devel] [PATCH v2 02/11] hvmctl: convert HVMOP_set_pci_intx_level

["Jan Beulich" <JBeulich@xxxxxxxx>]

Note that this adds validation of the "domain" interface structure
field, which previously got ignored.

Note further that this retains the hvmop interface definitions as those
had (wrongly) been exposed to non-tool stack consumers (albeit the
operation wouldn't have succeeded when requested by a domain for
itself).

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
Acked-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
Reviewed-by: Wei Liu <wei.liu2@xxxxxxxxxx>
Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
v2: Drop bogus comment from xen/xsm/flask/policy/access_vectors.

--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -473,30 +473,14 @@ int xc_hvm_set_pci_intx_level(
     uint8_t domain, uint8_t bus, uint8_t device, uint8_t intx,
     unsigned int level)
 {
-    DECLARE_HYPERCALL_BUFFER(struct xen_hvm_set_pci_intx_level, arg);
-    int rc;
+    DECLARE_HVMCTL(set_pci_intx_level, dom,
+                   .domain = domain,
+                   .bus    = bus,
+                   .device = device,
+                   .intx   = intx,
+                   .level  = level);
 
-    arg = xc_hypercall_buffer_alloc(xch, arg, sizeof(*arg));
-    if ( arg == NULL )
-    {
-        PERROR("Could not allocate memory for xc_hvm_set_pci_intx_level 
hypercall");
-        return -1;
-    }
-
-    arg->domid  = dom;
-    arg->domain = domain;
-    arg->bus    = bus;
-    arg->device = device;
-    arg->intx   = intx;
-    arg->level  = level;
-
-    rc = xencall2(xch->xcall, __HYPERVISOR_hvm_op,
-                  HVMOP_set_pci_intx_level,
-                  HYPERCALL_BUFFER_AS_ARG(arg));
-
-    xc_hypercall_buffer_free(xch, arg);
-
-    return rc;
+    return do_hvmctl(xch, &hvmctl);
 }
 
 int xc_hvm_set_isa_irq_level(
--- a/tools/libxc/xc_private.h
+++ b/tools/libxc/xc_private.h
@@ -34,6 +34,8 @@
 #define XC_INTERNAL_COMPAT_MAP_FOREIGN_API
 #include "xenctrl.h"
 
+#include <xen/hvm/control.h>
+
 #include <xencall.h>
 #include <xenforeignmemory.h>
 
@@ -61,6 +63,13 @@ struct iovec {
 
 #define DECLARE_DOMCTL struct xen_domctl domctl
 #define DECLARE_SYSCTL struct xen_sysctl sysctl
+#define DECLARE_HVMCTL(op, dom, init...) \
+    struct xen_hvmctl hvmctl = { \
+        .interface_version = XEN_HVMCTL_INTERFACE_VERSION, \
+        .domain = (dom), \
+        .cmd = XEN_HVMCTL_##op, \
+        .u.op = { init } \
+    }
 #define DECLARE_PHYSDEV_OP struct physdev_op physdev_op
 #define DECLARE_FLASK_OP struct xen_flask_op op
 #define DECLARE_PLATFORM_OP struct xen_platform_op platform_op
@@ -311,6 +320,31 @@ static inline int do_sysctl(xc_interface
     return ret;
 }
 
+static inline int do_hvmctl(xc_interface *xch, struct xen_hvmctl *hvmctl)
+{
+    int ret = -1;
+    DECLARE_HYPERCALL_BOUNCE(hvmctl, sizeof(*hvmctl), 
XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
+
+    if ( xc_hypercall_bounce_pre(xch, hvmctl) )
+    {
+        PERROR("Could not bounce buffer for hvmctl hypercall");
+        return -1;
+    }
+
+    ret = xencall1(xch->xcall, __HYPERVISOR_hvmctl,
+                   HYPERCALL_BUFFER_AS_ARG(hvmctl));
+    if ( ret < 0 )
+    {
+        if ( errno == EACCES )
+            DPRINTF("hvmctl operation failed -- need to"
+                    " rebuild the user-space tool set?\n");
+    }
+
+    xc_hypercall_bounce_post(xch, hvmctl);
+
+    return ret;
+}
+
 static inline int do_platform_op(xc_interface *xch,
                                  struct xen_platform_op *platform_op)
 {
--- a/xen/arch/x86/hvm/control.c
+++ b/xen/arch/x86/hvm/control.c
@@ -19,6 +19,30 @@
 #include <xen/sched.h>
 #include <xsm/xsm.h>
 
+static int set_pci_intx_level(struct domain *d,
+                              const struct xen_hvm_set_pci_intx_level *op)
+{
+    if ( op->domain || op->bus || (op->device > 31) || (op->intx > 3) )
+        return -EINVAL;
+
+    if ( !is_hvm_domain(d) )
+        return -EINVAL;
+
+    switch ( op->level )
+    {
+    case 0:
+        hvm_pci_intx_deassert(d, op->device, op->intx);
+        break;
+    case 1:
+        hvm_pci_intx_assert(d, op->device, op->intx);
+        break;
+    default:
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
 long do_hvmctl(XEN_GUEST_HANDLE_PARAM(xen_hvmctl_t) u_hvmctl)
 {
     xen_hvmctl_t op;
@@ -52,6 +76,10 @@ long do_hvmctl(XEN_GUEST_HANDLE_PARAM(xe
 
     switch ( op.cmd )
     {
+    case XEN_HVMCTL_set_pci_intx_level:
+        rc = set_pci_intx_level(d, &op.u.set_pci_intx_level);
+        break;
+
     default:
         rc = -EOPNOTSUPP;
         break;
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4303,50 +4303,6 @@ void hvm_hypercall_page_initialise(struc
     hvm_funcs.init_hypercall_page(d, hypercall_page);
 }
 
-static int hvmop_set_pci_intx_level(
-    XEN_GUEST_HANDLE_PARAM(xen_hvm_set_pci_intx_level_t) uop)
-{
-    struct xen_hvm_set_pci_intx_level op;
-    struct domain *d;
-    int rc;
-
-    if ( copy_from_guest(&op, uop, 1) )
-        return -EFAULT;
-
-    if ( (op.domain > 0) || (op.bus > 0) || (op.device > 31) || (op.intx > 3) )
-        return -EINVAL;
-
-    rc = rcu_lock_remote_domain_by_id(op.domid, &d);
-    if ( rc != 0 )
-        return rc;
-
-    rc = -EINVAL;
-    if ( !is_hvm_domain(d) )
-        goto out;
-
-    rc = xsm_hvm_set_pci_intx_level(XSM_DM_PRIV, d);
-    if ( rc )
-        goto out;
-
-    rc = 0;
-    switch ( op.level )
-    {
-    case 0:
-        hvm_pci_intx_deassert(d, op.device, op.intx);
-        break;
-    case 1:
-        hvm_pci_intx_assert(d, op.device, op.intx);
-        break;
-    default:
-        rc = -EINVAL;
-        break;
-    }
-
- out:
-    rcu_unlock_domain(d);
-    return rc;
-}
-
 void hvm_vcpu_reset_state(struct vcpu *v, uint16_t cs, uint16_t ip)
 {
     struct domain *d = v->domain;
@@ -5408,11 +5364,6 @@ long do_hvm_op(unsigned long op, XEN_GUE
             guest_handle_cast(arg, xen_hvm_param_t));
         break;
 
-    case HVMOP_set_pci_intx_level:
-        rc = hvmop_set_pci_intx_level(
-            guest_handle_cast(arg, xen_hvm_set_pci_intx_level_t));
-        break;
-
     case HVMOP_set_isa_irq_level:
         rc = hvmop_set_isa_irq_level(
             guest_handle_cast(arg, xen_hvm_set_isa_irq_level_t));
--- a/xen/include/public/hvm/control.h
+++ b/xen/include/public/hvm/control.h
@@ -29,12 +29,23 @@
 
 #define XEN_HVMCTL_INTERFACE_VERSION 0x00000001
 
+/* XEN_HVMCTL_set_pci_intx_level */
+/* Set the logical level of one of a domain's PCI INTx wires. */
+struct xen_hvm_set_pci_intx_level {
+    /* PCI INTx identification in PCI topology (domain:bus:device:intx). */
+    uint8_t domain, bus, device, intx;
+    /* Assertion level (0 = unasserted, 1 = asserted). */
+    uint8_t level;
+};
+
 struct xen_hvmctl {
     uint16_t interface_version;    /* XEN_HVMCTL_INTERFACE_VERSION */
     domid_t domain;
     uint32_t cmd;
+#define XEN_HVMCTL_set_pci_intx_level            1
     uint64_t opaque;               /* Must be zero on initial invocation. */
     union {
+        struct xen_hvm_set_pci_intx_level set_pci_intx_level;
         uint8_t pad[120];
     } u;
 };
--- a/xen/include/public/hvm/hvm_op.h
+++ b/xen/include/public/hvm/hvm_op.h
@@ -38,6 +38,8 @@ struct xen_hvm_param {
 typedef struct xen_hvm_param xen_hvm_param_t;
 DEFINE_XEN_GUEST_HANDLE(xen_hvm_param_t);
 
+#if __XEN_INTERFACE_VERSION__ < 0x00040800
+
 /* Set the logical level of one of a domain's PCI INTx wires. */
 #define HVMOP_set_pci_intx_level  2
 struct xen_hvm_set_pci_intx_level {
@@ -51,6 +53,8 @@ struct xen_hvm_set_pci_intx_level {
 typedef struct xen_hvm_set_pci_intx_level xen_hvm_set_pci_intx_level_t;
 DEFINE_XEN_GUEST_HANDLE(xen_hvm_set_pci_intx_level_t);
 
+#endif
+
 /* Set the logical level of one of a domain's ISA IRQ wires. */
 #define HVMOP_set_isa_irq_level   3
 struct xen_hvm_set_isa_irq_level {
--- a/xen/include/public/xen-compat.h
+++ b/xen/include/public/xen-compat.h
@@ -27,7 +27,7 @@
 #ifndef __XEN_PUBLIC_XEN_COMPAT_H__
 #define __XEN_PUBLIC_XEN_COMPAT_H__
 
-#define __XEN_LATEST_INTERFACE_VERSION__ 0x00040700
+#define __XEN_LATEST_INTERFACE_VERSION__ 0x00040800
 
 #if defined(__XEN__) || defined(__XEN_TOOLS__)
 /* Xen is built with matching headers and implements the latest interface. */
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -609,12 +609,6 @@ static XSM_INLINE int xsm_shadow_control
     return xsm_default_action(action, current->domain, d);
 }
 
-static XSM_INLINE int xsm_hvm_set_pci_intx_level(XSM_DEFAULT_ARG struct domain 
*d)
-{
-    XSM_ASSERT_ACTION(XSM_DM_PRIV);
-    return xsm_default_action(action, current->domain, d);
-}
-
 static XSM_INLINE int xsm_hvm_set_isa_irq_level(XSM_DEFAULT_ARG struct domain 
*d)
 {
     XSM_ASSERT_ACTION(XSM_DM_PRIV);
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -173,7 +173,6 @@ struct xsm_operations {
 #ifdef CONFIG_X86
     int (*do_mca) (void);
     int (*shadow_control) (struct domain *d, uint32_t op);
-    int (*hvm_set_pci_intx_level) (struct domain *d);
     int (*hvm_set_isa_irq_level) (struct domain *d);
     int (*hvm_set_pci_link_route) (struct domain *d);
     int (*hvm_inject_msi) (struct domain *d);
@@ -646,11 +645,6 @@ static inline int xsm_shadow_control (xs
     return xsm_ops->shadow_control(d, op);
 }
 
-static inline int xsm_hvm_set_pci_intx_level (xsm_default_t def, struct domain 
*d)
-{
-    return xsm_ops->hvm_set_pci_intx_level(d);
-}
-
 static inline int xsm_hvm_set_isa_irq_level (xsm_default_t def, struct domain 
*d)
 {
     return xsm_ops->hvm_set_isa_irq_level(d);
--- a/xen/xsm/dummy.c
+++ b/xen/xsm/dummy.c
@@ -145,7 +145,6 @@ void xsm_fixup_ops (struct xsm_operation
 #ifdef CONFIG_X86
     set_to_dummy_if_null(ops, do_mca);
     set_to_dummy_if_null(ops, shadow_control);
-    set_to_dummy_if_null(ops, hvm_set_pci_intx_level);
     set_to_dummy_if_null(ops, hvm_set_isa_irq_level);
     set_to_dummy_if_null(ops, hvm_set_pci_link_route);
     set_to_dummy_if_null(ops, hvm_inject_msi);
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -27,6 +27,7 @@
 #include <public/physdev.h>
 #include <public/platform.h>
 #include <public/version.h>
+#include <public/hvm/control.h>
 
 #include <public/xsm/flask_op.h>
 
@@ -1189,6 +1190,9 @@ static int flask_hvm_control(struct doma
 
     switch ( op )
     {
+    case XEN_HVMCTL_set_pci_intx_level:
+        perm = HVM__PCILEVEL;
+        break;
     default:
         perm = HVM__HVMCTL;
         break;
@@ -1513,11 +1517,6 @@ static int flask_ioport_mapping(struct d
     return flask_ioport_permission(d, start, end, access);
 }
 
-static int flask_hvm_set_pci_intx_level(struct domain *d)
-{
-    return current_has_perm(d, SECCLASS_HVM, HVM__PCILEVEL);
-}
-
 static int flask_hvm_set_isa_irq_level(struct domain *d)
 {
     return current_has_perm(d, SECCLASS_HVM, HVM__IRQLEVEL);
@@ -1806,7 +1805,6 @@ static struct xsm_operations flask_ops =
 #ifdef CONFIG_X86
     .do_mca = flask_do_mca,
     .shadow_control = flask_shadow_control,
-    .hvm_set_pci_intx_level = flask_hvm_set_pci_intx_level,
     .hvm_set_isa_irq_level = flask_hvm_set_isa_irq_level,
     .hvm_set_pci_link_route = flask_hvm_set_pci_link_route,
     .hvm_inject_msi = flask_hvm_inject_msi,
--- a/xen/xsm/flask/policy/access_vectors
+++ b/xen/xsm/flask/policy/access_vectors
@@ -261,7 +261,7 @@ class hvm
     setparam
 # HVMOP_get_param
     getparam
-# HVMOP_set_pci_intx_level (also needs hvmctl)
+# XEN_HVMCTL_set_pci_intx_level
     pcilevel
 # HVMOP_set_isa_irq_level
     irqlevel

Attachment: hvmctl-01.patch
Description: Text document

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