linux-kernel - Re: [PATCH v2 1/4] x86/mce: Add wrapper for struct mce to export vendor specific info

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240626104427.GNZnvxC1JHclKwwKQU@fat_crate.local>
Date: Wed, 26 Jun 2024 12:44:27 +0200
From: Borislav Petkov <bp@...en8.de>
To: tony.luck@...el.com
Cc: Avadhut Naik <avadhut.naik@....com>, x86@...nel.org,
	linux-edac@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
	linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
	rafael@...nel.org, tglx@...utronix.de, mingo@...hat.com,
	rostedt@...dmis.org, lenb@...nel.org, mchehab@...nel.org,
	james.morse@....com, airlied@...il.com, yazen.ghannam@....com,
	john.allen@....com, avadnaik@....com
Subject: Re: [PATCH v2 1/4] x86/mce: Add wrapper for struct mce to export
 vendor specific info

On Tue, Jun 25, 2024 at 02:56:21PM -0500, Avadhut Naik wrote:
> Currently, exporting new additional machine check error information
> involves adding new fields for the same at the end of the struct mce.
> This additional information can then be consumed through mcelog or
> tracepoint.
> 
> However, as new MSRs are being added (and will be added in the future)
> by CPU vendors on their newer CPUs with additional machine check error
> information to be exported, the size of struct mce will balloon on some
> CPUs, unnecessarily, since those fields are vendor-specific. Moreover,
> different CPU vendors may export the additional information in varying
> sizes.
> 
> The problem particularly intensifies since struct mce is exposed to
> userspace as part of UAPI. It's bloating through vendor-specific data
> should be avoided to limit the information being sent out to userspace.
> 
> Add a new structure mce_hw_err to wrap the existing struct mce. The same
> will prevent its ballooning since vendor-specifc data, if any, can now be
> exported through a union within the wrapper structure and through
> __dynamic_array in mce_record tracepoint.
> 
> Furthermore, new internal kernel fields can be added to the wrapper
> struct without impacting the user space API.
> 
> Note: Some Checkpatch checks have been ignored to maintain coding style.
> 
> [Yazen: Add last commit message paragraph.]
> 
> Suggested-by: Borislav Petkov (AMD) <bp@...en8.de>
> Signed-off-by: Avadhut Naik <avadhut.naik@....com>
> Signed-off-by: Yazen Ghannam <yazen.ghannam@....com>
> ---
>  arch/x86/include/asm/mce.h              |   6 +-
>  arch/x86/kernel/cpu/mce/amd.c           |  29 ++--
>  arch/x86/kernel/cpu/mce/apei.c          |  54 +++----
>  arch/x86/kernel/cpu/mce/core.c          | 178 +++++++++++++-----------
>  arch/x86/kernel/cpu/mce/dev-mcelog.c    |   2 +-
>  arch/x86/kernel/cpu/mce/genpool.c       |  20 +--
>  arch/x86/kernel/cpu/mce/inject.c        |   4 +-
>  arch/x86/kernel/cpu/mce/internal.h      |   4 +-
>  drivers/acpi/acpi_extlog.c              |   2 +-
>  drivers/acpi/nfit/mce.c                 |   2 +-
>  drivers/edac/i7core_edac.c              |   2 +-
>  drivers/edac/igen6_edac.c               |   2 +-
>  drivers/edac/mce_amd.c                  |   2 +-
>  drivers/edac/pnd2_edac.c                |   2 +-
>  drivers/edac/sb_edac.c                  |   2 +-
>  drivers/edac/skx_common.c               |   2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c |   2 +-
>  drivers/ras/amd/fmpm.c                  |   2 +-
>  drivers/ras/cec.c                       |   2 +-
>  include/trace/events/mce.h              |  42 +++---
>  20 files changed, 199 insertions(+), 162 deletions(-)

Ok, did some minor massaging but otherwise looks ok now.

Tony, any comments? You ok with this, would that fit any Intel-specific vendor
fields too or do you need some additional Intel-specific changes?

Thx.

-- 
Regards/Gruss,
    Boris.

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