Excerpts from Michael Ellerman's message of August 6, 2021 11:16 am:
> 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?
Yeah currently I think so. It's not that bad of a bug.
>
> 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.
Good point. Again for now it doesn't really matter because the HFAC
handler turns everything (except msgsndp) into a sigill anyway, so
becomes important when we start using HFACs. Put that way I'll probably
split it out.
>
>> 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?
It is, but you can get a softpatch interrupt on rfebb changing TM state.
Although I haven't actually tested to see if you get a softpatch when
HFSCR disables EBB or the hardware just gives you the HFAC. For that
matter, same for all the other facility tests.
Thanks,
Nick
>
>> }
>> 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
>
