diff mbox

[U-Boot] arch: armv8: Remove the error when dcache is off

Message ID 1498125654-10417-1-git-send-email-sivadur@xilinx.com
State Deferred
Delegated to: Michal Simek
Headers show

Commit Message

Siva Durga Prasad Paladugu June 22, 2017, 10 a.m. UTC
Remove the error which causes compilation failure when
dcache is off for builds otherthan SPL. There may be
cases where user wants to disable dcache completely
eventhough it is not SPL.

Signed-off-by: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
---
 arch/arm/cpu/armv8/cache_v8.c | 9 ---------
 1 file changed, 9 deletions(-)

Comments

Michal Simek June 27, 2017, 9:04 a.m. UTC | #1
Hi guys,

do you have any concern about this change?

Thanks,
Michal

On 22.6.2017 12:00, Siva Durga Prasad Paladugu wrote:
> Remove the error which causes compilation failure when
> dcache is off for builds otherthan SPL. There may be
> cases where user wants to disable dcache completely
> eventhough it is not SPL.
> 
> Signed-off-by: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
> ---
>  arch/arm/cpu/armv8/cache_v8.c | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c
> index adc7e17..d11efcc 100644
> --- a/arch/arm/cpu/armv8/cache_v8.c
> +++ b/arch/arm/cpu/armv8/cache_v8.c
> @@ -647,15 +647,6 @@ void mmu_change_region_attr(phys_addr_t addr, size_t siz, u64 attrs)
>  
>  #else	/* CONFIG_SYS_DCACHE_OFF */
>  
> -/*
> - * For SPL builds, we may want to not have dcache enabled. Any real U-Boot
> - * running however really wants to have dcache and the MMU active. Check that
> - * everything is sane and give the developer a hint if it isn't.
> - */
> -#ifndef CONFIG_SPL_BUILD
> -#error Please describe your MMU layout in CONFIG_SYS_MEM_MAP and enable dcache.
> -#endif
> -
>  void invalidate_dcache_all(void)
>  {
>  }
>
Alexander Graf June 27, 2017, 11:01 a.m. UTC | #2
I don't think that's going to work - at least not without compiler flag 
changes. By default, gcc will happily generate unaligned accesses. If 
you disable dcache, these will trap.

Alex

On 27.06.17 11:04, Michal Simek wrote:
> Hi guys,
> 
> do you have any concern about this change?
> 
> Thanks,
> Michal
> 
> On 22.6.2017 12:00, Siva Durga Prasad Paladugu wrote:
>> Remove the error which causes compilation failure when
>> dcache is off for builds otherthan SPL. There may be
>> cases where user wants to disable dcache completely
>> eventhough it is not SPL.
>>
>> Signed-off-by: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
>> ---
>>   arch/arm/cpu/armv8/cache_v8.c | 9 ---------
>>   1 file changed, 9 deletions(-)
>>
>> diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c
>> index adc7e17..d11efcc 100644
>> --- a/arch/arm/cpu/armv8/cache_v8.c
>> +++ b/arch/arm/cpu/armv8/cache_v8.c
>> @@ -647,15 +647,6 @@ void mmu_change_region_attr(phys_addr_t addr, size_t siz, u64 attrs)
>>   
>>   #else	/* CONFIG_SYS_DCACHE_OFF */
>>   
>> -/*
>> - * For SPL builds, we may want to not have dcache enabled. Any real U-Boot
>> - * running however really wants to have dcache and the MMU active. Check that
>> - * everything is sane and give the developer a hint if it isn't.
>> - */
>> -#ifndef CONFIG_SPL_BUILD
>> -#error Please describe your MMU layout in CONFIG_SYS_MEM_MAP and enable dcache.
>> -#endif
>> -
>>   void invalidate_dcache_all(void)
>>   {
>>   }
>>
>
Michal Simek June 27, 2017, 11:20 a.m. UTC | #3
Hi,

On 27.6.2017 13:01, Alexander Graf wrote:
> I don't think that's going to work - at least not without compiler flag
> changes. By default, gcc will happily generate unaligned accesses. If
> you disable dcache, these will trap.

What's that compiler flags we should be using to avoid that?

The reason for this change is to have really small u-boot which fits to
OCM without DDR to be able to do initial programming.

Thanks,
Michal
Alexander Graf June 27, 2017, 11:46 a.m. UTC | #4
On 27.06.17 13:20, Michal Simek wrote:
> Hi,
> 
> On 27.6.2017 13:01, Alexander Graf wrote:
>> I don't think that's going to work - at least not without compiler flag
>> changes. By default, gcc will happily generate unaligned accesses. If
>> you disable dcache, these will trap.
> 
> What's that compiler flags we should be using to avoid that?

It's a combination of

   -mstrict-align

plus crossing fingers with lots of praying plus making sure that every 
code you call also follows -mstrict-align plus double-checking that you 
don't break the kernel booting ABI:

 
http://elixir.free-electrons.com/linux/v4.12-rc7/source/Documentation/arm64/booting.txt

In the booti case, disabling dcache seems to be legitimate. In the 
bootefi case however, it's not.

