Received-SPF: pass (google.com: domain of linux-kernel+bounces-81903-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Mon, 26 Feb 2024 17:16:12 +0100
From: Borislav Petkov <bp@alien8.de>
To: Yazen Ghannam <yazen.ghannam@amd.com>
Cc: tony.luck@intel.com, linux-edac@vger.kernel.org,
	linux-kernel@vger.kernel.org, avadhut.naik@amd.com,
	john.allen@amd.com, muralidhara.mk@amd.com, sathyapriya.k@amd.com,
	naveenkrishna.chatradhi@amd.com
Subject: Re: [PATCH 2/3] RAS/AMD/FMPM: Save SPA values
Message-ID: <20240226161612.GDZdy5TH6H5VWRGWOK@fat_crate.local>
References: <20240226152941.2615007-1-yazen.ghannam@amd.com>
 <20240226152941.2615007-3-yazen.ghannam@amd.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20240226152941.2615007-3-yazen.ghannam@amd.com>

On Mon, Feb 26, 2024 at 09:29:40AM -0600, Yazen Ghannam wrote:
> @@ -111,6 +111,9 @@ struct fru_rec {
>   */
>  static struct fru_rec **fru_records;
>  
> +#define INVALID_SPA	~0ULL
> +static u64 *sys_addrs;

Let's do:

/* system physical addresses array */
static u64 *spa_addrs;

so that it is self-documenting.

> +
>  #define CPER_CREATOR_FMP						\
>  	GUID_INIT(0xcd5c2993, 0xf4b2, 0x41b2, 0xb5, 0xd4, 0xf9, 0xc3,	\
>  		  0xa0, 0x33, 0x08, 0x75)
> @@ -140,6 +143,9 @@ static unsigned int max_nr_fru;
>  /* Total length of record including headers and list of descriptor entries. */
>  static size_t max_rec_len;
>  
> +/* Total number of entries for the entire system. */
> +static unsigned int sys_nr_entries;

sys_ things are always related to syscalls etc. Since it is a static
var, just call it:

	/* Number of SPA entries */
	nr_entries

or so.

I was gonna say "nr_err_records" but we're calling them entries so...

>  /*
>   * Protect the local records cache in fru_records and prevent concurrent
>   * writes to storage. This is only needed after init once notifier block
> @@ -269,6 +275,40 @@ static bool rec_has_fpd(struct fru_rec *rec, struct cper_fru_poison_desc *fpd)
>  	return false;
>  }
>  
> +static void save_spa(struct fru_rec *rec, unsigned int entry,
> +		     u64 addr, u64 id, unsigned int cpu)
> +{
> +	unsigned int i, fru_idx, sys_entry;
> +	unsigned long sys_addr;
> +	struct atl_err a_err;
> +
> +	memset(&a_err, 0, sizeof(struct atl_err));
> +
> +	a_err.addr = addr;
> +	a_err.ipid = id;
> +	a_err.cpu  = cpu;
> +
> +	sys_addr = amd_convert_umc_mca_addr_to_sys_addr(&a_err);
> +	if (IS_ERR_VALUE(sys_addr)) {
> +		pr_debug("Failed to get system address\n");
> +		return;
> +	}
> +
> +	for (i = 0; i < sys_nr_entries; i += max_nr_entries) {
> +		fru_idx = i / max_nr_entries;
> +		if (fru_records[fru_idx] != rec)
> +			continue;
> +
> +		sys_entry = i + entry;
> +		if (sys_entry < sys_nr_entries) {
> +			sys_addrs[sys_entry] = sys_addr;
> +			pr_debug("fru_idx: %u, entry: %u, sys_entry: %u, sys_addr: 0x%016llx\n",
> +				 fru_idx, entry, sys_entry, sys_addrs[sys_entry]);
> +			break;
> +		}

	else {
		WARN_ON_ONCE("... indexing wrong bla... ");
	}

to catch crap.

-- 
Regards/Gruss,
    Boris.

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