[RFC] ITS fails to allocate on rk3568/rk3566

[RFC] ITS fails to allocate on rk3568/rk3566

[Peter Geis]

Good Afternoon,

I am assisting with early bringup of the rk3566 based quartz64
development board for mainline linux.
I've encountered a few issues with allocating ITS on their version of
the GIC-V3.
The first issue is the ITS controller can only use 32bit addresses.
This leads to the following error:
[    0.000000] NR_IRQS: 64, nr_irqs: 64, preallocated irqs: 0
[    0.000000] GICv3: GIC: Using split EOI/Deactivate mode
[    0.000000] GICv3: 320 SPIs implemented
[    0.000000] GICv3: 0 Extended SPIs implemented
[    0.000000] GICv3: Distributor has no Range Selector support
[    0.000000] Root IRQ handler: gic_handle_irq
[    0.000000] GICv3: 16 PPIs implemented
[    0.000000] GICv3: CPU0: found redistributor 0 region 0:0x00000000fd460000
[    0.000000] ITS [mem 0xfd440000-0xfd45ffff]
[    0.000000] ITS@0x00000000fd440000: Devices doesn't stick:
f907000100190600 f907000000190600
[    0.000000] ITS@0x00000000fd440000: failed probing (-6)
[    0.000000] ITS: No ITS available, not enabling LPIs

Downstream fixed this by adding the GFP_DMA32 flag to the memory allocations.
They also force clear the GITS_BASER_SHAREABILITY_MASK.
Unfortunately while this allowed ITS to allocate on downstream, as
soon as MSIs attempted to use it all interrupts would time out.

On upstream, we observe this during allocation:
[    0.000000] ITS [mem 0xfd440000-0xfd45ffff]
[    0.000000] ITS@0x00000000fd440000: allocated 8192 Devices @3810000
(indirect, esz 8, psz 64K, shr 1)
[    0.000000] ITS@0x00000000fd440000: allocated 32768 Interrupt
Collections @3820000 (flat, esz 2, psz 64K, shr 1)
[    0.000000] GICv3: using LPI property table @0x0000000003830000
[    0.000000] GICv3: CPU0: using allocated LPI pending table
@0x0000000003840000
[    0.000000] ITS queue timeout (64 1)
[    0.000000] ITS cmd its_build_mapc_cmd failed
[    0.000000] ITS queue timeout (96 1)
[    0.000000] ITS cmd its_build_invall_cmd failed
<snip>
[    0.150282] Platform MSI: interrupt-controller@fd440000 domain created
[    0.152409] PCI/MSI:
/interrupt-controller@fd400000/interrupt-controller@fd440000 domain
created
[    0.156158] EFI services will not be available.
[    0.161168] smp: Bringing up secondary CPUs ...
[    0.170252] Detected VIPT I-cache on CPU1
[    0.170325] GICv3: CPU1: found redistributor 100 region 0:0x00000000fd480000
[    0.170356] GICv3: GIC_READ_PMR old_pmr=0xf0
[    0.170371] GICv3: GIC_READ_PMR new_pmr=0x0
[    0.170393] GICv3: CPU1: using allocated LPI pending table
@0x0000000003850000
[    1.686709] ITS queue timeout (160 1)
[    1.686733] ITS cmd its_build_mapc_cmd failed
[    3.202979] ITS queue timeout (192 1)
[    3.202999] ITS cmd its_build_invall_cmd failed
[    3.203157] CPU1: Booted secondary processor 0x0000000100 [0x412fd050]
[    3.208303] Callback from call_rcu_tasks_rude() invoked.
[    3.228487] Detected VIPT I-cache on CPU2
[    3.228559] GICv3: CPU2: found redistributor 200 region 0:0x00000000fd4a0000
[    3.228590] GICv3: GIC_READ_PMR old_pmr=0xf0
[    3.228606] GICv3: GIC_READ_PMR new_pmr=0x0
[    3.228630] GICv3: CPU2: using allocated LPI pending table
@0x0000000003860000
[    4.744675] ITS queue timeout (256 1)
[    4.744698] ITS cmd its_build_mapc_cmd failed
[    6.260797] ITS queue timeout (288 1)
[    6.260816] ITS cmd its_build_invall_cmd failed
[    6.260948] CPU2: Booted secondary processor 0x0000000200 [0x412fd050]
[    6.269204] Callback from call_rcu_tasks() invoked.
[    6.278358] Detected VIPT I-cache on CPU3
[    6.278430] GICv3: CPU3: found redistributor 300 region 0:0x00000000fd4c0000
[    6.278460] GICv3: GIC_READ_PMR old_pmr=0xf0
[    6.278476] GICv3: GIC_READ_PMR new_pmr=0x0
[    6.278501] GICv3: CPU3: using allocated LPI pending table
@0x0000000003870000
[    7.794367] ITS queue timeout (352 1)
[    7.794390] ITS cmd its_build_mapc_cmd failed
[    9.310219] ITS queue timeout (384 1)
[    9.310237] ITS cmd its_build_invall_cmd failed
[    9.310368] CPU3: Booted secondary processor 0x0000000300 [0x412fd050]
[    9.311925] smp: Brought up 1 node, 4 CPUs
[    9.318831] SMP: Total of 4 processors activated.

Also, when MSIs activate the same timeouts seen downstream happen:
[   13.884698] pci 0000:00:00.0: PCI bridge to [bus 01-ff]
[   13.886476] pcieport 0000:00:00.0: of_irq_parse_pci: failed with rc=-22
[   13.887466] pcieport 0000:00:00.0: assign IRQ: got 0
[   15.414727] ITS queue timeout (416 1)
[   15.415476] ITS cmd its_build_mapd_cmd failed
[   16.957519] ITS queue timeout (480 1)
[   16.958040] ITS cmd its_build_mapti_cmd failed
[   18.482251] ITS queue timeout (544 1)
[   18.482762] ITS cmd its_build_mapti_cmd failed
<snip>
[   70.267783] ITS queue timeout (2656 1)
[   70.268363] ITS cmd its_build_inv_cmd failed
[   70.269782] pcieport 0000:00:00.0: PME: Signaling with IRQ 64

