| Message ID | 1507891472-4701-1-git-send-email-bhupinder.thakur@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [Xen-devel] libxl: vpl011: Fix hex to dec conversion of vuart_gfn in libxl__device_vuart_add | expand |
On Fri, Oct 13, 2017 at 04:14:32PM +0530, 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. > > Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org> Acked-by: Wei Liu <wei.liu2@citrix.com>
>>> On 13.10.17 at 12:44, <bhupinder.thakur@linaro.org> 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. Is this generic or vuart specific code in xenstore that does so? Could you perhaps simply point me at the consuming side? > Introduced a new format string "PRIu_xen_pfn" which formats the value as a > decimal value. I ask because I'm not really happy about this addition, i.e. I'd prefer the read side to change. Jan
On 13/10/17 13:08, Jan Beulich wrote: >>>> On 13.10.17 at 12:44, <bhupinder.thakur@linaro.org> 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. > Is this generic or vuart specific code in xenstore that does so? > Could you perhaps simply point me at the consuming side? > >> Introduced a new format string "PRIu_xen_pfn" which formats the value as a >> decimal value. > I ask because I'm not really happy about this addition, i.e. I'd > prefer the read side to change. Can the read side realistically change? Its been decimal for a decade now, and there definitely is 3rd party code which uses these values in xenstore (sadly). Then again, the ring-ref key is listed as deprecated in our documentation, without any reference describing which key should be used instead. It is also typically a grant reference, not a gfn, so something wonky is definitely going on here. ~Andrew
>>> On 13.10.17 at 14:19, <andrew.cooper3@citrix.com> wrote: > On 13/10/17 13:08, Jan Beulich wrote: >>>>> On 13.10.17 at 12:44, <bhupinder.thakur@linaro.org> 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. >> Is this generic or vuart specific code in xenstore that does so? >> Could you perhaps simply point me at the consuming side? >> >>> Introduced a new format string "PRIu_xen_pfn" which formats the value as a >>> decimal value. >> I ask because I'm not really happy about this addition, i.e. I'd >> prefer the read side to change. > > Can the read side realistically change? Well, that's why I had asked whether this is generic or specific code. I would have hoped/assumed that xenstore doesn't generically try to translate strings into numbers - how would it know a string is to represent a number in the first place? Hence I was hoping for this to be specific (and hence) new code. > Its been decimal for a decade now, and there definitely is 3rd party > code which uses these values in xenstore (sadly). Are you trying to tell me there's been a vuart frontend before the device type introduction in libxl, or is the new device type compatible with an existing one? > Then again, the ring-ref key is listed as deprecated in our > documentation, without any reference describing which key should be used > instead. It is also typically a grant reference, not a gfn, so > something wonky is definitely going on here. Which put under question how an existing frontend could work with this new device type. Jan
Hi, On 13/10/17 13:32, Jan Beulich wrote: >>>> On 13.10.17 at 14:19, <andrew.cooper3@citrix.com> wrote: >> On 13/10/17 13:08, Jan Beulich wrote: >>>>>> On 13.10.17 at 12:44, <bhupinder.thakur@linaro.org> 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. >>> Is this generic or vuart specific code in xenstore that does so? >>> Could you perhaps simply point me at the consuming side? >>> >>>> Introduced a new format string "PRIu_xen_pfn" which formats the value as a >>>> decimal value. >>> I ask because I'm not really happy about this addition, i.e. I'd >>> prefer the read side to change. >> >> Can the read side realistically change? > > Well, that's why I had asked whether this is generic or specific > code. I would have hoped/assumed that xenstore doesn't > generically try to translate strings into numbers - how would it > know a string is to represent a number in the first place? Hence > I was hoping for this to be specific (and hence) new code. > >> Its been decimal for a decade now, and there definitely is 3rd party >> code which uses these values in xenstore (sadly). > > Are you trying to tell me there's been a vuart frontend before > the device type introduction in libxl, or is the new device type > compatible with an existing one? > >> Then again, the ring-ref key is listed as deprecated in our >> documentation, without any reference describing which key should be used >> instead. It is also typically a grant reference, not a gfn, so >> something wonky is definitely going on here. > > Which put under question how an existing frontend could work > with this new device type. Well, vuart is replicating the behavior of console (see libxl__device_console_add). The console is passing a frame number in decimal in "ring-ref". Confusingly it is an MFN and would even break on 32-bit toolstack using 64-bit Xen... So this patch is just following the console behavior by passing a decimal value rather than an hexadecimal value. Cheers,
>>> On 13.10.17 at 15:03, <julien.grall@linaro.org> wrote: > On 13/10/17 13:32, Jan Beulich wrote: >>>>> On 13.10.17 at 14:19, <andrew.cooper3@citrix.com> wrote: >>> On 13/10/17 13:08, Jan Beulich wrote: >>>>>>> On 13.10.17 at 12:44, <bhupinder.thakur@linaro.org> 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. >>>> Is this generic or vuart specific code in xenstore that does so? >>>> Could you perhaps simply point me at the consuming side? >>>> >>>>> Introduced a new format string "PRIu_xen_pfn" which formats the value as a >>>>> decimal value. >>>> I ask because I'm not really happy about this addition, i.e. I'd >>>> prefer the read side to change. >>> >>> Can the read side realistically change? >> >> Well, that's why I had asked whether this is generic or specific >> code. I would have hoped/assumed that xenstore doesn't >> generically try to translate strings into numbers - how would it >> know a string is to represent a number in the first place? Hence >> I was hoping for this to be specific (and hence) new code. >> >>> Its been decimal for a decade now, and there definitely is 3rd party >>> code which uses these values in xenstore (sadly). >> >> Are you trying to tell me there's been a vuart frontend before >> the device type introduction in libxl, or is the new device type >> compatible with an existing one? >> >>> Then again, the ring-ref key is listed as deprecated in our >>> documentation, without any reference describing which key should be used >>> instead. It is also typically a grant reference, not a gfn, so >>> something wonky is definitely going on here. >> >> Which put under question how an existing frontend could work >> with this new device type. > > Well, vuart is replicating the behavior of console (see > libxl__device_console_add). The console is passing a frame number in > decimal in "ring-ref". Confusingly it is an MFN and would even break on > 32-bit toolstack using 64-bit Xen... > > So this patch is just following the console behavior by passing a > decimal value rather than an hexadecimal value. Well, that other code path should then also use PRIu_xen_pfn, at the very least. It's of course interesting that the apparent consumer of this (tools/console/daemon/io.c:domain_create_ring()) uses err = xs_gather(xs, dom->conspath, "ring-ref", "%u", &ring_ref, "port", "%i", &remote_port, NULL); in order to then cast(!) the result to unsigned long in the invocation of xc_map_foreign_range(). Suggests to me that the console can't work reliably on a system with memory extending past the 1Tb boundary. It of course escapes me why %i (or really %lli) wasn't used here from the beginning, eliminating all radix concerns and matching what is being done for the port. Jan
Hi Jan, On 13/10/17 15:03, Jan Beulich wrote: >>>> On 13.10.17 at 15:03, <julien.grall@linaro.org> wrote: >> On 13/10/17 13:32, Jan Beulich wrote: >>>>>> On 13.10.17 at 14:19, <andrew.cooper3@citrix.com> wrote: >>>> On 13/10/17 13:08, Jan Beulich wrote: >>>>>>>> On 13.10.17 at 12:44, <bhupinder.thakur@linaro.org> 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. >>>>> Is this generic or vuart specific code in xenstore that does so? >>>>> Could you perhaps simply point me at the consuming side? >>>>> >>>>>> Introduced a new format string "PRIu_xen_pfn" which formats the value as a >>>>>> decimal value. >>>>> I ask because I'm not really happy about this addition, i.e. I'd >>>>> prefer the read side to change. >>>> >>>> Can the read side realistically change? >>> >>> Well, that's why I had asked whether this is generic or specific >>> code. I would have hoped/assumed that xenstore doesn't >>> generically try to translate strings into numbers - how would it >>> know a string is to represent a number in the first place? Hence >>> I was hoping for this to be specific (and hence) new code. >>> >>>> Its been decimal for a decade now, and there definitely is 3rd party >>>> code which uses these values in xenstore (sadly). >>> >>> Are you trying to tell me there's been a vuart frontend before >>> the device type introduction in libxl, or is the new device type >>> compatible with an existing one? >>> >>>> Then again, the ring-ref key is listed as deprecated in our >>>> documentation, without any reference describing which key should be used >>>> instead. It is also typically a grant reference, not a gfn, so >>>> something wonky is definitely going on here. >>> >>> Which put under question how an existing frontend could work >>> with this new device type. >> >> Well, vuart is replicating the behavior of console (see >> libxl__device_console_add). The console is passing a frame number in >> decimal in "ring-ref". Confusingly it is an MFN and would even break on >> 32-bit toolstack using 64-bit Xen... >> >> So this patch is just following the console behavior by passing a >> decimal value rather than an hexadecimal value. > > Well, that other code path should then also use PRIu_xen_pfn, at > the very least. By other code path, you mean the console code right? In that case, mfn should also be moved from unsigned long to xen_pfn_t. > It's of course interesting that the apparent consumer > of this (tools/console/daemon/io.c:domain_create_ring()) uses > > err = xs_gather(xs, dom->conspath, > "ring-ref", "%u", &ring_ref, > "port", "%i", &remote_port, > NULL); > > in order to then cast(!) the result to unsigned long in the > invocation of xc_map_foreign_range(). Suggests to me that > the console can't work reliably on a system with memory > extending past the 1Tb boundary. It likely a latent bug. Probably a silly question, would there any compatibility issue to switch the format to the correct one? > > It of course escapes me why %i (or really %lli) wasn't used here > from the beginning, eliminating all radix concerns and matching > what is being done for the port. Why %i? Should not the GFN be unsigned? Although, I can see the field ring_reg is int and will store -1 as not mapped. This is quite confusing and likely we want to turned into xen_pfn_t + use ~(xen_pfn_t)0. But then, xc_map_foreign_range is using unsigned long instead of xen_pfn_t. So I guess we should also switch the parameter to xen_pfn_t. Note that the implementation of xc_map_foreign_range is using xen_pfn_t. Cheers,
>>> On 13.10.17 at 16:35, <julien.grall@linaro.org> wrote: > Hi Jan, > > On 13/10/17 15:03, Jan Beulich wrote: >>>>> On 13.10.17 at 15:03, <julien.grall@linaro.org> wrote: >>> On 13/10/17 13:32, Jan Beulich wrote: >>>>>>> On 13.10.17 at 14:19, <andrew.cooper3@citrix.com> wrote: >>>>> On 13/10/17 13:08, Jan Beulich wrote: >>>>>>>>> On 13.10.17 at 12:44, <bhupinder.thakur@linaro.org> 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. >>>>>> Is this generic or vuart specific code in xenstore that does so? >>>>>> Could you perhaps simply point me at the consuming side? >>>>>> >>>>>>> Introduced a new format string "PRIu_xen_pfn" which formats the value as a >>>>>>> decimal value. >>>>>> I ask because I'm not really happy about this addition, i.e. I'd >>>>>> prefer the read side to change. >>>>> >>>>> Can the read side realistically change? >>>> >>>> Well, that's why I had asked whether this is generic or specific >>>> code. I would have hoped/assumed that xenstore doesn't >>>> generically try to translate strings into numbers - how would it >>>> know a string is to represent a number in the first place? Hence >>>> I was hoping for this to be specific (and hence) new code. >>>> >>>>> Its been decimal for a decade now, and there definitely is 3rd party >>>>> code which uses these values in xenstore (sadly). >>>> >>>> Are you trying to tell me there's been a vuart frontend before >>>> the device type introduction in libxl, or is the new device type >>>> compatible with an existing one? >>>> >>>>> Then again, the ring-ref key is listed as deprecated in our >>>>> documentation, without any reference describing which key should be used >>>>> instead. It is also typically a grant reference, not a gfn, so >>>>> something wonky is definitely going on here. >>>> >>>> Which put under question how an existing frontend could work >>>> with this new device type. >>> >>> Well, vuart is replicating the behavior of console (see >>> libxl__device_console_add). The console is passing a frame number in >>> decimal in "ring-ref". Confusingly it is an MFN and would even break on >>> 32-bit toolstack using 64-bit Xen... >>> >>> So this patch is just following the console behavior by passing a >>> decimal value rather than an hexadecimal value. >> >> Well, that other code path should then also use PRIu_xen_pfn, at >> the very least. > > By other code path, you mean the console code right? In that case, mfn > should also be moved from unsigned long to xen_pfn_t. Yes. >> It's of course interesting that the apparent consumer >> of this (tools/console/daemon/io.c:domain_create_ring()) uses >> >> err = xs_gather(xs, dom->conspath, >> "ring-ref", "%u", &ring_ref, >> "port", "%i", &remote_port, >> NULL); >> >> in order to then cast(!) the result to unsigned long in the >> invocation of xc_map_foreign_range(). Suggests to me that >> the console can't work reliably on a system with memory >> extending past the 1Tb boundary. > > It likely a latent bug. Probably a silly question, would there any > compatibility issue to switch the format to the correct one? I don't think so. >> It of course escapes me why %i (or really %lli) wasn't used here >> from the beginning, eliminating all radix concerns and matching >> what is being done for the port. > > Why %i? Should not the GFN be unsigned? Signedness is secondary here - the important thing is that %i is the only one allowing all of decimal, hex, and octal formatting of the string (the latter two of course with the usual 0 / 0x prefixes). Port numbers are unsigned too, yet %i is being used there. > Although, I can see the field > ring_reg is int and will store -1 as not mapped. This is quite confusing > and likely we want to turned into xen_pfn_t + use ~(xen_pfn_t)0. Indeed. > But then, xc_map_foreign_range is using unsigned long instead of > xen_pfn_t. So I guess we should also switch the parameter to xen_pfn_t. Yes. > Note that the implementation of xc_map_foreign_range is using xen_pfn_t. And yes again. Jan
Hi, Sorry for the top-posting. Bhupinder, can you give a look? Cheers, On 13/10/17 16:06, Jan Beulich wrote: >>>> On 13.10.17 at 16:35, <julien.grall@linaro.org> wrote: >> Hi Jan, >> >> On 13/10/17 15:03, Jan Beulich wrote: >>>>>> On 13.10.17 at 15:03, <julien.grall@linaro.org> wrote: >>>> On 13/10/17 13:32, Jan Beulich wrote: >>>>>>>> On 13.10.17 at 14:19, <andrew.cooper3@citrix.com> wrote: >>>>>> On 13/10/17 13:08, Jan Beulich wrote: >>>>>>>>>> On 13.10.17 at 12:44, <bhupinder.thakur@linaro.org> 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. >>>>>>> Is this generic or vuart specific code in xenstore that does so? >>>>>>> Could you perhaps simply point me at the consuming side? >>>>>>> >>>>>>>> Introduced a new format string "PRIu_xen_pfn" which formats the value as a >>>>>>>> decimal value. >>>>>>> I ask because I'm not really happy about this addition, i.e. I'd >>>>>>> prefer the read side to change. >>>>>> >>>>>> Can the read side realistically change? >>>>> >>>>> Well, that's why I had asked whether this is generic or specific >>>>> code. I would have hoped/assumed that xenstore doesn't >>>>> generically try to translate strings into numbers - how would it >>>>> know a string is to represent a number in the first place? Hence >>>>> I was hoping for this to be specific (and hence) new code. >>>>> >>>>>> Its been decimal for a decade now, and there definitely is 3rd party >>>>>> code which uses these values in xenstore (sadly). >>>>> >>>>> Are you trying to tell me there's been a vuart frontend before >>>>> the device type introduction in libxl, or is the new device type >>>>> compatible with an existing one? >>>>> >>>>>> Then again, the ring-ref key is listed as deprecated in our >>>>>> documentation, without any reference describing which key should be used >>>>>> instead. It is also typically a grant reference, not a gfn, so >>>>>> something wonky is definitely going on here. >>>>> >>>>> Which put under question how an existing frontend could work >>>>> with this new device type. >>>> >>>> Well, vuart is replicating the behavior of console (see >>>> libxl__device_console_add). The console is passing a frame number in >>>> decimal in "ring-ref". Confusingly it is an MFN and would even break on >>>> 32-bit toolstack using 64-bit Xen... >>>> >>>> So this patch is just following the console behavior by passing a >>>> decimal value rather than an hexadecimal value. >>> >>> Well, that other code path should then also use PRIu_xen_pfn, at >>> the very least. >> >> By other code path, you mean the console code right? In that case, mfn >> should also be moved from unsigned long to xen_pfn_t. > > Yes. > >>> It's of course interesting that the apparent consumer >>> of this (tools/console/daemon/io.c:domain_create_ring()) uses >>> >>> err = xs_gather(xs, dom->conspath, >>> "ring-ref", "%u", &ring_ref, >>> "port", "%i", &remote_port, >>> NULL); >>> >>> in order to then cast(!) the result to unsigned long in the >>> invocation of xc_map_foreign_range(). Suggests to me that >>> the console can't work reliably on a system with memory >>> extending past the 1Tb boundary. >> >> It likely a latent bug. Probably a silly question, would there any >> compatibility issue to switch the format to the correct one? > > I don't think so. > >>> It of course escapes me why %i (or really %lli) wasn't used here >>> from the beginning, eliminating all radix concerns and matching >>> what is being done for the port. >> >> Why %i? Should not the GFN be unsigned? > > Signedness is secondary here - the important thing is that %i is > the only one allowing all of decimal, hex, and octal formatting of > the string (the latter two of course with the usual 0 / 0x prefixes). > Port numbers are unsigned too, yet %i is being used there. > >> Although, I can see the field >> ring_reg is int and will store -1 as not mapped. This is quite confusing >> and likely we want to turned into xen_pfn_t + use ~(xen_pfn_t)0. > > Indeed. > >> But then, xc_map_foreign_range is using unsigned long instead of >> xen_pfn_t. So I guess we should also switch the parameter to xen_pfn_t. > > Yes. > >> Note that the implementation of xc_map_foreign_range is using xen_pfn_t. > > And yes again. > > Jan >
On 13 October 2017 at 20:36, Jan Beulich <JBeulich@suse.com> wrote: >>>> On 13.10.17 at 16:35, <julien.grall@linaro.org> wrote: >> Hi Jan, >> >> On 13/10/17 15:03, Jan Beulich wrote: >>>>>> On 13.10.17 at 15:03, <julien.grall@linaro.org> wrote: >>>> On 13/10/17 13:32, Jan Beulich wrote: >>>>>>>> On 13.10.17 at 14:19, <andrew.cooper3@citrix.com> wrote: >>>>>> On 13/10/17 13:08, Jan Beulich wrote: >>>>>>>>>> On 13.10.17 at 12:44, <bhupinder.thakur@linaro.org> 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. >>>>>>> Is this generic or vuart specific code in xenstore that does so? >>>>>>> Could you perhaps simply point me at the consuming side? >>>>>>> >>>>>>>> Introduced a new format string "PRIu_xen_pfn" which formats the value as a >>>>>>>> decimal value. >>>>>>> I ask because I'm not really happy about this addition, i.e. I'd >>>>>>> prefer the read side to change. >>>>>> >>>>>> Can the read side realistically change? >>>>> >>>>> Well, that's why I had asked whether this is generic or specific >>>>> code. I would have hoped/assumed that xenstore doesn't >>>>> generically try to translate strings into numbers - how would it >>>>> know a string is to represent a number in the first place? Hence >>>>> I was hoping for this to be specific (and hence) new code. >>>>> >>>>>> Its been decimal for a decade now, and there definitely is 3rd party >>>>>> code which uses these values in xenstore (sadly). >>>>> >>>>> Are you trying to tell me there's been a vuart frontend before >>>>> the device type introduction in libxl, or is the new device type >>>>> compatible with an existing one? >>>>> >>>>>> Then again, the ring-ref key is listed as deprecated in our >>>>>> documentation, without any reference describing which key should be used >>>>>> instead. It is also typically a grant reference, not a gfn, so >>>>>> something wonky is definitely going on here. >>>>> >>>>> Which put under question how an existing frontend could work >>>>> with this new device type. >>>> >>>> Well, vuart is replicating the behavior of console (see >>>> libxl__device_console_add). The console is passing a frame number in >>>> decimal in "ring-ref". Confusingly it is an MFN and would even break on >>>> 32-bit toolstack using 64-bit Xen... >>>> >>>> So this patch is just following the console behavior by passing a >>>> decimal value rather than an hexadecimal value. >>> >>> Well, that other code path should then also use PRIu_xen_pfn, at >>> the very least. >> >> By other code path, you mean the console code right? In that case, mfn >> should also be moved from unsigned long to xen_pfn_t. > > Yes. > ok. >>> It's of course interesting that the apparent consumer >>> of this (tools/console/daemon/io.c:domain_create_ring()) uses >>> >>> err = xs_gather(xs, dom->conspath, >>> "ring-ref", "%u", &ring_ref, >>> "port", "%i", &remote_port, >>> NULL); >>> >>> in order to then cast(!) the result to unsigned long in the >>> invocation of xc_map_foreign_range(). Suggests to me that >>> the console can't work reliably on a system with memory >>> extending past the 1Tb boundary. >> >> It likely a latent bug. Probably a silly question, would there any >> compatibility issue to switch the format to the correct one? > > I don't think so. > >>> It of course escapes me why %i (or really %lli) wasn't used here >>> from the beginning, eliminating all radix concerns and matching >>> what is being done for the port. >> >> Why %i? Should not the GFN be unsigned? > > Signedness is secondary here - the important thing is that %i is > the only one allowing all of decimal, hex, and octal formatting of > the string (the latter two of course with the usual 0 / 0x prefixes). > Port numbers are unsigned too, yet %i is being used there. > >> Although, I can see the field >> ring_reg is int and will store -1 as not mapped. This is quite confusing >> and likely we want to turned into xen_pfn_t + use ~(xen_pfn_t)0. > > Indeed. > ok. I will modify the ring-ref type to xen_pfn_t. >> But then, xc_map_foreign_range is using unsigned long instead of >> xen_pfn_t. So I guess we should also switch the parameter to xen_pfn_t. > > Yes. > ok. Regards, Bhupinder
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