[v2] arm/vm_event: get/set registers

Commit Message

Tamas Lengyel Aug. 1, 2016, 5:59 p.m. UTC

Add support for getting/setting registers through vm_event on ARM. Only
TTB/CR/R0/R1, PC and CPSR are sent as part of a request and only PC is set
as part of a response. The set of registers can be expanded in the future to
include other registers as well if necessary.

Signed-off-by: Tamas K Lengyel <[e-mail]>
Reviewed-by: Andrew Cooper <[e-mail]>
---
Cc: Stefano Stabellini <[e-mail]>
Cc: Julien Grall <[e-mail]>
Cc: Razvan Cojocaru <[e-mail]>
Cc: Jan Beulich <[e-mail]>

v2: Add assert for vCPU pause check
    Use correct regs pointer on setting registers
    Only set the register 'pc'
---
 xen/arch/arm/Makefile          |  1 +
 xen/arch/arm/vm_event.c        | 52 ++++++++++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/vm_event.h | 11 ---------
 xen/include/asm-x86/vm_event.h |  4 ----
 xen/include/public/vm_event.h  | 18 +++++++++++++--
 xen/include/xen/vm_event.h     |  3 +++
 6 files changed, 72 insertions(+), 17 deletions(-)
 create mode 100644 xen/arch/arm/vm_event.c

Comments

Razvan Cojocaru Aug. 2, 2016, 8:10 a.m. UTC | #1

On 08/01/2016 08:59 PM, Tamas K Lengyel wrote:
> Add support for getting/setting registers through vm_event on ARM. Only
> TTB/CR/R0/R1, PC and CPSR are sent as part of a request and only PC is set
> as part of a response. The set of registers can be expanded in the future to
> include other registers as well if necessary.
> 
> Signed-off-by: Tamas K Lengyel <[e-mail]>
> Reviewed-by: Andrew Cooper <[e-mail]>

Acked-by: Razvan Cojocaru <[e-mail]>

Tamas Lengyel Sept. 2, 2016, 5:37 p.m. UTC | #2

On Tue, Aug 2, 2016 at 2:10 AM, Razvan Cojocaru
<[e-mail]> wrote:
> On 08/01/2016 08:59 PM, Tamas K Lengyel wrote:
>> Add support for getting/setting registers through vm_event on ARM. Only
>> TTB/CR/R0/R1, PC and CPSR are sent as part of a request and only PC is set
>> as part of a response. The set of registers can be expanded in the future to
>> include other registers as well if necessary.
>>
>> Signed-off-by: Tamas K Lengyel <[e-mail]>
>> Reviewed-by: Andrew Cooper <[e-mail]>
>
> Acked-by: Razvan Cojocaru <[e-mail]>

Patch ping.

Andrew Cooper Sept. 2, 2016, 5:45 p.m. UTC | #3

On 02/09/16 18:37, Tamas K Lengyel wrote:
> On Tue, Aug 2, 2016 at 2:10 AM, Razvan Cojocaru
> <[e-mail]> wrote:
>> On 08/01/2016 08:59 PM, Tamas K Lengyel wrote:
>>> Add support for getting/setting registers through vm_event on ARM. Only
>>> TTB/CR/R0/R1, PC and CPSR are sent as part of a request and only PC is set
>>> as part of a response. The set of registers can be expanded in the future to
>>> include other registers as well if necessary.
>>>
>>> Signed-off-by: Tamas K Lengyel <[e-mail]>
>>> Reviewed-by: Andrew Cooper <[e-mail]>
>> Acked-by: Razvan Cojocaru <[e-mail]>
> Patch ping.

Requires an ARM ack.

~Andrew

Julien Grall Sept. 2, 2016, 6:40 p.m. UTC | #4

On 02/09/2016 18:45, Andrew Cooper wrote:
> On 02/09/16 18:37, Tamas K Lengyel wrote:
>> On Tue, Aug 2, 2016 at 2:10 AM, Razvan Cojocaru
>> <[e-mail]> wrote:
>>> On 08/01/2016 08:59 PM, Tamas K Lengyel wrote:
>>>> Add support for getting/setting registers through vm_event on ARM. Only
>>>> TTB/CR/R0/R1, PC and CPSR are sent as part of a request and only PC is set
>>>> as part of a response. The set of registers can be expanded in the future to
>>>> include other registers as well if necessary.
>>>>
>>>> Signed-off-by: Tamas K Lengyel <[e-mail]>
>>>> Reviewed-by: Andrew Cooper <[e-mail]>
>>> Acked-by: Razvan Cojocaru <[e-mail]>
>> Patch ping.
>
> Requires an ARM ack.

