Nicholas Piggin <npiggin@xxxxxxxxx> writes:
> The softpatch interrupt sets HSRR0 to the faulting instruction +4, so
> it should subtract 4 for the faulting instruction address. Also have it
> emulate and deliver HFAC interrupts correctly, which is important for
> nested HV and facility demand-faulting in future.
The nip being off by 4 sounds bad. But I guess it's not that big a deal
because it's only used for reporting the instruction address?
Would also be good to have some more explanation of why it's OK to
change from illegal to HFAC, which is a guest visible change.
> diff --git a/arch/powerpc/kvm/book3s_hv_tm.c b/arch/powerpc/kvm/book3s_hv_tm.c
> index cc90b8b82329..e4fd4a9dee08 100644
> --- a/arch/powerpc/kvm/book3s_hv_tm.c
> +++ b/arch/powerpc/kvm/book3s_hv_tm.c
> @@ -74,19 +74,23 @@ int kvmhv_p9_tm_emulation(struct kvm_vcpu *vcpu)
> case PPC_INST_RFEBB:
> if ((msr & MSR_PR) && (vcpu->arch.vcore->pcr & PCR_ARCH_206)) {
> /* generate an illegal instruction interrupt */
> + vcpu->arch.regs.nip -= 4;
> kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
> return RESUME_GUEST;
> }
> /* check EBB facility is available */
> if (!(vcpu->arch.hfscr & HFSCR_EBB)) {
> - /* generate an illegal instruction interrupt */
> - kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
> - return RESUME_GUEST;
> + vcpu->arch.regs.nip -= 4;
> + vcpu->arch.hfscr &= ~HFSCR_INTR_CAUSE;
> + vcpu->arch.hfscr |= (u64)FSCR_EBB_LG << 56;
> + vcpu->arch.trap = BOOK3S_INTERRUPT_H_FAC_UNAVAIL;
> + return -1; /* rerun host interrupt handler */
This is EBB not TM. Probably OK to leave it in this patch as long as
it's mentioned in the change log?
> }
> if ((msr & MSR_PR) && !(vcpu->arch.fscr & FSCR_EBB)) {
> /* generate a facility unavailable interrupt */
> - vcpu->arch.fscr = (vcpu->arch.fscr & ~(0xffull << 56)) |
> - ((u64)FSCR_EBB_LG << 56);
> + vcpu->arch.regs.nip -= 4;
> + vcpu->arch.fscr &= ~FSCR_INTR_CAUSE;
> + vcpu->arch.fscr |= (u64)FSCR_EBB_LG << 56;
Same.
cheers
