On Mon, Oct 12, 2015 at 3:49 AM, Marc Zyngier <[email protected]> wrote: > On 11/10/15 18:13, Duc Dang wrote: >> On Fri, Oct 9, 2015 at 2:10 AM, Marc Zyngier <[email protected]> wrote: >>> Hi Duc, >>> >>> On 08/10/15 08:48, Duc Dang wrote: >>>> GICv2m driver currently only supports single v2m frame. This >>>> patch extend this driver to support multiple v2m frames. All of >>>> the v2m frames will be own by a single MSI domain. Each PCIe node >>>> can specify msi-parent as the first frame of the v2m frame list >>>> to be able to use all available v2m frames for MSI interrupts. >>>> >>>> This patch should be applied on top of patch >>>> (irqchip/GICv2m: Add workaround for APM X-Gene GICv2m erratum): >>>> https://lkml.org/lkml/2015/10/6/922 >>>> >>>> Signed-off-by: Duc Dang <[email protected]> >>>> --- >>>> drivers/irqchip/irq-gic-v2m.c | 221 >>>> ++++++++++++++++++++++++++++++------------ >>>> 1 file changed, 159 insertions(+), 62 deletions(-) >>>> >>>> diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c >>>> index 4c17c18..8ecaf9e 100644 >>>> --- a/drivers/irqchip/irq-gic-v2m.c >>>> +++ b/drivers/irqchip/irq-gic-v2m.c >>>> @@ -51,13 +51,19 @@ >>>> #define GICV2M_NEEDS_SPI_OFFSET 0x00000001 >>>> >>>> struct v2m_data { >>>> - spinlock_t msi_cnt_lock; >>>> struct resource res; /* GICv2m resource */ >>>> void __iomem *base; /* GICv2m virt address */ >>>> u32 spi_start; /* The SPI number that MSIs start */ >>>> u32 nr_spis; /* The number of SPIs for MSIs */ >>>> - unsigned long *bm; /* MSI vector bitmap */ >>>> u32 flags; /* v2m flags for specific implementation */ >>>> + struct list_head list; /* Link to other v2m frames */ >>>> +}; >>>> + >>>> +struct gicv2m_ctrlr { >>>> + spinlock_t v2m_ctrlr_lock; /* Lock for all v2m frames */ >>>> + u32 nr_vecs; /* Total MSI vectors */ >>>> + unsigned long *bm; /* MSI vector bitmap */ >>>> + struct list_head v2m_frms; /* List of v2m frames */ >>>> }; >>>> >>>> static void gicv2m_mask_msi_irq(struct irq_data *d) >>>> @@ -98,11 +104,29 @@ static int gicv2m_set_affinity(struct irq_data >>>> *irq_data, >>>> return ret; >>>> } >>>> >>>> +static struct v2m_data *irq_data_to_v2m_frm(struct irq_data *data, >>>> + struct gicv2m_ctrlr *v2m_ctrlr) >>>> +{ >>>> + struct v2m_data *v2m; >>>> + >>>> + list_for_each_entry(v2m, &v2m_ctrlr->v2m_frms, list) { >>>> + if ((data->hwirq >= v2m->spi_start) && >>>> + (data->hwirq < (v2m->spi_start + v2m->nr_spis))) >>>> + return v2m; >>>> + } >>>> + >>>> + return NULL; >>>> +} >>>> + >>>> static void gicv2m_compose_msi_msg(struct irq_data *data, struct msi_msg >>>> *msg) >>>> { >>>> - struct v2m_data *v2m = irq_data_get_irq_chip_data(data); >>>> - phys_addr_t addr = v2m->res.start + V2M_MSI_SETSPI_NS; >>>> + struct gicv2m_ctrlr *v2m_ctrlr = irq_data_get_irq_chip_data(data); >>>> + struct v2m_data *v2m; >>>> + phys_addr_t addr; >>>> >>>> + v2m = irq_data_to_v2m_frm(data, v2m_ctrlr); >>>> + WARN_ON(!v2m); >>>> + addr = v2m->res.start + V2M_MSI_SETSPI_NS; >>> >>> At that particular point, I've stopped reading. >>> >>> Pointlessly iterating over the frames when you can store the right one >>> in irq_data was bad enough, but having a WARN_ON() followed by a panic >>> is just not acceptable. Either you warn and avoid the crash, or you just >>> assume that the whole thing is sane and will never be inconsistent. >> >> Yes, my bad. >>> >>> You are also changing the way this driver works, and not for the best. >>> I've just written something fairly similar, with the following diffstat: >>> >>> 1 file changed, 76 insertions(+), 45 deletions(-) >>> >>> The current infrastructure is sound, you just have to make use of it. >>> >>> I've pushed the following (untested) patch: >>> >>> https://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/commit/?h=irq/multi-v2m&id=45e35adb60087792626570ff21bb23ab0f67a6ac >>> >>> Let me know if that works for you. >> >> Yes! I try and your patch definitely works on my X-Gene 2 board. > > OK, I'll post that for review, together with your Tested-by tag.
Thanks a lot, Marc. > > Thanks, > > M. > -- > Jazz is not dead. It just smells funny... Regards, Duc Dang. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/

