| Message ID | 20210525173255.620606-9-valentin.schneider@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | irqchip/irq-gic: Optimize masking by leveraging EOImode=1 | expand |
On Tue, 25 May 2021 18:32:53 +0100, Valentin Schneider <valentin.schneider@arm.com> wrote: > > Subsequent patches will make the GIC irqchips use a flow handler that > issues an ->irq_ack(). irqchips of child domains need to handle this. > > Note: I'm very much not fond of this; this is treacherous and explodes if > any parent chip doesn't have an ->ack() callback. It turns out okay with > EOImode=0 because handle_fasteoi_irq() doesn't issue any ->ack(), but that > is very fragile at best. > > An alternative would be to > o make irq_chip_ack_parent() check the callback against NULL That's an overhead I'd like to avoid in the general case, given that we already have a bunch of users. > o make irq_chip_ack_parent() the default chip->irq_ack() via > MSI_FLAG_USE_DEF_CHIP_OPS. Seem like a reasonable approach: how about a custom irq_ack() callback that iterates over the hierarchy until it finds an a non-NULL entry? Flows that don't use ack won't be impacted, users that need ack will provide one if they want, and the default will do something slightly slower, but at least unsurprising. > XXX: what about pMSI and fMSI ? Same thing. They are just bus-specific domains on top of the ITS domain, and must follow the same convention. However, this patch is perfectly acceptable to me (as long as you take care of platform and fsl -MSI). Thanks, M.
On 27/05/21 13:17, Marc Zyngier wrote: > On Tue, 25 May 2021 18:32:53 +0100, > Valentin Schneider <valentin.schneider@arm.com> wrote: >> o make irq_chip_ack_parent() the default chip->irq_ack() via >> MSI_FLAG_USE_DEF_CHIP_OPS. > > Seem like a reasonable approach: how about a custom irq_ack() callback > that iterates over the hierarchy until it finds an a non-NULL entry? > Flows that don't use ack won't be impacted, users that need ack will > provide one if they want, and the default will do something slightly > slower, but at least unsurprising. > Sounds about right! >> XXX: what about pMSI and fMSI ? > > Same thing. They are just bus-specific domains on top of the ITS > domain, and must follow the same convention. > > However, this patch is perfectly acceptable to me (as long as you take > care of platform and fsl -MSI). > Noted. > Thanks, > > M. > > -- > Without deviation from the norm, progress is not possible.
diff --git a/drivers/irqchip/irq-gic-v3-its-pci-msi.c b/drivers/irqchip/irq-gic-v3-its-pci-msi.c index ad2810c017ed..5bc2787ee86a 100644 --- a/drivers/irqchip/irq-gic-v3-its-pci-msi.c +++ b/drivers/irqchip/irq-gic-v3-its-pci-msi.c @@ -27,6 +27,7 @@ static struct irq_chip its_msi_irq_chip = { .name = "ITS-MSI", .irq_unmask = its_unmask_msi_irq, .irq_mask = its_mask_msi_irq, + .irq_ack = irq_chip_ack_parent, .irq_eoi = irq_chip_eoi_parent, .irq_write_msi_msg = pci_msi_domain_write_msg, }; diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c index 2e6923c2c8a8..ce39d52409e9 100644 --- a/drivers/irqchip/irq-gic-v3-its.c +++ b/drivers/irqchip/irq-gic-v3-its.c @@ -1976,6 +1976,7 @@ static struct irq_chip its_irq_chip = { .name = "ITS", .irq_mask = its_mask_irq, .irq_unmask = its_unmask_irq, + .irq_ack = irq_chip_ack_parent, .irq_eoi = irq_chip_eoi_parent, .irq_set_affinity = its_set_affinity, .irq_compose_msi_msg = its_irq_compose_msi_msg,
Subsequent patches will make the GIC irqchips use a flow handler that issues an ->irq_ack(). irqchips of child domains need to handle this. Note: I'm very much not fond of this; this is treacherous and explodes if any parent chip doesn't have an ->ack() callback. It turns out okay with EOImode=0 because handle_fasteoi_irq() doesn't issue any ->ack(), but that is very fragile at best. An alternative would be to o make irq_chip_ack_parent() check the callback against NULL o make irq_chip_ack_parent() the default chip->irq_ack() via MSI_FLAG_USE_DEF_CHIP_OPS. XXX: what about pMSI and fMSI ? Signed-off-by: Valentin Schneider <valentin.schneider@arm.com> --- drivers/irqchip/irq-gic-v3-its-pci-msi.c | 1 + drivers/irqchip/irq-gic-v3-its.c | 1 + 2 files changed, 2 insertions(+)