* [PATCH] irqchip/gic-v3-its: Flush GICR caching for a cross node collection move of an irq @ 2017-12-20 9:15 Ganapatrao Kulkarni 2017-12-20 9:26 ` Marc Zyngier 0 siblings, 1 reply; 6+ messages in thread From: Ganapatrao Kulkarni @ 2017-12-20 9:15 UTC (permalink / raw) To: linux-arm-kernel When an interrupt is moved, it is possible that an implementation that supports caching might still have cached data for a previous (no longer valid) mapping of the interrupt. In particular, in a distributed GIC implementation like multi-socket SoC platfroms. Hence it is necessary to flush cached entries after cross node collection migration. Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com> --- drivers/irqchip/irq-gic-v3-its.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c index 4039e64..ea849a1 100644 --- a/drivers/irqchip/irq-gic-v3-its.c +++ b/drivers/irqchip/irq-gic-v3-its.c @@ -1119,6 +1119,12 @@ static int its_set_affinity(struct irq_data *d, const struct cpumask *mask_val, if (cpu != its_dev->event_map.col_map[id]) { target_col = &its_dev->its->collections[cpu]; its_send_movi(its_dev, target_col, id); + /* Issue INV for cross node collection move on + * multi socket systems. + */ + if (cpu_to_node(cpu) != + cpu_to_node(its_dev->event_map.col_map[id])) + its_send_inv(its_dev, id); its_dev->event_map.col_map[id] = cpu; irq_data_update_effective_affinity(d, cpumask_of(cpu)); } -- 2.9.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] irqchip/gic-v3-its: Flush GICR caching for a cross node collection move of an irq 2017-12-20 9:15 [PATCH] irqchip/gic-v3-its: Flush GICR caching for a cross node collection move of an irq Ganapatrao Kulkarni @ 2017-12-20 9:26 ` Marc Zyngier 2017-12-20 9:34 ` Ganapatrao Kulkarni 0 siblings, 1 reply; 6+ messages in thread From: Marc Zyngier @ 2017-12-20 9:26 UTC (permalink / raw) To: linux-arm-kernel On 20/12/17 09:15, Ganapatrao Kulkarni wrote: > When an interrupt is moved, it is possible that an implementation that > supports caching might still have cached data for a previous > (no longer valid) mapping of the interrupt. In particular, in a distributed > GIC implementation like multi-socket SoC platfroms. Hence it is necessary > to flush cached entries after cross node collection migration. > > Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com> > --- > drivers/irqchip/irq-gic-v3-its.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c > index 4039e64..ea849a1 100644 > --- a/drivers/irqchip/irq-gic-v3-its.c > +++ b/drivers/irqchip/irq-gic-v3-its.c > @@ -1119,6 +1119,12 @@ static int its_set_affinity(struct irq_data *d, const struct cpumask *mask_val, > if (cpu != its_dev->event_map.col_map[id]) { > target_col = &its_dev->its->collections[cpu]; > its_send_movi(its_dev, target_col, id); > + /* Issue INV for cross node collection move on > + * multi socket systems. > + */ > + if (cpu_to_node(cpu) != > + cpu_to_node(its_dev->event_map.col_map[id])) > + its_send_inv(its_dev, id); > its_dev->event_map.col_map[id] = cpu; > irq_data_update_effective_affinity(d, cpumask_of(cpu)); > } > The MOVI command doesn't have any such requirement (it only mandates synchronization), and doesn't say anything about distributed vs monolithic. What am I missing? M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] irqchip/gic-v3-its: Flush GICR caching for a cross node collection move of an irq 2017-12-20 9:26 ` Marc Zyngier @ 2017-12-20 9:34 ` Ganapatrao Kulkarni 2017-12-20 9:49 ` Marc Zyngier 2017-12-20 13:12 ` Marc Zyngier 0 siblings, 2 replies; 6+ messages in thread From: Ganapatrao Kulkarni @ 2017-12-20 9:34 UTC (permalink / raw) To: linux-arm-kernel Hi Marc, On Wed, Dec 20, 2017 at 2:56 PM, Marc Zyngier <marc.zyngier@arm.com> wrote: > On 20/12/17 09:15, Ganapatrao Kulkarni wrote: >> When an interrupt is moved, it is possible that an implementation that >> supports caching might still have cached data for a previous >> (no longer valid) mapping of the interrupt. In particular, in a distributed >> GIC implementation like multi-socket SoC platfroms. Hence it is necessary >> to flush cached entries after cross node collection migration. >> >> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com> >> --- >> drivers/irqchip/irq-gic-v3-its.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c >> index 4039e64..ea849a1 100644 >> --- a/drivers/irqchip/irq-gic-v3-its.c >> +++ b/drivers/irqchip/irq-gic-v3-its.c >> @@ -1119,6 +1119,12 @@ static int its_set_affinity(struct irq_data *d, const struct cpumask *mask_val, >> if (cpu != its_dev->event_map.col_map[id]) { >> target_col = &its_dev->its->collections[cpu]; >> its_send_movi(its_dev, target_col, id); >> + /* Issue INV for cross node collection move on >> + * multi socket systems. >> + */ >> + if (cpu_to_node(cpu) != >> + cpu_to_node(its_dev->event_map.col_map[id])) >> + its_send_inv(its_dev, id); >> its_dev->event_map.col_map[id] = cpu; >> irq_data_update_effective_affinity(d, cpumask_of(cpu)); >> } >> > > The MOVI command doesn't have any such requirement (it only mandates > synchronization), and doesn't say anything about distributed vs monolithic. GIC-v3 spec do mention to issue ITS INV command or a write to GICR_INVLPIR. pasting below snippet of MOVI command description. "When an interrupt is moved to a collection, it is possible that an implementation that supports speculative caching might still have cached data for a previous (no longer valid) mapping of the interrupt. Hence, implementations must take care to invalidate any data associated with an interrupt when it is moved. In particular, in a distributed implementation, the ITS must write to the appropriate GICR_* register to perform the invalidation in the redistributor." > > What am I missing? > > M. > -- > Jazz is not dead. It just smells funny... thanks Ganapat ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] irqchip/gic-v3-its: Flush GICR caching for a cross node collection move of an irq 2017-12-20 9:34 ` Ganapatrao Kulkarni @ 2017-12-20 9:49 ` Marc Zyngier 2017-12-20 13:12 ` Marc Zyngier 1 sibling, 0 replies; 6+ messages in thread From: Marc Zyngier @ 2017-12-20 9:49 UTC (permalink / raw) To: linux-arm-kernel On 20/12/17 09:34, Ganapatrao Kulkarni wrote: > Hi Marc, > > On Wed, Dec 20, 2017 at 2:56 PM, Marc Zyngier <marc.zyngier@arm.com> wrote: >> On 20/12/17 09:15, Ganapatrao Kulkarni wrote: >>> When an interrupt is moved, it is possible that an implementation that >>> supports caching might still have cached data for a previous >>> (no longer valid) mapping of the interrupt. In particular, in a distributed >>> GIC implementation like multi-socket SoC platfroms. Hence it is necessary >>> to flush cached entries after cross node collection migration. >>> >>> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com> >>> --- >>> drivers/irqchip/irq-gic-v3-its.c | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c >>> index 4039e64..ea849a1 100644 >>> --- a/drivers/irqchip/irq-gic-v3-its.c >>> +++ b/drivers/irqchip/irq-gic-v3-its.c >>> @@ -1119,6 +1119,12 @@ static int its_set_affinity(struct irq_data *d, const struct cpumask *mask_val, >>> if (cpu != its_dev->event_map.col_map[id]) { >>> target_col = &its_dev->its->collections[cpu]; >>> its_send_movi(its_dev, target_col, id); >>> + /* Issue INV for cross node collection move on >>> + * multi socket systems. >>> + */ >>> + if (cpu_to_node(cpu) != >>> + cpu_to_node(its_dev->event_map.col_map[id])) >>> + its_send_inv(its_dev, id); >>> its_dev->event_map.col_map[id] = cpu; >>> irq_data_update_effective_affinity(d, cpumask_of(cpu)); >>> } >>> >> >> The MOVI command doesn't have any such requirement (it only mandates >> synchronization), and doesn't say anything about distributed vs monolithic. > > GIC-v3 spec do mention to issue ITS INV command or a write to GICR_INVLPIR. > pasting below snippet of MOVI command description. > > "When an interrupt is moved to a collection, it is possible that an > implementation that supports speculative caching > might still have cached data for a previous (no longer valid) mapping > of the interrupt. Hence, implementations > must take care to invalidate any data associated with an interrupt > when it is moved. In particular, in a distributed > implementation, the ITS must write to the appropriate GICR_* register > to perform the invalidation in the redistributor." Which document is that from? The only official document that should be used is the GICv3/v4 Architecture Specification[1], which doesn't contain that text. Thanks, M. [1]: https://developer.arm.com/products/architecture/a-profile/docs/ihi0069/latest/arm-generic-interrupt-controller-architecture-specification-gic-architecture-version-30-and-40 -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] irqchip/gic-v3-its: Flush GICR caching for a cross node collection move of an irq 2017-12-20 9:34 ` Ganapatrao Kulkarni 2017-12-20 9:49 ` Marc Zyngier @ 2017-12-20 13:12 ` Marc Zyngier 2017-12-21 6:49 ` Ganapatrao Kulkarni 1 sibling, 1 reply; 6+ messages in thread From: Marc Zyngier @ 2017-12-20 13:12 UTC (permalink / raw) To: linux-arm-kernel On 20/12/17 09:34, Ganapatrao Kulkarni wrote: > Hi Marc, > > On Wed, Dec 20, 2017 at 2:56 PM, Marc Zyngier <marc.zyngier@arm.com> wrote: >> On 20/12/17 09:15, Ganapatrao Kulkarni wrote: >>> When an interrupt is moved, it is possible that an implementation that >>> supports caching might still have cached data for a previous >>> (no longer valid) mapping of the interrupt. In particular, in a distributed >>> GIC implementation like multi-socket SoC platfroms. Hence it is necessary >>> to flush cached entries after cross node collection migration. >>> >>> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com> >>> --- >>> drivers/irqchip/irq-gic-v3-its.c | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c >>> index 4039e64..ea849a1 100644 >>> --- a/drivers/irqchip/irq-gic-v3-its.c >>> +++ b/drivers/irqchip/irq-gic-v3-its.c >>> @@ -1119,6 +1119,12 @@ static int its_set_affinity(struct irq_data *d, const struct cpumask *mask_val, >>> if (cpu != its_dev->event_map.col_map[id]) { >>> target_col = &its_dev->its->collections[cpu]; >>> its_send_movi(its_dev, target_col, id); >>> + /* Issue INV for cross node collection move on >>> + * multi socket systems. >>> + */ >>> + if (cpu_to_node(cpu) != >>> + cpu_to_node(its_dev->event_map.col_map[id])) >>> + its_send_inv(its_dev, id); >>> its_dev->event_map.col_map[id] = cpu; >>> irq_data_update_effective_affinity(d, cpumask_of(cpu)); >>> } >>> >> >> The MOVI command doesn't have any such requirement (it only mandates >> synchronization), and doesn't say anything about distributed vs monolithic. > > GIC-v3 spec do mention to issue ITS INV command or a write to GICR_INVLPIR. > pasting below snippet of MOVI command description. > > "When an interrupt is moved to a collection, it is possible that an > implementation that supports speculative caching > might still have cached data for a previous (no longer valid) mapping > of the interrupt. Hence, implementations > must take care to invalidate any data associated with an interrupt > when it is moved. In particular, in a distributed > implementation, the ITS must write to the appropriate GICR_* register > to perform the invalidation in the redistributor." Doing some documentation archaeology, I found that this wording has been dropped from the engineering specification in August 2014, and was never included in the architecture specification. I suggest you start using a slightly more up-to-date set of documentation... Now, back to your point: what it says in the bit of (confidential) document that you quoted is that the *HW* must perform the invalidation (that's what the words "implementations" and "ITS" refer to), not some random bits of SW. If you know of an implementation that suffers from this, please resend a patch that handles this as a quirk, with a proper erratum number. Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] irqchip/gic-v3-its: Flush GICR caching for a cross node collection move of an irq 2017-12-20 13:12 ` Marc Zyngier @ 2017-12-21 6:49 ` Ganapatrao Kulkarni 0 siblings, 0 replies; 6+ messages in thread From: Ganapatrao Kulkarni @ 2017-12-21 6:49 UTC (permalink / raw) To: linux-arm-kernel On Wed, Dec 20, 2017 at 6:42 PM, Marc Zyngier <marc.zyngier@arm.com> wrote: > On 20/12/17 09:34, Ganapatrao Kulkarni wrote: >> Hi Marc, >> >> On Wed, Dec 20, 2017 at 2:56 PM, Marc Zyngier <marc.zyngier@arm.com> wrote: >>> On 20/12/17 09:15, Ganapatrao Kulkarni wrote: >>>> When an interrupt is moved, it is possible that an implementation that >>>> supports caching might still have cached data for a previous >>>> (no longer valid) mapping of the interrupt. In particular, in a distributed >>>> GIC implementation like multi-socket SoC platfroms. Hence it is necessary >>>> to flush cached entries after cross node collection migration. >>>> >>>> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com> >>>> --- >>>> drivers/irqchip/irq-gic-v3-its.c | 6 ++++++ >>>> 1 file changed, 6 insertions(+) >>>> >>>> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c >>>> index 4039e64..ea849a1 100644 >>>> --- a/drivers/irqchip/irq-gic-v3-its.c >>>> +++ b/drivers/irqchip/irq-gic-v3-its.c >>>> @@ -1119,6 +1119,12 @@ static int its_set_affinity(struct irq_data *d, const struct cpumask *mask_val, >>>> if (cpu != its_dev->event_map.col_map[id]) { >>>> target_col = &its_dev->its->collections[cpu]; >>>> its_send_movi(its_dev, target_col, id); >>>> + /* Issue INV for cross node collection move on >>>> + * multi socket systems. >>>> + */ >>>> + if (cpu_to_node(cpu) != >>>> + cpu_to_node(its_dev->event_map.col_map[id])) >>>> + its_send_inv(its_dev, id); >>>> its_dev->event_map.col_map[id] = cpu; >>>> irq_data_update_effective_affinity(d, cpumask_of(cpu)); >>>> } >>>> >>> >>> The MOVI command doesn't have any such requirement (it only mandates >>> synchronization), and doesn't say anything about distributed vs monolithic. >> >> GIC-v3 spec do mention to issue ITS INV command or a write to GICR_INVLPIR. >> pasting below snippet of MOVI command description. >> >> "When an interrupt is moved to a collection, it is possible that an >> implementation that supports speculative caching >> might still have cached data for a previous (no longer valid) mapping >> of the interrupt. Hence, implementations >> must take care to invalidate any data associated with an interrupt >> when it is moved. In particular, in a distributed >> implementation, the ITS must write to the appropriate GICR_* register >> to perform the invalidation in the redistributor." > > Doing some documentation archaeology, I found that this wording has been > dropped from the engineering specification in August 2014, and was never > included in the architecture specification. I suggest you start using a > slightly more up-to-date set of documentation... thanks Marc for digging in to archive. > > Now, back to your point: what it says in the bit of (confidential) > document that you quoted is that the *HW* must perform the invalidation > (that's what the words "implementations" and "ITS" refer to), not some > random bits of SW. > > If you know of an implementation that suffers from this, please resend a > patch that handles this as a quirk, with a proper erratum number. Sure, this is being discussed internally and will repost as errata fix at the earliest. > > Thanks, > > M. > -- > Jazz is not dead. It just smells funny... thanks Ganapat ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-12-21 6:49 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-12-20 9:15 [PATCH] irqchip/gic-v3-its: Flush GICR caching for a cross node collection move of an irq Ganapatrao Kulkarni 2017-12-20 9:26 ` Marc Zyngier 2017-12-20 9:34 ` Ganapatrao Kulkarni 2017-12-20 9:49 ` Marc Zyngier 2017-12-20 13:12 ` Marc Zyngier 2017-12-21 6:49 ` Ganapatrao Kulkarni
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox