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

[Andre Przywara <andre.przywara@xxxxxxx>]

On Wed, 30 Nov 2022 15:39:56 +0000
Ayan Kumar Halder <ayankuma@xxxxxxx> wrote:

Hi Ayan,

> On 30/11/2022 13:13, Andre Przywara wrote:
> > On Wed, 30 Nov 2022 08:09:53 +0100
> > Jan Beulich <jbeulich@xxxxxxxx> wrote:
> >
> > Hi Ayan,  
> Hi Andre,
> >  
> >> 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 *).  
> > So I am wondering why you hit that code for your port in the first case?
> > If you check, then ARM32 won't pass this, because PADDR_BITS is 40, while
> > a long is still 32 bits.  
> 
> But, PADDR_BITS should be equal to 32 for R52.
> 
> Refer Cortex R52 TRM, Section 2.2.12 "Memory Model"
> "...This is because the physical address is always the same as the
>   virtual address...The virtual and physical address can be treated as
>   synonyms for Cortex-R52."
> 
>  From this, I understood that as virtual address is 32 bits for AArch32, 
> so physical address will also be 32 bits.
> 
> Please correct me if I am misunderstanding ?

This is correct, but it's also a big distraction, as no VMSA means that
most of the traditional Xen code becomes irrelevant, since you cannot map
anything anyways. You have to check how the v8-R64 code handles this, I
guess it uses separate code paths?

> If this is correct, then ...
> 
> > So digging deeper this is code for the case when we want to map the entire
> > physical memory into Xen (the Xen heap, or direct map in Linux terms).
> > Which we cannot do for ARM32, and that's why the code is protected by
> > !CONFIG_SEPARATE_XENHEAP, which is forced to 1 for CONFIG_ARM_32 (in a
> > hacked up way, btw).  
> 
> we can map entire physical memory into Xen as VA == PA.

We cannot map anything to begin with, since we have no tables. So without
mapping, indeed the whole physical memory naturally fits into the CPU
address space. But in any case you must not use this code, as this tries
to *map* something, which we do not support on R-class. Again check what
the v8-R64 code has to say on this topic.

Cheers,
Andre

> > So I think you must just force the same thing for your port, then this
> > code will never be compiled.
> >
> > Does that make sense?
> >
> > Cheers,
> > Andre
> >  
> >> 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