[KVM x86 vPMU Patch 0/2] Two vPMU Trivial Patches

[KVM x86 vPMU Patch 0/2] Two vPMU Trivial Patches

[Wei Huang]

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

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

[Wei Huang]

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 <wei@redhat.com>
---
 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

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

[Andrew Jones]

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 <wei@redhat.com>
> ---
>  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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reviewed-by: Andrew Jones <drjones@redhat.com>

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

[Wei Huang]

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 <wei@redhat.com>
---
 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

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

[Andrew Jones]

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 <wei@redhat.com>
> ---
>  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 <drjones@redhat.com>

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

[Paolo Bonzini]

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

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

[Wei Huang]

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
>