All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] libfdt: Add option to disable arch_fixup_fdt() calls
@ 2016-06-09 14:23 Michal Simek
  2016-06-09 14:29 ` Alexander Graf
  2016-06-10 16:44 ` Simon Glass
  0 siblings, 2 replies; 12+ messages in thread
From: Michal Simek @ 2016-06-09 14:23 UTC (permalink / raw)
  To: u-boot

Disable arch_fixup_fdt() calls for cases where U-Boot shouldn't update
memory setup in DTB file.
One example of usage of this option is to boot OS with different memory
setup than U-Boot use.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 Kconfig                  | 8 ++++++++
 arch/arm/lib/bootm-fdt.c | 2 ++
 common/image-fdt.c       | 3 +++
 3 files changed, 13 insertions(+)

diff --git a/Kconfig b/Kconfig
index 4b4621666560..3efba624ecff 100644
--- a/Kconfig
+++ b/Kconfig
@@ -291,6 +291,14 @@ config SYS_CLK_FREQ
 	help
 	  TODO: Move CONFIG_SYS_CLK_FREQ for all the architecture
 
+config DISABLE_ARCH_FIXUP_FDT
+	bool "Disable arch_fixup_fdt() call"
+	depends on ARM
+	help
+	  Disable FDT memory map syncup before OS boot. This feature can be
+	  used for booting OS with different memory setup where the part of
+	  the memory location should be used for different purpose.
+
 endmenu		# Boot images
 
 source "common/Kconfig"
diff --git a/arch/arm/lib/bootm-fdt.c b/arch/arm/lib/bootm-fdt.c
index 7677358742e1..a873b778ade9 100644
--- a/arch/arm/lib/bootm-fdt.c
+++ b/arch/arm/lib/bootm-fdt.c
@@ -24,6 +24,7 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+#ifndef CONFIG_DISABLE_ARCH_FIXUP_FDT
 int arch_fixup_fdt(void *blob)
 {
 	bd_t *bd = gd->bd;
@@ -50,3 +51,4 @@ int arch_fixup_fdt(void *blob)
 #endif
 	return ret;
 }
+#endif
diff --git a/common/image-fdt.c b/common/image-fdt.c
index 6cac7dbb7f8b..9e05becd464a 100644
--- a/common/image-fdt.c
+++ b/common/image-fdt.c
@@ -460,6 +460,9 @@ __weak int ft_verify_fdt(void *fdt)
 
 __weak int arch_fixup_fdt(void *blob)
 {
+#ifdef CONFIG_DISABLE_ARCH_FIXUP_FDT
+	printf("## Disable arch_fixup_fdt()\n");
+#endif
 	return 0;
 }
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] libfdt: Add option to disable arch_fixup_fdt() calls
  2016-06-09 14:23 [U-Boot] [PATCH] libfdt: Add option to disable arch_fixup_fdt() calls Michal Simek
@ 2016-06-09 14:29 ` Alexander Graf
  2016-06-09 14:32   ` Michal Simek
  2016-06-10 16:44 ` Simon Glass
  1 sibling, 1 reply; 12+ messages in thread
From: Alexander Graf @ 2016-06-09 14:29 UTC (permalink / raw)
  To: u-boot

On 06/09/2016 04:23 PM, Michal Simek wrote:
> Disable arch_fixup_fdt() calls for cases where U-Boot shouldn't update
> memory setup in DTB file.
> One example of usage of this option is to boot OS with different memory
> setup than U-Boot use.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>

Could we instead just have the board file provide a fixup? It could then 
also fix up the efi memory map.


Alex

> ---
>
>   Kconfig                  | 8 ++++++++
>   arch/arm/lib/bootm-fdt.c | 2 ++
>   common/image-fdt.c       | 3 +++
>   3 files changed, 13 insertions(+)
>
> diff --git a/Kconfig b/Kconfig
> index 4b4621666560..3efba624ecff 100644
> --- a/Kconfig
> +++ b/Kconfig
> @@ -291,6 +291,14 @@ config SYS_CLK_FREQ
>   	help
>   	  TODO: Move CONFIG_SYS_CLK_FREQ for all the architecture
>   
> +config DISABLE_ARCH_FIXUP_FDT
> +	bool "Disable arch_fixup_fdt() call"
> +	depends on ARM
> +	help
> +	  Disable FDT memory map syncup before OS boot. This feature can be
> +	  used for booting OS with different memory setup where the part of
> +	  the memory location should be used for different purpose.
> +
>   endmenu		# Boot images
>   
>   source "common/Kconfig"
> diff --git a/arch/arm/lib/bootm-fdt.c b/arch/arm/lib/bootm-fdt.c
> index 7677358742e1..a873b778ade9 100644
> --- a/arch/arm/lib/bootm-fdt.c
> +++ b/arch/arm/lib/bootm-fdt.c
> @@ -24,6 +24,7 @@
>   
>   DECLARE_GLOBAL_DATA_PTR;
>   
> +#ifndef CONFIG_DISABLE_ARCH_FIXUP_FDT
>   int arch_fixup_fdt(void *blob)
>   {
>   	bd_t *bd = gd->bd;
> @@ -50,3 +51,4 @@ int arch_fixup_fdt(void *blob)
>   #endif
>   	return ret;
>   }
> +#endif
> diff --git a/common/image-fdt.c b/common/image-fdt.c
> index 6cac7dbb7f8b..9e05becd464a 100644
> --- a/common/image-fdt.c
> +++ b/common/image-fdt.c
> @@ -460,6 +460,9 @@ __weak int ft_verify_fdt(void *fdt)
>   
>   __weak int arch_fixup_fdt(void *blob)
>   {
> +#ifdef CONFIG_DISABLE_ARCH_FIXUP_FDT
> +	printf("## Disable arch_fixup_fdt()\n");
> +#endif
>   	return 0;
>   }
>   

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] libfdt: Add option to disable arch_fixup_fdt() calls
  2016-06-09 14:29 ` Alexander Graf
@ 2016-06-09 14:32   ` Michal Simek
  2016-06-09 14:40     ` Alexander Graf
  0 siblings, 1 reply; 12+ messages in thread
From: Michal Simek @ 2016-06-09 14:32 UTC (permalink / raw)
  To: u-boot

On 9.6.2016 16:29, Alexander Graf wrote:
> On 06/09/2016 04:23 PM, Michal Simek wrote:
>> Disable arch_fixup_fdt() calls for cases where U-Boot shouldn't update
>> memory setup in DTB file.
>> One example of usage of this option is to boot OS with different memory
>> setup than U-Boot use.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> 
> Could we instead just have the board file provide a fixup? It could then
> also fix up the efi memory map.

Not sure what exactly you are asking for.
Do you mean to add fixup function to board file and overwrite default one?
For me, I want to have this option selectable because it is not valid
for all case even I have to admit that in xilinx tree I have removed
this function in past because it was problematic for AMP cases on zynq.

Thanks,
Michal

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] libfdt: Add option to disable arch_fixup_fdt() calls
  2016-06-09 14:32   ` Michal Simek
@ 2016-06-09 14:40     ` Alexander Graf
  2016-06-10 11:07       ` Michal Simek
  0 siblings, 1 reply; 12+ messages in thread
From: Alexander Graf @ 2016-06-09 14:40 UTC (permalink / raw)
  To: u-boot

On 06/09/2016 04:32 PM, Michal Simek wrote:
> On 9.6.2016 16:29, Alexander Graf wrote:
>> On 06/09/2016 04:23 PM, Michal Simek wrote:
>>> Disable arch_fixup_fdt() calls for cases where U-Boot shouldn't update
>>> memory setup in DTB file.
>>> One example of usage of this option is to boot OS with different memory
>>> setup than U-Boot use.
>>>
>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> Could we instead just have the board file provide a fixup? It could then
>> also fix up the efi memory map.
> Not sure what exactly you are asking for.
> Do you mean to add fixup function to board file and overwrite default one?

You have to touch some other code anyway to make this work with a 
particular board, right? In that case, you can as well add a function to 
the board file that explicitly provides a different, known good memory map.

Can you have a weaker overload? Basically I would like to have a board 
provide arch_fixup_fdt() which would override the one in bootm-fdt.c. 
But we can also rename arch_fixup_fdt() in bootm-fdt.c to 
board_fixup_fdt() function, declare it weak and have arch_fixup_fdt() 
call that.


Alex

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] libfdt: Add option to disable arch_fixup_fdt() calls
  2016-06-09 14:40     ` Alexander Graf
@ 2016-06-10 11:07       ` Michal Simek
  2016-06-10 11:13         ` Alexander Graf
  0 siblings, 1 reply; 12+ messages in thread
From: Michal Simek @ 2016-06-10 11:07 UTC (permalink / raw)
  To: u-boot

On 9.6.2016 16:40, Alexander Graf wrote:
> On 06/09/2016 04:32 PM, Michal Simek wrote:
>> On 9.6.2016 16:29, Alexander Graf wrote:
>>> On 06/09/2016 04:23 PM, Michal Simek wrote:
>>>> Disable arch_fixup_fdt() calls for cases where U-Boot shouldn't update
>>>> memory setup in DTB file.
>>>> One example of usage of this option is to boot OS with different memory
>>>> setup than U-Boot use.
>>>>
>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>> Could we instead just have the board file provide a fixup? It could then
>>> also fix up the efi memory map.
>> Not sure what exactly you are asking for.
>> Do you mean to add fixup function to board file and overwrite default
>> one?
> 
> You have to touch some other code anyway to make this work with a
> particular board, right? In that case, you can as well add a function to
> the board file that explicitly provides a different, known good memory map.

I cant' see the reason to touch particular board. In past AMP solution
where one core use the part of memory and second another part was the
reason I needed this.
For ARM64 case as you know from arm IRC I was trying to boot Linux from
memory above 32bit space and for these tests I need to convince u-boot
not to touch dtb memory setup.

But for the same board I want to use standard behavior but for some case
this needs to be enabled.

> 
> Can you have a weaker overload? Basically I would like to have a board
> provide arch_fixup_fdt() which would override the one in bootm-fdt.c.
> But we can also rename arch_fixup_fdt() in bootm-fdt.c to
> board_fixup_fdt() function, declare it weak and have arch_fixup_fdt()
> call that.

arch_fixup_fdt is already weaker function in image-fdt.c and ARM and
MIPS define it.

I don't think that renaming solve anything. This patch is really just
ON/OFF switch of fixup behavior.

Thanks,
Michal

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] libfdt: Add option to disable arch_fixup_fdt() calls
  2016-06-10 11:07       ` Michal Simek
@ 2016-06-10 11:13         ` Alexander Graf
  2016-06-10 11:51           ` Michal Simek
  0 siblings, 1 reply; 12+ messages in thread
From: Alexander Graf @ 2016-06-10 11:13 UTC (permalink / raw)
  To: u-boot



On 10.06.16 13:07, Michal Simek wrote:
> On 9.6.2016 16:40, Alexander Graf wrote:
>> On 06/09/2016 04:32 PM, Michal Simek wrote:
>>> On 9.6.2016 16:29, Alexander Graf wrote:
>>>> On 06/09/2016 04:23 PM, Michal Simek wrote:
>>>>> Disable arch_fixup_fdt() calls for cases where U-Boot shouldn't update
>>>>> memory setup in DTB file.
>>>>> One example of usage of this option is to boot OS with different memory
>>>>> setup than U-Boot use.
>>>>>
>>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>> Could we instead just have the board file provide a fixup? It could then
>>>> also fix up the efi memory map.
>>> Not sure what exactly you are asking for.
>>> Do you mean to add fixup function to board file and overwrite default
>>> one?
>>
>> You have to touch some other code anyway to make this work with a
>> particular board, right? In that case, you can as well add a function to
>> the board file that explicitly provides a different, known good memory map.
> 
> I cant' see the reason to touch particular board. In past AMP solution
> where one core use the part of memory and second another part was the
> reason I needed this.
> For ARM64 case as you know from arm IRC I was trying to boot Linux from
> memory above 32bit space and for these tests I need to convince u-boot
> not to touch dtb memory setup.
> 
> But for the same board I want to use standard behavior but for some case
> this needs to be enabled.

For that particular case, can you try to see whether you can convince
U-Boot to just run in high memory instead? That would make things much
more consistent IMHO.

Giving U-Boot and Linux different views of the memory map is really just
asking for trouble. You'd have to make sure that you flush your caches
for example so that Linux doesn't suddenly get memory overwritten from
stale cache entries on the low memory even though Linux is already
accessing the high addresses.

For testing, sure, but to actually make use of it I'd rather see a clean
solution.


Alex

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] libfdt: Add option to disable arch_fixup_fdt() calls
  2016-06-10 11:13         ` Alexander Graf
