Re: [PATCH] irqchip/gic-v3-its: remove the shareability of ITS

[Marc Zyngier]

On Thu, 08 Dec 2022 16:58:20 +0000,
Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx> wrote:
> 
> [1  <text/plain; us-ascii (quoted-printable)>]
> Hi,
> 
> On Thu, Dec 08, 2022 at 10:28:29AM +0800, Harry Song wrote:
> > On Wed, Dec 7, 2022 at 11:19 PM Marc Zyngier <maz@xxxxxxxxxx> wrote:
> > >
> > > On Wed, 07 Dec 2022 13:52:23 +0000,
> > > Harry Song <jundongsong1@xxxxxxxxx> wrote:
> > > >
> > > > I know this is a very wrong patch, but my platform
> > > > has an abnormal ITS problem caused by data consistency:
> > > > My chip does not support Cache Coherent Interconnect (CCI).
> > >
> > > That doesn't mean much. Nothing mandates to have a CCI, and plenty of
> > > systems have other ways to maintain coherency.
> > >
> > > > By default, ITS driver is the inner memory attribute.
> > > > gits_write_cbaser() is used to write the inner memory
> > > > attribute. But hw doesn't return the hardware's non-shareable
> > > > property,so I don't think reading GITS_CBASER and GICR_PROPBASER
> > > > here will get the real property of the current hardware: inner
> > > > or outer shareable is not supported, so I would like to know
> > > > whether ITS driver cannot be used on chips without CCI, or
> > > > what method can be used to use ITS driver on chips without CCI?
> > >
> > > It's not about CCI or not CCI. It is about which shareability domain
> > > your ITS is in.
> > >
> > > And it doesn't only affect the ITS. It also affects the
> > > redistributors, and anything that accesses memory.
> > >
> > 
> > Yes, my current chip is Rockchip platform (rk3588), so is it because the chip
> > is not designed as a proper shared domain for ITS, so the exception of
> > ITS is caused?
> > 
> > > >
> > > > The current patch is designed to make ITS think that the current
> > > > chip has no inner or outer memory properties, and then use
> > > > its by flushing dcache.
> > > >
> > > > This is the log for bug reports without patches:
> > > >
> > > > [    0.000000] GICv3: CPU0: found redistributor 0 region 0:0x0000000003460000
> > > > [    0.000000] ITS [mem 0x03440000-0x0345ffff]
> > > > [    0.000000] ITS@0x0000000003440000: allocated 8192 Devices @41850000 (indirect, esz 8, psz 64K, shr 0)
> > > > [    0.000000] ITS@0x0000000003440000: allocated 32768 Interrupt Collections @41860000 (flat, esz 2, psz 64K, shr 0)
> > > > [    0.000000] GICv3: using LPI property table @0x0000000041870000
> > > > [    0.000000] GICv3: CPU0: using allocated LPI pending table @0x0000000041880000
> > > > [    0.000000] ITS queue timeout (64 1)
> > > > [    0.000000] ITS cmd its_build_mapc_cmd failed
> > > > [    0.000000] ITS queue timeout (128 1)
> > > > [    0.000000] ITS cmd its_build_invall_cmd failed
> > >
> > > Ah, this suspiciously looks like a Rockchip machine...
> > >
> > > >
> > > > Signed-off-by: Harry Song <jundongsong1@xxxxxxxxx>
> > > > ---
> > > >
> > > > I am very sorry to bother you. This problem has been bothering me
> > > > for several weeks.  I am looking forward to your reply.
> > >
> > > If you have such issue, this needs to be handled as per-platform
> > > quirk. I'm not putting such generic hacks in the driver.
> > >
> > >         M.
> > >
> > > --
> > > Without deviation from the norm, progress is not possible.
> > 
> > Are there currently other chip platforms that have this problem, and then ITS
> > is already compatible with the problem? Please tell me how to submit
> > hacks patch for rk3588 platform?
> > 
> > If the hacks patch cannot be submitted, please tell me whether it
> > requires the next generation
> > chip to have any design requirements for ITS?
> > 
> > Design ITS and CPU to be the same inner memory property?
> 
> Previous rockchip generation has the same bug and it got discussed
> here:
> 
> https://lore.kernel.org/lkml/871rbdt4tu.wl-maz@xxxxxxxxxx/T/#m5dbc70ff308d81e98dd0d797e23d3fbf9c353245
> 
> You can use this DT as base with mainline to avoid ITS:
> 
> https://lore.kernel.org/all/20221205172350.75234-1-sebastian.reichel@xxxxxxxxxxxxx/

Using the MBI region really isn't the same thing. You're limited to a
handful of MSI instead of 64k, and you don't get any device isolation.

Also, your usage of the GIC binding is pretty broken. A single PPI
partition covering all the CPUs, the two PMU having the same PPI and
#interrupt-calls=3, that's all completely wrong.

	M.

-- 
Without deviation from the norm, progress is not possible.