On Wed, Jun 1, 2016 at 4:23 PM, Michael Neuling [off-list ref] wrote:

On Tue, 2016-05-31 at 17:16 +1000, Oliver O'Halloran wrote:

quoted

+#define IS_LD_ENABLED(reg)                 \
+     mfspr  reg,SPRN_LPCR;              \
+     andis. reg,reg,(LPCR_LD >> 16);

FWIW you can use:
        andis. reg,reg,(LPCR_LD)@ha

quoted

+#define GET_DEC(reg)                       \
+     IS_LD_ENABLED(reg);                \
+     mfspr reg, SPRN_DEC;               \
+     bne 99f;                           \
+     extsw reg, reg;                    \
+99:

It's a little painful that GET_DEC() is now 2 SPR moves.  SPR moves can be
a bit expensive.  Probably ok for now, but might be nice to store the guest
dec LD state somewhere so we don't have to retrieve it from the LPCR SPR.

Seems reasonable. It looks like it stashes the LPCR value in the KVM vcpu
structure already

Actually, it's probably best to do this now since checking the LD bit in
the LPCR on P8 is a bit dodgy and unnecessary. There is a kvm->arch.lpcr
you might be able use for this and you can add an asm-offsets for it too
(like KVM_LPID).

Is GET_DEC ever run in HV=0, where the guest couldn't read the LPCR?

It's only used in book3s_hv_rmhandlers.S, which contains the real mode h-call
handlers and none of that should be executed outside the host. Moving that
code into there from the generic exception header file is a good idea though.

Also, this now trashes cr0... have you checked that's ok in the paths it's
used?

It looks fine, but I'll document that.

quoted

+
+     LOAD_REG_ADDR(r6, decrementer_max);
+     ld      r6,0(r6);
      mtspr   SPRN_HDEC, r6
      /* and set per-LPAR registers, if doing dynamic micro-threading */
      ld      r6, HSTATE_SPLIT_MODE(r13)

@@ -897,7 +902,7 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_207S)
      mftb    r7
      subf    r3,r7,r8
      mtspr   SPRN_DEC,r3
-     stw     r3,VCPU_DEC(r4)
+     std     r3,VCPU_DEC(r4)

      ld      r5, VCPU_SPRG0(r4)
      ld      r6, VCPU_SPRG1(r4)

@@ -953,8 +958,8 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_207S)
      isync

      /* Check if HDEC expires soon */
-     mfspr   r3, SPRN_HDEC
-     cmpwi   r3, 512         /* 1 microsecond */
+     GET_HDEC(r3)
+     cmpdi   r3, 512         /* 1 microsecond */
      blt     hdec_soon

      ld      r6, VCPU_CTR(r4)

@@ -990,8 +995,9 @@ deliver_guest_interrupt:
      beq     5f
      li      r0, BOOK3S_INTERRUPT_EXTERNAL
      bne     cr1, 12f
-     mfspr   r0, SPRN_DEC
-     cmpwi   r0, 0
+
+     GET_DEC(r0)
+     cmpdi   r0, 0

We could just use mfspr DEC here since we are just comparing to 0.  It
should work in any mode.

I'm not sure about that. The result of the comparison is used below:

quoted

      li      r0, BOOK3S_INTERRUPT_DECREMENTER
      bge     5f

It's checking for the DEC overflowing rather than checking if it's zero. If
LD=0 the mfspr result would not be sign extended causing the branch to be
taken even if the DEC overflowed.

Anyway I'm thinking I might drop this patch for now and let Balbir post it
as a part of his KVM series when that's ready.