@ 2016-06-10 11:51           ` Michal Simek
  2016-06-10 12:12             ` Alexander Graf
  0 siblings, 1 reply; 12+ messages in thread
From: Michal Simek @ 2016-06-10 11:51 UTC (permalink / raw)
  To: u-boot

On 10.6.2016 13:13, Alexander Graf wrote:
> 
> 
> On 10.06.16 13:07, Michal Simek wrote:
>> On 9.6.2016 16:40, Alexander Graf wrote:
>>> On 06/09/2016 04:32 PM, Michal Simek wrote:
>>>> On 9.6.2016 16:29, Alexander Graf wrote:
>>>>> On 06/09/2016 04:23 PM, Michal Simek wrote:
>>>>>> Disable arch_fixup_fdt() calls for cases where U-Boot shouldn't update
>>>>>> memory setup in DTB file.
>>>>>> One example of usage of this option is to boot OS with different memory
>>>>>> setup than U-Boot use.
>>>>>>
>>>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>>> Could we instead just have the board file provide a fixup? It could then
>>>>> also fix up the efi memory map.
>>>> Not sure what exactly you are asking for.
>>>> Do you mean to add fixup function to board file and overwrite default
>>>> one?
>>>
>>> You have to touch some other code anyway to make this work with a
>>> particular board, right? In that case, you can as well add a function to
>>> the board file that explicitly provides a different, known good memory map.
>>
>> I cant' see the reason to touch particular board. In past AMP solution
>> where one core use the part of memory and second another part was the
>> reason I needed this.
>> For ARM64 case as you know from arm IRC I was trying to boot Linux from
>> memory above 32bit space and for these tests I need to convince u-boot
>> not to touch dtb memory setup.
>>
>> But for the same board I want to use standard behavior but for some case
>> this needs to be enabled.
> 
> For that particular case, can you try to see whether you can convince
> U-Boot to just run in high memory instead? That would make things much
> more consistent IMHO.
> 
> Giving U-Boot and Linux different views of the memory map is really just
> asking for trouble. You'd have to make sure that you flush your caches
> for example so that Linux doesn't suddenly get memory overwritten from
> stale cache entries on the low memory even though Linux is already
> accessing the high addresses.

For systems where you have one main memory and standard usage model with
OS I agree that having this option enabled is bring more problems.
But for our use cases you can add memory controller to PL and it will be
ready when bitstream is loaded which can be done after u-boot is loaded.
It means u-boot have to work with different memory setup then Linux
later on.

Caches should be flushed before u-boot pass control to Linux that's why
there shouldn't be any stale entries.

For ZynqMP you can partitioned memory that part of it goes to A53s and
part to R5s and u-boot is capable to boot both cores.

> For testing, sure, but to actually make use of it I'd rather see a clean
> solution.

Do we have any experimental Kconfig entry which I can use for it?

