* [U-Boot] [PATCH] efi_loader: Always flush in cache line size granularity
@ 2016-04-04 7:32 Alexander Graf
2016-04-04 7:56 ` Marek Vasut
2016-04-10 22:24 ` Andreas Färber
0 siblings, 2 replies; 9+ messages in thread
From: Alexander Graf @ 2016-04-04 7:32 UTC (permalink / raw)
To: u-boot
The cache line flush helpers only work properly when they get aligned
start and end addresses. Round our flush range to cache line size. It's
safe because we're guaranteed to flush within a single page which has the
same cache attributes.
Reported-by: Marek Vasut <marex@denx.de>
Signed-off-by: Alexander Graf <agraf@suse.de>
---
lib/efi_loader/efi_runtime.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
index 22bcd08..40acec0 100644
--- a/lib/efi_loader/efi_runtime.c
+++ b/lib/efi_loader/efi_runtime.c
@@ -194,7 +194,8 @@ void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map)
#endif
*p = newaddr;
- flush_dcache_range((ulong)p, (ulong)&p[1]);
+ flush_dcache_range((ulong)p & ~(CONFIG_SYS_CACHELINE_SIZE - 1),
+ ALIGN((ulong)&p[1], CONFIG_SYS_CACHELINE_SIZE));
}
#ifndef IS_RELA
--
1.8.5.6
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH] efi_loader: Always flush in cache line size granularity
2016-04-04 7:32 [U-Boot] [PATCH] efi_loader: Always flush in cache line size granularity Alexander Graf
@ 2016-04-04 7:56 ` Marek Vasut
2016-04-10 22:24 ` Andreas Färber
1 sibling, 0 replies; 9+ messages in thread
From: Marek Vasut @ 2016-04-04 7:56 UTC (permalink / raw)
To: u-boot
On 04/04/2016 09:32 AM, Alexander Graf wrote:
> The cache line flush helpers only work properly when they get aligned
> start and end addresses. Round our flush range to cache line size. It's
> safe because we're guaranteed to flush within a single page which has the
> same cache attributes.
>
> Reported-by: Marek Vasut <marex@denx.de>
> Signed-off-by: Alexander Graf <agraf@suse.de>
Thanks!
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH] efi_loader: Always flush in cache line size granularity
2016-04-04 7:32 [U-Boot] [PATCH] efi_loader: Always flush in cache line size granularity Alexander Graf
2016-04-04 7:56 ` Marek Vasut
@ 2016-04-10 22:24 ` Andreas Färber
2016-04-10 22:31 ` Alexander Graf
1 sibling, 1 reply; 9+ messages in thread
From: Andreas Färber @ 2016-04-10 22:24 UTC (permalink / raw)
To: u-boot
Am 04.04.2016 um 09:32 schrieb Alexander Graf:
> The cache line flush helpers only work properly when they get aligned
> start and end addresses. Round our flush range to cache line size. It's
> safe because we're guaranteed to flush within a single page which has the
> same cache attributes.
>
> Reported-by: Marek Vasut <marex@denx.de>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
> lib/efi_loader/efi_runtime.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
> index 22bcd08..40acec0 100644
> --- a/lib/efi_loader/efi_runtime.c
> +++ b/lib/efi_loader/efi_runtime.c
> @@ -194,7 +194,8 @@ void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map)
> #endif
>
> *p = newaddr;
> - flush_dcache_range((ulong)p, (ulong)&p[1]);
> + flush_dcache_range((ulong)p & ~(CONFIG_SYS_CACHELINE_SIZE - 1),
> + ALIGN((ulong)&p[1], CONFIG_SYS_CACHELINE_SIZE));
dragonboard410c_defconfig fails to build with this due to undefined
CONFIG_SYS_CACHELINE_SIZE. Do we need to #ifdef here or is the
dragonboard410c at fault for not defining it?
jetson-tk1_defconfig for instance builds fine with this patch.
Regards,
Andreas
> }
>
> #ifndef IS_RELA
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH] efi_loader: Always flush in cache line size granularity
2016-04-10 22:24 ` Andreas Färber
@ 2016-04-10 22:31 ` Alexander Graf
2016-04-10 22:36 ` Marek Vasut
0 siblings, 1 reply; 9+ messages in thread
From: Alexander Graf @ 2016-04-10 22:31 UTC (permalink / raw)
To: u-boot
On 11.04.16 00:24, Andreas F?rber wrote:
> Am 04.04.2016 um 09:32 schrieb Alexander Graf:
>> The cache line flush helpers only work properly when they get aligned
>> start and end addresses. Round our flush range to cache line size. It's
>> safe because we're guaranteed to flush within a single page which has the
>> same cache attributes.
>>
>> Reported-by: Marek Vasut <marex@denx.de>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>> lib/efi_loader/efi_runtime.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
>> index 22bcd08..40acec0 100644
>> --- a/lib/efi_loader/efi_runtime.c
>> +++ b/lib/efi_loader/efi_runtime.c
>> @@ -194,7 +194,8 @@ void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map)
>> #endif
>>
>> *p = newaddr;
>> - flush_dcache_range((ulong)p, (ulong)&p[1]);
>> + flush_dcache_range((ulong)p & ~(CONFIG_SYS_CACHELINE_SIZE - 1),
>> + ALIGN((ulong)&p[1], CONFIG_SYS_CACHELINE_SIZE));
>
> dragonboard410c_defconfig fails to build with this due to undefined
> CONFIG_SYS_CACHELINE_SIZE. Do we need to #ifdef here or is the
> dragonboard410c at fault for not defining it?
Some USB drivers use it unconditionally, but I guess the dragonboard401c
doesn't enable USB.
Let me come up with a more compatible version.
Alex
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH] efi_loader: Always flush in cache line size granularity
2016-04-10 22:31 ` Alexander Graf
@ 2016-04-10 22:36 ` Marek Vasut
2016-04-12 0:39 ` Tom Rini
0 siblings, 1 reply; 9+ messages in thread
From: Marek Vasut @ 2016-04-10 22:36 UTC (permalink / raw)
To: u-boot
On 04/11/2016 12:31 AM, Alexander Graf wrote:
>
>
> On 11.04.16 00:24, Andreas F?rber wrote:
>> Am 04.04.2016 um 09:32 schrieb Alexander Graf:
>>> The cache line flush helpers only work properly when they get aligned
>>> start and end addresses. Round our flush range to cache line size. It's
>>> safe because we're guaranteed to flush within a single page which has the
>>> same cache attributes.
>>>
>>> Reported-by: Marek Vasut <marex@denx.de>
>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>> ---
>>> lib/efi_loader/efi_runtime.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
>>> index 22bcd08..40acec0 100644
>>> --- a/lib/efi_loader/efi_runtime.c
>>> +++ b/lib/efi_loader/efi_runtime.c
>>> @@ -194,7 +194,8 @@ void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map)
>>> #endif
>>>
>>> *p = newaddr;
>>> - flush_dcache_range((ulong)p, (ulong)&p[1]);
>>> + flush_dcache_range((ulong)p & ~(CONFIG_SYS_CACHELINE_SIZE - 1),
>>> + ALIGN((ulong)&p[1], CONFIG_SYS_CACHELINE_SIZE));
>>
>> dragonboard410c_defconfig fails to build with this due to undefined
>> CONFIG_SYS_CACHELINE_SIZE. Do we need to #ifdef here or is the
>> dragonboard410c at fault for not defining it?
>
> Some USB drivers use it unconditionally, but I guess the dragonboard401c
> doesn't enable USB.
>
> Let me come up with a more compatible version.
This version is fine, dragonboard needs fixing. Every single board must
define CONFIG_SYS_CACHELINE_SIZE .
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH] efi_loader: Always flush in cache line size granularity
2016-04-10 22:36 ` Marek Vasut
@ 2016-04-12 0:39 ` Tom Rini
2016-04-12 0:59 ` Marek Vasut
0 siblings, 1 reply; 9+ messages in thread
From: Tom Rini @ 2016-04-12 0:39 UTC (permalink / raw)
To: u-boot
On Mon, Apr 11, 2016 at 12:36:33AM +0200, Marek Vasut wrote:
> On 04/11/2016 12:31 AM, Alexander Graf wrote:
> >
> >
> > On 11.04.16 00:24, Andreas F?rber wrote:
> >> Am 04.04.2016 um 09:32 schrieb Alexander Graf:
> >>> The cache line flush helpers only work properly when they get aligned
> >>> start and end addresses. Round our flush range to cache line size. It's
> >>> safe because we're guaranteed to flush within a single page which has the
> >>> same cache attributes.
> >>>
> >>> Reported-by: Marek Vasut <marex@denx.de>
> >>> Signed-off-by: Alexander Graf <agraf@suse.de>
> >>> ---
> >>> lib/efi_loader/efi_runtime.c | 3 ++-
> >>> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
> >>> index 22bcd08..40acec0 100644
> >>> --- a/lib/efi_loader/efi_runtime.c
> >>> +++ b/lib/efi_loader/efi_runtime.c
> >>> @@ -194,7 +194,8 @@ void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map)
> >>> #endif
> >>>
> >>> *p = newaddr;
> >>> - flush_dcache_range((ulong)p, (ulong)&p[1]);
> >>> + flush_dcache_range((ulong)p & ~(CONFIG_SYS_CACHELINE_SIZE - 1),
> >>> + ALIGN((ulong)&p[1], CONFIG_SYS_CACHELINE_SIZE));
> >>
> >> dragonboard410c_defconfig fails to build with this due to undefined
> >> CONFIG_SYS_CACHELINE_SIZE. Do we need to #ifdef here or is the
> >> dragonboard410c at fault for not defining it?
> >
> > Some USB drivers use it unconditionally, but I guess the dragonboard401c
> > doesn't enable USB.
> >
> > Let me come up with a more compatible version.
>
> This version is fine, dragonboard needs fixing. Every single board must
> define CONFIG_SYS_CACHELINE_SIZE .
Yeah... but this isn't quite the right time to break all of:
arm: + mx28evk integratorap_cm720t at91sam9261ek_dataflash_cs3
at91sam9261ek_dataflash_cs0 integratorap_cm946es mx28evk_nand
integratorcp_cm1136 at91sam9x5ek_nandflash
at91sam9xeek_dataflash_cs1 at91sam9260ek_dataflash_cs0
at91sam9260ek_dataflash_cs1 at91sam9g20ek_nandflash
meesc_dataflash at91sam9260ek_nandflash mx35pdk xfi3
at91sam9263ek_dataflash_cs0 at91sam9xeek_dataflash_cs0
at91sam9xeek_nandflash bg0900 highbank sansa_fuze_plus mx25pdk
mx23evk at91sam9x5ek_mmc igep0030_nand m28evk
at91sam9g20ek_dataflash_cs1 at91sam9g20ek_dataflash_cs0
integratorcp_cm946es at91sam9n12ek_nandflash
at91sam9n12ek_spiflash da850_am18xxevm devkit3250
at91rm9200ek_ram picosam9g45 at91sam9n12ek_mmc
integratorcp_cm926ejs integratorap_cm920t
at91sam9m10g45ek_nandflash sc_sps_1 omapl138_lcdk mx28evk_spi
integratorap_cm926ejs at91sam9rlek_mmc apx4devkit mx23_olinuxino
at91sam9x5ek_spiflash at91sam9g10ek_nandflash
at91sam9g10ek_dataflash_cs3 at91sam9g10ek_dataflash_cs0
at91sam9g20ek_2mmc_nandflash apf27 stv0991
at91sam9261ek_nandflash at91rm9200ek at91sam9g20ek_2mmc
da850evm_direct_nor at91sam9263ek_norflash meesc
at91sam9263ek_dataflash at91sam9m10g45ek_mmc armadillo-800eva
at91sam9x5ek_dataflash da850evm usb_a9263_dataflash
at91sam9rlek_nandflash mx28evk_auart_console
at91sam9263ek_norflash_boot integratorcp_cm920t
at91sam9263ek_nandflash at91sam9rlek_dataflash
(and maybe a few more, toolchain here craps out due to libgcc problems),
and we've talked about this before too I think. The right long term
answer is to borrow the logic of arch/arm/mm/Kconfig and asm/cache.h
from the kernel to get the right size, always. For the short term,
we'll just work-around it.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160411/fdcc7b78/attachment.sig>
^ permalink raw reply [flat|nested] 9+ messages in thread* [U-Boot] [PATCH] efi_loader: Always flush in cache line size granularity
2016-04-12 0:39 ` Tom Rini
@ 2016-04-12 0:59 ` Marek Vasut
0 siblings, 0 replies; 9+ messages in thread
From: Marek Vasut @ 2016-04-12 0:59 UTC (permalink / raw)
To: u-boot
On 04/12/2016 02:39 AM, Tom Rini wrote:
> On Mon, Apr 11, 2016 at 12:36:33AM +0200, Marek Vasut wrote:
>
>> On 04/11/2016 12:31 AM, Alexander Graf wrote:
>>>
>>>
>>> On 11.04.16 00:24, Andreas F?rber wrote:
>>>> Am 04.04.2016 um 09:32 schrieb Alexander Graf:
>>>>> The cache line flush helpers only work properly when they get aligned
>>>>> start and end addresses. Round our flush range to cache line size. It's
>>>>> safe because we're guaranteed to flush within a single page which has the
>>>>> same cache attributes.
>>>>>
>>>>> Reported-by: Marek Vasut <marex@denx.de>
>>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>>> ---
>>>>> lib/efi_loader/efi_runtime.c | 3 ++-
>>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
>>>>> index 22bcd08..40acec0 100644
>>>>> --- a/lib/efi_loader/efi_runtime.c
>>>>> +++ b/lib/efi_loader/efi_runtime.c
>>>>> @@ -194,7 +194,8 @@ void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map)
>>>>> #endif
>>>>>
>>>>> *p = newaddr;
>>>>> - flush_dcache_range((ulong)p, (ulong)&p[1]);
>>>>> + flush_dcache_range((ulong)p & ~(CONFIG_SYS_CACHELINE_SIZE - 1),
>>>>> + ALIGN((ulong)&p[1], CONFIG_SYS_CACHELINE_SIZE));
>>>>
>>>> dragonboard410c_defconfig fails to build with this due to undefined
>>>> CONFIG_SYS_CACHELINE_SIZE. Do we need to #ifdef here or is the
>>>> dragonboard410c at fault for not defining it?
>>>
>>> Some USB drivers use it unconditionally, but I guess the dragonboard401c
>>> doesn't enable USB.
>>>
>>> Let me come up with a more compatible version.
>>
>> This version is fine, dragonboard needs fixing. Every single board must
>> define CONFIG_SYS_CACHELINE_SIZE .
>
> Yeah... but this isn't quite the right time to break all of:
> arm: + mx28evk integratorap_cm720t at91sam9261ek_dataflash_cs3
> at91sam9261ek_dataflash_cs0 integratorap_cm946es mx28evk_nand
> integratorcp_cm1136 at91sam9x5ek_nandflash
> at91sam9xeek_dataflash_cs1 at91sam9260ek_dataflash_cs0
> at91sam9260ek_dataflash_cs1 at91sam9g20ek_nandflash
> meesc_dataflash at91sam9260ek_nandflash mx35pdk xfi3
> at91sam9263ek_dataflash_cs0 at91sam9xeek_dataflash_cs0
> at91sam9xeek_nandflash bg0900 highbank sansa_fuze_plus mx25pdk
> mx23evk at91sam9x5ek_mmc igep0030_nand m28evk
> at91sam9g20ek_dataflash_cs1 at91sam9g20ek_dataflash_cs0
> integratorcp_cm946es at91sam9n12ek_nandflash
> at91sam9n12ek_spiflash da850_am18xxevm devkit3250
> at91rm9200ek_ram picosam9g45 at91sam9n12ek_mmc
> integratorcp_cm926ejs integratorap_cm920t
> at91sam9m10g45ek_nandflash sc_sps_1 omapl138_lcdk mx28evk_spi
> integratorap_cm926ejs at91sam9rlek_mmc apx4devkit mx23_olinuxino
> at91sam9x5ek_spiflash at91sam9g10ek_nandflash
> at91sam9g10ek_dataflash_cs3 at91sam9g10ek_dataflash_cs0
> at91sam9g20ek_2mmc_nandflash apf27 stv0991
> at91sam9261ek_nandflash at91rm9200ek at91sam9g20ek_2mmc
> da850evm_direct_nor at91sam9263ek_norflash meesc
> at91sam9263ek_dataflash at91sam9m10g45ek_mmc armadillo-800eva
> at91sam9x5ek_dataflash da850evm usb_a9263_dataflash
> at91sam9rlek_nandflash mx28evk_auart_console
> at91sam9263ek_norflash_boot integratorcp_cm920t
> at91sam9263ek_nandflash at91sam9rlek_dataflash
> (and maybe a few more, toolchain here craps out due to libgcc problems),
Ha, libgcc ... :-(
> and we've talked about this before too I think. The right long term
> answer is to borrow the logic of arch/arm/mm/Kconfig and asm/cache.h
> from the kernel to get the right size, always. For the short term,
> we'll just work-around it.
I thought we fixed these cache issues already, but that was on v7 only,
dang. Now it is v4/v5 which broke.
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH] efi_loader: Always flush in cache line size granularity
@ 2016-04-11 21:20 Alexander Graf
2016-04-13 12:53 ` Andreas Färber
0 siblings, 1 reply; 9+ messages in thread
From: Alexander Graf @ 2016-04-11 21:20 UTC (permalink / raw)
To: u-boot
The cache line flush helpers only work properly when they get aligned
start and end addresses. Round our flush range to cache line size. It's
safe because we're guaranteed to flush within a single page which has the
same cache attributes.
Reported-by: Marek Vasut <marex@denx.de>
Signed-off-by: Alexander Graf <agraf@suse.de>
---
v1 -> v2:
- Fix compilation on systems without CONFIG_SYS_CACHELINE_SIZE
diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
index 22bcd08..3ee27ca 100644
--- a/lib/efi_loader/efi_runtime.c
+++ b/lib/efi_loader/efi_runtime.c
@@ -20,6 +20,13 @@ static efi_status_t EFI_RUNTIME_TEXT EFIAPI efi_unimplemented(void);
static efi_status_t EFI_RUNTIME_TEXT EFIAPI efi_device_error(void);
static efi_status_t EFI_RUNTIME_TEXT EFIAPI efi_invalid_parameter(void);
+#ifdef CONFIG_SYS_CACHELINE_SIZE
+#define EFI_CACHELINE_SIZE CONFIG_SYS_CACHELINE_SIZE
+#else
+/* Just use the greatest cache flush alignment requirement I'm aware of */
+#define EFI_CACHELINE_SIZE 128
+#endif
+
#if defined(CONFIG_ARM64)
#define R_RELATIVE 1027
#define R_MASK 0xffffffffULL
@@ -194,7 +201,8 @@ void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map)
#endif
*p = newaddr;
- flush_dcache_range((ulong)p, (ulong)&p[1]);
+ flush_dcache_range((ulong)p & ~(EFI_CACHELINE_SIZE - 1),
+ ALIGN((ulong)&p[1], EFI_CACHELINE_SIZE));
}
#ifndef IS_RELA
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH] efi_loader: Always flush in cache line size granularity
2016-04-11 21:20 Alexander Graf
@ 2016-04-13 12:53 ` Andreas Färber
0 siblings, 0 replies; 9+ messages in thread
From: Andreas Färber @ 2016-04-13 12:53 UTC (permalink / raw)
To: u-boot
Am 11.04.2016 um 23:20 schrieb Alexander Graf:
> The cache line flush helpers only work properly when they get aligned
> start and end addresses. Round our flush range to cache line size. It's
> safe because we're guaranteed to flush within a single page which has the
> same cache attributes.
>
> Reported-by: Marek Vasut <marex@denx.de>
> Signed-off-by: Alexander Graf <agraf@suse.de>
Reviewed-by: Andreas F?rber <afaerber@suse.de>
Tested-by: Andreas F?rber <afaerber@suse.de>
Thanks,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-04-13 12:53 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-04 7:32 [U-Boot] [PATCH] efi_loader: Always flush in cache line size granularity Alexander Graf
2016-04-04 7:56 ` Marek Vasut
2016-04-10 22:24 ` Andreas Färber
2016-04-10 22:31 ` Alexander Graf
2016-04-10 22:36 ` Marek Vasut
2016-04-12 0:39 ` Tom Rini
2016-04-12 0:59 ` Marek Vasut
-- strict thread matches above, loose matches on Subject: below --
2016-04-11 21:20 Alexander Graf
2016-04-13 12:53 ` Andreas Färber
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox