[PATCH 0/3] KVM: small type cleanups

[PATCH 0/3] KVM: small type cleanups

[Mathias Krause]

Hi,

this small series contains a few type and style cleanups. It has no
impact on the generated code but removes a few small nits from the
code.

Please apply!

Thanks,

Mathias Krause (3):
  KVM: VMX: Use proper types to access const arrays
  KVM: VMX: Use size_t to store sizeof() values
  KVM: x86: Drop useless cast

 arch/x86/kvm/vmx.c |   19 +++++++++----------
 arch/x86/kvm/x86.c |    2 +-
 2 files changed, 10 insertions(+), 11 deletions(-)

-- 
1.7.10.4

[PATCH 1/3] KVM: VMX: Use proper types to access const arrays

[Mathias Krause]

Use a const pointer type instead of casting away the const qualifier
from const arrays. Keep the pointer array on the stack, nonetheless.
Making it static just increases the object size.

Signed-off-by: Mathias Krause <minipli@googlemail.com>
---
 arch/x86/kvm/vmx.c |   15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 260a919..7393164 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -5956,8 +5956,8 @@ static void copy_shadow_to_vmcs12(struct vcpu_vmx *vmx)
 	unsigned long field;
 	u64 field_value;
 	struct vmcs *shadow_vmcs = vmx->nested.current_shadow_vmcs;
-	unsigned long *fields = (unsigned long *)shadow_read_write_fields;
-	int num_fields = max_shadow_read_write_fields;
+	const unsigned long *fields = shadow_read_write_fields;
+	const int num_fields = max_shadow_read_write_fields;
 
 	vmcs_load(shadow_vmcs);
 
