Messages in this thread | | | Date | Wed, 19 Aug 2020 14:42:48 +0100 | From | Marc Zyngier <> | Subject | Re: [PATCH 1/2] irqchip: irq-mst: Add MStar interrupt controller support |
| |
On 2020-08-19 14:31, Mark-PK Tsai wrote: > From: Marc Zyngier <maz@kernel.org> > >> > + >> > +static int mst_intc_domain_alloc(struct irq_domain *domain, unsigned >> > int virq, >> > + unsigned int nr_irqs, void *data) >> > +{ >> > + int i; >> > + irq_hw_number_t hwirq; >> > + struct irq_fwspec parent_fwspec, *fwspec = data; >> > + struct mst_intc_chip_data *cd = (struct mst_intc_chip_data >> > *)domain->host_data; >> >> No cast necessary here. >> >> > + >> > + /* Not GIC compliant */ >> > + if (fwspec->param_count != 3) >> > + return -EINVAL; >> > + >> > + /* No PPI should point to this domain */ >> > + if (fwspec->param[0]) >> > + return -EINVAL; >> > + >> > + if (fwspec->param[1] >= cd->nr_irqs) >> >> This condition is bogus, as it doesn't take into account the nr_irqs >> parameter. >> > > > The hwirq number need to be in the irq map range. (property: > mstar,irqs-map-range) > If it's not, it must be incorrect configuration.
I agree. And since you are checking whether the configuration is correct, it'd better be completely correct.
> So how about use the condition as following? > > if (hwirq >= cd->nr_irqs) > return -EINVAL;
Again, this says nothing of the validity of (hwirq + nr_irqs - 1)...
> >> > + return -EINVAL; >> > + >> > + hwirq = fwspec->param[1]; >> > + for (i = 0; i < nr_irqs; i++) >> > + irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i, >> > + &mst_intc_chip, >> > + domain->host_data);
... which you are using here.
M. -- Jazz is not dead. It just smells funny...
| |