[Xen-devel,v4,2/2] xen/domain: Use typesafe gfn in map_vcpu_info

Message ID 20190819142651.11058-3-julien.grall@arm.com
State New
Headers show
Series
  • More typesafe conversion of common interface
Related show

Commit Message

Julien Grall Aug. 19, 2019, 2:26 p.m.
At the same time, modify the documentation of the hypercall to reflect
the real meaning of the field mfn.

Signed-off-by: Julien Grall <julien.grall@arm.com>

---
    Changes in v4:
        - Patch added
---
 xen/common/domain.c       | 6 +++---
 xen/include/public/vcpu.h | 2 +-
 xen/include/xen/domain.h  | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

Comments

Andrew Cooper Aug. 23, 2019, 2:16 p.m. | #1
On 19/08/2019 15:26, Julien Grall wrote:
> diff --git a/xen/include/public/vcpu.h b/xen/include/public/vcpu.h
> index 3623af932f..dc4c6a72a0 100644
> --- a/xen/include/public/vcpu.h
> +++ b/xen/include/public/vcpu.h
> @@ -182,7 +182,7 @@ DEFINE_XEN_GUEST_HANDLE(vcpu_set_singleshot_timer_t);
>   */
>  #define VCPUOP_register_vcpu_info   10  /* arg == vcpu_register_vcpu_info_t */
>  struct vcpu_register_vcpu_info {
> -    uint64_t mfn;    /* mfn of page to place vcpu_info */
> +    uint64_t mfn;    /* gfn of page to place vcpu_info */

I wonder if we can do some __XEN_INTERFACE_VERSION__ magic here to
properly change the name to gfn.

Leaving it as mfn is going to be an ongoing source of confusion.

~Andrew
Jan Beulich Aug. 29, 2019, 3:43 p.m. | #2
On 23.08.2019 16:16, Andrew Cooper wrote:
> On 19/08/2019 15:26, Julien Grall wrote:
>> diff --git a/xen/include/public/vcpu.h b/xen/include/public/vcpu.h
>> index 3623af932f..dc4c6a72a0 100644
>> --- a/xen/include/public/vcpu.h
>> +++ b/xen/include/public/vcpu.h
>> @@ -182,7 +182,7 @@ DEFINE_XEN_GUEST_HANDLE(vcpu_set_singleshot_timer_t);
>>   */
>>  #define VCPUOP_register_vcpu_info   10  /* arg == vcpu_register_vcpu_info_t */
>>  struct vcpu_register_vcpu_info {
>> -    uint64_t mfn;    /* mfn of page to place vcpu_info */
>> +    uint64_t mfn;    /* gfn of page to place vcpu_info */
> 
> I wonder if we can do some __XEN_INTERFACE_VERSION__ magic here to
> properly change the name to gfn.
> 
> Leaving it as mfn is going to be an ongoing source of confusion.

This would be pretty nice indeed.

Jan

Patch

diff --git a/xen/common/domain.c b/xen/common/domain.c
index e8f6dfbdf1..915ebca190 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -1282,7 +1282,7 @@  int vcpu_reset(struct vcpu *v)
  * of memory, and it sets a pending event to make sure that a pending
  * event doesn't get missed.
  */
-int map_vcpu_info(struct vcpu *v, unsigned long gfn, unsigned offset)
+int map_vcpu_info(struct vcpu *v, gfn_t gfn, unsigned offset)
 {
     struct domain *d = v->domain;
     void *mapping;
@@ -1299,7 +1299,7 @@  int map_vcpu_info(struct vcpu *v, unsigned long gfn, unsigned offset)
     if ( (v != current) && !(v->pause_flags & VPF_down) )
         return -EINVAL;
 
-    page = get_page_from_gfn(d, _gfn(gfn), NULL, P2M_ALLOC);
+    page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC);
     if ( !page )
         return -EINVAL;
 
@@ -1538,7 +1538,7 @@  long do_vcpu_op(int cmd, unsigned int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
             break;
 
         domain_lock(d);
-        rc = map_vcpu_info(v, info.mfn, info.offset);
+        rc = map_vcpu_info(v, _gfn(info.mfn), info.offset);
         domain_unlock(d);
 
         break;
diff --git a/xen/include/public/vcpu.h b/xen/include/public/vcpu.h
index 3623af932f..dc4c6a72a0 100644
--- a/xen/include/public/vcpu.h
+++ b/xen/include/public/vcpu.h
@@ -182,7 +182,7 @@  DEFINE_XEN_GUEST_HANDLE(vcpu_set_singleshot_timer_t);
  */
 #define VCPUOP_register_vcpu_info   10  /* arg == vcpu_register_vcpu_info_t */
 struct vcpu_register_vcpu_info {
-    uint64_t mfn;    /* mfn of page to place vcpu_info */
+    uint64_t mfn;    /* gfn of page to place vcpu_info */
     uint32_t offset; /* offset within page */
     uint32_t rsvd;   /* unused */
 };
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index 3f09cb66c0..7e754f7cc0 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -58,7 +58,7 @@  void free_pirq_struct(void *);
 int  arch_vcpu_create(struct vcpu *v);
 void arch_vcpu_destroy(struct vcpu *v);
 
-int map_vcpu_info(struct vcpu *v, unsigned long gfn, unsigned offset);
+int map_vcpu_info(struct vcpu *v, gfn_t gfn, unsigned offset);
 void unmap_vcpu_info(struct vcpu *v);
 
 int arch_domain_create(struct domain *d,