Thanks,
Michal

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] libfdt: Add option to disable arch_fixup_fdt() calls
  2016-06-10 11:51           ` Michal Simek
@ 2016-06-10 12:12             ` Alexander Graf
  2016-06-10 12:31               ` Michal Simek
  0 siblings, 1 reply; 12+ messages in thread
From: Alexander Graf @ 2016-06-10 12:12 UTC (permalink / raw)
  To: u-boot



On 10.06.16 13:51, Michal Simek wrote:
> On 10.6.2016 13:13, Alexander Graf wrote:
>>
>>
>> On 10.06.16 13:07, Michal Simek wrote:
>>> On 9.6.2016 16:40, Alexander Graf wrote:
>>>> On 06/09/2016 04:32 PM, Michal Simek wrote:
>>>>> On 9.6.2016 16:29, Alexander Graf wrote:
>>>>>> On 06/09/2016 04:23 PM, Michal Simek wrote:
>>>>>>> Disable arch_fixup_fdt() calls for cases where U-Boot shouldn't update
>>>>>>> memory setup in DTB file.
>>>>>>> One example of usage of this option is to boot OS with different memory
>>>>>>> setup than U-Boot use.
>>>>>>>
>>>>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>>>> Could we instead just have the board file provide a fixup? It could then
>>>>>> also fix up the efi memory map.
>>>>> Not sure what exactly you are asking for.
>>>>> Do you mean to add fixup function to board file and overwrite default
>>>>> one?
>>>>
>>>> You have to touch some other code anyway to make this work with a
>>>> particular board, right? In that case, you can as well add a function to
>>>> the board file that explicitly provides a different, known good memory map.
>>>
>>> I cant' see the reason to touch particular board. In past AMP solution
>>> where one core use the part of memory and second another part was the
>>> reason I needed this.
>>> For ARM64 case as you know from arm IRC I was trying to boot Linux from
>>> memory above 32bit space and for these tests I need to convince u-boot
>>> not to touch dtb memory setup.
>>>
>>> But for the same board I want to use standard behavior but for some case
>>> this needs to be enabled.
>>
>> For that particular case, can you try to see whether you can convince
>> U-Boot to just run in high memory instead? That would make things much
>> more consistent IMHO.
>>
>> Giving U-Boot and Linux different views of the memory map is really just
>> asking for trouble. You'd have to make sure that you flush your caches
>> for example so that Linux doesn't suddenly get memory overwritten from
>> stale cache entries on the low memory even though Linux is already
>> accessing the high addresses.
> 
> For systems where you have one main memory and standard usage model with
> OS I agree that having this option enabled is bring more problems.
> But for our use cases you can add memory controller to PL and it will be
> ready when bitstream is loaded which can be done after u-boot is loaded.
> It means u-boot have to work with different memory setup then Linux
> later on.

Oh, I see. So U-Boot actually uses completely different memory? But that
means that the original memory is also still there, doesn't it? Wouldn't
that mean we'd merely have to extend the bdinfo?

> Caches should be flushed before u-boot pass control to Linux that's why
> there shouldn't be any stale entries.
> 
> For ZynqMP you can partitioned memory that part of it goes to A53s and
> part to R5s and u-boot is capable to boot both cores.

Hm, maybe I'm not fully grasping the exact scenario you're envisioning.

>> For testing, sure, but to actually make use of it I'd rather see a clean
>> solution.
> 
> Do we have any experimental Kconfig entry which I can use for it?

Uh, the patch you posted I guess. Just make sure people don't set it if
they don't know *exactly* what they're getting themselves into.


Alex

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] libfdt: Add option to disable arch_fixup_fdt() calls
  2016-06-10 12:12             ` Alexander Graf
@ 2016-06-10 12:31               ` Michal Simek
  2016-06-10 12:34                 ` Alexander Graf
  0 siblings, 1 reply; 12+ messages in thread
From: Michal Simek @ 2016-06-10 12:31 UTC (permalink / raw)
  To: u-boot

On 10.6.2016 14:12, Alexander Graf wrote:
> 
> 
> On 10.06.16 13:51, Michal Simek wrote:
>> On 10.6.2016 13:13, Alexander Graf wrote:
>>>
>>>
>>> On 10.06.16 13:07, Michal Simek wrote:
>>>> On 9.6.2016 16:40, Alexander Graf wrote:
>>>>> On 06/09/2016 04:32 PM, Michal Simek wrote:
>>>>>> On 9.6.2016 16:29, Alexander Graf wrote:
>>>>>>> On 06/09/2016 04:23 PM, Michal Simek wrote:
>>>>>>>> Disable arch_fixup_fdt() calls for cases where U-Boot shouldn't update
>>>>>>>> memory setup in DTB file.
>>>>>>>> One example of usage of this option is to boot OS with different memory
>>>>>>>> setup than U-Boot use.
>>>>>>>>
>>>>>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>>>>> Could we instead just have the board file provide a fixup? It could then
>>>>>>> also fix up the efi memory map.
>>>>>> Not sure what exactly you are asking for.
>>>>>> Do you mean to add fixup function to board file and overwrite default
>>>>>> one?
>>>>>
>>>>> You have to touch some other code anyway to make this work with a
>>>>> particular board, right? In that case, you can as well add a function to
>>>>> the board file that explicitly provides a different, known good memory map.
>>>>
>>>> I cant' see the reason to touch particular board. In past AMP solution
>>>> where one core use the part of memory and second another part was the
>>>> reason I needed this.
>>>> For ARM64 case as you know from arm IRC I was trying to boot Linux from
>>>> memory above 32bit space and for these tests I need to convince u-boot
>>>> not to touch dtb memory setup.
>>>>
>>>> But for the same board I want to use standard behavior but for some case
>>>> this needs to be enabled.
>>>
>>> For that particular case, can you try to see whether you can convince
>>> U-Boot to just run in high memory instead? That would make things much
>>> more consistent IMHO.
>>>
>>> Giving U-Boot and Linux different views of the memory map is really just
>>> asking for trouble. You'd have to make sure that you flush your caches
>>> for example so that Linux doesn't suddenly get memory overwritten from
>>> stale cache entries on the low memory even though Linux is already
>>> accessing the high addresses.
>>
>> For systems where you have one main memory and standard usage model with
>> OS I agree that having this option enabled is bring more problems.
>> But for our use cases you can add memory controller to PL and it will be
>> ready when bitstream is loaded which can be done after u-boot is loaded.
>> It means u-boot have to work with different memory setup then Linux
>> later on.
> 
> Oh, I see. So U-Boot actually uses completely different memory? But that
> means that the original memory is also still there, doesn't it? Wouldn't
> that mean we'd merely have to extend the bdinfo?

