Duplicated memory node in the Device-Tree (WAS [XEN] Re: Duplicated memory nodes cause the BUG())

Duplicated memory node in the Device-Tree (WAS [XEN] Re: Duplicated memory nodes cause the BUG())

[Julien Grall]

On 25/07/17 13:24, Andrii Anisov wrote:
> Dear All,

Hi,

>
>
> In case of memory nodes duplication (or overlapping) in a device tree,
> we get a BUG at page_alloc.c:226.
>
> This appears e.g. for Renesas R-Car Gen3 SoC based boards. Their u-boot
> does write memory nodes to the device tree before image (Linux kernel,
> xen) boot. So having following memory description in the compiled device
> tree for Salvator-X:
> \ {
>         memory@48000000 {
>                 device_type = "memory";
>                 reg = <0x0 0x48000000 0x0 0x38000000>;
>         };
>         memory@500000000 {
>                 device_type = "memory";
>                 reg = <0x5 0x00000000 0x0 0x40000000>;
>         };
>         memory@600000000 {
>                 device_type = "memory";
>                 reg = <0x6 0x00000000 0x0 0x40000000>;
>         };
>         memory@700000000 {
>                 device_type = "memory";
>                 reg = <0x7 0x00000000 0x0 0x40000000>;
>         };
> }
>
> The hypervisor in runtime receives following:
> \ {
>         memory@48000000 {
>                 device_type = "memory";
>                 reg = <0x0 0x48000000 0x0 0x38000000>,
>                           <0x5 0x00000000 0x0 0x40000000>,
>                           <0x6 0x00000000 0x0 0x40000000>,
>                           <0x7 0x00000000 0x0 0x40000000>;
>         };
>         memory@500000000 {
>                 device_type = "memory";
>                 reg = <0x5 0x00000000 0x0 0x40000000>;
>         };
>         memory@600000000 {
>                 device_type = "memory";
>                 reg = <0x6 0x00000000 0x0 0x40000000>;
>         };
>         memory@700000000 {
>                 device_type = "memory";
>                 reg = <0x7 0x00000000 0x0 0x40000000>;
>         };
> }
>
> And crashes:
>
> (XEN) Checking for initrd in /chosen
> (XEN) RAM: 0000000048000000 - 000000007fffffff
> (XEN) RAM: 0000000500000000 - 000000053fffffff
> (XEN) RAM: 0000000600000000 - 000000063fffffff
> (XEN) RAM: 0000000700000000 - 000000073fffffff
> (XEN) RAM: 0000000500000000 - 000000053fffffff
> (XEN) RAM: 0000000600000000 - 000000063fffffff
> (XEN) RAM: 0000000700000000 - 000000073fffffff
> (XEN)
> (XEN) MODULE[0]: 0000000048000000 - 0000000048011000 Device Tree
> (XEN) MODULE[1]: 000000007a000000 - 000000007c000000 Kernel
> (XEN)  RESVD[0]: 0000000048000000 - 0000000048011000
> (XEN)
> (XEN) Command line: dom0_mem=752M console=dtuart dtuart=serial0
> dom0_max_vcpus=4
> (XEN) Placing Xen at 0x000000073fe00000-0x0000000740000000
> (XEN) Update BOOTMOD_XEN from 0000000078080000-0000000078192d81 =>
> 000000073fe00000-000000073ff12d81
> (XEN) PFN compression on bits 19...19
> (XEN)
> (XEN) ****************************************
> (XEN) Panic on CPU 0:
> (XEN) Boot BUG at page_alloc.c:226
> (XEN) ****************************************
> (XEN)
> (XEN) Reboot in five seconds...
>
> This mainly happens because duplicated memory nodes are accounted as
> separate memory banks (in process_memory_node()).
> I select now of two approaches to fix the issue:
> * ignore next duplicating (overlapping) memory node in favor of one
> already in a memory banks list
> * merge duplicating (overlapping), even neighboring, memory banks
>
> Actually I tend to follow the second approach, but want to get some
> advisory from community first.
> ps. Linux kernel does tolerate duplicated memory nodes by merging memory
> blocks. I.e. memblock_add_range() function is commented as following:

I think this is by chance rather than by design. The first question to 
answer is why a Firmware would specify twice the same memory banks? Is 
it valid from the specification?

