* libxl/xenconsole: vpl011: Fix hex to dec conversion of vuart_gfn in libxl__device_vuart_add
@ 2017-10-16 9:02 Bhupinder Thakur
2017-10-16 10:18 ` Julien Grall
2017-10-16 10:23 ` Jan Beulich
0 siblings, 2 replies; 6+ messages in thread
From: Bhupinder Thakur @ 2017-10-16 9:02 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Ian Jackson,
Julien Grall, Jan Beulich
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(-)
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
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: libxl/xenconsole: vpl011: Fix hex to dec conversion of vuart_gfn in libxl__device_vuart_add
2017-10-16 9:02 libxl/xenconsole: vpl011: Fix hex to dec conversion of vuart_gfn in libxl__device_vuart_add Bhupinder Thakur
@ 2017-10-16 10:18 ` Julien Grall
2017-10-16 10:28 ` Jan Beulich
2017-10-16 10:23 ` Jan Beulich
1 sibling, 1 reply; 6+ messages in thread
From: Julien Grall @ 2017-10-16 10:18 UTC (permalink / raw)
To: Bhupinder Thakur, xen-devel
Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Ian Jackson,
Julien Grall, Jan Beulich
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,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: libxl/xenconsole: vpl011: Fix hex to dec conversion of vuart_gfn in libxl__device_vuart_add
2017-10-16 10:18 ` Julien Grall
@ 2017-10-16 10:28 ` Jan Beulich
0 siblings, 0 replies; 6+ messages in thread
From: Jan Beulich @ 2017-10-16 10:28 UTC (permalink / raw)
To: Julien Grall
Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Ian Jackson,
Julien Grall, Bhupinder Thakur, xen-devel
>>> 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
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: libxl/xenconsole: vpl011: Fix hex to dec conversion of vuart_gfn in libxl__device_vuart_add
2017-10-16 9:02 libxl/xenconsole: vpl011: Fix hex to dec conversion of vuart_gfn in libxl__device_vuart_add Bhupinder Thakur
2017-10-16 10:18 ` Julien Grall
@ 2017-10-16 10:23 ` Jan Beulich
2017-10-16 15:47 ` Bhupinder Thakur
1 sibling, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2017-10-16 10:23 UTC (permalink / raw)
To: Bhupinder Thakur
Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Ian Jackson,
Julien Grall, xen-devel
>>> 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
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: libxl/xenconsole: vpl011: Fix hex to dec conversion of vuart_gfn in libxl__device_vuart_add
2017-10-16 10:23 ` Jan Beulich
@ 2017-10-16 15:47 ` Bhupinder Thakur
2017-10-16 15:54 ` Jan Beulich
0 siblings, 1 reply; 6+ messages in thread
From: Bhupinder Thakur @ 2017-10-16 15:47 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Ian Jackson,
Julien Grall, Xen-devel
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
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: libxl/xenconsole: vpl011: Fix hex to dec conversion of vuart_gfn in libxl__device_vuart_add
2017-10-16 15:47 ` Bhupinder Thakur
@ 2017-10-16 15:54 ` Jan Beulich
0 siblings, 0 replies; 6+ messages in thread
From: Jan Beulich @ 2017-10-16 15:54 UTC (permalink / raw)
To: Bhupinder Thakur
Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Ian Jackson,
Julien Grall, Xen-devel
>>> 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
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-10-16 15:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-16 9:02 libxl/xenconsole: vpl011: Fix hex to dec conversion of vuart_gfn in libxl__device_vuart_add Bhupinder Thakur
2017-10-16 10:18 ` Julien Grall
2017-10-16 10:28 ` Jan Beulich
2017-10-16 10:23 ` Jan Beulich
2017-10-16 15:47 ` Bhupinder Thakur
2017-10-16 15:54 ` Jan Beulich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).