Any assistance you can provide would be greatly appreciated.

Very Respectfully,
Peter Geis

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

Re: [RFC] ITS fails to allocate on rk3568/rk3566

[Marc Zyngier]

Hi Peter,

On Mon, 12 Apr 2021 21:49:59 +0100,
Peter Geis <pgwipeout@gmail.com> wrote:
> 
> Good Afternoon,
> 
> I am assisting with early bringup of the rk3566 based quartz64
> development board for mainline linux.
> I've encountered a few issues with allocating ITS on their version of
> the GIC-V3.
> The first issue is the ITS controller can only use 32bit addresses.
> This leads to the following error:
> [    0.000000] NR_IRQS: 64, nr_irqs: 64, preallocated irqs: 0
> [    0.000000] GICv3: GIC: Using split EOI/Deactivate mode
> [    0.000000] GICv3: 320 SPIs implemented
> [    0.000000] GICv3: 0 Extended SPIs implemented
> [    0.000000] GICv3: Distributor has no Range Selector support
> [    0.000000] Root IRQ handler: gic_handle_irq
> [    0.000000] GICv3: 16 PPIs implemented
> [    0.000000] GICv3: CPU0: found redistributor 0 region 0:0x00000000fd460000
> [    0.000000] ITS [mem 0xfd440000-0xfd45ffff]
> [    0.000000] ITS@0x00000000fd440000: Devices doesn't stick:
> f907000100190600 f907000000190600

Ouch. That looks pretty bad. Bit 32 of the register doesn't stick, and
that's right in the middle of the address. The register should be
fully writable as far as the address field is concerned.

Please dump the distributor and ITS IIDR registers so that I can find
the TRM for the exact IP.

> [    0.000000] ITS@0x00000000fd440000: failed probing (-6)
> [    0.000000] ITS: No ITS available, not enabling LPIs
> 
> Downstream fixed this by adding the GFP_DMA32 flag to the memory
> allocations.

Urgh... this really looks like broken silicon to me.

> They also force clear the GITS_BASER_SHAREABILITY_MASK.

Why? Does this also apply to the command queue? Are they forcing cache
flushing?

> Unfortunately while this allowed ITS to allocate on downstream, as
> soon as MSIs attempted to use it all interrupts would time out.
> 
> On upstream, we observe this during allocation:
> [    0.000000] ITS [mem 0xfd440000-0xfd45ffff]
> [    0.000000] ITS@0x00000000fd440000: allocated 8192 Devices @3810000
> (indirect, esz 8, psz 64K, shr 1)
> [    0.000000] ITS@0x00000000fd440000: allocated 32768 Interrupt
> Collections @3820000 (flat, esz 2, psz 64K, shr 1)
> [    0.000000] GICv3: using LPI property table @0x0000000003830000
> [    0.000000] GICv3: CPU0: using allocated LPI pending table
> @0x0000000003840000
> [    0.000000] ITS queue timeout (64 1)
> [    0.000000] ITS cmd its_build_mapc_cmd failed
> [    0.000000] ITS queue timeout (96 1)
> [    0.000000] ITS cmd its_build_invall_cmd failed
> <snip>

So the command queue is not making forward progress. Either because
the ITS cannot access the commands, or because it cannot use the
memory it has been allocated. Please dump GITS_CBASER (and the value
that has been written to it), just in case it shows the same
brokenness as the GITS_BASER registers...

[...]

> Any assistance you can provide would be greatly appreciated.

I'm not sure there is much we can do without a lot more details about
the HW. We need to know the exact GIC implementation they are using
(ARM has two versions of the GICv3 IP), and we also need to find out
*how* this has been integrated. Only Rockchip can tell you that.

Once we know which version they are using, and how it is wired, we can
start looking at some IMPDEF debug registers.

The Linux driver assumes that the ITS is able to access the whole
memory. If there are restrictions on what memory ranges can be used,
it is going to be a pain to support :-(.

	M.

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

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

Re: [RFC] ITS fails to allocate on rk3568/rk3566

[Peter Geis]

On Tue, Apr 13, 2021 at 10:01 AM Marc Zyngier <maz@kernel.org> wrote:
>
> On Tue, 13 Apr 2021 14:29:32 +0100,
> Peter Geis <pgwipeout@gmail.com> wrote:
> >
> > On Tue, Apr 13, 2021 at 5:23 AM Marc Zyngier <maz@kernel.org> wrote:
> > >
> > > Hi Peter,
> > >
> > > On Mon, 12 Apr 2021 21:49:59 +0100,
> > > Peter Geis <pgwipeout@gmail.com> wrote:
> > > >
> > > > Good Afternoon,
> > > >
> > > > I am assisting with early bringup of the rk3566 based quartz64
> > > > development board for mainline linux.
> > > > I've encountered a few issues with allocating ITS on their version of
> > > > the GIC-V3.
> > > > The first issue is the ITS controller can only use 32bit addresses.
> > > > This leads to the following error:
> > > > [    0.000000] NR_IRQS: 64, nr_irqs: 64, preallocated irqs: 0
> > > > [    0.000000] GICv3: GIC: Using split EOI/Deactivate mode
> > > > [    0.000000] GICv3: 320 SPIs implemented
> > > > [    0.000000] GICv3: 0 Extended SPIs implemented
> > > > [    0.000000] GICv3: Distributor has no Range Selector support
> > > > [    0.000000] Root IRQ handler: gic_handle_irq
> > > > [    0.000000] GICv3: 16 PPIs implemented
> > > > [    0.000000] GICv3: CPU0: found redistributor 0 region 0:0x00000000fd460000
> > > > [    0.000000] ITS [mem 0xfd440000-0xfd45ffff]
> > > > [    0.000000] ITS@0x00000000fd440000: Devices doesn't stick:
> > > > f907000100190600 f907000000190600
> > >
> > > Ouch. That looks pretty bad. Bit 32 of the register doesn't stick, and
> > > that's right in the middle of the address. The register should be
> > > fully writable as far as the address field is concerned.
> > >
> > > Please dump the distributor and ITS IIDR registers so that I can find
> > > the TRM for the exact IP.
> >
> > Yeah, I'm getting a crash course in how GIC-V3 sets up, so this is a
> > learning experience too.
> >
> > The GICD_IIDR, GICR_IIDR, and GITS_IIDR are all 0x0201743B
>
> Looks like our mate GIC600 r1p6, which is the most recent revision,
> and reasonably bug free AFAIK. Other examples of this GIC integrated
> on HW have access to seem to work rather well, and that's with (a lot)
> more than 4GB of RAM.
>
> >
> > >
> > > > [    0.000000] ITS@0x00000000fd440000: failed probing (-6)
> > > > [    0.000000] ITS: No ITS available, not enabling LPIs
> > > >
> > > > Downstream fixed this by adding the GFP_DMA32 flag to the memory
> > > > allocations.
> > >
> > > Urgh... this really looks like broken silicon to me.
> >
> > I was afraid of that.
> >
> > >
> > > > They also force clear the GITS_BASER_SHAREABILITY_MASK.
> > >
> > > Why? Does this also apply to the command queue? Are they forcing cache
> > > flushing?
> >
> > The patch doesn't have a description, but the name of the function is
> > "force_inner_cache mode".
> > It disables shareability and caching, but I've experimented with
> > removing this part and it doesn't make a difference in upstream setup.
> > (Things are still broken in the same way). It ends up setting
> > ITS_FLAGS_CMDQ_NEEDS_FLUSHING as well.
> >
> > >
> > > > Unfortunately while this allowed ITS to allocate on downstream, as
> > > > soon as MSIs attempted to use it all interrupts would time out.
> > > >
> > > > On upstream, we observe this during allocation:
> > > > [    0.000000] ITS [mem 0xfd440000-0xfd45ffff]
> > > > [    0.000000] ITS@0x00000000fd440000: allocated 8192 Devices @3810000
> > > > (indirect, esz 8, psz 64K, shr 1)
> > > > [    0.000000] ITS@0x00000000fd440000: allocated 32768 Interrupt
> > > > Collections @3820000 (flat, esz 2, psz 64K, shr 1)
> > > > [    0.000000] GICv3: using LPI property table @0x0000000003830000
> > > > [    0.000000] GICv3: CPU0: using allocated LPI pending table
> > > > @0x0000000003840000
> > > > [    0.000000] ITS queue timeout (64 1)
> > > > [    0.000000] ITS cmd its_build_mapc_cmd failed
> > > > [    0.000000] ITS queue timeout (96 1)
> > > > [    0.000000] ITS cmd its_build_invall_cmd failed
> > > > <snip>
> > >
> > > So the command queue is not making forward progress. Either because
> > > the ITS cannot access the commands, or because it cannot use the
> > > memory it has been allocated. Please dump GITS_CBASER (and the value
> > > that has been written to it), just in case it shows the same
> > > brokenness as the GITS_BASER registers...
> >
> > It seems yes, CBASER is broken in the same way:
> > Value written: b8000001001b040f
> > Value read:    b8000000001b040f
>
> Here you go. They haven't only half broken it. Please also check the
> GICR_PENDBASER/GICR_PROPBASER values on each RD to see if they have
> similar behaviours.

Yes, it seems all accesses are limited to 32bit.

>
> What happens if you hack all the allocations to happen in the low 4GB
> of the PA space?

It seems to work correctly.
The downstream hacks used GFP_DMA32 which gets discarded by
kmalloc_fix_flags on certain allocations.
Switching to GFP_DMA seems to have satisfied it, but it feels wrong
using this code.
Need to check the corner cases to make sure I'm not missing something.

>
> > >
> > > [...]
> > >
> > > > Any assistance you can provide would be greatly appreciated.
> > >
> > > I'm not sure there is much we can do without a lot more details about
> > > the HW. We need to know the exact GIC implementation they are using
> > > (ARM has two versions of the GICv3 IP), and we also need to find out
> > > *how* this has been integrated. Only Rockchip can tell you that.
> >
> > The rk3568 TRM says to reference the
> > "ARM_GIC-600_r1p6-00rel0_Technical_Reference_Manual.pdf" for
> > information on the GIC, if that helps narrow it down too.
>
> Here you go:
>
> https://documentation-service.arm.com/static/5e7ddddacbfe76649ba53034
>
> That doesn't tell us what they did to it though, and whether only the
> ITS is affected or the RDs are similarly brain-damaged. I guess that's
> yet another piece of HW I will not have to spend any money on...
>
>         M.
>
> --
> Without deviation from the norm, progress is not possible.

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

Re: [RFC] ITS fails to allocate on rk3568/rk3566

[Marc Zyngier]

On Tue, 13 Apr 2021 16:03:51 +0100,
Peter Geis <pgwipeout@gmail.com> wrote:
> 
> On Tue, Apr 13, 2021 at 10:01 AM Marc Zyngier <maz@kernel.org> wrote:

[...]

> > What happens if you hack all the allocations to happen in the low 4GB
> > of the PA space?
> 
> It seems to work correctly.
> The downstream hacks used GFP_DMA32 which gets discarded by
> kmalloc_fix_flags on certain allocations.
> Switching to GFP_DMA seems to have satisfied it, but it feels wrong
> using this code.
> Need to check the corner cases to make sure I'm not missing something.

The problem is that GFP_DMA doesn't always mean the same thing.
Overall, we need to hear from Rockchip about the exact nature of the
problem, and then we *may* be able to work something out.

I'd also like to understand whether it is broken because you happen to
have pre-release silicon that will never make it into the wild, or if
this is the real thing that is going to ship on millions of devices.

Thanks,

	M.

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

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

Re: [RFC] ITS fails to allocate on rk3568/rk3566

[Peter Geis]

On Tue, Apr 13, 2021 at 11:51 AM Marc Zyngier <maz@kernel.org> wrote:
>
> On Tue, 13 Apr 2021 16:03:51 +0100,
> Peter Geis <pgwipeout@gmail.com> wrote:
> >
> > On Tue, Apr 13, 2021 at 10:01 AM Marc Zyngier <maz@kernel.org> wrote:
>
> [...]
>
> > > What happens if you hack all the allocations to happen in the low 4GB
> > > of the PA space?
> >
> > It seems to work correctly.
> > The downstream hacks used GFP_DMA32 which gets discarded by
> > kmalloc_fix_flags on certain allocations.
> > Switching to GFP_DMA seems to have satisfied it, but it feels wrong
> > using this code.
> > Need to check the corner cases to make sure I'm not missing something.
>
> The problem is that GFP_DMA doesn't always mean the same thing.
> Overall, we need to hear from Rockchip about the exact nature of the
> problem, and then we *may* be able to work something out.

From what I've read, GFP_DMA allocates as low as possible, while
GFP_DMA32 ensures it's in the 32 bit address range, am I understanding
this correctly?
Is there a reason GFP_DMA is permitted while GFP_DMA32 is not, aside
from backwards compatibility?
(I saw the notes about how we aren't really supposed to rely on these flags)

I've also confirmed that their disabling shareability and caching is necessary.

>
> I'd also like to understand whether it is broken because you happen to
> have pre-release silicon that will never make it into the wild, or if
> this is the real thing that is going to ship on millions of devices.

My understanding is these chips are samples prior to the full
production run, but we are waiting on official comment from Rockchip
about this particular errata.

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

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

Re: [RFC] ITS fails to allocate on rk3568/rk3566

[Marc Zyngier]

On Wed, 14 Apr 2021 12:41:20 +0100,
Peter Geis <pgwipeout@gmail.com> wrote:
> 
> On Tue, Apr 13, 2021 at 11:51 AM Marc Zyngier <maz@kernel.org> wrote:
> >
> > On Tue, 13 Apr 2021 16:03:51 +0100,
> > Peter Geis <pgwipeout@gmail.com> wrote:
> > >
> > > On Tue, Apr 13, 2021 at 10:01 AM Marc Zyngier <maz@kernel.org> wrote:
> >
> > [...]
> >
> > > > What happens if you hack all the allocations to happen in the low 4GB
> > > > of the PA space?
> > >
> > > It seems to work correctly.
> > > The downstream hacks used GFP_DMA32 which gets discarded by
> > > kmalloc_fix_flags on certain allocations.
> > > Switching to GFP_DMA seems to have satisfied it, but it feels wrong
> > > using this code.
> > > Need to check the corner cases to make sure I'm not missing something.
> >
> > The problem is that GFP_DMA doesn't always mean the same thing.
> > Overall, we need to hear from Rockchip about the exact nature of the
> > problem, and then we *may* be able to work something out.
> 
> From what I've read, GFP_DMA allocates as low as possible, while
> GFP_DMA32 ensures it's in the 32 bit address range, am I understanding
> this correctly?

ZONE_DMA{,32} aren't necessarily selected, and can vary in size (some
equally broken systems can only DMA over 30bits...).

> Is there a reason GFP_DMA is permitted while GFP_DMA32 is not, aside
> from backwards compatibility?  (I saw the notes about how we aren't
> really supposed to rely on these flags)

They are completely independent, and they can either be selected or
not. And plenty of systems do not have any memory in the low
4GB. FWIW, one of my main machines has its first byte of RAM at 1TB.

Which means that supporting this system is going to require some very
specific handling.

> I've also confirmed that their disabling shareability and caching is
> necessary.

Confirmed how? For which tables? We really cannot guess this kind of
thing.

> > I'd also like to understand whether it is broken because you happen to
> > have pre-release silicon that will never make it into the wild, or if
> > this is the real thing that is going to ship on millions of devices.
> 
> My understanding is these chips are samples prior to the full
> production run, but we are waiting on official comment from Rockchip
> about this particular errata.

OK. Please let me know once you get a full description of the problem
from Rockchip. We will also need an official erratum number for this
if this is to be worked around in mainline.

	M.

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

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

Re: [RFC] ITS fails to allocate on rk3568/rk3566

[Kever Yang]

Hi Marc, Peter,

     RK356x GIC has two issues:

1. GIC only support 32bit address while rk356x supports 8GB DDR SDRAM, 
so we use ZONE_DMA32 to fix this issue;

2. GIC version is r1p6-00rel0, RK356x interconnect does not support GIC 
and CPU snoop to each other, hence the GIC does not support the 
shareability feature.  The read of register value for shareability  
feature does not return as expect in GICR and GITS, so we have to 
workaround for it.

Thanks,
- Kever
On 2021/4/14 下午8:42, Marc Zyngier wrote:
> On Wed, 14 Apr 2021 12:41:20 +0100,
> Peter Geis <pgwipeout@gmail.com> wrote:
>> On Tue, Apr 13, 2021 at 11:51 AM Marc Zyngier <maz@kernel.org> wrote:
>>> On Tue, 13 Apr 2021 16:03:51 +0100,
>>> Peter Geis <pgwipeout@gmail.com> wrote:
>>>> On Tue, Apr 13, 2021 at 10:01 AM Marc Zyngier <maz@kernel.org> wrote:
>>> [...]
>>>
>>>>> What happens if you hack all the allocations to happen in the low 4GB
>>>>> of the PA space?
>>>> It seems to work correctly.
>>>> The downstream hacks used GFP_DMA32 which gets discarded by
>>>> kmalloc_fix_flags on certain allocations.
>>>> Switching to GFP_DMA seems to have satisfied it, but it feels wrong
>>>> using this code.
>>>> Need to check the corner cases to make sure I'm not missing something.
>>> The problem is that GFP_DMA doesn't always mean the same thing.
>>> Overall, we need to hear from Rockchip about the exact nature of the
>>> problem, and then we *may* be able to work something out.
>>  From what I've read, GFP_DMA allocates as low as possible, while
>> GFP_DMA32 ensures it's in the 32 bit address range, am I understanding
>> this correctly?
> ZONE_DMA{,32} aren't necessarily selected, and can vary in size (some
> equally broken systems can only DMA over 30bits...).
>
>> Is there a reason GFP_DMA is permitted while GFP_DMA32 is not, aside
>> from backwards compatibility?  (I saw the notes about how we aren't
>> really supposed to rely on these flags)
> They are completely independent, and they can either be selected or
> not. And plenty of systems do not have any memory in the low
> 4GB. FWIW, one of my main machines has its first byte of RAM at 1TB.
>
> Which means that supporting this system is going to require some very
> specific handling.
>
>> I've also confirmed that their disabling shareability and caching is
>> necessary.
> Confirmed how? For which tables? We really cannot guess this kind of
> thing.
>
>>> I'd also like to understand whether it is broken because you happen to
>>> have pre-release silicon that will never make it into the wild, or if
>>> this is the real thing that is going to ship on millions of devices.
>> My understanding is these chips are samples prior to the full
>> production run, but we are waiting on official comment from Rockchip
>> about this particular errata.
> OK. Please let me know once you get a full description of the problem
> from Rockchip. We will also need an official erratum number for this
> if this is to be worked around in mainline.
>
> 	M.
>



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

Re: [RFC] ITS fails to allocate on rk3568/rk3566

[Marc Zyngier]

Hi Kever,

On Thu, 15 Apr 2021 08:24:33 +0100,
Kever Yang <kever.yang@rock-chips.com> wrote:
> 
> Hi Marc, Peter,
> 
>     RK356x GIC has two issues:
> 
> 1. GIC only support 32bit address while rk356x supports 8GB DDR SDRAM,
> so we use ZONE_DMA32 to fix this issue;

What transactions does this affect exactly? Only some ITS tables? Or
all of them, including the command queue? What about the configuration
and pending tables associated with the redistributors?

> 2. GIC version is r1p6-00rel0, RK356x interconnect does not support
> GIC and CPU snoop to each other, hence the GIC does not support the
> shareability feature.  The read of register value for shareability 
> feature does not return as expect in GICR and GITS, so we have to
> workaround for it.

How about the cacheability attribute? Can you please provide the exact
set of attributes that this system actually supports for each of the
ITS and redistributor base registers?

Also, please provide errata numbers for these two issues so that we
can properly document them and track the workarounds.

Thanks,

	M.

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

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

Re: [RFC] ITS fails to allocate on rk3568/rk3566

[Kever Yang]

Hi Marc,

On 2021/4/15 下午4:11, Marc Zyngier wrote:
> Hi Kever,
>
> On Thu, 15 Apr 2021 08:24:33 +0100,
> Kever Yang <kever.yang@rock-chips.com> wrote:
>> Hi Marc, Peter,
>>
>>      RK356x GIC has two issues:
>>
>> 1. GIC only support 32bit address while rk356x supports 8GB DDR SDRAM,
>> so we use ZONE_DMA32 to fix this issue;
> What transactions does this affect exactly?
The GIC on rk356x is a 32bit master, which means all the space its logic 
need to access should be in the 4GB range.
> Only some ITS tables? Or
> all of them, including the command queue? What about the configuration
> and pending tables associated with the redistributors?
>
>> 2. GIC version is r1p6-00rel0, RK356x interconnect does not support
>> GIC and CPU snoop to each other, hence the GIC does not support the
>> shareability feature.  The read of register value for shareability
>> feature does not return as expect in GICR and GITS, so we have to
>> workaround for it.
> How about the cacheability attribute? Can you please provide the exact
> set of attributes that this system actually supports for each of the
> ITS and redistributor base registers?