I have CCed the devicetree ML to get feedback from them here.

Regardless that, it looks like to me that the device-tree you give to 
the board should not contain the memory nodes.

>     /**
>      * memblock_add_range - add new memblock region
>      * @type: memblock type to add new region into
>      * @base: base address of the new region
>      * @size: size of the new region
>      * @nid: nid of the new region
>      * @flags: flags of the new region
>      *
>      * Add new memblock region [@base,@base+@size) into @type.  The new
> region
>      * is allowed to overlap with existing ones - overlaps don't affect
> already
>      * existing regions.  @type is guaranteed to be minimal (all
> neighbouring
>      * compatible regions are merged) after the addition.
>      *
>      * RETURNS:
>      * 0 on success, -errno on failure.
>      */
>

Cheers,

-- 
Julien Grall
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Duplicated memory node in the Device-Tree (WAS [XEN] Re: Duplicated memory nodes cause the BUG())

[Andrii Anisov]

Hello Julien,


On 25.07.17 17:23, Julien Grall wrote:
> I think this is by chance rather than by design. The first question to 
> answer is why a Firmware would specify twice the same memory banks? Is 
> it valid from the specification?
Yep, that is the question.

> Regardless that, it looks like to me that the device-tree you give to 
> the board should not contain the memory nodes.
The device-tree is provided by vendor in that form, and u-boot is 
theirs. It seems to me that they do not really care since the kernel 
tolerates duplication.

>>  ps. Linux kernel does tolerate duplicated memory nodes by merging 
>> memory blocks. I.e. memblock_add_range() function is commented as 
>> following:
>> /**
>>      * memblock_add_range - add new memblock region
>>      * @type: memblock type to add new region into
>>      * @base: base address of the new region
>>      * @size: size of the new region
>>      * @nid: nid of the new region
>>      * @flags: flags of the new region
>>      *
>>      * Add new memblock region [@base,@base+@size) into @type. The new
>> region
>>      * is allowed to overlap with existing ones - overlaps don't affect
>> already
>>      * existing regions.  @type is guaranteed to be minimal (all
>> neighbouring
>>      * compatible regions are merged) after the addition.
>>      *
>>      * RETURNS:
>>      * 0 on success, -errno on failure.
>>      */
IMO the function description is pretty straight-forward.
But let us wait for device tree guys feedback.

-- 

*Andrii Anisov*


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Duplicated memory node in the Device-Tree (WAS [XEN] Re: Duplicated memory nodes cause the BUG())

[Julien Grall]

On 25/07/17 16:27, Andrii Anisov wrote:
> Hello Julien,

Hi Andrii,

> On 25.07.17 17:23, Julien Grall wrote:
>> I think this is by chance rather than by design. The first question to
>> answer is why a Firmware would specify twice the same memory banks? Is
>> it valid from the specification?
> Yep, that is the question.
>
>> Regardless that, it looks like to me that the device-tree you give to
>> the board should not contain the memory nodes.
> The device-tree is provided by vendor in that form, and u-boot is
> theirs. It seems to me that they do not really care since the kernel
> tolerates duplication.

I have seen work on this board for the past year and it is the first 
time I have seen a complain about memory overlap. So why this sudden 
change? Is that a comment from the vendor or your guess?

>
>>>  ps. Linux kernel does tolerate duplicated memory nodes by merging
>>> memory blocks. I.e. memblock_add_range() function is commented as
>>> following:
>>> /**
>>>      * memblock_add_range - add new memblock region
>>>      * @type: memblock type to add new region into
>>>      * @base: base address of the new region
>>>      * @size: size of the new region
>>>      * @nid: nid of the new region
>>>      * @flags: flags of the new region
>>>      *
>>>      * Add new memblock region [@base,@base+@size) into @type. The new
>>> region
>>>      * is allowed to overlap with existing ones - overlaps don't affect
>>> already
>>>      * existing regions.  @type is guaranteed to be minimal (all
>>> neighbouring
>>>      * compatible regions are merged) after the addition.
>>>      *
>>>      * RETURNS:
>>>      * 0 on success, -errno on failure.
>>>      */
> IMO the function description is pretty straight-forward.

