Re: [PATCH v2] xen/arm: Add 0x prefix when printing memory size in construct_domU

[Michal Orzel <michal.orzel@xxxxxxx>]

On 05/01/2023 13:15, Andrew Cooper wrote:
> 
> 
> On 05/01/2023 11:19 am, Julien Grall wrote:
>> On 05/01/2023 09:59, Ayan Kumar Halder wrote:
>>> Hi Julien,
>>
>> Hi,
>>
>>> I have a clarification.
>>>
>>> On 05/01/2023 09:26, Julien Grall wrote:
>>>> CAUTION: This message has originated from an External Source. Please
>>>> use proper judgment and caution when opening attachments, clicking
>>>> links, or responding to this email.
>>>>
>>>>
>>>> Hi Stefano,
>>>>
>>>> On 04/01/2023 23:47, Stefano Stabellini wrote:
>>>>> On Tue, 3 Jan 2023, Michal Orzel wrote:
>>>>>> Printing memory size in hex without 0x prefix can be misleading, so
>>>>>> add it. Also, take the opportunity to adhere to 80 chars line length
>>>>>> limit by moving the printk arguments to the next line.
>>>>>>
>>>>>> Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
>>>>>> ---
>>>>>> Changes in v2:
>>>>>>   - was: "Print memory size in decimal in construct_domU"
>>>>>>   - stick to hex but add a 0x prefix
>>>>>>   - adhere to 80 chars line length limit
>>>>>
>>>>> Honestly I prefer decimal but also hex is fine.
>>>>
>>>> decimal is perfect for very small values, but as we print the amount in
>>>> KB it will become a big mess. Here some examples (decimal first, then
>>>> hexadecimal):
>>>>
>>>>   512MB: 524288 vs 0x80000
>>>>   555MB: 568320 vs 0x8ac00
>>>>   1GB: 1048576 vs 0x100000
>>>>   512GB: 536870912 vs 0x20000000
>>>>   1TB: 1073741824 vs 0x40000000
>>>>
>>>> For power of two values, you might be able to find your way with
>>>> decimal. It is more difficult for non power of two unless you have a
>>>> calculator in hand.
>>>>
>>>> The other option I suggested in v1 is to print the amount in KB/GB/MB.
>>>> Would that be better?
>>>>
>>>> That said, to be honest, I am not entirely sure why we are actually
>>>> printing in KB. It would seems strange that someone would create a
>>>> guest
>>>> with memory not aligned to 1MB.
>>>
>>> For RTOS (Zephyr and FreeRTOS), it should be possible for guests to
>>> have memory less than 1 MB, isn't it ?
>>
>> Yes. So does XTF. But most of the users are likely going allocate at
>> least 1MB (or even 2MB to reduce the TLB pressure).
>>
>> So it would be better to print the value in a way that is more
>> meaningful for the majority of the users.
>>
>>>> So I would consider to check the size is 1MB-aligned and then print the
>>
>> I will retract my suggestion to check the size. There are technically
>> no restriction to run a guest with a size not aligned to 1MB.
>> Although, it would still seem strange.
> 
> I have a need to extend tools/tests/tsx with a VM that is a single 4k
> page.  Something which can execute CPUID in the context of a VM and
> cross-check the results with what the "toolstack" (test) tried to configure.
> 
> Xen is buggy if it cannot operate a VM which looks like that, and a
> bonus of explicitly testing like this is that it helps to remove
> inappropriate checks.
I can see we are all settled that it is fully ok to boot guests with memory 
size less than 1MB.
The 'memory' dt parameter for dom0less domUs requires being specified in KB and 
this is the
smallest common multiple so it makes the process of cross-checking easier. 
Stefano is ok with
either decimal or hex, Julien wanted hex (hence my v2), I'm more towards 
printing in hex as well.
Let's not forget that this patch aims to fix a misleading print that was 
missing 0x and can cause
someone to take it as a decimal value.

> 
> ~Andrew

~Michal