KVM: PPC: Fetch prefixed instructions from the guest
authorPaul Mackerras <paulus@ozlabs.org>
Wed, 8 Mar 2023 06:35:23 +0000 (17:35 +1100)
committerMichael Ellerman <mpe@ellerman.id.au>
Mon, 3 Apr 2023 05:45:50 +0000 (15:45 +1000)
In order to handle emulation of prefixed instructions in the guest,
this first makes vcpu->arch.last_inst be an unsigned long, i.e. 64
bits on 64-bit platforms.  For prefixed instructions, the upper 32
bits are used for the prefix and the lower 32 bits for the suffix, and
both halves are byte-swapped if the guest endianness differs from the
host.

Next, vcpu->arch.emul_inst is now 64 bits wide, to match the HEIR
register on POWER10.  Like HEIR, for a prefixed instruction it is
defined to have the prefix is in the top 32 bits and the suffix in the
bottom 32 bits, with both halves in the correct byte order.

kvmppc_get_last_inst is extended on 64-bit machines to put the prefix
and suffix in the right places in the ppc_inst_t being returned.

kvmppc_load_last_inst now returns the instruction in an unsigned long
in the same format as vcpu->arch.last_inst.  It makes the decision
about whether to fetch a suffix based on the SRR1_PREFIXED bit in the
MSR image stored in the vcpu struct, which generally comes from SRR1
or HSRR1 on an interrupt.  This bit is defined in Power ISA v3.1B to
be set if the interrupt occurred due to a prefixed instruction and
cleared otherwise for all interrupts except for instruction storage
interrupt, which does not come to the hypervisor.  It is set to zero
for asynchronous interrupts such as external interrupts.  In previous
ISA versions it was always set to 0 for all interrupts except
instruction storage interrupt.

The code in book3s_hv_rmhandlers.S that loads the faulting instruction
on a HDSI is only used on POWER8 and therefore doesn't ever need to
load a suffix.

[npiggin@gmail.com - check that the is-prefixed bit in SRR1 matches the
type of instruction that was fetched.]

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Tested-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://msgid.link/ZAgsq9h1CCzouQuV@cleo
arch/powerpc/include/asm/kvm_host.h
arch/powerpc/include/asm/kvm_ppc.h
arch/powerpc/kvm/book3s.c
arch/powerpc/kvm/book3s_64_mmu_hv.c
arch/powerpc/kvm/book3s_hv.c
arch/powerpc/kvm/book3s_hv_rmhandlers.S
arch/powerpc/kvm/booke.c
arch/powerpc/kvm/bookehv_interrupts.S
arch/powerpc/kvm/e500_mmu_host.c
arch/powerpc/kvm/emulate.c
arch/powerpc/kvm/emulate_loadstore.c

