| Message ID | 20210726035036.739609-15-npiggin@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1,01/55] KVM: PPC: Book3S HV: Remove TM emulation from POWER7/8 path | expand |
Nicholas Piggin <npiggin@gmail.com> writes: > Revert the workaround added by commit 63279eeb7f93a ("KVM: PPC: Book3S > HV: Always save guest pmu for guest capable of nesting"). > > Nested capable guests running with the earlier commit ("KVM: PPC: Book3S > HV Nested: Indicate guest PMU in-use in VPA") will now indicate the PMU > in-use status of their guests, which means the parent does not need to > unconditionally save the PMU for nested capable guests. > > This will cause the PMU to break for nested guests when running older > nested hypervisor guests under a kernel with this change. It's unclear > there's an easy way to avoid that, so this could wait for a release or > so for the fix to filter into stable kernels. I'm not sure PMU inside nested guests is getting much use, but I don't think we can break this quite so casually :) Especially as the failure mode will be PMU counts that don't match reality, which is hard to diagnose. It took nearly a year for us to find the original bug. I think we need to hold this back for a while. We could put it under a CONFIG option, and then flip that option to off at some point in the future. cheers > index e7f8cc04944b..ab89db561c85 100644 > --- a/arch/powerpc/kvm/book3s_hv.c > +++ b/arch/powerpc/kvm/book3s_hv.c > @@ -4003,8 +4003,6 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit, > vcpu->arch.vpa.dirty = 1; > save_pmu = lp->pmcregs_in_use; > } > - /* Must save pmu if this guest is capable of running nested guests */ > - save_pmu |= nesting_enabled(vcpu->kvm); > > kvmhv_save_guest_pmu(vcpu, save_pmu); > #ifdef CONFIG_PPC_PSERIES > -- > 2.23.0
Excerpts from Michael Ellerman's message of August 6, 2021 5:34 pm: > Nicholas Piggin <npiggin@gmail.com> writes: >> Revert the workaround added by commit 63279eeb7f93a ("KVM: PPC: Book3S >> HV: Always save guest pmu for guest capable of nesting"). >> >> Nested capable guests running with the earlier commit ("KVM: PPC: Book3S >> HV Nested: Indicate guest PMU in-use in VPA") will now indicate the PMU >> in-use status of their guests, which means the parent does not need to >> unconditionally save the PMU for nested capable guests. >> >> This will cause the PMU to break for nested guests when running older >> nested hypervisor guests under a kernel with this change. It's unclear >> there's an easy way to avoid that, so this could wait for a release or >> so for the fix to filter into stable kernels. > > I'm not sure PMU inside nested guests is getting much use, but I don't > think we can break this quite so casually :) > > Especially as the failure mode will be PMU counts that don't match > reality, which is hard to diagnose. It took nearly a year for us to find > the original bug. > > I think we need to hold this back for a while. > > We could put it under a CONFIG option, and then flip that option to off > at some point in the future. Okay that might be a good compromise, I'll do that. Thanks, Nick
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c index e7f8cc04944b..ab89db561c85 100644 --- a/arch/powerpc/kvm/book3s_hv.c +++ b/arch/powerpc/kvm/book3s_hv.c @@ -4003,8 +4003,6 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit, vcpu->arch.vpa.dirty = 1; save_pmu = lp->pmcregs_in_use; } - /* Must save pmu if this guest is capable of running nested guests */ - save_pmu |= nesting_enabled(vcpu->kvm); kvmhv_save_guest_pmu(vcpu, save_pmu); #ifdef CONFIG_PPC_PSERIES
Revert the workaround added by commit 63279eeb7f93a ("KVM: PPC: Book3S HV: Always save guest pmu for guest capable of nesting"). Nested capable guests running with the earlier commit ("KVM: PPC: Book3S HV Nested: Indicate guest PMU in-use in VPA") will now indicate the PMU in-use status of their guests, which means the parent does not need to unconditionally save the PMU for nested capable guests. This will cause the PMU to break for nested guests when running older nested hypervisor guests under a kernel with this change. It's unclear there's an easy way to avoid that, so this could wait for a release or so for the fix to filter into stable kernels. -134 cycles (8982) POWER9 virt-mode NULL hcall Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- arch/powerpc/kvm/book3s_hv.c | 2 -- 1 file changed, 2 deletions(-)