So you will also need to set CONFIG_EFI_LOADER to depend on 
!CONFIG_SYS_DCACHE_OFF which means you will want to convert 
CONFIG_SYS_DCACHE_OFF to Kconfig first :).

> The reason for this change is to have really small u-boot which fits to
> OCM without DDR to be able to do initial programming.

Yup, makes sense. I'm just slightly scared by the idea :).


Alex
Michal Simek June 27, 2017, 11:52 a.m. UTC | #5
On 27.6.2017 13:46, Alexander Graf wrote:
> 
> 
> On 27.06.17 13:20, Michal Simek wrote:
>> Hi,
>>
>> On 27.6.2017 13:01, Alexander Graf wrote:
>>> I don't think that's going to work - at least not without compiler flag
>>> changes. By default, gcc will happily generate unaligned accesses. If
>>> you disable dcache, these will trap.
>>
>> What's that compiler flags we should be using to avoid that?
> 
> It's a combination of
> 
>   -mstrict-align
> 
> plus crossing fingers with lots of praying plus making sure that every
> code you call also follows -mstrict-align plus double-checking that you
> don't break the kernel booting ABI:
> 
> 
> http://elixir.free-electrons.com/linux/v4.12-rc7/source/Documentation/arm64/booting.txt
> 
> 
> In the booti case, disabling dcache seems to be legitimate. In the
> bootefi case however, it's not.

Non wants to boot the kernel. It is really about programming stuff.

> 
> So you will also need to set CONFIG_EFI_LOADER to depend on
> !CONFIG_SYS_DCACHE_OFF which means you will want to convert
> CONFIG_SYS_DCACHE_OFF to Kconfig first :).

ok. I will let Siva to do it just wanted to refresh this topic.


>> The reason for this change is to have really small u-boot which fits to
>> OCM without DDR to be able to do initial programming.
> 
> Yup, makes sense. I'm just slightly scared by the idea :).

The same stuff we did on Zynq in past.
I have never had enough time to look at that MMU mapping why it is so
huge. Maybe reduce size of that tables or using different page size is
better way to go.

Thanks,
Michal
Alexander Graf June 27, 2017, 12:37 p.m. UTC | #6
> Am 27.06.2017 um 13:52 schrieb Michal Simek <michal.simek@xilinx.com>:
> 
>> On 27.6.2017 13:46, Alexander Graf wrote:
>> 
>> 
>>> On 27.06.17 13:20, Michal Simek wrote:
>>> Hi,
>>> 
>>>> On 27.6.2017 13:01, Alexander Graf wrote:
>>>> I don't think that's going to work - at least not without compiler flag
>>>> changes. By default, gcc will happily generate unaligned accesses. If
>>>> you disable dcache, these will trap.
>>> 
>>> What's that compiler flags we should be using to avoid that?
>> 
>> It's a combination of
>> 
>>  -mstrict-align
>> 
>> plus crossing fingers with lots of praying plus making sure that every
>> code you call also follows -mstrict-align plus double-checking that you
>> don't break the kernel booting ABI:
>> 
>> 
>> http://elixir.free-electrons.com/linux/v4.12-rc7/source/Documentation/arm64/booting.txt
>> 
>> 
>> In the booti case, disabling dcache seems to be legitimate. In the
>> bootefi case however, it's not.
> 
> Non wants to boot the kernel. It is really about programming stuff.
> 
>> 
>> So you will also need to set CONFIG_EFI_LOADER to depend on
>> !CONFIG_SYS_DCACHE_OFF which means you will want to convert
>> CONFIG_SYS_DCACHE_OFF to Kconfig first :).
> 
> ok. I will let Siva to do it just wanted to refresh this topic.
> 
> 
>>> The reason for this change is to have really small u-boot which fits to
>>> OCM without DDR to be able to do initial programming.
>> 
>> Yup, makes sense. I'm just slightly scared by the idea :).
> 
> The same stuff we did on Zynq in past.
> I have never had enough time to look at that MMU mapping why it is so
> huge. Maybe reduce size of that tables or using different page size is
> better way to go.

If you configure your section boundaries on natural alignments (1GB IIRC), you should get away with quite few tables.

Alex
Siva Durga Prasad Paladugu June 29, 2017, 1:44 p.m. UTC | #7
Hi,

> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Tuesday, June 27, 2017 6:08 PM
> To: Michal Simek <michal.simek@xilinx.com>
> Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>; u-boot@lists.denx.de;
> Tom Rini <trini@konsulko.com>; Simon Glass <sjg@chromium.org>; Siva
> Durga Prasad Paladugu <sivadur@xilinx.com>
> Subject: Re: [U-Boot] [PATCH] arch: armv8: Remove the error when dcache is
> off
> 
> 
> 
> > Am 27.06.2017 um 13:52 schrieb Michal Simek <michal.simek@xilinx.com>:
> >
> >> On 27.6.2017 13:46, Alexander Graf wrote:
> >>
> >>
> >>> On 27.06.17 13:20, Michal Simek wrote:
> >>> Hi,
> >>>
> >>>> On 27.6.2017 13:01, Alexander Graf wrote:
> >>>> I don't think that's going to work - at least not without compiler
> >>>> flag changes. By default, gcc will happily generate unaligned
> >>>> accesses. If you disable dcache, these will trap.
> >>>
> >>> What's that compiler flags we should be using to avoid that?
> >>
> >> It's a combination of
> >>
> >>  -mstrict-align
> >>
> >> plus crossing fingers with lots of praying plus making sure that
> >> every code you call also follows -mstrict-align plus double-checking
> >> that you don't break the kernel booting ABI:
> >>
> >>
> >> http://elixir.free-electrons.com/linux/v4.12-rc7/source/Documentation
> >> /arm64/booting.txt
> >>
> >>
> >> In the booti case, disabling dcache seems to be legitimate. In the
> >> bootefi case however, it's not.
> >
> > Non wants to boot the kernel. It is really about programming stuff.
> >
> >>
> >> So you will also need to set CONFIG_EFI_LOADER to depend on
> >> !CONFIG_SYS_DCACHE_OFF which means you will want to convert
> >> CONFIG_SYS_DCACHE_OFF to Kconfig first :).
> >
> > ok. I will let Siva to do it just wanted to refresh this topic.
> >
> >
> >>> The reason for this change is to have really small u-boot which fits
> >>> to OCM without DDR to be able to do initial programming.
> >>
> >> Yup, makes sense. I'm just slightly scared by the idea :).

I can see that, we are anyway avoiding that error message for SPL BUILD where
dcache is off. 
Also, dcache will be ON by default, until and unless user disable it
through CONFIG_SYS_DCACHE_OFF.

-mstrict-align is already used for armv8 in arch/arm/cpu/armv8/config.mk

Finally I feel, we can remove this check or atleast we should make it as #warning instead of #error.

Thanks,
Siva.

> >
> > The same stuff we did on Zynq in past.
> > I have never had enough time to look at that MMU mapping why it is so
> > huge. Maybe reduce size of that tables or using different page size is
> > better way to go.
> 
> If you configure your section boundaries on natural alignments (1GB IIRC),
> you should get away with quite few tables.
> 
> Alex
>
Alexander Graf June 29, 2017, 2:04 p.m. UTC | #8
On 29.06.17 15:44, Siva Durga Prasad Paladugu wrote:
> Hi,
> 
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf@suse.de]
>> Sent: Tuesday, June 27, 2017 6:08 PM
>> To: Michal Simek <michal.simek@xilinx.com>
>> Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>; u-boot@lists.denx.de;
>> Tom Rini <trini@konsulko.com>; Simon Glass <sjg@chromium.org>; Siva
>> Durga Prasad Paladugu <sivadur@xilinx.com>
>> Subject: Re: [U-Boot] [PATCH] arch: armv8: Remove the error when dcache is
>> off
>>
>>
>>
>>> Am 27.06.2017 um 13:52 schrieb Michal Simek <michal.simek@xilinx.com>:
>>>
>>>> On 27.6.2017 13:46, Alexander Graf wrote:
>>>>
>>>>
>>>>> On 27.06.17 13:20, Michal Simek wrote:
>>>>> Hi,
>>>>>
>>>>>> On 27.6.2017 13:01, Alexander Graf wrote:
>>>>>> I don't think that's going to work - at least not without compiler
>>>>>> flag changes. By default, gcc will happily generate unaligned
>>>>>> accesses. If you disable dcache, these will trap.
>>>>>
>>>>> What's that compiler flags we should be using to avoid that?
>>>>
>>>> It's a combination of
>>>>
>>>>   -mstrict-align
>>>>
>>>> plus crossing fingers with lots of praying plus making sure that
>>>> every code you call also follows -mstrict-align plus double-checking
>>>> that you don't break the kernel booting ABI:
>>>>
>>>>
>>>> http://elixir.free-electrons.com/linux/v4.12-rc7/source/Documentation
>>>> /arm64/booting.txt
>>>>
>>>>
>>>> In the booti case, disabling dcache seems to be legitimate. In the
>>>> bootefi case however, it's not.
>>>
>>> Non wants to boot the kernel. It is really about programming stuff.
>>>
>>>>
>>>> So you will also need to set CONFIG_EFI_LOADER to depend on
>>>> !CONFIG_SYS_DCACHE_OFF which means you will want to convert
>>>> CONFIG_SYS_DCACHE_OFF to Kconfig first :).
>>>
>>> ok. I will let Siva to do it just wanted to refresh this topic.
>>>
>>>
>>>>> The reason for this change is to have really small u-boot which fits
>>>>> to OCM without DDR to be able to do initial programming.
>>>>
>>>> Yup, makes sense. I'm just slightly scared by the idea :).
> 
> I can see that, we are anyway avoiding that error message for SPL BUILD where
> dcache is off.
> Also, dcache will be ON by default, until and unless user disable it
> through CONFIG_SYS_DCACHE_OFF.
> 
> -mstrict-align is already used for armv8 in arch/arm/cpu/armv8/config.mk
> 
> Finally I feel, we can remove this check or atleast we should make it as #warning instead of #error.