You need to differentiate the device-tree spec itself and Linux 
implementation. memblock is something common to all architecture. It 
does not mean it is something valid to do.

> But let us wait for device tree guys feedback.
>

Cheers,

-- 
Julien Grall
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Duplicated memory node in the Device-Tree (WAS [XEN] Re: Duplicated memory nodes cause the BUG())

[Andrii Anisov]

Hello Julien,


On 25.07.17 18:44, Julien Grall wrote:
> I have seen work on this board for the past year and it is the first 
> time I have seen a complain about memory overlap. So why this sudden 
> change?
It just an approach change. I'm cleaning up nits for the board.

> Is that a comment from the vendor or your guess?
It is my impression.
But yes, it could be worth to ask them why do they hardcode the memory 
layout in their u-boot but stuff the dts with number of memory nodes.
Actually from the beginning of our time for this board the solution was 
merging all memory description nodes in one in a dts for XEN, like [1]. 
So u-boot runtime updates to the dtb were just ignored.

> You need to differentiate the device-tree spec itself and Linux 
> implementation. memblock is something common to all architecture. It 
> does not mean it is something valid to do.
That is why I asked for an advice.

[1] 
https://github.com/xen-troops/meta-demo/blob/master/meta-rcar-gen3-xen/recipes-kernel/linux/linux-renesas/r8a7795-salvator-x-xen.dts#L61
-- 

*Andrii Anisov*


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Duplicated memory node in the Device-Tree (WAS [XEN] Re: Duplicated memory nodes cause the BUG())

[Andrew Cooper]

On 25/07/17 17:02, Andrii Anisov wrote:
> Hello Julien,
>
>
> On 25.07.17 18:44, Julien Grall wrote:
>> I have seen work on this board for the past year and it is the first
>> time I have seen a complain about memory overlap. So why this sudden
>> change?
> It just an approach change. I'm cleaning up nits for the board.
>
>> Is that a comment from the vendor or your guess?
> It is my impression.
> But yes, it could be worth to ask them why do they hardcode the memory
> layout in their u-boot but stuff the dts with number of memory nodes.
> Actually from the beginning of our time for this board the solution
> was merging all memory description nodes in one in a dts for XEN, like
> [1]. So u-boot runtime updates to the dtb were just ignored.
>
>> You need to differentiate the device-tree spec itself and Linux
>> implementation. memblock is something common to all architecture. It
>> does not mean it is something valid to do.
> That is why I asked for an advice.
>
> [1]
> https://github.com/xen-troops/meta-demo/blob/master/meta-rcar-gen3-xen/recipes-kernel/linux/linux-renesas/r8a7795-salvator-x-xen.dts#L61

As a general rule, Xen needs to be able to tolerate and cope with any
quantity of crap described by the firmware.  On the x86 side, we have
large quantities of workarounds for buggy ACPI/MP/SMBIOS tables.

It might be the case that the best Xen can do is give up, but it should
do so with a clear error message identifying what the firmware has done
which is sufficiently crazy to prevent further booting.  Hitting a BUG()
is not a user friendly thing to do, so should be fixed.

~Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Duplicated memory node in the Device-Tree (WAS [XEN] Re: Duplicated memory nodes cause the BUG())

[Andrii Anisov]

Hello Andrew,


On 25.07.17 19:23, Andrew Cooper wrote:
> As a general rule, Xen needs to be able to tolerate and cope with any
> quantity of crap described by the firmware.  On the x86 side, we have
> large quantities of workarounds for buggy ACPI/MP/SMBIOS tables.
That approach somehow covered with early mentioned options:

On 25.07.17 15:24, Andrii Anisov wrote:
> * ignore next duplicating (overlapping) memory node in favor of one 
> already in a memory banks list
> * merge duplicating (overlapping), even neighboring, memory banks 

On 25.07.17 19:23, Andrew Cooper wrote:
> It might be the case that the best Xen can do is give up, but it should
> do so with a clear error message identifying what the firmware has done
> which is sufficiently crazy to prevent further booting.
We have one more option to choose for the case:

* BUG() with clear notification at the moment we are trying to add 
overlapping memory bank

So what to choose?

ps:
> Hitting a BUG()
Actually silently dying without earlyprintk enabled in this case. It 
happens pretty early.

-- 

*Andrii Anisov*


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Xen-devel] Duplicated memory node in the Device-Tree (WAS [XEN] Re: Duplicated memory nodes cause the BUG())

[Stefano Stabellini]

On Tue, 25 Jul 2017, Andrii Anisov wrote:
> Hello Andrew,
> 
> 
> On 25.07.17 19:23, Andrew Cooper wrote:
> > As a general rule, Xen needs to be able to tolerate and cope with any
> > quantity of crap described by the firmware.  On the x86 side, we have
> > large quantities of workarounds for buggy ACPI/MP/SMBIOS tables.
> That approach somehow covered with early mentioned options:
> 
> On 25.07.17 15:24, Andrii Anisov wrote:
> > * ignore next duplicating (overlapping) memory node in favor of one already
> > in a memory banks list
> > * merge duplicating (overlapping), even neighboring, memory banks 
> 
> On 25.07.17 19:23, Andrew Cooper wrote:
> > It might be the case that the best Xen can do is give up, but it should
> > do so with a clear error message identifying what the firmware has done
> > which is sufficiently crazy to prevent further booting.
> We have one more option to choose for the case:
> 
> * BUG() with clear notification at the moment we are trying to add overlapping
> memory bank
> 
> So what to choose?

Certainly we need to print a clear warning.

Then, we can decide whether we prefer to crash (as we do today), or
work-around the broken device-tree. I think it would be more beneficial
to Xen users if we tried to continue anyway, and probably the best way
to do that would be by merging the overlapping memory regions.

I fully understand that this is not required by the spec, but lots of
hardware (x86 and ARM) get released every day with some sort of broken
spec compliance. It's our job to decide on a case by case basis whether
it makes sense for us to support these platforms anyway.

In this case, the cost of supporting the Renesas R-Car Gen3 seems pretty
limited to me. Of course, I would have to see the patch, but if we can
make this work with limited amount of changes, very maintainable, I
would take them. Otherwise, screw it :-)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Xen-devel] Duplicated memory node in the Device-Tree (WAS [XEN] Re: Duplicated memory nodes cause the BUG())

[Julien Grall]

On 25/07/17 18:52, Stefano Stabellini wrote:
> On Tue, 25 Jul 2017, Andrii Anisov wrote:
>> Hello Andrew,
>>
>>
>> On 25.07.17 19:23, Andrew Cooper wrote:
>>> As a general rule, Xen needs to be able to tolerate and cope with any
>>> quantity of crap described by the firmware.  On the x86 side, we have
>>> large quantities of workarounds for buggy ACPI/MP/SMBIOS tables.
>> That approach somehow covered with early mentioned options:
>>
>> On 25.07.17 15:24, Andrii Anisov wrote:
>>> * ignore next duplicating (overlapping) memory node in favor of one already
>>> in a memory banks list
>>> * merge duplicating (overlapping), even neighboring, memory banks
>>
>> On 25.07.17 19:23, Andrew Cooper wrote:
>>> It might be the case that the best Xen can do is give up, but it should
>>> do so with a clear error message identifying what the firmware has done
>>> which is sufficiently crazy to prevent further booting.
>> We have one more option to choose for the case:
>>
>> * BUG() with clear notification at the moment we are trying to add overlapping
>> memory bank
>>
>> So what to choose?
>
> Certainly we need to print a clear warning.

+1 here.

> Then, we can decide whether we prefer to crash (as we do today), or
> work-around the broken device-tree. I think it would be more beneficial
> to Xen users if we tried to continue anyway, and probably the best way
> to do that would be by merging the overlapping memory regions.
> I fully understand that this is not required by the spec, but lots of
> hardware (x86 and ARM) get released every day with some sort of broken
> spec compliance. It's our job to decide on a case by case basis whether
> it makes sense for us to support these platforms anyway.
>
> In this case, the cost of supporting the Renesas R-Car Gen3 seems pretty
> limited to me. Of course, I would have to see the patch, but if we can
> make this work with limited amount of changes, very maintainable, I
> would take them. Otherwise, screw it :-)

I tend to disagree here. This is the first board were the bug occurs and 
the Device-Tree is replaceable.

Furthermore, if you look at the wikipage for Renesas R-Car on Xen (see 
[1]), a specific DT for Xen is provided.

