Re: [Xen-devel] arm: Boot allocator fails with multi node memory

[Julien Grall <julien.grall@xxxxxxx>]

Hi Jan,

On 17/01/17 09:09, Jan Beulich wrote:
> > > > On 16.01.17 at 19:46, <julien.grall@xxxxxxx> wrote:
> > On 09/01/17 08:40, Jan Beulich wrote:
> > > > > > On 07.01.17 at 07:05, <vijay.kilari@xxxxxxxxx> wrote:
> > > > Question: Why this address is not mapped?. If mapped where this va is
> > > > mapped?.
> > > Well, I think this is the wrong question to ask. Why would it be mapped
> > > if there's no memory there?
> > >
> > > > (XEN) Walking Hypervisor VA 0x847fffffffff on CPU0 via TTBR
> > > > 0x00000000ffcf8000
> > > > (XEN) 0TH[0x108] = 0x0000000000000000
> > > >
> > > >
> > > > static unsigned long init_node_heap(int node, unsigned long mfn,
> > > >                                     unsigned long nr, bool_t *use_tail, int d)
> > > > {
> > > > #ifdef DIRECTMAP_VIRT_END
> > > >     unsigned long eva = min(DIRECTMAP_VIRT_END, HYPERVISOR_VIRT_END);
> > > > #endif
> > > >
> > > >
> > > > ....
> > > >
> > > >     else if ( nr >= needed &&
> > > >               (mfn + needed) <= (virt_to_mfn(eva - 1) + 1) &&  // <===
> > > > FAILS here
> > > The assumption here is that a virtual address inside the direct map
> > > can always be translated, and from your report I'm gaining the
> > > understanding that this is simply not true on ARM (but the code
> > > here pre-dates ARM iirc). If that assumption doesn't hold (and
> > > cannot be made so), apart from adjusting the code here there may
> > > need to be a full audit of common code to see whether there are\
> > > any other such uses.
> > On ARM, the function virt_to_mfn uses the hardware to do the address
> > translation. So if the virtual address is not mapped, it will fail.
> >
> > This is different from the assumption made by the code and x86
> > behavior. I'd still like to keep the current behavior on ARM as it is
> > very handy to directly get debug information when the virtual address is
> > not mapped. Stefano, do you have any opinions?
> >
> > Looking at the code pointed out by Vijay. If I understand correctly,
> > virt_to_mfn(eva) is used to find out the maximum MFN that can be mapped.
> Right.
>
> > On ARM64, all the memory is direct mapped so far, so this check will
> > always be false.
> DYM "will always be true"?
Yes sorry.

> > This might not be true in the future.
> Right. But regardless of that we clearly need another arch_*()
> abstraction here, which (for now) returns constant true on ARM.
>
> Or wait - why does ARM have DIRECTMAP_VIRT_END defined? I
> can't see any use of it outside of page_alloc.c, and there all the
> problematic code would be compiled out if the symbol wasn't
> defined.
DIRECTMAP_VIRT_END is defined because all the virtual region defined are 
bound using 2 defines (*_VIRT_START and *_VIRT_END). Removing 
DIRECTMAP_VIRT_END would be the worst solution because it only defers 
the problem until we decide to not map all the memory in Xen for ARM64.
So I would prefer to see an arch_* helper here.

Regards,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel