On 07/07/16 09:23, Jan Beulich wrote:
On 06.07.16 at 21:12, <tamas.leng...@zentific.com> wrote:
On Wed, Jul 6, 2016 at 12:04 PM, Julien Grall <julien.gr...@arm.com> wrote:
On 05/07/16 19:37, Tamas K Lengyel wrote:
+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.ttbr0 = READ_SYSREG64(TTBR0_EL1);
+ req->data.regs.arm.ttbr1 = READ_SYSREG64(TTBR1_EL1);
+
+ if ( psr_mode_is_32bit(regs->cpsr) )
+ {
+ req->data.regs.arm.arch.arm32.r0 = regs->r0;
+ req->data.regs.arm.arch.arm32.r1 = regs->r1;
+ req->data.regs.arm.arch.arm32.r2 = regs->r2;
+ req->data.regs.arm.arch.arm32.r3 = regs->r3;
+ req->data.regs.arm.arch.arm32.r4 = regs->r4;
+ req->data.regs.arm.arch.arm32.r5 = regs->r5;
+ req->data.regs.arm.arch.arm32.r6 = regs->r6;
+ req->data.regs.arm.arch.arm32.r7 = regs->r7;
+ req->data.regs.arm.arch.arm32.r8 = regs->r8;
+ req->data.regs.arm.arch.arm32.r9 = regs->r9;
+ req->data.regs.arm.arch.arm32.r10 = regs->r10;
+ req->data.regs.arm.arch.arm32.r11 = regs->fp;
+ req->data.regs.arm.arch.arm32.r12 = regs->r12;
+ req->data.regs.arm.arch.arm32.r13 = regs->sp;
Please look at the description of "sp" in cpu_user_regs. You will notice
this is only valid for the hypervisor.
There are multiple stack pointers for the guest depending on the running
mode (see B9.2.1 in ARM DDI 0406C.c), so you may want to pass all of them.
+ req->data.regs.arm.arch.arm32.r14 = regs->lr;
Whilst lr is an union with lr_usr on ARM32 port, for the ARM64 port they are
distinct (see cpu_users). So you would use the wrong register here.
However, as for sp, there are multiple lr pointers for the guest depending
on the running mode. So you will pass the wrong lr if the guest is running
in another mode than user.
This patch is becoming a lot more work then what I need it for so I'm
inclined to just reduce it to the bare minimum my use-case requires,
which is only cpsr, pc and ttbr0 and ttbr1. I had reservation about
growing the data field in the vm_event struct anyway, especially since
there is no use-case that requires it. In case someone has an actual
use-case in the future that requires other registers to be submitted
through vm_event, the interface is available for extension.
Rather than dropping this patch entirely (as you suggest in your
other reply) I am, fwiw, fine with a register set not including any
GPRs at all. Not sure about Julien though.
Well, the SMC instructions are used to communicate with the secure mode
(usually a trusted firmware).
In general, if you want to trap the SMC instructions it is for emulating
them and not using them for breakpoint in the guest (as for Tamas's
use-case). In this case, you want to have the set of GPRs available in
the vm_event request to avoid the overhead of the DOMCTL_getvcpucontext
(assuming the vCPU has been paused).
Adding the GPRs in the vm_event will likely require to bump the version
number (VM_EVENT_INTERFACE_VERSION) because the ABI will not be
compatible. If we consider that it is ok to ask developers to rebuild
their vm-event app, then fine.
Anyway, I think this patch was in a good state (though few registers
were needed clarification). Assuming it is ok to break the compatibility
later on, I will not oppose to have a reduce set.
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
