Received-SPF: pass (google.com: domain of linux-kernel+bounces-12808-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Thu, 28 Dec 2023 18:16:25 +0100
From: Borislav Petkov <bp@alien8.de>
To: Yazen Ghannam <yazen.ghannam@amd.com>
Cc: linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org,
	avadhut.naik@amd.com, tony.luck@intel.com, john.allen@amd.com,
	william.roche@oracle.com, muralidhara.mk@amd.com
Subject: Re: [PATCH v4 1/3] RAS: Introduce AMD Address Translation Library
Message-ID: <20231228171625.GAZY2taVm+C0rM7x1M@fat_crate.local>
References: <20231218190406.27479-1-yazen.ghannam@amd.com>
 <20231218190406.27479-2-yazen.ghannam@amd.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20231218190406.27479-2-yazen.ghannam@amd.com>

On Mon, Dec 18, 2023 at 01:04:04PM -0600, Yazen Ghannam wrote:
> diff --git a/drivers/ras/amd/atl/dehash.c b/drivers/ras/amd/atl/dehash.c
> new file mode 100644
> index 000000000000..51721094dd06
> --- /dev/null
> +++ b/drivers/ras/amd/atl/dehash.c
> @@ -0,0 +1,416 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * AMD Address Translation Library
> + *
> + * dehash.c : Functions to account for hashing bits
> + *
> + * Copyright (c) 2023, Advanced Micro Devices, Inc.
> + * All Rights Reserved.
> + *
> + * Author: Yazen Ghannam <Yazen.Ghannam@amd.com>
> + */
> +
> +#include "internal.h"
> +
> +static inline bool valid_map_bits(struct addr_ctx *ctx, u8 bit1, u8 bit2,
> +				  u8 num_intlv_dies, u8 num_intlv_sockets)
> +{
> +	if (!(ctx->map.intlv_bit_pos == bit1 || ctx->map.intlv_bit_pos == bit2)) {
> +		pr_debug("Invalid interleave bit: %u", ctx->map.intlv_bit_pos);
> +		return false;
> +	}
> +
> +	if (ctx->map.num_intlv_dies > num_intlv_dies) {
> +		pr_debug("Invalid number of interleave dies: %u", ctx->map.num_intlv_dies);
> +		return false;
> +	}
> +
> +	if (ctx->map.num_intlv_sockets > num_intlv_sockets) {
> +		pr_debug("Invalid number of interleave sockets: %u", ctx->map.num_intlv_sockets);
> +		return false;
> +	}
> +
> +	return true;
> +}

Ontop:

diff --git a/drivers/ras/amd/atl/dehash.c b/drivers/ras/amd/atl/dehash.c
index 51721094dd06..6f414926e6fe 100644
--- a/drivers/ras/amd/atl/dehash.c
+++ b/drivers/ras/amd/atl/dehash.c
@@ -12,7 +12,14 @@
 
 #include "internal.h"
 
-static inline bool valid_map_bits(struct addr_ctx *ctx, u8 bit1, u8 bit2,
+/*
+ * Verify the interleave bits are correct in the different interleaving
+ * settings.
+ *
+ * If @num_intlv_dies and/or @num_intlv_sockets are 1, it means the
+ * respective interleaving is disabled.
+ */
+static inline bool map_bits_valid(struct addr_ctx *ctx, u8 bit1, u8 bit2,
 				  u8 num_intlv_dies, u8 num_intlv_sockets)
 {
 	if (!(ctx->map.intlv_bit_pos == bit1 || ctx->map.intlv_bit_pos == bit2)) {
@@ -37,11 +44,7 @@ static int df2_dehash_addr(struct addr_ctx *ctx)
 {
 	u8 hashed_bit, intlv_bit, intlv_bit_pos;
 
-	/*
-	 * Assert that interleave bit is 8 or 9 and that die and socket
-	 * interleaving are disabled.
-	 */
-	if (!valid_map_bits(ctx, 8, 9, 1, 1))
+	if (!map_bits_valid(ctx, 8, 9, 1, 1))
 		return -EINVAL;
 
 	intlv_bit_pos = ctx->map.intlv_bit_pos;
@@ -64,11 +67,7 @@ static int df3_dehash_addr(struct addr_ctx *ctx)
 	bool hash_ctl_64k, hash_ctl_2M, hash_ctl_1G;
 	u8 hashed_bit, intlv_bit, intlv_bit_pos;
 
-	/*
-	 * Assert that interleave bit is 8 or 9 and that die and socket
-	 * interleaving are disabled.
-	 */
-	if (!valid_map_bits(ctx, 8, 9, 1, 1))
+	if (!map_bits_valid(ctx, 8, 9, 1, 1))
 		return -EINVAL;
 
 	hash_ctl_64k = FIELD_GET(DF3_HASH_CTL_64K, ctx->map.ctl);
@@ -172,11 +171,7 @@ static int df4_dehash_addr(struct addr_ctx *ctx)
 	bool hash_ctl_64k, hash_ctl_2M, hash_ctl_1G;
 	u8 hashed_bit, intlv_bit;
 
-	/*
-	 * Assert that interleave bit is 8, die interleaving is disabled,
-	 * and no more than 2 sockets are interleaved.
-	 */
-	if (!valid_map_bits(ctx, 8, 8, 1, 2))
+	if (!map_bits_valid(ctx, 8, 8, 1, 2))
 		return -EINVAL;
 
 	hash_ctl_64k = FIELD_GET(DF4_HASH_CTL_64K, ctx->map.ctl);
@@ -252,11 +247,7 @@ static int df4p5_dehash_addr(struct addr_ctx *ctx)
 	u8 hashed_bit, intlv_bit;
 	u64 rehash_vector;
 
-	/*
-	 * Assert that interleave bit is 8, die interleaving is disabled,
-	 * and no more than 2 sockets are interleaved.
-	 */
-	if (!valid_map_bits(ctx, 8, 8, 1, 2))
+	if (!map_bits_valid(ctx, 8, 8, 1, 2))
 		return -EINVAL;
 
 	hash_ctl_64k = FIELD_GET(DF4_HASH_CTL_64K, ctx->map.ctl);

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette