| Message ID | 1508923628-26446-2-git-send-email-bhupinder.thakur@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <bhupinder.thakur@linaro.org> Acked-by: Wei Liu <wei.liu2@citrix.com>
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 <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> > > 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.
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 <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> >> >> 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
Hi, On 26 October 2017 at 16:47, Andrew Cooper <andrew.cooper3@citrix.com> 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 <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> >>> >>> 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
On Mon, Oct 30, 2017 at 02:42:57PM +0530, Bhupinder Thakur wrote: > Hi, > > On 26 October 2017 at 16:47, Andrew Cooper <andrew.cooper3@citrix.com> 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 <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> > >>> > >>> 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.
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;
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 <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> 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(-)