[Xen-devel] [PATCH v2] xen/x86: Remove unnecessary cast on void pointer

Simran Singhal posted 1 patch 4 years ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/xen tags/patchew/20200329045512.GA28203@simran-Inspiron-5558
Maintainers: Kevin Tian <kevin.tian@intel.com>, Jun Nakajima <jun.nakajima@intel.com>, "Roger Pau Monné" <roger.pau@citrix.com>, Jan Beulich <jbeulich@suse.com>, Andrew Cooper <andrew.cooper3@citrix.com>, Wei Liu <wl@xen.org>
xen/arch/x86/acpi/cpufreq/powernow.c |  2 +-
xen/arch/x86/cpu/vpmu.c              |  2 +-
xen/arch/x86/hpet.c                  |  2 +-
xen/arch/x86/hvm/save.c              |  2 +-
xen/arch/x86/hvm/vmx/vvmx.c          | 12 ++++++------
5 files changed, 10 insertions(+), 10 deletions(-)
[Xen-devel] [PATCH v2] xen/x86: Remove unnecessary cast on void pointer
Posted by Simran Singhal 4 years ago
Assignment to a typed pointer is sufficient in C.
No cast is needed.

Also, changed some u64/u32 to uint64_t/uint32_t.

Signed-off-by: Simran Singhal <singhalsimran0@gmail.com>
---
Changes in v2:
	- Took the chance to change some uintX to uintX_t.

 xen/arch/x86/acpi/cpufreq/powernow.c |  2 +-
 xen/arch/x86/cpu/vpmu.c              |  2 +-
 xen/arch/x86/hpet.c                  |  2 +-
 xen/arch/x86/hvm/save.c              |  2 +-
 xen/arch/x86/hvm/vmx/vvmx.c          | 12 ++++++------
 5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/acpi/cpufreq/powernow.c b/xen/arch/x86/acpi/cpufreq/powernow.c
