[edk2-devel] [PATCH 6/6] ArmPkg/ArmLib: deprecate set/way cache maintenance routines

Ard Biesheuvel posted 6 patches 5 years, 11 months ago
There is a newer version of this series
[edk2-devel] [PATCH 6/6] ArmPkg/ArmLib: deprecate set/way cache maintenance routines
Posted by Ard Biesheuvel 5 years, 11 months ago
Cache maintenance on ARMv7 systems and up should be done by virtual
address if the purpose is to manage the cached state of contents of
memory. Set/way operations are only intended to maintain the caches
themselves, e.g., to ensure that the contents of dirty cachelines
are brought to main memory before the core is powered off entirely.

UEFI on ARM is typically not involved in the latter at all, and any
cache maintenance it does is to ensure that the memory it occupies
and modifies remains in a consistent state with respect to the
caches.

So let's deprecate the set/way routines now that we have removed all
uses of it in the core code.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPkg/Include/Library/ArmLib.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
index 5a27b7c2fc27..8330339302ca 100644
--- a/ArmPkg/Include/Library/ArmLib.h
+++ b/ArmPkg/Include/Library/ArmLib.h
@@ -156,6 +156,8 @@ ArmIsMpCore (
   VOID
   );
 
+#ifndef DISABLE_NEW_DEPRECATED_INTERFACES
+
 VOID
 EFIAPI
 ArmInvalidateDataCache (
@@ -169,6 +171,8 @@ ArmCleanInvalidateDataCache (
   VOID
   );
 
+#endif
+
 VOID
 EFIAPI
 ArmCleanDataCache (
-- 
2.17.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54852): https://edk2.groups.io/g/devel/message/54852
Mute This Topic: https://groups.io/mt/71562852/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 6/6] ArmPkg/ArmLib: deprecate set/way cache maintenance routines
Posted by Leif Lindholm 5 years, 11 months ago
On Wed, Feb 26, 2020 at 11:03:53 +0100, Ard Biesheuvel wrote:
> Cache maintenance on ARMv7 systems and up should be done by virtual
> address if the purpose is to manage the cached state of contents of
> memory. Set/way operations are only intended to maintain the caches
> themselves, e.g., to ensure that the contents of dirty cachelines
> are brought to main memory before the core is powered off entirely.
> 
> UEFI on ARM is typically not involved in the latter at all, and any
> cache maintenance it does is to ensure that the memory it occupies
> and modifies remains in a consistent state with respect to the
> caches.
> 
> So let's deprecate the set/way routines now that we have removed all
> uses of it in the core code.

Does this patch simply get dropped in favour of the ASSERT variant?

/
    Leif

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmPkg/Include/Library/ArmLib.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
> index 5a27b7c2fc27..8330339302ca 100644
> --- a/ArmPkg/Include/Library/ArmLib.h
> +++ b/ArmPkg/Include/Library/ArmLib.h
> @@ -156,6 +156,8 @@ ArmIsMpCore (
>    VOID
>    );
>  
> +#ifndef DISABLE_NEW_DEPRECATED_INTERFACES
> +
>  VOID
>  EFIAPI
>  ArmInvalidateDataCache (
> @@ -169,6 +171,8 @@ ArmCleanInvalidateDataCache (
>    VOID
>    );
>  
> +#endif
> +
>  VOID
>  EFIAPI
>  ArmCleanDataCache (
> -- 
> 2.17.1
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55181): https://edk2.groups.io/g/devel/message/55181
Mute This Topic: https://groups.io/mt/71562852/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 6/6] ArmPkg/ArmLib: deprecate set/way cache maintenance routines
Posted by Ard Biesheuvel 5 years, 11 months ago
On Mon, 2 Mar 2020 at 14:13, Leif Lindholm <leif@nuviainc.com> wrote:
>
> On Wed, Feb 26, 2020 at 11:03:53 +0100, Ard Biesheuvel wrote:
> > Cache maintenance on ARMv7 systems and up should be done by virtual
> > address if the purpose is to manage the cached state of contents of
> > memory. Set/way operations are only intended to maintain the caches
> > themselves, e.g., to ensure that the contents of dirty cachelines
> > are brought to main memory before the core is powered off entirely.
> >
> > UEFI on ARM is typically not involved in the latter at all, and any
> > cache maintenance it does is to ensure that the memory it occupies
> > and modifies remains in a consistent state with respect to the
> > caches.
> >
> > So let's deprecate the set/way routines now that we have removed all
> > uses of it in the core code.
>
> Does this patch simply get dropped in favour of the ASSERT variant?
>

Yes.

But I realised that we still need a fix for CmdRunAxf in that case :-(

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55183): https://edk2.groups.io/g/devel/message/55183
Mute This Topic: https://groups.io/mt/71562852/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 6/6] ArmPkg/ArmLib: deprecate set/way cache maintenance routines
Posted by Ard Biesheuvel 5 years, 11 months ago
On Mon, 2 Mar 2020 at 14:16, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> On Mon, 2 Mar 2020 at 14:13, Leif Lindholm <leif@nuviainc.com> wrote:
> >
> > On Wed, Feb 26, 2020 at 11:03:53 +0100, Ard Biesheuvel wrote:
> > > Cache maintenance on ARMv7 systems and up should be done by virtual
> > > address if the purpose is to manage the cached state of contents of
> > > memory. Set/way operations are only intended to maintain the caches
> > > themselves, e.g., to ensure that the contents of dirty cachelines
> > > are brought to main memory before the core is powered off entirely.
> > >
> > > UEFI on ARM is typically not involved in the latter at all, and any
> > > cache maintenance it does is to ensure that the memory it occupies
> > > and modifies remains in a consistent state with respect to the
> > > caches.
> > >
> > > So let's deprecate the set/way routines now that we have removed all
> > > uses of it in the core code.
> >
> > Does this patch simply get dropped in favour of the ASSERT variant?
> >
>
> Yes.
>
> But I realised that we still need a fix for CmdRunAxf in that case :-(

As discussed on IRC, with CmdRunAxf out of the way, we can ASSERT ()
on any use of the set/way ops with the MMU/dcache enabled. In any
case, this 6/6 will simply be dropped, and superseded by my followup
series '[PATCH 0/3] ArmPkg/ArmLib: ASSERT() on misuse of set/way ops'

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55405): https://edk2.groups.io/g/devel/message/55405
Mute This Topic: https://groups.io/mt/71562852/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-