The shareability attributes in GICR_PENDBASEER, GICR_PROPBASER, 
GITS_BASERn, GITS_CBASER default value is 0b00, when we set 0b01 then 
read returns 0b01.

Since there is no ACE coherency interface for this GIC controller, all 
the cacheability in the GIC is not support in hardware.

>
> Also, please provide errata numbers for these two issues so that we
> can properly document them and track the workarounds.

What kind of errata do you need, could you please share any kind of 
example close to this case?

We consider this as a SoC implement design instead of a bug, so we will 
add document in RK356X  TRM to describe the GIC design, but no idea how 
to provide the errata.

Here is the shareabily attribute from ARM GIC architecture specification:
Shareability, bits [11:10] (from GITS_CBASER)
Indicates the Shareability attributes of accesses to the command queue. 
The possible values of this field are:
0b00 Non-shareable.
0b01 Inner Shareable.
0b10 Outer Shareable.
0b11 Reserved. Treated as 0b00.
It is IMPLEMENTATION DEFINED whether this field has a fixed value or can 
be programmed by software. Implementing this field with a fixed value is 
deprecated.
On a Warm reset, this field resets to an architecturally UNKNOWN value

As you can see, "Implementing this field with a fixed value is 
deprecated", so software should program this field to '0b00 
Non-shareable' if the SoC design does not support the cache shareability.

Thanks,
- Kever
>
> Thanks,
>
> 	M.
>



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

Re: [RFC] ITS fails to allocate on rk3568/rk3566

[Marc Zyngier]

On Fri, 16 Apr 2021 02:13:38 +0100,
Kever Yang <kever.yang@rock-chips.com> wrote:
> 
> Hi Marc,
> 
> On 2021/4/15 下午4:11, Marc Zyngier wrote:
> > Hi Kever,
> > 
> > On Thu, 15 Apr 2021 08:24:33 +0100,
> > Kever Yang <kever.yang@rock-chips.com> wrote:
> >> Hi Marc, Peter,
> >> 
> >>      RK356x GIC has two issues:
> >> 
> >> 1. GIC only support 32bit address while rk356x supports 8GB DDR SDRAM,
> >> so we use ZONE_DMA32 to fix this issue;
> > What transactions does this affect exactly?
> The GIC on rk356x is a 32bit master, which means all the space its
> logic need to access should be in the 4GB range.

Well, at least this is consistently broken.

> > Only some ITS tables? Or
> > all of them, including the command queue? What about the configuration
> > and pending tables associated with the redistributors?
> > 
> >> 2. GIC version is r1p6-00rel0, RK356x interconnect does not support
> >> GIC and CPU snoop to each other, hence the GIC does not support the
> >> shareability feature.  The read of register value for shareability
> >> feature does not return as expect in GICR and GITS, so we have to
> >> workaround for it.
> > How about the cacheability attribute? Can you please provide the exact
> > set of attributes that this system actually supports for each of the
> > ITS and redistributor base registers?
> 
> The shareability attributes in GICR_PENDBASEER, GICR_PROPBASER,
> GITS_BASERn, GITS_CBASER default value is 0b00, when we set 0b01 then
> read returns 0b01.

And I claim that this is a perfectly broken behaviour. How do you
expect software to find about the gory details of the integration?
That's the only way for SW to find out what the HW is capable of...

> Since there is no ACE coherency interface for this GIC controller, all
> the cacheability in the GIC is not support in hardware.
> 
> > 
> > Also, please provide errata numbers for these two issues so that we
> > can properly document them and track the workarounds.
> 
> What kind of errata do you need, could you please share any kind of
> example close to this case?

I would like something that says:

"ROCKCHIP_ERRATUM_123456: The GIC600 integration in RK356x doesn't
 support any of the shareability or cacheability attributes, and
 requires both values to be set to 0b00 for all the ITS and
 Redistributor tables."

This is pretty similar to the bug affecting ThunderX with its "erratum
24313" (covered by CONFIG_CAVIUM_ERRATUM_22375), where the tables have
to be flagged as non-cacheable. The Rockchip one is just worse.

We need an official erratum number so that we can refer to it in the
source, commit log and documentation, as well as cross-reference it
with the TRM. This number will be part of a configuration symbol that
will make the compilation conditional so that people don't have to
carry the extra burden generated by this bug if they don't need to.

Same thing goes for the 32bit bug.

> 
> We consider this as a SoC implement design instead of a bug, so we
> will add document in RK356X  TRM to describe the GIC design, but no
> idea how to provide the errata.
> 
> Here is the shareabily attribute from ARM GIC architecture specification:
> Shareability, bits [11:10] (from GITS_CBASER)
> Indicates the Shareability attributes of accesses to the command
> queue. The possible values of this field are:
> 0b00 Non-shareable.
> 0b01 Inner Shareable.
> 0b10 Outer Shareable.
> 0b11 Reserved. Treated as 0b00.
> It is IMPLEMENTATION DEFINED whether this field has a fixed value or
> can be programmed by software. Implementing this field with a fixed
> value is deprecated.
> On a Warm reset, this field resets to an architecturally UNKNOWN value
> 
> As you can see, "Implementing this field with a fixed value is
> deprecated", so software should program this field to '0b00
> Non-shareable' if the SoC design does not support the cache
> shareability.

[I really feel special when people quote the GIC spec at me]

That isn't what it says. Hardcoding the field with a fixed value is
indeed deprecated, but that doesn't mean this field should accept
values that the HW cannot support. If anything, what this says is "try
and implement the options that SW is going to use".

But you need to give SW an indication of what is usable, because there
is no other way to *discover* what the SoC is capable of at runtime.
Otherwise, we would need to carry a per-SoC list of what the HW
supports. I don't think that's the right thing to do (and you're about
8 years too late anyway).

Other GIC600 integrations got it perfectly right, by the way. Same for
other GIC implementations, with the notable exception of Cavium and
their first GIC in ThunderX, as described above.

Thanks,

	M.

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

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

Re: [RFC] ITS fails to allocate on rk3568/rk3566

[Kever Yang]

Hi Marc,

On 2021/4/16 下午11:23, Marc Zyngier wrote:
> On Fri, 16 Apr 2021 02:13:38 +0100,
> Kever Yang <kever.yang@rock-chips.com> wrote:
>> Hi Marc,
>>
>> On 2021/4/15 下午4:11, Marc Zyngier wrote:
>>> Hi Kever,
>>>
>>> On Thu, 15 Apr 2021 08:24:33 +0100,
>>> Kever Yang <kever.yang@rock-chips.com> wrote:
>>>> Hi Marc, Peter,
>>>>
>>>>       RK356x GIC has two issues:
>>>>
>>>> 1. GIC only support 32bit address while rk356x supports 8GB DDR SDRAM,
>>>> so we use ZONE_DMA32 to fix this issue;
>>> What transactions does this affect exactly?
>> The GIC on rk356x is a 32bit master, which means all the space its
>> logic need to access should be in the 4GB range.
> Well, at least this is consistently broken.

There are many legacy IPs which only support 32bit bus, we have to use 
them as is in the new

64bit SoCs, I think the 32bit GIC can be considered the same as those 
case, can we add CONFIG_ZONE_DMA32

support in GIC driver?

eg. Other peripheral driver use dma_alloc_coherent() instead of 
alloc_pages(), so then can support

ZONE_DMA32 without any code change.


>>> Only some ITS tables? Or
>>> all of them, including the command queue? What about the configuration
>>> and pending tables associated with the redistributors?
>>>
>>>> 2. GIC version is r1p6-00rel0, RK356x interconnect does not support
>>>> GIC and CPU snoop to each other, hence the GIC does not support the
>>>> shareability feature.  The read of register value for shareability
>>>> feature does not return as expect in GICR and GITS, so we have to
>>>> workaround for it.
>>> How about the cacheability attribute? Can you please provide the exact
>>> set of attributes that this system actually supports for each of the
>>> ITS and redistributor base registers?
>> The shareability attributes in GICR_PENDBASEER, GICR_PROPBASER,
>> GITS_BASERn, GITS_CBASER default value is 0b00, when we set 0b01 then
>> read returns 0b01.
> And I claim that this is a perfectly broken behaviour. How do you
> expect software to find about the gory details of the integration?
> That's the only way for SW to find out what the HW is capable of...

As a software engineer, I totally agree with you on this point, but when 
we back to the truth,

we should know that these registers has never been defined as "module 
capability for shareability"

in hardware. The software use it as capability detect, and it works 
before, but not means this

match the original hardware design, in this case the new hardware may 
not follow the legacy

design because it's not a standard or a clear enough guide for it.

The capability register should always read-only instead of read-write, 
the understanding from

software engineer and from hardware engineer always have a gap.

I would proposal to add a optional dts property for driver to identify 
if the board's GIC support

shareability or not, which is a method used by many other module drivers.

>> Since there is no ACE coherency interface for this GIC controller, all
>> the cacheability in the GIC is not support in hardware.
>>
>>> Also, please provide errata numbers for these two issues so that we
>>> can properly document them and track the workarounds.
>> What kind of errata do you need, could you please share any kind of
>> example close to this case?
> I would like something that says:
>
> "ROCKCHIP_ERRATUM_123456: The GIC600 integration in RK356x doesn't
>   support any of the shareability or cacheability attributes, and
>   requires both values to be set to 0b00 for all the ITS and
>   Redistributor tables."
>
> This is pretty similar to the bug affecting ThunderX with its "erratum
> 24313" (covered by CONFIG_CAVIUM_ERRATUM_22375), where the tables have
> to be flagged as non-cacheable. The Rockchip one is just worse.
>
> We need an official erratum number so that we can refer to it in the
> source, commit log and documentation, as well as cross-reference it
> with the TRM. This number will be part of a configuration symbol that
> will make the compilation conditional so that people don't have to
> carry the extra burden generated by this bug if they don't need to.
>
> Same thing goes for the 32bit bug.
>
>> We consider this as a SoC implement design instead of a bug, so we
>> will add document in RK356X  TRM to describe the GIC design, but no
>> idea how to provide the errata.
>>
>> Here is the shareabily attribute from ARM GIC architecture specification:
>> Shareability, bits [11:10] (from GITS_CBASER)
>> Indicates the Shareability attributes of accesses to the command
>> queue. The possible values of this field are:
>> 0b00 Non-shareable.
>> 0b01 Inner Shareable.
>> 0b10 Outer Shareable.
>> 0b11 Reserved. Treated as 0b00.
>> It is IMPLEMENTATION DEFINED whether this field has a fixed value or
>> can be programmed by software. Implementing this field with a fixed
>> value is deprecated.
>> On a Warm reset, this field resets to an architecturally UNKNOWN value
>>
>> As you can see, "Implementing this field with a fixed value is
>> deprecated", so software should program this field to '0b00
>> Non-shareable' if the SoC design does not support the cache
>> shareability.
> [I really feel special when people quote the GIC spec at me]
>
> That isn't what it says. Hardcoding the field with a fixed value is
> indeed deprecated, but that doesn't mean this field should accept
> values that the HW cannot support. If anything, what this says is "try
> and implement the options that SW is going to use".
>
> But you need to give SW an indication of what is usable, because there
> is no other way to *discover* what the SoC is capable of at runtime.
> Otherwise, we would need to carry a per-SoC list of what the HW
> supports. I don't think that's the right thing to do (and you're about
> 8 years too late anyway).
>
> Other GIC600 integrations got it perfectly right, by the way. Same for
> other GIC implementations, with the notable exception of Cavium and
> their first GIC in ThunderX, as described above.

I'm not sure if you still work for ARM or not, but we do have double 
check again and again

with IP vendor about this point, the GIC500 works because it's hardware 
bind to a fix value inside the IP,

but GIC600 does not do the same binding, and there is no any config 
option to hardcode the reg field

to a fixed value in the IP when we don't need the ACE-lite.

This is why I insist that this is a different SoC implementation instead 
of a SoC bug, we should add the

support in the GIC driver for different hardware case.

When you say we're about 8 years too late, I think there still not much 
SoCs using GIC600, maybe the GIC600 IP

is available for user at mid of 2017?


Thanks,
- Kever
>
> Thanks,
>
> 	M.
>



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

Re: [RFC] ITS fails to allocate on rk3568/rk3566

[Marc Zyngier]

On Wed, 21 Apr 2021 02:40:26 +0100,
Kever Yang <kever.yang@rock-chips.com> wrote:
> 
> Hi Marc,
> 
> On 2021/4/16 下午11:23, Marc Zyngier wrote:
> > On Fri, 16 Apr 2021 02:13:38 +0100,
> > Kever Yang <kever.yang@rock-chips.com> wrote:
> >> Hi Marc,
> >> 
> >> On 2021/4/15 下午4:11, Marc Zyngier wrote:
> >>> Hi Kever,
> >>> 
> >>> On Thu, 15 Apr 2021 08:24:33 +0100,
> >>> Kever Yang <kever.yang@rock-chips.com> wrote:
> >>>> Hi Marc, Peter,
> >>>> 
> >>>>       RK356x GIC has two issues:
> >>>> 
> >>>> 1. GIC only support 32bit address while rk356x supports 8GB DDR SDRAM,
> >>>> so we use ZONE_DMA32 to fix this issue;
> >>> What transactions does this affect exactly?
> >> The GIC on rk356x is a 32bit master, which means all the space its
> >> logic need to access should be in the 4GB range.
> > Well, at least this is consistently broken.
> 
> There are many legacy IPs which only support 32bit bus, we have to
> use them as is in the new 64bit SoCs, I think the 32bit GIC can be
> considered the same as those case, can we add CONFIG_ZONE_DMA32
> support in GIC driver?

Only as an erratum workaround. The GIC isn't some random device that
can live behind an IOMMU that will sort the mapping problem for
you. By design, it must be untranslated, so limiting it to only a
portion of the physical address space is a bug, full stop.

> eg. Other peripheral driver use dma_alloc_coherent() instead of
> alloc_pages(), so then can support ZONE_DMA32 without any code
> change.

You do realise that dma_alloc_coherent() requires the use of a struct
device, and that the GIC is required to be up and running long before
the device model is available, right?

