lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 15 Oct 2015 09:03:36 -0500 From: Suravee Suthikulanit <suravee.suthikulpanit@....com> To: Tomasz Nowicki <tn@...ihalf.com>, <marc.zyngier@....com>, <tglx@...utronix.de>, <jason@...edaemon.net>, <rjw@...ysocki.net> CC: Lorenzo Pieralisi <lorenzo.pieralisi@....com>, Will Deacon <will.deacon@....com>, Catalin Marinas <Catalin.Marinas@....com>, <hanjun.guo@...aro.org>, <graeme.gregory@...aro.org>, <dhdang@....com>, <linux-arm-kernel@...ts.infradead.org>, <linux-kernel@...r.kernel.org>, <linux-acpi@...r.kernel.org> Subject: Re: [PATCH V2 6/6] gicv2m: acpi: Introducing GICv2m ACPI support Hi Tomasz, Thanks for the feedback. On 10/15/2015 1:15 AM, Tomasz Nowicki wrote: >> [..] >> @@ -138,6 +140,11 @@ static int gicv2m_irq_gic_domain_alloc(struct >> irq_domain *domain, >> fwspec.param[0] = 0; >> fwspec.param[1] = hwirq - 32; >> fwspec.param[2] = IRQ_TYPE_EDGE_RISING; >> + } else if (is_fwnode_irqchip(domain->parent->fwnode)) { >> + fwspec.fwnode = domain->parent->fwnode; >> + fwspec.param_count = 2; >> + fwspec.param[0] = hwirq; >> + fwspec.param[1] = IRQ_TYPE_EDGE_RISING & IRQ_TYPE_SENSE_MASK; > How about just: > fwspec.param[1] = IRQ_TYPE_EDGE_RISING; Right. > >> } else { >> return -EINVAL; >> } >> @@ -255,6 +262,8 @@ static void gicv2m_teardown(void) >> kfree(v2m->bm); >> iounmap(v2m->base); >> of_node_put(to_of_node(v2m->fwnode)); >> + if (is_fwnode_irqchip(v2m->fwnode)) >> + irq_domain_free_fwnode(v2m->fwnode); >> kfree(v2m); >> } >> } >> @@ -359,6 +368,8 @@ static int __init gicv2m_init_one(struct >> fwnode_handle *fwnode, >> >> if (to_of_node(fwnode)) >> name = to_of_node(fwnode)->name; >> + else >> + name = irq_domain_get_irqchip_fwnode_name(fwnode); >> >> pr_info("Frame %s: range[%#lx:%#lx], SPI[%d:%d]\n", name, >> (unsigned long)res->start, (unsigned long)res->end, >> @@ -415,3 +426,86 @@ int __init gicv2m_of_init(struct device_node >> *node, struct irq_domain *parent) >> gicv2m_teardown(); >> return ret; >> } >> + >> +#ifdef CONFIG_ACPI >> +static int acpi_num_msi; >> + >> +static struct fwnode_handle *gicv2m_get_fwnode(struct device *dev) >> +{ >> + struct v2m_data *data; >> + >> + if (WARN_ON(acpi_num_msi <= 0)) >> + return NULL; >> + >> + /* We only return the fwnode of the first MSI frame. */ >> + data = list_first_entry_or_null(&v2m_nodes, >> + struct v2m_data, entry); > This can be one line and still fits within 80 characters. Ok. >> + if (!data) >> + return NULL; >> + >> + return data->fwnode; >> +} >> + >> +static int __init >> +acpi_parse_madt_msi(struct acpi_subtable_header *header, >> + const unsigned long end) >> +{ >> + int ret; >> + struct resource res; >> + u32 spi_start = 0, nr_spis = 0; >> + struct acpi_madt_generic_msi_frame *m; >> + struct fwnode_handle *fwnode = NULL; >> + >> + m = (struct acpi_madt_generic_msi_frame *)header; >> + if (BAD_MADT_ENTRY(m, end)) >> + return -EINVAL; >> + >> + res.start = m->base_address; >> + res.end = m->base_address + 0x1000; >> + >> + if (m->flags & ACPI_MADT_OVERRIDE_SPI_VALUES) { >> + spi_start = m->spi_base; >> + nr_spis = m->spi_count; >> + >> + pr_info("ACPI overriding V2M MSI_TYPER (base:%u, num:%u)\n", >> + spi_start, nr_spis); >> + } >> + >> + fwnode = irq_domain_alloc_fwnode((void *)m->base_address); >> + if (!fwnode) { >> + pr_err("Unable to allocate GICv2m domain token\n"); >> + return -EINVAL; >> + } >> + >> + ret = gicv2m_init_one(fwnode, spi_start, nr_spis, &res); > I case of error, we should call here: > irq_domain_free_fwnode(fwnode); This should have already been handled when returning from the acpi_parse_madt_msi() in gicv2m_teardown() since we would need to iterate through all existing MSI frame to clean up. >> + >> + return ret; >> +} >> + >> +int __init gicv2m_acpi_init(struct irq_domain *parent) >> +{ >> + int ret; >> + >> + if (acpi_num_msi > 0) >> + return 0; >> + >> + acpi_num_msi = >> acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_MSI_FRAME, >> + acpi_parse_madt_msi, 0); >> + >> + if (acpi_num_msi <= 0) >> + goto err_out; > If acpi_table_parse_madt return 0, then we don't need to call > gicv2m_teardown(). Instead we can simply return, optionally add some > pr_info. Well, gicv2m_teardown would do nothing, so this is just > cosmetic and up to you. I'd be hesitate to add pr_info here since V2m is optional, we already print information for each frame found. I think I would just leave this one the way it is. >> [..] >> diff --git a/include/linux/irqchip/arm-gic.h >> b/include/linux/irqchip/arm-gic.h >> index bae69e5..7398538 100644 >> --- a/include/linux/irqchip/arm-gic.h >> +++ b/include/linux/irqchip/arm-gic.h >> @@ -108,6 +108,12 @@ void gic_init(unsigned int nr, int start, >> >> int gicv2m_of_init(struct device_node *node, struct irq_domain >> *parent); >> >> +#ifdef CONFIG_ACPI >> +#include <linux/acpi.h> > I think, we don't need this include here. You already added it to itq-gic.c Right. Thanks, Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists