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

[Julien Grall <julien.grall@xxxxxxx>]

Hi Tamas,

On 28/07/16 23:45, Tamas K Lengyel wrote:
> On Thu, Jul 28, 2016 at 4:03 PM, Julien Grall <julien.grall@xxxxxxx> wrote:
> > On 28/07/2016 22:33, Tamas K Lengyel wrote:
> > > On Jul 28, 2016 15:25, "Julien Grall" <julien.grall@xxxxxxx
> > > <mailto:julien.grall@xxxxxxx>> wrote:
> > > > On 28/07/2016 22:05, Tamas K Lengyel wrote:
> > > > > On Thu, Jul 28, 2016 at 3:01 PM, Julien Grall <julien.grall@xxxxxxx
> > > <mailto:julien.grall@xxxxxxx>> wrote:
> > > > > That's not how we do it with vm_event. Even on x86 we only selectively
> > > > > set registers using the VM_EVENT_FLAG_SET_REGISTERS flag (albeit it
> > > > > not being documented in the header). As for "not exposing them" it's a
> > > > > waste to declare separate structures for getting and setting. I'll
> > > > > change my mind about that if Razvan is on the side that we should
> > > > > start doing that, but I don't think that's the case at the moment.
> > > >
> > > >
> > > > Is there any rationale to only set a subset of the information you
> > > retrieved?
> > > >
> > > I just did a testrun with setting every register through this method to
> > > 0 other then pc and it resulted in hypervisor crash. Not sure if it's
> > > just my setup or not though so I'm still poking at it. However, I don't
> > > really see a usecase where setting ttbr regs to be required either via
> > > the fast method so it simply may not worth digging into it more at this
> > > time.
> >
> > To confirm, do you mean setting CPSR, TTBR0_EL1, TTBR1_EL1 to 0?
> >
> > TTBR*_EL1 are safe to set to any values (they are directly accessible by the
> > guest anyway). However this is not the case of CPSR. From my understanding
> > of the ARM ARM (B1-1150 in ARM DDI 0406C.b) is writing 0 to M[4:0] will lead
> > to an unpredictable behavior (that could cause an hypervisor trap).
> >
> > Can you copy/paste the hypervisor crash log?
> OK, the issue I had was that right now mem_access on ARM doesn't send
> the registers (yet) as it needs my monitor_traps work from the other
> patch so I kept setting all registers to 0. Rebasing on top of my
> other patch now I was able to verify that indeed only setting cpsr to
> arbitrary values (like 0) can cause hypervisor crash as follows:
Thank you for the stack trace. I managed to reproduce it and did further 
testing.
We need to sanitize the CPSR mode in Xen before writing it. So far, this 
can be only set by XEN_DOMCTL_setvcpucontext.
[...]

> Setting ttb/cr/r0/r1 is fine however as it only causes an in-guest
> kernel crash. So we still need to selectively set registers and as at
> this point only PC makes sense we will start with just that.
Which lead to my next question. For instance, we have an app A which is 
built for Xen 4.N and they want to also support Xen 4.(N + 1) which will 
set more registers and take advantage of it. How the app will 
differentiate the 2 versions?
Regards,

--
Julien Grall

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