<html><head></head><body dir="auto" style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="ApplePlainTextBody"><div dir="auto" style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="ApplePlainTextBody"><div class="ApplePlainTextBody"><br><br><blockquote type="cite">On 12-Jul-2021, at 8:11 AM, Nicholas Piggin <npiggin@gmail.com> wrote:<br><br>Excerpts from Athira Rajeev's message of July 10, 2021 12:50 pm:<br><blockquote type="cite"><br><br><blockquote type="cite">On 22-Jun-2021, at 4:27 PM, Nicholas Piggin <npiggin@gmail.com> wrote:<br><br>KVM PMU management code looks for particular frozen/disabled bits in<br>the PMU registers so it knows whether it must clear them when coming<br>out of a guest or not. Setting this up helps KVM make these optimisations<br>without getting confused. Longer term the better approach might be to<br>move guest/host PMU switching to the perf subsystem.<br><br>Signed-off-by: Nicholas Piggin <npiggin@gmail.com><br>---<br>arch/powerpc/kernel/cpu_setup_power.c | 4 ++--<br>arch/powerpc/kernel/dt_cpu_ftrs.c | 6 +++---<br>arch/powerpc/kvm/book3s_hv.c | 5 +++++<br>arch/powerpc/perf/core-book3s.c | 7 +++++++<br>4 files changed, 17 insertions(+), 5 deletions(-)<br><br>diff --git a/arch/powerpc/kernel/cpu_setup_power.c b/arch/powerpc/kernel/cpu_setup_power.c<br>index a29dc8326622..3dc61e203f37 100644<br>--- a/arch/powerpc/kernel/cpu_setup_power.c<br>+++ b/arch/powerpc/kernel/cpu_setup_power.c<br>@@ -109,7 +109,7 @@ static void init_PMU_HV_ISA207(void)<br>static void init_PMU(void)<br>{<br><span class="Apple-tab-span" style="white-space:pre"> </span>mtspr(SPRN_MMCRA, 0);<br>-<span class="Apple-tab-span" style="white-space:pre"> </span>mtspr(SPRN_MMCR0, 0);<br>+<span class="Apple-tab-span" style="white-space:pre"> </span>mtspr(SPRN_MMCR0, MMCR0_FC);<br><span class="Apple-tab-span" style="white-space:pre"> </span>mtspr(SPRN_MMCR1, 0);<br><span class="Apple-tab-span" style="white-space:pre"> </span>mtspr(SPRN_MMCR2, 0);<br>}<br>@@ -123,7 +123,7 @@ static void init_PMU_ISA31(void)<br>{<br><span class="Apple-tab-span" style="white-space:pre"> </span>mtspr(SPRN_MMCR3, 0);<br><span class="Apple-tab-span" style="white-space:pre"> </span>mtspr(SPRN_MMCRA, MMCRA_BHRB_DISABLE);<br>-<span class="Apple-tab-span" style="white-space:pre"> </span>mtspr(SPRN_MMCR0, MMCR0_PMCCEXT);<br>+<span class="Apple-tab-span" style="white-space:pre"> </span>mtspr(SPRN_MMCR0, MMCR0_FC | MMCR0_PMCCEXT);<br>}<br><br>/*<br>diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c<br>index 0a6b36b4bda8..06a089fbeaa7 100644<br>--- a/arch/powerpc/kernel/dt_cpu_ftrs.c<br>+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c<br>@@ -353,7 +353,7 @@ static void init_pmu_power8(void)<br><span class="Apple-tab-span" style="white-space:pre"> </span>}<br><br><span class="Apple-tab-span" style="white-space:pre"> </span>mtspr(SPRN_MMCRA, 0);<br>-<span class="Apple-tab-span" style="white-space:pre"> </span>mtspr(SPRN_MMCR0, 0);<br>+<span class="Apple-tab-span" style="white-space:pre"> </span>mtspr(SPRN_MMCR0, MMCR0_FC);<br><span class="Apple-tab-span" style="white-space:pre"> </span>mtspr(SPRN_MMCR1, 0);<br><span class="Apple-tab-span" style="white-space:pre"> </span>mtspr(SPRN_MMCR2, 0);<br><span class="Apple-tab-span" style="white-space:pre"> </span>mtspr(SPRN_MMCRS, 0);<br>@@ -392,7 +392,7 @@ static void init_pmu_power9(void)<br><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>mtspr(SPRN_MMCRC, 0);<br><br><span class="Apple-tab-span" style="white-space:pre"> </span>mtspr(SPRN_MMCRA, 0);<br>-<span class="Apple-tab-span" style="white-space:pre"> </span>mtspr(SPRN_MMCR0, 0);<br>+<span class="Apple-tab-span" style="white-space:pre"> </span>mtspr(SPRN_MMCR0, MMCR0_FC);<br><span class="Apple-tab-span" style="white-space:pre"> </span>mtspr(SPRN_MMCR1, 0);<br><span class="Apple-tab-span" style="white-space:pre"> </span>mtspr(SPRN_MMCR2, 0);<br>}<br>@@ -428,7 +428,7 @@ static void init_pmu_power10(void)<br><br><span class="Apple-tab-span" style="white-space:pre"> </span>mtspr(SPRN_MMCR3, 0);<br><span class="Apple-tab-span" style="white-space:pre"> </span>mtspr(SPRN_MMCRA, MMCRA_BHRB_DISABLE);<br>-<span class="Apple-tab-span" style="white-space:pre"> </span>mtspr(SPRN_MMCR0, MMCR0_PMCCEXT);<br>+<span class="Apple-tab-span" style="white-space:pre"> </span>mtspr(SPRN_MMCR0, MMCR0_FC | MMCR0_PMCCEXT);<br>}<br><br>static int __init feat_enable_pmu_power10(struct dt_cpu_feature *f)<br>diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c<br>index 1f30f98b09d1..f7349d150828 100644<br>--- a/arch/powerpc/kvm/book3s_hv.c<br>+++ b/arch/powerpc/kvm/book3s_hv.c<br>@@ -2593,6 +2593,11 @@ static int kvmppc_core_vcpu_create_hv(struct kvm_vcpu *vcpu)<br>#endif<br>#endif<br><span class="Apple-tab-span" style="white-space:pre"> </span>vcpu->arch.mmcr[0] = MMCR0_FC;<br>+<span class="Apple-tab-span" style="white-space:pre"> </span>if (cpu_has_feature(CPU_FTR_ARCH_31)) {<br>+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>vcpu->arch.mmcr[0] |= MMCR0_PMCCEXT;<br>+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>vcpu->arch.mmcra = MMCRA_BHRB_DISABLE;<br>+<span class="Apple-tab-span" style="white-space:pre"> </span>}<br>+<br><span class="Apple-tab-span" style="white-space:pre"> </span>vcpu->arch.ctrl = CTRL_RUNLATCH;<br><span class="Apple-tab-span" style="white-space:pre"> </span>/* default to host PVR, since we can't spoof it */<br><span class="Apple-tab-span" style="white-space:pre"> </span>kvmppc_set_pvr_hv(vcpu, mfspr(SPRN_PVR));<br>diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c<br>index 51622411a7cc..e33b29ec1a65 100644<br>--- a/arch/powerpc/perf/core-book3s.c<br>+++ b/arch/powerpc/perf/core-book3s.c<br>@@ -1361,6 +1361,13 @@ static void power_pmu_enable(struct pmu *pmu)<br><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>goto out;<br><br><span class="Apple-tab-span" style="white-space:pre"> </span>if (cpuhw->n_events == 0) {<br>+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>if (cpu_has_feature(CPU_FTR_ARCH_31)) {<br>+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>mtspr(SPRN_MMCRA, MMCRA_BHRB_DISABLE);<br>+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>mtspr(SPRN_MMCR0, MMCR0_FC | MMCR0_PMCCEXT);<br>+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>} else {<br>+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>mtspr(SPRN_MMCRA, 0);<br>+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>mtspr(SPRN_MMCR0, MMCR0_FC);<br>+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>}<br></blockquote><br><br>Hi Nick,<br><br>We are setting these bits in “power_pmu_disable” function. And disable will be called before any event gets deleted/stopped. Can you please help to understand why this is needed in power_pmu_enable path also ?<br></blockquote><br>I'll have to go back and check what I needed it for.<br><br>Basically what I'm trying to achieve is that when the guest and host <br>are not running perf, then the registers should match. This way the host <br>can test them with mfspr but does not need to fix them with mtspr.<br><br>It's not too important for performance after TM facility demand faulting<br>and the nested guest PMU fix means you can usually just skip that <br>entirely, but I think it's cleaner. I'll double check my perf/ changes<br>it's possible that part should be split out or is unnecessary.<br><br></blockquote><br>Sure Nick<br><br>power_pmu_disable already sets MMCRA_BHRB_DISABLE/MMCR0_FC/MMCR0_PMCCEXT bits.<br>Hence trying to understand in which scenario we found this was needed for power_pmu_enable.<br><br>Thanks<br>Athira<br><br><blockquote type="cite">Thanks,<br>Nick<br></blockquote><br></div></div></body></html>