From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 96F4EC5478C for ; Mon, 4 Mar 2024 12:08:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Subject:Cc:To:From:Message-ID:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=ZZR0/yUws4n+nIdsCyuZgxlVN7o9UMjKynjbu5/LnKE=; b=PqOPceB8W34Zec vr+4gaH5O0WSGhChjqML2uZuiQxe+ljOHTIGDaPMDdxJBXtGgsFh97AvP8rcX/1ah02sBXuk7BJ0d +qvgSVBxDsGHP0iokaaXJZwKX+zRwxQGnPWlw3jEYvXe3sMz7i2yGRf2OLSUPVAwLm8T6f0qFEpBu ZUz0V1zKJt+RQiw9AfWjsOinfVpePQmhQgSPBIk4zSWGKvlLRG4m9d7u97HHDkPTSbYfPNkAJynYk JxAouhOMxym/3eU2zb1GbsixgPU4fF4Ei2seMG31/8+J3xWcjjgiFhg9HQWKusAZuJDG8b44H/JQL Xaa9gX8b6GsVZuLS2nFg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rh775-00000008vRZ-3oOG; Mon, 04 Mar 2024 12:08:11 +0000 Received: from sin.source.kernel.org ([2604:1380:40e1:4800::1]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rh770-00000008vQm-1UcW for linux-arm-kernel@lists.infradead.org; Mon, 04 Mar 2024 12:08:08 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 2DF24CE10BF; Mon, 4 Mar 2024 12:08:01 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id B2A20C43390; Mon, 4 Mar 2024 12:07:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1709554080; bh=Cl+0SxVOA1juHMZEj8vL0DIgeLYC9OGfZf7ASrVHyx8=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=sNUc23nSt0y4osxPfL1NMofCrkhH6WNxukWDh9L85TBaBCzR+aYpV5vC+mPcl+SEw HGKAYPnFferiX3Ngs8l5wKPbjpMxrtLLseulKBuyq5SObxY+g/6stb600hjSei7SdZ ZOQEqaOfu5UAlnwtuZ28i13aXFKijXehGSu/8CW5auyeMsyre3+KqsWeC0H1Ta4VeV kzxqH7x29dQ7R+lzrYhVW3vPfQSx9YCogMUtZvyx4NM59tjrXaU6L1zXA+jfbu0Qwu nmW1jfzPNuiU4YcOGWH6tc4vX+bQdY4WsUQ+UiLAjbVEQ3+S+lNvbUCgEZqXlPWkro wqzUhI9k+pqbQ== Received: from sofa.misterjones.org ([185.219.108.64] helo=goblin-girl.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1rh76q-009D4H-TR; Mon, 04 Mar 2024 12:07:57 +0000 Date: Mon, 04 Mar 2024 12:07:55 +0000 Message-ID: <86wmqi19pg.wl-maz@kernel.org> From: Marc Zyngier To: Ruidong Tian Cc: catalin.marinas@arm.com, will@kernel.org, lpieralisi@kernel.org, guohanjun@huawei.com, sudeep.holla@arm.com, xueshuai@linux.alibaba.com, baolin.wang@linux.alibaba.com, linux-kernel@vger.kernel.org, linux-acpi@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Tyler Baicar Subject: Re: [PATCH 1/2] ACPI/AEST: Initial AEST driver In-Reply-To: <20240304111517.33001-2-tianruidong@linux.alibaba.com> References: <20240304111517.33001-1-tianruidong@linux.alibaba.com> <20240304111517.33001-2-tianruidong@linux.alibaba.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/29.1 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: tianruidong@linux.alibaba.com, catalin.marinas@arm.com, will@kernel.org, lpieralisi@kernel.org, guohanjun@huawei.com, sudeep.holla@arm.com, xueshuai@linux.alibaba.com, baolin.wang@linux.alibaba.com, linux-kernel@vger.kernel.org, linux-acpi@vger.kernel.org, linux-arm-kernel@lists.infradead.org, baicar@os.amperecomputing.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240304_040806_798576_69563C77 X-CRM114-Status: GOOD ( 40.12 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, 04 Mar 2024 11:15:16 +0000, Ruidong Tian wrote: > > From: Tyler Baicar > > 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. > > All detected errors will be collected to a workqueue in irq context and > print error information later. > > Signed-off-by: Tyler Baicar > Signed-off-by: Ruidong Tian > --- > MAINTAINERS | 11 + > arch/arm64/include/asm/ras.h | 38 ++ > drivers/acpi/arm64/Kconfig | 10 + > drivers/acpi/arm64/Makefile | 1 + > drivers/acpi/arm64/aest.c | 723 +++++++++++++++++++++++++++++++++++ > include/linux/acpi_aest.h | 91 +++++ > include/linux/cpuhotplug.h | 1 + > 7 files changed, 875 insertions(+) > create mode 100644 arch/arm64/include/asm/ras.h > create mode 100644 drivers/acpi/arm64/aest.c > create mode 100644 include/linux/acpi_aest.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index 2a7a90eeec49..5df845763a9c 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -329,6 +329,17 @@ L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers) > S: Maintained > F: drivers/acpi/arm64 > > +ACPI AEST > +M: Tyler Baicar > +M: Ruidong Tian > +L: linux-acpi@vger.kernel.org > +L: linux-arm-kernel@lists.infradead.org > +S: Supported > +F: arch/arm64/include/asm/ras.h > +F: drivers/acpi/arm64/aest.c > +F: include/linux/acpi_aest.h > + > + > ACPI FOR RISC-V (ACPI/riscv) > M: Sunil V L > L: linux-acpi@vger.kernel.org > diff --git a/arch/arm64/include/asm/ras.h b/arch/arm64/include/asm/ras.h > new file mode 100644 > index 000000000000..2fb0d9741567 > --- /dev/null > +++ b/arch/arm64/include/asm/ras.h > @@ -0,0 +1,38 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef __ASM_RAS_H > +#define __ASM_RAS_H > + > +#include > +#include > + > +#define ERR_STATUS_AV BIT(31) > +#define ERR_STATUS_V BIT(30) > +#define ERR_STATUS_UE BIT(29) > +#define ERR_STATUS_ER BIT(28) > +#define ERR_STATUS_OF BIT(27) > +#define ERR_STATUS_MV BIT(26) > +#define ERR_STATUS_CE (BIT(25) | BIT(24)) > +#define ERR_STATUS_DE BIT(23) > +#define ERR_STATUS_PN BIT(22) > +#define ERR_STATUS_UET (BIT(21) | BIT(20)) > +#define ERR_STATUS_CI BIT(19) > +#define ERR_STATUS_IERR GENMASK_ULL(15, 8) > +#define ERR_STATUS_SERR GENMASK_ULL(7, 0) All these bits need to be defined in arch/arm64/tools/sysreg as ERXSTATUS_EL1 fields. > + > +/* These bit is write-one-to-clear */ > +#define ERR_STATUS_W1TC (ERR_STATUS_AV | ERR_STATUS_V | ERR_STATUS_UE | \ > + ERR_STATUS_ER | ERR_STATUS_OF | ERR_STATUS_MV | \ > + ERR_STATUS_CE | ERR_STATUS_DE | ERR_STATUS_PN | \ > + ERR_STATUS_UET | ERR_STATUS_CI) > + > +#define RAS_REV_v1_1 0x1 What is this? We already have ID_AA64PFR1_EL1.RAS_frac. > + > +struct ras_ext_regs { > + u64 err_fr; > + u64 err_ctlr; > + u64 err_status; > + u64 err_addr; > + u64 err_misc[4]; > +}; > + > +#endif /* __ASM_RAS_H */ > diff --git a/drivers/acpi/arm64/Kconfig b/drivers/acpi/arm64/Kconfig > index b3ed6212244c..639db671c5cf 100644 > --- a/drivers/acpi/arm64/Kconfig > +++ b/drivers/acpi/arm64/Kconfig > @@ -21,3 +21,13 @@ config ACPI_AGDI > > config ACPI_APMT > bool > + > +config ACPI_AEST > + bool "ARM Error Source Table Support" > + > + help > + The Arm Error Source Table (AEST) provides details on ACPI > + extensions that enable kernel-first handling of errors in a > + system that supports the Armv8 RAS extensions. > + > + If set, the kernel will report and log hardware errors. > diff --git a/drivers/acpi/arm64/Makefile b/drivers/acpi/arm64/Makefile > index 726944648c9b..5ea82b196b90 100644 > --- a/drivers/acpi/arm64/Makefile > +++ b/drivers/acpi/arm64/Makefile > @@ -6,3 +6,4 @@ obj-$(CONFIG_ACPI_APMT) += apmt.o > obj-$(CONFIG_ARM_AMBA) += amba.o > obj-y += dma.o init.o > obj-y += thermal_cpufreq.o > +obj-$(CONFIG_ACPI_AEST) += aest.o > diff --git a/drivers/acpi/arm64/aest.c b/drivers/acpi/arm64/aest.c > new file mode 100644 > index 000000000000..be0883316449 > --- /dev/null > +++ b/drivers/acpi/arm64/aest.c > @@ -0,0 +1,723 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * ARM Error Source Table Support > + * > + * Copyright (c) 2021, Ampere Computing LLC > + * Copyright (c) 2021-2024, Alibaba Group. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#undef pr_fmt > +#define pr_fmt(fmt) "ACPI AEST: " fmt > + > +#define CASE_READ_CLEAR(x, clear) \ > + case (x): { \ > + res = read_sysreg_s(SYS_##x##_EL1); \ > + if (clear) \ > + write_sysreg_s(0, SYS_##x##_EL1); \ > + break; \ > + } Please don't use macros with side effects. This is horrible to debug. Instead, *return* the value from the macro, or pass the variable you want to affect as a parameter. Also, what ensures the synchronisation of this write? How is the W1TC aspect enforced? > + > +#define CASE_WRITE(val, x) \ > + case (x): { \ > + write_sysreg_s((val), SYS_##x##_EL1); \ > + break; \ > + } > + > +static struct acpi_table_header *aest_table; > + > +static struct aest_node __percpu **ppi_data; > + > +static int *ppi_irqs; > +static u8 num_ppi; > +static u8 ppi_idx; > + > +static struct work_struct aest_work; > + > +static struct gen_pool *aest_node_pool; > +static struct llist_head aest_node_llist; > + > +/* > + * This memory pool is only to be used to save AEST node in AEST irq context. > + * Use 8 pages to save AEST node for now (~500 AEST node at most). > + */ > +#define AEST_NODE_POOLSZ (8 * PAGE_SIZE) This doesn't make sense. PAGE_SIZE is a variable concept (ranging from 4 to 64kB). What is this "~500" number coming from? If you want to store a given number of records, allocate the size you actually want. > + > +static u64 aest_sysreg_read_clear(u64 base, u32 offset, bool clear) > +{ > + u64 res; > + > + switch (offset) { > + CASE_READ_CLEAR(ERXFR, clear) > + CASE_READ_CLEAR(ERXCTLR, clear) > + CASE_READ_CLEAR(ERXSTATUS, clear) > + CASE_READ_CLEAR(ERXADDR, clear) > + CASE_READ_CLEAR(ERXMISC0, clear) > + CASE_READ_CLEAR(ERXMISC1, clear) > + CASE_READ_CLEAR(ERXMISC2, clear) > + CASE_READ_CLEAR(ERXMISC3, clear) > + default : > + res = 0; > + } > + return res; > +} > + > +static void aest_sysreg_write(u64 base, u32 offset, u64 val) > +{ > + switch (offset) { > + CASE_WRITE(val, ERXFR) > + CASE_WRITE(val, ERXCTLR) > + CASE_WRITE(val, ERXSTATUS) > + CASE_WRITE(val, ERXADDR) > + CASE_WRITE(val, ERXMISC0) > + CASE_WRITE(val, ERXMISC1) > + CASE_WRITE(val, ERXMISC2) > + CASE_WRITE(val, ERXMISC3) > + default : > + return; > + } > +} > + > +static u64 aest_iomem_read_clear(u64 base, u32 offset, bool clear) > +{ > + u64 res; > + > + res = readq((void *)(base + offset)); > + if (clear) > + writeq(0, (void *)(base + offset)); Do you need the explicit synchronisation? What ordering are you trying to guarantee? > + return res; > +} > + > +static void aest_iomem_write(u64 base, u32 offset, u64 val) > +{ > + writeq(val, (void *)(base + offset)); Same question. > +} > + > +static void aest_print(struct aest_node_llist *lnode) > +{ > + static atomic_t seqno; Uninitialised atomic? > + unsigned int curr_seqno; > + char pfx_seq[64]; Magic number? > + int index; > + struct ras_ext_regs *regs; > + > + curr_seqno = atomic_inc_return(&seqno); > + snprintf(pfx_seq, sizeof(pfx_seq), "{%u}" HW_ERR, curr_seqno); > + pr_info("%sHardware error from %s\n", pfx_seq, lnode->node_name); > + > + switch (lnode->type) { > + case ACPI_AEST_PROCESSOR_ERROR_NODE: > + pr_err("%s Error from CPU%d\n", pfx_seq, lnode->id0); > + break; > + case ACPI_AEST_MEMORY_ERROR_NODE: > + pr_err("%s Error from memory at SRAT proximity domain 0x%x\n", > + pfx_seq, lnode->id0); > + break; > + case ACPI_AEST_SMMU_ERROR_NODE: > + pr_err("%s Error from SMMU IORT node 0x%x subcomponent 0x%x\n", > + pfx_seq, lnode->id0, lnode->id1); > + break; > + case ACPI_AEST_VENDOR_ERROR_NODE: > + pr_err("%s Error from vendor hid 0x%x uid 0x%x\n", > + pfx_seq, lnode->id0, lnode->id1); > + break; > + case ACPI_AEST_GIC_ERROR_NODE: > + pr_err("%s Error from GIC type 0x%x instance 0x%x\n", > + pfx_seq, lnode->id0, lnode->id1); > + break; > + default: > + pr_err("%s Unknown AEST node type\n", pfx_seq); > + return; > + } > + > + index = lnode->index; > + regs = &lnode->regs; > + > + pr_err("%s ERR%uFR: 0x%llx\n", pfx_seq, index, regs->err_fr); > + pr_err("%s ERR%uCTRL: 0x%llx\n", pfx_seq, index, regs->err_ctlr); > + pr_err("%s ERR%uSTATUS: 0x%llx\n", pfx_seq, index, regs->err_status); > + if (regs->err_status & ERR_STATUS_AV) > + pr_err("%s ERR%uADDR: 0x%llx\n", pfx_seq, index, regs->err_addr); > + > + if (regs->err_status & ERR_STATUS_MV) { > + pr_err("%s ERR%uMISC0: 0x%llx\n", pfx_seq, index, regs->err_misc[0]); > + pr_err("%s ERR%uMISC1: 0x%llx\n", pfx_seq, index, regs->err_misc[1]); > + pr_err("%s ERR%uMISC2: 0x%llx\n", pfx_seq, index, regs->err_misc[2]); > + pr_err("%s ERR%uMISC3: 0x%llx\n", pfx_seq, index, regs->err_misc[3]); > + } > +} > + > + > +static void aest_node_pool_process(struct work_struct *__unused) > +{ > + struct llist_node *head; > + struct aest_node_llist *lnode, *tmp; > + > + head = llist_del_all(&aest_node_llist); > + if (!head) > + return; > + > + head = llist_reverse_order(head); > + llist_for_each_entry_safe(lnode, tmp, head, llnode) { > + aest_print(lnode); > + gen_pool_free(aest_node_pool, (unsigned long)lnode, > + sizeof(*lnode)); > + } > +} > + > +static int aest_node_gen_pool_add(struct aest_node *node, int index, > + struct ras_ext_regs *regs) > +{ > + struct aest_node_llist *list; > + > + if (!aest_node_pool) > + return -EINVAL; > + > + list = (void *)gen_pool_alloc(aest_node_pool, sizeof(*list)); > + if (!list) > + return -ENOMEM; > + > + list->type = node->type; > + list->node_name = node->name; > + switch (node->type) { > + case ACPI_AEST_PROCESSOR_ERROR_NODE: > + list->id0 = node->spec.processor.processor_id; > + if (node->spec.processor.flags & (ACPI_AEST_PROC_FLAG_SHARED | > + ACPI_AEST_PROC_FLAG_GLOBAL)) > + list->id0 = smp_processor_id(); > + > + list->id1 = node->spec.processor.resource_type; > + break; > + case ACPI_AEST_MEMORY_ERROR_NODE: > + list->id0 = node->spec.memory.srat_proximity_domain; > + break; > + case ACPI_AEST_SMMU_ERROR_NODE: > + list->id0 = node->spec.smmu.iort_node_reference; > + list->id1 = node->spec.smmu.subcomponent_reference; > + break; > + case ACPI_AEST_VENDOR_ERROR_NODE: > + list->id0 = node->spec.vendor.acpi_hid; > + list->id1 = node->spec.vendor.acpi_uid; > + break; > + case ACPI_AEST_GIC_ERROR_NODE: > + list->id0 = node->spec.gic.interface_type; > + list->id1 = node->spec.gic.instance_id; > + break; > + default: > + list->id0 = 0; > + list->id1 = 0; > + } > + > + memcpy(&list->regs, regs, sizeof(*regs)); You have vmalloced the record. Why do you need to copy it instead of simply pointing to it? > + list->index = index; > + llist_add(&list->llnode, &aest_node_llist); > + > + return 0; > +} > + > +static int aest_node_pool_init(void) > +{ > + unsigned long addr, size; > + int rc; > + > + if (aest_node_pool) > + return 0; > + > + aest_node_pool = gen_pool_create(ilog2(sizeof(struct aest_node_llist)), -1); > + if (!aest_node_pool) > + return -ENOMEM; > + > + size = PAGE_ALIGN(AEST_NODE_POOLSZ); > + addr = (unsigned long)vmalloc(size); > + if (!addr) > + goto err_pool_alloc; > + > + rc = gen_pool_add(aest_node_pool, addr, size, -1); > + if (rc) > + goto err_pool_add; > + > + return 0; > + > +err_pool_add: > + vfree((void *)addr); > + > +err_pool_alloc: > + gen_pool_destroy(aest_node_pool); > + > + return -ENOMEM; > +} > + > +static void aest_log(struct aest_node *node, int index, struct ras_ext_regs *regs) > +{ > + if (!aest_node_gen_pool_add(node, index, regs)) > + schedule_work(&aest_work); > +} > + > +/* > + * you must select cpu number first in order to operate RAS register belonged > + * that cpu. > + */ > +static void aest_select_cpu(struct aest_node *node, int i) > +{ > + if (node->interface.type == ACPI_AEST_NODE_SYSTEM_REGISTER) { > + write_sysreg_s(i, SYS_ERRSELR_EL1); ERRSELR_EL1 doesn't select a CPU. It selects a RAS record. How records and CPUs are mapped isn't specified in the architecture. > + isb(); > + } > +} > + > +static void aest_proc(struct aest_node *node) > +{ > + struct ras_ext_regs regs = {0}; > + struct aest_access *access; > + int i; > + u64 regs_p; > + > + > + for (i = node->interface.record_start; i < node->interface.record_end; i++) { > + /* 1b: Error record at i index is not implemented */ > + if (test_bit(i, &node->interface.record_implemented)) > + continue; > + > + aest_select_cpu(node, i); > + > + access = node->access; > + regs_p = (u64)&node->interface.regs[i]; > + > + regs.err_status = access->read_clear(regs_p, ERXSTATUS, false); > + if (!(regs.err_status & ERR_STATUS_V)) > + continue; > + > + if (regs.err_status & ERR_STATUS_AV) > + regs.err_addr = access->read_clear(regs_p, ERXADDR, false); > + > + regs.err_fr = access->read_clear(regs_p, ERXFR, false); > + regs.err_ctlr = access->read_clear(regs_p, ERXCTLR, false); > + > + if (regs.err_status & ERR_STATUS_MV) { > + bool clear = node->interface.flags & ACPI_AEST_INTERFACE_CLEAR_MISC; > + > + regs.err_misc[0] = access->read_clear(regs_p, ERXMISC0, clear); > + regs.err_misc[1] = access->read_clear(regs_p, ERXMISC1, clear); > + regs.err_misc[2] = access->read_clear(regs_p, ERXMISC2, clear); > + regs.err_misc[3] = access->read_clear(regs_p, ERXMISC3, clear); > + } > + > + aest_log(node, i, ®s); > + > + if (regs.err_status & ERR_STATUS_UE) > + panic("AEST: uncorrectable error encountered"); > + > + /* Write-one-to-clear the bits we've seen */ > + regs.err_status &= ERR_STATUS_W1TC; > + > + /* Multi bit filed need to write all-ones to clear. */ > + if (regs.err_status & ERR_STATUS_CE) > + regs.err_status |= ERR_STATUS_CE; > + > + /* Multi bit filed need to write all-ones to clear. */ > + if (regs.err_status & ERR_STATUS_UET) > + regs.err_status |= ERR_STATUS_UET; > + > + access->write(regs_p, ERXSTATUS, regs.err_status); > + } > +} > + > +static irqreturn_t aest_irq_func(int irq, void *input) > +{ > + struct aest_node *node = input; > + > + aest_proc(node); > + > + return IRQ_HANDLED; > +} > + > +static int __init aest_register_gsi(u32 gsi, int trigger, void *data, > + irq_handler_t aest_irq_func) > +{ > + int cpu, irq; > + > + irq = acpi_register_gsi(NULL, gsi, trigger, ACPI_ACTIVE_HIGH); > + > + if (irq == -EINVAL) { > + pr_err("failed to map AEST GSI %d\n", gsi); > + return -EINVAL; > + } > + > + if (gsi < 16) { > + pr_err("invalid GSI %d\n", gsi); > + return -EINVAL; > + } else if (gsi < 32) { > + if (ppi_idx >= AEST_MAX_PPI) { > + pr_err("Unable to register PPI %d\n", gsi); > + return -EINVAL; > + } > + ppi_irqs[ppi_idx] = irq; > + enable_percpu_irq(irq, IRQ_TYPE_NONE); Enabling the PPI before requesting it? Looks... great. And how does this work on a system that supports EPPIs, which are in the [1119:1056] range? Also, if you get a trigger as a parameter, why the IRQ_TYPE_NONE? > + for_each_possible_cpu(cpu) { > + memcpy(per_cpu_ptr(ppi_data[ppi_idx], cpu), data, > + sizeof(struct aest_node)); > + } > + if (request_percpu_irq(irq, aest_irq_func, "AEST", > + ppi_data[ppi_idx++])) { > + pr_err("failed to register AEST IRQ %d\n", irq); > + return -EINVAL; > + } > + } else if (gsi < 1020) { > + if (request_irq(irq, aest_irq_func, IRQF_SHARED, "AEST", > + data)) { Why SHARED? Who would share a RAS interrupt????? > + pr_err("failed to register AEST IRQ %d\n", irq); > + return -EINVAL; Same question about extended SPIs. All in all, this whole logic is totally useless. It isn't the driver's job to classify the GIC INTIDs... M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel