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

[Tamas K Lengyel <tamas.lengyel@xxxxxxxxxxxx>]

On Wed, Jul 6, 2016 at 12:04 PM, Julien Grall <julien.grall@xxxxxxx> 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.

Thanks,
Tamas

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