I don't think it requires an ack from Stefano and I, it touches only the 
vm_event subsystem.

If you still want an ARM ack, then I will defer to Stefano.

Cheers,

Tamas Lengyel Sept. 2, 2016, 6:47 p.m. UTC | #5

On Fri, Sep 2, 2016 at 12:40 PM, Julien Grall <[e-mail]> wrote:
> On 02/09/2016 18:45, Andrew Cooper wrote:
>>
>> On 02/09/16 18:37, Tamas K Lengyel wrote:
>>>
>>> On Tue, Aug 2, 2016 at 2:10 AM, Razvan Cojocaru
>>> <[e-mail]> wrote:
>>>>
>>>> On 08/01/2016 08:59 PM, Tamas K Lengyel wrote:
>>>>>
>>>>> Add support for getting/setting registers through vm_event on ARM. Only
>>>>> TTB/CR/R0/R1, PC and CPSR are sent as part of a request and only PC is
>>>>> set
>>>>> as part of a response. The set of registers can be expanded in the
>>>>> future to
>>>>> include other registers as well if necessary.
>>>>>
>>>>> Signed-off-by: Tamas K Lengyel <[e-mail]>
>>>>> Reviewed-by: Andrew Cooper <[e-mail]>
>>>>
>>>> Acked-by: Razvan Cojocaru <[e-mail]>
>>>
>>> Patch ping.
>>
>>
>> Requires an ARM ack.
>
>
> I don't think it requires an ack from Stefano and I, it touches only the
> vm_event subsystem.
>
> If you still want an ARM ack, then I will defer to Stefano.

Indeed it only touches the vm_event system so just wanted to double
check it's OK from your side as we had lengthy discussion about it. If
there are no objections a formal ack should not be necessary and it's
good to go.

Thanks,
Tamas

Julien Grall Sept. 2, 2016, 7:10 p.m. UTC | #6

On 02/09/2016 19:47, Tamas K Lengyel wrote:
> On Fri, Sep 2, 2016 at 12:40 PM, Julien Grall <[e-mail]> wrote:
>> On 02/09/2016 18:45, Andrew Cooper wrote:
>>>
>>> On 02/09/16 18:37, Tamas K Lengyel wrote:
>>>>
>>>> On Tue, Aug 2, 2016 at 2:10 AM, Razvan Cojocaru
>>>> <[e-mail]> wrote:
>>>>>
>>>>> On 08/01/2016 08:59 PM, Tamas K Lengyel wrote:
>>>>>>
>>>>>> Add support for getting/setting registers through vm_event on ARM. Only
>>>>>> TTB/CR/R0/R1, PC and CPSR are sent as part of a request and only PC is
>>>>>> set
>>>>>> as part of a response. The set of registers can be expanded in the
>>>>>> future to
>>>>>> include other registers as well if necessary.
>>>>>>
>>>>>> Signed-off-by: Tamas K Lengyel <[e-mail]>
>>>>>> Reviewed-by: Andrew Cooper <[e-mail]>
>>>>>
>>>>> Acked-by: Razvan Cojocaru <[e-mail]>
>>>>
>>>> Patch ping.
>>>
>>>
>>> Requires an ARM ack.
>>
>>
>> I don't think it requires an ack from Stefano and I, it touches only the
>> vm_event subsystem.
>>
>> If you still want an ARM ack, then I will defer to Stefano.
>
> Indeed it only touches the vm_event system so just wanted to double
> check it's OK from your side as we had lengthy discussion about it. If
> there are no objections a formal ack should not be necessary and it's
> good to go.

My objections are still there. Hence why I said I will defer to Stefano.

Regards,

Tamas Lengyel Sept. 2, 2016, 7:19 p.m. UTC | #7

