[Xen-devel] libxl/xenconsole: vpl011: Fix hex to dec conversion of vuart_gfn in libxl__device_vuart_add

Message ID 1508144567-23300-1-git-send-email-bhupinder.thakur@linaro.org
State New
Headers show
Series
  • [Xen-devel] libxl/xenconsole: vpl011: Fix hex to dec conversion of vuart_gfn in libxl__device_vuart_add
Related show

Commit Message

Bhupinder Thakur Oct. 16, 2017, 9:02 a.m.
In libxl__device_vuart_add vuart_gfn is getting stored as a hex value:

> flexarray_append(ro_front, GCSPRINTF("%"PRI_xen_pfn, state->vuart_gfn));

However, xenstore reads this value as a decimal value and tries to map the
wrong address and fails.

Introduced a new format string "PRIu_xen_pfn" which formats the value as a
decimal value.  Also corrected the type of ring_ref to xen_pfn_t in xenconsole.

Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
---

This patch was verified for arm64 and arm32 toolstack compilation.

CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>

 tools/console/daemon/io.c            | 20 +++++++++++---------
 tools/libxc/include/xenctrl_compat.h |  2 +-
 tools/libxc/xc_foreign_memory.c      |  2 +-
 tools/libxl/libxl_console.c          |  4 ++--
 tools/libxl/libxl_dom.c              |  2 +-
 tools/libxl/libxl_internal.h         |  2 +-
 xen/include/public/arch-arm.h        |  1 +
 xen/include/public/arch-x86/xen.h    |  1 +
 8 files changed, 19 insertions(+), 15 deletions(-)

Comments

Julien Grall Oct. 16, 2017, 10:18 a.m. | #1
Hi Bhupinder,

On 16/10/17 10:02, Bhupinder Thakur wrote:
> In libxl__device_vuart_add vuart_gfn is getting stored as a hex value:
> 
>> flexarray_append(ro_front, GCSPRINTF("%"PRI_xen_pfn, state->vuart_gfn));
> 
> However, xenstore reads this value as a decimal value and tries to map the
> wrong address and fails.
> 
> Introduced a new format string "PRIu_xen_pfn" which formats the value as a
> decimal value.  Also corrected the type of ring_ref to xen_pfn_t in xenconsole.

The commit message does not give any reason why xc_map_foreign_page(...) 
was switch to xen_paddr_t.

But I think this patch is doing too much. You try to fix too distinct 
errors here:
	1) Fixing the bug introduced by commit 2b668a84e5 "libxl: use correct 
type modifier for vuart_gfn"
	2) The fact that the other side will at best truncate the value

With that you also modify the xc_map_foreign_page() to avoid truncating 
the value. So you at least need to split this patch in 3 if not more.

> 
> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
> ---
> 
> This patch was verified for arm64 and arm32 toolstack compilation.
> 
> CC: Ian Jackson <ian.jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien.grall@arm.com>
> CC: Jan Beulich <jbeulich@suse.com>
> CC: Andrew Cooper <andrew.cooper3@citrix.com>
> 
>   tools/console/daemon/io.c            | 20 +++++++++++---------
>   tools/libxc/include/xenctrl_compat.h |  2 +-
>   tools/libxc/xc_foreign_memory.c      |  2 +-
>   tools/libxl/libxl_console.c          |  4 ++--
>   tools/libxl/libxl_dom.c              |  2 +-
>   tools/libxl/libxl_internal.h         |  2 +-
>   xen/include/public/arch-arm.h        |  1 +
>   xen/include/public/arch-x86/xen.h    |  1 +
>   8 files changed, 19 insertions(+), 15 deletions(-)
> 
> diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
> index e22009a..6369bf4 100644
> --- a/tools/console/daemon/io.c
> +++ b/tools/console/daemon/io.c
> @@ -80,6 +80,7 @@ static unsigned int current_array_size;
>   static unsigned int nr_fds;
>   
>   #define ROUNDUP(_x,_w) (((unsigned long)(_x)+(1UL<<(_w))-1) & ~((1UL<<(_w))-1))
> +#define INVALID_RING_REF    (~(xen_pfn_t)0)

I am a bit surprised we don't have an INVALID_XEN_PFN definition in the 
public header. Jan would you have an objection to introduce that?

