Date:   Wed, 24 Nov 2021 18:09:14 +0000
Message-ID: <87czmpcto5.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     Tyler Baicar <baicar@os.amperecomputing.com>
Cc:     patches@amperecomputing.com, abdulhamid@os.amperecomputing.com,
        darren@os.amperecomputing.com, catalin.marinas@arm.com,
        will@kernel.org, james.morse@arm.com, alexandru.elisei@arm.com,
        suzuki.poulose@arm.com, lorenzo.pieralisi@arm.com,
        guohanjun@huawei.com, sudeep.holla@arm.com, rafael@kernel.org,
        lenb@kernel.org, tony.luck@intel.com, bp@alien8.de,
        mark.rutland@arm.com, anshuman.khandual@arm.com,
        vincenzo.frascino@arm.com, tabba@google.com, marcan@marcan.st,
        keescook@chromium.org, jthierry@redhat.com, masahiroy@kernel.org,
        samitolvanen@google.com, john.garry@huawei.com,
        daniel.lezcano@linaro.org, gor@linux.ibm.com,
        zhangshaokun@hisilicon.com, tmricht@linux.ibm.com,
        dchinner@redhat.com, tglx@linutronix.de,
        linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        kvmarm@lists.cs.columbia.edu, linux-acpi@vger.kernel.org,
        linux-edac@vger.kernel.org, ishii.shuuichir@fujitsu.com,
        Vineeth.Pillai@microsoft.com
Subject: Re: [PATCH 1/2] ACPI/AEST: Initial AEST driver
In-Reply-To: <20211124170708.3874-2-baicar@os.amperecomputing.com>
References: <20211124170708.3874-1-baicar@os.amperecomputing.com>
        <20211124170708.3874-2-baicar@os.amperecomputing.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1
 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Wed, 24 Nov 2021 17:07:07 +0000,
Tyler Baicar <baicar@os.amperecomputing.com> wrote:
> 
> Add support for parsing the ARM Error Source Table and basic handling of
> errors reported through both memory mapped and system register interfaces.
> 
> Assume system register interfaces are only registered with private
> peripheral interrupts (PPIs); otherwise there is no guarantee the
> core handling the error is the core which took the error and has the
> syndrome info in its system registers.
> 
> Add logging for all detected errors and trigger a kernel panic if there is
> any uncorrected error present.
> 
> Signed-off-by: Tyler Baicar <baicar@os.amperecomputing.com>
> ---
>  MAINTAINERS                     |   1 +
>  arch/arm64/include/asm/ras.h    |  52 ++++
>  arch/arm64/include/asm/sysreg.h |   2 +
>  arch/arm64/kernel/Makefile      |   1 +
>  arch/arm64/kernel/ras.c         | 125 +++++++++
>  arch/arm64/kvm/sys_regs.c       |   2 +
>  drivers/acpi/arm64/Kconfig      |   3 +
>  drivers/acpi/arm64/Makefile     |   1 +
>  drivers/acpi/arm64/aest.c       | 450 ++++++++++++++++++++++++++++++++
>  include/linux/acpi_aest.h       |  50 ++++
>  include/linux/cpuhotplug.h      |   1 +
>  11 files changed, 688 insertions(+)
>  create mode 100644 arch/arm64/include/asm/ras.h
>  create mode 100644 arch/arm64/kernel/ras.c
>  create mode 100644 drivers/acpi/arm64/aest.c
>  create mode 100644 include/linux/acpi_aest.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5250298d2817..aa0483726606 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -382,6 +382,7 @@ ACPI FOR ARM64 (ACPI/arm64)
>  M:	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>  M:	Hanjun Guo <guohanjun@huawei.com>
>  M:	Sudeep Holla <sudeep.holla@arm.com>
> +R:	Tyler Baicar <baicar@os.amperecomputing.com>
>  L:	linux-acpi@vger.kernel.org
>  L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
>  S:	Maintained

Isn't this a bit premature? This isn't even mentioned in the commit
message, only in passing in the cover letter.

> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index 16b3f1a1d468..6bbed061d835 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -230,6 +230,8 @@
>  #define SYS_ERXADDR_EL1			sys_reg(3, 0, 5, 4, 3)
>  #define SYS_ERXMISC0_EL1		sys_reg(3, 0, 5, 5, 0)
>  #define SYS_ERXMISC1_EL1		sys_reg(3, 0, 5, 5, 1)
> +#define SYS_ERXMISC2_EL1		sys_reg(3, 0, 5, 5, 2)
> +#define SYS_ERXMISC3_EL1		sys_reg(3, 0, 5, 5, 3)
>  #define SYS_TFSR_EL1			sys_reg(3, 0, 5, 6, 0)
>  #define SYS_TFSRE0_EL1			sys_reg(3, 0, 5, 6, 1)
>  
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index e3ec1a44f94d..dc15e9896db4 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1573,6 +1573,8 @@ static const struct sys_reg_desc sys_reg_descs[] = {
>  	{ SYS_DESC(SYS_ERXADDR_EL1), trap_raz_wi },
>  	{ SYS_DESC(SYS_ERXMISC0_EL1), trap_raz_wi },
>  	{ SYS_DESC(SYS_ERXMISC1_EL1), trap_raz_wi },
> +	{ SYS_DESC(SYS_ERXMISC2_EL1), trap_raz_wi },
> +	{ SYS_DESC(SYS_ERXMISC3_EL1), trap_raz_wi },
>

This looks like a fix that would deserve its own patch.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.