It can be completely different memory or just part can be shared.
Depends on use case.
Also you can run just SW on particular core/cores.

I don't think that there is need for bootloader to know all stuff about
system. For me bootloader is here to help me to bring my app (or better
OS with APP) how I like.
And doing smart stuff is good but not for complicated cases that's why I
would like to have a button to disable it.

I can imagine for zcu102 which you also have to read SPD from memory and
based on that setup memory sizes and push this setting to OS as very
useful but running this standard OS style is just one particular use case.
R5s on the chip + the whole PL with possible cpus there requires
different behavior.

> 
>> Caches should be flushed before u-boot pass control to Linux that's why
>> there shouldn't be any stale entries.
>>
>> For ZynqMP you can partitioned memory that part of it goes to A53s and
>> part to R5s and u-boot is capable to boot both cores.
> 
> Hm, maybe I'm not fully grasping the exact scenario you're envisioning.

:-)

> 
>>> For testing, sure, but to actually make use of it I'd rather see a clean
>>> solution.
>>
>> Do we have any experimental Kconfig entry which I can use for it?
> 
> Uh, the patch you posted I guess. Just make sure people don't set it if
> they don't know *exactly* what they're getting themselves into.

It is not enabled by default but I am happy to add any other more
advance dependency.

Thanks,
Michal

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] libfdt: Add option to disable arch_fixup_fdt() calls
  2016-06-10 12:31               ` Michal Simek
@ 2016-06-10 12:34                 ` Alexander Graf
  0 siblings, 0 replies; 12+ messages in thread
From: Alexander Graf @ 2016-06-10 12:34 UTC (permalink / raw)
  To: u-boot