>   
>   struct buffer {
>   	char *data;
> @@ -98,7 +99,7 @@ struct console {
>   	struct buffer buffer;
>   	char *xspath;
>   	char *log_suffix;
> -	int ring_ref;
> +	xen_pfn_t ring_ref;
>   	xenevtchn_handle *xce_handle;
>   	int xce_pollfd_idx;
>   	int event_count;
> @@ -651,22 +652,23 @@ static void console_unmap_interface(struct console *con)
>   {
>   	if (con->interface == NULL)
>   		return;
> -	if (xgt_handle && con->ring_ref == -1)
> +	if (xgt_handle && con->ring_ref == INVALID_RING_REF)
>   		xengnttab_unmap(xgt_handle, con->interface, 1);
>   	else
>   		munmap(con->interface, XC_PAGE_SIZE);
>   	con->interface = NULL;
> -	con->ring_ref = -1;
> +	con->ring_ref = INVALID_RING_REF;
>   }
>    
>   static int console_create_ring(struct console *con)
>   {
> -	int err, remote_port, ring_ref, rc;
> +	int err, remote_port, rc;
> +	xen_pfn_t ring_ref;
>   	char *type, path[PATH_MAX];
>   	struct domain *dom = con->d;
>   
>   	err = xs_gather(xs, con->xspath,
> -			"ring-ref", "%u", &ring_ref,
> +			"ring-ref", "%i", &ring_ref,

Hmmm, I think %i is wrong here. You don't fix the problem that the GFN 
can be wider than 32-bit.

>   			"port", "%i", &remote_port,
>   			NULL);
>   
> @@ -690,7 +692,7 @@ static int console_create_ring(struct console *con)
>   	free(type);
>   
>   	/* If using ring_ref and it has changed, remap */
> -	if (ring_ref != con->ring_ref && con->ring_ref != -1)
> +	if (ring_ref != con->ring_ref && con->ring_ref != INVALID_RING_REF)
>   		console_unmap_interface(con);
>   
>   	if (!con->interface && xgt_handle && con->use_gnttab) {
> @@ -698,14 +700,14 @@ static int console_create_ring(struct console *con)
>   		con->interface = xengnttab_map_grant_ref(xgt_handle,
>   			dom->domid, GNTTAB_RESERVED_CONSOLE,
>   			PROT_READ|PROT_WRITE);
> -		con->ring_ref = -1;
> +		con->ring_ref = INVALID_RING_REF;
>   	}
>   	if (!con->interface) {
>   		/* Fall back to xc_map_foreign_range */
>   		con->interface = xc_map_foreign_range(
>   			xc, dom->domid, XC_PAGE_SIZE,
>   			PROT_READ|PROT_WRITE,
> -			(unsigned long)ring_ref);
> +			ring_ref);
>   		if (con->interface == NULL) {
>   			err = EINVAL;
>   			goto out;
> @@ -804,7 +806,7 @@ static int console_init(struct console *con, struct domain *dom, void **data)
>   	con->master_pollfd_idx = -1;
>   	con->slave_fd = -1;
>   	con->log_fd = -1;
> -	con->ring_ref = -1;
> +	con->ring_ref = INVALID_RING_REF;
>   	con->local_port = -1;
>   	con->remote_port = -1;
>   	con->xce_pollfd_idx = -1;
> diff --git a/tools/libxc/include/xenctrl_compat.h b/tools/libxc/include/xenctrl_compat.h
> index a655e47..2d930d8 100644
> --- a/tools/libxc/include/xenctrl_compat.h
> +++ b/tools/libxc/include/xenctrl_compat.h
> @@ -26,7 +26,7 @@
>    */
>   void *xc_map_foreign_range(xc_interface *xch, uint32_t dom,
>                               int size, int prot,
> -                            unsigned long mfn );
> +                            xen_pfn_t mfn );

Can you please drop the spurious space before ')' at the same time. But 
the term mfn here is slightly odd as you pass GFN for HVM/PVH/ARM Guest. 
You might want to rename it to 'pfn' I think. Am I right?

>   
>   void *xc_map_foreign_pages(xc_interface *xch, uint32_t dom, int prot,
>                              const xen_pfn_t *arr, int num );
> diff --git a/tools/libxc/xc_foreign_memory.c b/tools/libxc/xc_foreign_memory.c
> index 4053d26..8b65e52 100644
> --- a/tools/libxc/xc_foreign_memory.c
> +++ b/tools/libxc/xc_foreign_memory.c
> @@ -33,7 +33,7 @@ void *xc_map_foreign_pages(xc_interface *xch, uint32_t dom, int prot,
>   
>   void *xc_map_foreign_range(xc_interface *xch,
>                              uint32_t dom, int size, int prot,
> -                           unsigned long mfn)
> +                           xen_pfn_t mfn)
>   {
>       xen_pfn_t *arr;
>       int num;
> diff --git a/tools/libxl/libxl_console.c b/tools/libxl/libxl_console.c
> index c05dc28..f2ca689 100644
> --- a/tools/libxl/libxl_console.c
> +++ b/tools/libxl/libxl_console.c
> @@ -329,7 +329,7 @@ int libxl__device_console_add(libxl__gc *gc, uint32_t domid,
>           flexarray_append(ro_front, "port");
>           flexarray_append(ro_front, GCSPRINTF("%"PRIu32, state->console_port));
>           flexarray_append(ro_front, "ring-ref");
> -        flexarray_append(ro_front, GCSPRINTF("%lu", state->console_mfn));
> +        flexarray_append(ro_front, GCSPRINTF("%"PRIu_xen_pfn, state->console_mfn));

All the changes regarding console_mfn in libxl should really be separated.

>       } else {
>           flexarray_append(front, "state");
>           flexarray_append(front, GCSPRINTF("%d", XenbusStateInitialising));
> @@ -376,7 +376,7 @@ int libxl__device_vuart_add(libxl__gc *gc, uint32_t domid,
>       flexarray_append(ro_front, "port");
>       flexarray_append(ro_front, GCSPRINTF("%"PRIu32, state->vuart_port));
>       flexarray_append(ro_front, "ring-ref");
> -    flexarray_append(ro_front, GCSPRINTF("%"PRI_xen_pfn, state->vuart_gfn));
> +    flexarray_append(ro_front, GCSPRINTF("%"PRIu_xen_pfn, state->vuart_gfn));
>       flexarray_append(ro_front, "limit");
>       flexarray_append(ro_front, GCSPRINTF("%d", LIBXL_XENCONSOLE_LIMIT));
>       flexarray_append(ro_front, "type");
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index ef834e6..a58e74f 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -869,7 +869,7 @@ out:
>   static int hvm_build_set_params(xc_interface *handle, uint32_t domid,
>                                   libxl_domain_build_info *info,
>                                   int store_evtchn, unsigned long *store_mfn,
> -                                int console_evtchn, unsigned long *console_mfn,
> +                                int console_evtchn, xen_pfn_t *console_mfn,
>                                   domid_t store_domid, domid_t console_domid)
>   {
>       struct hvm_info_table *va_hvm;
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 45e6df6..f52aeb3 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -1128,7 +1128,7 @@ typedef struct {
>   
>       uint32_t console_port;
>       uint32_t console_domid;
> -    unsigned long console_mfn;
> +    xen_pfn_t console_mfn;
>       char *console_tty;
>   
>       char *saved_state;
> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> index 5708cd2..05fd11c 100644
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -274,6 +274,7 @@ DEFINE_XEN_GUEST_HANDLE(vcpu_guest_core_regs_t);
>   
>   typedef uint64_t xen_pfn_t;
>   #define PRI_xen_pfn PRIx64
> +#define PRIu_xen_pfn PRIu64
>   
>   /* Maximum number of virtual CPUs in legacy multi-processor guests. */
>   /* Only one. All other VCPUS must use VCPUOP_register_vcpu_info */
> diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h
> index ff91831..3b0b1d6 100644
> --- a/xen/include/public/arch-x86/xen.h
> +++ b/xen/include/public/arch-x86/xen.h
> @@ -75,6 +75,7 @@ __DeFiNe__ __DECL_REG_LO16(name) e ## name
>   #ifndef __ASSEMBLY__
>   typedef unsigned long xen_pfn_t;
>   #define PRI_xen_pfn "lx"
> +#define PRIu_xen_pfn "lu"
>   #endif
>   
>   #define XEN_HAVE_PV_GUEST_ENTRY 1
> 

Cheers,
Jan Beulich Oct. 16, 2017, 10:23 a.m. | #2
>>> On 16.10.17 at 11:02, <bhupinder.thakur@linaro.org> wrote:
>  static int console_create_ring(struct console *con)
>  {
> -	int err, remote_port, ring_ref, rc;
> +	int err, remote_port, rc;
> +	xen_pfn_t ring_ref;
>  	char *type, path[PATH_MAX];
>  	struct domain *dom = con->d;
>  
>  	err = xs_gather(xs, con->xspath,
> -			"ring-ref", "%u", &ring_ref,
> +			"ring-ref", "%i", &ring_ref,

How would you gather a 64-bit value using %i without any length
modifier? With just %i you're even going to use partially initialized
data, so unless somewhere else the upper 32 bits got clipped off
again the console wouldn't work anymore.

Jan
Jan Beulich Oct. 16, 2017, 10:28 a.m. | #3
>>> On 16.10.17 at 12:18, <julien.grall@linaro.org> wrote:
> On 16/10/17 10:02, Bhupinder Thakur wrote:
>> --- a/tools/console/daemon/io.c
>> +++ b/tools/console/daemon/io.c
>> @@ -80,6 +80,7 @@ static unsigned int current_array_size;
>>   static unsigned int nr_fds;
>>   
>>   #define ROUNDUP(_x,_w) (((unsigned long)(_x)+(1UL<<(_w))-1) & ~((1UL<<(_w))-1))
>> +#define INVALID_RING_REF    (~(xen_pfn_t)0)
> 
> I am a bit surprised we don't have an INVALID_XEN_PFN definition in the 
> public header. Jan would you have an objection to introduce that?

If this was for a use with a hypercall, I probably wouldn't mind. But
here this is a completely local constant, unrelated to anything Xen
produces or consumes. Of course seeing mention of INVALID_GFN in
a public header comment, there likely are hypercalls that could
benefit from such a constant (or maybe more than one, for the
various frame number flavors).

Jan
Bhupinder Thakur Oct. 16, 2017, 3:47 p.m. | #4
On 16 October 2017 at 15:53, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 16.10.17 at 11:02, <bhupinder.thakur@linaro.org> wrote:
>>  static int console_create_ring(struct console *con)
>>  {
>> -     int err, remote_port, ring_ref, rc;
>> +     int err, remote_port, rc;
>> +     xen_pfn_t ring_ref;
>>       char *type, path[PATH_MAX];
>>       struct domain *dom = con->d;
>>
>>       err = xs_gather(xs, con->xspath,
>> -                     "ring-ref", "%u", &ring_ref,
>> +                     "ring-ref", "%i", &ring_ref,
>
> How would you gather a 64-bit value using %i without any length
> modifier? With just %i you're even going to use partially initialized
> data, so unless somewhere else the upper 32 bits got clipped off
> again the console wouldn't work anymore.
>
I should use "%lli" here to read it as a 64-bit value for all
architectures. Correct?

Regards,
Bhupinder
Jan Beulich Oct. 16, 2017, 3:54 p.m. | #5
>>> On 16.10.17 at 17:47, <bhupinder.thakur@linaro.org> wrote:
> On 16 October 2017 at 15:53, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 16.10.17 at 11:02, <bhupinder.thakur@linaro.org> wrote:
>>>  static int console_create_ring(struct console *con)
>>>  {
>>> -     int err, remote_port, ring_ref, rc;
>>> +     int err, remote_port, rc;
>>> +     xen_pfn_t ring_ref;
>>>       char *type, path[PATH_MAX];
>>>       struct domain *dom = con->d;
>>>
>>>       err = xs_gather(xs, con->xspath,
>>> -                     "ring-ref", "%u", &ring_ref,
>>> +                     "ring-ref", "%i", &ring_ref,
>>
>> How would you gather a 64-bit value using %i without any length
>> modifier? With just %i you're even going to use partially initialized
>> data, so unless somewhere else the upper 32 bits got clipped off
>> again the console wouldn't work anymore.
>>
> I should use "%lli" here to read it as a 64-bit value for all
> architectures. Correct?

No, that's break for a 32-bit tool stack on x86. You really need
an abstraction similar to PRIu_xen_pfn, just that I'd rather not
see SCNi_xen_pfn added to the public headers.

Jan

Patch

diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
index e22009a..6369bf4 100644
--- a/tools/console/daemon/io.c
+++ b/tools/console/daemon/io.c
@@ -80,6 +80,7 @@  static unsigned int current_array_size;
 static unsigned int nr_fds;
 
 #define ROUNDUP(_x,_w) (((unsigned long)(_x)+(1UL<<(_w))-1) & ~((1UL<<(_w))-1))
+#define INVALID_RING_REF    (~(xen_pfn_t)0)
 
 struct buffer {
 	char *data;
@@ -98,7 +99,7 @@  struct console {
 	struct buffer buffer;
 	char *xspath;
 	char *log_suffix;
-	int ring_ref;
+	xen_pfn_t ring_ref;
 	xenevtchn_handle *xce_handle;
 	int xce_pollfd_idx;
 	int event_count;
@@ -651,22 +652,23 @@  static void console_unmap_interface(struct console *con)
 {
 	if (con->interface == NULL)
 		return;
-	if (xgt_handle && con->ring_ref == -1)
+	if (xgt_handle && con->ring_ref == INVALID_RING_REF)
 		xengnttab_unmap(xgt_handle, con->interface, 1);
 	else
 		munmap(con->interface, XC_PAGE_SIZE);
 	con->interface = NULL;
-	con->ring_ref = -1;
+	con->ring_ref = INVALID_RING_REF;
 }
  
 static int console_create_ring(struct console *con)
 {
-	int err, remote_port, ring_ref, rc;
+	int err, remote_port, rc;
+	xen_pfn_t ring_ref;
 	char *type, path[PATH_MAX];
 	struct domain *dom = con->d;
 
 	err = xs_gather(xs, con->xspath,
-			"ring-ref", "%u", &ring_ref,
+			"ring-ref", "%i", &ring_ref,
 			"port", "%i", &remote_port,
 			NULL);
 
@@ -690,7 +692,7 @@  static int console_create_ring(struct console *con)
 	free(type);
 
 	/* If using ring_ref and it has changed, remap */
-	if (ring_ref != con->ring_ref && con->ring_ref != -1)
+	if (ring_ref != con->ring_ref && con->ring_ref != INVALID_RING_REF)
 		console_unmap_interface(con);
 
 	if (!con->interface && xgt_handle && con->use_gnttab) {
@@ -698,14 +700,14 @@  static int console_create_ring(struct console *con)
 		con->interface = xengnttab_map_grant_ref(xgt_handle,
 			dom->domid, GNTTAB_RESERVED_CONSOLE,
 			PROT_READ|PROT_WRITE);
-		con->ring_ref = -1;
+		con->ring_ref = INVALID_RING_REF;
 	}
 	if (!con->interface) {
 		/* Fall back to xc_map_foreign_range */
 		con->interface = xc_map_foreign_range(
 			xc, dom->domid, XC_PAGE_SIZE,
 			PROT_READ|PROT_WRITE,
-			(unsigned long)ring_ref);
+			ring_ref);
 		if (con->interface == NULL) {
 			err = EINVAL;
 			goto out;
@@ -804,7 +806,7 @@  static int console_init(struct console *con, struct domain *dom, void **data)
 	con->master_pollfd_idx = -1;
 	con->slave_fd = -1;
 	con->log_fd = -1;
-	con->ring_ref = -1;
+	con->ring_ref = INVALID_RING_REF;
 	con->local_port = -1;
 	con->remote_port = -1;
 	con->xce_pollfd_idx = -1;
diff --git a/tools/libxc/include/xenctrl_compat.h b/tools/libxc/include/xenctrl_compat.h
index a655e47..2d930d8 100644
--- a/tools/libxc/include/xenctrl_compat.h
+++ b/tools/libxc/include/xenctrl_compat.h
@@ -26,7 +26,7 @@ 
  */
 void *xc_map_foreign_range(xc_interface *xch, uint32_t dom,
                             int size, int prot,
-                            unsigned long mfn );
+                            xen_pfn_t mfn );
 
 void *xc_map_foreign_pages(xc_interface *xch, uint32_t dom, int prot,
                            const xen_pfn_t *arr, int num );
diff --git a/tools/libxc/xc_foreign_memory.c b/tools/libxc/xc_foreign_memory.c
index 4053d26..8b65e52 100644
--- a/tools/libxc/xc_foreign_memory.c
+++ b/tools/libxc/xc_foreign_memory.c
@@ -33,7 +33,7 @@  void *xc_map_foreign_pages(xc_interface *xch, uint32_t dom, int prot,
 
 void *xc_map_foreign_range(xc_interface *xch,
                            uint32_t dom, int size, int prot,
-                           unsigned long mfn)
+                           xen_pfn_t mfn)
 {
     xen_pfn_t *arr;
     int num;
diff --git a/tools/libxl/libxl_console.c b/tools/libxl/libxl_console.c
index c05dc28..f2ca689 100644
--- a/tools/libxl/libxl_console.c
+++ b/tools/libxl/libxl_console.c
@@ -329,7 +329,7 @@  int libxl__device_console_add(libxl__gc *gc, uint32_t domid,
         flexarray_append(ro_front, "port");
         flexarray_append(ro_front, GCSPRINTF("%"PRIu32, state->console_port));
         flexarray_append(ro_front, "ring-ref");
-        flexarray_append(ro_front, GCSPRINTF("%lu", state->console_mfn));
+        flexarray_append(ro_front, GCSPRINTF("%"PRIu_xen_pfn, state->console_mfn));
     } else {
         flexarray_append(front, "state");
         flexarray_append(front, GCSPRINTF("%d", XenbusStateInitialising));
@@ -376,7 +376,7 @@  int libxl__device_vuart_add(libxl__gc *gc, uint32_t domid,
     flexarray_append(ro_front, "port");
     flexarray_append(ro_front, GCSPRINTF("%"PRIu32, state->vuart_port));
     flexarray_append(ro_front, "ring-ref");
-    flexarray_append(ro_front, GCSPRINTF("%"PRI_xen_pfn, state->vuart_gfn));
+    flexarray_append(ro_front, GCSPRINTF("%"PRIu_xen_pfn, state->vuart_gfn));
     flexarray_append(ro_front, "limit");
     flexarray_append(ro_front, GCSPRINTF("%d", LIBXL_XENCONSOLE_LIMIT));
     flexarray_append(ro_front, "type");
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index ef834e6..a58e74f 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -869,7 +869,7 @@  out:
 static int hvm_build_set_params(xc_interface *handle, uint32_t domid,
                                 libxl_domain_build_info *info,
                                 int store_evtchn, unsigned long *store_mfn,
-                                int console_evtchn, unsigned long *console_mfn,
+                                int console_evtchn, xen_pfn_t *console_mfn,
                                 domid_t store_domid, domid_t console_domid)
 {
     struct hvm_info_table *va_hvm;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 45e6df6..f52aeb3 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1128,7 +1128,7 @@  typedef struct {
 
     uint32_t console_port;
     uint32_t console_domid;
-    unsigned long console_mfn;
+    xen_pfn_t console_mfn;
     char *console_tty;
 
     char *saved_state;
diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index 5708cd2..05fd11c 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -274,6 +274,7 @@  DEFINE_XEN_GUEST_HANDLE(vcpu_guest_core_regs_t);
 
 typedef uint64_t xen_pfn_t;
 #define PRI_xen_pfn PRIx64
+#define PRIu_xen_pfn PRIu64
 
 /* Maximum number of virtual CPUs in legacy multi-processor guests. */
 /* Only one. All other VCPUS must use VCPUOP_register_vcpu_info */
diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h
index ff91831..3b0b1d6 100644
--- a/xen/include/public/arch-x86/xen.h
+++ b/xen/include/public/arch-x86/xen.h
@@ -75,6 +75,7 @@  __DeFiNe__ __DECL_REG_LO16(name) e ## name
 #ifndef __ASSEMBLY__
 typedef unsigned long xen_pfn_t;
 #define PRI_xen_pfn "lx"
+#define PRIu_xen_pfn "lu"
 #endif
 
 #define XEN_HAVE_PV_GUEST_ENTRY 1