index 3cf9c6cd05..f620bebc7e 100644
--- a/xen/arch/x86/acpi/cpufreq/powernow.c
+++ b/xen/arch/x86/acpi/cpufreq/powernow.c
@@ -58,7 +58,7 @@ static void transition_pstate(void *pstate)
 
 static void update_cpb(void *data)
 {
-    struct cpufreq_policy *policy = (struct cpufreq_policy *)data;
+    struct cpufreq_policy *policy = data;
 
     if (policy->turbo != CPUFREQ_TURBO_UNSUPPORTED) {
         uint64_t msr_content;
diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
index e50d478d23..1ed39ef03f 100644
--- a/xen/arch/x86/cpu/vpmu.c
+++ b/xen/arch/x86/cpu/vpmu.c
@@ -337,7 +337,7 @@ void vpmu_do_interrupt(struct cpu_user_regs *regs)
 
 static void vpmu_save_force(void *arg)
 {
-    struct vcpu *v = (struct vcpu *)arg;
+    struct vcpu *v = arg;
     struct vpmu_struct *vpmu = vcpu_vpmu(v);
 
     if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) )
diff --git a/xen/arch/x86/hpet.c b/xen/arch/x86/hpet.c
index 86929b9ba1..c46e7cf4ee 100644
--- a/xen/arch/x86/hpet.c
+++ b/xen/arch/x86/hpet.c
@@ -215,7 +215,7 @@ again:
 static void hpet_interrupt_handler(int irq, void *data,
         struct cpu_user_regs *regs)
 {
-    struct hpet_event_channel *ch = (struct hpet_event_channel *)data;
+    struct hpet_event_channel *ch = data;
 
     this_cpu(irq_count)--;
 
diff --git a/xen/arch/x86/hvm/save.c b/xen/arch/x86/hvm/save.c
index 0fc59d3487..a2c56fbc1e 100644
--- a/xen/arch/x86/hvm/save.c
+++ b/xen/arch/x86/hvm/save.c
@@ -417,7 +417,7 @@ void _hvm_read_entry(struct hvm_domain_context *h, void *dest,
     memcpy(dest, &h->data[h->cur], d->length);
 
     if ( d->length < dest_len )
-        memset((char *)dest + d->length, 0, dest_len - d->length);
+        memset(dest + d->length, 0, dest_len - d->length);
 
     h->cur += d->length;
 }
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index f049920196..2edb103205 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -253,12 +253,12 @@ static int vvmcs_offset(u32 width, u32 type, u32 index)
     return offset;
 }
 
-u64 get_vvmcs_virtual(void *vvmcs, u32 vmcs_encoding)
+uint64_t get_vvmcs_virtual(void *vvmcs, uint32_t vmcs_encoding)
 {
     union vmcs_encoding enc;
-    u64 *content = (u64 *) vvmcs;
+    uint64_t *content = vvmcs;
     int offset;
-    u64 res;
+    uint64_t res;
 
     enc.word = vmcs_encoding;
     offset = vvmcs_offset(enc.width, enc.type, enc.index);
@@ -307,12 +307,12 @@ enum vmx_insn_errno get_vvmcs_real_safe(const struct vcpu *v, u32 encoding,
     return virtual_vmcs_vmread_safe(v, encoding, val);
 }
 
-void set_vvmcs_virtual(void *vvmcs, u32 vmcs_encoding, u64 val)
+void set_vvmcs_virtual(void *vvmcs, uint32_t vmcs_encoding, uint64_t val)
 {
     union vmcs_encoding enc;
-    u64 *content = (u64 *) vvmcs;
+    uint64_t *content = vvmcs;
     int offset;
-    u64 res;
+    uint64_t res;
 
     enc.word = vmcs_encoding;
     offset = vvmcs_offset(enc.width, enc.type, enc.index);
-- 
2.17.1


Re: [Xen-devel] [PATCH v2] xen/x86: Remove unnecessary cast on void pointer
Posted by Wei Liu 4 years ago
On Sun, Mar 29, 2020 at 10:25:12AM +0530, Simran Singhal wrote:
> Assignment to a typed pointer is sufficient in C.
> No cast is needed.
> 
> Also, changed some u64/u32 to uint64_t/uint32_t.
> 
> Signed-off-by: Simran Singhal <singhalsimran0@gmail.com>
> ---
> Changes in v2:
> 	- Took the chance to change some uintX to uintX_t.
> 
>  xen/arch/x86/acpi/cpufreq/powernow.c |  2 +-
>  xen/arch/x86/cpu/vpmu.c              |  2 +-
>  xen/arch/x86/hpet.c                  |  2 +-
>  xen/arch/x86/hvm/save.c              |  2 +-
>  xen/arch/x86/hvm/vmx/vvmx.c          | 12 ++++++------
>  5 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/xen/arch/x86/acpi/cpufreq/powernow.c b/xen/arch/x86/acpi/cpufreq/powernow.c
> index 3cf9c6cd05..f620bebc7e 100644
> --- a/xen/arch/x86/acpi/cpufreq/powernow.c
> +++ b/xen/arch/x86/acpi/cpufreq/powernow.c
> @@ -58,7 +58,7 @@ static void transition_pstate(void *pstate)
>  
>  static void update_cpb(void *data)
>  {
> -    struct cpufreq_policy *policy = (struct cpufreq_policy *)data;
> +    struct cpufreq_policy *policy = data;
>  
>      if (policy->turbo != CPUFREQ_TURBO_UNSUPPORTED) {
>          uint64_t msr_content;
> diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
> index e50d478d23..1ed39ef03f 100644
> --- a/xen/arch/x86/cpu/vpmu.c
> +++ b/xen/arch/x86/cpu/vpmu.c
> @@ -337,7 +337,7 @@ void vpmu_do_interrupt(struct cpu_user_regs *regs)
>  
>  static void vpmu_save_force(void *arg)
>  {
> -    struct vcpu *v = (struct vcpu *)arg;
> +    struct vcpu *v = arg;
>      struct vpmu_struct *vpmu = vcpu_vpmu(v);
>  
>      if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) )
> diff --git a/xen/arch/x86/hpet.c b/xen/arch/x86/hpet.c
> index 86929b9ba1..c46e7cf4ee 100644
> --- a/xen/arch/x86/hpet.c
> +++ b/xen/arch/x86/hpet.c
> @@ -215,7 +215,7 @@ again:
>  static void hpet_interrupt_handler(int irq, void *data,
>          struct cpu_user_regs *regs)
>  {
> -    struct hpet_event_channel *ch = (struct hpet_event_channel *)data;
> +    struct hpet_event_channel *ch = data;
>  
>      this_cpu(irq_count)--;
>  
> diff --git a/xen/arch/x86/hvm/save.c b/xen/arch/x86/hvm/save.c
> index 0fc59d3487..a2c56fbc1e 100644
> --- a/xen/arch/x86/hvm/save.c
> +++ b/xen/arch/x86/hvm/save.c
> @@ -417,7 +417,7 @@ void _hvm_read_entry(struct hvm_domain_context *h, void *dest,
>      memcpy(dest, &h->data[h->cur], d->length);
>  
>      if ( d->length < dest_len )
> -        memset((char *)dest + d->length, 0, dest_len - d->length);
> +        memset(dest + d->length, 0, dest_len - d->length);

I believe you shouldn't drop the cast here either because dest is of
type void*.

Although the calculation in the end is the same (void* considered of
size 1), I would still keep the cast such that the semantics is correct.

Wei.

Re: [Xen-devel] [PATCH v2] xen/x86: Remove unnecessary cast on void pointer
Posted by Roger Pau Monné 4 years ago
On Sun, Mar 29, 2020 at 02:36:51PM +0100, Wei Liu wrote:
> On Sun, Mar 29, 2020 at 10:25:12AM +0530, Simran Singhal wrote:
> > Assignment to a typed pointer is sufficient in C.
> > No cast is needed.
> > 
> > Also, changed some u64/u32 to uint64_t/uint32_t.
> > 
> > Signed-off-by: Simran Singhal <singhalsimran0@gmail.com>
> > ---
> > Changes in v2:
> > 	- Took the chance to change some uintX to uintX_t.
> > 
> >  xen/arch/x86/acpi/cpufreq/powernow.c |  2 +-
> >  xen/arch/x86/cpu/vpmu.c              |  2 +-
> >  xen/arch/x86/hpet.c                  |  2 +-
> >  xen/arch/x86/hvm/save.c              |  2 +-
> >  xen/arch/x86/hvm/vmx/vvmx.c          | 12 ++++++------
> >  5 files changed, 10 insertions(+), 10 deletions(-)
> > 
> > diff --git a/xen/arch/x86/acpi/cpufreq/powernow.c b/xen/arch/x86/acpi/cpufreq/powernow.c
> > index 3cf9c6cd05..f620bebc7e 100644
> > --- a/xen/arch/x86/acpi/cpufreq/powernow.c
> > +++ b/xen/arch/x86/acpi/cpufreq/powernow.c
> > @@ -58,7 +58,7 @@ static void transition_pstate(void *pstate)
> >  
> >  static void update_cpb(void *data)
> >  {
> > -    struct cpufreq_policy *policy = (struct cpufreq_policy *)data;
> > +    struct cpufreq_policy *policy = data;
> >  
> >      if (policy->turbo != CPUFREQ_TURBO_UNSUPPORTED) {
> >          uint64_t msr_content;
> > diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
> > index e50d478d23..1ed39ef03f 100644
> > --- a/xen/arch/x86/cpu/vpmu.c
> > +++ b/xen/arch/x86/cpu/vpmu.c
> > @@ -337,7 +337,7 @@ void vpmu_do_interrupt(struct cpu_user_regs *regs)
> >  
> >  static void vpmu_save_force(void *arg)
> >  {
> > -    struct vcpu *v = (struct vcpu *)arg;
> > +    struct vcpu *v = arg;
> >      struct vpmu_struct *vpmu = vcpu_vpmu(v);
> >  
> >      if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) )
> > diff --git a/xen/arch/x86/hpet.c b/xen/arch/x86/hpet.c
> > index 86929b9ba1..c46e7cf4ee 100644
> > --- a/xen/arch/x86/hpet.c
> > +++ b/xen/arch/x86/hpet.c
> > @@ -215,7 +215,7 @@ again:
> >  static void hpet_interrupt_handler(int irq, void *data,
> >          struct cpu_user_regs *regs)
> >  {
> > -    struct hpet_event_channel *ch = (struct hpet_event_channel *)data;
> > +    struct hpet_event_channel *ch = data;
> >  
> >      this_cpu(irq_count)--;
> >  
> > diff --git a/xen/arch/x86/hvm/save.c b/xen/arch/x86/hvm/save.c
> > index 0fc59d3487..a2c56fbc1e 100644
> > --- a/xen/arch/x86/hvm/save.c
> > +++ b/xen/arch/x86/hvm/save.c
> > @@ -417,7 +417,7 @@ void _hvm_read_entry(struct hvm_domain_context *h, void *dest,
> >      memcpy(dest, &h->data[h->cur], d->length);
> >  
> >      if ( d->length < dest_len )
> > -        memset((char *)dest + d->length, 0, dest_len - d->length);
> > +        memset(dest + d->length, 0, dest_len - d->length);
> 
> I believe you shouldn't drop the cast here either because dest is of
> type void*.
> 
> Although the calculation in the end is the same (void* considered of
> size 1), I would still keep the cast such that the semantics is correct.

IMO dropping the case here is fine, as dest is of type void * the
calculation is correct and the cast just obfuscates it.

I usually cast things to void * instead of char * or uint8_t * in
order to do pointer additions with size 1.

Thanks, Roger.

Re: [Xen-devel] [PATCH v2] xen/x86: Remove unnecessary cast on void pointer
Posted by Jan Beulich 4 years ago
On 30.03.2020 12:05, Roger Pau Monné wrote:
> On Sun, Mar 29, 2020 at 02:36:51PM +0100, Wei Liu wrote:
>> On Sun, Mar 29, 2020 at 10:25:12AM +0530, Simran Singhal wrote:
>>> Assignment to a typed pointer is sufficient in C.
>>> No cast is needed.
>>>
>>> Also, changed some u64/u32 to uint64_t/uint32_t.
>>>
>>> Signed-off-by: Simran Singhal <singhalsimran0@gmail.com>
>>> ---
>>> Changes in v2:
>>> 	- Took the chance to change some uintX to uintX_t.
>>>
>>>  xen/arch/x86/acpi/cpufreq/powernow.c |  2 +-
>>>  xen/arch/x86/cpu/vpmu.c              |  2 +-
>>>  xen/arch/x86/hpet.c                  |  2 +-
>>>  xen/arch/x86/hvm/save.c              |  2 +-
>>>  xen/arch/x86/hvm/vmx/vvmx.c          | 12 ++++++------
>>>  5 files changed, 10 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/xen/arch/x86/acpi/cpufreq/powernow.c b/xen/arch/x86/acpi/cpufreq/powernow.c
>>> index 3cf9c6cd05..f620bebc7e 100644
>>> --- a/xen/arch/x86/acpi/cpufreq/powernow.c
>>> +++ b/xen/arch/x86/acpi/cpufreq/powernow.c
>>> @@ -58,7 +58,7 @@ static void transition_pstate(void *pstate)
>>>  
>>>  static void update_cpb(void *data)
>>>  {
>>> -    struct cpufreq_policy *policy = (struct cpufreq_policy *)data;
>>> +    struct cpufreq_policy *policy = data;
>>>  
>>>      if (policy->turbo != CPUFREQ_TURBO_UNSUPPORTED) {
>>>          uint64_t msr_content;
>>> diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
>>> index e50d478d23..1ed39ef03f 100644
>>> --- a/xen/arch/x86/cpu/vpmu.c
>>> +++ b/xen/arch/x86/cpu/vpmu.c
>>> @@ -337,7 +337,7 @@ void vpmu_do_interrupt(struct cpu_user_regs *regs)
>>>  
>>>  static void vpmu_save_force(void *arg)
>>>  {
>>> -    struct vcpu *v = (struct vcpu *)arg;
>>> +    struct vcpu *v = arg;
>>>      struct vpmu_struct *vpmu = vcpu_vpmu(v);
>>>  
>>>      if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) )
>>> diff --git a/xen/arch/x86/hpet.c b/xen/arch/x86/hpet.c
>>> index 86929b9ba1..c46e7cf4ee 100644
>>> --- a/xen/arch/x86/hpet.c
>>> +++ b/xen/arch/x86/hpet.c
>>> @@ -215,7 +215,7 @@ again:
>>>  static void hpet_interrupt_handler(int irq, void *data,
>>>          struct cpu_user_regs *regs)
>>>  {
>>> -    struct hpet_event_channel *ch = (struct hpet_event_channel *)data;
>>> +    struct hpet_event_channel *ch = data;
>>>  
>>>      this_cpu(irq_count)--;
>>>  
>>> diff --git a/xen/arch/x86/hvm/save.c b/xen/arch/x86/hvm/save.c
>>> index 0fc59d3487..a2c56fbc1e 100644
>>> --- a/xen/arch/x86/hvm/save.c
>>> +++ b/xen/arch/x86/hvm/save.c
>>> @@ -417,7 +417,7 @@ void _hvm_read_entry(struct hvm_domain_context *h, void *dest,
>>>      memcpy(dest, &h->data[h->cur], d->length);
>>>  
>>>      if ( d->length < dest_len )
>>> -        memset((char *)dest + d->length, 0, dest_len - d->length);
>>> +        memset(dest + d->length, 0, dest_len - d->length);
>>
>> I believe you shouldn't drop the cast here either because dest is of
>> type void*.
>>
>> Although the calculation in the end is the same (void* considered of
>> size 1), I would still keep the cast such that the semantics is correct.
> 
> IMO dropping the case here is fine, as dest is of type void * the
> calculation is correct and the cast just obfuscates it.

+1

Jan

Re: [Xen-devel] [PATCH v2] xen/x86: Remove unnecessary cast on void pointer
Posted by Roger Pau Monné 4 years ago
On Sun, Mar 29, 2020 at 10:25:12AM +0530, Simran Singhal wrote:
> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
> index f049920196..2edb103205 100644
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -253,12 +253,12 @@ static int vvmcs_offset(u32 width, u32 type, u32 index)
>      return offset;
>  }
>  
> -u64 get_vvmcs_virtual(void *vvmcs, u32 vmcs_encoding)
> +uint64_t get_vvmcs_virtual(void *vvmcs, uint32_t vmcs_encoding)
>  {
>      union vmcs_encoding enc;
> -    u64 *content = (u64 *) vvmcs;
> +    uint64_t *content = vvmcs;
>      int offset;
> -    u64 res;
> +    uint64_t res;
>  
>      enc.word = vmcs_encoding;
>      offset = vvmcs_offset(enc.width, enc.type, enc.index);
> @@ -307,12 +307,12 @@ enum vmx_insn_errno get_vvmcs_real_safe(const struct vcpu *v, u32 encoding,
>      return virtual_vmcs_vmread_safe(v, encoding, val);
>  }
>  
> -void set_vvmcs_virtual(void *vvmcs, u32 vmcs_encoding, u64 val)
> +void set_vvmcs_virtual(void *vvmcs, uint32_t vmcs_encoding, uint64_t val)
>  {
>      union vmcs_encoding enc;
> -    u64 *content = (u64 *) vvmcs;
> +    uint64_t *content = vvmcs;
>      int offset;
> -    u64 res;
> +    uint64_t res;

Thanks for doing the switch of content to type uint64_t. You should
however not change the type of res to uint64_t also IMO, as you are
not touching that line anyway.

With that fixed:

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks!

Re: [Xen-devel] [PATCH v2] xen/x86: Remove unnecessary cast on void pointer
Posted by Jan Beulich 4 years ago
On 30.03.2020 12:11, Roger Pau Monné wrote:
> On Sun, Mar 29, 2020 at 10:25:12AM +0530, Simran Singhal wrote:
>> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
>> index f049920196..2edb103205 100644
>> --- a/xen/arch/x86/hvm/vmx/vvmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
>> @@ -253,12 +253,12 @@ static int vvmcs_offset(u32 width, u32 type, u32 index)
>>      return offset;
>>  }
>>  
>> -u64 get_vvmcs_virtual(void *vvmcs, u32 vmcs_encoding)
>> +uint64_t get_vvmcs_virtual(void *vvmcs, uint32_t vmcs_encoding)
>>  {
>>      union vmcs_encoding enc;
>> -    u64 *content = (u64 *) vvmcs;
>> +    uint64_t *content = vvmcs;
>>      int offset;
>> -    u64 res;
>> +    uint64_t res;
>>  
>>      enc.word = vmcs_encoding;
>>      offset = vvmcs_offset(enc.width, enc.type, enc.index);
>> @@ -307,12 +307,12 @@ enum vmx_insn_errno get_vvmcs_real_safe(const struct vcpu *v, u32 encoding,
>>      return virtual_vmcs_vmread_safe(v, encoding, val);
>>  }
>>  
>> -void set_vvmcs_virtual(void *vvmcs, u32 vmcs_encoding, u64 val)
>> +void set_vvmcs_virtual(void *vvmcs, uint32_t vmcs_encoding, uint64_t val)
>>  {
>>      union vmcs_encoding enc;
>> -    u64 *content = (u64 *) vvmcs;
>> +    uint64_t *content = vvmcs;
>>      int offset;
>> -    u64 res;
>> +    uint64_t res;
> 
> Thanks for doing the switch of content to type uint64_t. You should
> however not change the type of res to uint64_t also IMO, as you are
> not touching that line anyway.

I actually wouldn't mind the patch being left as is?

> With that fixed:
> 
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Roger - please clarify if your R-b is also fine without the requested
adjustment.

Jan

Re: [Xen-devel] [PATCH v2] xen/x86: Remove unnecessary cast on void pointer
Posted by Roger Pau Monné 4 years ago
On Mon, Mar 30, 2020 at 01:09:20PM +0200, Jan Beulich wrote:
> On 30.03.2020 12:11, Roger Pau Monné wrote:
> > On Sun, Mar 29, 2020 at 10:25:12AM +0530, Simran Singhal wrote:
> >> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
> >> index f049920196..2edb103205 100644
> >> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> >> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> >> @@ -253,12 +253,12 @@ static int vvmcs_offset(u32 width, u32 type, u32 index)
> >>      return offset;
> >>  }
> >>  
> >> -u64 get_vvmcs_virtual(void *vvmcs, u32 vmcs_encoding)
> >> +uint64_t get_vvmcs_virtual(void *vvmcs, uint32_t vmcs_encoding)
> >>  {
> >>      union vmcs_encoding enc;
> >> -    u64 *content = (u64 *) vvmcs;
> >> +    uint64_t *content = vvmcs;
> >>      int offset;
> >> -    u64 res;
> >> +    uint64_t res;
> >>  
> >>      enc.word = vmcs_encoding;
> >>      offset = vvmcs_offset(enc.width, enc.type, enc.index);
> >> @@ -307,12 +307,12 @@ enum vmx_insn_errno get_vvmcs_real_safe(const struct vcpu *v, u32 encoding,
> >>      return virtual_vmcs_vmread_safe(v, encoding, val);
> >>  }
> >>  
> >> -void set_vvmcs_virtual(void *vvmcs, u32 vmcs_encoding, u64 val)
> >> +void set_vvmcs_virtual(void *vvmcs, uint32_t vmcs_encoding, uint64_t val)
> >>  {
> >>      union vmcs_encoding enc;
> >> -    u64 *content = (u64 *) vvmcs;
> >> +    uint64_t *content = vvmcs;
> >>      int offset;
> >> -    u64 res;
> >> +    uint64_t res;
> > 
> > Thanks for doing the switch of content to type uint64_t. You should
> > however not change the type of res to uint64_t also IMO, as you are
> > not touching that line anyway.
> 
> I actually wouldn't mind the patch being left as is?
> 
> > With that fixed:
> > 
> > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> Acked-by: Jan Beulich <jbeulich@suse.com>
> 
> Roger - please clarify if your R-b is also fine without the requested
> adjustment.

Yes, TBH I was borderline on requesting the change, as the type change
is correct.

Roger.

Re: [Xen-devel] [PATCH v2] xen/x86: Remove unnecessary cast on void pointer
Posted by Simran Singhal 4 years ago
On Mon, Mar 30, 2020 at 3:41 PM Roger Pau Monné <roger.pau@citrix.com>
wrote:

> On Sun, Mar 29, 2020 at 10:25:12AM +0530, Simran Singhal wrote:
> > diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
> > index f049920196..2edb103205 100644
> > --- a/xen/arch/x86/hvm/vmx/vvmx.c
> > +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> > @@ -253,12 +253,12 @@ static int vvmcs_offset(u32 width, u32 type, u32
> index)
> >      return offset;
> >  }
> >
> > -u64 get_vvmcs_virtual(void *vvmcs, u32 vmcs_encoding)
> > +uint64_t get_vvmcs_virtual(void *vvmcs, uint32_t vmcs_encoding)
> >  {
> >      union vmcs_encoding enc;
> > -    u64 *content = (u64 *) vvmcs;
> > +    uint64_t *content = vvmcs;
> >      int offset;
> > -    u64 res;
> > +    uint64_t res;
> >
> >      enc.word = vmcs_encoding;
> >      offset = vvmcs_offset(enc.width, enc.type, enc.index);
> > @@ -307,12 +307,12 @@ enum vmx_insn_errno get_vvmcs_real_safe(const
> struct vcpu *v, u32 encoding,
> >      return virtual_vmcs_vmread_safe(v, encoding, val);
> >  }
> >
> > -void set_vvmcs_virtual(void *vvmcs, u32 vmcs_encoding, u64 val)
> > +void set_vvmcs_virtual(void *vvmcs, uint32_t vmcs_encoding, uint64_t
> val)
> >  {
> >      union vmcs_encoding enc;
> > -    u64 *content = (u64 *) vvmcs;
> > +    uint64_t *content = vvmcs;
> >      int offset;
> > -    u64 res;
> > +    uint64_t res;
>
> Thanks for doing the switch of content to type uint64_t. You should
> however not change the type of res to uint64_t also IMO, as you are
> not touching that line anyway.
>

Thanks for your feedback Roger and Wei.
My bad, I thought I need to change all the unintX to uintX_t in the function
set_vvmcs_virtual().

Sorry for the inconvenience.


>
> With that fixed:
>
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
>
> Thanks!
>