@@ -5986,12 +5986,11 @@ static void copy_shadow_to_vmcs12(struct vcpu_vmx *vmx)
 
 static void copy_vmcs12_to_shadow(struct vcpu_vmx *vmx)
 {
-	unsigned long *fields[] = {
-		(unsigned long *)shadow_read_write_fields,
-		(unsigned long *)shadow_read_only_fields
+	const unsigned long *fields[] = {
+		shadow_read_write_fields,
+		shadow_read_only_fields
 	};
-	int num_lists =  ARRAY_SIZE(fields);
-	int max_fields[] = {
+	const int max_fields[] = {
 		max_shadow_read_write_fields,
 		max_shadow_read_only_fields
 	};
@@ -6002,7 +6001,7 @@ static void copy_vmcs12_to_shadow(struct vcpu_vmx *vmx)
 
 	vmcs_load(shadow_vmcs);
 
-	for (q = 0; q < num_lists; q++) {
+	for (q = 0; q < ARRAY_SIZE(fields); q++) {
 		for (i = 0; i < max_fields[q]; i++) {
 			field = fields[q][i];
 			vmcs12_read_any(&vmx->vcpu, field, &field_value);
-- 
1.7.10.4

Re: [PATCH 1/3] KVM: VMX: Use proper types to access const arrays

[Paolo Bonzini]

Il 26/06/2013 20:36, Mathias Krause ha scritto:
> Use a const pointer type instead of casting away the const qualifier
> from const arrays. Keep the pointer array on the stack, nonetheless.
> Making it static just increases the object size.
> 
> Signed-off-by: Mathias Krause <minipli@googlemail.com>
> ---
>  arch/x86/kvm/vmx.c |   15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 260a919..7393164 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -5956,8 +5956,8 @@ static void copy_shadow_to_vmcs12(struct vcpu_vmx *vmx)
>  	unsigned long field;
>  	u64 field_value;
>  	struct vmcs *shadow_vmcs = vmx->nested.current_shadow_vmcs;
> -	unsigned long *fields = (unsigned long *)shadow_read_write_fields;
> -	int num_fields = max_shadow_read_write_fields;
> +	const unsigned long *fields = shadow_read_write_fields;
> +	const int num_fields = max_shadow_read_write_fields;
>  
>  	vmcs_load(shadow_vmcs);
>  
> @@ -5986,12 +5986,11 @@ static void copy_shadow_to_vmcs12(struct vcpu_vmx *vmx)
>  
>  static void copy_vmcs12_to_shadow(struct vcpu_vmx *vmx)
>  {
> -	unsigned long *fields[] = {
> -		(unsigned long *)shadow_read_write_fields,
> -		(unsigned long *)shadow_read_only_fields
> +	const unsigned long *fields[] = {
> +		shadow_read_write_fields,
> +		shadow_read_only_fields
>  	};
> -	int num_lists =  ARRAY_SIZE(fields);
> -	int max_fields[] = {
> +	const int max_fields[] = {
>  		max_shadow_read_write_fields,
>  		max_shadow_read_only_fields
>  	};
> @@ -6002,7 +6001,7 @@ static void copy_vmcs12_to_shadow(struct vcpu_vmx *vmx)
>  
>  	vmcs_load(shadow_vmcs);
>  
> -	for (q = 0; q < num_lists; q++) {
> +	for (q = 0; q < ARRAY_SIZE(fields); q++) {
>  		for (i = 0; i < max_fields[q]; i++) {
>  			field = fields[q][i];
>  			vmcs12_read_any(&vmx->vcpu, field, &field_value);
> 

The "const int" is not particularly useful, but doesn't hurt either.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [PATCH 1/3] KVM: VMX: Use proper types to access const arrays

[Mathias Krause]

On 27 June 2013 15:33, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 26/06/2013 20:36, Mathias Krause ha scritto:
>> Use a const pointer type instead of casting away the const qualifier
>> from const arrays. Keep the pointer array on the stack, nonetheless.
>> Making it static just increases the object size.
>>
>> Signed-off-by: Mathias Krause <minipli@googlemail.com>
>> ---
>>  arch/x86/kvm/vmx.c |   15 +++++++--------
>>  1 file changed, 7 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index 260a919..7393164 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -5956,8 +5956,8 @@ static void copy_shadow_to_vmcs12(struct vcpu_vmx *vmx)
>>       unsigned long field;
>>       u64 field_value;
>>       struct vmcs *shadow_vmcs = vmx->nested.current_shadow_vmcs;
>> -     unsigned long *fields = (unsigned long *)shadow_read_write_fields;
>> -     int num_fields = max_shadow_read_write_fields;
>> +     const unsigned long *fields = shadow_read_write_fields;
>> +     const int num_fields = max_shadow_read_write_fields;
>>
>>       vmcs_load(shadow_vmcs);
>>
>> @@ -5986,12 +5986,11 @@ static void copy_shadow_to_vmcs12(struct vcpu_vmx *vmx)
>>
>>  static void copy_vmcs12_to_shadow(struct vcpu_vmx *vmx)
>>  {
>> -     unsigned long *fields[] = {
>> -             (unsigned long *)shadow_read_write_fields,
>> -             (unsigned long *)shadow_read_only_fields
>> +     const unsigned long *fields[] = {
>> +             shadow_read_write_fields,
>> +             shadow_read_only_fields
>>       };
>> -     int num_lists =  ARRAY_SIZE(fields);
>> -     int max_fields[] = {
>> +     const int max_fields[] = {
>>               max_shadow_read_write_fields,
>>               max_shadow_read_only_fields
>>       };
>> @@ -6002,7 +6001,7 @@ static void copy_vmcs12_to_shadow(struct vcpu_vmx *vmx)
>>
>>       vmcs_load(shadow_vmcs);
>>
>> -     for (q = 0; q < num_lists; q++) {
>> +     for (q = 0; q < ARRAY_SIZE(fields); q++) {
>>               for (i = 0; i < max_fields[q]; i++) {
>>                       field = fields[q][i];
>>                       vmcs12_read_any(&vmx->vcpu, field, &field_value);
>>
>
> The "const int" is not particularly useful, but doesn't hurt either.

It's more of a hint for the compiler to take the values verbatim
instead of allocating stack space for them. But it'll probably already
do it even without that "hint".

>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Mathias

Re: [PATCH 1/3] KVM: VMX: Use proper types to access const arrays

[Paolo Bonzini]

Il 27/06/2013 15:42, Mathias Krause ha scritto:
> > The "const int" is not particularly useful, but doesn't hurt either.
> 
> It's more of a hint for the compiler to take the values verbatim
> instead of allocating stack space for them. But it'll probably already
> do it even without that "hint".

It won't change anything really.  Maybe for "const int foo[]", but I'm
not even sure about that and it depends a lot on the circumstances (loop
unrolling, inlining, whether you ever store "foo" in a variable or pass
it as a parameter, ...).  The compiler may leave it on the stack anyway.

Paolo

[PATCH 2/3] KVM: VMX: Use size_t to store sizeof() values

[Mathias Krause]

The type for storing values of the sizeof operator should be size_t.
No semantical changes, only type correctness.

Signed-off-by: Mathias Krause <minipli@googlemail.com>
---
 arch/x86/kvm/vmx.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 7393164..cd9090f 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3909,7 +3909,7 @@ static void free_vpid(struct vcpu_vmx *vmx)
 static void __vmx_disable_intercept_for_msr(unsigned long *msr_bitmap,
 						u32 msr, int type)
 {
-	int f = sizeof(unsigned long);
+	const size_t f = sizeof(unsigned long);
 
 	if (!cpu_has_vmx_msr_bitmap())
 		return;
@@ -3944,7 +3944,7 @@ static void __vmx_disable_intercept_for_msr(unsigned long *msr_bitmap,
 static void __vmx_enable_intercept_for_msr(unsigned long *msr_bitmap,
 						u32 msr, int type)
 {
-	int f = sizeof(unsigned long);
+	const size_t f = sizeof(unsigned long);
 
 	if (!cpu_has_vmx_msr_bitmap())
 		return;
-- 
1.7.10.4

Re: [PATCH 2/3] KVM: VMX: Use size_t to store sizeof() values

[Paolo Bonzini]

Il 26/06/2013 20:36, Mathias Krause ha scritto:
> The type for storing values of the sizeof operator should be size_t.
> No semantical changes, only type correctness.
> 
> Signed-off-by: Mathias Krause <minipli@googlemail.com>
> ---
>  arch/x86/kvm/vmx.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 7393164..cd9090f 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -3909,7 +3909,7 @@ static void free_vpid(struct vcpu_vmx *vmx)
>  static void __vmx_disable_intercept_for_msr(unsigned long *msr_bitmap,
>  						u32 msr, int type)
>  {
> -	int f = sizeof(unsigned long);
> +	const size_t f = sizeof(unsigned long);
>  
>  	if (!cpu_has_vmx_msr_bitmap())
>  		return;
> @@ -3944,7 +3944,7 @@ static void __vmx_disable_intercept_for_msr(unsigned long *msr_bitmap,
>  static void __vmx_enable_intercept_for_msr(unsigned long *msr_bitmap,
>  						u32 msr, int type)
>  {
> -	int f = sizeof(unsigned long);
> +	const size_t f = sizeof(unsigned long);
>  
>  	if (!cpu_has_vmx_msr_bitmap())
>  		return;
> 

Both the "const" and the change seem like useless churn.  It is only
used to adjust a pointer by a given number of bytes.

Paolo

[PATCH 3/3] KVM: x86: Drop useless cast

[Mathias Krause]

Void pointers don't need no casting, drop it.

Signed-off-by: Mathias Krause <minipli@googlemail.com>
---
 arch/x86/kvm/x86.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index e8ba99c..472350c 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5300,7 +5300,7 @@ static struct notifier_block pvclock_gtod_notifier = {
 int kvm_arch_init(void *opaque)
 {
 	int r;
-	struct kvm_x86_ops *ops = (struct kvm_x86_ops *)opaque;
+	struct kvm_x86_ops *ops = opaque;
 
 	if (kvm_x86_ops) {
 		printk(KERN_ERR "kvm: already loaded the other module\n");
-- 
1.7.10.4

Re: [PATCH 3/3] KVM: x86: Drop useless cast

[Paolo Bonzini]

Il 26/06/2013 20:36, Mathias Krause ha scritto:
> Void pointers don't need no casting, drop it.
> 
> Signed-off-by: Mathias Krause <minipli@googlemail.com>
> ---
>  arch/x86/kvm/x86.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index e8ba99c..472350c 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -5300,7 +5300,7 @@ static struct notifier_block pvclock_gtod_notifier = {
>  int kvm_arch_init(void *opaque)
>  {
>  	int r;
> -	struct kvm_x86_ops *ops = (struct kvm_x86_ops *)opaque;
> +	struct kvm_x86_ops *ops = opaque;
>  
>  	if (kvm_x86_ops) {
>  		printk(KERN_ERR "kvm: already loaded the other module\n");
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [PATCH 0/3] KVM: small type cleanups

[Gleb Natapov]

On Wed, Jun 26, 2013 at 08:36:20PM +0200, Mathias Krause wrote:
> Hi,
> 
> this small series contains a few type and style cleanups. It has no
> impact on the generated code but removes a few small nits from the
> code.
> 
> Please apply!
> 
Applied 1 and 3. Thanks.

> Thanks,
> 
> Mathias Krause (3):
>   KVM: VMX: Use proper types to access const arrays
>   KVM: VMX: Use size_t to store sizeof() values
>   KVM: x86: Drop useless cast
> 
>  arch/x86/kvm/vmx.c |   19 +++++++++----------
>  arch/x86/kvm/x86.c |    2 +-
>  2 files changed, 10 insertions(+), 11 deletions(-)
> 
> -- 
> 1.7.10.4

--
			Gleb.