[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v3] vm_event: Implement ARM SMC events
Hi Tamas,
On 15/09/2016 20:24, Tamas K Lengyel wrote:
The ARM SMC instructions are already configured to trap to Xen by default. In
this patch we allow a user-space process in a privileged domain to receive
notification of when such event happens through the vm_event subsystem by
introducing the PRIVILEGED_CALL type.
The intended use-case for this feature is for a monitor application to be able
insert tap-points into the domU kernel-code. For this task only unconditional
SMC instruction should be used.
Signed-off-by: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
Acked-by: Wei Liu <wei.liu2@xxxxxxxxxx>
---
Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Cc: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
Cc: Julien Grall <julien.grall@xxxxxxx>
v3: Rebase on latest master
Note: previous discussion around this patch proposed filtering SMCs with failed
condition checks. As that patch is yet-to-be implemented and the 4.8
code-freeze rapidly approaching I would like this patch to get included.
It is still in my todo list. But as I said last time, it is towards the
bottom of it. I would have expected a bit of help from your side here
rather than putting the fault on me.
In this patch a proper warning is placed in the public header for
potential users not to rely on SMCs with failed condition checks being
trapped. As the intended use-case for this feature doesn't use
conditional SMCs this warning should be sufficient. Hardware that does
issue events for SMCs with failed condition checks doesn't pose a problem
for a monitor application in any way. The xen-access test tool illustrates
how SMCs issued by the guest can be safely handled for all cases.
This is nice, but how about passing the immediate to the event monitor?
The SMC call is not meant to be a breakpoint instruction but a way to
call the supervisor monitor (and potentially be emulated by the hypervisor).
---
tools/libxc/include/xenctrl.h | 2 +
tools/libxc/xc_monitor.c | 14 +++++++
tools/tests/xen-access/xen-access.c | 32 ++++++++++++++-
xen/arch/arm/Makefile | 1 +
xen/arch/arm/monitor.c | 79 +++++++++++++++++++++++++++++++++++++
xen/arch/arm/traps.c | 15 ++++++-
xen/include/asm-arm/domain.h | 5 +++
xen/include/asm-arm/monitor.h | 18 +++------
xen/include/public/domctl.h | 1 +
xen/include/public/vm_event.h | 7 ++++
10 files changed, 158 insertions(+), 16 deletions(-)
create mode 100644 xen/arch/arm/monitor.c
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 560ce7b..eb53172 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2168,6 +2168,8 @@ int xc_monitor_guest_request(xc_interface *xch, domid_t
domain_id,
int xc_monitor_debug_exceptions(xc_interface *xch, domid_t domain_id,
bool enable, bool sync);
int xc_monitor_cpuid(xc_interface *xch, domid_t domain_id, bool enable);
+int xc_monitor_privileged_call(xc_interface *xch, domid_t domain_id,
+ bool enable);
/**
* This function enables / disables emulation for each REP for a
* REP-compatible instruction.
diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
index 4298813..15a7c32 100644
--- a/tools/libxc/xc_monitor.c
+++ b/tools/libxc/xc_monitor.c
@@ -185,6 +185,20 @@ int xc_monitor_cpuid(xc_interface *xch, domid_t domain_id,
bool enable)
return do_domctl(xch, &domctl);
}
+int xc_monitor_privileged_call(xc_interface *xch, domid_t domain_id,
+ bool enable)
+{
+ DECLARE_DOMCTL;
+
+ domctl.cmd = XEN_DOMCTL_monitor_op;
+ domctl.domain = domain_id;
+ domctl.u.monitor_op.op = enable ? XEN_DOMCTL_MONITOR_OP_ENABLE
+ : XEN_DOMCTL_MONITOR_OP_DISABLE;
+ domctl.u.monitor_op.event = XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL;
+
+ return do_domctl(xch, &domctl);
+}
+
/*
* Local variables:
* mode: C
diff --git a/tools/tests/xen-access/xen-access.c
b/tools/tests/xen-access/xen-access.c
index ed18c71..6eefe0c 100644
--- a/tools/tests/xen-access/xen-access.c
+++ b/tools/tests/xen-access/xen-access.c
@@ -338,6 +338,8 @@ void usage(char* progname)
fprintf(stderr, "Usage: %s [-m] <domain_id> write|exec", progname);
#if defined(__i386__) || defined(__x86_64__)
fprintf(stderr,
"|breakpoint|altp2m_write|altp2m_exec|debug|cpuid");
+#elif defined(__arm__) || defined(__aarch64__)
+ fprintf(stderr, "|privcall");
#endif
fprintf(stderr,
"\n"
@@ -362,6 +364,7 @@ int main(int argc, char *argv[])
int required = 0;
int breakpoint = 0;
int shutting_down = 0;
+ int privcall = 0;
int altp2m = 0;
int debug = 0;
int cpuid = 0;
@@ -431,6 +434,11 @@ int main(int argc, char *argv[])
{
cpuid = 1;
}
+#elif defined(__arm__) || defined(__aarch64__)
+ else if ( !strcmp(argv[0], "privcall") )
+ {
+ privcall = 1;
+ }
#endif
else
{
@@ -563,6 +571,16 @@ int main(int argc, char *argv[])
}
}
+ if ( privcall )
+ {
+ rc = xc_monitor_privileged_call(xch, domain_id, 1);
+ if ( rc < 0 )
+ {
+ ERROR("Error %d setting privileged call trapping with vm_event\n",
rc);
+ goto exit;
+ }
+ }
+
/* Wait for access */
for (;;)
{
@@ -578,6 +596,9 @@ int main(int argc, char *argv[])
if ( cpuid )
rc = xc_monitor_cpuid(xch, domain_id, 0);
+ if ( privcall )
+ rc = xc_monitor_privileged_call(xch, domain_id, 0);
+
if ( altp2m )
{
rc = xc_altp2m_switch_to_view( xch, domain_id, 0 );
@@ -678,7 +699,7 @@ int main(int argc, char *argv[])
rsp.u.mem_access = req.u.mem_access;
break;
case VM_EVENT_REASON_SOFTWARE_BREAKPOINT:
- printf("Breakpoint: rip=%016"PRIx64", gfn=%"PRIx64" (vcpu
%d)\n",
+ printf("Breakpoint: rip=%"PRIx64" gfn=%"PRIx64" (vcpu %d)\n",
This change looks spurious.
req.data.regs.x86.rip,
req.u.software_breakpoint.gfn,
req.vcpu_id);
@@ -695,6 +716,15 @@ int main(int argc, char *argv[])
continue;
}
break;
+ case VM_EVENT_REASON_PRIVILEGED_CALL:
+ printf("Privileged call: pc=%"PRIx64" (vcpu %d)\n",
+ req.data.regs.arm.pc,
+ req.vcpu_id);
+
+ rsp.data.regs.arm = req.data.regs.arm;
+ rsp.data.regs.arm.pc += 4;
+ rsp.flags |= VM_EVENT_FLAG_SET_REGISTERS;
+ break;
case VM_EVENT_REASON_SINGLESTEP:
printf("Singlestep: rip=%016"PRIx64", vcpu %d, altp2m %u\n",
req.data.regs.x86.rip,
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 64fdf41..b140d7e 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -25,6 +25,7 @@ obj-y += irq.o
obj-y += kernel.o
obj-$(CONFIG_LIVEPATCH) += livepatch.o
obj-y += mm.o
+obj-y += monitor.o
obj-y += p2m.o
obj-y += percpu.o
obj-y += platform.o
diff --git a/xen/arch/arm/monitor.c b/xen/arch/arm/monitor.c
new file mode 100644
index 0000000..db05453
--- /dev/null
+++ b/xen/arch/arm/monitor.c
@@ -0,0 +1,79 @@
+/*
+ * arch/x86/monitor.c
This should be arch/arm/monitor.c
+ *
+ * Arch-specific monitor_op domctl handler.
+ *
+ * Copyright (c) 2016 Tamas K Lengyel (tamas.lengyel@xxxxxxxxxxxx)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that 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/vm_event.h>
+#include <xen/monitor.h>
+#include <asm/monitor.h>
+#include <asm/vm_event.h>
+#include <public/vm_event.h>
+
+int arch_monitor_domctl_event(struct domain *d,
+ struct xen_domctl_monitor_op *mop)
+{
+ struct arch_domain *ad = &d->arch;
+ bool_t requested_status = (XEN_DOMCTL_MONITOR_OP_ENABLE == mop->op);
+
+ switch ( mop->event )
+ {
+ case XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL:
+ {
+ bool_t old_status = ad->monitor.privileged_call_enabled;
+
+ if ( unlikely(old_status == requested_status) )
+ return -EEXIST;
+
+ domain_pause(d);
+ ad->monitor.privileged_call_enabled = requested_status;
+ domain_unpause(d);
+ break;
+ }
+
+ default:
+ /*
+ * Should not be reached unless arch_monitor_get_capabilities() is
+ * not properly implemented.
+ */
+ ASSERT_UNREACHABLE();
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+}
+
+int monitor_smc(void) {
Coding style.
+ struct vcpu *curr = current;
+ vm_event_request_t req = {
+ .reason = VM_EVENT_REASON_PRIVILEGED_CALL
+ };
+
+ if ( !curr->domain->arch.monitor.privileged_call_enabled )
+ return 0;
+
+ return monitor_traps(curr, 1, &req);
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 39a05fd..cf58fd5 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -41,6 +41,7 @@
#include <asm/mmio.h>
#include <asm/cpufeature.h>
#include <asm/flushtlb.h>
+#include <asm/monitor.h>
#include "decode.h"
#include "vtimer.h"
@@ -2527,6 +2528,16 @@ bad_data_abort:
inject_dabt_exception(regs, info.gva, hsr.len);
}
+static void do_trap_smc(struct cpu_user_regs *regs, const union hsr hsr)
+{
+ int rc = 0;
Missing blank line.
+ if ( current->domain->arch.monitor.privileged_call_enabled )
+ rc = monitor_smc();
+
+ if ( rc != 1 )
+ inject_undef_exception(regs, hsr);
It would be worth mentioning somewhere that you expect the monitor app
skipping the instruction.
+}
+
static void enter_hypervisor_head(struct cpu_user_regs *regs)
{
if ( guest_mode(regs) )
@@ -2602,7 +2613,7 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs
*regs)
*/
GUEST_BUG_ON(!psr_mode_is_32bit(regs->cpsr));
perfc_incr(trap_smc32);
- inject_undef32_exception(regs);
+ do_trap_smc(regs, hsr);
break;
case HSR_EC_HVC32:
GUEST_BUG_ON(!psr_mode_is_32bit(regs->cpsr));
@@ -2635,7 +2646,7 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs
*regs)
*/
GUEST_BUG_ON(psr_mode_is_32bit(regs->cpsr));
perfc_incr(trap_smc64);
- inject_undef64_exception(regs, hsr.len);
+ do_trap_smc(regs, hsr);
break;
case HSR_EC_SYSREG:
GUEST_BUG_ON(psr_mode_is_32bit(regs->cpsr));
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 9452fcd..2d6fbb1 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -126,6 +126,11 @@ struct arch_domain
paddr_t efi_acpi_gpa;
paddr_t efi_acpi_len;
#endif
+
+ /* Monitor options */
+ struct {
+ uint8_t privileged_call_enabled : 1;
+ } monitor;
} __cacheline_aligned;
struct arch_vcpu
diff --git a/xen/include/asm-arm/monitor.h b/xen/include/asm-arm/monitor.h
index 4af707a..1c4fea3 100644
--- a/xen/include/asm-arm/monitor.h
+++ b/xen/include/asm-arm/monitor.h
@@ -32,19 +32,8 @@ int arch_monitor_domctl_op(struct domain *d, struct
xen_domctl_monitor_op *mop)
return -EOPNOTSUPP;
}
-static inline
int arch_monitor_domctl_event(struct domain *d,
- struct xen_domctl_monitor_op *mop)
-{
- /*
- * No arch-specific monitor vm-events on ARM.
- *
- * Should not be reached unless arch_monitor_get_capabilities() is not
- * properly implemented.
- */
- ASSERT_UNREACHABLE();
- return -EOPNOTSUPP;
-}
+ struct xen_domctl_monitor_op *mop);
static inline
int arch_monitor_init_domain(struct domain *d)
@@ -63,9 +52,12 @@ static inline uint32_t arch_monitor_get_capabilities(struct
domain *d)
{
uint32_t capabilities = 0;
- capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
+ capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST |
+ 1U << XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL);
return capabilities;
}
+int monitor_smc(void);
+
#endif /* __ASM_ARM_MONITOR_H__ */
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index ddd3de4..177319d 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1085,6 +1085,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cmt_op_t);
#define XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST 4
#define XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION 5
#define XEN_DOMCTL_MONITOR_EVENT_CPUID 6
+#define XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL 7
struct xen_domctl_monitor_op {
uint32_t op; /* XEN_DOMCTL_MONITOR_OP_* */
diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h
index f756126..dc48e4c 100644
--- a/xen/include/public/vm_event.h
+++ b/xen/include/public/vm_event.h
@@ -124,6 +124,13 @@
#define VM_EVENT_REASON_DEBUG_EXCEPTION 9
/* CPUID executed */
#define VM_EVENT_REASON_CPUID 10
+/*
+ * Privileged call executed (e.g. SMC).
+ * Note: event may be generated even if SMC condition check fails on some CPUs.
+ * As this behavior is CPU-specific users are advised to not rely on it.
Missing comma after CPU-specific.
+ * Such events are planned to be forcibly filtered out in future
versions.
+ */
+#define VM_EVENT_REASON_PRIVILEGED_CALL 11
/* Supported values for the vm_event_write_ctrlreg index. */
#define VM_EVENT_X86_CR0 0
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
