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

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 16/01/17 19:59, Stefano Stabellini wrote:
> On Mon, 16 Jan 2017, Julien Grall 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?
> I think it is great to use the hardware to implement virt_to_mfn in the
> general case, for both performance and simplicity.
To be honest, I don't think implementing virt_to_mfn using hardware 
instruction will result to a faster translation. If you look at the 
virt_to_mfn implementation on x86, it is a few instructions because it 
only cares about the direct mapping and xen binary address.
In the case of ARM, virt_to_mfn is able to translate any addresses (such 
as vmap one).
> However, it is
> important not to diverge too much from x86 to avoid this class of
> problems in the future. In other words, the semantics of virt_to_mfn
> must be the same on x86 and ARM. We can either:
>
> 1) change x86 to match ARM
> This could be as simple as adding a check, only for debug builds, in the
> x86 implementation of virt_to_mfn to make sure that the virtual address
> passed is mapped or mappable, throw an error if it is not.
>
> 2) change ARM to match x86
> Add a special case in the arm64 implementation of virt_to_mfn to check
> if the address is in the directmap region and translate it directly,
> without ATS1HR and friends, in that case.
Both directmap and xen virtual address would need to have a specific 
case. I am not a big fan of adding specific case because I think it is 
just a workaround to make the code happy. After all the MFN returned is 
just theoretical and should only be used to do checking. You will also 
lose the ability to catch as soon as possible a problem with page table.
Cheers,

--
Julien Grall

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