Excerpts from Fabiano Rosas's message of July 27, 2021 6:17 am: > If the nested hypervisor has no access to a facility because it has > been disabled by the host, it should also not be able to see the > Hypervisor Facility Unavailable that arises from one of its guests > trying to access the facility. > > This patch turns a HFU that happened in L2 into a Hypervisor Emulation > Assistance interrupt and forwards it to L1 for handling. The ones that > happened because L1 explicitly disabled the facility for L2 are still > let through, along with the corresponding Cause bits in the HFSCR. > > Signed-off-by: Fabiano Rosas <farosas@xxxxxxxxxxxxx> > Reviewed-by: Nicholas Piggin <npiggin@xxxxxxxxx> > --- > arch/powerpc/kvm/book3s_hv_nested.c | 32 +++++++++++++++++++++++------ > 1 file changed, 26 insertions(+), 6 deletions(-) > > diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c > index 8215dbd4be9a..d544b092b49a 100644 > --- a/arch/powerpc/kvm/book3s_hv_nested.c > +++ b/arch/powerpc/kvm/book3s_hv_nested.c > @@ -99,7 +99,7 @@ static void byteswap_hv_regs(struct hv_guest_state *hr) > hr->dawrx1 = swab64(hr->dawrx1); > } > > -static void save_hv_return_state(struct kvm_vcpu *vcpu, int trap, > +static void save_hv_return_state(struct kvm_vcpu *vcpu, > struct hv_guest_state *hr) > { > struct kvmppc_vcore *vc = vcpu->arch.vcore; > @@ -118,7 +118,7 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu, int trap, > hr->pidr = vcpu->arch.pid; > hr->cfar = vcpu->arch.cfar; > hr->ppr = vcpu->arch.ppr; > - switch (trap) { > + switch (vcpu->arch.trap) { > case BOOK3S_INTERRUPT_H_DATA_STORAGE: > hr->hdar = vcpu->arch.fault_dar; > hr->hdsisr = vcpu->arch.fault_dsisr; > @@ -128,9 +128,29 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu, int trap, > hr->asdr = vcpu->arch.fault_gpa; > break; > case BOOK3S_INTERRUPT_H_FAC_UNAVAIL: > - hr->hfscr = ((~HFSCR_INTR_CAUSE & hr->hfscr) | > - (HFSCR_INTR_CAUSE & vcpu->arch.hfscr)); > - break; > + { > + u8 cause = vcpu->arch.hfscr >> 56; Can this be u64 just to help gcc? > + > + WARN_ON_ONCE(cause >= BITS_PER_LONG); > + > + if (!(hr->hfscr & (1UL << cause))) { > + hr->hfscr = ((~HFSCR_INTR_CAUSE & hr->hfscr) | > + (HFSCR_INTR_CAUSE & vcpu->arch.hfscr)); > + break; > + } > + > + /* > + * We have disabled this facility, so it does not > + * exist from L1's perspective. Turn it into a HEAI. > + */ > + vcpu->arch.trap = BOOK3S_INTERRUPT_H_EMUL_ASSIST; > + kvmppc_load_last_inst(vcpu, INST_GENERIC, &vcpu->arch.emul_inst); Hmm, this doesn't handle kvmpc_load_last_inst failure. Other code tends to just resume guest and retry in this case. Can we do that here? > + > + /* Don't leak the cause field */ > + hr->hfscr &= ~HFSCR_INTR_CAUSE; This hunk also remains -- shouldn't change HFSCR for HEA, only HFAC. Thanks, Nick