Re: [PATCH 2/3] RAS/AMD/FMPM: Save SPA values

[Borislav Petkov]

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