Notice: We have migrated to GitLab launching 2024-05-01 see here: https://gitlab.rtems.org/

Opened on 04/27/17 at 14:23:57

Closed on 06/14/17 at 13:46:21

Last modified on 04/16/20 at 23:48:10

#3007 closed defect (fixed)

ARM caching issues

Reported by: munster Owned by: joel.sherrill@…
Priority: normal Milestone: 5.1
Component: arch/arm Version: 4.11
Severity: normal Keywords: ARM, cache, L2C 310
Cc: Blocked By:
Blocking:

Description

There are two problems with the caching on ARM:

  • In cases where the buffer is not aligned to line boundary at the beginning or the end, the invalidate operation would lose modifications done on the adjacent data. This applies to both L1 and L2 caches.
  • The L2C-310 cache management operations use excessive locking. According to ​manual, the used operations (Clean Line by PA, Clean and Invalidate Line by PA, Cache Sync) are atomic and do not require locking.

I have attached the proposed patch.

Change History (25)

Changed on 04/27/17 at 14:24:20 by munster

Attachment: cache.diff​ added

comment:1 Changed on 04/27/17 at 23:44:36 by Chris Johns

Why have you changed the signature of some of the functions to be uint32_t, uint32_t from const void* , const size_t ?

comment:2 Changed on 04/28/17 at 15:13:35 by munster

Well, I was just following the Linux driver ​example. If you don't like it, I can use the previous function signature.

comment:3 in reply to:  2 Changed on 04/30/17 at 22:09:35 by Chris Johns

Replying to munster:

Well, I was just following the Linux driver ​example. If you don't like it, I can use the previous function signature.

I think it is best a change like this is as small as can be and keeping the existing function signatures helps do this so please revert that part of the change and post a new patch. Thanks.

comment:4 Changed on 05/02/17 at 05:30:52 by Sebastian Huber

Please don't change the function signatures. Please generate one patch for each issue.

Its up to the user to ensure that a cache line invalidation is safe. The cache manager should not take care of this.

Changed on 05/08/17 at 09:58:29 by munster

Attachment: cache_invalidate.diff​ added

Changed on 05/08/17 at 09:58:44 by munster

Attachment: cache_locking.diff​ added

comment:5 in reply to:  4 ; Changed on 05/08/17 at 10:02:40 by munster

Replying to Sebastian Huber:

Please don't change the function signatures. Please generate one patch for each issue.

I have attached separate patches for these two issues.

Its up to the user to ensure that a cache line invalidation is safe. The cache manager should not take care of this.

Generally you are right. However, the user may not be aware of this particular problem.

comment:6 in reply to:  5 ; Changed on 05/09/17 at 07:58:16 by Sebastian Huber

Replying to munster:

Replying to Sebastian Huber:

Please don't change the function signatures. Please generate one patch for each issue.

I have attached separate patches for these two issues.

Could you please format the patches via "git format-patch":

​https://docs.rtems.org/branches/master/user/support/contrib.html

Please help to improve the wiki and website if something is unclear or hard to find.


Its up to the user to ensure that a cache line invalidation is safe. The cache manager should not take care of this.

Generally you are right. However, the user may not be aware of this particular problem.

The we should fix this issue with better documentation. In

​https://git.rtems.org/rtems/tree/cpukit/rtems/include/rtems/rtems/cache.h#n111

we have

/**
 * @brief Invalidates multiple data cache lines.
 *
 * The cache lines covering the area are marked as invalid.  A later read
 * access in the area will load the data from memory.
 *
 * In case the area is not aligned on cache line boundaries, then this
 * operation may destroy unrelated data.
 *
 * @param[in] addr The start address of the area to invalidate.
 * @param[in] size The size in bytes of the area to invalidate.
 */
void rtems_cache_invalidate_multiple_data_lines(
  const void *addr,
  size_t size
);

The cache manager documentation probably needs to move into the user manual. Maybe some examples help to show what can go wrong.

Last edited on 04/16/20 at 23:48:10 by Joel Sherrill (previous) (diff)

comment:7 Changed on 05/11/17 at 07:31:02 by Sebastian Huber

Milestone: 4.12 → 4.12.0

Changed on 05/11/17 at 14:25:40 by munster

Attachment: 0001-Remove-excessive-locking-from-cache-operations.patch​ added

Changed on 05/11/17 at 14:25:54 by munster

Attachment: 0002-Fix-cache-invalidate-behavior.patch​ added

comment:8 in reply to:  6 Changed on 05/11/17 at 14:27:34 by munster

Replying to Sebastian Huber:

Could you please format the patches via "git format-patch":

I have attached the patches.

comment:9 Changed on 05/12/17 at 05:23:15 by Sebastian Huber

See

​https://devel.rtems.org/wiki/Developer/Git/Users#CreatingaPatch

Please use your real name for the patch with a valid e-mail address.

comment:10 Changed on 06/08/17 at 07:46:40 by Sebastian Huber

Milestone: 4.12.0 → 4.12.1

comment:11 Changed on 06/12/17 at 07:34:17 by Sebastian Huber

Without a real name I cannot apply this patch.

Changed on 06/14/17 at 11:43:59 by munster

Attachment: v2-0001-Remove-excessive-locking-from-cache-operations.patch​ added

Changed on 06/14/17 at 11:44:10 by munster

Attachment: v2-0002-Fix-cache-invalidate-behavior.patch​ added

comment:12 in reply to:  11 Changed on 06/14/17 at 11:45:18 by munster

Replying to Sebastian Huber:

Without a real name I cannot apply this patch.

Re-attached with real name.

comment:13 Changed on 06/14/17 at 13:41:30 by Alexei Pososin <m09123874@…>

In [changeset:"fd10817a8355653043f8addfe2ef975bedad093a/rtems" fd10817/rtems]:

Remove excessive locking from cache operations.

According to manual, the used operations (Clean Line by PA, Clean and
Invalidate Line by PA, Cache Sync) are atomic and do not require
locking.

Update #3007.

comment:14 Changed on 06/14/17 at 13:46:21 by Sebastian Huber

Resolution: → fixed
Status: new → closed

It is up to the user to ensure that its safe to invalidate a certain data area.

comment:15 Changed on 06/14/17 at 13:46:35 by Sebastian Huber

Version: 4.12 → 4.11

comment:16 Changed on 07/12/17 at 22:32:43 by Chris Johns

Milestone: 4.12.1 → 4.12.0

comment:17 Changed on 10/10/17 at 06:54:12 by Sebastian Huber

Component: bsps → arch/arm

comment:18 Changed on 11/09/17 at 06:27:14 by Sebastian Huber

Milestone: 4.12.0 → 5.1

Milestone renamed

Note: See TracTickets for help on using tickets.