I put it in because UEFI binaries can assume that unaligned instructions 
work. So please at least convert CONFIG_SYS_DCACHE_OFF to Kconfig and 
make it mutually exclusive to CONFIG_EFI_LOADER.


Alex
Siva Durga Prasad Paladugu July 3, 2017, 6:29 a.m. UTC | #9
Hi Alex,

> -----Original Message-----

> From: Alexander Graf [mailto:agraf@suse.de]

> Sent: Thursday, June 29, 2017 7:34 PM

> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>; Michal Simek

> <michal.simek@xilinx.com>

> Cc: u-boot@lists.denx.de; Tom Rini <trini@konsulko.com>; Simon Glass

> <sjg@chromium.org>

> Subject: Re: [U-Boot] [PATCH] arch: armv8: Remove the error when dcache is

> off

> 

> 

> 

> On 29.06.17 15:44, Siva Durga Prasad Paladugu wrote:

> > Hi,

> >

> >> -----Original Message-----

> >> From: Alexander Graf [mailto:agraf@suse.de]

> >> Sent: Tuesday, June 27, 2017 6:08 PM

> >> To: Michal Simek <michal.simek@xilinx.com>

> >> Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>;

> >> u-boot@lists.denx.de; Tom Rini <trini@konsulko.com>; Simon Glass

> >> <sjg@chromium.org>; Siva Durga Prasad Paladugu <sivadur@xilinx.com>

> >> Subject: Re: [U-Boot] [PATCH] arch: armv8: Remove the error when

> >> dcache is off

> >>

> >>

> >>

> >>> Am 27.06.2017 um 13:52 schrieb Michal Simek

> <michal.simek@xilinx.com>:

> >>>

> >>>> On 27.6.2017 13:46, Alexander Graf wrote:

> >>>>

> >>>>

> >>>>> On 27.06.17 13:20, Michal Simek wrote:

> >>>>> Hi,

> >>>>>

> >>>>>> On 27.6.2017 13:01, Alexander Graf wrote:

> >>>>>> I don't think that's going to work - at least not without

> >>>>>> compiler flag changes. By default, gcc will happily generate

> >>>>>> unaligned accesses. If you disable dcache, these will trap.

> >>>>>

> >>>>> What's that compiler flags we should be using to avoid that?

> >>>>

> >>>> It's a combination of

> >>>>

> >>>>   -mstrict-align

> >>>>

> >>>> plus crossing fingers with lots of praying plus making sure that

> >>>> every code you call also follows -mstrict-align plus

> >>>> double-checking that you don't break the kernel booting ABI:

> >>>>

> >>>>

> >>>> http://elixir.free-electrons.com/linux/v4.12-rc7/source/Documentati

> >>>> on

> >>>> /arm64/booting.txt

> >>>>

> >>>>

> >>>> In the booti case, disabling dcache seems to be legitimate. In the

> >>>> bootefi case however, it's not.

> >>>

> >>> Non wants to boot the kernel. It is really about programming stuff.

> >>>

> >>>>

> >>>> So you will also need to set CONFIG_EFI_LOADER to depend on

> >>>> !CONFIG_SYS_DCACHE_OFF which means you will want to convert

> >>>> CONFIG_SYS_DCACHE_OFF to Kconfig first :).

> >>>

> >>> ok. I will let Siva to do it just wanted to refresh this topic.

> >>>

> >>>

> >>>>> The reason for this change is to have really small u-boot which

> >>>>> fits to OCM without DDR to be able to do initial programming.

> >>>>

> >>>> Yup, makes sense. I'm just slightly scared by the idea :).

> >

> > I can see that, we are anyway avoiding that error message for SPL

> > BUILD where dcache is off.

> > Also, dcache will be ON by default, until and unless user disable it

> > through CONFIG_SYS_DCACHE_OFF.

> >

> > -mstrict-align is already used for armv8 in

> > arch/arm/cpu/armv8/config.mk

> >

> > Finally I feel, we can remove this check or atleast we should make it as

> #warning instead of #error.

> 

> I put it in because UEFI binaries can assume that unaligned instructions

> work. So please at least convert CONFIG_SYS_DCACHE_OFF to Kconfig and

> make it mutually exclusive to CONFIG_EFI_LOADER.


I got your point but, Iam bit concerned to do this change as it may break for someone 
if they are using both DCACHE_OFF and EFI_LOADER, Isn't it? 
I don’t have idea on UEFI side, all assume unaligned instructions work or it is configurable when building?

Thanks,
Siva

> 

> 

