lkml.org 
[lkml]   [2024]   [Mar]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 2/4] RAS: ATL: Expand helpers for adding and removing base and hole
From
On 3/14/24 12:35, John Allen wrote:
> Data fabric 4.5 denormalization will need to frequently add and remove

More specifically, the non-power-of-2 cases will need this.

> the base and the legacy MMIO hole. Modify existing helpers to improve DF
> 4.5 denormalization flow and add helper to remove the base and hole.

Please write the what/context, why/issue, and how/fix information as
separate paragraphs even if they're just a single sentence each. I think
this helps to find the details more easily.

>
> Signed-off-by: John Allen <john.allen@amd.com>
> ---
> drivers/ras/amd/atl/core.c | 43 ++++++++++++++++++++++------------
> drivers/ras/amd/atl/internal.h | 3 +++
> 2 files changed, 31 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/ras/amd/atl/core.c b/drivers/ras/amd/atl/core.c
> index c1710d233adb..cafdfc57d929 100644
> --- a/drivers/ras/amd/atl/core.c
> +++ b/drivers/ras/amd/atl/core.c
> @@ -49,15 +49,26 @@ static bool legacy_hole_en(struct addr_ctx *ctx)
> return FIELD_GET(DF_LEGACY_MMIO_HOLE_EN, reg);
> }
>
> -static int add_legacy_hole(struct addr_ctx *ctx)
> +static u64 add_legacy_hole(struct addr_ctx *ctx, u64 addr)
> {
> if (!legacy_hole_en(ctx))
> - return 0;
> + return addr;
>
> - if (ctx->addr >= df_cfg.dram_hole_base)
> - ctx->addr += (BIT_ULL(32) - df_cfg.dram_hole_base);
> + if (addr >= df_cfg.dram_hole_base)
> + addr += (BIT_ULL(32) - df_cfg.dram_hole_base);
>
> - return 0;
> + return addr;
> +}
> +
> +static u64 remove_legacy_hole(struct addr_ctx *ctx, u64 addr)
> +{
> + if (!legacy_hole_en(ctx))
> + return addr;
> +
> + if (addr >= df_cfg.dram_hole_base)
> + addr -= (BIT_ULL(32) - df_cfg.dram_hole_base);
> +
> + return addr;
> }
>
> static u64 get_base_addr(struct addr_ctx *ctx)
> @@ -72,14 +83,16 @@ static u64 get_base_addr(struct addr_ctx *ctx)
> return base_addr << DF_DRAM_BASE_LIMIT_LSB;
> }
>
> -static int add_base_and_hole(struct addr_ctx *ctx)
> +u64 add_base_and_hole(struct addr_ctx *ctx, u64 addr)
> {
> - ctx->ret_addr += get_base_addr(ctx);
> -
> - if (add_legacy_hole(ctx))
> - return -EINVAL;
> + addr += get_base_addr(ctx);
> + return add_legacy_hole(ctx, addr);
> +}
>
> - return 0;
> +u64 remove_base_and_hole(struct addr_ctx *ctx, u64 addr)
> +{
> + addr -= get_base_addr(ctx);
> + return remove_legacy_hole(ctx, addr);

This should be the inverse of the "add" operation, I think. So remove
the legacy hole first, then remove the base address.

> }
>
> static bool late_hole_remove(struct addr_ctx *ctx)
> @@ -123,14 +136,14 @@ unsigned long norm_to_sys_addr(u8 socket_id, u8 die_id, u8 coh_st_inst_id, unsig
> if (denormalize_address(&ctx))
> return -EINVAL;
>
> - if (!late_hole_remove(&ctx) && add_base_and_hole(&ctx))
> - return -EINVAL;
> + if (!late_hole_remove(&ctx))
> + ctx.ret_addr = add_base_and_hole(&ctx, ctx.ret_addr);
>
> if (dehash_address(&ctx))
> return -EINVAL;
>
> - if (late_hole_remove(&ctx) && add_base_and_hole(&ctx))
> - return -EINVAL;
> + if (late_hole_remove(&ctx))
> + ctx.ret_addr = add_base_and_hole(&ctx, ctx.ret_addr);
>
> if (addr_over_limit(&ctx))
> return -EINVAL;
> diff --git a/drivers/ras/amd/atl/internal.h b/drivers/ras/amd/atl/internal.h
> index 1413c8ddc6c5..05b870fcb24e 100644
> --- a/drivers/ras/amd/atl/internal.h
> +++ b/drivers/ras/amd/atl/internal.h
> @@ -236,6 +236,9 @@ int dehash_address(struct addr_ctx *ctx);
> unsigned long norm_to_sys_addr(u8 socket_id, u8 die_id, u8 coh_st_inst_id, unsigned long addr);
> unsigned long convert_umc_mca_addr_to_sys_addr(struct atl_err *err);
>
> +u64 add_base_and_hole(struct addr_ctx *ctx, u64 addr);
> +u64 remove_base_and_hole(struct addr_ctx *ctx, u64 addr);

remove_base_and_hole() is only used in denormalize.c, correct? So why
not define it there as static? Other than trying to keep the code
together and symmetrical, I mean.

Thanks,
Yazen

\
 
 \ /
  Last update: 2024-05-27 15:53    [W:0.595 / U:0.020 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site