Hi, Tyler. We would like to make a few comments.
> -----Original Message----- > From: Tyler Baicar <[email protected]> > Sent: Thursday, November 25, 2021 2:07 AM > To: [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; > [email protected]; [email protected]; > [email protected]; [email protected]; Ishii, Shuuichirou/石井 > 周一郎 <[email protected]>; [email protected] > Cc: Tyler Baicar <[email protected]> > 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 <[email protected]> > --- [...] > +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. _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
