Messages in this thread | | | Date | Mon, 9 May 2022 09:37:41 -0400 | Subject | Re: [PATCH 1/2] ACPI/AEST: Initial AEST driver | From | Tyler Baicar <> |
| |
Hi Shuuichirou,
I should be able to get a v2 patch series out by the end of the month.
Thanks, Tyler
On 4/20/2022 3:54 AM, ishii.shuuichir@fujitsu.com wrote: > Hi, Tyler. > > When do you plan to post the v2 patch series? > Please let me know if you don't mind. > > Best regards. > >> -----Original Message----- >> From: Tyler Baicar <baicar@amperemail.onmicrosoft.com> >> Sent: Friday, December 17, 2021 8:33 AM >> To: Ishii, Shuuichirou/石井 周一郎 <ishii.shuuichir@fujitsu.com>; 'Tyler Baicar' >> <baicar@os.amperecomputing.com>; patches@amperecomputing.com; >> abdulhamid@os.amperecomputing.com; darren@os.amperecomputing.com; >> catalin.marinas@arm.com; will@kernel.org; maz@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; >> Vineeth.Pillai@microsoft.com >> Subject: Re: [PATCH 1/2] ACPI/AEST: Initial AEST driver >> >> Hi Shuuichirou, >> >> Thank you for your feedback! >> >> On 12/9/2021 3:10 AM, ishii.shuuichir@fujitsu.com wrote: >>> Hi, Tyler. >>> >>> We would like to make a few comments. >>> >>>> -----Original Message----- >>>> From: Tyler Baicar <baicar@os.amperecomputing.com> >>>> Sent: Thursday, November 25, 2021 2:07 AM >>>> To: patches@amperecomputing.com; abdulhamid@os.amperecomputing.com; >>>> darren@os.amperecomputing.com; catalin.marinas@arm.com; >>>> will@kernel.org; maz@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, Shuuichirou/石井 >>>> 周一郎 <ishii.shuuichir@fujitsu.com>; Vineeth.Pillai@microsoft.com >>>> Cc: Tyler Baicar <baicar@os.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@os.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? >> aest_init_interrupts() returns an error if any of the interrupts in the interrupt list >> fails, but it's possible that some interrupts in the list registered successfully. So >> we attempt to keep chugging along in that scenario because some interrupts may >> be enabled and registered with the interface successfully. If any interrupt >> registration fails, there will be a print notifying that there was a failure when >> initializing that node. >>>> +} >>>> + >>>> +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()? >> >> Good point. I will add that in the next version. >> >> Thanks, >> >> Tyler >
| |