Hi, Huacai
在 2024/8/8 下午4:03, Huacai Chen 写道:
Hi, Tianyang,
On Thu, Aug 8, 2024 at 2:52 PM Tianyang Zhang <zhangtianyang@xxxxxxxxxxx> wrote:
Hi, Thomas
Thank you for your feedback.
在 2024/8/8 上午6:01, Thomas Gleixner 写道:
+ guard(raw_spinlock_irqsave)(&loongarch_avec.lock);
+
+ for (i = 0; i < nr_irqs; i++) {
+ d = irq_domain_get_irq_data(domain, virq + i);
+ if (d) {
+ clear_free_vector(d);
+ irq_domain_reset_irq_data(d);
+
Stray newline, but the more important question is what kfree()'s 'd'?
AFAICT, nothing. So that's a memory leak, no?
With my understand , 'd' as 'struct irq_data' can be free at public
irqdomain process, and really miss a kfree targeting 'struct chip_data'
+static int __init avecintc_init(struct irq_domain *parent)
+{
+ parent_irq = irq_create_mapping(parent, INT_AVEC);
+ if (!parent_irq) {
+ pr_err("Failed to mapping hwirq\n");
+ ret = -EINVAL;
+ goto out_remove_domain;
+ }
+ irq_set_chained_handler_and_data(parent_irq, avecintc_irq_dispatch, NULL);
+
+ ret = irq_matrix_init();
+ if (ret < 0) {
+ pr_err("Failed to init irq matrix\n");
+ goto out_remove_domain;
Which still leaves the disfunct chained handler installed and the
mapping intact.
There is indeed a problem here, but we have not found a similar approach
for reference.
Is it reasonable to replace here with handle_bad_irq in case of failure?
or is there any other more suitable way. We hope you can give us some
suggestions, thank you very much
Maybe we can move irq_set_chained_handler_and_data(parent_irq,
avecintc_irq_dispatch, NULL) after the checking of irq_matrix_init().
Huacai
I think is a good idea~~
+#endif
+ value = iocsr_read64(LOONGARCH_IOCSR_MISC_FUNC);
+ value |= IOCSR_MISC_FUNC_AVEC_EN;
+ iocsr_write64(value, LOONGARCH_IOCSR_MISC_FUNC);
+
+ return ret;
+
+out_remove_domain:
+ irq_domain_remove(loongarch_avec.domain);
+out_free_handle:
+ irq_domain_free_fwnode(loongarch_avec.fwnode);
+out:
+ return ret;
+}
+
+static int __init pch_msi_parse_madt(union acpi_subtable_headers *header,
+ const unsigned long end)
+{
+ struct acpi_madt_msi_pic *pchmsi_entry = (struct acpi_madt_msi_pic *)header;
+
+ msi_base_addr = pchmsi_entry->msg_address - AVEC_MSG_OFFSET;
What validates that msi_base_addr has none of the lower 16 bits set, as
they are required to be zero to make MSI message composing work, right?
This operation originates from some hardware designs.
In 3C6000, either eiointc or avecintc can be the parent controller for
MSI interrupts and these two controllers have different MSI msg address.
In our platform design scheme, we fix avec-msg-address to the address of
(eiointc-msg-address - 0x100000). Therefore, here we need to subtract
AVEC_MSG_OFFSET from the msg_address obtained by MCFG
The main purpose of the design that users of 3C6000 can freely choose
the version of the Linux kernel that supports loongarch (regardless of
whether AVEC is supported or not) without having to change the firmware
Thanks again
Tianyang