index 959f566..14ee0de 100644 (file)
@@ -758,7 +758,7 @@ struct kvm_vcpu_arch {
        u8 prodded;
        u8 doorbell_request;
        u8 irq_pending; /* Used by XIVE to signal pending guest irqs */
-       u32 last_inst;
+       unsigned long last_inst;
 
        struct rcuwait wait;
        struct rcuwait *waitp;
@@ -818,7 +818,7 @@ struct kvm_vcpu_arch {
        u64 busy_stolen;
        u64 busy_preempt;
 
-       u32 emul_inst;
+       u64 emul_inst;
 
        u32 online;
 
index d703d1b..bc57d05 100644 (file)
@@ -85,7 +85,8 @@ extern int kvmppc_handle_vsx_store(struct kvm_vcpu *vcpu,
                                int is_default_endian);
 
 extern int kvmppc_load_last_inst(struct kvm_vcpu *vcpu,
-                                enum instruction_fetch_type type, u32 *inst);
+                                enum instruction_fetch_type type,
+                                unsigned long *inst);
 
 extern int kvmppc_ld(struct kvm_vcpu *vcpu, ulong *eaddr, int size, void *ptr,
                     bool data);
@@ -336,15 +337,30 @@ static inline int kvmppc_get_last_inst(struct kvm_vcpu *vcpu,
                ret = kvmppc_load_last_inst(vcpu, type, &vcpu->arch.last_inst);
 
        /*  Write fetch_failed unswapped if the fetch failed */
-       if (ret == EMULATE_DONE)
-               fetched_inst = kvmppc_need_byteswap(vcpu) ?
-                               swab32(vcpu->arch.last_inst) :
-                               vcpu->arch.last_inst;
-       else
-               fetched_inst = vcpu->arch.last_inst;
+       if (ret != EMULATE_DONE) {
+               *inst = ppc_inst(KVM_INST_FETCH_FAILED);
+               return ret;
+       }
+
+#ifdef CONFIG_PPC64
+       /* Is this a prefixed instruction? */
+       if ((vcpu->arch.last_inst >> 32) != 0) {
+               u32 prefix = vcpu->arch.last_inst >> 32;
+               u32 suffix = vcpu->arch.last_inst;
+               if (kvmppc_need_byteswap(vcpu)) {
+                       prefix = swab32(prefix);
+                       suffix = swab32(suffix);
+               }
+               *inst = ppc_inst_prefix(prefix, suffix);
+               return EMULATE_DONE;
+       }
+#endif
 
+       fetched_inst = kvmppc_need_byteswap(vcpu) ?
+               swab32(vcpu->arch.last_inst) :
+               vcpu->arch.last_inst;
        *inst = ppc_inst(fetched_inst);
-       return ret;
+       return EMULATE_DONE;
 }
 
 static inline bool is_kvmppc_hv_enabled(struct kvm *kvm)
index fa6ac24..686d8d9 100644 (file)
@@ -481,20 +481,42 @@ int kvmppc_xlate(struct kvm_vcpu *vcpu, ulong eaddr, enum xlate_instdata xlid,
        return r;
 }
 
+/*
+ * Returns prefixed instructions with the prefix in the high 32 bits
+ * of *inst and suffix in the low 32 bits.  This is the same convention
+ * as used in HEIR, vcpu->arch.last_inst and vcpu->arch.emul_inst.
+ * Like vcpu->arch.last_inst but unlike vcpu->arch.emul_inst, each
+ * half of the value needs byte-swapping if the guest endianness is
+ * different from the host endianness.
+ */
 int kvmppc_load_last_inst(struct kvm_vcpu *vcpu,
-               enum instruction_fetch_type type, u32 *inst)
+               enum instruction_fetch_type type, unsigned long *inst)
 {
        ulong pc = kvmppc_get_pc(vcpu);
        int r;
+       u32 iw;
 
        if (type == INST_SC)
                pc -= 4;
 
-       r = kvmppc_ld(vcpu, &pc, sizeof(u32), inst, false);
-       if (r == EMULATE_DONE)
-               return r;
-       else
+       r = kvmppc_ld(vcpu, &pc, sizeof(u32), &iw, false);
+       if (r != EMULATE_DONE)
                return EMULATE_AGAIN;
+       /*
+        * If [H]SRR1 indicates that the instruction that caused the
+        * current interrupt is a prefixed instruction, get the suffix.
+        */
+       if (kvmppc_get_msr(vcpu) & SRR1_PREFIXED) {
+               u32 suffix;
+               pc += 4;
+               r = kvmppc_ld(vcpu, &pc, sizeof(u32), &suffix, false);
+               if (r != EMULATE_DONE)
+                       return EMULATE_AGAIN;
+               *inst = ((u64)iw << 32) | suffix;
+       } else {
+               *inst = iw;
+       }
+       return r;
 }
 EXPORT_SYMBOL_GPL(kvmppc_load_last_inst);
 