> Alex
Alexander Graf July 3, 2017, 6:48 a.m. UTC | #10
On 03.07.17 08:29, Siva Durga Prasad Paladugu wrote:
> Hi Alex,
> 
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf@suse.de]
>> Sent: Thursday, June 29, 2017 7:34 PM
>> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>; Michal Simek
>> <michal.simek@xilinx.com>
>> Cc: u-boot@lists.denx.de; Tom Rini <trini@konsulko.com>; Simon Glass
>> <sjg@chromium.org>
>> Subject: Re: [U-Boot] [PATCH] arch: armv8: Remove the error when dcache is
>> off
>>
>>
>>
>> On 29.06.17 15:44, Siva Durga Prasad Paladugu wrote:
>>> Hi,
>>>
>>>> -----Original Message-----
>>>> From: Alexander Graf [mailto:agraf@suse.de]
>>>> Sent: Tuesday, June 27, 2017 6:08 PM
>>>> To: Michal Simek <michal.simek@xilinx.com>
>>>> Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>;
>>>> u-boot@lists.denx.de; Tom Rini <trini@konsulko.com>; Simon Glass
>>>> <sjg@chromium.org>; Siva Durga Prasad Paladugu <sivadur@xilinx.com>
>>>> Subject: Re: [U-Boot] [PATCH] arch: armv8: Remove the error when
>>>> dcache is off
>>>>
>>>>
>>>>
>>>>> Am 27.06.2017 um 13:52 schrieb Michal Simek
>> <michal.simek@xilinx.com>:
>>>>>
>>>>>> On 27.6.2017 13:46, Alexander Graf wrote:
>>>>>>
>>>>>>
>>>>>>> On 27.06.17 13:20, Michal Simek wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>>> On 27.6.2017 13:01, Alexander Graf wrote:
>>>>>>>> I don't think that's going to work - at least not without
>>>>>>>> compiler flag changes. By default, gcc will happily generate
>>>>>>>> unaligned accesses. If you disable dcache, these will trap.
>>>>>>>
>>>>>>> What's that compiler flags we should be using to avoid that?
>>>>>>
>>>>>> It's a combination of
>>>>>>
>>>>>>    -mstrict-align
>>>>>>
>>>>>> plus crossing fingers with lots of praying plus making sure that
>>>>>> every code you call also follows -mstrict-align plus
>>>>>> double-checking that you don't break the kernel booting ABI:
>>>>>>
>>>>>>
>>>>>> http://elixir.free-electrons.com/linux/v4.12-rc7/source/Documentati
>>>>>> on
>>>>>> /arm64/booting.txt
>>>>>>
>>>>>>
>>>>>> In the booti case, disabling dcache seems to be legitimate. In the
>>>>>> bootefi case however, it's not.
>>>>>
>>>>> Non wants to boot the kernel. It is really about programming stuff.
>>>>>
>>>>>>
>>>>>> So you will also need to set CONFIG_EFI_LOADER to depend on
>>>>>> !CONFIG_SYS_DCACHE_OFF which means you will want to convert
>>>>>> CONFIG_SYS_DCACHE_OFF to Kconfig first :).
>>>>>
>>>>> ok. I will let Siva to do it just wanted to refresh this topic.
>>>>>
>>>>>
>>>>>>> The reason for this change is to have really small u-boot which
>>>>>>> fits to OCM without DDR to be able to do initial programming.
>>>>>>
>>>>>> Yup, makes sense. I'm just slightly scared by the idea :).
>>>
>>> I can see that, we are anyway avoiding that error message for SPL
>>> BUILD where dcache is off.
>>> Also, dcache will be ON by default, until and unless user disable it
>>> through CONFIG_SYS_DCACHE_OFF.
>>>
>>> -mstrict-align is already used for armv8 in
>>> arch/arm/cpu/armv8/config.mk
>>>
>>> Finally I feel, we can remove this check or atleast we should make it as
>> #warning instead of #error.
>>
>> I put it in because UEFI binaries can assume that unaligned instructions
>> work. So please at least convert CONFIG_SYS_DCACHE_OFF to Kconfig and
>> make it mutually exclusive to CONFIG_EFI_LOADER.
> 
> I got your point but, Iam bit concerned to do this change as it may break for someone
> if they are using both DCACHE_OFF and EFI_LOADER, Isn't it?

Today you can not use DCACHE_OFF at all, so I don't think it'd be a 
regression ;). I want to make sure that we prohibit the combination on 
the Kconfig level already to make sure that we do not break anyone using 
DCACHE_OFF if we start to allow it.

> I don’t have idea on UEFI side, all assume unaligned instructions work or it is configurable when building?

With UEFI you run 3rd party binaries in U-Boot context. The UEFI spec 
explicitly states that unaligned accesses are valid. So because we by 
definition don't control the payloads, we can not break unaligned 
accesses which DCACHE_OFF does.


