Re: Arm: AArch32: Need suggestions to support 32 bit physical addresses

[Jan Beulich <jbeulich@xxxxxxxx>]

On 29.11.2022 19:18, Ayan Kumar Halder wrote:
> On 29/11/2022 15:52, Julien Grall wrote:
>> On 29/11/2022 16:23, Ayan Kumar Halder wrote:
>>> On 29/11/2022 14:52, Julien Grall wrote:
>>>> On 29/11/2022 14:57, Ayan Kumar Halder wrote:
>>> --- a/xen/common/page_alloc.c
>>> +++ b/xen/common/page_alloc.c
>>> @@ -2245,7 +2245,9 @@ void __init xenheap_max_mfn(unsigned long mfn)
>>>   {
>>>       ASSERT(!first_node_initialised);
>>>       ASSERT(!xenheap_bits);
>>> +#ifndef CONFIG_AARCH32_V8R
>>>       BUILD_BUG_ON(PADDR_BITS >= BITS_PER_LONG);
>>> +#endif
>>
>> BUILD_BUG_ON() are used to indicate that the code would fall over the 
>> check pass. I can't find the justification for this change in the 
>> commit message.
> 
> I had a look at the following commit which introduced this, but I 
> couldn't get the explaination for this.
> 
> commit 88e3ed61642bb393458acc7a9bd2f96edc337190
> Author: Jan Beulich <jbeulich@xxxxxxxx>
> Date:   Tue Sep 1 14:02:57 2015 +0200
> 
> @Jan :- Do you know why BUILD_BUG_ON() was introduced ?

You've cut too much context - the next line explains this all by itself,
I think:

    xenheap_bits = min(flsl(mfn + 1) - 1 + PAGE_SHIFT, PADDR_BITS);

Clearly addresses used for the Xen heap need to be representable in an
unsigned long (which we assume to be the same size as void *).

But I'm afraid there's further context missing for your question: Why
would that construct be a problem in your case? Is it just that you'd
need it to be > rather than the >= that's used presently? If so, why
do you add an #ifdef rather than dealing with the (apparent) off-by-1?
(I say "apparent" because I haven't checked whether the >= is really
depended upon anywhere.)

Jan