diff mbox series

[v3,10/52] KVM: PPC: Book3S HV: Don't always save PMU for guest capable of nesting

Message ID 20211004160049.1338837-11-npiggin@gmail.com
State New
Headers show
Series KVM: PPC: Book3S HV P9: entry/exit optimisations | expand

Commit Message

Nicholas Piggin Oct. 4, 2021, 4 p.m. UTC
Provide a config option that controls the workaround added by commit
63279eeb7f93 ("KVM: PPC: Book3S HV: Always save guest pmu for guest
capable of nesting"). The option defaults to y for now, but is expected
to go away within a few releases.

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.

After this latest round of performance optimisations, this option costs
about 540 cycles or 10% entry/exit performance on a POWER9 nested-capable
guest.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kvm/Kconfig     | 15 +++++++++++++++
 arch/powerpc/kvm/book3s_hv.c | 10 ++++++++--
 2 files changed, 23 insertions(+), 2 deletions(-)

Comments

Fabiano Rosas Oct. 16, 2021, 12:38 p.m. UTC | #1
Nicholas Piggin <npiggin@gmail.com> writes:

> Provide a config option that controls the workaround added by commit
> 63279eeb7f93 ("KVM: PPC: Book3S HV: Always save guest pmu for guest
> capable of nesting"). The option defaults to y for now, but is expected
> to go away within a few releases.
>
> 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

I think the commit reference is now: 178266389794 (KVM: PPC: Book3S HV
Nested: Reflect guest PMU in-use to L0 when guest SPRs are live)

> in-use status of their guests, which means the parent does not need to
> unconditionally save the PMU for nested capable guests.
>
> After this latest round of performance optimisations, this option costs
> about 540 cycles or 10% entry/exit performance on a POWER9 nested-capable
> guest.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  arch/powerpc/kvm/Kconfig     | 15 +++++++++++++++
>  arch/powerpc/kvm/book3s_hv.c | 10 ++++++++--
>  2 files changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kvm/Kconfig b/arch/powerpc/kvm/Kconfig
> index ff581d70f20c..1e7aae522be8 100644
> --- a/arch/powerpc/kvm/Kconfig
> +++ b/arch/powerpc/kvm/Kconfig
> @@ -130,6 +130,21 @@ config KVM_BOOK3S_HV_EXIT_TIMING
>
>  	  If unsure, say N.
>
> +config KVM_BOOK3S_HV_NESTED_PMU_WORKAROUND
> +	bool "Nested L0 host workaround for L1 KVM host PMU handling bug" if EXPERT
> +	depends on KVM_BOOK3S_HV_POSSIBLE
> +	default !EXPERT
> +	help
> +	  Old nested HV capable Linux guests have a bug where the don't

s/the/they/

Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
Nicholas Piggin Oct. 20, 2021, 5:26 a.m. UTC | #2
Excerpts from Fabiano Rosas's message of October 16, 2021 10:38 pm:
> Nicholas Piggin <npiggin@gmail.com> writes:
> 
>> Provide a config option that controls the workaround added by commit
>> 63279eeb7f93 ("KVM: PPC: Book3S HV: Always save guest pmu for guest
>> capable of nesting"). The option defaults to y for now, but is expected
>> to go away within a few releases.
>>
>> 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
> 
> I think the commit reference is now: 178266389794 (KVM: PPC: Book3S HV
> Nested: Reflect guest PMU in-use to L0 when guest SPRs are live)

Ah yes good point, would be good to that in the changelog. I guess we
can add a References: tag as well, just in case anybody mines this 
stuff.

> 
>> in-use status of their guests, which means the parent does not need to
>> unconditionally save the PMU for nested capable guests.
>>
>> After this latest round of performance optimisations, this option costs
>> about 540 cycles or 10% entry/exit performance on a POWER9 nested-capable
>> guest.
>>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>>  arch/powerpc/kvm/Kconfig     | 15 +++++++++++++++
>>  arch/powerpc/kvm/book3s_hv.c | 10 ++++++++--
>>  2 files changed, 23 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/powerpc/kvm/Kconfig b/arch/powerpc/kvm/Kconfig
>> index ff581d70f20c..1e7aae522be8 100644
>> --- a/arch/powerpc/kvm/Kconfig
>> +++ b/arch/powerpc/kvm/Kconfig
>> @@ -130,6 +130,21 @@ config KVM_BOOK3S_HV_EXIT_TIMING
>>
>>  	  If unsure, say N.
>>
>> +config KVM_BOOK3S_HV_NESTED_PMU_WORKAROUND
>> +	bool "Nested L0 host workaround for L1 KVM host PMU handling bug" if EXPERT
>> +	depends on KVM_BOOK3S_HV_POSSIBLE
>> +	default !EXPERT
>> +	help
>> +	  Old nested HV capable Linux guests have a bug where the don't
> 
> s/the/they/
> 
> Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
> 

Good catch.

Thanks,
Nick
diff mbox series

Patch

diff --git a/arch/powerpc/kvm/Kconfig b/arch/powerpc/kvm/Kconfig
index ff581d70f20c..1e7aae522be8 100644
--- a/arch/powerpc/kvm/Kconfig
+++ b/arch/powerpc/kvm/Kconfig
@@ -130,6 +130,21 @@  config KVM_BOOK3S_HV_EXIT_TIMING
 
 	  If unsure, say N.
 
+config KVM_BOOK3S_HV_NESTED_PMU_WORKAROUND
+	bool "Nested L0 host workaround for L1 KVM host PMU handling bug" if EXPERT
+	depends on KVM_BOOK3S_HV_POSSIBLE
+	default !EXPERT
+	help
+	  Old nested HV capable Linux guests have a bug where the don't
+	  reflect the PMU in-use status of their L2 guest to the L0 host
+	  while the L2 PMU registers are live. This can result in loss
+          of L2 PMU register state, causing perf to not work correctly in
+	  L2 guests.
+
+	  Selecting this option for the L0 host implements a workaround for
+	  those buggy L1s which saves the L2 state, at the cost of performance
+	  in all nested-capable guest entry/exit.
+
 config KVM_BOOKE_HV
 	bool
 
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 463534402107..945fc9a96439 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -4034,8 +4034,14 @@  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);
+	if (IS_ENABLED(CONFIG_KVM_BOOK3S_HV_NESTED_PMU_WORKAROUND)) {
+		/*
+		 * Save pmu if this guest is capable of running nested guests.
+		 * This is option is for old L1s that do not set their
+		 * lppaca->pmcregs_in_use properly when entering their L2.
+		 */
+		save_pmu |= nesting_enabled(vcpu->kvm);
+	}
 
 	kvmhv_save_guest_pmu(vcpu, save_pmu);
 #ifdef CONFIG_PPC_PSERIES