2022-05-09 11:38:11

by Like Xu

[permalink] [raw]
Subject: [PATCH 2/3] KVM: x86/pmu: Don't pre-set the pmu->global_ctrl when refreshing

From: Like Xu <[email protected]>

Assigning a value to pmu->global_ctrl just to set the value of
pmu->global_ctrl_mask in a more readable way leaves a side effect of
not conforming to the specification. The global_ctrl is reset to zero on
Power up and Reset but keeps unchanged on INIT, like an ordinary MSR.

Signed-off-by: Like Xu <[email protected]>
---
arch/x86/kvm/vmx/pmu_intel.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
index cff03baf8921..4d6cc95bc770 100644
--- a/arch/x86/kvm/vmx/pmu_intel.c
+++ b/arch/x86/kvm/vmx/pmu_intel.c
@@ -525,9 +525,8 @@ static void intel_pmu_refresh(struct kvm_vcpu *vcpu)
setup_fixed_pmc_eventsel(pmu);
}

- pmu->global_ctrl = ((1ull << pmu->nr_arch_gp_counters) - 1) |
+ pmu->global_ctrl_mask = ~((1ull << pmu->nr_arch_gp_counters) - 1) |
(((1ull << pmu->nr_arch_fixed_counters) - 1) << INTEL_PMC_IDX_FIXED);
- pmu->global_ctrl_mask = ~pmu->global_ctrl;
pmu->global_ovf_ctrl_mask = pmu->global_ctrl_mask
& ~(MSR_CORE_PERF_GLOBAL_OVF_CTRL_OVF_BUF |
MSR_CORE_PERF_GLOBAL_OVF_CTRL_COND_CHGD);
--
2.36.1



2022-05-10 11:08:14

by Like Xu

[permalink] [raw]
Subject: [PATCH v2] KVM: x86/pmu: Don't pre-set the pmu->global_ctrl when refreshing

From: Like Xu <[email protected]>

Assigning a value to pmu->global_ctrl just to set the value of
pmu->global_ctrl_mask in a more readable way leaves a side effect of
not conforming to the specification. The value is reset to zero on
Power up and Reset but keeps unchanged on INIT, like an ordinary MSR.

Signed-off-by: Like Xu <[email protected]>
---
v1 -> v2 Changelog:
- Explicitly add parentheses around;

arch/x86/kvm/vmx/pmu_intel.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
index b82b6709d7a8..7829ec457b28 100644
--- a/arch/x86/kvm/vmx/pmu_intel.c
+++ b/arch/x86/kvm/vmx/pmu_intel.c
@@ -522,9 +522,8 @@ static void intel_pmu_refresh(struct kvm_vcpu *vcpu)
setup_fixed_pmc_eventsel(pmu);
}

- pmu->global_ctrl = ((1ull << pmu->nr_arch_gp_counters) - 1) |
- (((1ull << pmu->nr_arch_fixed_counters) - 1) << INTEL_PMC_IDX_FIXED);
- pmu->global_ctrl_mask = ~pmu->global_ctrl;
+ pmu->global_ctrl_mask = ~(((1ull << pmu->nr_arch_gp_counters) - 1) |
+ (((1ull << pmu->nr_arch_fixed_counters) - 1) << INTEL_PMC_IDX_FIXED));
pmu->global_ovf_ctrl_mask = pmu->global_ctrl_mask
& ~(MSR_CORE_PERF_GLOBAL_OVF_CTRL_OVF_BUF |
MSR_CORE_PERF_GLOBAL_OVF_CTRL_COND_CHGD);
--
2.36.1


2022-05-10 11:54:50

by Like Xu

[permalink] [raw]
Subject: Re: [PATCH V2 2/3] KVM: x86/pmu: Don't pre-set the pmu->global_ctrl when refreshing

From: Like Xu <[email protected]>

Assigning a value to pmu->global_ctrl just to set the value of
pmu->global_ctrl_mask in a more readable way leaves a side effect of
not conforming to the specification. The value is reset to zero on
Power up and Reset but keeps unchanged on INIT, like an ordinary MSR.

Signed-off-by: Like Xu <[email protected]>
---
v1 -> v2 Changelog:
- Explicitly add parentheses around;

 arch/x86/kvm/vmx/pmu_intel.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
index cff03baf8921..7945e97db0af 100644
--- a/arch/x86/kvm/vmx/pmu_intel.c
+++ b/arch/x86/kvm/vmx/pmu_intel.c
@@ -525,9 +525,8 @@ static void intel_pmu_refresh(struct kvm_vcpu *vcpu)
         setup_fixed_pmc_eventsel(pmu);
     }

