[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] arm/vm_event: get/set registers





On 28/07/2016 21:36, Tamas K Lengyel wrote:
On Thu, Jul 28, 2016 at 2:26 PM, Andrew Cooper
<andrew.cooper3@xxxxxxxxxx> wrote:
On 28/07/2016 21:05, Tamas K Lengyel wrote:
Add support for getting/setting registers through vm_event on ARM.
The set of registers can be expanded in the future to include other registers
as well if necessary but for now it is limited to TTB/CR/R0/R1, PC and CPSR.

Signed-off-by: Tamas K Lengyel <tamas.lengyel@xxxxxxxxxxxx>
---
Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
Cc: Julien Grall <julien.grall@xxxxxxx>
Cc: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
Cc: Jan Beulich <jbeulich@xxxxxxxx>
Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

For the x86 and common bits, Reviewed-by: Andrew Cooper
<andrew.cooper3@xxxxxxxxxx>

However,

+#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 = guest_cpu_user_regs();
+
+    regs->cpsr = rsp->data.regs.arm.cpsr;
+    regs->pc = rsp->data.regs.arm.pc;
+    v->arch.ttbcr = rsp->data.regs.arm.ttbcr;
+    v->arch.ttbr0 = rsp->data.regs.arm.ttbr0;
+    v->arch.ttbr1 = rsp->data.regs.arm.ttbr1;

Not knowing anything about ARM, but this looks like it is missing some
sanity/plausibility checks (to protect Xen against accidental clobbering
from the vm_event listener), and some WRITE_SYSREG() to reload the new
values (unless this is done unconditionally later, at which point you
should at least leave a comment here saying so).


This function only ever gets called if the vm_event response
specifically has the VM_EVENT_FLAG_SET_REGISTERS set, so accidental
clobbering is not possible. Also, using WRITE_SYSREG() is not safe at
this point because current != v. However, I have another issue here
with regs which should actually be:

What if the vCPU is running? If it cannot happen, please document and add an ASSERT/BUG_ON.


    struct cpu_user_regs *regs = &v->arch.cpu_info->guest_cpu_user_regs;

I'll fix that shortly.

Thanks,
Tamas


--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.