Messages in this thread Patch in this message | | | Date | Wed, 22 Jan 2020 10:44:06 +0000 | From | Marc Zyngier <> | Subject | Re: [PATCH] irqchip/gic-v3-its: Don't confuse get_vlpi_map() by writing DB config |
| |
Hi Zenghui,
Thanks for this.
On 2020-01-22 08:56, Zenghui Yu wrote: > When we're writing config for the doorbell interrupt, get_vlpi_map() > will > get confused by doorbell's d->parent_data hack and find the wrong > its_dev > as chip data and the wrong event. > > Fix this issue by making sure no doorbells will be involved before > invoking > get_vlpi_map(), which restore some of the logic in lpi_write_config(). > > Fixes: c1d4d5cd203c ("irqchip/gic-v3-its: Add its_vlpi_map helpers") > Signed-off-by: Zenghui Yu <yuzenghui@huawei.com> > --- > > This is based on mainline and can't be directly applied to the current > irqchip-next. > > drivers/irqchip/irq-gic-v3-its.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/irqchip/irq-gic-v3-its.c > b/drivers/irqchip/irq-gic-v3-its.c > index e05673bcd52b..cc8a4fcbd6d6 100644 > --- a/drivers/irqchip/irq-gic-v3-its.c > +++ b/drivers/irqchip/irq-gic-v3-its.c > @@ -1181,12 +1181,13 @@ static struct its_vlpi_map > *get_vlpi_map(struct irq_data *d) > > static void lpi_write_config(struct irq_data *d, u8 clr, u8 set) > { > - struct its_vlpi_map *map = get_vlpi_map(d); > irq_hw_number_t hwirq; > void *va; > u8 *cfg; > > - if (map) { > + if (irqd_is_forwarded_to_vcpu(d)) { > + struct its_vlpi_map *map = get_vlpi_map(d); > + > va = page_address(map->vm->vprop_page); > hwirq = map->vintid;
Shouldn't we fix get_vlpi_map() instead? Something like (untested):
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c index e05673bcd52b..b704214390c0 100644 --- a/drivers/irqchip/irq-gic-v3-its.c +++ b/drivers/irqchip/irq-gic-v3-its.c @@ -1170,13 +1170,14 @@ static void its_send_vclear(struct its_device *dev, u32 event_id) */ static struct its_vlpi_map *get_vlpi_map(struct irq_data *d) { - struct its_device *its_dev = irq_data_get_irq_chip_data(d); - u32 event = its_get_event_id(d); + if (irqd_is_forwarded_to_vcpu(d)) { + struct its_device *its_dev = irq_data_get_irq_chip_data(d); + u32 event = its_get_event_id(d);
- if (!irqd_is_forwarded_to_vcpu(d)) - return NULL; + return dev_event_to_vlpi_map(its_dev, event); + }
- return dev_event_to_vlpi_map(its_dev, event); + return NULL; }
static void lpi_write_config(struct irq_data *d, u8 clr, u8 set)
Or am I missing the actual problem?
Overall, I'm starting to hate that ->parent hack as it's been the source of a number of bugs.
The main issue is that the VPE hierarchy is missing one level (it has no ITS domain, and goes directly from the VPE domain to the low-level GIC domain). It means we end-up special-casing things, and that's never good...
M. -- Jazz is not dead. It just smells funny...
| |