Re: [PATCH v4 3/3] KVM: PPC: epapr: install ev_idle hcall for e500 guest

On 16.02.2012, at 18:28, Scott Wood wrote:

On 02/16/2012 11:18 AM, Alexander Graf wrote:

quoted

=20
On 16.02.2012, at 17:58, Scott Wood wrote:
=20

quoted

On 02/16/2012 04:24 AM, Alexander Graf wrote:

quoted

On 16.02.2012, at 10:26, Liu Yu [off-list ref] wrote:

quoted

+_GLOBAL(epapr_ev_idle)
+epapr_ev_idle:
+    rlwinm    r3,r1,0,0,31-THREAD_SHIFT    /* current thread_info =

*/

quoted

quoted

quoted

quoted

+    lwz    r4,TI_LOCAL_FLAGS(r3)    /* set napping bit */
+    ori    r4,r4,_TLF_NAPPING    /* so when we take an exception =

*/

quoted

quoted

quoted

quoted

+    stw    r4,TI_LOCAL_FLAGS(r3)    /* it will return to our =

caller */

quoted

quoted

quoted

quoted

+
+    wrteei    1
+
+idle_loop:
+    LOAD_REG_IMMEDIATE(r11, HC_VENDOR_EPAPR | HC_EV_IDLE)
+
+.global epapr_ev_idle_start
+epapr_ev_idle_start:
+    li    r3, -1
+    nop
+    nop
+    nop

=20
Can't you just bl into epapr_hypercall_start? You don't even have =

to save the old lr. because we never return anyways :)

quoted

quoted

=20
The interrupt will branch to LR, so no, we can't trash it or put it
anywhere else.

=20
Hrm. But we can clobber ctr, right? So how about we make the generic =

version do a bctr and then just do a small C wrapper that takes lr, =
moves it to ctr and branches to the generic one?

=20
If it's just for this, I would say don't mess with the normal hcall =

path

for the sake of idle.  If using CTR would let us get away without
creating a stack frame in call sites, maybe that would be worthwhile,
depending on what sort of hcalls we end up having.
=20

quoted

Then we don't have to replicate the hypercall code all over again for =

every invocation.

=20
We shouldn't need to do it for every invocation.  Idle is special due =

to

the TLF_NAPPING hack.

Famous last words. If it's the only case, duplication should be ok. =
Let's hope there are no others.


Alex