linux-kernel - Re: [PATCH v3 4/5] RAS/ATL: Unified ATL interface for ARM64 and AMD

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250219210004.GTZ7ZGVHn9h5h88_fJ@fat_crate.local>
Date: Wed, 19 Feb 2025 22:00:04 +0100
From: Borislav Petkov <bp@...en8.de>
To: Ruidong Tian <tianruidong@...ux.alibaba.com>
Cc: catalin.marinas@....com, will@...nel.org, lpieralisi@...nel.org,
	guohanjun@...wei.com, sudeep.holla@....com,
	xueshuai@...ux.alibaba.com, baolin.wang@...ux.alibaba.com,
	linux-kernel@...r.kernel.org, linux-acpi@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, rafael@...nel.org,
	lenb@...nel.org, tony.luck@...el.com, yazen.ghannam@....com
Subject: Re: [PATCH v3 4/5] RAS/ATL: Unified ATL interface for ARM64 and AMD

On Wed, Jan 15, 2025 at 04:42:27PM +0800, Ruidong Tian wrote:
> Subject: Re: [PATCH v3 4/5] RAS/ATL: Unified ATL interface for ARM64 and AMD

The condensed patch description in the subject line should start with a
uppercase letter and should be written in imperative tone.


> Translate device normalize address in AMD, also named logical address,
> to system physical address is a common interface in RAS. Provides common
> interface both for AMD and ARM.

This needs a lot more explanation.

> Signed-off-by: Ruidong Tian <tianruidong@...ux.alibaba.com>
> ---
>  drivers/edac/amd64_edac.c      |  2 +-
>  drivers/ras/aest/aest-core.c   | 12 ++++++------
>  drivers/ras/amd/atl/core.c     |  4 ++--
>  drivers/ras/amd/atl/internal.h |  2 +-
>  drivers/ras/amd/atl/umc.c      |  3 ++-
>  drivers/ras/ras.c              | 24 +++++++++++-------------
>  include/linux/ras.h            |  9 ++++-----
>  7 files changed, 27 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index ddfbdb66b794..1e9c96e4daa8 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -2832,7 +2832,7 @@ static void decode_umc_error(int node_id, struct mce *m)
>  	a_err.ipid = m->ipid;
>  	a_err.cpu  = m->extcpu;
>  
> -	sys_addr = amd_convert_umc_mca_addr_to_sys_addr(&a_err);
> +	sys_addr = convert_ras_la_to_spa(&a_err);

No, this is not how all this is done. You don't rename functions and make them
generic - you *extract* generic functionality into generic functions and have
other functions which use them, call them.

And you do that when there are users, not before.

Ok, that should be enough feedback for now.

Thx.

-- 
Regards/Gruss,
    Boris.

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