index 0be313e..af1f060 100644 (file)
@@ -433,6 +433,7 @@ int kvmppc_hv_emulate_mmio(struct kvm_vcpu *vcpu,
                           unsigned long gpa, gva_t ea, int is_store)
 {
        ppc_inst_t last_inst;
+       bool is_prefixed = !!(kvmppc_get_msr(vcpu) & SRR1_PREFIXED);
 
        /*
         * Fast path - check if the guest physical address corresponds to a
@@ -447,7 +448,7 @@ int kvmppc_hv_emulate_mmio(struct kvm_vcpu *vcpu,
                                       NULL);
                srcu_read_unlock(&vcpu->kvm->srcu, idx);
                if (!ret) {
-                       kvmppc_set_pc(vcpu, kvmppc_get_pc(vcpu) + 4);
+                       kvmppc_set_pc(vcpu, kvmppc_get_pc(vcpu) + (is_prefixed ? 8 : 4));
                        return RESUME_GUEST;
                }
        }
@@ -462,7 +463,16 @@ int kvmppc_hv_emulate_mmio(struct kvm_vcpu *vcpu,
        /*
         * WARNING: We do not know for sure whether the instruction we just
         * read from memory is the same that caused the fault in the first
-        * place.  If the instruction we read is neither an load or a store,
+        * place.
+        *
+        * If the fault is prefixed but the instruction is not or vice
+        * versa, try again so that we don't advance pc the wrong amount.
+        */
+       if (ppc_inst_prefixed(last_inst) != is_prefixed)
+               return RESUME_GUEST;
+
+       /*
+        * If the instruction we read is neither an load or a store,
         * then it can't access memory, so we don't need to worry about
         * enforcing access permissions.  So, assuming it is a load or
         * store, we just check that its direction (load or store) is
index 6a573bc..283b9bc 100644 (file)
@@ -474,7 +474,7 @@ static void kvmppc_dump_regs(struct kvm_vcpu *vcpu)
        for (r = 0; r < vcpu->arch.slb_max; ++r)
                pr_err("  ESID = %.16llx VSID = %.16llx\n",
                       vcpu->arch.slb[r].orige, vcpu->arch.slb[r].origv);
-       pr_err("lpcr = %.16lx sdr1 = %.16lx last_inst = %.8x\n",
+       pr_err("lpcr = %.16lx sdr1 = %.16lx last_inst = %.16lx\n",
               vcpu->arch.vcore->lpcr, vcpu->kvm->arch.sdr1,
               vcpu->arch.last_inst);
 }
index 0a97811..800892d 100644 (file)
@@ -1071,11 +1071,11 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
        /* Save HEIR (HV emulation assist reg) in emul_inst
           if this is an HEI (HV emulation interrupt, e40) */
        li      r3,KVM_INST_FETCH_FAILED
-       stw     r3,VCPU_LAST_INST(r9)
+       std     r3,VCPU_LAST_INST(r9)
        cmpwi   r12,BOOK3S_INTERRUPT_H_EMUL_ASSIST
        bne     11f
        mfspr   r3,SPRN_HEIR
-11:    stw     r3,VCPU_HEIR(r9)
+11:    std     r3,VCPU_HEIR(r9)
 
        /* these are volatile across C function calls */
        mfctr   r3
@@ -1676,7 +1676,7 @@ fast_interrupt_c_return:
        mtmsrd  r3
 
        /* Store the result */
-       stw     r8, VCPU_LAST_INST(r9)
+       std     r8, VCPU_LAST_INST(r9)
 
        /* Unset guest mode. */
        li      r0, KVM_GUEST_MODE_HOST_HV
index 7e1d9d7..6a5be02 100644 (file)
@@ -844,7 +844,7 @@ static int emulation_exit(struct kvm_vcpu *vcpu)
                return RESUME_GUEST;
 
        case EMULATE_FAIL:
-               printk(KERN_CRIT "%s: emulation at %lx failed (%08x)\n",
+               printk(KERN_CRIT "%s: emulation at %lx failed (%08lx)\n",
                       __func__, vcpu->arch.regs.nip, vcpu->arch.last_inst);
                /* For debugging, encode the failing instruction and
                 * report it to userspace. */
index b5fe6fb..8b4a402 100644 (file)
@@ -139,7 +139,7 @@ END_BTB_FLUSH_SECTION
         * kvmppc_get_last_inst().
         */
        li      r9, KVM_INST_FETCH_FAILED
-       stw     r9, VCPU_LAST_INST(r4)
+       PPC_STL r9, VCPU_LAST_INST(r4)
        .endif
 
        .if     \flags & NEED_ESR
index 05668e9..ccb8f16 100644 (file)
@@ -623,7 +623,7 @@ void kvmppc_mmu_map(struct kvm_vcpu *vcpu, u64 eaddr, gpa_t gpaddr,
 
 #ifdef CONFIG_KVM_BOOKE_HV
 int kvmppc_load_last_inst(struct kvm_vcpu *vcpu,
-               enum instruction_fetch_type type, u32 *instr)
+               enum instruction_fetch_type type, unsigned long *instr)
 {
        gva_t geaddr;
        hpa_t addr;
@@ -713,7 +713,7 @@ int kvmppc_load_last_inst(struct kvm_vcpu *vcpu,
 }
 #else
 int kvmppc_load_last_inst(struct kvm_vcpu *vcpu,
-               enum instruction_fetch_type type, u32 *instr)
+               enum instruction_fetch_type type, unsigned long *instr)
 {
        return EMULATE_AGAIN;
 }
index 2a51d5b..355d520 100644 (file)
@@ -301,6 +301,10 @@ int kvmppc_emulate_instruction(struct kvm_vcpu *vcpu)
        trace_kvm_ppc_instr(inst, kvmppc_get_pc(vcpu), emulated);
 
        /* Advance past emulated instruction. */
+       /*
+        * If this ever handles prefixed instructions, the 4
+        * will need to become ppc_inst_len(pinst) instead.
+        */
        if (advance)
                kvmppc_set_pc(vcpu, kvmppc_get_pc(vcpu) + 4);
 
index 64b93eb..059c08a 100644 (file)
@@ -360,7 +360,7 @@ int kvmppc_emulate_loadstore(struct kvm_vcpu *vcpu)
 
        /* Advance past emulated instruction. */
        if (emulated != EMULATE_FAIL)
-               kvmppc_set_pc(vcpu, kvmppc_get_pc(vcpu) + 4);
+               kvmppc_set_pc(vcpu, kvmppc_get_pc(vcpu) + ppc_inst_len(inst));
 
        return emulated;
 }