Message-ID: <874jia1ugj.ffs@tglx>
Date:   Sat, 28 Oct 2023 20:18:52 +0200
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Björn Töpel <bjorn@...nel.org>,
        Anup Patel <apatel@...tanamicro.com>
Cc:     Palmer Dabbelt <palmer@...belt.com>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Frank Rowand <frowand.list@...il.com>,
        Conor Dooley <conor+dt@...nel.org>,
        Marc Zyngier <maz@...nel.org>,
        Atish Patra <atishp@...shpatra.org>,
        Andrew Jones <ajones@...tanamicro.com>,
        Sunil V L <sunilvl@...tanamicro.com>,
        Saravana Kannan <saravanak@...gle.com>,
        Anup Patel <anup@...infault.org>,
        linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org
Subject: Re: [PATCH v11 07/14] irqchip: Add RISC-V incoming MSI controller
 early driver

On Thu, Oct 26 2023 at 10:51, Björn Töpel wrote:
>>> >> > +     raw_spin_lock_irqsave(&lpriv->ids_lock, flags);
>>> >> > +     bitmap_clear(lpriv->ids_enabled_bitmap, vec->local_id, 1);
>>> >> > +     raw_spin_unlock_irqrestore(&lpriv->ids_lock, flags);
>>> >> > +
>>> >> > +     imsic_remote_sync(vec->cpu);
>>> >>
>>> >> x86 seems to set a timer instead, for the remote cpu cleanup, which can
>>> >> be much cheaper, and less in instrusive. Is that applicable here?
>>> >
>>> > The issue with that approach is deciding the right duration
>>> > of timer interrupt. There might be platforms who need
>>> > immediate mask/unmask response. We can certainely
>>> > keep improving/tuning this over-time.
>>>
>>> Any concrete examples where this is an actual problem?
>>
>> Do you have a concrete timer duration with proper justification ?
>
> I would simply mimic what x86 does for now -- jiffies + 1.

That's good enough. The point is that the interrupt might still end up
on the old target CPU depending on timing, but the next one is
guaranteed to be targeted to the new target CPU.

So you can't cleanup the vector on the old target immediately, but it
does not matter at all whether you clean it up 10ms or 10s later. It's
just wasting a vector on the old target.

Doing it with an IPI (as x86 did before) only works when the IPI vector
is of lower priority than the vector which got moved. Otherwise the IPI
will be served first, find the vector pending and then it's up a creek
without a paddle because it can't retrigger the IPI as that would again
be served first. So it can't clean up ever...

The timer just avoids this and as I said the delay is completely
irrelevant.

>>> >> The reason I'm asking is because I'm pretty certain that x86 has proper
>>> >> MSI support (Thomas Gleixner can answer for sure! ;-))

It has proper MSI support with some limitations.

>>> >> IMSIC smells a lot like the the LAPIC.

Eeew. :)

> My claim is that x86 does support legacy-MSI, but for design decision,
> has avoided multi-MSI.

There are two variants of PCI/MSI:

  1) MSI
  2) MSI-X

Neither of them is legacy and both support multiple vectors at the
device hardware level.

  #1 MSI

      Affinity setting requires to move all vectors to the new target in
      one go because the device gets only the base vector in the MSI
      message and uses the lower bits as index.

      So that's of limited use anyway because it's impossible to use
      that for multi-queue or other purposes where the main point is to
      spread the interrupts accross CPUs.

      It does not have mandatory masking which makes affinity changes
      even more problematic at least on x86 because the update to the
      message store in the PCI config space is non-atomic. See the dance
      which is required for a single vector in msi_set_affity().

      IOW, if the MSI message is directly delivered to the target CPU
      and the device does not support masking then single vector is
      already complext and multi-MSI support becomes a horrorshow.

      Another issue especially on x86 with the limitation of about 200
      device vectors per CPU is the requirement to allocate a
      consecutive vector space power of 2 aligned. That's pretty fast at
      the point of vector exhaustion.

      That _are_ the reasons why X86 does not support multi-MSI without
      interrupt remapping. It just does the only sane thing and limits
      to one vector per device.

      Interrupt remapping avoids the problem because it allows to steer
      the vectors individually and the affinity update is atomic. It
      obviously also lifts the requirement for a consecutive vector
      space.

      Serioulsy w/o interrupt remapping or an equivalent translation
      mechanism which allows to steer the vectors individually multi-MSI
      is absolutely pointless and not worth the trouble to support it.


  #2 MSI-X

       Has a message store per vector and mandatory per vector masking
       which makes multi vector support trivial even w/o interrupt
       remapping. It does neither require a consecutive vector space.

So if AIA is similar to the APIC, then single MSI needs the same dance
and multi-MSI needs that theatre ^ N.

> AFAIU, there are few multi-MSI devices out there.

You wish. MSI-X is "more expensive" (probaly 0.5 Cent). Now that
interrupt remapping is pretty much always available on x86, the problem
is "fixed" indirectly. So especially x86 on-chip devices still use MSI
and not MSI-X. MSI-X is primarily used in multi-queue devices as
multi-MSI is limited to 32 vectors.

Thanks,

        tglx

Powered by blists - more mailing lists