> >>> Only some ITS tables? Or
> >>> all of them, including the command queue? What about the configuration
> >>> and pending tables associated with the redistributors?
> >>> 
> >>>> 2. GIC version is r1p6-00rel0, RK356x interconnect does not support
> >>>> GIC and CPU snoop to each other, hence the GIC does not support the
> >>>> shareability feature.  The read of register value for shareability
> >>>> feature does not return as expect in GICR and GITS, so we have to
> >>>> workaround for it.
> >>> How about the cacheability attribute? Can you please provide the exact
> >>> set of attributes that this system actually supports for each of the
> >>> ITS and redistributor base registers?
> >> The shareability attributes in GICR_PENDBASEER, GICR_PROPBASER,
> >> GITS_BASERn, GITS_CBASER default value is 0b00, when we set 0b01 then
> >> read returns 0b01.
> > And I claim that this is a perfectly broken behaviour. How do you
> > expect software to find about the gory details of the integration?
> > That's the only way for SW to find out what the HW is capable of...
> 
> As a software engineer, I totally agree with you on this point, but
> when we back to the truth, we should know that these registers has
> never been defined as "module capability for shareability"

Says who? If SW cannot discover the capability of the HW, then nothing
works.

> in hardware. The software use it as capability detect, and it works
> before, but not means match the original hardware design, in this
> case the new hardware may not follow the design because it's not a
> standard or a clear enough guide for it.

The spec has been clear enough for *everyone* so far, and other GIC600
implementations got it perfectly right. There has been *one*
exception, which is documented and handled as a quirk.

> The capability register should always read-only instead of
> read-write, the understanding software engineer and from hardware
> engineer always have a gap.
> 
> I would proposal to add a optional dts property for driver to
> identify if the board's GIC shareability or not, which is a method
> used by many other module drivers.

Are you also going to propose and support an ACPI specification update
to cope with similarly broken devices? Either this is something that
is available across the various firmware implementations, or it
doesn't exist.

>
> >> Since there is no ACE coherency interface for this GIC controller, all
> >> the cacheability in the GIC is not support in hardware.
> >> 
> >>> Also, please provide errata numbers for these two issues so that we
> >>> can properly document them and track the workarounds.
> >> What kind of errata do you need, could you please share any kind of
> >> example close to this case?
> > I would like something that says:
> > 
> > "ROCKCHIP_ERRATUM_123456: The GIC600 integration in RK356x doesn't
> >   support any of the shareability or cacheability attributes, and
> >   requires both values to be set to 0b00 for all the ITS and
> >   Redistributor tables."
> > 
> > This is pretty similar to the bug affecting ThunderX with its "erratum
> > 24313" (covered by CONFIG_CAVIUM_ERRATUM_22375), where the tables have
> > to be flagged as non-cacheable. The Rockchip one is just worse.
> > 
> > We need an official erratum number so that we can refer to it in the
> > source, commit log and documentation, as well as cross-reference it
> > with the TRM. This number will be part of a configuration symbol that
> > will make the compilation conditional so that people don't have to
> > carry the extra burden generated by this bug if they don't need to.
> > 
> > Same thing goes for the 32bit bug.
> > 
> >> We consider this as a SoC implement design instead of a bug, so we
> >> will add document in RK356X  TRM to describe the GIC design, but no
> >> idea how to provide the errata.
> >> 
> >> Here is the shareabily attribute from ARM GIC architecture specification:
> >> Shareability, bits [11:10] (from GITS_CBASER)
> >> Indicates the Shareability attributes of accesses to the command
> >> queue. The possible values of this field are:
> >> 0b00 Non-shareable.
> >> 0b01 Inner Shareable.
> >> 0b10 Outer Shareable.
> >> 0b11 Reserved. Treated as 0b00.
> >> It is IMPLEMENTATION DEFINED whether this field has a fixed value or
> >> can be programmed by software. Implementing this field with a fixed
> >> value is deprecated.
> >> On a Warm reset, this field resets to an architecturally UNKNOWN value
> >> 
> >> As you can see, "Implementing this field with a fixed value is
> >> deprecated", so software should program this field to '0b00
> >> Non-shareable' if the SoC design does not support the cache
> >> shareability.
> > [I really feel special when people quote the GIC spec at me]
> > 
> > That isn't what it says. Hardcoding the field with a fixed value is
> > indeed deprecated, but that doesn't mean this field should accept
> > values that the HW cannot support. If anything, what this says is "try
> > and implement the options that SW is going to use".
> > 
> > But you need to give SW an indication of what is usable, because there
> > is no other way to *discover* what the SoC is capable of at runtime.
> > Otherwise, we would need to carry a per-SoC list of what the HW
> > supports. I don't think that's the right thing to do (and you're about
> > 8 years too late anyway).
> > 
> > Other GIC600 integrations got it perfectly right, by the way. Same for
> > other GIC implementations, with the notable exception of Cavium and
> > their first GIC in ThunderX, as described above.
> 
> I'm not sure if you still work for ARM or not, but we do have double

I don't think whoever pays my salary is relevant in this context. I am
writing as the guy who maintains the GIC architecture in Linux, and
the fact that I have worked, work, or will work for ARM or not has no
bearing on what I am saying here.

> check again and again with IP vendor about this point, the GIC500
> works because it's hardware bind to a fix value inside the IP, but
> GIC600 does not do the same binding, and there is no any config
> option to hardcode the reg field to a fixed value in the IP when
> we don't need the ACE-lite.
> 
> This is why I insist that this is a different SoC implementation
> instead of a SoC bug, we should add the support in the GIC driver
> for different hardware case.
> 
> When you say we're about 8 years too late, I think there still not
> much SoCs using GIC600, maybe the GIC600 IP is available for user at
> mid of 2017?

I'm not sure what the year 2017 does here. SW written as early as 2013
and compliant with the architecture still works on GIC600 when it is
properly integrated. Linux also has specific requirements, and you
have decided to ignore them. That's your choice. But you also get to
keep the pieces when things break.

Now, this discussion has lasted long enough, and isn't going anywhere.
If you want upstream support for your HW, I have explained what needs
to be done: document the two problems, implement the workarounds using
the GIC quirk infrastructure just like any other silicon vendor has
done over the past 8 years.

If you don't want to do it, it suits me just as well. I won't have to
maintain yet another set of SoC-specific workarounds.

Thanks,

	M.

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

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