diff mbox

[v2,01/11] public / x86: introduce hvmctl hypercall

Message ID 576D276F02000078000F86DA@prv-mh.provo.novell.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jan Beulich June 24, 2016, 10:28 a.m. UTC
... as a means to replace all HVMOP_* which a domain can't issue on
itself (i.e. intended for use by only the control domain or device
model).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
v2: Widen cmd field to 32 bits and opaque one to 64. Drop
    HVMCTL_iter_*.
public / x86: introduce hvmctl hypercall

... as a means to replace all HVMOP_* which a domain can't issue on
itself (i.e. intended for use by only the control domain or device
model).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
v2: Widen cmd field to 32 bits and opaque one to 64. Drop
    HVMCTL_iter_*.

--- a/xen/arch/x86/hvm/Makefile
+++ b/xen/arch/x86/hvm/Makefile
@@ -2,6 +2,7 @@ subdir-y += svm
 subdir-y += vmx
 
 obj-y += asid.o
+obj-y += control.o
 obj-y += emulate.o
 obj-y += event.o
 obj-y += hpet.o
--- /dev/null
+++ b/xen/arch/x86/hvm/control.c
@@ -0,0 +1,82 @@
+/*
+ * control.c: Hardware virtual machine control operations.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <xen/hypercall.h>
+#include <xen/guest_access.h>
+#include <xen/sched.h>
+#include <xsm/xsm.h>
+
+long do_hvmctl(XEN_GUEST_HANDLE_PARAM(xen_hvmctl_t) u_hvmctl)
+{
+    xen_hvmctl_t op;
+    struct domain *d;
+    int rc;
+
+    BUILD_BUG_ON(sizeof(op.u) > sizeof(op.u.pad));
+
+    if ( copy_from_guest(&op, u_hvmctl, 1) )
+        return -EFAULT;
+
+    if ( op.interface_version != XEN_HVMCTL_INTERFACE_VERSION )
+        return -EACCES;
+
+    rc = rcu_lock_remote_domain_by_id(op.domain, &d);
+    if ( rc )
+        return rc;
+
+    if ( !has_hvm_container_domain(d) )
+    {
+        rcu_unlock_domain(d);
+        return -EINVAL;
+    }
+
+    rc = xsm_hvm_control(XSM_DM_PRIV, d, op.cmd);
+    if ( rc )
+    {
+        rcu_unlock_domain(d);
+        return rc;
+    }
+
+    switch ( op.cmd )
+    {
+    default:
+        rc = -EOPNOTSUPP;
+        break;
+    }
+
+    rcu_unlock_domain(d);
+
+    if ( rc == -ERESTART )
+    {
+        if ( unlikely(copy_field_to_guest(u_hvmctl, &op, opaque)) )
+            rc = -EFAULT;
+        else
+            rc = hypercall_create_continuation(__HYPERVISOR_hvmctl, "h",
+                                               u_hvmctl);
+    }
+
+    return rc;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4111,6 +4111,7 @@ static const struct {
     COMPAT_CALL(platform_op),
     COMPAT_CALL(mmuext_op),
     HYPERCALL(xenpmu_op),
+    HYPERCALL(hvmctl),
     HYPERCALL(arch_1)
 };
 
--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -469,6 +469,7 @@ ENTRY(compat_hypercall_table)
         .quad do_tmem_op
         .quad do_ni_hypercall           /* reserved for XenClient */
         .quad do_xenpmu_op              /* 40 */
+        .quad do_hvmctl
         .rept __HYPERVISOR_arch_0-((.-compat_hypercall_table)/8)
         .quad compat_ni_hypercall
         .endr
@@ -520,6 +521,7 @@ ENTRY(compat_hypercall_args_table)
         .byte 1 /* do_tmem_op               */
         .byte 0 /* reserved for XenClient   */
         .byte 2 /* do_xenpmu_op             */  /* 40 */
+        .byte 1 /* do_hvmctl                */
         .rept __HYPERVISOR_arch_0-(.-compat_hypercall_args_table)
         .byte 0 /* compat_ni_hypercall      */
         .endr
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -792,6 +792,7 @@ ENTRY(hypercall_table)
         .quad do_tmem_op
         .quad do_ni_hypercall       /* reserved for XenClient */
         .quad do_xenpmu_op          /* 40 */