On 10.06.16 14:31, Michal Simek wrote:
> On 10.6.2016 14:12, Alexander Graf wrote:
>>
>>
>> On 10.06.16 13:51, Michal Simek wrote:
>>> On 10.6.2016 13:13, Alexander Graf wrote:
>>>>
>>>>
>>>> On 10.06.16 13:07, Michal Simek wrote:
>>>>> On 9.6.2016 16:40, Alexander Graf wrote:
>>>>>> On 06/09/2016 04:32 PM, Michal Simek wrote:
>>>>>>> On 9.6.2016 16:29, Alexander Graf wrote:
>>>>>>>> On 06/09/2016 04:23 PM, Michal Simek wrote:
>>>>>>>>> Disable arch_fixup_fdt() calls for cases where U-Boot shouldn't update
>>>>>>>>> memory setup in DTB file.
>>>>>>>>> One example of usage of this option is to boot OS with different memory
>>>>>>>>> setup than U-Boot use.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>>>>>> Could we instead just have the board file provide a fixup? It could then
>>>>>>>> also fix up the efi memory map.
>>>>>>> Not sure what exactly you are asking for.
>>>>>>> Do you mean to add fixup function to board file and overwrite default
>>>>>>> one?
>>>>>>
>>>>>> You have to touch some other code anyway to make this work with a
>>>>>> particular board, right? In that case, you can as well add a function to
>>>>>> the board file that explicitly provides a different, known good memory map.
>>>>>
>>>>> I cant' see the reason to touch particular board. In past AMP solution
>>>>> where one core use the part of memory and second another part was the
>>>>> reason I needed this.
>>>>> For ARM64 case as you know from arm IRC I was trying to boot Linux from
>>>>> memory above 32bit space and for these tests I need to convince u-boot
>>>>> not to touch dtb memory setup.
>>>>>
>>>>> But for the same board I want to use standard behavior but for some case
>>>>> this needs to be enabled.
>>>>
>>>> For that particular case, can you try to see whether you can convince
>>>> U-Boot to just run in high memory instead? That would make things much
>>>> more consistent IMHO.
>>>>
>>>> Giving U-Boot and Linux different views of the memory map is really just
>>>> asking for trouble. You'd have to make sure that you flush your caches
>>>> for example so that Linux doesn't suddenly get memory overwritten from
>>>> stale cache entries on the low memory even though Linux is already
>>>> accessing the high addresses.
>>>
>>> For systems where you have one main memory and standard usage model with
>>> OS I agree that having this option enabled is bring more problems.
>>> But for our use cases you can add memory controller to PL and it will be
>>> ready when bitstream is loaded which can be done after u-boot is loaded.
>>> It means u-boot have to work with different memory setup then Linux
>>> later on.
>>
>> Oh, I see. So U-Boot actually uses completely different memory? But that
>> means that the original memory is also still there, doesn't it? Wouldn't
>> that mean we'd merely have to extend the bdinfo?
> 
> It can be completely different memory or just part can be shared.
> Depends on use case.
> Also you can run just SW on particular core/cores.
> 
> I don't think that there is need for bootloader to know all stuff about
> system. For me bootloader is here to help me to bring my app (or better
> OS with APP) how I like.
> And doing smart stuff is good but not for complicated cases that's why I
> would like to have a button to disable it.
> 
> I can imagine for zcu102 which you also have to read SPD from memory and
> based on that setup memory sizes and push this setting to OS as very
> useful but running this standard OS style is just one particular use case.
> R5s on the chip + the whole PL with possible cpus there requires
> different behavior.
> 
>>
>>> Caches should be flushed before u-boot pass control to Linux that's why
>>> there shouldn't be any stale entries.
>>>
>>> For ZynqMP you can partitioned memory that part of it goes to A53s and
>>> part to R5s and u-boot is capable to boot both cores.
>>
>> Hm, maybe I'm not fully grasping the exact scenario you're envisioning.
> 
> :-)
> 
>>
>>>> For testing, sure, but to actually make use of it I'd rather see a clean
>>>> solution.
>>>
>>> Do we have any experimental Kconfig entry which I can use for it?
>>
>> Uh, the patch you posted I guess. Just make sure people don't set it if
>> they don't know *exactly* what they're getting themselves into.
> 
> It is not enabled by default but I am happy to add any other more
> advance dependency.

I don't think there's a need for an advanced dependency. Just a more
explicit warning in the help text should be enough.


Alex

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] libfdt: Add option to disable arch_fixup_fdt() calls
  2016-06-09 14:23 [U-Boot] [PATCH] libfdt: Add option to disable arch_fixup_fdt() calls Michal Simek
  2016-06-09 14:29 ` Alexander Graf
@ 2016-06-10 16:44 ` Simon Glass
  2016-07-15  7:36   ` Michal Simek
  1 sibling, 1 reply; 12+ messages in thread
From: Simon Glass @ 2016-06-10 16:44 UTC (permalink / raw)
  To: u-boot

Hi Michal,

On 9 June 2016 at 08:23, Michal Simek <michal.simek@xilinx.com> wrote:
> Disable arch_fixup_fdt() calls for cases where U-Boot shouldn't update
> memory setup in DTB file.
> One example of usage of this option is to boot OS with different memory
> setup than U-Boot use.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>
>  Kconfig                  | 8 ++++++++
>  arch/arm/lib/bootm-fdt.c | 2 ++
>  common/image-fdt.c       | 3 +++
>  3 files changed, 13 insertions(+)
>
> diff --git a/Kconfig b/Kconfig
> index 4b4621666560..3efba624ecff 100644
> --- a/Kconfig
> +++ b/Kconfig
> @@ -291,6 +291,14 @@ config SYS_CLK_FREQ
>         help
>           TODO: Move CONFIG_SYS_CLK_FREQ for all the architecture
>
> +config DISABLE_ARCH_FIXUP_FDT
> +       bool "Disable arch_fixup_fdt() call"
> +       depends on ARM
> +       help
> +         Disable FDT memory map syncup before OS boot. This feature can be
> +         used for booting OS with different memory setup where the part of
> +         the memory location should be used for different purpose.
> +
>  endmenu                # Boot images
>

I think this would be better as a positive Kconfig - ARCH_FIXUP_FDT.

Also this is pretty ugly - with a weak function that gets #ifdefed
out. How about changing it so that the function is only called if the
Kconfig is set, and then it does not have to be weak?

>  source "common/Kconfig"
> diff --git a/arch/arm/lib/bootm-fdt.c b/arch/arm/lib/bootm-fdt.c
> index 7677358742e1..a873b778ade9 100644
> --- a/arch/arm/lib/bootm-fdt.c
> +++ b/arch/arm/lib/bootm-fdt.c
> @@ -24,6 +24,7 @@
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> +#ifndef CONFIG_DISABLE_ARCH_FIXUP_FDT
>  int arch_fixup_fdt(void *blob)
>  {
>         bd_t *bd = gd->bd;
> @@ -50,3 +51,4 @@ int arch_fixup_fdt(void *blob)
>  #endif
>         return ret;
>  }
> +#endif
> diff --git a/common/image-fdt.c b/common/image-fdt.c
> index 6cac7dbb7f8b..9e05becd464a 100644
> --- a/common/image-fdt.c
> +++ b/common/image-fdt.c
> @@ -460,6 +460,9 @@ __weak int ft_verify_fdt(void *fdt)
>
>  __weak int arch_fixup_fdt(void *blob)
>  {
> +#ifdef CONFIG_DISABLE_ARCH_FIXUP_FDT
> +       printf("## Disable arch_fixup_fdt()\n");
> +#endif
>         return 0;
>  }
>
> --
> 1.9.1
>

Regards,
Simon

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] libfdt: Add option to disable arch_fixup_fdt() calls
  2016-06-10 16:44 ` Simon Glass