-    pmu->global_ctrl = ((1ull << pmu->nr_arch_gp_counters) - 1) |
-        (((1ull << pmu->nr_arch_fixed_counters) - 1) << INTEL_PMC_IDX_FIXED);
-    pmu->global_ctrl_mask = ~pmu->global_ctrl;
+    pmu->global_ctrl_mask = ~(((1ull << pmu->nr_arch_gp_counters) - 1) |
+        (((1ull << pmu->nr_arch_fixed_counters) - 1) << INTEL_PMC_IDX_FIXED));
     pmu->global_ovf_ctrl_mask = pmu->global_ctrl_mask
             & ~(MSR_CORE_PERF_GLOBAL_OVF_CTRL_OVF_BUF |
                 MSR_CORE_PERF_GLOBAL_OVF_CTRL_COND_CHGD);
--
2.36.1




2022-05-15 10:33:06

by kernel test robot

[permalink] [raw]
Subject: [KVM] db16e9b28b: kvm-unit-tests.pmu.fail



Greeting,

FYI, we noticed the following commit (built with gcc-11):

commit: db16e9b28b05f069d01b76886b683599fd7bbf4e ("[PATCH 2/3] KVM: x86/pmu: Don't pre-set the pmu->global_ctrl when refreshing")
url: https://github.com/intel-lab-lkp/linux/commits/Like-Xu/KVM-x86-pmu-Ignore-pmu-global_ctrl-check-if-vPMU-doesn-t-support-global_ctrl/20220509-182712
base: https://git.kernel.org/cgit/virt/kvm/kvm.git master
patch link: https://lore.kernel.org/kvm/[email protected]

in testcase: kvm-unit-tests
version: kvm-unit-tests-x86_64-1a4529c-1_20220412
with following parameters:

ucode: 0x28



on test machine: 8 threads 1 sockets Intel(R) Core(TM) i7-4790 v3 @ 3.60GHz with 6G memory

caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):




If you fix the issue, kindly add following tag
Reported-by: kernel test robot <[email protected]>

2022-05-14 21:03:37 ./run_tests.sh
...
PASS msr (17 tests)
FAIL pmu
PASS pmu_lbr (3 tests)
PASS pmu_emulation (4 tests)
...


To reproduce:

git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
sudo bin/lkp install job.yaml # job file is attached in this email
bin/lkp split-job --compatible job.yaml # generate the yaml file for lkp run
sudo bin/lkp run generated-yaml-file

# if come across any failure that blocks the test,
# please remove ~/.lkp and /lkp dir to run from a clean state.



--
0-DAY CI Kernel Test Service
https://01.org/lkp



Attachments:
(No filename) (1.58 kB)
config-5.18.0-rc3-00041-gdb16e9b28b05 (168.55 kB)
job-script (5.71 kB)
dmesg.xz (18.17 kB)
kvm-unit-tests (2.83 kB)
job.yaml (4.61 kB)
reproduce (16.00 B)
Download all attachments

2022-05-16 02:16:50

by Like Xu

[permalink] [raw]
Subject: Re: [KVM] db16e9b28b: kvm-unit-tests.pmu.fail

On 15/5/2022 5:22 pm, kernel test robot wrote:
> patch link:https://lore.kernel.org/kvm/[email protected]

Fixed by the V2 version. Please help try.

2022-05-25 01:16:12

by kernel test robot

[permalink] [raw]
Subject: Re: [KVM] db16e9b28b: kvm-unit-tests.pmu.fail

Hi Like Xu,

On Sun, May 15, 2022 at 05:25:21PM +0800, Like Xu wrote:
> On 15/5/2022 5:22 pm, kernel test robot wrote:
> > patch link:https://lore.kernel.org/kvm/[email protected]
>
> Fixed by the V2 version. Please help try.

we tested the V2 version and could not reproduce pmu.fail on it:

=========================================================================================
compiler/kconfig/rootfs/tbox_group/testcase/ucode:
gcc-11/x86_64-rhel-8.3-func/debian-10.4-x86_64-20200603.cgz/lkp-hsw-d02/kvm-unit-tests/0x28

commit:
bec9b596f911 ("KVM: x86/pmu: Ignore pmu->global_ctrl check if vPMU doesn't support global_ctrl")
db16e9b28b05 ("KVM: x86/pmu: Don't pre-set the pmu->global_ctrl when refreshing")
3fa1b82c692b ("KVM: x86/pmu: Don't pre-set the pmu->global_ctrl when refreshing") v2

bec9b596f911252e db16e9b28b05f069d01b76886b6 3fa1b82c692b513878a23e07efc
---------------- --------------------------- ---------------------------
fail:runs %reproduction fail:runs %reproduction fail:runs
| | | | |
:6 50% 3:3 0% :10 kvm-unit-tests.pmu.fail