+        .quad do_hvmctl
         .rept __HYPERVISOR_arch_0-((.-hypercall_table)/8)
         .quad do_ni_hypercall
         .endr
@@ -843,6 +844,7 @@ ENTRY(hypercall_args_table)
         .byte 1 /* do_tmem_op           */
         .byte 0 /* reserved for XenClient */
         .byte 2 /* do_xenpmu_op         */  /* 40 */
+        .byte 1 /* do_hvmctl            */
         .rept __HYPERVISOR_arch_0-(.-hypercall_args_table)
         .byte 0 /* do_ni_hypercall      */
         .endr
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -93,7 +93,7 @@ all: headers.chk headers++.chk
 
 PUBLIC_HEADERS := $(filter-out public/arch-% public/dom0_ops.h, $(wildcard public/*.h public/*/*.h) $(public-y))
 
-PUBLIC_ANSI_HEADERS := $(filter-out public/%ctl.h public/xsm/% public/%hvm/save.h, $(PUBLIC_HEADERS))
+PUBLIC_ANSI_HEADERS := $(filter-out public/%ctl.h public/hvm/control.h public/xsm/% public/%hvm/save.h,$(PUBLIC_HEADERS))
 
 headers.chk: $(PUBLIC_ANSI_HEADERS) Makefile
 	for i in $(filter %.h,$^); do \
--- /dev/null
+++ b/xen/include/public/hvm/control.h
@@ -0,0 +1,54 @@
+/*
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#ifndef __XEN_PUBLIC_HVM_CONTROL_H__
+#define __XEN_PUBLIC_HVM_CONTROL_H__
+
+#if !defined(__XEN__) && !defined(__XEN_TOOLS__)
+#error "HVM control operations are intended for use by control tools only"
+#endif
+
+#include "../xen.h"
+
+#define XEN_HVMCTL_INTERFACE_VERSION 0x00000001
+
+struct xen_hvmctl {
+    uint16_t interface_version;    /* XEN_HVMCTL_INTERFACE_VERSION */
+    domid_t domain;
+    uint32_t cmd;
+    uint64_t opaque;               /* Must be zero on initial invocation. */
+    union {
+        uint8_t pad[120];
+    } u;
+};
+typedef struct xen_hvmctl xen_hvmctl_t;
+DEFINE_XEN_GUEST_HANDLE(xen_hvmctl_t);
+
+#endif /* __XEN_PUBLIC_HVM_CONTROL_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--- a/xen/include/public/xen.h
+++ b/xen/include/public/xen.h
@@ -115,6 +115,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_ulong_t);
 #define __HYPERVISOR_tmem_op              38
 #define __HYPERVISOR_xc_reserved_op       39 /* reserved for XenClient */
 #define __HYPERVISOR_xenpmu_op            40
+#define __HYPERVISOR_hvmctl               41
 
 /* Architecture-specific hypercall definitions. */
 #define __HYPERVISOR_arch_0               48
--- a/xen/include/xen/hypercall.h
+++ b/xen/include/xen/hypercall.h
@@ -15,6 +15,7 @@
 #include <public/tmem.h>
 #include <public/version.h>
 #include <public/pmu.h>
+#include <public/hvm/control.h>
 #include <asm/hypercall.h>
 #include <xsm/xsm.h>
 
@@ -46,6 +47,10 @@ arch_do_sysctl(
     XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl);
 
 extern long
+do_hvmctl(
+    XEN_GUEST_HANDLE_PARAM(xen_hvmctl_t) u_hvmctl);
+
+extern long
 do_platform_op(
     XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op);
 
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1183,6 +1183,20 @@ static int flask_hvm_param(struct domain
     return current_has_perm(d, SECCLASS_HVM, perm);
 }
 
