efi_loader: Always flush in cache line size granularity

PATCH New Alexander Graf 4 participants 7 messages Apr 4, 2016 – Apr 12, 2016

Share & tools

Alexander Graf

author

2016-04-04 07:32 UTC #0 lore raw

PATCH 1/1 efi_loader: Always flush in cache line size granularity

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 <[email protected]>

Signed-off-by Alexander Graf <[email protected]>

---
 lib/efi_loader/efi_runtime.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

lib/efi_loader/efi_runtime.c +2 -1

index 22bcd08..40acec0 100644
@@ -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

Signature · 1 line

1.8.5.6

Marek Vasut

2016-04-04 07:56 UTC #1 lore raw

PATCH 1/1 efi_loader: Always flush in cache line size granularity

Quoted 7 lines · 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 <[email protected]>
Signed-off-by Alexander Graf <[email protected]>

Thanks!

Signature · 2 lines

Best regards,
Marek Vasut

Andreas Färber

2016-04-10 22:24 UTC #2 lore raw

PATCH 1/1 efi_loader: Always flush in cache line size granularity

In reply to Alexander Graf

Am 04.04.2016 um 09:32 schrieb Alexander Graf:

Quoted 16 lines

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 <[email protected]>
Signed-off-by Alexander Graf <[email protected]>
---
 lib/efi_loader/efi_runtime.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
lib/efi_loader/efi_runtime.c +0 -0
index 22bcd08..40acec0 100644

lib/efi_loader/efi_runtime.c void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map) · line 198

Quoted 6 lines

lib/efi_loader/efi_runtime.c +2 -1

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

Quoted 3 lines

lib/efi_loader/efi_runtime.c +0 -0
 	}
 
 #ifndef IS_RELA

Signature · 3 lines

SUSE Linux GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)

Alexander Graf

author

2016-04-10 22:31 UTC #3 lore raw

PATCH 1/1 efi_loader: Always flush in cache line size granularity

Quoted 19 lines · Andreas F?rber

Am 04.04.2016 um 09:32 schrieb Alexander Graf:
Quoted 18 lines
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 <[email protected]>
Signed-off-by Alexander Graf <[email protected]>
---
 lib/efi_loader/efi_runtime.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
lib/efi_loader/efi_runtime.c +0 -0
index 22bcd08..40acec0 100644
@@ -194,7 +194,8 @@ void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map)
 #endif
 

Quoted 8 lines

lib/efi_loader/efi_runtime.c void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map) · line 198
Quoted 4 lines
lib/efi_loader/efi_runtime.c +2 -1
 		*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

Marek Vasut

2016-04-10 22:36 UTC #4 lore raw

PATCH 1/1 efi_loader: Always flush in cache line size granularity

Quoted 31 lines · Alexander Graf

Quoted 19 lines · Andreas F?rber
Am 04.04.2016 um 09:32 schrieb Alexander Graf:
Quoted 18 lines
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 <[email protected]>
Signed-off-by Alexander Graf <[email protected]>
---
 lib/efi_loader/efi_runtime.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
lib/efi_loader/efi_runtime.c +0 -0
index 22bcd08..40acec0 100644
@@ -194,7 +194,8 @@ void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map)
 #endif
 
Quoted 8 lines
lib/efi_loader/efi_runtime.c void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map) · line 198
Quoted 4 lines
lib/efi_loader/efi_runtime.c +2 -1
 		*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?

Quoted 4 lines

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 .

Signature · 2 lines

Best regards,
Marek Vasut

Tom Rini

2016-04-12 00:39 UTC #5 lore raw

PATCH 1/1 efi_loader: Always flush in cache line size granularity

Quoted 32 lines · Marek Vasut

Quoted 31 lines · Alexander Graf
Quoted 27 lines · Andreas F?rber
Am 04.04.2016 um 09:32 schrieb Alexander Graf:
Quoted 16 lines
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 <[email protected]>
Signed-off-by Alexander Graf <[email protected]>
---
 lib/efi_loader/efi_runtime.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
lib/efi_loader/efi_runtime.c +0 -0
index 22bcd08..40acec0 100644
lib/efi_loader/efi_runtime.c void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map) · line 198
Quoted 6 lines
lib/efi_loader/efi_runtime.c +2 -1
@@ -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?

Quoted 7 lines

Quoted 4 lines
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.

Signature · 8 lines

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>

Marek Vasut

2016-04-12 00:59 UTC #6 lore raw

PATCH 1/1 efi_loader: Always flush in cache line size granularity

Quoted 69 lines · Tom Rini

Quoted 32 lines · Marek Vasut
Quoted 31 lines · Alexander Graf
Quoted 27 lines · Andreas F?rber
Am 04.04.2016 um 09:32 schrieb Alexander Graf:
Quoted 16 lines
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 <[email protected]>
Signed-off-by Alexander Graf <[email protected]>
---
 lib/efi_loader/efi_runtime.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
lib/efi_loader/efi_runtime.c +0 -0
index 22bcd08..40acec0 100644
lib/efi_loader/efi_runtime.c void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map) · line 198
Quoted 6 lines
lib/efi_loader/efi_runtime.c +2 -1
@@ -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?
Quoted 7 lines
Quoted 4 lines
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  

Quoted 1 line

(and maybe a few more, toolchain here craps out due to libgcc problems),

Ha, libgcc ... :-(

Quoted 4 lines

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.

Signature · 2 lines

Best regards,
Marek Vasut