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

[Vijay Kilari <vijay.kilari@xxxxxxxxx>]

On Fri, Mar 10, 2017 at 3:09 PM, Julien Grall <julien.grall@xxxxxxx> wrote:
> 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.

I got below maintainers when I ran the script on complete patch as below.

ubuntu@ubuntu:~/xen$ ./scripts/get_maintainer.pl
outgoing/0001-boot-allocator-Use-arch-helper-for-virt_to_mfn-on-DI.patch
Stefano Stabellini <sstabellini@xxxxxxxxxx>
Julien Grall <julien.grall@xxxxxxx>
Jan Beulich <jbeulich@xxxxxxxx>
Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
xen-devel@xxxxxxxxxxxxx
ubuntu@ubuntu:~/xen$

But I think you are seeing different/full maintainer list with
./scripts/get_maintainer.pl -f xen/common/page_alloc.c

>
> 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.

Thanks for pointing out.
I was biased by your statement "On ARM64, all the memory is direct
mapped so far, so this check will
always be false.", Sorry, I missed your later reply.

>
> 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