+static int flask_hvm_control(struct domain *d, unsigned long op)
+{
+    u32 perm;
+
+    switch ( op )
+    {
+    default:
+        perm = HVM__HVMCTL;
+        break;
+    }
+
+    return current_has_perm(d, SECCLASS_HVM, perm);
+}
+
 static int flask_hvm_param_nested(struct domain *d)
 {
     return current_has_perm(d, SECCLASS_HVM, HVM__NESTED);
@@ -1745,7 +1759,7 @@ static struct xsm_operations flask_ops =
     .page_offline = flask_page_offline,
     .tmem_op = flask_tmem_op,
     .hvm_param = flask_hvm_param,
-    .hvm_control = flask_hvm_param,
+    .hvm_control = flask_hvm_control,
     .hvm_param_nested = flask_hvm_param_nested,
     .hvm_param_altp2mhvm = flask_hvm_param_altp2mhvm,
     .hvm_altp2mhvm_op = flask_hvm_altp2mhvm_op,

Comments

Jan Beulich July 1, 2016, 4:18 p.m. UTC | #1
>>> On 24.06.16 at 12:28, <JBeulich@suse.com> wrote:
> ... as a means to replace all HVMOP_* which a domain can't issue on
> itself (i.e. intended for use by only the control domain or device
> model).
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Reviewed-by: Wei Liu <wei.liu2@citrix.com>

On the x86 side I'm just lacking feedback for this patch.

On the XSM side I so far got an ack only for patch 2.

Thanks, Jan
Andrew Cooper July 1, 2016, 4:42 p.m. UTC | #2
On 01/07/16 17:18, Jan Beulich wrote:
>>>> On 24.06.16 at 12:28, <JBeulich@suse.com> wrote:
>> ... as a means to replace all HVMOP_* which a domain can't issue on
>> itself (i.e. intended for use by only the control domain or device
>> model).
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> Reviewed-by: Wei Liu <wei.liu2@citrix.com>
> On the x86 side I'm just lacking feedback for this patch.

I have just spent the afternoon being bitten extremely hard by our
current unstable domctl abi, and in particular, the change of
DOMCTL_API_VERSION when nothing relevant has changed, and am leaning
towards David's views.

With the current definition, we have 32 bits of cmd space, proper
continuation logic via the opaque field, and 120 bytes of per-cmd space
in the union, which plenty.

How about making a proactive start to our ABI stabilisation effort,
dropping the interface_version entirely and declaring this stable?  We
would of course want to triple check the suitability of the existing
ops, but that can easily be rolled into this series (if any action is
needed).


Another area (which is related to the issue which bit me) is the
stability of operations like DOMCTL_pausedomain, which is unlikely to
ever change.

If we do chose to stabilise, we should design the new calls around how
they would be used.  We could do with a stable interface for "general
emulator routines", which applies equally to things like pause/unpause
and ioreq_server*, as opposed to most of the new hvmctl ops, which are
specific to qemu being an LPC bus emulator.

One thing I hadn't considered until now is that this takes an existing
stable ABI and replaces it with an unstable ABI, which is a step
backwards in terms of usability.  There are certainly other advantages
to moving the ops out of hvmop, but the instability is a detracting factor.

~Andrew
Jan Beulich July 4, 2016, 7:31 a.m. UTC | #3
>>> On 01.07.16 at 18:42, <andrew.cooper3@citrix.com> wrote:
> On 01/07/16 17:18, Jan Beulich wrote:
>>>>> On 24.06.16 at 12:28, <JBeulich@suse.com> wrote:
>>> ... as a means to replace all HVMOP_* which a domain can't issue on
>>> itself (i.e. intended for use by only the control domain or device
>>> model).
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> Reviewed-by: Wei Liu <wei.liu2@citrix.com>
>> On the x86 side I'm just lacking feedback for this patch.
> 
> I have just spent the afternoon being bitten extremely hard by our
> current unstable domctl abi, and in particular, the change of
> DOMCTL_API_VERSION when nothing relevant has changed, and am leaning
> towards David's views.
> 
> With the current definition, we have 32 bits of cmd space, proper
> continuation logic via the opaque field, and 120 bytes of per-cmd space
> in the union, which plenty.
> 
> How about making a proactive start to our ABI stabilisation effort,
> dropping the interface_version entirely and declaring this stable?  We
> would of course want to triple check the suitability of the existing
> ops, but that can easily be rolled into this series (if any action is
> needed).

I have to admit that I'm a little frustrated by this request: The series
has been out for quite some time, and was supposedly ready to go
in if all acks had been given. Yet with what you say above you
effectively would withdraw the ones you gave on the later patches
in the series, even if you don't say so explicitly. The fact that you've
got bitten by the domctl being modeled in similar ways (yet without
actually saying how you got bitten, or what's wrong with that model)
shouldn't really have much of an influence here. Even more so if, as
I would guess, that issue of yours was with the domctl wrapping
logic in your privcmd driver in Dom0 (which is already conceptually
problematic, as the kernel isn't intended to know of or make use of
domctl-s, and hence you having made it know set you up for such
problems).

Nor do I see myself do the auditing of the involved operations right
now (and basically as a prereq for this series to go in), the more that
I've learned from commits aa956976a9 ("domctl: perform initial
post-XSA-77 auditing") and then 5590bd17c4 ("XSA-77: widen scope
again") that it's very easy to overlook some aspect(s), no matter
how much time and effort one invests. I really think that for any
such future efforts we first need to put down a complete check list
of things that need to be ensured prior to making _any_ interface
stable and usable by other than fully trusted entities.

As to the option of marking this interface stable without doing the
security audit - I don't think I would see the point.

> Another area (which is related to the issue which bit me) is the
> stability of operations like DOMCTL_pausedomain, which is unlikely to
> ever change.
> 
> If we do chose to stabilise, we should design the new calls around how
> they would be used.  We could do with a stable interface for "general
> emulator routines", which applies equally to things like pause/unpause
> and ioreq_server*, as opposed to most of the new hvmctl ops, which are
> specific to qemu being an LPC bus emulator.
> 
> One thing I hadn't considered until now is that this takes an existing
> stable ABI and replaces it with an unstable ABI, which is a step
> backwards in terms of usability.  There are certainly other advantages
> to moving the ops out of hvmop, but the instability is a detracting factor.

This is not true imo: The existing interface wasn't stable (demonstrated
by it being framed by __XEN_ / __XEN_TOOLS__ conditionals), yet it
_also_ wasn't versioned, so its instablility wasn't represented properly.
So as said to David already, I continue to think this series is an
improvement, albeit other than in the direction that both of you would
like things to move. And while I'm with you for those intentions, I don't
think I should be required to make two steps at once here.

Plus you don't comment at all on my counter proposal to stabilize the
libxc wrappers around these hypercalls instead of the hypercalls
themselves.

Jan
Daniel De Graaf July 5, 2016, 5:54 p.m. UTC | #4
On 06/24/2016 06:28 AM, Jan Beulich wrote:
> ... as a means to replace all HVMOP_* which a domain can't issue on
> itself (i.e. intended for use by only the control domain or device
> model).
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Reviewed-by: Wei Liu <wei.liu2@citrix.com>

Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
George Dunlap Aug. 3, 2016, 11:06 a.m. UTC | #5
On 24/06/16 11:28, Jan Beulich wrote:
> ... as a means to replace all HVMOP_* which a domain can't issue on
> itself (i.e. intended for use by only the control domain or device
> model).
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Reviewed-by: Wei Liu <wei.liu2@citrix.com>

Jan,

Could you post this branch to a git repo somewhere?

I normally review patches "in situ" by importing them to a tree; and
saving each of your individual patches and applying them in order is a
bit much.

 -George

> ---
> v2: Widen cmd field to 32 bits and opaque one to 64. Drop
>     HVMCTL_iter_*.
> 
> --- a/xen/arch/x86/hvm/Makefile
> +++ b/xen/arch/x86/hvm/Makefile
> @@ -2,6 +2,7 @@ subdir-y += svm
>  subdir-y += vmx
>  
>  obj-y += asid.o
> +obj-y += control.o
>  obj-y += emulate.o
>  obj-y += event.o
>  obj-y += hpet.o
> --- /dev/null
> +++ b/xen/arch/x86/hvm/control.c
> @@ -0,0 +1,82 @@
> +/*
> + * control.c: Hardware virtual machine control operations.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program; If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <xen/hypercall.h>
> +#include <xen/guest_access.h>
> +#include <xen/sched.h>
> +#include <xsm/xsm.h>
> +
> +long do_hvmctl(XEN_GUEST_HANDLE_PARAM(xen_hvmctl_t) u_hvmctl)
> +{
> +    xen_hvmctl_t op;
> +    struct domain *d;
> +    int rc;
> +
> +    BUILD_BUG_ON(sizeof(op.u) > sizeof(op.u.pad));
> +
> +    if ( copy_from_guest(&op, u_hvmctl, 1) )
> +        return -EFAULT;
> +
> +    if ( op.interface_version != XEN_HVMCTL_INTERFACE_VERSION )
> +        return -EACCES;
> +
> +    rc = rcu_lock_remote_domain_by_id(op.domain, &d);
> +    if ( rc )
> +        return rc;
> +
> +    if ( !has_hvm_container_domain(d) )
> +    {
> +        rcu_unlock_domain(d);
> +        return -EINVAL;
> +    }
> +
> +    rc = xsm_hvm_control(XSM_DM_PRIV, d, op.cmd);
> +    if ( rc )
> +    {
> +        rcu_unlock_domain(d);
> +        return rc;
> +    }
> +
> +    switch ( op.cmd )
> +    {
> +    default:
> +        rc = -EOPNOTSUPP;
> +        break;
> +    }
> +
> +    rcu_unlock_domain(d);
> +
> +    if ( rc == -ERESTART )
> +    {
> +        if ( unlikely(copy_field_to_guest(u_hvmctl, &op, opaque)) )
> +            rc = -EFAULT;
> +        else
> +            rc = hypercall_create_continuation(__HYPERVISOR_hvmctl, "h",
> +                                               u_hvmctl);
> +    }
> +
> +    return rc;
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -4111,6 +4111,7 @@ static const struct {
>      COMPAT_CALL(platform_op),
>      COMPAT_CALL(mmuext_op),
>      HYPERCALL(xenpmu_op),
> +    HYPERCALL(hvmctl),
>      HYPERCALL(arch_1)
>  };
>  
> --- a/xen/arch/x86/x86_64/compat/entry.S
> +++ b/xen/arch/x86/x86_64/compat/entry.S
> @@ -469,6 +469,7 @@ ENTRY(compat_hypercall_table)
>          .quad do_tmem_op
>          .quad do_ni_hypercall           /* reserved for XenClient */
>          .quad do_xenpmu_op              /* 40 */
> +        .quad do_hvmctl
>          .rept __HYPERVISOR_arch_0-((.-compat_hypercall_table)/8)
>          .quad compat_ni_hypercall
>          .endr
> @@ -520,6 +521,7 @@ ENTRY(compat_hypercall_args_table)
>          .byte 1 /* do_tmem_op               */
>          .byte 0 /* reserved for XenClient   */
>          .byte 2 /* do_xenpmu_op             */  /* 40 */
> +        .byte 1 /* do_hvmctl                */
>          .rept __HYPERVISOR_arch_0-(.-compat_hypercall_args_table)
>          .byte 0 /* compat_ni_hypercall      */
>          .endr
> --- a/xen/arch/x86/x86_64/entry.S
> +++ b/xen/arch/x86/x86_64/entry.S
> @@ -792,6 +792,7 @@ ENTRY(hypercall_table)
>          .quad do_tmem_op
>          .quad do_ni_hypercall       /* reserved for XenClient */
>          .quad do_xenpmu_op          /* 40 */
> +        .quad do_hvmctl
>          .rept __HYPERVISOR_arch_0-((.-hypercall_table)/8)
>          .quad do_ni_hypercall
>          .endr
> @@ -843,6 +844,7 @@ ENTRY(hypercall_args_table)
>          .byte 1 /* do_tmem_op           */
>          .byte 0 /* reserved for XenClient */
>          .byte 2 /* do_xenpmu_op         */  /* 40 */
> +        .byte 1 /* do_hvmctl            */
>          .rept __HYPERVISOR_arch_0-(.-hypercall_args_table)
>          .byte 0 /* do_ni_hypercall      */
>          .endr
> --- a/xen/include/Makefile
> +++ b/xen/include/Makefile
> @@ -93,7 +93,7 @@ all: headers.chk headers++.chk
>  
>  PUBLIC_HEADERS := $(filter-out public/arch-% public/dom0_ops.h, $(wildcard public/*.h public/*/*.h) $(public-y))
>  
> -PUBLIC_ANSI_HEADERS := $(filter-out public/%ctl.h public/xsm/% public/%hvm/save.h, $(PUBLIC_HEADERS))
> +PUBLIC_ANSI_HEADERS := $(filter-out public/%ctl.h public/hvm/control.h public/xsm/% public/%hvm/save.h,$(PUBLIC_HEADERS))
>  
>  headers.chk: $(PUBLIC_ANSI_HEADERS) Makefile
>  	for i in $(filter %.h,$^); do \
> --- /dev/null
> +++ b/xen/include/public/hvm/control.h
> @@ -0,0 +1,54 @@
> +/*
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to
> + * deal in the Software without restriction, including without limitation the
> + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
> + * sell copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + */
> +
> +#ifndef __XEN_PUBLIC_HVM_CONTROL_H__
> +#define __XEN_PUBLIC_HVM_CONTROL_H__
> +
> +#if !defined(__XEN__) && !defined(__XEN_TOOLS__)
> +#error "HVM control operations are intended for use by control tools only"
> +#endif
> +
> +#include "../xen.h"
> +
> +#define XEN_HVMCTL_INTERFACE_VERSION 0x00000001
> +
> +struct xen_hvmctl {
> +    uint16_t interface_version;    /* XEN_HVMCTL_INTERFACE_VERSION */
> +    domid_t domain;
> +    uint32_t cmd;
> +    uint64_t opaque;               /* Must be zero on initial invocation. */
> +    union {
> +        uint8_t pad[120];
> +    } u;
> +};
> +typedef struct xen_hvmctl xen_hvmctl_t;
> +DEFINE_XEN_GUEST_HANDLE(xen_hvmctl_t);
> +
> +#endif /* __XEN_PUBLIC_HVM_CONTROL_H__ */
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> --- a/xen/include/public/xen.h
> +++ b/xen/include/public/xen.h
> @@ -115,6 +115,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_ulong_t);
>  #define __HYPERVISOR_tmem_op              38
>  #define __HYPERVISOR_xc_reserved_op       39 /* reserved for XenClient */
>  #define __HYPERVISOR_xenpmu_op            40
> +#define __HYPERVISOR_hvmctl               41
>  
>  /* Architecture-specific hypercall definitions. */
>  #define __HYPERVISOR_arch_0               48
> --- a/xen/include/xen/hypercall.h
> +++ b/xen/include/xen/hypercall.h
> @@ -15,6 +15,7 @@
>  #include <public/tmem.h>
>  #include <public/version.h>
>  #include <public/pmu.h>
> +#include <public/hvm/control.h>
>  #include <asm/hypercall.h>
>  #include <xsm/xsm.h>
>  
> @@ -46,6 +47,10 @@ arch_do_sysctl(
>      XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl);
>  
>  extern long
> +do_hvmctl(
> +    XEN_GUEST_HANDLE_PARAM(xen_hvmctl_t) u_hvmctl);
> +
> +extern long
>  do_platform_op(
>      XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op);
>  
> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -1183,6 +1183,20 @@ static int flask_hvm_param(struct domain
>      return current_has_perm(d, SECCLASS_HVM, perm);
>  }
>  
> +static int flask_hvm_control(struct domain *d, unsigned long op)
> +{
> +    u32 perm;
> +
> +    switch ( op )
> +    {
> +    default:
> +        perm = HVM__HVMCTL;
> +        break;
> +    }
> +
> +    return current_has_perm(d, SECCLASS_HVM, perm);
> +}
> +
>  static int flask_hvm_param_nested(struct domain *d)
>  {
>      return current_has_perm(d, SECCLASS_HVM, HVM__NESTED);
> @@ -1745,7 +1759,7 @@ static struct xsm_operations flask_ops =
>      .page_offline = flask_page_offline,
>      .tmem_op = flask_tmem_op,
>      .hvm_param = flask_hvm_param,
> -    .hvm_control = flask_hvm_param,
> +    .hvm_control = flask_hvm_control,
>      .hvm_param_nested = flask_hvm_param_nested,
>      .hvm_param_altp2mhvm = flask_hvm_param_altp2mhvm,
>      .hvm_altp2mhvm_op = flask_hvm_altp2mhvm_op,
> 
>
Jan Beulich Aug. 3, 2016, 11:52 a.m. UTC | #6
>>> On 03.08.16 at 13:06, <george.dunlap@citrix.com> wrote:
> On 24/06/16 11:28, Jan Beulich wrote:
>> ... as a means to replace all HVMOP_* which a domain can't issue on
>> itself (i.e. intended for use by only the control domain or device
>> model).
>> 
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> Reviewed-by: Wei Liu <wei.liu2@citrix.com>
> 
> Could you post this branch to a git repo somewhere?

Well, I can see to find time to get some git tree set up for such a
purpose, but I don't normally use git for my work, so I have
nothing available right away.

Jan
diff mbox

Patch

--- a/xen/arch/x86/hvm/Makefile
+++ b/xen/arch/x86/hvm/Makefile
@@ -2,6 +2,7 @@  subdir-y += svm
 subdir-y += vmx
 
 obj-y += asid.o
+obj-y += control.o
 obj-y += emulate.o
 obj-y += event.o
 obj-y += hpet.o
--- /dev/null
+++ b/xen/arch/x86/hvm/control.c
@@ -0,0 +1,82 @@ 
+/*
+ * control.c: Hardware virtual machine control operations.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <xen/hypercall.h>
+#include <xen/guest_access.h>
+#include <xen/sched.h>
+#include <xsm/xsm.h>
+
+long do_hvmctl(XEN_GUEST_HANDLE_PARAM(xen_hvmctl_t) u_hvmctl)
+{
+    xen_hvmctl_t op;
+    struct domain *d;
+    int rc;
+
+    BUILD_BUG_ON(sizeof(op.u) > sizeof(op.u.pad));
+
+    if ( copy_from_guest(&op, u_hvmctl, 1) )
+        return -EFAULT;
+
+    if ( op.interface_version != XEN_HVMCTL_INTERFACE_VERSION )
+        return -EACCES;
+
+    rc = rcu_lock_remote_domain_by_id(op.domain, &d);
+    if ( rc )
+        return rc;
+
+    if ( !has_hvm_container_domain(d) )
+    {
+        rcu_unlock_domain(d);
+        return -EINVAL;
+    }
+
+    rc = xsm_hvm_control(XSM_DM_PRIV, d, op.cmd);
+    if ( rc )
+    {
+        rcu_unlock_domain(d);
+        return rc;
+    }
+
+    switch ( op.cmd )
+    {
+    default:
+        rc = -EOPNOTSUPP;
+        break;
+    }
+
+    rcu_unlock_domain(d);
+
+    if ( rc == -ERESTART )
+    {
+        if ( unlikely(copy_field_to_guest(u_hvmctl, &op, opaque)) )
+            rc = -EFAULT;
+        else
+            rc = hypercall_create_continuation(__HYPERVISOR_hvmctl, "h",
+                                               u_hvmctl);
+    }
+
+    return rc;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4111,6 +4111,7 @@  static const struct {
     COMPAT_CALL(platform_op),
     COMPAT_CALL(mmuext_op),
     HYPERCALL(xenpmu_op),
+    HYPERCALL(hvmctl),
     HYPERCALL(arch_1)
 };
 
--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -469,6 +469,7 @@  ENTRY(compat_hypercall_table)
         .quad do_tmem_op
         .quad do_ni_hypercall           /* reserved for XenClient */
         .quad do_xenpmu_op              /* 40 */
+        .quad do_hvmctl
         .rept __HYPERVISOR_arch_0-((.-compat_hypercall_table)/8)
         .quad compat_ni_hypercall
         .endr
@@ -520,6 +521,7 @@  ENTRY(compat_hypercall_args_table)
         .byte 1 /* do_tmem_op               */
         .byte 0 /* reserved for XenClient   */
         .byte 2 /* do_xenpmu_op             */  /* 40 */
+        .byte 1 /* do_hvmctl                */
         .rept __HYPERVISOR_arch_0-(.-compat_hypercall_args_table)
         .byte 0 /* compat_ni_hypercall      */
         .endr
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -792,6 +792,7 @@  ENTRY(hypercall_table)
         .quad do_tmem_op
         .quad do_ni_hypercall       /* reserved for XenClient */
         .quad do_xenpmu_op          /* 40 */
+        .quad do_hvmctl
         .rept __HYPERVISOR_arch_0-((.-hypercall_table)/8)
         .quad do_ni_hypercall
         .endr
@@ -843,6 +844,7 @@  ENTRY(hypercall_args_table)
         .byte 1 /* do_tmem_op           */
         .byte 0 /* reserved for XenClient */
         .byte 2 /* do_xenpmu_op         */  /* 40 */
+        .byte 1 /* do_hvmctl            */
         .rept __HYPERVISOR_arch_0-(.-hypercall_args_table)
         .byte 0 /* do_ni_hypercall      */
         .endr
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -93,7 +93,7 @@  all: headers.chk headers++.chk
 
 PUBLIC_HEADERS := $(filter-out public/arch-% public/dom0_ops.h, $(wildcard public/*.h public/*/*.h) $(public-y))
 
-PUBLIC_ANSI_HEADERS := $(filter-out public/%ctl.h public/xsm/% public/%hvm/save.h, $(PUBLIC_HEADERS))
+PUBLIC_ANSI_HEADERS := $(filter-out public/%ctl.h public/hvm/control.h public/xsm/% public/%hvm/save.h,$(PUBLIC_HEADERS))
 
 headers.chk: $(PUBLIC_ANSI_HEADERS) Makefile
 	for i in $(filter %.h,$^); do \
--- /dev/null
+++ b/xen/include/public/hvm/control.h
@@ -0,0 +1,54 @@ 
+/*
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#ifndef __XEN_PUBLIC_HVM_CONTROL_H__
+#define __XEN_PUBLIC_HVM_CONTROL_H__
+
+#if !defined(__XEN__) && !defined(__XEN_TOOLS__)
+#error "HVM control operations are intended for use by control tools only"
+#endif
+
+#include "../xen.h"
+
+#define XEN_HVMCTL_INTERFACE_VERSION 0x00000001
+
+struct xen_hvmctl {
+    uint16_t interface_version;    /* XEN_HVMCTL_INTERFACE_VERSION */
+    domid_t domain;
+    uint32_t cmd;
+    uint64_t opaque;               /* Must be zero on initial invocation. */
+    union {
+        uint8_t pad[120];
+    } u;
+};
+typedef struct xen_hvmctl xen_hvmctl_t;
+DEFINE_XEN_GUEST_HANDLE(xen_hvmctl_t);
+
+#endif /* __XEN_PUBLIC_HVM_CONTROL_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--- a/xen/include/public/xen.h
+++ b/xen/include/public/xen.h
@@ -115,6 +115,7 @@  DEFINE_XEN_GUEST_HANDLE(xen_ulong_t);
 #define __HYPERVISOR_tmem_op              38
 #define __HYPERVISOR_xc_reserved_op       39 /* reserved for XenClient */
 #define __HYPERVISOR_xenpmu_op            40
+#define __HYPERVISOR_hvmctl               41
 
 /* Architecture-specific hypercall definitions. */
 #define __HYPERVISOR_arch_0               48
--- a/xen/include/xen/hypercall.h
+++ b/xen/include/xen/hypercall.h
@@ -15,6 +15,7 @@ 
 #include <public/tmem.h>
 #include <public/version.h>
 #include <public/pmu.h>
+#include <public/hvm/control.h>
 #include <asm/hypercall.h>
 #include <xsm/xsm.h>
 
@@ -46,6 +47,10 @@  arch_do_sysctl(
     XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl);
 
 extern long
+do_hvmctl(
+    XEN_GUEST_HANDLE_PARAM(xen_hvmctl_t) u_hvmctl);
+
+extern long
 do_platform_op(
     XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op);
 
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1183,6 +1183,20 @@  static int flask_hvm_param(struct domain
     return current_has_perm(d, SECCLASS_HVM, perm);
 }
 
+static int flask_hvm_control(struct domain *d, unsigned long op)
+{
+    u32 perm;
+
+    switch ( op )
+    {
+    default:
+        perm = HVM__HVMCTL;
+        break;
+    }
+
+    return current_has_perm(d, SECCLASS_HVM, perm);
+}
+
 static int flask_hvm_param_nested(struct domain *d)
 {
     return current_has_perm(d, SECCLASS_HVM, HVM__NESTED);
@@ -1745,7 +1759,7 @@  static struct xsm_operations flask_ops =
     .page_offline = flask_page_offline,
     .tmem_op = flask_tmem_op,
     .hvm_param = flask_hvm_param,
-    .hvm_control = flask_hvm_param,
+    .hvm_control = flask_hvm_control,
     .hvm_param_nested = flask_hvm_param_nested,
     .hvm_param_altp2mhvm = flask_hvm_param_altp2mhvm,
     .hvm_altp2mhvm_op = flask_hvm_altp2mhvm_op,