Alex
Alexander Graf July 3, 2017, 6:54 a.m. UTC | #11
On 27.06.17 13:52, Michal Simek wrote:
> On 27.6.2017 13:46, Alexander Graf wrote:
>>
>>
>> On 27.06.17 13:20, Michal Simek wrote:
>>> Hi,
>>>
>>> On 27.6.2017 13:01, Alexander Graf wrote:
>>>> I don't think that's going to work - at least not without compiler flag
>>>> changes. By default, gcc will happily generate unaligned accesses. If
>>>> you disable dcache, these will trap.
>>>
>>> What's that compiler flags we should be using to avoid that?
>>
>> It's a combination of
>>
>>    -mstrict-align
>>
>> plus crossing fingers with lots of praying plus making sure that every
>> code you call also follows -mstrict-align plus double-checking that you
>> don't break the kernel booting ABI:
>>
>>
>> http://elixir.free-electrons.com/linux/v4.12-rc7/source/Documentation/arm64/booting.txt
>>
>>
>> In the booti case, disabling dcache seems to be legitimate. In the
>> bootefi case however, it's not.
> 
> Non wants to boot the kernel. It is really about programming stuff.
> 
>>
>> So you will also need to set CONFIG_EFI_LOADER to depend on
>> !CONFIG_SYS_DCACHE_OFF which means you will want to convert
>> CONFIG_SYS_DCACHE_OFF to Kconfig first :).
> 
> ok. I will let Siva to do it just wanted to refresh this topic.
> 
> 
>>> The reason for this change is to have really small u-boot which fits to
>>> OCM without DDR to be able to do initial programming.
>>
>> Yup, makes sense. I'm just slightly scared by the idea :).
> 
> The same stuff we did on Zynq in past.
> I have never had enough time to look at that MMU mapping why it is so
> huge. Maybe reduce size of that tables or using different page size is
> better way to go.

Actually thinking about this one again. I'm fairly sure you could 
provide page-aligned gigabyte page maps in .rodata manually for your 
board in your specific configuration. The mmu setup function is weak, so 
you can simply override it and have it point the mmu to your 
pregenerated page table.

With that, the overhead of having caches on shouldn't be too much. 
Especially if you can make the page table aligned, but not padded to 4k.


Alex
Siva Durga Prasad Paladugu July 3, 2017, 6:54 a.m. UTC | #12
Hi Alex,

> -----Original Message-----

> From: Alexander Graf [mailto:agraf@suse.de]

> Sent: Monday, July 03, 2017 12:19 PM

> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>; Michal Simek

> <michal.simek@xilinx.com>

> Cc: u-boot@lists.denx.de; Tom Rini <trini@konsulko.com>; Simon Glass

> <sjg@chromium.org>

> Subject: Re: [U-Boot] [PATCH] arch: armv8: Remove the error when dcache is

> off

> 

> 

> 

> On 03.07.17 08:29, Siva Durga Prasad Paladugu wrote:

> > Hi Alex,

> >

> >> -----Original Message-----

> >> From: Alexander Graf [mailto:agraf@suse.de]

> >> Sent: Thursday, June 29, 2017 7:34 PM

> >> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>; Michal Simek

> >> <michal.simek@xilinx.com>

> >> Cc: u-boot@lists.denx.de; Tom Rini <trini@konsulko.com>; Simon Glass

> >> <sjg@chromium.org>

> >> Subject: Re: [U-Boot] [PATCH] arch: armv8: Remove the error when

> >> dcache is off

> >>

> >>

> >>

> >> On 29.06.17 15:44, Siva Durga Prasad Paladugu wrote:

> >>> Hi,

> >>>

> >>>> -----Original Message-----

> >>>> From: Alexander Graf [mailto:agraf@suse.de]

> >>>> Sent: Tuesday, June 27, 2017 6:08 PM

> >>>> To: Michal Simek <michal.simek@xilinx.com>

> >>>> Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>;

> >>>> u-boot@lists.denx.de; Tom Rini <trini@konsulko.com>; Simon Glass

> >>>> <sjg@chromium.org>; Siva Durga Prasad Paladugu

> <sivadur@xilinx.com>

> >>>> Subject: Re: [U-Boot] [PATCH] arch: armv8: Remove the error when

> >>>> dcache is off

> >>>>

> >>>>

> >>>>

> >>>>> Am 27.06.2017 um 13:52 schrieb Michal Simek

> >> <michal.simek@xilinx.com>:

> >>>>>

> >>>>>> On 27.6.2017 13:46, Alexander Graf wrote:

> >>>>>>

> >>>>>>

> >>>>>>> On 27.06.17 13:20, Michal Simek wrote:

> >>>>>>> Hi,

> >>>>>>>

> >>>>>>>> On 27.6.2017 13:01, Alexander Graf wrote:

> >>>>>>>> I don't think that's going to work - at least not without

> >>>>>>>> compiler flag changes. By default, gcc will happily generate

> >>>>>>>> unaligned accesses. If you disable dcache, these will trap.

> >>>>>>>

> >>>>>>> What's that compiler flags we should be using to avoid that?

> >>>>>>

> >>>>>> It's a combination of

> >>>>>>

> >>>>>>    -mstrict-align

> >>>>>>

> >>>>>> plus crossing fingers with lots of praying plus making sure that

> >>>>>> every code you call also follows -mstrict-align plus

> >>>>>> double-checking that you don't break the kernel booting ABI:

> >>>>>>

> >>>>>>

> >>>>>> http://elixir.free-electrons.com/linux/v4.12-rc7/source/Documenta

> >>>>>> ti

> >>>>>> on

> >>>>>> /arm64/booting.txt

> >>>>>>

> >>>>>>

> >>>>>> In the booti case, disabling dcache seems to be legitimate. In

> >>>>>> the bootefi case however, it's not.

> >>>>>

> >>>>> Non wants to boot the kernel. It is really about programming stuff.

> >>>>>

> >>>>>>