So I can't see any reason to implement that in Xen at the moment.

Cheers,

[1] 
https://wiki.xenproject.org/wiki/Xen_ARM_with_Virtualization_Extensions/Salvator-X

-- 
Julien Grall
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Xen-devel] Duplicated memory node in the Device-Tree (WAS [XEN] Re: Duplicated memory nodes cause the BUG())

[Stefano Stabellini]

On Tue, 25 Jul 2017, Julien Grall wrote:
> On 25/07/17 18:52, Stefano Stabellini wrote:
> > On Tue, 25 Jul 2017, Andrii Anisov wrote:
> > > Hello Andrew,
> > > 
> > > 
> > > On 25.07.17 19:23, Andrew Cooper wrote:
> > > > As a general rule, Xen needs to be able to tolerate and cope with any
> > > > quantity of crap described by the firmware.  On the x86 side, we have
> > > > large quantities of workarounds for buggy ACPI/MP/SMBIOS tables.
> > > That approach somehow covered with early mentioned options:
> > > 
> > > On 25.07.17 15:24, Andrii Anisov wrote:
> > > > * ignore next duplicating (overlapping) memory node in favor of one
> > > > already
> > > > in a memory banks list
> > > > * merge duplicating (overlapping), even neighboring, memory banks
> > > 
> > > On 25.07.17 19:23, Andrew Cooper wrote:
> > > > It might be the case that the best Xen can do is give up, but it should
> > > > do so with a clear error message identifying what the firmware has done
> > > > which is sufficiently crazy to prevent further booting.
> > > We have one more option to choose for the case:
> > > 
> > > * BUG() with clear notification at the moment we are trying to add
> > > overlapping
> > > memory bank
> > > 
> > > So what to choose?
> > 
> > Certainly we need to print a clear warning.
> 
> +1 here.
> 
> > Then, we can decide whether we prefer to crash (as we do today), or
> > work-around the broken device-tree. I think it would be more beneficial
> > to Xen users if we tried to continue anyway, and probably the best way
> > to do that would be by merging the overlapping memory regions.
> > I fully understand that this is not required by the spec, but lots of
> > hardware (x86 and ARM) get released every day with some sort of broken
> > spec compliance. It's our job to decide on a case by case basis whether
> > it makes sense for us to support these platforms anyway.
> > 
> > In this case, the cost of supporting the Renesas R-Car Gen3 seems pretty
> > limited to me. Of course, I would have to see the patch, but if we can
> > make this work with limited amount of changes, very maintainable, I
> > would take them. Otherwise, screw it :-)
> 
> I tend to disagree here. This is the first board were the bug occurs and the
> Device-Tree is replaceable.
> 
> Furthermore, if you look at the wikipage for Renesas R-Car on Xen (see [1]), a
> specific DT for Xen is provided.
> 
> So I can't see any reason to implement that in Xen at the moment.

That's true. It does not make sense to do this until we get rid of the
Xen specific R-Car device tree on the wiki.


> [1]
> https://wiki.xenproject.org/wiki/Xen_ARM_with_Virtualization_Extensions/Salvator-X

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Xen-devel] Duplicated memory node in the Device-Tree (WAS [XEN] Re: Duplicated memory nodes cause the BUG())

[Andrii Anisov]

Hello,


On 25.07.17 21:11, Stefano Stabellini wrote:
> On Tue, 25 Jul 2017, Julien Grall wrote:
>> Furthermore, if you look at the wikipage for Renesas R-Car on Xen (see [1]), a
>> specific DT for Xen is provided.
Well, we introduced a specific DT as a faster way to get the system up 
and running. But the target is to get rid of it.
Except that nasty issue with memory banks, there is another one: how to 
handle reserved memory regions needed for some IP's functionality (e.g. 
ADSP).
As I already said I do cleanup remaining nits one by one.

> That's true. It does not make sense to do this until we get rid of the
> Xen specific R-Car device tree on the wiki.
IMO, we it should appear correspondent functionality in XEN first, and 
latter get rid of specific device tree on the wiki. So that we will have 
the wiki steps adequate to the current master.

-- 

*Andrii Anisov*


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Xen-devel] Duplicated memory node in the Device-Tree (WAS [XEN] Re: Duplicated memory nodes cause the BUG())

