| Message ID | 20230102144904.17619-1-michal.orzel@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/arm: Print memory size in decimal in construct_domU | expand |
Hi Michal, On 02/01/2023 14:49, Michal Orzel wrote: > Printing domain's memory size in hex without even prepending it > with 0x is not very useful and can be misleading. Switch to decimal > notation. > > Signed-off-by: Michal Orzel <michal.orzel@amd.com> > --- > xen/arch/arm/domain_build.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index 829cea8de84f..7e204372368c 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -3774,7 +3774,7 @@ static int __init construct_domU(struct domain *d, > if ( rc != 0 ) > return rc; > > - printk("*** LOADING DOMU cpus=%u memory=%"PRIx64"KB ***\n", d->max_vcpus, mem); > + printk("*** LOADING DOMU cpus=%u memory=%"PRIu64"KB ***\n", d->max_vcpus, mem); I will prefer it to be printed in hex format with 0x prefixed. The reason being the mem is obtained from device-tree domU's 'memory' prop where the values are in hex. It will help the user to debug easily without requiring the the person to manually calculate the hex equivalent and then trying to correlate it with what is written in dts. - Ayan > > kinfo.vpl011 = dt_property_read_bool(node, "vpl011"); >
Hi Ayan, On 03/01/2023 10:21, Ayan Kumar Halder wrote: > > > Hi Michal, > > On 02/01/2023 14:49, Michal Orzel wrote: >> Printing domain's memory size in hex without even prepending it >> with 0x is not very useful and can be misleading. Switch to decimal >> notation. >> >> Signed-off-by: Michal Orzel <michal.orzel@amd.com> >> --- >> xen/arch/arm/domain_build.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c >> index 829cea8de84f..7e204372368c 100644 >> --- a/xen/arch/arm/domain_build.c >> +++ b/xen/arch/arm/domain_build.c >> @@ -3774,7 +3774,7 @@ static int __init construct_domU(struct domain *d, >> if ( rc != 0 ) >> return rc; >> >> - printk("*** LOADING DOMU cpus=%u memory=%"PRIx64"KB ***\n", d->max_vcpus, mem); >> + printk("*** LOADING DOMU cpus=%u memory=%"PRIu64"KB ***\n", d->max_vcpus, mem); > > I will prefer it to be printed in hex format with 0x prefixed. The > reason being the mem is obtained from device-tree domU's 'memory' prop > where the values are in hex. No, I cannot agree. Refer to booting.txt documentation: "A 64-bit integer specifying the amount of kilobytes of RAM to allocate to the guest." Also note that in the provided examples, we are using the decimal values. All in all it does not matter the notation, you can provide e.g. "memory = 131072;" or "memory = 0x20000". I find it a bit odd to print e.g. 0x20000KB and decimal is easier to read. ~Michal
Hi Ayan, On 03/01/2023 09:21, Ayan Kumar Halder wrote: > On 02/01/2023 14:49, Michal Orzel wrote: >> Printing domain's memory size in hex without even prepending it >> with 0x is not very useful and can be misleading. Switch to decimal >> notation. >> >> Signed-off-by: Michal Orzel <michal.orzel@amd.com> >> --- >> xen/arch/arm/domain_build.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c >> index 829cea8de84f..7e204372368c 100644 >> --- a/xen/arch/arm/domain_build.c >> +++ b/xen/arch/arm/domain_build.c >> @@ -3774,7 +3774,7 @@ static int __init construct_domU(struct domain *d, >> if ( rc != 0 ) >> return rc; >> - printk("*** LOADING DOMU cpus=%u memory=%"PRIx64"KB ***\n", >> d->max_vcpus, mem); >> + printk("*** LOADING DOMU cpus=%u memory=%"PRIu64"KB ***\n", >> d->max_vcpus, mem); > > I will prefer it to be printed in hex format with 0x prefixed. The > reason being the mem is obtained from device-tree domU's 'memory' prop > where the values are in hex. > > It will help the user to debug easily without requiring the the person > to manually calculate the hex equivalent and then trying to correlate it > with what is written in dts. I am a bit confused with your reasoning. The value stored in the device-tree is a 64-bit value. This is then up to the consumer to decide whether the output provided is in hexadecimal or decimal. So are you saying that the tool dumping the device-tree will show the values in hexadecimal? If so, the argument is the same for the number of CPUs (you could have more than 15 vCPUs). So I think this argument to be used here. TBH, I am a bit split between using hexadecimal and decimal here. For smaller values, decimal is definitely easier to read but for larger one (i.e. GB), then the hexadecimal would help (it is easier to do the math). So I would leaning towards using hexadecimal for the memory (so adding the 0x). Cheers,
Hi Michal, On 03/01/2023 09:39, Michal Orzel wrote: > On 03/01/2023 10:21, Ayan Kumar Halder wrote: >> >> >> Hi Michal, >> >> On 02/01/2023 14:49, Michal Orzel wrote: >>> Printing domain's memory size in hex without even prepending it >>> with 0x is not very useful and can be misleading. Switch to decimal >>> notation. >>> >>> Signed-off-by: Michal Orzel <michal.orzel@amd.com> >>> --- >>> xen/arch/arm/domain_build.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c >>> index 829cea8de84f..7e204372368c 100644 >>> --- a/xen/arch/arm/domain_build.c >>> +++ b/xen/arch/arm/domain_build.c >>> @@ -3774,7 +3774,7 @@ static int __init construct_domU(struct domain *d, >>> if ( rc != 0 ) >>> return rc; >>> >>> - printk("*** LOADING DOMU cpus=%u memory=%"PRIx64"KB ***\n", d->max_vcpus, mem); >>> + printk("*** LOADING DOMU cpus=%u memory=%"PRIu64"KB ***\n", d->max_vcpus, mem); >> >> I will prefer it to be printed in hex format with 0x prefixed. The >> reason being the mem is obtained from device-tree domU's 'memory' prop >> where the values are in hex. > No, I cannot agree. Refer to booting.txt documentation: > "A 64-bit integer specifying the amount of kilobytes of RAM to allocate to the guest." > Also note that in the provided examples, we are using the decimal values. > All in all it does not matter the notation, you can provide e.g. "memory = 131072;" or "memory = 0x20000". > I find it a bit odd to print e.g. 0x20000KB and decimal is easier to read. By easier, do you mean you can easily figure out how much memory in GB/MB/KB you gave to the guest? If so, then I have to disagree. Without a calculator, I will find quicker the split. If you want to print in decimal, then I think we should split the amount in GB/MB/KB. Otherwise, we should stick in hexadecimal (so add 0x). Cheers,
Hi Julien, On 03/01/2023 10:56, Julien Grall wrote: > > > Hi Michal, > > On 03/01/2023 09:39, Michal Orzel wrote: >> On 03/01/2023 10:21, Ayan Kumar Halder wrote: >>> >>> >>> Hi Michal, >>> >>> On 02/01/2023 14:49, Michal Orzel wrote: >>>> Printing domain's memory size in hex without even prepending it >>>> with 0x is not very useful and can be misleading. Switch to decimal >>>> notation. >>>> >>>> Signed-off-by: Michal Orzel <michal.orzel@amd.com> >>>> --- >>>> xen/arch/arm/domain_build.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c >>>> index 829cea8de84f..7e204372368c 100644 >>>> --- a/xen/arch/arm/domain_build.c >>>> +++ b/xen/arch/arm/domain_build.c >>>> @@ -3774,7 +3774,7 @@ static int __init construct_domU(struct domain *d, >>>> if ( rc != 0 ) >>>> return rc; >>>> >>>> - printk("*** LOADING DOMU cpus=%u memory=%"PRIx64"KB ***\n", d->max_vcpus, mem); >>>> + printk("*** LOADING DOMU cpus=%u memory=%"PRIu64"KB ***\n", d->max_vcpus, mem); >>> >>> I will prefer it to be printed in hex format with 0x prefixed. The >>> reason being the mem is obtained from device-tree domU's 'memory' prop >>> where the values are in hex. >> No, I cannot agree. Refer to booting.txt documentation: >> "A 64-bit integer specifying the amount of kilobytes of RAM to allocate to the guest." >> Also note that in the provided examples, we are using the decimal values. >> All in all it does not matter the notation, you can provide e.g. "memory = 131072;" or "memory = 0x20000". >> I find it a bit odd to print e.g. 0x20000KB and decimal is easier to read. > By easier, do you mean you can easily figure out how much memory in > GB/MB/KB you gave to the guest? If so, then I have to disagree. Without > a calculator, I will find quicker the split. I guess it depends on the size but you have a valid point. > > If you want to print in decimal, then I think we should split the amount > in GB/MB/KB. Otherwise, we should stick in hexadecimal (so add 0x). Ok, I will then just add a prefix. > > Cheers, > > -- > Julien Grall ~Michal
On Mon, 2 Jan 2023, Michal Orzel wrote: > Printing domain's memory size in hex without even prepending it > with 0x is not very useful and can be misleading. Switch to decimal > notation. > > Signed-off-by: Michal Orzel <michal.orzel@amd.com> Acked-by: Stefano Stabellini <sstabellini@kernel.org> > --- > xen/arch/arm/domain_build.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index 829cea8de84f..7e204372368c 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -3774,7 +3774,7 @@ static int __init construct_domU(struct domain *d, > if ( rc != 0 ) > return rc; > > - printk("*** LOADING DOMU cpus=%u memory=%"PRIx64"KB ***\n", d->max_vcpus, mem); > + printk("*** LOADING DOMU cpus=%u memory=%"PRIu64"KB ***\n", d->max_vcpus, mem); > > kinfo.vpl011 = dt_property_read_bool(node, "vpl011"); > > -- > 2.25.1 >
Hi Stefano, On 04/01/2023 23:41, Stefano Stabellini wrote: > On Mon, 2 Jan 2023, Michal Orzel wrote: >> Printing domain's memory size in hex without even prepending it >> with 0x is not very useful and can be misleading. Switch to decimal >> notation. >> >> Signed-off-by: Michal Orzel <michal.orzel@amd.com> > > Acked-by: Stefano Stabellini <sstabellini@kernel.org> Was this intended for v2 rather than v1? Cheers, > > >> --- >> xen/arch/arm/domain_build.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c >> index 829cea8de84f..7e204372368c 100644 >> --- a/xen/arch/arm/domain_build.c >> +++ b/xen/arch/arm/domain_build.c >> @@ -3774,7 +3774,7 @@ static int __init construct_domU(struct domain *d, >> if ( rc != 0 ) >> return rc; >> >> - printk("*** LOADING DOMU cpus=%u memory=%"PRIx64"KB ***\n", d->max_vcpus, mem); >> + printk("*** LOADING DOMU cpus=%u memory=%"PRIu64"KB ***\n", d->max_vcpus, mem); >> >> kinfo.vpl011 = dt_property_read_bool(node, "vpl011"); >> >> -- >> 2.25.1 >>
On Wed, 4 Jan 2023, Julien Grall wrote: > Hi Stefano, > > On 04/01/2023 23:41, Stefano Stabellini wrote: > > On Mon, 2 Jan 2023, Michal Orzel wrote: > > > Printing domain's memory size in hex without even prepending it > > > with 0x is not very useful and can be misleading. Switch to decimal > > > notation. > > > > > > Signed-off-by: Michal Orzel <michal.orzel@amd.com> > > > > Acked-by: Stefano Stabellini <sstabellini@kernel.org> > > Was this intended for v2 rather than v1? I didn't notice v2 was already out. I did test v2 and made sure it is working so go ahead and pick whichever version you prefer > > > --- > > > xen/arch/arm/domain_build.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > > > index 829cea8de84f..7e204372368c 100644 > > > --- a/xen/arch/arm/domain_build.c > > > +++ b/xen/arch/arm/domain_build.c > > > @@ -3774,7 +3774,7 @@ static int __init construct_domU(struct domain *d, > > > if ( rc != 0 ) > > > return rc; > > > - printk("*** LOADING DOMU cpus=%u memory=%"PRIx64"KB ***\n", > > > d->max_vcpus, mem); > > > + printk("*** LOADING DOMU cpus=%u memory=%"PRIu64"KB ***\n", > > > d->max_vcpus, mem); > > > kinfo.vpl011 = dt_property_read_bool(node, "vpl011"); > > > > > > -- > > > 2.25.1 > > > > > -- > Julien Grall >
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index 829cea8de84f..7e204372368c 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -3774,7 +3774,7 @@ static int __init construct_domU(struct domain *d, if ( rc != 0 ) return rc; - printk("*** LOADING DOMU cpus=%u memory=%"PRIx64"KB ***\n", d->max_vcpus, mem); + printk("*** LOADING DOMU cpus=%u memory=%"PRIu64"KB ***\n", d->max_vcpus, mem); kinfo.vpl011 = dt_property_read_bool(node, "vpl011");
Printing domain's memory size in hex without even prepending it with 0x is not very useful and can be misleading. Switch to decimal notation. Signed-off-by: Michal Orzel <michal.orzel@amd.com> --- xen/arch/arm/domain_build.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)