Re: [Xen-devel] [PATCH 2/5 v2] libxl: Change the type of console_mfn to xen_pfn_t
On Mon, Oct 30, 2017 at 02:42:57PM +0530, Bhupinder Thakur wrote: > Hi, > > On 26 October 2017 at 16:47, Andrew Cooper wrote: > > On 26/10/17 12:13, Wei Liu wrote: > >> On Wed, Oct 25, 2017 at 02:57:05PM +0530, Bhupinder Thakur wrote: > >>> Currently the type of console mfn is unsigned long in libxl. This may be > >>> an issue for 32-bit toolstack running on 64-bit Xen, where the pfn are > >>> 64 bit. To ensure that console_mfn can hold any valid 64-bit pfn, the > >>> type of console_mfn is changed to xen_pfn_t. > >>> > >>> Signed-off-by: Bhupinder Thakur > >>> --- > >>> CC: Ian Jackson > >>> CC: Wei Liu > >>> CC: Stefano Stabellini > >>> CC: Julien Grall > >>> > >>> This patch is as per the review of commit fa1f157 > >>> libxl: Fix the bug introduced in commit "libxl: use correct type > >>> > >>> 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)); > >> Actually, please consider changing console_mfn to console_pfn. > > > > If you are going to make this change, then it is a gfn, not a pfn. > > (console_pfn would be as equally wrong for PV guests as console_mfn is > > currently wrong for HVM guest.) > > Changing console_mfn to console_gfn will require changes in many > files. Should I go ahead and change all the files? > $ cd libxl && git grep \\bconsole_mfn | wc -l 14 Not too bad, so please go ahead. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/5 v2] libxl: Change the type of console_mfn to xen_pfn_t
Hi, On 26 October 2017 at 16:47, Andrew Cooper wrote: > On 26/10/17 12:13, Wei Liu wrote: >> On Wed, Oct 25, 2017 at 02:57:05PM +0530, Bhupinder Thakur wrote: >>> Currently the type of console mfn is unsigned long in libxl. This may be >>> an issue for 32-bit toolstack running on 64-bit Xen, where the pfn are >>> 64 bit. To ensure that console_mfn can hold any valid 64-bit pfn, the >>> type of console_mfn is changed to xen_pfn_t. >>> >>> Signed-off-by: Bhupinder Thakur >>> --- >>> CC: Ian Jackson >>> CC: Wei Liu >>> CC: Stefano Stabellini >>> CC: Julien Grall >>> >>> This patch is as per the review of commit fa1f157 >>> libxl: Fix the bug introduced in commit "libxl: use correct type >>> >>> 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)); >> Actually, please consider changing console_mfn to console_pfn. > > If you are going to make this change, then it is a gfn, not a pfn. > (console_pfn would be as equally wrong for PV guests as console_mfn is > currently wrong for HVM guest.) Changing console_mfn to console_gfn will require changes in many files. Should I go ahead and change all the files? Regards, Bhupinder ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/5 v2] libxl: Change the type of console_mfn to xen_pfn_t
On 26/10/17 12:13, Wei Liu wrote: > On Wed, Oct 25, 2017 at 02:57:05PM +0530, Bhupinder Thakur wrote: >> Currently the type of console mfn is unsigned long in libxl. This may be >> an issue for 32-bit toolstack running on 64-bit Xen, where the pfn are >> 64 bit. To ensure that console_mfn can hold any valid 64-bit pfn, the >> type of console_mfn is changed to xen_pfn_t. >> >> Signed-off-by: Bhupinder Thakur >> --- >> CC: Ian Jackson >> CC: Wei Liu >> CC: Stefano Stabellini >> CC: Julien Grall >> >> This patch is as per the review of commit fa1f157 >> libxl: Fix the bug introduced in commit "libxl: use correct type >> >> 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)); > Actually, please consider changing console_mfn to console_pfn. If you are going to make this change, then it is a gfn, not a pfn. (console_pfn would be as equally wrong for PV guests as console_mfn is currently wrong for HVM guest.) ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/5 v2] libxl: Change the type of console_mfn to xen_pfn_t
On Wed, Oct 25, 2017 at 02:57:05PM +0530, Bhupinder Thakur wrote: > Currently the type of console mfn is unsigned long in libxl. This may be > an issue for 32-bit toolstack running on 64-bit Xen, where the pfn are > 64 bit. To ensure that console_mfn can hold any valid 64-bit pfn, the > type of console_mfn is changed to xen_pfn_t. > > Signed-off-by: Bhupinder Thakur > --- > CC: Ian Jackson > CC: Wei Liu > CC: Stefano Stabellini > CC: Julien Grall > > This patch is as per the review of commit fa1f157 > libxl: Fix the bug introduced in commit "libxl: use correct type > > 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)); Actually, please consider changing console_mfn to console_pfn. You can keep my ack if you make such change. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/5 v2] libxl: Change the type of console_mfn to xen_pfn_t
On Wed, Oct 25, 2017 at 02:57:05PM +0530, Bhupinder Thakur wrote: > Currently the type of console mfn is unsigned long in libxl. This may be > an issue for 32-bit toolstack running on 64-bit Xen, where the pfn are > 64 bit. To ensure that console_mfn can hold any valid 64-bit pfn, the > type of console_mfn is changed to xen_pfn_t. > > Signed-off-by: Bhupinder Thakur Acked-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 2/5 v2] libxl: Change the type of console_mfn to xen_pfn_t
Currently the type of console mfn is unsigned long in libxl. This may be an issue for 32-bit toolstack running on 64-bit Xen, where the pfn are 64 bit. To ensure that console_mfn can hold any valid 64-bit pfn, the type of console_mfn is changed to xen_pfn_t. Signed-off-by: Bhupinder Thakur --- CC: Ian Jackson CC: Wei Liu CC: Stefano Stabellini CC: Julien Grall This patch is as per the review of commit fa1f157 libxl: Fix the bug introduced in commit "libxl: use correct type 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