[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <TYCPR01MB6160D05580A6E8C9510D25A5E9709@TYCPR01MB6160.jpnprd01.prod.outlook.com>
Date: Thu, 9 Dec 2021 08:10:50 +0000
From: "ishii.shuuichir@...itsu.com" <ishii.shuuichir@...itsu.com>
To: 'Tyler Baicar' <baicar@...amperecomputing.com>,
"patches@...erecomputing.com" <patches@...erecomputing.com>,
"abdulhamid@...amperecomputing.com"
<abdulhamid@...amperecomputing.com>,
"darren@...amperecomputing.com" <darren@...amperecomputing.com>,
"catalin.marinas@....com" <catalin.marinas@....com>,
"will@...nel.org" <will@...nel.org>,
"maz@...nel.org" <maz@...nel.org>,
"james.morse@....com" <james.morse@....com>,
"alexandru.elisei@....com" <alexandru.elisei@....com>,
"suzuki.poulose@....com" <suzuki.poulose@....com>,
"lorenzo.pieralisi@....com" <lorenzo.pieralisi@....com>,
"guohanjun@...wei.com" <guohanjun@...wei.com>,
"sudeep.holla@....com" <sudeep.holla@....com>,
"rafael@...nel.org" <rafael@...nel.org>,
"lenb@...nel.org" <lenb@...nel.org>,
"tony.luck@...el.com" <tony.luck@...el.com>,
"bp@...en8.de" <bp@...en8.de>,
"mark.rutland@....com" <mark.rutland@....com>,
"anshuman.khandual@....com" <anshuman.khandual@....com>,
"vincenzo.frascino@....com" <vincenzo.frascino@....com>,
"tabba@...gle.com" <tabba@...gle.com>,
"marcan@...can.st" <marcan@...can.st>,
"keescook@...omium.org" <keescook@...omium.org>,
"jthierry@...hat.com" <jthierry@...hat.com>,
"masahiroy@...nel.org" <masahiroy@...nel.org>,
"samitolvanen@...gle.com" <samitolvanen@...gle.com>,
"john.garry@...wei.com" <john.garry@...wei.com>,
"daniel.lezcano@...aro.org" <daniel.lezcano@...aro.org>,
"gor@...ux.ibm.com" <gor@...ux.ibm.com>,
"zhangshaokun@...ilicon.com" <zhangshaokun@...ilicon.com>,
"tmricht@...ux.ibm.com" <tmricht@...ux.ibm.com>,
"dchinner@...hat.com" <dchinner@...hat.com>,
"tglx@...utronix.de" <tglx@...utronix.de>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"kvmarm@...ts.cs.columbia.edu" <kvmarm@...ts.cs.columbia.edu>,
"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
"linux-edac@...r.kernel.org" <linux-edac@...r.kernel.org>,
"Vineeth.Pillai@...rosoft.com" <Vineeth.Pillai@...rosoft.com>
CC: "ishii.shuuichir@...itsu.com" <ishii.shuuichir@...itsu.com>
Subject: RE: [PATCH 1/2] ACPI/AEST: Initial AEST driver
Hi, Tyler.
We would like to make a few comments.
> -----Original Message-----
> From: Tyler Baicar <baicar@...amperecomputing.com>
> Sent: Thursday, November 25, 2021 2:07 AM
> To: patches@...erecomputing.com; abdulhamid@...amperecomputing.com;
> darren@...amperecomputing.com; catalin.marinas@....com; will@...nel.org;
> maz@...nel.org; james.morse@....com; alexandru.elisei@....com;
> suzuki.poulose@....com; lorenzo.pieralisi@....com; guohanjun@...wei.com;
> sudeep.holla@....com; rafael@...nel.org; lenb@...nel.org;
> tony.luck@...el.com; bp@...en8.de; mark.rutland@....com;
> anshuman.khandual@....com; vincenzo.frascino@....com;
> tabba@...gle.com; marcan@...can.st; keescook@...omium.org;
> jthierry@...hat.com; masahiroy@...nel.org; samitolvanen@...gle.com;
> john.garry@...wei.com; daniel.lezcano@...aro.org; gor@...ux.ibm.com;
> zhangshaokun@...ilicon.com; tmricht@...ux.ibm.com; dchinner@...hat.com;
> tglx@...utronix.de; linux-kernel@...r.kernel.org;
> linux-arm-kernel@...ts.infradead.org; kvmarm@...ts.cs.columbia.edu;
> linux-acpi@...r.kernel.org; linux-edac@...r.kernel.org; Ishii, Shuuichirou/石井
> 周一郎 <ishii.shuuichir@...itsu.com>; Vineeth.Pillai@...rosoft.com
> Cc: Tyler Baicar <baicar@...amperecomputing.com>
> Subject: [PATCH 1/2] ACPI/AEST: Initial AEST driver
>
> 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@...amperecomputing.com>
> ---
[...]
> +static int __init aest_init_node(struct acpi_aest_hdr *node)
> +{
> + union acpi_aest_processor_data *proc_data;
> + union aest_node_spec *node_spec;
> + struct aest_node_data *data;
> + int ret;
> +
> + data = kzalloc(sizeof(struct aest_node_data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + data->node_type = node->type;
> +
> + node_spec = ACPI_ADD_PTR(union aest_node_spec, node,
> node->node_specific_offset);
> +
> + switch (node->type) {
> + case ACPI_AEST_PROCESSOR_ERROR_NODE:
> + memcpy(&data->data, node_spec, sizeof(struct
> acpi_aest_processor));
> + break;
> + case ACPI_AEST_MEMORY_ERROR_NODE:
> + memcpy(&data->data, node_spec, sizeof(struct
> acpi_aest_memory));
> + break;
> + case ACPI_AEST_SMMU_ERROR_NODE:
> + memcpy(&data->data, node_spec, sizeof(struct
> acpi_aest_smmu));
> + break;
> + case ACPI_AEST_VENDOR_ERROR_NODE:
> + memcpy(&data->data, node_spec, sizeof(struct
> acpi_aest_vendor));
> + break;
> + case ACPI_AEST_GIC_ERROR_NODE:
> + memcpy(&data->data, node_spec, sizeof(struct
> acpi_aest_gic));
> + break;
> + default:
> + kfree(data);
> + return -EINVAL;
> + }
> +
> + if (node->type == ACPI_AEST_PROCESSOR_ERROR_NODE) {
> + proc_data = ACPI_ADD_PTR(union acpi_aest_processor_data,
> node_spec,
> + sizeof(acpi_aest_processor));
> +
> + switch (data->data.processor.resource_type) {
> + case ACPI_AEST_CACHE_RESOURCE:
> + memcpy(&data->proc_data, proc_data,
> + sizeof(struct acpi_aest_processor_cache));
> + break;
> + case ACPI_AEST_TLB_RESOURCE:
> + memcpy(&data->proc_data, proc_data,
> + sizeof(struct acpi_aest_processor_tlb));
> + break;
> + case ACPI_AEST_GENERIC_RESOURCE:
> + memcpy(&data->proc_data, proc_data,
> + sizeof(struct acpi_aest_processor_generic));
> + break;
> + }
> + }
> +
> + ret = aest_init_interface(node, data);
> + if (ret) {
> + kfree(data);
> + return ret;
> + }
> +
> + return aest_init_interrupts(node, data);
If aest_init_interrupts() failed, is it necessary to release
the data pointer acquired by kzalloc?
> +}
> +
> +static void aest_count_ppi(struct acpi_aest_hdr *node)
> +{
> + struct acpi_aest_node_interrupt *interrupt;
> + int i;
> +
> + interrupt = ACPI_ADD_PTR(struct acpi_aest_node_interrupt, node,
> + node->node_interrupt_offset);
> +
> + for (i = 0; i < node->node_interrupt_count; i++, interrupt++) {
> + if (interrupt->gsiv >= 16 && interrupt->gsiv < 32)
> + num_ppi++;
> + }
> +}
> +
> +static int aest_starting_cpu(unsigned int cpu)
> +{
> + int i;
> +
> + for (i = 0; i < num_ppi; i++)
> + enable_percpu_irq(ppi_irqs[i], IRQ_TYPE_NONE);
> +
> + return 0;
> +}
> +
> +static int aest_dying_cpu(unsigned int cpu)
> +{
Wouldn't it be better to execute disable_percpu_irq(), which is paired
with enable_percpu_irq(), in aest_dying_cpu()?
> + return 0;
> +}
> +
[...]
Best regards,
Shuuichirou.
Powered by blists - more mailing lists