[Julien Grall]

On 26/07/17 10:08, Andrii Anisov wrote:
> Hello,

Hi Andrii,

> On 25.07.17 21:11, Stefano Stabellini wrote:
>> On Tue, 25 Jul 2017, Julien Grall wrote:
>>> Furthermore, if you look at the wikipage for Renesas R-Car on Xen
>>> (see [1]), a
>>> specific DT for Xen is provided.
> Well, we introduced a specific DT as a faster way to get the system up
> and running. But the target is to get rid of it.
> Except that nasty issue with memory banks, there is another one: how to
> handle reserved memory regions needed for some IP's functionality (e.g.
> ADSP).
> As I already said I do cleanup remaining nits one by one.
>
>> That's true. It does not make sense to do this until we get rid of the
>> Xen specific R-Car device tree on the wiki.
> IMO, we it should appear correspondent functionality in XEN first, and
> latter get rid of specific device tree on the wiki. So that we will have
> the wiki steps adequate to the current master.

It sounds like to me that a non-modified DT for Xen on R-Car is not for 
tomorrow...

So here, the first and only sensible option is to work with the vendor 
to modify the device-tree.

If the vendor is not willing to do it, then we can discuss about 
implementing a fix in Xen.

Cheers,

-- 
Julien Grall
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Xen-devel] Duplicated memory node in the Device-Tree (WAS [XEN] Re: Duplicated memory nodes cause the BUG())

[Andrii Anisov]

Dear Julien.


On 26.07.17 12:22, Julien Grall wrote:
> It sounds like to me that a non-modified DT for Xen on R-Car is not 
> for tomorrow...
Yep, unfortunately it is not for tomorrow.

> So here, the first and only sensible option is to work with the vendor 
> to modify the device-tree.
OK, I'll move in that direction instead of pushing changes into hypervisor.

> If the vendor is not willing to do it, then we can discuss about 
> implementing a fix in Xen.
I had it in my mind as a generic stuff. Partially inspired by LK approach.

-- 

*Andrii Anisov*


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Duplicated memory node in the Device-Tree (WAS [XEN] Re: Duplicated memory nodes cause the BUG())

[Mark Rutland]

On Tue, Jul 25, 2017 at 06:27:31PM +0300, Andrii Anisov wrote:
> On 25.07.17 17:23, Julien Grall wrote:
> >I think this is by chance rather than by design. The first
> >question to answer is why a Firmware would specify twice the same
> >memory banks? Is it valid from the specification?
> Yep, that is the question.

I beleive that all memory regions described in any memory node's reg
entries should be disjoint (i.e. should not overlap). Per IEEE-1275, reg
entries (within a node) are supposed to be disjoint, and I would expect
this requirement to extend across nodes in the case of memory nodes.

Currently, the DT spec is somewhat sparse in wording, but this can be
corrected.

> >Regardless that, it looks like to me that the device-tree you give
> >to the board should not contain the memory nodes.
> The device-tree is provided by vendor in that form, and u-boot is
> theirs. It seems to me that they do not really care since the kernel
> tolerates duplication.
> 
> >> ps. Linux kernel does tolerate duplicated memory nodes by
> >>merging memory blocks. I.e. memblock_add_range() function is
> >>commented as following:
> >>/**
> >>     * memblock_add_range - add new memblock region
> >>     * @type: memblock type to add new region into
> >>     * @base: base address of the new region
> >>     * @size: size of the new region
> >>     * @nid: nid of the new region
> >>     * @flags: flags of the new region
> >>     *
> >>     * Add new memblock region [@base,@base+@size) into @type. The new
> >>region
> >>     * is allowed to overlap with existing ones - overlaps don't affect
> >>already
> >>     * existing regions.  @type is guaranteed to be minimal (all
> >>neighbouring
> >>     * compatible regions are merged) after the addition.
> >>     *
> >>     * RETURNS:
> >>     * 0 on success, -errno on failure.
> >>     */
> IMO the function description is pretty straight-forward.
> But let us wait for device tree guys feedback.

This might be the designed of *memblock*, but that does not mean that it
is a deliberate part of the DT handling. I beleive this is simply an
implementation detail that happens to mask a DT bug.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html