> >>>>>> So you will also need to set CONFIG_EFI_LOADER to depend on

> >>>>>> !CONFIG_SYS_DCACHE_OFF which means you will want to convert

> >>>>>> CONFIG_SYS_DCACHE_OFF to Kconfig first :).

> >>>>>

> >>>>> ok. I will let Siva to do it just wanted to refresh this topic.

> >>>>>

> >>>>>

> >>>>>>> The reason for this change is to have really small u-boot which

> >>>>>>> fits to OCM without DDR to be able to do initial programming.

> >>>>>>

> >>>>>> Yup, makes sense. I'm just slightly scared by the idea :).

> >>>

> >>> I can see that, we are anyway avoiding that error message for SPL

> >>> BUILD where dcache is off.

> >>> Also, dcache will be ON by default, until and unless user disable it

> >>> through CONFIG_SYS_DCACHE_OFF.

> >>>

> >>> -mstrict-align is already used for armv8 in

> >>> arch/arm/cpu/armv8/config.mk

> >>>

> >>> Finally I feel, we can remove this check or atleast we should make

> >>> it as

> >> #warning instead of #error.

> >>

> >> I put it in because UEFI binaries can assume that unaligned

> >> instructions work. So please at least convert CONFIG_SYS_DCACHE_OFF

> >> to Kconfig and make it mutually exclusive to CONFIG_EFI_LOADER.

> >

> > I got your point but, Iam bit concerned to do this change as it may

> > break for someone if they are using both DCACHE_OFF and EFI_LOADER,

> Isn't it?

> 

> Today you can not use DCACHE_OFF at all, so I don't think it'd be a

> regression ;). I want to make sure that we prohibit the combination on the

> Kconfig level already to make sure that we do not break anyone using

> DCACHE_OFF if we start to allow it.

> 

> > I don’t have idea on UEFI side, all assume unaligned instructions work or it

> is configurable when building?

> 

> With UEFI you run 3rd party binaries in U-Boot context. The UEFI spec

> explicitly states that unaligned accesses are valid. So because we by

> definition don't control the payloads, we can not break unaligned accesses

> which DCACHE_OFF does.


Thanks for clarification, then I will go ahead and make the change as you 
suggested.(convert CONFIG_SYS_DCACHE_OFF to Kconfig and make it mutually
exclusive to CONFIG_EFI_LOADER).

Thanks,
Siva
> 

> 

> Alex
Siva Durga Prasad Paladugu July 3, 2017, 7:03 a.m. UTC | #13
Hi Alex,

> -----Original Message-----

> From: Alexander Graf [mailto:agraf@suse.de]

> Sent: Monday, July 03, 2017 12:24 PM

> To: Michal Simek <michal.simek@xilinx.com>; Siva Durga Prasad Paladugu

> <sivadur@xilinx.com>; u-boot@lists.denx.de; Tom Rini

> <trini@konsulko.com>; Simon Glass <sjg@chromium.org>

> Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>

> Subject: Re: [U-Boot] [PATCH] arch: armv8: Remove the error when dcache is

> off

> 

> 

> 

> On 27.06.17 13:52, Michal Simek wrote:

> > On 27.6.2017 13:46, Alexander Graf wrote:

> >>

> >>

> >> On 27.06.17 13:20, Michal Simek wrote:

> >>> Hi,

> >>>

> >>> On 27.6.2017 13:01, Alexander Graf wrote:

> >>>> I don't think that's going to work - at least not without compiler

> >>>> flag changes. By default, gcc will happily generate unaligned

> >>>> accesses. If you disable dcache, these will trap.

> >>>

> >>> What's that compiler flags we should be using to avoid that?

> >>

> >> It's a combination of

> >>

> >>    -mstrict-align

> >>

> >> plus crossing fingers with lots of praying plus making sure that

> >> every code you call also follows -mstrict-align plus double-checking

> >> that you don't break the kernel booting ABI:

> >>

> >>

> >> http://elixir.free-electrons.com/linux/v4.12-rc7/source/Documentation

> >> /arm64/booting.txt

> >>

> >>

> >> In the booti case, disabling dcache seems to be legitimate. In the

> >> bootefi case however, it's not.

> >

> > Non wants to boot the kernel. It is really about programming stuff.

> >

> >>

> >> So you will also need to set CONFIG_EFI_LOADER to depend on

> >> !CONFIG_SYS_DCACHE_OFF which means you will want to convert

> >> CONFIG_SYS_DCACHE_OFF to Kconfig first :).

> >

> > ok. I will let Siva to do it just wanted to refresh this topic.

> >

> >

> >>> The reason for this change is to have really small u-boot which fits

> >>> to OCM without DDR to be able to do initial programming.

> >>

> >> Yup, makes sense. I'm just slightly scared by the idea :).

> >

> > The same stuff we did on Zynq in past.

> > I have never had enough time to look at that MMU mapping why it is so

> > huge. Maybe reduce size of that tables or using different page size is

> > better way to go.

> 

> Actually thinking about this one again. I'm fairly sure you could provide

> page-aligned gigabyte page maps in .rodata manually for your board in your

