* libxl: Fix the bug introduced in commit "libxl: use correct type modifier for vuart_gfn"
@ 2017-10-17 16:46 Bhupinder Thakur
2017-10-17 16:46 ` libxl: Change the type of console_mfn to xen_pfn_t Bhupinder Thakur
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Bhupinder Thakur @ 2017-10-17 16:46 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.
This patch introduces a new format specifier "PRIu_xen_pfn" which formats the value as a
decimal value.
Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
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/libxl/libxl_console.c | 2 +-
xen/include/public/arch-arm.h | 1 +
xen/include/public/arch-x86/xen.h | 1 +
3 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/libxl/libxl_console.c b/tools/libxl/libxl_console.c
index c05dc28..6bfc0e5 100644
--- a/tools/libxl/libxl_console.c
+++ b/tools/libxl/libxl_console.c
@@ -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/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] 11+ messages in thread
* libxl: Change the type of console_mfn to xen_pfn_t
2017-10-17 16:46 libxl: Fix the bug introduced in commit "libxl: use correct type modifier for vuart_gfn" Bhupinder Thakur
@ 2017-10-17 16:46 ` Bhupinder Thakur
2017-10-17 20:43 ` Konrad Rzeszutek Wilk
2017-10-17 16:46 ` xenconsole: Change the type of ring_ref to xen_pfn_t in console_create_ring Bhupinder Thakur
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Bhupinder Thakur @ 2017-10-17 16:46 UTC (permalink / raw)
To: xen-devel; +Cc: Wei Liu, Julien Grall, Stefano Stabellini, Ian Jackson
Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
---
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>
tools/libxl/libxl_console.c | 2 +-
tools/libxl/libxl_dom.c | 2 +-
tools/libxl/libxl_internal.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/libxl/libxl_console.c b/tools/libxl/libxl_console.c
index 6bfc0e5..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));
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;
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* xenconsole: Change the type of ring_ref to xen_pfn_t in console_create_ring
2017-10-17 16:46 libxl: Fix the bug introduced in commit "libxl: use correct type modifier for vuart_gfn" Bhupinder Thakur
2017-10-17 16:46 ` libxl: Change the type of console_mfn to xen_pfn_t Bhupinder Thakur
@ 2017-10-17 16:46 ` Bhupinder Thakur
2017-10-17 16:46 ` libxc: Fix the data type of mfn parameter passed to xc_map_foreign_range() Bhupinder Thakur
2017-10-17 16:46 ` xenconsole: Define and use a macro INVALID_XEN_PFN instead of -1 Bhupinder Thakur
3 siblings, 0 replies; 11+ messages in thread
From: Bhupinder Thakur @ 2017-10-17 16:46 UTC (permalink / raw)
To: xen-devel; +Cc: Wei Liu, Julien Grall, Stefano Stabellini, Ian Jackson
Currently, ring_ref is read as an integer in console_create_ring which could lead to
truncation of the value as it is reading a 64-bit value.
The fix is to modify the type of ring_ref to xen_pfn_t and use the correct format
specifier to read the value correctly for all architectures.
Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
---
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>
tools/console/daemon/io.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
index e22009a..1839973 100644
--- a/tools/console/daemon/io.c
+++ b/tools/console/daemon/io.c
@@ -19,6 +19,7 @@
#define _GNU_SOURCE
+#include <inttypes.h>
#include "utils.h"
#include "io.h"
#include <xenevtchn.h>
@@ -81,6 +82,12 @@ static unsigned int nr_fds;
#define ROUNDUP(_x,_w) (((unsigned long)(_x)+(1UL<<(_w))-1) & ~((1UL<<(_w))-1))
+#if defined(CONFIG_ARM)
+# define SCNi_xen_pfn SCNi64
+#else
+# define SCNi_xen_pfn "li"
+#endif
+
struct buffer {
char *data;
size_t consumed;
@@ -98,7 +105,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;
@@ -661,12 +668,13 @@ static void console_unmap_interface(struct console *con)
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", "%"SCNi_xen_pfn, &ring_ref,
"port", "%i", &remote_port,
NULL);
@@ -705,7 +713,7 @@ static int console_create_ring(struct console *con)
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;
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* libxc: Fix the data type of mfn parameter passed to xc_map_foreign_range()
2017-10-17 16:46 libxl: Fix the bug introduced in commit "libxl: use correct type modifier for vuart_gfn" Bhupinder Thakur
2017-10-17 16:46 ` libxl: Change the type of console_mfn to xen_pfn_t Bhupinder Thakur
2017-10-17 16:46 ` xenconsole: Change the type of ring_ref to xen_pfn_t in console_create_ring Bhupinder Thakur
@ 2017-10-17 16:46 ` Bhupinder Thakur
2017-10-18 11:52 ` Wei Liu
2017-10-17 16:46 ` xenconsole: Define and use a macro INVALID_XEN_PFN instead of -1 Bhupinder Thakur
3 siblings, 1 reply; 11+ messages in thread
From: Bhupinder Thakur @ 2017-10-17 16:46 UTC (permalink / raw)
To: xen-devel; +Cc: Wei Liu, Julien Grall, Stefano Stabellini, Ian Jackson
Currently the data type of mfn paramter passed to xc_map_foreign_range() is unsigned
long. This could be problem for 32-bit arm architectures where the lengh of long is
32 bits while mfn happens to be a 64-bit value.
To avoid truncating a 64-bit value, the type of mfn is changed from "unsigned long" to
xen_pfn_t. Also the parameter name "mfn" is changed to "pfn" which is a more accurate
indication of what this parameter represents.
Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
---
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>
tools/libxc/include/xenctrl_compat.h | 2 +-
tools/libxc/xc_foreign_memory.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/libxc/include/xenctrl_compat.h b/tools/libxc/include/xenctrl_compat.h
index a655e47..5ee72bf 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 pfn);
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..c1f114a 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 pfn)
{
xen_pfn_t *arr;
int num;
@@ -46,7 +46,7 @@ void *xc_map_foreign_range(xc_interface *xch,
return NULL;
for ( i = 0; i < num; i++ )
- arr[i] = mfn + i;
+ arr[i] = pfn + i;
ret = xc_map_foreign_pages(xch, dom, prot, arr, num);
free(arr);
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* xenconsole: Define and use a macro INVALID_XEN_PFN instead of -1
2017-10-17 16:46 libxl: Fix the bug introduced in commit "libxl: use correct type modifier for vuart_gfn" Bhupinder Thakur
` (2 preceding siblings ...)
2017-10-17 16:46 ` libxc: Fix the data type of mfn parameter passed to xc_map_foreign_range() Bhupinder Thakur
@ 2017-10-17 16:46 ` Bhupinder Thakur
2017-10-18 10:02 ` Jan Beulich
3 siblings, 1 reply; 11+ messages in thread
From: Bhupinder Thakur @ 2017-10-17 16:46 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, Julien Grall,
Jan Beulich
xenconsole will use a new macro INVALID_XEN_PFN instead of -1 for initializing ring-ref.
Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
---
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>
tools/console/daemon/io.c | 10 +++++-----
xen/include/public/xen.h | 2 ++
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
index 1839973..9129f5a 100644
--- a/tools/console/daemon/io.c
+++ b/tools/console/daemon/io.c
@@ -658,12 +658,12 @@ 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_XEN_PFN)
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_XEN_PFN;
}
static int console_create_ring(struct console *con)
@@ -698,7 +698,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_XEN_PFN)
console_unmap_interface(con);
if (!con->interface && xgt_handle && con->use_gnttab) {
@@ -706,7 +706,7 @@ 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_XEN_PFN;
}
if (!con->interface) {
/* Fall back to xc_map_foreign_range */
@@ -812,7 +812,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_XEN_PFN;
con->local_port = -1;
con->remote_port = -1;
con->xce_pollfd_idx = -1;
diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
index 308109f..fc383ca 100644
--- a/xen/include/public/xen.h
+++ b/xen/include/public/xen.h
@@ -37,6 +37,8 @@
#error "Unsupported architecture"
#endif
+#define INVALID_XEN_PFN (~(xen_pfn_t)0)
+
#ifndef __ASSEMBLY__
/* Guest handles for primitive C types. */
DEFINE_XEN_GUEST_HANDLE(char);
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: libxl: Change the type of console_mfn to xen_pfn_t
2017-10-17 16:46 ` libxl: Change the type of console_mfn to xen_pfn_t Bhupinder Thakur
@ 2017-10-17 20:43 ` Konrad Rzeszutek Wilk
0 siblings, 0 replies; 11+ messages in thread
From: Konrad Rzeszutek Wilk @ 2017-10-17 20:43 UTC (permalink / raw)
To: Bhupinder Thakur
Cc: xen-devel, Julien Grall, Stefano Stabellini, Wei Liu, Ian Jackson
On Tue, Oct 17, 2017 at 10:16:30PM +0530, Bhupinder Thakur wrote:
One also explains the rationale in the commit. Perhaps also
include 'Reported-by' ..
> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
> ---
> 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>
>
> tools/libxl/libxl_console.c | 2 +-
> tools/libxl/libxl_dom.c | 2 +-
> tools/libxl/libxl_internal.h | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/libxl/libxl_console.c b/tools/libxl/libxl_console.c
> index 6bfc0e5..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));
> 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;
> --
> 2.7.4
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: xenconsole: Define and use a macro INVALID_XEN_PFN instead of -1
2017-10-17 16:46 ` xenconsole: Define and use a macro INVALID_XEN_PFN instead of -1 Bhupinder Thakur
@ 2017-10-18 10:02 ` Jan Beulich
2017-10-18 11:53 ` Wei Liu
0 siblings, 1 reply; 11+ messages in thread
From: Jan Beulich @ 2017-10-18 10:02 UTC (permalink / raw)
To: Bhupinder Thakur
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, Julien Grall,
xen-devel
>>> On 17.10.17 at 18:46, <bhupinder.thakur@linaro.org> wrote:
> --- a/tools/console/daemon/io.c
> +++ b/tools/console/daemon/io.c
> @@ -658,12 +658,12 @@ 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_XEN_PFN)
> 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_XEN_PFN;
> }
>
> static int console_create_ring(struct console *con)
> @@ -698,7 +698,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_XEN_PFN)
> console_unmap_interface(con);
>
> if (!con->interface && xgt_handle && con->use_gnttab) {
> @@ -706,7 +706,7 @@ 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_XEN_PFN;
> }
> if (!con->interface) {
> /* Fall back to xc_map_foreign_range */
> @@ -812,7 +812,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_XEN_PFN;
> con->local_port = -1;
> con->remote_port = -1;
> con->xce_pollfd_idx = -1;
> --- a/xen/include/public/xen.h
> +++ b/xen/include/public/xen.h
> @@ -37,6 +37,8 @@
> #error "Unsupported architecture"
> #endif
>
> +#define INVALID_XEN_PFN (~(xen_pfn_t)0)
As said before, the uses of this which you introduce don't warrant
this addition to the public interface (which, if it was added, also
should start with XEN_). I'm not going to NAK such a (corrected)
addition to the public interface, but given the users I'm also not
going to ACK it (but perhaps another REST maintainer would).
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: libxc: Fix the data type of mfn parameter passed to xc_map_foreign_range()
2017-10-17 16:46 ` libxc: Fix the data type of mfn parameter passed to xc_map_foreign_range() Bhupinder Thakur
@ 2017-10-18 11:52 ` Wei Liu
0 siblings, 0 replies; 11+ messages in thread
From: Wei Liu @ 2017-10-18 11:52 UTC (permalink / raw)
To: Bhupinder Thakur
Cc: xen-devel, Julien Grall, Stefano Stabellini, Ian Jackson, Wei Liu
I appear to receive this series out of order.
This one appears in my inbox later then "xenconsole: Change the type of
ring_ref to xen_pfn_t in console_create_ring", which at a glance depends
on this one.
Please resend this series with proper numbering.
On Tue, Oct 17, 2017 at 10:16:32PM +0530, Bhupinder Thakur wrote:
> Currently the data type of mfn paramter passed to xc_map_foreign_range() is unsigned
> long. This could be problem for 32-bit arm architectures where the lengh of long is
> 32 bits while mfn happens to be a 64-bit value.
>
> To avoid truncating a 64-bit value, the type of mfn is changed from "unsigned long" to
> xen_pfn_t. Also the parameter name "mfn" is changed to "pfn" which is a more accurate
> indication of what this parameter represents.
>
> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
> ---
> 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>
>
> tools/libxc/include/xenctrl_compat.h | 2 +-
> tools/libxc/xc_foreign_memory.c | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/libxc/include/xenctrl_compat.h b/tools/libxc/include/xenctrl_compat.h
> index a655e47..5ee72bf 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 pfn);
>
> 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..c1f114a 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 pfn)
> {
> xen_pfn_t *arr;
> int num;
> @@ -46,7 +46,7 @@ void *xc_map_foreign_range(xc_interface *xch,
> return NULL;
>
> for ( i = 0; i < num; i++ )
> - arr[i] = mfn + i;
> + arr[i] = pfn + i;
>
> ret = xc_map_foreign_pages(xch, dom, prot, arr, num);
> free(arr);
> --
> 2.7.4
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: xenconsole: Define and use a macro INVALID_XEN_PFN instead of -1
2017-10-18 10:02 ` Jan Beulich
@ 2017-10-18 11:53 ` Wei Liu
2017-10-18 13:05 ` Julien Grall
0 siblings, 1 reply; 11+ messages in thread
From: Wei Liu @ 2017-10-18 11:53 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, Julien Grall,
Bhupinder Thakur, xen-devel
On Wed, Oct 18, 2017 at 04:02:45AM -0600, Jan Beulich wrote:
> >>> On 17.10.17 at 18:46, <bhupinder.thakur@linaro.org> wrote:
> > --- a/tools/console/daemon/io.c
> > +++ b/tools/console/daemon/io.c
> > @@ -658,12 +658,12 @@ 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_XEN_PFN)
> > 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_XEN_PFN;
> > }
> >
> > static int console_create_ring(struct console *con)
> > @@ -698,7 +698,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_XEN_PFN)
> > console_unmap_interface(con);
> >
> > if (!con->interface && xgt_handle && con->use_gnttab) {
> > @@ -706,7 +706,7 @@ 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_XEN_PFN;
> > }
> > if (!con->interface) {
> > /* Fall back to xc_map_foreign_range */
> > @@ -812,7 +812,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_XEN_PFN;
> > con->local_port = -1;
> > con->remote_port = -1;
> > con->xce_pollfd_idx = -1;
> > --- a/xen/include/public/xen.h
> > +++ b/xen/include/public/xen.h
> > @@ -37,6 +37,8 @@
> > #error "Unsupported architecture"
> > #endif
> >
> > +#define INVALID_XEN_PFN (~(xen_pfn_t)0)
>
> As said before, the uses of this which you introduce don't warrant
> this addition to the public interface (which, if it was added, also
> should start with XEN_). I'm not going to NAK such a (corrected)
> addition to the public interface, but given the users I'm also not
> going to ACK it (but perhaps another REST maintainer would).
>
I agree with you here. We don't need this in public interface yet.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: xenconsole: Define and use a macro INVALID_XEN_PFN instead of -1
2017-10-18 11:53 ` Wei Liu
@ 2017-10-18 13:05 ` Julien Grall
2017-10-18 13:16 ` Jan Beulich
0 siblings, 1 reply; 11+ messages in thread
From: Julien Grall @ 2017-10-18 13:05 UTC (permalink / raw)
To: Wei Liu, Jan Beulich
Cc: Stefano Stabellini, Konrad Rzeszutek Wilk, George Dunlap,
Andrew Cooper, Ian Jackson, Julien Grall, Bhupinder Thakur,
xen-devel
Hi Wei,
On 10/18/2017 12:53 PM, Wei Liu wrote:
> On Wed, Oct 18, 2017 at 04:02:45AM -0600, Jan Beulich wrote:
>>>>> On 17.10.17 at 18:46, <bhupinder.thakur@linaro.org> wrote:
>>> --- a/xen/include/public/xen.h
>>> +++ b/xen/include/public/xen.h
>>> @@ -37,6 +37,8 @@
>>> #error "Unsupported architecture"
>>> #endif
>>>
>>> +#define INVALID_XEN_PFN (~(xen_pfn_t)0)
>>
>> As said before, the uses of this which you introduce don't warrant
>> this addition to the public interface (which, if it was added, also
>> should start with XEN_). I'm not going to NAK such a (corrected)
>> addition to the public interface, but given the users I'm also not
>> going to ACK it (but perhaps another REST maintainer would).
>>
>
> I agree with you here. We don't need this in public interface yet.
Couldn't this new define be used in place like xc_mem_access.c (they
have a plain ~0UL for invalid GFN) or even LIBXL_INVALID_GFN?
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: xenconsole: Define and use a macro INVALID_XEN_PFN instead of -1
2017-10-18 13:05 ` Julien Grall
@ 2017-10-18 13:16 ` Jan Beulich
0 siblings, 0 replies; 11+ messages in thread
From: Jan Beulich @ 2017-10-18 13:16 UTC (permalink / raw)
To: Julien Grall
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, Julien Grall,
Bhupinder Thakur, xen-devel
>>> On 18.10.17 at 15:05, <julien.grall@linaro.org> wrote:
> On 10/18/2017 12:53 PM, Wei Liu wrote:
>> On Wed, Oct 18, 2017 at 04:02:45AM -0600, Jan Beulich wrote:
>>>>>> On 17.10.17 at 18:46, <bhupinder.thakur@linaro.org> wrote:
>>>> --- a/xen/include/public/xen.h
>>>> +++ b/xen/include/public/xen.h
>>>> @@ -37,6 +37,8 @@
>>>> #error "Unsupported architecture"
>>>> #endif
>>>>
>>>> +#define INVALID_XEN_PFN (~(xen_pfn_t)0)
>>>
>>> As said before, the uses of this which you introduce don't warrant
>>> this addition to the public interface (which, if it was added, also
>>> should start with XEN_). I'm not going to NAK such a (corrected)
>>> addition to the public interface, but given the users I'm also not
>>> going to ACK it (but perhaps another REST maintainer would).
>>>
>>
>> I agree with you here. We don't need this in public interface yet.
>
> Couldn't this new define be used in place like xc_mem_access.c (they
> have a plain ~0UL for invalid GFN) or even LIBXL_INVALID_GFN?
Sure it could be, but it should be introduced to the public header
when a hypercall producer and/or consumer appears, not for the
internal purposes of xenconsole.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-10-18 13:16 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-17 16:46 libxl: Fix the bug introduced in commit "libxl: use correct type modifier for vuart_gfn" Bhupinder Thakur
2017-10-17 16:46 ` libxl: Change the type of console_mfn to xen_pfn_t Bhupinder Thakur
2017-10-17 20:43 ` Konrad Rzeszutek Wilk
2017-10-17 16:46 ` xenconsole: Change the type of ring_ref to xen_pfn_t in console_create_ring Bhupinder Thakur
2017-10-17 16:46 ` libxc: Fix the data type of mfn parameter passed to xc_map_foreign_range() Bhupinder Thakur
2017-10-18 11:52 ` Wei Liu
2017-10-17 16:46 ` xenconsole: Define and use a macro INVALID_XEN_PFN instead of -1 Bhupinder Thakur
2017-10-18 10:02 ` Jan Beulich
2017-10-18 11:53 ` Wei Liu
2017-10-18 13:05 ` Julien Grall
2017-10-18 13:16 ` Jan Beulich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.