On Fri, Sep 2, 2016 at 1:10 PM, Julien Grall <[e-mail]> wrote:
>
>
> On 02/09/2016 19:47, Tamas K Lengyel wrote:
>>
>> On Fri, Sep 2, 2016 at 12:40 PM, Julien Grall <[e-mail]>
>> wrote:
>>>
>>> On 02/09/2016 18:45, Andrew Cooper wrote:
>>>>
>>>>
>>>> On 02/09/16 18:37, Tamas K Lengyel wrote:
>>>>>
>>>>>
>>>>> On Tue, Aug 2, 2016 at 2:10 AM, Razvan Cojocaru
>>>>> <[e-mail]> wrote:
>>>>>>
>>>>>>
>>>>>> On 08/01/2016 08:59 PM, Tamas K Lengyel wrote:
>>>>>>>
>>>>>>>
>>>>>>> Add support for getting/setting registers through vm_event on ARM.
>>>>>>> Only
>>>>>>> TTB/CR/R0/R1, PC and CPSR are sent as part of a request and only PC
>>>>>>> is
>>>>>>> set
>>>>>>> as part of a response. The set of registers can be expanded in the
>>>>>>> future to
>>>>>>> include other registers as well if necessary.
>>>>>>>
>>>>>>> Signed-off-by: Tamas K Lengyel <[e-mail]>
>>>>>>> Reviewed-by: Andrew Cooper <[e-mail]>
>>>>>>
>>>>>>
>>>>>> Acked-by: Razvan Cojocaru <[e-mail]>
>>>>>
>>>>>
>>>>> Patch ping.
>>>>
>>>>
>>>>
>>>> Requires an ARM ack.
>>>
>>>
>>>
>>> I don't think it requires an ack from Stefano and I, it touches only the
>>> vm_event subsystem.
>>>
>>> If you still want an ARM ack, then I will defer to Stefano.
>>
>>
>> Indeed it only touches the vm_event system so just wanted to double
>> check it's OK from your side as we had lengthy discussion about it. If
>> there are no objections a formal ack should not be necessary and it's
>> good to go.
>
>
> My objections are still there. Hence why I said I will defer to Stefano.

Fair enough.

Thanks,
Tamas

Stefano Stabellini Sept. 5, 2016, 7:45 p.m. UTC | #8

On Fri, 2 Sep 2016, Julien Grall wrote:
> On 02/09/2016 18:45, Andrew Cooper wrote:
> > On 02/09/16 18:37, Tamas K Lengyel wrote:
> > > On Tue, Aug 2, 2016 at 2:10 AM, Razvan Cojocaru
> > > <[e-mail]> wrote:
> > > > On 08/01/2016 08:59 PM, Tamas K Lengyel wrote:
> > > > > Add support for getting/setting registers through vm_event on ARM.
> > > > > Only
> > > > > TTB/CR/R0/R1, PC and CPSR are sent as part of a request and only PC is
> > > > > set
> > > > > as part of a response. The set of registers can be expanded in the
> > > > > future to
> > > > > include other registers as well if necessary.
> > > > > 
> > > > > Signed-off-by: Tamas K Lengyel <[e-mail]>
> > > > > Reviewed-by: Andrew Cooper <[e-mail]>
> > > > Acked-by: Razvan Cojocaru <[e-mail]>
> > > Patch ping.
> > 
> > Requires an ARM ack.
> 
> I don't think it requires an ack from Stefano and I, it touches only the
> vm_event subsystem.
> 
> If you still want an ARM ack, then I will defer to Stefano.

Acked-by: Stefano Stabellini <[e-mail]>

Tamas Lengyel Sept. 9, 2016, 4:26 p.m. UTC | #9

On Mon, Sep 5, 2016 at 1:45 PM, Stefano Stabellini
<[e-mail]> wrote:
> On Fri, 2 Sep 2016, Julien Grall wrote:
>> On 02/09/2016 18:45, Andrew Cooper wrote:
>> > On 02/09/16 18:37, Tamas K Lengyel wrote:
>> > > On Tue, Aug 2, 2016 at 2:10 AM, Razvan Cojocaru
>> > > <[e-mail]> wrote:
>> > > > On 08/01/2016 08:59 PM, Tamas K Lengyel wrote:
>> > > > > Add support for getting/setting registers through vm_event on ARM.
>> > > > > Only
>> > > > > TTB/CR/R0/R1, PC and CPSR are sent as part of a request and only PC is
>> > > > > set
>> > > > > as part of a response. The set of registers can be expanded in the
>> > > > > future to
>> > > > > include other registers as well if necessary.
>> > > > >
>> > > > > Signed-off-by: Tamas K Lengyel <[e-mail]>
>> > > > > Reviewed-by: Andrew Cooper <[e-mail]>
>> > > > Acked-by: Razvan Cojocaru <[e-mail]>
>> > > Patch ping.
>> >
>> > Requires an ARM ack.
>>
>> I don't think it requires an ack from Stefano and I, it touches only the
>> vm_event subsystem.
>>
>> If you still want an ARM ack, then I will defer to Stefano.
>
> Acked-by: Stefano Stabellini <[e-mail]>

Thanks! I think this is ready for merging.

Tamas

Patch

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index b264ed4..5752830 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -41,6 +41,7 @@  obj-y += traps.o
 obj-y += vgic.o
 obj-y += vgic-v2.o
 obj-$(CONFIG_ARM_64) += vgic-v3.o
