|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 2/5] vm_event: Implement ARM SMC events
On 04/29/16 21:07, 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.
>
> Signed-off-by: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
> ---
> Cc: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> Cc: Julien Grall <julien.grall@xxxxxxx>
> Cc: Keir Fraser <keir@xxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>
> v2: introduce VM_EVENT_REASON_PRIVELEGED_CALL
> aarch64 support
> ---
> MAINTAINERS | 6 +-
> tools/libxc/include/xenctrl.h | 2 +
> tools/libxc/xc_monitor.c | 26 +++++++-
> tools/tests/xen-access/xen-access.c | 31 ++++++++-
> xen/arch/arm/Makefile | 2 +
> xen/arch/arm/monitor.c | 80 +++++++++++++++++++++++
> xen/arch/arm/traps.c | 20 +++++-
> xen/arch/arm/vm_event.c | 127
> ++++++++++++++++++++++++++++++++++++
> xen/arch/x86/hvm/event.c | 2 +
> xen/common/vm_event.c | 1 -
> xen/include/asm-arm/domain.h | 5 ++
> xen/include/asm-arm/monitor.h | 20 ++----
> xen/include/asm-arm/vm_event.h | 16 ++---
> xen/include/public/domctl.h | 1 +
> xen/include/public/vm_event.h | 27 ++++++++
> 15 files changed, 333 insertions(+), 33 deletions(-)
> create mode 100644 xen/arch/arm/monitor.c
> create mode 100644 xen/arch/arm/vm_event.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5af7a0c..36d8591 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -355,12 +355,10 @@ VM EVENT AND MEM ACCESS
> M: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
> M: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
> S: Supported
> -F: xen/common/vm_event.c
> +F: xen/*/vm_event.c
> +F: xen/*/monitor.c
> F: xen/common/mem_access.c
> -F: xen/common/monitor.c
> F: xen/arch/x86/hvm/event.c
> -F: xen/arch/x86/monitor.c
> -F: xen/arch/*/vm_event.c
> F: tools/tests/xen-access
>
> VTPM
This patch touches MAINTANERS, but so does the last patch in the series
(which does nothing else but touch MAINTAINERS). I wouldn't block this
patch on this account, but would it be problematic for all changes to
MAINTAINERS to occur in that patch?
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index 42f201b..4b75ae4 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -2160,6 +2160,8 @@ int xc_monitor_software_breakpoint(xc_interface *xch,
> domid_t domain_id,
> bool enable);
> int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id,
> bool enable, bool sync);
> +int xc_monitor_privileged_call(xc_interface *xch, domid_t domain_id,
> + bool enable);
>
> /**
> * This function enables / disables emulation for each REP for a
> diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
> index b1705dd..072df70 100644
> --- a/tools/libxc/xc_monitor.c
> +++ b/tools/libxc/xc_monitor.c
> @@ -4,7 +4,7 @@
> *
> * Interface to VM event monitor
> *
> - * Copyright (c) 2015 Tamas K Lengyel (tamas@xxxxxxxxxxxxx)
> + * Copyright (c) 2015-2016 Tamas K Lengyel (tamas@xxxxxxxxxxxxx)
> *
> * This library is free software; you can redistribute it and/or
> * modify it under the terms of the GNU Lesser General Public
> @@ -156,3 +156,27 @@ int xc_monitor_emulate_each_rep(xc_interface *xch,
> domid_t domain_id,
>
> 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
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/tools/tests/xen-access/xen-access.c
> b/tools/tests/xen-access/xen-access.c
> index f26e723..33e8044 100644
> --- a/tools/tests/xen-access/xen-access.c
> +++ b/tools/tests/xen-access/xen-access.c
> @@ -334,6 +334,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");
> +#elif defined(__arm__) || defined(__aarch64__)
> + fprintf(stderr, "|privcall");
> #endif
> fprintf(stderr,
> "\n"
> @@ -357,6 +359,7 @@ int main(int argc, char *argv[])
> int required = 0;
> int breakpoint = 0;
> int shutting_down = 0;
> + int privcall = 0;
> int altp2m = 0;
> uint16_t altp2m_view_id = 0;
>
> @@ -412,6 +415,11 @@ int main(int argc, char *argv[])
> default_access = XENMEM_access_rw;
> altp2m = 1;
> }
> +#elif defined(__arm__) || defined(__aarch64__)
> + else if ( !strcmp(argv[0], "privcall") )
> + {
> + privcall = 1;
> + }
> #endif
> else
> {
> @@ -524,6 +532,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 (;;)
> {
> @@ -535,6 +553,9 @@ int main(int argc, char *argv[])
> if ( breakpoint )
> rc = xc_monitor_software_breakpoint(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 );
> @@ -635,7 +656,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",
> req.data.regs.x86.rip,
> req.u.software_breakpoint.gfn,
> req.vcpu_id);
> @@ -650,7 +671,15 @@ int main(int argc, char *argv[])
> interrupted = -1;
> 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",
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 0328b50..118be99 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -40,6 +40,8 @@ obj-y += device.o
> obj-y += decode.o
> obj-y += processor.o
> obj-y += smc.o
> +obj-y += monitor.o
> +obj-y += vm_event.o
>
> #obj-bin-y += ....o
>
> diff --git a/xen/arch/arm/monitor.c b/xen/arch/arm/monitor.c
> new file mode 100644
> index 0000000..e845f28
> --- /dev/null
> +++ b/xen/arch/arm/monitor.c
> @@ -0,0 +1,80 @@
> +/*
> + * arch/arm/monitor.c
> + *
> + * Arch-specific monitor_op domctl handler.
> + *
> + * Copyright (c) 2015-2016 Tamas K Lengyel (tamas@xxxxxxxxxxxxx)
> + * Copyright (c) 2016, Bitdefender S.R.L.
> + *
> + * 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 <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(const struct cpu_user_regs *regs) {
> + struct vcpu *curr = current;
> + vm_event_request_t req = { 0 };
> +
> + if ( !curr->domain->arch.monitor.privileged_call_enabled )
> + return 0;
> +
> + req.reason = VM_EVENT_REASON_PRIVILEGED_CALL;
> + req.vcpu_id = curr->vcpu_id;
> +
> + vm_event_fill_regs(&req, regs, curr->domain);
> +
> + return vm_event_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 9abfc3c..9c8d395 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"
> @@ -2491,6 +2492,21 @@ 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;
> + if ( current->domain->arch.monitor.privileged_call_enabled )
> + {
> + rc = monitor_smc(regs);
> + }
If you need to increment the patch version, maybe remove the curly
braces here?
> +
> + if ( rc != 1 )
> + {
> + GUEST_BUG_ON(!psr_mode_is_32bit(regs->cpsr));
> + inject_undef_exception(regs, hsr);
> + }
> +}
> +
> static void enter_hypervisor_head(struct cpu_user_regs *regs)
> {
> if ( guest_mode(regs) )
> @@ -2566,7 +2582,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));
> @@ -2599,7 +2615,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/arch/arm/vm_event.c b/xen/arch/arm/vm_event.c
> new file mode 100644
> index 0000000..3369a96
> --- /dev/null
> +++ b/xen/arch/arm/vm_event.c
> @@ -0,0 +1,127 @@
> +/*
> + * arch/arm/vm_event.c
> + *
> + * Architecture-specific vm_event handling routines
> + *
> + * Copyright (c) 2016 Tamas K Lengyel (tamas@xxxxxxxxxxxxx)
> + *
> + * 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/sched.h>
> +#include <asm/vm_event.h>
> +
> +void vm_event_fill_regs(vm_event_request_t *req,
> + const struct cpu_user_regs *regs,
> + struct domain *d)
> +{
> + if ( is_32bit_domain(d) )
> + {
> + req->data.regs.arm.x0 = regs->r0;
> + req->data.regs.arm.x1 = regs->r1;
> + req->data.regs.arm.x2 = regs->r2;
> + req->data.regs.arm.x3 = regs->r3;
> + req->data.regs.arm.x4 = regs->r4;
> + req->data.regs.arm.x5 = regs->r5;
> + req->data.regs.arm.x6 = regs->r6;
> + req->data.regs.arm.x7 = regs->r7;
> + req->data.regs.arm.x8 = regs->r8;
> + req->data.regs.arm.x9 = regs->r9;
> + req->data.regs.arm.x10 = regs->r10;
> + req->data.regs.arm.pc = regs->pc32;
> + req->data.regs.arm.sp_el0 = regs->sp_usr;
> + req->data.regs.arm.sp_el1 = regs->sp_svc;
> + }
> +#ifdef CONFIG_ARM_64
> + else
> + {
> + req->data.regs.arm.x0 = regs->x0;
> + req->data.regs.arm.x1 = regs->x1;
> + req->data.regs.arm.x2 = regs->x2;
> + req->data.regs.arm.x3 = regs->x3;
> + req->data.regs.arm.x4 = regs->x4;
> + req->data.regs.arm.x5 = regs->x5;
> + req->data.regs.arm.x6 = regs->x6;
> + req->data.regs.arm.x7 = regs->x7;
> + req->data.regs.arm.x8 = regs->x8;
> + req->data.regs.arm.x9 = regs->x9;
> + req->data.regs.arm.x10 = regs->x10;
> + req->data.regs.arm.pc = regs->pc;
> + req->data.regs.arm.sp_el0 = regs->sp_el0;
> + req->data.regs.arm.sp_el1 = regs->sp_el1;
> + }
> +#endif
> + req->data.regs.arm.fp = regs->fp;
> + req->data.regs.arm.lr = regs->lr;
> + req->data.regs.arm.cpsr = regs->cpsr;
> + req->data.regs.arm.spsr_el1 = regs->spsr_svc;
> + req->data.regs.arm.ttbr0 = READ_SYSREG64(TTBR0_EL1);
> + req->data.regs.arm.ttbr1 = READ_SYSREG64(TTBR1_EL1);
> +}
> +
> +void vm_event_set_registers(struct vcpu *v, vm_event_response_t *rsp)
> +{
> + struct cpu_user_regs *regs = &v->arch.cpu_info->guest_cpu_user_regs;
> +
> + if ( is_32bit_domain(v->domain) )
> + {
> + regs->r0 = rsp->data.regs.arm.x0;
> + regs->r1 = rsp->data.regs.arm.x1;
> + regs->r2 = rsp->data.regs.arm.x2;
> + regs->r3 = rsp->data.regs.arm.x3;
> + regs->r4 = rsp->data.regs.arm.x4;
> + regs->r5 = rsp->data.regs.arm.x5;
> + regs->r6 = rsp->data.regs.arm.x6;
> + regs->r7 = rsp->data.regs.arm.x7;
> + regs->r8 = rsp->data.regs.arm.x8;
> + regs->r9 = rsp->data.regs.arm.x9;
> + regs->r10 = rsp->data.regs.arm.x10;
> + regs->pc32 = rsp->data.regs.arm.pc;
> + regs->sp_usr = rsp->data.regs.arm.sp_el0;
> + regs->sp_svc = rsp->data.regs.arm.sp_el1;
> + }
> +#ifdef CONFIG_ARM_64
> + else
> + {
> + regs->x0 = rsp->data.regs.arm.x0;
> + regs->x1 = rsp->data.regs.arm.x1;
> + regs->x2 = rsp->data.regs.arm.x2;
> + regs->x3 = rsp->data.regs.arm.x3;
> + regs->x4 = rsp->data.regs.arm.x4;
> + regs->x5 = rsp->data.regs.arm.x5;
> + regs->x6 = rsp->data.regs.arm.x6;
> + regs->x7 = rsp->data.regs.arm.x7;
> + regs->x8 = rsp->data.regs.arm.x8;
> + regs->x9 = rsp->data.regs.arm.x9;
> + regs->x10 = rsp->data.regs.arm.x10;
> + regs->pc = rsp->data.regs.arm.pc;
> + regs->sp_el0 = rsp->data.regs.arm.sp_el0;
> + regs->sp_el1 = rsp->data.regs.arm.sp_el1;
> + }
> +#endif
> +
> + regs->fp = rsp->data.regs.arm.fp;
> + regs->lr = rsp->data.regs.arm.lr;
> + regs->cpsr = rsp->data.regs.arm.cpsr;
> + v->arch.ttbr0 = rsp->data.regs.arm.ttbr0;
> + v->arch.ttbr1 = rsp->data.regs.arm.ttbr1;
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/x86/hvm/event.c b/xen/arch/x86/hvm/event.c
> index 56c5514..f7d1418 100644
> --- a/xen/arch/x86/hvm/event.c
> +++ b/xen/arch/x86/hvm/event.c
> @@ -47,6 +47,7 @@ bool_t hvm_event_cr(unsigned int index, unsigned long
> value, unsigned long old)
> .u.write_ctrlreg.old_value = old
> };
>
> + vm_event_fill_regs(&req);
> vm_event_monitor_traps(curr, sync, &req);
> return 1;
> }
> @@ -115,6 +116,7 @@ int hvm_event_breakpoint(unsigned long rip,
> }
>
> req.vcpu_id = curr->vcpu_id;
> + vm_event_fill_regs(&req);
>
> return vm_event_monitor_traps(curr, 1, &req);
> }
> diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
> index 2906407..a29bda8 100644
> --- a/xen/common/vm_event.c
> +++ b/xen/common/vm_event.c
> @@ -818,7 +818,6 @@ int vm_event_monitor_traps(struct vcpu *v, uint8_t sync,
> req->altp2m_idx = altp2m_vcpu_idx(v);
> }
>
> - vm_event_fill_regs(req);
> vm_event_put_request(d, &d->vm_event->monitor, req);
>
> return 1;
So now for x86 we only vm_fill_regs() for CR writes and breakpoints (and
EPT faults, but that's in p2m.c which hasn't been touched by this
patch)? That's a pretty big change, and one that's not explained in the
patch description (which makes no mention of any x86 changes).
Having that call in vm_event_monitor_traps() made sure that all
vm_events get a copy of the respective registers. In the x86 case, that
includes the guest request and MSR write events, which now no longer
seem to carry that information, unless I'm missing something.
That behaviour should not change for x86 events, please.
Thanks,
Razvan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |