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

[Julien Grall <julien@xxxxxxx>]

Hi Michal,

On 06/01/2023 10:41, Michal Orzel wrote:
> 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.
I have committed it with my acked-by.

Cheers,

--
Julien Grall