> specific configuration. The mmu setup function is weak, so you can simply

> override it and have it point the mmu to your pregenerated page table.

> 

> With that, the overhead of having caches on shouldn't be too much.

> Especially if you can make the page table aligned, but not padded to 4k.


Yeah you are correct, I considered this, the other issue is that while relocating, we are aligning it
to 64K boundary which again result in eating up more space in my case.
Refer file common/board_f.c and routine reserve_mmu().

Also other thought is, If we are providing the flexibility of caches off in other arm architectures
then why not here?

Thanks,
Siva.


> 

> 

> Alex
Alexander Graf July 3, 2017, 7:08 a.m. UTC | #14
On 03.07.17 09:03, Siva Durga Prasad Paladugu wrote:
> Hi Alex,
> 
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf@suse.de]
>> Sent: Monday, July 03, 2017 12:24 PM
>> To: Michal Simek <michal.simek@xilinx.com>; Siva Durga Prasad Paladugu
>> <sivadur@xilinx.com>; u-boot@lists.denx.de; Tom Rini
>> <trini@konsulko.com>; Simon Glass <sjg@chromium.org>
>> Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
>> Subject: Re: [U-Boot] [PATCH] arch: armv8: Remove the error when dcache is
>> off
>>
>>
>>
>> On 27.06.17 13:52, Michal Simek wrote:
>>> On 27.6.2017 13:46, Alexander Graf wrote:
>>>>
>>>>
>>>> On 27.06.17 13:20, Michal Simek wrote:
>>>>> Hi,
>>>>>
>>>>> On 27.6.2017 13:01, Alexander Graf wrote:
>>>>>> I don't think that's going to work - at least not without compiler
>>>>>> flag changes. By default, gcc will happily generate unaligned
>>>>>> accesses. If you disable dcache, these will trap.
>>>>>
>>>>> What's that compiler flags we should be using to avoid that?
>>>>
>>>> It's a combination of
>>>>
>>>>     -mstrict-align
>>>>
>>>> plus crossing fingers with lots of praying plus making sure that
>>>> every code you call also follows -mstrict-align plus double-checking
>>>> that you don't break the kernel booting ABI:
>>>>
>>>>
>>>> http://elixir.free-electrons.com/linux/v4.12-rc7/source/Documentation
>>>> /arm64/booting.txt
>>>>
>>>>
>>>> In the booti case, disabling dcache seems to be legitimate. In the
>>>> bootefi case however, it's not.
>>>
>>> Non wants to boot the kernel. It is really about programming stuff.
>>>
>>>>
>>>> So you will also need to set CONFIG_EFI_LOADER to depend on
>>>> !CONFIG_SYS_DCACHE_OFF which means you will want to convert
>>>> CONFIG_SYS_DCACHE_OFF to Kconfig first :).
>>>
>>> ok. I will let Siva to do it just wanted to refresh this topic.
>>>
>>>
>>>>> The reason for this change is to have really small u-boot which fits
>>>>> to OCM without DDR to be able to do initial programming.
>>>>
>>>> Yup, makes sense. I'm just slightly scared by the idea :).
>>>
>>> The same stuff we did on Zynq in past.
>>> I have never had enough time to look at that MMU mapping why it is so
>>> huge. Maybe reduce size of that tables or using different page size is
>>> better way to go.
>>
>> Actually thinking about this one again. I'm fairly sure you could provide
>> page-aligned gigabyte page maps in .rodata manually for your board in your
>> specific configuration. The mmu setup function is weak, so you can simply
>> override it and have it point the mmu to your pregenerated page table.
>>
>> With that, the overhead of having caches on shouldn't be too much.
>> Especially if you can make the page table aligned, but not padded to 4k.
> 
> Yeah you are correct, I considered this, the other issue is that while relocating, we are aligning it
> to 64K boundary which again result in eating up more space in my case.
> Refer file common/board_f.c and routine reserve_mmu().

Yes, you would need to have a special case - something like 
CONFIG_SYS_STATIC_MMU_CONFIG and in that case not reserve anything.

> 
> Also other thought is, If we are providing the flexibility of caches off in other arm architectures
> then why not here?

Because when I went through all supported boards, a lot of them had 
caches disabled by accident. And almost nobody wants to run with caches 
off. It's significantly slower for almost no gain.


Alex
diff mbox

Patch

diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c
index adc7e17..d11efcc 100644
--- a/arch/arm/cpu/armv8/cache_v8.c
+++ b/arch/arm/cpu/armv8/cache_v8.c
@@ -647,15 +647,6 @@  void mmu_change_region_attr(phys_addr_t addr, size_t siz, u64 attrs)
 
 #else	/* CONFIG_SYS_DCACHE_OFF */
 
-/*
- * For SPL builds, we may want to not have dcache enabled. Any real U-Boot
- * running however really wants to have dcache and the MMU active. Check that
- * everything is sane and give the developer a hint if it isn't.
- */
-#ifndef CONFIG_SPL_BUILD
-#error Please describe your MMU layout in CONFIG_SYS_MEM_MAP and enable dcache.
-#endif
-
 void invalidate_dcache_all(void)
 {
 }