Re: [PATCH v15 08/10] irqchip/riscv-aplic: Add support for MSI-mode

From: Anup Patel <apatel@ventanamicro.com>
To: "Björn Töpel" <bjorn@kernel.org>
Cc: Palmer Dabbelt <palmer@dabbelt.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	 Thomas Gleixner <tglx@linutronix.de>,
	Rob Herring <robh+dt@kernel.org>,
	 Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Frank Rowand <frowand.list@gmail.com>,
	 Conor Dooley <conor+dt@kernel.org>,
	devicetree@vger.kernel.org,
	 Saravana Kannan <saravanak@google.com>,
	Marc Zyngier <maz@kernel.org>, Anup Patel <anup@brainfault.org>,
	 linux-kernel@vger.kernel.org,
	Atish Patra <atishp@atishpatra.org>,
	 linux-riscv@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org,
	 Andrew Jones <ajones@ventanamicro.com>
Subject: Re: [PATCH v15 08/10] irqchip/riscv-aplic: Add support for MSI-mode
Date: Wed, 6 Mar 2024 21:44:01 +0530	[thread overview]
Message-ID: <CAK9=C2Ud+CJzWfY0Lp97OMt9QvJBFX=hHJidn_90XY5cEB9LHw@mail.gmail.com> (raw)
In-Reply-To: <87y1avbboj.fsf@all.your.base.are.belong.to.us>

On Wed, Mar 6, 2024 at 9:22 PM Björn Töpel <bjorn@kernel.org> wrote:
>
> Anup Patel <apatel@ventanamicro.com> writes:
>
> > diff --git a/drivers/irqchip/irq-riscv-aplic-msi.c b/drivers/irqchip/irq-riscv-aplic-msi.c
> > new file mode 100644
> > index 000000000000..b2a25e011bb2
> > --- /dev/null
> > +++ b/drivers/irqchip/irq-riscv-aplic-msi.c
> > +static void aplic_msi_write_msg(struct irq_data *d, struct msi_msg *msg)
> > +{
> > +     unsigned int group_index, hart_index, guest_index, val;
> > +     struct aplic_priv *priv = irq_data_get_irq_chip_data(d);
> > +     struct aplic_msicfg *mc = &priv->msicfg;
> > +     phys_addr_t tppn, tbppn, msg_addr;
> > +     void __iomem *target;
> > +
> > +     /* For zeroed MSI, simply write zero into the target register */
> > +     if (!msg->address_hi && !msg->address_lo && !msg->data) {
> > +             target = priv->regs + APLIC_TARGET_BASE;
> > +             target += (d->hwirq - 1) * sizeof(u32);
> > +             writel(0, target);
>
> Is the fence needed here (writel_relaxed())...

The pci_write_msg_msix() (called via pci_msi_domain_write_msg())
uses writel() hence taking inspiration from that we use writel() over here
as well.

If that's wrong then pci_write_msg_msix() must be fixed as well.

>
> > +             return;
> > +     }
> > +
> > +     /* Sanity check on message data */
> > +     WARN_ON(msg->data > APLIC_TARGET_EIID_MASK);
> > +
> > +     /* Compute target MSI address */
> > +     msg_addr = (((u64)msg->address_hi) << 32) | msg->address_lo;
> > +     tppn = msg_addr >> APLIC_xMSICFGADDR_PPN_SHIFT;
> > +
> > +     /* Compute target HART Base PPN */
> > +     tbppn = tppn;
> > +     tbppn &= ~APLIC_xMSICFGADDR_PPN_HART(mc->lhxs);
> > +     tbppn &= ~APLIC_xMSICFGADDR_PPN_LHX(mc->lhxw, mc->lhxs);
> > +     tbppn &= ~APLIC_xMSICFGADDR_PPN_HHX(mc->hhxw, mc->hhxs);
> > +     WARN_ON(tbppn != mc->base_ppn);
> > +
> > +     /* Compute target group and hart indexes */
> > +     group_index = (tppn >> APLIC_xMSICFGADDR_PPN_HHX_SHIFT(mc->hhxs)) &
> > +                  APLIC_xMSICFGADDR_PPN_HHX_MASK(mc->hhxw);
> > +     hart_index = (tppn >> APLIC_xMSICFGADDR_PPN_LHX_SHIFT(mc->lhxs)) &
> > +                  APLIC_xMSICFGADDR_PPN_LHX_MASK(mc->lhxw);
> > +     hart_index |= (group_index << mc->lhxw);
> > +     WARN_ON(hart_index > APLIC_TARGET_HART_IDX_MASK);
> > +
> > +     /* Compute target guest index */
> > +     guest_index = tppn & APLIC_xMSICFGADDR_PPN_HART(mc->lhxs);
> > +     WARN_ON(guest_index > APLIC_TARGET_GUEST_IDX_MASK);
> > +
> > +     /* Update IRQ TARGET register */
> > +     target = priv->regs + APLIC_TARGET_BASE;
> > +     target += (d->hwirq - 1) * sizeof(u32);
> > +     val = FIELD_PREP(APLIC_TARGET_HART_IDX, hart_index);
> > +     val |= FIELD_PREP(APLIC_TARGET_GUEST_IDX, guest_index);
> > +     val |= FIELD_PREP(APLIC_TARGET_EIID, msg->data);
> > +     writel(val, target);
>
> ...and here?

Same as above.

Regards,
Anup

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv