2015-08-07 19:53:36

by Wei Huang

[permalink] [raw]
Subject: [KVM x86 vPMU Patch 0/2] Two vPMU Trivial Patches

These two trivial patches are related to x86 vPMU code. They were
actually suggested by Andrew Jones while he was reviewing the last
big vPMU patch set.

These patches have been compiled and tested on AMD system using
a 64-bit guest VM with various perf commands (e.g. bench, test, top,
stat). No obvious problems were found.

Thanks,
-Wei

Wei Huang (2):
KVM: x86/vPMU: Move the definition of kvm_pmu_ops to arch-specific
files
KVM: x86/vPMU: Fix unnecessary signed extesion for AMD PERFCTRn

arch/x86/kvm/pmu.h | 2 --
arch/x86/kvm/pmu_amd.c | 2 --
arch/x86/kvm/svm.c | 1 +
arch/x86/kvm/vmx.c | 1 +
4 files changed, 2 insertions(+), 4 deletions(-)

--
1.8.3.1


2015-08-07 19:54:00

by Wei Huang

[permalink] [raw]
Subject: [KVM x86 vPMU Patch 1/2] KVM: x86/vPMU: Move the definition of kvm_pmu_ops to arch-specific files

Instead of being defined in a common header file, the kvm_pmu_ops struct
is arch (vmx/svm) specific. This trivial patch relocates two extern
variable definition to their arch-specific files.

Signed-off-by: Wei Huang <[email protected]>
---
arch/x86/kvm/pmu.h | 2 --
arch/x86/kvm/svm.c | 1 +
arch/x86/kvm/vmx.c | 1 +
3 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/pmu.h b/arch/x86/kvm/pmu.h
index f96e1f9..95184fd 100644
--- a/arch/x86/kvm/pmu.h
+++ b/arch/x86/kvm/pmu.h
@@ -113,6 +113,4 @@ void kvm_pmu_reset(struct kvm_vcpu *vcpu);
void kvm_pmu_init(struct kvm_vcpu *vcpu);
void kvm_pmu_destroy(struct kvm_vcpu *vcpu);

-extern struct kvm_pmu_ops intel_pmu_ops;
-extern struct kvm_pmu_ops amd_pmu_ops;
#endif /* __KVM_X86_PMU_H */
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 8e0c084..8abf980 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -4452,6 +4452,7 @@ static void svm_sched_in(struct kvm_vcpu *vcpu, int cpu)
{
}

+extern struct kvm_pmu_ops amd_pmu_ops;
static struct kvm_x86_ops svm_x86_ops = {
.cpu_has_kvm_support = has_svm,
.disabled_by_bios = is_disabled,
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 83b7b5c..6b2419d 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -10302,6 +10302,7 @@ static void vmx_enable_log_dirty_pt_masked(struct kvm *kvm,
kvm_mmu_clear_dirty_pt_masked(kvm, memslot, offset, mask);
}

+extern struct kvm_pmu_ops intel_pmu_ops;
static struct kvm_x86_ops vmx_x86_ops = {
.cpu_has_kvm_support = cpu_has_kvm_support,
.disabled_by_bios = vmx_disabled_by_bios,
--
1.8.3.1

2015-08-07 19:53:38

by Wei Huang

[permalink] [raw]
Subject: [KVM x86 vPMU Patch 2/2] KVM: x86/vPMU: Fix unnecessary signed extension for AMD PERFCTRn

According to AMD programmer's manual, AMD PERFCTRn is 64-bit MSR which,
unlike Intel perf counters, doesn't require signed extension. This
patch removes the unnecessary conversion in SVM vPMU code when PERFCTRn
is being updated.

Signed-off-by: Wei Huang <[email protected]>
---
arch/x86/kvm/pmu_amd.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/arch/x86/kvm/pmu_amd.c b/arch/x86/kvm/pmu_amd.c
index 886aa25..39b9112 100644
--- a/arch/x86/kvm/pmu_amd.c
+++ b/arch/x86/kvm/pmu_amd.c
@@ -133,8 +133,6 @@ static int amd_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
/* MSR_K7_PERFCTRn */
pmc = get_gp_pmc(pmu, msr, MSR_K7_PERFCTR0);
if (pmc) {
- if (!msr_info->host_initiated)
- data = (s64)data;
pmc->counter += data - pmc_read_counter(pmc);
return 0;
}
--
1.8.3.1

2015-08-11 07:32:58

by Andrew Jones

[permalink] [raw]
Subject: Re: [KVM x86 vPMU Patch 1/2] KVM: x86/vPMU: Move the definition of kvm_pmu_ops to arch-specific files

On Fri, Aug 07, 2015 at 03:53:29PM -0400, Wei Huang wrote:
> Instead of being defined in a common header file, the kvm_pmu_ops struct
> is arch (vmx/svm) specific. This trivial patch relocates two extern
> variable definition to their arch-specific files.
>
> Signed-off-by: Wei Huang <[email protected]>
> ---
> arch/x86/kvm/pmu.h | 2 --
> arch/x86/kvm/svm.c | 1 +
> arch/x86/kvm/vmx.c | 1 +
> 3 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/pmu.h b/arch/x86/kvm/pmu.h
> index f96e1f9..95184fd 100644
> --- a/arch/x86/kvm/pmu.h
> +++ b/arch/x86/kvm/pmu.h
> @@ -113,6 +113,4 @@ void kvm_pmu_reset(struct kvm_vcpu *vcpu);
> void kvm_pmu_init(struct kvm_vcpu *vcpu);
> void kvm_pmu_destroy(struct kvm_vcpu *vcpu);
>
> -extern struct kvm_pmu_ops intel_pmu_ops;
> -extern struct kvm_pmu_ops amd_pmu_ops;
> #endif /* __KVM_X86_PMU_H */
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 8e0c084..8abf980 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -4452,6 +4452,7 @@ static void svm_sched_in(struct kvm_vcpu *vcpu, int cpu)
> {
> }
>
> +extern struct kvm_pmu_ops amd_pmu_ops;
> static struct kvm_x86_ops svm_x86_ops = {
> .cpu_has_kvm_support = has_svm,
> .disabled_by_bios = is_disabled,
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 83b7b5c..6b2419d 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -10302,6 +10302,7 @@ static void vmx_enable_log_dirty_pt_masked(struct kvm *kvm,
> kvm_mmu_clear_dirty_pt_masked(kvm, memslot, offset, mask);
> }
>
> +extern struct kvm_pmu_ops intel_pmu_ops;
> static struct kvm_x86_ops vmx_x86_ops = {
> .cpu_has_kvm_support = cpu_has_kvm_support,
> .disabled_by_bios = vmx_disabled_by_bios,
> --
> 1.8.3.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

Reviewed-by: Andrew Jones <[email protected]>

2015-08-11 07:33:27

by Andrew Jones

[permalink] [raw]
Subject: Re: [KVM x86 vPMU Patch 2/2] KVM: x86/vPMU: Fix unnecessary signed extension for AMD PERFCTRn

On Fri, Aug 07, 2015 at 03:53:30PM -0400, Wei Huang wrote:
> According to AMD programmer's manual, AMD PERFCTRn is 64-bit MSR which,
> unlike Intel perf counters, doesn't require signed extension. This
> patch removes the unnecessary conversion in SVM vPMU code when PERFCTRn
> is being updated.
>
> Signed-off-by: Wei Huang <[email protected]>
> ---
> arch/x86/kvm/pmu_amd.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/arch/x86/kvm/pmu_amd.c b/arch/x86/kvm/pmu_amd.c
> index 886aa25..39b9112 100644
> --- a/arch/x86/kvm/pmu_amd.c
> +++ b/arch/x86/kvm/pmu_amd.c
> @@ -133,8 +133,6 @@ static int amd_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> /* MSR_K7_PERFCTRn */
> pmc = get_gp_pmc(pmu, msr, MSR_K7_PERFCTR0);
> if (pmc) {
> - if (!msr_info->host_initiated)
> - data = (s64)data;
> pmc->counter += data - pmc_read_counter(pmc);
> return 0;
> }
> --
> 1.8.3.1

Reviewed-by: Andrew Jones <[email protected]>

2015-08-11 13:21:26

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [KVM x86 vPMU Patch 0/2] Two vPMU Trivial Patches



On 07/08/2015 21:53, Wei Huang wrote:
> These two trivial patches are related to x86 vPMU code. They were
> actually suggested by Andrew Jones while he was reviewing the last
> big vPMU patch set.
>
> These patches have been compiled and tested on AMD system using
> a 64-bit guest VM with various perf commands (e.g. bench, test, top,
> stat). No obvious problems were found.
>
> Thanks,
> -Wei
>
> Wei Huang (2):
> KVM: x86/vPMU: Move the definition of kvm_pmu_ops to arch-specific
> files
> KVM: x86/vPMU: Fix unnecessary signed extesion for AMD PERFCTRn
>
> arch/x86/kvm/pmu.h | 2 --
> arch/x86/kvm/pmu_amd.c | 2 --
> arch/x86/kvm/svm.c | 1 +
> arch/x86/kvm/vmx.c | 1 +
> 4 files changed, 2 insertions(+), 4 deletions(-)
>

Applied patch 2. For patch 1 I'm not sure, because I do not really like
1) externs in .c files; 2) globals with no declarations in a .h file.
So I'm leaving it out while I think more about it.

Paolo

2015-08-11 14:52:57

by Wei Huang

[permalink] [raw]
Subject: Re: [KVM x86 vPMU Patch 0/2] Two vPMU Trivial Patches



On 8/11/15 08:21, Paolo Bonzini wrote:
>
>
> On 07/08/2015 21:53, Wei Huang wrote:
>> These two trivial patches are related to x86 vPMU code. They were
>> actually suggested by Andrew Jones while he was reviewing the last
>> big vPMU patch set.
>>
>> These patches have been compiled and tested on AMD system using
>> a 64-bit guest VM with various perf commands (e.g. bench, test, top,
>> stat). No obvious problems were found.
>>
>> Thanks,
>> -Wei
>>
>> Wei Huang (2):
>> KVM: x86/vPMU: Move the definition of kvm_pmu_ops to arch-specific
>> files
>> KVM: x86/vPMU: Fix unnecessary signed extesion for AMD PERFCTRn
>>
>> arch/x86/kvm/pmu.h | 2 --
>> arch/x86/kvm/pmu_amd.c | 2 --
>> arch/x86/kvm/svm.c | 1 +
>> arch/x86/kvm/vmx.c | 1 +
>> 4 files changed, 2 insertions(+), 4 deletions(-)
>>
>
> Applied patch 2. For patch 1 I'm not sure, because I do not really like
> 1) externs in .c files; 2) globals with no declarations in a .h file.
> So I'm leaving it out while I think more about it.
Thanks. The first one is minor anyway. I won't complain about it. :-)

-Wei

>
> Paolo
>