Re: [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v1 6/22]: MdePkg/BaseCacheMaintenanceLib: RISC-V cache maintenance implementation.

["Leif Lindholm" <leif.lindholm@linaro.org>]

On Wed, Sep 04, 2019 at 06:43:01PM +0800, Abner Chang wrote:
> Implement RISC-V cache maintenance functions in BaseCacheMaintenanceLib.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Abner Chang <abner.chang@hpe.com>
> ---
>  .../BaseCacheMaintenanceLib.inf                    |   4 +
>  .../Library/BaseCacheMaintenanceLib/RiscVCache.c   | 242 +++++++++++++++++++++

I would really prefer to see the header added wit this patch rather
than BaseLib.

>  2 files changed, 246 insertions(+)
>  create mode 100644 MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c
> 
> diff --git a/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf b/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf
> index ec7feec..d9bfa04 100644
> --- a/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf
> +++ b/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf
> @@ -6,6 +6,7 @@
>  #
>  #  Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>
>  #  Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
> +#  Copyright (c) 2016, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -41,6 +42,9 @@
>  [Sources.AARCH64]
>    ArmCache.c
>  
> +[Sources.RISCV64]
> +  RiscVCache.c
> +
>  [Packages]
>    MdePkg/MdePkg.dec
>  
> diff --git a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c
> new file mode 100644
> index 0000000..2d376a4
> --- /dev/null
> +++ b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c
> @@ -0,0 +1,242 @@
> +/** @file
> +  RISC-V specific functionality for cache.
> +
> +  Copyright (c) 2016, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
> +
> +  This program and the accompanying materials
> +  are licensed and made available under the terms and conditions of the BSD License
> +  which accompanies this distribution. The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +**/
> +
> +#include <Base.h>
> +#include <Library/BaseLib.h>
> +#include <Library/DebugLib.h>
> +
> +/**
> +  Invalidates the entire instruction cache in cache coherency domain of the
> +  calling CPU.
> +
> +**/
> +VOID
> +EFIAPI
> +InvalidateInstructionCache (
> +  VOID
> +  )
> +{
> +  RiscVInvdInstCacheAsm ();
> +}
> +
> +/**
> +  Invalidates a range of instruction cache lines in the cache coherency domain
> +  of the calling CPU.
> +
> +  Invalidates the instruction cache lines specified by Address and Length. If
> +  Address is not aligned on a cache line boundary, then entire instruction
> +  cache line containing Address is invalidated. If Address + Length is not
> +  aligned on a cache line boundary, then the entire instruction cache line
> +  containing Address + Length -1 is invalidated. This function may choose to
> +  invalidate the entire instruction cache if that is more efficient than
> +  invalidating the specified range. If Length is 0, then no instruction cache
> +  lines are invalidated. Address is returned.
> +
> +  If Length is greater than (MAX_ADDRESS - Address + 1), then ASSERT().
> +
> +  @param  Address The base address of the instruction cache lines to
> +                  invalidate. If the CPU is in a physical addressing mode, then
> +                  Address is a physical address. If the CPU is in a virtual
> +                  addressing mode, then Address is a virtual address.
> +
> +  @param  Length  The number of bytes to invalidate from the instruction cache.
> +
> +  @return Address.
> +
> +**/
> +VOID *
> +EFIAPI
> +InvalidateInstructionCacheRange (
> +  IN VOID *Address,
> +  IN UINTN Length
> +  )
> +{
> +  DEBUG((DEBUG_ERROR, "%a:RISC-V unsupported function.\n", __FUNCTION__));
> +  //ASSERT(FALSE);

I'm OK with all of the unimplemented functions asserting false, but
not with them all having a commented out ASSERT in them. Please either
uncomment all of them or delete all of them.

/
    Leif

> +  return Address;
> +}
> +
> +/**
> +  Writes back and invalidates the entire data cache in cache coherency domain
> +  of the calling CPU.
> +
> +  Writes back and invalidates the entire data cache in cache coherency domain
> +  of the calling CPU. This function guarantees that all dirty cache lines are
> +  written back to system memory, and also invalidates all the data cache lines
> +  in the cache coherency domain of the calling CPU.
> +
> +**/
> +VOID
> +EFIAPI
> +WriteBackInvalidateDataCache (
> +  VOID
> +  )
> +{
> +  DEBUG((DEBUG_ERROR, "%a:RISC-V unsupported function.\n", __FUNCTION__));
> +  //ASSERT(FALSE);
> +}
> +
> +/**
> +  Writes back and invalidates a range of data cache lines in the cache
> +  coherency domain of the calling CPU.
> +
> +  Writes back and invalidates the data cache lines specified by Address and
> +  Length. If Address is not aligned on a cache line boundary, then entire data
> +  cache line containing Address is written back and invalidated. If Address +
> +  Length is not aligned on a cache line boundary, then the entire data cache
> +  line containing Address + Length -1 is written back and invalidated. This
> +  function may choose to write back and invalidate the entire data cache if
> +  that is more efficient than writing back and invalidating the specified
> +  range. If Length is 0, then no data cache lines are written back and
> +  invalidated. Address is returned.
> +
> +  If Length is greater than (MAX_ADDRESS - Address + 1), then ASSERT().
> +
> +  @param  Address The base address of the data cache lines to write back and
> +                  invalidate. If the CPU is in a physical addressing mode, then
> +                  Address is a physical address. If the CPU is in a virtual
> +                  addressing mode, then Address is a virtual address.
> +  @param  Length  The number of bytes to write back and invalidate from the
> +                  data cache.
> +
> +  @return Address of cache invalidation.
> +
> +**/
> +VOID *
> +EFIAPI
> +WriteBackInvalidateDataCacheRange (
> +  IN      VOID                      *Address,
> +  IN      UINTN                     Length
> +  )
> +{
> +  DEBUG((DEBUG_ERROR, "%a:RISC-V unsupportted function.\n", __FUNCTION__));
> +  //ASSERT(FALSE);
> +  return Address;
> +}
> +
> +/**
> +  Writes back the entire data cache in cache coherency domain of the calling
> +  CPU.
> +
> +  Writes back the entire data cache in cache coherency domain of the calling
> +  CPU. This function guarantees that all dirty cache lines are written back to
> +  system memory. This function may also invalidate all the data cache lines in
> +  the cache coherency domain of the calling CPU.
> +
> +**/
> +VOID
> +EFIAPI
> +WriteBackDataCache (
> +  VOID
> +  )
> +{
> +  DEBUG((DEBUG_ERROR, "%a:RISC-V unsupportted function.\n", __FUNCTION__));
> +  //ASSERT(FALSE);
> +}
> +
> +/**
> +  Writes back a range of data cache lines in the cache coherency domain of the
> +  calling CPU.
> +
> +  Writes back the data cache lines specified by Address and Length. If Address
> +  is not aligned on a cache line boundary, then entire data cache line
> +  containing Address is written back. If Address + Length is not aligned on a
> +  cache line boundary, then the entire data cache line containing Address +
> +  Length -1 is written back. This function may choose to write back the entire
> +  data cache if that is more efficient than writing back the specified range.
> +  If Length is 0, then no data cache lines are written back. This function may
> +  also invalidate all the data cache lines in the specified range of the cache
> +  coherency domain of the calling CPU. Address is returned.
> +
> +  If Length is greater than (MAX_ADDRESS - Address + 1), then ASSERT().
> +
> +  @param  Address The base address of the data cache lines to write back. If
> +                  the CPU is in a physical addressing mode, then Address is a
> +                  physical address. If the CPU is in a virtual addressing
> +                  mode, then Address is a virtual address.
> +  @param  Length  The number of bytes to write back from the data cache.
> +
> +  @return Address of cache written in main memory.
> +
> +**/
> +VOID *
> +EFIAPI
> +WriteBackDataCacheRange (
> +  IN      VOID                      *Address,
> +  IN      UINTN                     Length
> +  )
> +{
> +  DEBUG((DEBUG_ERROR, "%a:RISC-V unsupportted function.\n", __FUNCTION__));
> +  //ASSERT(FALSE);
> +  return Address;
> +}
> +
> +/**
> +  Invalidates the entire data cache in cache coherency domain of the calling
> +  CPU.
> +
> +  Invalidates the entire data cache in cache coherency domain of the calling
> +  CPU. This function must be used with care because dirty cache lines are not
> +  written back to system memory. It is typically used for cache diagnostics. If
> +  the CPU does not support invalidation of the entire data cache, then a write
> +  back and invalidate operation should be performed on the entire data cache.
> +
> +**/
> +VOID
> +EFIAPI
> +InvalidateDataCache (
> +  VOID
> +  )
> +{
> +  RiscVInvdDataCacheAsm ();
> +}
> +
> +/**
> +  Invalidates a range of data cache lines in the cache coherency domain of the
> +  calling CPU.
> +
> +  Invalidates the data cache lines specified by Address and Length. If Address
> +  is not aligned on a cache line boundary, then entire data cache line
> +  containing Address is invalidated. If Address + Length is not aligned on a
> +  cache line boundary, then the entire data cache line containing Address +
> +  Length -1 is invalidated. This function must never invalidate any cache lines
> +  outside the specified range. If Length is 0, then no data cache lines are
> +  invalidated. Address is returned. This function must be used with care
> +  because dirty cache lines are not written back to system memory. It is
> +  typically used for cache diagnostics. If the CPU does not support
> +  invalidation of a data cache range, then a write back and invalidate
> +  operation should be performed on the data cache range.
> +
> +  If Length is greater than (MAX_ADDRESS - Address + 1), then ASSERT().
> +
> +  @param  Address The base address of the data cache lines to invalidate. If
> +                  the CPU is in a physical addressing mode, then Address is a
> +                  physical address. If the CPU is in a virtual addressing mode,
> +                  then Address is a virtual address.
> +  @param  Length  The number of bytes to invalidate from the data cache.
> +
> +  @return Address.
> +
> +**/
> +VOID *
> +EFIAPI
> +InvalidateDataCacheRange (
> +  IN      VOID                      *Address,
> +  IN      UINTN                     Length
> +  )
> +{
> +  DEBUG((DEBUG_ERROR, "%a:RISC-V unsupportted function.\n", __FUNCTION__));
> +  //ASSERT(FALSE);
> +  return Address;
> +}
> -- 
> 2.7.4
> 
> 
> 
>