On 08.01.2014, at 11:25, Paul Mackerras <pau...@samba.org> wrote:

> Currently in book3s_hv_rmhandlers.S we have three places where we
> have woken up from nap mode and we check the reason field in SRR1
> to see what event woke us up.  This consolidates them into a new
> function, kvmppc_check_wake_reason.  It looks at the wake reason
> field in SRR1, and if it indicates that an external interrupt caused
> the wakeup, calls kvmppc_read_intr to check what sort of interrupt
> it was.
> 
> This also consolidates the two places where we synthesize an external
> interrupt (0x500 vector) for the guest.  Now, if the guest exit code
> finds that there was an external interrupt which has been handled
> (i.e. it was an IPI indicating that there is now an interrupt pending
> for the guest), it jumps to deliver_guest_interrupt, which is in the
> last part of the guest entry code, where we synthesize guest external
> and decrementer interrupts.  That code has been streamlined a little
> and now clears LPCR[MER] when appropriate as well as setting it.
> 
> The extra clearing of any pending IPI on a secondary, offline CPU
> thread before going back to nap mode has been removed.  It is no longer
> necessary now that we have code to read and acknowledge IPIs in the
> guest exit path.
> 
> This fixes a minor bug in the H_CEDE real-mode handling - previously,
> if we found that other threads were already exiting the guest when we
> were about to go to nap mode, we would branch to the cede wakeup path
> and end up looking in SRR1 for a wakeup reason.  Now we branch to a
> point after we have checked the wakeup reason.
> 
> This also fixes a minor bug in kvmppc_read_intr - previously it could
> return 0xff rather than 1, in the case where we find that a host IPI
> is pending after we have cleared the IPI.  Now it returns 1.
> 
> Signed-off-by: Paul Mackerras <pau...@samba.org>
> ---
> arch/powerpc/kvm/book3s_hv_rmhandlers.S | 192 +++++++++++++-------------------
> 1 file changed, 77 insertions(+), 115 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S 
> b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> index c0a5d8a..61125426 100644
> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> @@ -192,8 +192,10 @@ kvm_novcpu_wakeup:
>       stb     r0, HSTATE_NAPPING(r13)
>       stb     r0, HSTATE_HWTHREAD_REQ(r13)
> 
> +     /* check the wake reason */
> +     bl      kvmppc_check_wake_reason
> +     
>       /* see if any other thread is already exiting */
> -     li      r12, 0
>       lwz     r0, VCORE_ENTRY_EXIT(r5)
>       cmpwi   r0, 0x100
>       bge     kvm_novcpu_exit
> @@ -203,23 +205,14 @@ kvm_novcpu_wakeup:
>       li      r0, 1
>       sld     r0, r0, r7
>       addi    r6, r5, VCORE_NAPPING_THREADS
> -4:   lwarx   r3, 0, r6
> -     andc    r3, r3, r0
> -     stwcx.  r3, 0, r6
> +4:   lwarx   r7, 0, r6
> +     andc    r7, r7, r0
> +     stwcx.  r7, 0, r6
>       bne     4b
> 
> -     /* Check the wake reason in SRR1 to see why we got here */
> -     mfspr   r3, SPRN_SRR1
> -     rlwinm  r3, r3, 44-31, 0x7      /* extract wake reason field */
> -     cmpwi   r3, 4                   /* was it an external interrupt? */
> -     bne     kvm_novcpu_exit         /* if not, exit the guest */
> -
> -     /* extern interrupt - read and handle it */
> -     li      r12, BOOK3S_INTERRUPT_EXTERNAL
> -     bl      kvmppc_read_intr
> +     /* See if the wake reason means we need to exit */
>       cmpdi   r3, 0
>       bge     kvm_novcpu_exit
> -     li      r12, 0
> 
>       /* Got an IPI but other vcpus aren't yet exiting, must be a latecomer */
>       ld      r4, HSTATE_KVM_VCPU(r13)
> @@ -263,40 +256,16 @@ kvm_start_guest:
>        */
> 
>       /* Check the wake reason in SRR1 to see why we got here */
> -     mfspr   r3,SPRN_SRR1
> -     rlwinm  r3,r3,44-31,0x7         /* extract wake reason field */
> -     cmpwi   r3,4                    /* was it an external interrupt? */
> -     bne     27f                     /* if not */
> -     ld      r5,HSTATE_XICS_PHYS(r13)
> -     li      r7,XICS_XIRR            /* if it was an external interrupt, */
> -     lwzcix  r8,r5,r7                /* get and ack the interrupt */
> -     sync
> -     clrldi. r9,r8,40                /* get interrupt source ID. */
> -     beq     28f                     /* none there? */
> -     cmpwi   r9,XICS_IPI             /* was it an IPI? */
> -     bne     29f
> -     li      r0,0xff
> -     li      r6,XICS_MFRR
> -     stbcix  r0,r5,r6                /* clear IPI */
> -     stwcix  r8,r5,r7                /* EOI the interrupt */
> -     sync                            /* order loading of vcpu after that */
> +     bl      kvmppc_check_wake_reason
> +     cmpdi   r3, 0
> +     bge     kvm_no_guest
> 
>       /* get vcpu pointer, NULL if we have no vcpu to run */
>       ld      r4,HSTATE_KVM_VCPU(r13)
>       cmpdi   r4,0
>       /* if we have no vcpu to run, go back to sleep */
>       beq     kvm_no_guest
> -     b       30f
> 
> -27:  /* XXX should handle hypervisor maintenance interrupts etc. here */
> -     b       kvm_no_guest
> -28:  /* SRR1 said external but ICP said nope?? */
> -     b       kvm_no_guest
> -29:  /* External non-IPI interrupt to offline secondary thread? help?? */
> -     stw     r8,HSTATE_SAVED_XIRR(r13)
> -     b       kvm_no_guest
> -
> -30:
>       /* Set HSTATE_DSCR(r13) to something sensible */
>       LOAD_REG_ADDR(r6, dscr_default)
>       ld      r6, 0(r6)
> @@ -314,18 +283,6 @@ kvm_start_guest:
>        * visible we could be given another vcpu.
>        */
>       lwsync
> -     /* Clear any pending IPI - we're an offline thread */
> -     ld      r5, HSTATE_XICS_PHYS(r13)
> -     li      r7, XICS_XIRR
> -     lwzcix  r3, r5, r7              /* ack any pending interrupt */
> -     rlwinm. r0, r3, 0, 0xffffff     /* any pending? */
> -     beq     37f
> -     sync
> -     li      r0, 0xff
> -     li      r6, XICS_MFRR
> -     stbcix  r0, r5, r6              /* clear the IPI */
> -     stwcix  r3, r5, r7              /* EOI it */
> -37:  sync
> 
>       /* increment the nap count and then go to nap mode */
>       ld      r4, HSTATE_KVM_VCORE(r13)
> @@ -821,47 +778,46 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_206)
>       mtctr   r6
>       mtxer   r7
> 
> +kvmppc_cede_reentry:         /* r4 = vcpu, r13 = paca */
>       ld      r10, VCPU_PC(r4)
>       ld      r11, VCPU_MSR(r4)
> -kvmppc_cede_reentry:         /* r4 = vcpu, r13 = paca */
>       ld      r6, VCPU_SRR0(r4)
>       ld      r7, VCPU_SRR1(r4)
> +     mtspr   SPRN_SRR0, r6
> +     mtspr   SPRN_SRR1, r7
> 
> +deliver_guest_interrupt:
>       /* r11 = vcpu->arch.msr & ~MSR_HV */
>       rldicl  r11, r11, 63 - MSR_HV_LG, 1
>       rotldi  r11, r11, 1 + MSR_HV_LG
>       ori     r11, r11, MSR_ME
> 
>       /* Check if we can deliver an external or decrementer interrupt now */
> -     ld      r0,VCPU_PENDING_EXC(r4)
> -     lis     r8,(1 << BOOK3S_IRQPRIO_EXTERNAL_LEVEL)@h
> -     and     r0,r0,r8
> -     cmpdi   cr1,r0,0
> -     andi.   r0,r11,MSR_EE
> -     beq     cr1,11f
> +     ld      r0, VCPU_PENDING_EXC(r4)
> +     rldicl  r0, r0, 64 - BOOK3S_IRQPRIO_EXTERNAL_LEVEL, 63
> +     cmpdi   cr1, r0, 0
> +     andi.   r8, r11, MSR_EE
> BEGIN_FTR_SECTION
> -     mfspr   r8,SPRN_LPCR
> -     ori     r8,r8,LPCR_MER
> -     mtspr   SPRN_LPCR,r8
> +     mfspr   r8, SPRN_LPCR
> +     /* Insert EXTERNAL_LEVEL bit into LPCR at the MER bit position */
> +     rldimi  r8, r0, LPCR_MER_SH, 63 - LPCR_MER_SH
> +     mtspr   SPRN_LPCR, r8
>       isync
> END_FTR_SECTION_IFSET(CPU_FTR_ARCH_206)
>       beq     5f
> -     li      r0,BOOK3S_INTERRUPT_EXTERNAL
> -12:  mr      r6,r10
> +     li      r0, BOOK3S_INTERRUPT_EXTERNAL
> +     bne     cr1, 12f
> +     mfspr   r0, SPRN_DEC
> +     cmpwi   r0, 0
> +     li      r0, BOOK3S_INTERRUPT_DECREMENTER
> +     bge     5f
> +
> +12:  mtspr   SPRN_SRR0, r10
>       mr      r10,r0
> -     mr      r7,r11
> +     mtspr   SPRN_SRR1, r11
>       li      r11,(MSR_ME << 1) | 1   /* synthesize MSR_SF | MSR_ME */
>       rotldi  r11,r11,63
> -     b       5f
> -11:  beq     5f
> -     mfspr   r0,SPRN_DEC
> -     cmpwi   r0,0
> -     li      r0,BOOK3S_INTERRUPT_DECREMENTER
> -     blt     12b
> -
> -     /* Move SRR0 and SRR1 into the respective regs */
> -5:   mtspr   SPRN_SRR0, r6
> -     mtspr   SPRN_SRR1, r7
> +5:
> 
> /*
>  * Required state:
> @@ -1049,39 +1005,19 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_206)
>       /* External interrupt, first check for host_ipi. If this is
>        * set, we know the host wants us out so let's do it now
>        */
> -do_ext_interrupt:
>       bl      kvmppc_read_intr
>       cmpdi   r3, 0
>       bgt     ext_interrupt_to_host
> 
> -     /* Allright, looks like an IPI for the guest, we need to set MER */
>       /* Check if any CPU is heading out to the host, if so head out too */
>       ld      r5, HSTATE_KVM_VCORE(r13)
>       lwz     r0, VCORE_ENTRY_EXIT(r5)
>       cmpwi   r0, 0x100
>       bge     ext_interrupt_to_host
> 
> -     /* See if there is a pending interrupt for the guest */
> -     mfspr   r8, SPRN_LPCR
> -     ld      r0, VCPU_PENDING_EXC(r9)
> -     /* Insert EXTERNAL_LEVEL bit into LPCR at the MER bit position */
> -     rldicl. r0, r0, 64 - BOOK3S_IRQPRIO_EXTERNAL_LEVEL, 63
> -     rldimi  r8, r0, LPCR_MER_SH, 63 - LPCR_MER_SH
> -     beq     2f
> -
> -     /* And if the guest EE is set, we can deliver immediately, else
> -      * we return to the guest with MER set
> -      */
> -     andi.   r0, r11, MSR_EE
> -     beq     2f
> -     mtspr   SPRN_SRR0, r10
> -     mtspr   SPRN_SRR1, r11
> -     li      r10, BOOK3S_INTERRUPT_EXTERNAL
> -     li      r11, (MSR_ME << 1) | 1  /* synthesize MSR_SF | MSR_ME */
> -     rotldi  r11, r11, 63
> -2:   mr      r4, r9
> -     mtspr   SPRN_LPCR, r8
> -     b       fast_guest_return
> +     /* Return to guest after delivering any pending interrupt */
> +     mr      r4, r9
> +     b       deliver_guest_interrupt
> 
> ext_interrupt_to_host:
> 
> @@ -1890,7 +1826,6 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_206)
>       isync
>       li      r0,NAPPING_CEDE
>       stb     r0,HSTATE_NAPPING(r13)
> -     mr      r4,r3
>       lwz     r7,VCORE_ENTRY_EXIT(r5)
>       cmpwi   r7,0x100
>       bge     33f             /* another thread already exiting */
> @@ -1943,6 +1878,11 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_206)
>       nap
>       b       .
> 
> +33:  mr      r4, r3
> +     li      r3, 0
> +     li      r12, 0
> +     b       34f
> +
> kvm_end_cede:
>       /* get vcpu pointer */
>       ld      r4, HSTATE_KVM_VCPU(r13)
> @@ -1972,12 +1912,15 @@ kvm_end_cede:
>       ld      r29, VCPU_GPR(R29)(r4)
>       ld      r30, VCPU_GPR(R30)(r4)
>       ld      r31, VCPU_GPR(R31)(r4)
> + 
> +     /* Check the wake reason in SRR1 to see why we got here */
> +     bl      kvmppc_check_wake_reason
> 
>       /* clear our bit in vcore->napping_threads */
> -33:  ld      r5,HSTATE_KVM_VCORE(r13)
> -     lbz     r3,HSTATE_PTID(r13)
> +34:  ld      r5,HSTATE_KVM_VCORE(r13)
> +     lbz     r7,HSTATE_PTID(r13)
>       li      r0,1
> -     sld     r0,r0,r3
> +     sld     r0,r0,r7
>       addi    r6,r5,VCORE_NAPPING_THREADS
> 32:   lwarx   r7,0,r6
>       andc    r7,r7,r0
> @@ -1986,23 +1929,18 @@ kvm_end_cede:
>       li      r0,0
>       stb     r0,HSTATE_NAPPING(r13)
> 
> -     /* Check the wake reason in SRR1 to see why we got here */
> -     mfspr   r3, SPRN_SRR1
> -     rlwinm  r3, r3, 44-31, 0x7      /* extract wake reason field */
> -     cmpwi   r3, 4                   /* was it an external interrupt? */
> -     li      r12, BOOK3S_INTERRUPT_EXTERNAL
> +     /* See if the wake reason means we need to exit */
> +     stw     r12, VCPU_TRAP(r4)
>       mr      r9, r4
> -     ld      r10, VCPU_PC(r9)
> -     ld      r11, VCPU_MSR(r9)
> -     beq     do_ext_interrupt        /* if so */
> +     cmpdi   r3, 0
> +     bgt     guest_exit_cont
> 
>       /* see if any other thread is already exiting */
>       lwz     r0,VCORE_ENTRY_EXIT(r5)

^

>       cmpwi   r0,0x100
> -     blt     kvmppc_cede_reentry     /* if not go back to guest */
> +     bge     guest_exit_cont
> 
> -     /* some threads are exiting, so go to the guest exit path */
> -     b       hcall_real_fallback
> +     b       kvmppc_cede_reentry     /* if not go back to guest */
> 
>       /* cede when already previously prodded case */
> kvm_cede_prodded:
> @@ -2033,6 +1971,29 @@ machine_check_realmode:
>       b       fast_interrupt_c_return
> 
> /*
> + * Check the reason we woke from nap, and take appropriate action.
> + * Returns:
> + *   0 if nothing needs to be done
> + *   1 if something happened that needs to be handled by the host
> + *   -1 if there was a guest wakeup (IPI)
> + *
> + * Also sets r12 to the interrupt vector for any interrupt that needs
> + * to be handled now by the host (0x500 for external interrupt), or zero.
> + */
> +kvmppc_check_wake_reason:
> +     mfspr   r6, SPRN_SRR1
> +     rlwinm  r6, r6, 44-31, 0x7      /* extract wake reason field */
> +     cmpwi   r6, 4                   /* was it an external interrupt? */
> +     li      r12, BOOK3S_INTERRUPT_EXTERNAL
> +     beq     kvmppc_read_intr        /* if so, see what it was */
> +     li      r3, 0
> +     li      r12, 0
> +     cmpwi   r6, 6                   /* was it the decrementer? */
> +     beq     0f
> +     li      r3, 1                   /* anything else, return 1 */
> +0:   blr

Tricky :). Please document somewhere (in a follow-up patch) which registers 
this function actually uses. Up there you assume that it doesn't modify r5 for 
example.


Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to