@ 2016-07-15  7:36   ` Michal Simek
  0 siblings, 0 replies; 12+ messages in thread
From: Michal Simek @ 2016-07-15  7:36 UTC (permalink / raw)
  To: u-boot

On 10.6.2016 18:44, Simon Glass wrote:
> Hi Michal,
> 
> On 9 June 2016 at 08:23, Michal Simek <michal.simek@xilinx.com> wrote:
>> Disable arch_fixup_fdt() calls for cases where U-Boot shouldn't update
>> memory setup in DTB file.
>> One example of usage of this option is to boot OS with different memory
>> setup than U-Boot use.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>>  Kconfig                  | 8 ++++++++
>>  arch/arm/lib/bootm-fdt.c | 2 ++
>>  common/image-fdt.c       | 3 +++
>>  3 files changed, 13 insertions(+)
>>
>> diff --git a/Kconfig b/Kconfig
>> index 4b4621666560..3efba624ecff 100644
>> --- a/Kconfig
>> +++ b/Kconfig
>> @@ -291,6 +291,14 @@ config SYS_CLK_FREQ
>>         help
>>           TODO: Move CONFIG_SYS_CLK_FREQ for all the architecture
>>
>> +config DISABLE_ARCH_FIXUP_FDT
>> +       bool "Disable arch_fixup_fdt() call"
>> +       depends on ARM
>> +       help
>> +         Disable FDT memory map syncup before OS boot. This feature can be
>> +         used for booting OS with different memory setup where the part of
>> +         the memory location should be used for different purpose.
>> +
>>  endmenu                # Boot images
>>
> 
> I think this would be better as a positive Kconfig - ARCH_FIXUP_FDT.
> 
> Also this is pretty ugly - with a weak function that gets #ifdefed
> out. How about changing it so that the function is only called if the
> Kconfig is set, and then it does not have to be weak?

All fixed in v2.

Thanks,
Michal

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2016-07-15  7:36 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-09 14:23 [U-Boot] [PATCH] libfdt: Add option to disable arch_fixup_fdt() calls Michal Simek
2016-06-09 14:29 ` Alexander Graf
2016-06-09 14:32   ` Michal Simek
2016-06-09 14:40     ` Alexander Graf
2016-06-10 11:07       ` Michal Simek
2016-06-10 11:13         ` Alexander Graf
2016-06-10 11:51           ` Michal Simek
2016-06-10 12:12             ` Alexander Graf
2016-06-10 12:31               ` Michal Simek
2016-06-10 12:34                 ` Alexander Graf
2016-06-10 16:44 ` Simon Glass
2016-07-15  7:36   ` Michal Simek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.