Re: [Xen-devel] [PATCH v1] boot allocator: Use arch helper for virt_to_mfn on DIRECTMAP

[Julien Grall <julien.grall@xxxxxxx>]

Hello,

It is not the first time I am saying this. Please CC *all* the 
maintainers of the code you modify. Give a look at 
scripts/get_maintainers.pl.
On 03/10/2017 07:32 AM, vijay.kilari@xxxxxxxxx wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxx>
>
> On ARM, virt_to_mfn uses the hardware for address
> translation. So if the virtual address is not mapped translation
> fault is raised.
> On ARM with NUMA, While initializing second memory node,
> panic is triggered from init_node_heap() when virt_to_mfn()
> is called for DIRECTMAP_VIRT region address.
>
> The init_node_heap() makes a check on MFN passed to ensure that
> MFN less than max MFN. For this, check is made against virt_to_mfn of
> DIRECTMAP_VIRT_END region. Since DIRECMAP_VIRT region is not mapped
> to any physical memory on ARM, it fails.
>
> In this patch, instead of calling virt_to_mfn(), arch helper
> arch_directmap_virt_to_mfn() is introduced. For ARM this arch helper
> will return 0 and for x86 this helper does virt_to_mfn.
I don't understand why you return 0 for ARM. It will prevent the code to 
optimize the case where all the node memory is in the direct mapped 
region. Instead it will allocate extra page in xenheap.
On the previous discussion [1], it has been said that on ARM64 all the 
memory is currently direct mapped. So this check should *always* be true 
and not false. It was suggested to move the whole check in arch specific 
code.
If this suggestion does not fit, please explain why. Similarly you need 
to justify why you return 0 for ARM because so far it looks a random value.
Regards,

[1] https://lists.xen.org/archives/html/xen-devel/2017-01/msg00575.html

--
Julien Grall

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