diff mbox

[v3] vm_event: Implement ARM SMC events

Message ID 20160915182441.21356-1-tamas.lengyel@zentific.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tamas Lengyel Sept. 15, 2016, 6:24 p.m. UTC
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@tklengyel.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>

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.
      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.
---
 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

Comments

Razvan Cojocaru Sept. 16, 2016, 7:23 a.m. UTC | #1
On 09/15/16 21: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@tklengyel.com>
> Acked-by: Wei Liu <wei.liu2@citrix.com>
> ---
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>

Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>


Thanks,
Razvan
Julien Grall Sept. 16, 2016, 1:54 p.m. UTC | #2
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@tklengyel.com>
> Acked-by: Wei Liu <wei.liu2@citrix.com>
> ---
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
>
> 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@zentific.com)
> + *
> + * 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,
Tamas Lengyel Sept. 16, 2016, 3:48 p.m. UTC | #3
On Fri, Sep 16, 2016 at 7:54 AM, Julien Grall <julien.grall@arm.com> wrote:
> 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@tklengyel.com>
>> Acked-by: Wei Liu <wei.liu2@citrix.com>
>> ---
>> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>> Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
>> Cc: Stefano Stabellini <sstabellini@kernel.org>
>> Cc: Julien Grall <julien.grall@arm.com>
>>
>> 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.

Not putting a fault on anyone. I did take a look at it but there had
been enough unknowns for me that I would rather have you do it when
you get to it. I think this is just a minor issue that should not be a
roadblock for this patch. If it gets filtered at some point in the
future it will have no impact on applications that this interface is
intended for. For all else, a warning is in place not to rely on this
behavior being a "feature".

>
>>       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).

I understand that was the original intention behind the instruction.
So when the emulation use-case becomes actual the immediate can be
included and passed along too, we would just bump the
VM_EVENT_INTERFACE_VERSION. As that was not my usecase it is beyond
scope for this patch. It can certainly be implemented at some later
time.

>
>
>> ---
>>  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

Yeap.

>
>
>> + *
>> + * Arch-specific monitor_op domctl handler.
>> + *
>> + * Copyright (c) 2016 Tamas K Lengyel (tamas.lengyel@zentific.com)
>> + *
>> + * 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.

Ack.

>
>
>> +    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.

Ack.

>
>> +    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.

Ack.

>
>> + *       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

Thanks!
Tamas
Tamas Lengyel Sept. 16, 2016, 5 p.m. UTC | #4
>> 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.
>

Not necessarily. If it's an SMC the guest issues by itself then yes,
it can be safely turned effectively into a NOP by incrementing PC. But
there are other things the monitor application can do as well. For
example, during malware analysis if we want to remain stealthy we
would still want to inject an undefined instruction exception, or even
destroy the guest altogether, otherwise the NOP-d SMC would reveal the
presence of the monitor application. For SMC's that the monitor
application itself injects, just incrementing the PC would not be
enough either as we overwrote an instruction that should be executed
to continue normal execution. That problem can also be solved, and
also not by incrementing the PC.

Tamas
diff mbox

Patch

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",
                        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
+ *
+ * Arch-specific monitor_op domctl handler.
+ *
+ * Copyright (c) 2016 Tamas K Lengyel (tamas.lengyel@zentific.com)
+ *
+ * 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) {
+    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;
+    if ( current->domain->arch.monitor.privileged_call_enabled )
+        rc = monitor_smc();
+
+    if ( rc != 1 )
+        inject_undef_exception(regs, hsr);
+}
+
 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.
+ *       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