+obj-y += vm_event.o
 obj-y += vtimer.o
 obj-y += vpsci.o
 obj-y += vuart.o
diff --git a/xen/arch/arm/vm_event.c b/xen/arch/arm/vm_event.c
new file mode 100644
index 0000000..47312e9
--- /dev/null
+++ b/xen/arch/arm/vm_event.c
@@ -0,0 +1,52 @@ 
+/*
+ * arch/arm/vm_event.c
+ *
+ * Architecture-specific vm_event handling routines
+ *
+ * Copyright (c) 2016 Tamas K Lengyel ([e-mail])
+ *
+ * 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 = guest_cpu_user_regs();
+
+    req->data.regs.arm.cpsr = regs->cpsr;
+    req->data.regs.arm.pc = regs->pc;
+    req->data.regs.arm.ttbcr = READ_SYSREG(TCR_EL1);
+    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;
+
+    /* vCPU should be paused */
+    ASSERT(atomic_read(&v->vm_event_pause_count));
+
+    regs->pc = rsp->data.regs.arm.pc;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/asm-arm/vm_event.h b/xen/include/asm-arm/vm_event.h
index ccc4b60..9482636 100644
--- a/xen/include/asm-arm/vm_event.h
+++ b/xen/include/asm-arm/vm_event.h
@@ -45,15 +45,4 @@  void vm_event_register_write_resume(struct vcpu *v, vm_event_response_t *rsp)
     /* Not supported on ARM. */
 }
 
-static inline
-void vm_event_set_registers(struct vcpu *v, vm_event_response_t *rsp)
-{
-    /* Not supported on ARM. */
-}
-
-static inline void vm_event_fill_regs(vm_event_request_t *req)
-{
-    /* Not supported on ARM. */
-}
-
 #endif /* __ASM_ARM_VM_EVENT_H__ */
diff --git a/xen/include/asm-x86/vm_event.h b/xen/include/asm-x86/vm_event.h
index 7e6adff..294def6 100644
--- a/xen/include/asm-x86/vm_event.h
+++ b/xen/include/asm-x86/vm_event.h
@@ -39,8 +39,4 @@  void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v);
 
 void vm_event_register_write_resume(struct vcpu *v, vm_event_response_t *rsp);
 
-void vm_event_set_registers(struct vcpu *v, vm_event_response_t *rsp);
-
-void vm_event_fill_regs(vm_event_request_t *req);
-
 #endif /* __ASM_X86_VM_EVENT_H__ */
diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h
index 64e6857..32cd4d7 100644
--- a/xen/include/public/vm_event.h
+++ b/xen/include/public/vm_event.h
@@ -132,8 +132,8 @@ 
 #define VM_EVENT_X86_XCR0   3
 
 /*
- * Using a custom struct (not hvm_hw_cpu) so as to not fill
- * the vm_event ring buffer too quickly.
+ * Using custom vCPU structs (i.e. not hvm_hw_cpu) for both x86 and ARM
+ * so as to not fill the vm_event ring buffer too quickly.
  */
 struct vm_event_regs_x86 {
     uint64_t rax;
@@ -172,6 +172,19 @@  struct vm_event_regs_x86 {
 };
 
 /*
+ * Only the register 'pc' can be set on a vm_event response using the
+ * VM_EVENT_FLAG_SET_REGISTERS flag.
+ */
+struct vm_event_regs_arm {
+    uint64_t ttbr0;
+    uint64_t ttbr1;
+    uint64_t ttbcr;
+    uint64_t pc;
+    uint32_t cpsr;
+    uint32_t _pad;
+};
+
+/*
  * mem_access flag definitions
  *
  * These flags are set only as part of a mem_event request.
@@ -273,6 +286,7 @@  typedef struct vm_event_st {
     union {
         union {
             struct vm_event_regs_x86 x86;
+            struct vm_event_regs_arm arm;
         } regs;
 
         struct vm_event_emul_read_data emul_read_data;
diff --git a/xen/include/xen/vm_event.h b/xen/include/xen/vm_event.h
index c09f723..4f088c8 100644
--- a/xen/include/xen/vm_event.h
+++ b/xen/include/xen/vm_event.h
@@ -75,6 +75,9 @@  int vm_event_domctl(struct domain *d, xen_domctl_vm_event_op_t *vec,
 void vm_event_vcpu_pause(struct vcpu *v);
 void vm_event_vcpu_unpause(struct vcpu *v);
 
+void vm_event_fill_regs(vm_event_request_t *req);
+void vm_event_set_registers(struct vcpu *v, vm_event_response_t *rsp);
+
 #endif /* __VM_EVENT_H__ */
 
 /*