| Message ID | 20250215235431.143138-2-dmitry.osipenko@collabora.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add Rockchip 3568001/2 errata workarounds and enable ITS on RK356x | expand |
On Sat, 15 Feb 2025 23:54:28 +0000, Dmitry Osipenko <dmitry.osipenko@collabora.com> wrote: > > Rockchip RK3566/RK3568 GIC600 integration has DDR addressing > limited to first 4GB of DRAM. Rockchip assigned Erratum ID #3568002 > for this issue. Add driver quirk for this Rockchip GIC Erratum. Thanks for taking the time to submit this. It only took 5 years for this erratum to be published... However, my understanding of this issue is that the integration is limited to the first 32bit of physical address space, not the first 32bit of RAM. If the memory is placed as physical address 0, then they represent the same space. But this is still an important distinction. > > Note, that the 0x0201743b ID is not Rockchip 356x specific and thus > there is an extra of_machine_is_compatible() check. Rockchip 3588 uses > same ID and it is not affected by this errata. This ID is that of ARM's GIC600, which is a very common GICv3 implementation, and is not Rockchip-specific. Please capture this in the commit message. > > Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> > --- > Documentation/arch/arm64/silicon-errata.rst | 2 ++ > arch/arm64/Kconfig | 9 ++++++++ > drivers/irqchip/irq-gic-v3-its.c | 23 ++++++++++++++++++++- > 3 files changed, 33 insertions(+), 1 deletion(-) > > diff --git a/Documentation/arch/arm64/silicon-errata.rst b/Documentation/arch/arm64/silicon-errata.rst > index f074f6219f5c..f968c13b46a7 100644 > --- a/Documentation/arch/arm64/silicon-errata.rst > +++ b/Documentation/arch/arm64/silicon-errata.rst > @@ -284,6 +284,8 @@ stable kernels. > +----------------+-----------------+-----------------+-----------------------------+ > | Rockchip | RK3588 | #3588001 | ROCKCHIP_ERRATUM_3588001 | > +----------------+-----------------+-----------------+-----------------------------+ > +| Rockchip | RK3568 | #3568002 | ROCKCHIP_ERRATUM_3568002 | > ++----------------+-----------------+-----------------+-----------------------------+ > +----------------+-----------------+-----------------+-----------------------------+ > | Fujitsu | A64FX | E#010001 | FUJITSU_ERRATUM_010001 | > +----------------+-----------------+-----------------+-----------------------------+ > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index c997b27b7da1..0428ad8f324d 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -1302,6 +1302,15 @@ config NVIDIA_CARMEL_CNP_ERRATUM > > If unsure, say Y. > > +config ROCKCHIP_ERRATUM_3568002 > + bool "Rockchip 3568002: can not support DDR addresses higher than 4G" > + default y > + help > + The Rockchip RK3566 and RK3568 GIC600 SoC integrations have DDR > + addressing limited to first 4GB. > + > + If unsure, say Y. > + s/DDR addresses/physical addresses/ > config ROCKCHIP_ERRATUM_3588001 > bool "Rockchip 3588001: GIC600 can not support shareability attributes" > default y > diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c > index 8c3ec5734f1e..f30ed281882f 100644 > --- a/drivers/irqchip/irq-gic-v3-its.c > +++ b/drivers/irqchip/irq-gic-v3-its.c > @@ -205,13 +205,15 @@ static DEFINE_IDA(its_vpeid_ida); > #define gic_data_rdist_rd_base() (gic_data_rdist()->rd_base) > #define gic_data_rdist_vlpi_base() (gic_data_rdist_rd_base() + SZ_128K) > > +static gfp_t gfp_flags_quirk; > + > static struct page *its_alloc_pages_node(int node, gfp_t gfp, > unsigned int order) > { > struct page *page; > int ret = 0; > > - page = alloc_pages_node(node, gfp, order); > + page = alloc_pages_node(node, gfp | gfp_flags_quirk, order); > > if (!page) > return NULL; > @@ -4887,6 +4889,17 @@ static bool __maybe_unused its_enable_quirk_hip09_162100801(void *data) > return true; > } > > +static bool __maybe_unused its_enable_rk3568002(void *data) > +{ > + if (!of_machine_is_compatible("rockchip,rk3566") && > + !of_machine_is_compatible("rockchip,rk3568")) > + return false; > + > + gfp_flags_quirk |= GFP_DMA32; > + > + return true; > +} > + > static const struct gic_quirk its_quirks[] = { > #ifdef CONFIG_CAVIUM_ERRATUM_22375 > { > @@ -4954,6 +4967,14 @@ static const struct gic_quirk its_quirks[] = { > .property = "dma-noncoherent", > .init = its_set_non_coherent, > }, > +#ifdef CONFIG_ROCKCHIP_ERRATUM_3568002 > + { > + .desc = "ITS: Rockchip erratum RK3568002", > + .iidr = 0x0201743b, > + .mask = 0xffffffff, > + .init = its_enable_rk3568002, > + }, > +#endif > { > } > }; Another thing is that this patch conflates ITS and redistributors. As it turns out, we use the same allocator for both, but they are distinct architectural concepts, even if GIC600 is a monolithic implementation. It is OK for now, but it will have to be revisited if we ever move the redistributor management outside of the ITS driver. With the other comments addressed: Acked-by: Marc Zyngier <maz@kernel.org> M.
On 2/16/25 12:55, Marc Zyngier wrote: > On Sat, 15 Feb 2025 23:54:28 +0000, > Dmitry Osipenko <dmitry.osipenko@collabora.com> wrote: >> >> Rockchip RK3566/RK3568 GIC600 integration has DDR addressing >> limited to first 4GB of DRAM. Rockchip assigned Erratum ID #3568002 >> for this issue. Add driver quirk for this Rockchip GIC Erratum. > > Thanks for taking the time to submit this. It only took 5 years for > this erratum to be published... The erratum document itself actually is dated by 5 years ago. Only wish the doc was made publicly available, which would've accelerated the upstreaming process. > However, my understanding of this issue is that the integration is > limited to the first 32bit of physical address space, not the first > 32bit of RAM. If the memory is placed as physical address 0, then they > represent the same space. But this is still an important distinction. Indeed, will correct the description in v2. >> Note, that the 0x0201743b ID is not Rockchip 356x specific and thus >> there is an extra of_machine_is_compatible() check. Rockchip 3588 uses >> same ID and it is not affected by this errata. > > This ID is that of ARM's GIC600, which is a very common GICv3 > implementation, and is not Rockchip-specific. Please capture this in > the commit message. Ack ... >> +config ROCKCHIP_ERRATUM_3568002 >> + bool "Rockchip 3568002: can not support DDR addresses higher than 4G" >> + default y >> + help >> + The Rockchip RK3566 and RK3568 GIC600 SoC integrations have DDR >> + addressing limited to first 4GB. >> + >> + If unsure, say Y. >> + > > s/DDR addresses/physical addresses/ Ack ... > Another thing is that this patch conflates ITS and redistributors. As > it turns out, we use the same allocator for both, but they are > distinct architectural concepts, even if GIC600 is a monolithic > implementation. It is OK for now, but it will have to be revisited if > we ever move the redistributor management outside of the ITS driver. > > With the other comments addressed: > > Acked-by: Marc Zyngier <maz@kernel.org> Thanks for the review!
On Sun, 16 Feb 2025 15:25:53 +0000, Dmitry Osipenko <dmitry.osipenko@collabora.com> wrote: > > On 2/16/25 12:55, Marc Zyngier wrote: > > On Sat, 15 Feb 2025 23:54:28 +0000, > > Dmitry Osipenko <dmitry.osipenko@collabora.com> wrote: > >> > >> Rockchip RK3566/RK3568 GIC600 integration has DDR addressing > >> limited to first 4GB of DRAM. Rockchip assigned Erratum ID #3568002 > >> for this issue. Add driver quirk for this Rockchip GIC Erratum. > > > > Thanks for taking the time to submit this. It only took 5 years for > > this erratum to be published... > > The erratum document itself actually is dated by 5 years ago. Only wish > the doc was made publicly available, which would've accelerated the > upstreaming process. The funny thing is that RockChip was very public about the issue in 2019, but refused to acknowledge that it was a bug (see the list archives). 5 years lost with bad performance and bad upstream support, only to finally admit the bleeding obvious. On the other hand, we're getting close to 10 years of rk3399, and the botched integration of GIC500 still hasn't been officially disclosed. I guess this counts as progress ;-). M.
diff --git a/Documentation/arch/arm64/silicon-errata.rst b/Documentation/arch/arm64/silicon-errata.rst index f074f6219f5c..f968c13b46a7 100644 --- a/Documentation/arch/arm64/silicon-errata.rst +++ b/Documentation/arch/arm64/silicon-errata.rst @@ -284,6 +284,8 @@ stable kernels. +----------------+-----------------+-----------------+-----------------------------+ | Rockchip | RK3588 | #3588001 | ROCKCHIP_ERRATUM_3588001 | +----------------+-----------------+-----------------+-----------------------------+ +| Rockchip | RK3568 | #3568002 | ROCKCHIP_ERRATUM_3568002 | ++----------------+-----------------+-----------------+-----------------------------+ +----------------+-----------------+-----------------+-----------------------------+ | Fujitsu | A64FX | E#010001 | FUJITSU_ERRATUM_010001 | +----------------+-----------------+-----------------+-----------------------------+ diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index c997b27b7da1..0428ad8f324d 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -1302,6 +1302,15 @@ config NVIDIA_CARMEL_CNP_ERRATUM If unsure, say Y. +config ROCKCHIP_ERRATUM_3568002 + bool "Rockchip 3568002: can not support DDR addresses higher than 4G" + default y + help + The Rockchip RK3566 and RK3568 GIC600 SoC integrations have DDR + addressing limited to first 4GB. + + If unsure, say Y. + config ROCKCHIP_ERRATUM_3588001 bool "Rockchip 3588001: GIC600 can not support shareability attributes" default y diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c index 8c3ec5734f1e..f30ed281882f 100644 --- a/drivers/irqchip/irq-gic-v3-its.c +++ b/drivers/irqchip/irq-gic-v3-its.c @@ -205,13 +205,15 @@ static DEFINE_IDA(its_vpeid_ida); #define gic_data_rdist_rd_base() (gic_data_rdist()->rd_base) #define gic_data_rdist_vlpi_base() (gic_data_rdist_rd_base() + SZ_128K) +static gfp_t gfp_flags_quirk; + static struct page *its_alloc_pages_node(int node, gfp_t gfp, unsigned int order) { struct page *page; int ret = 0; - page = alloc_pages_node(node, gfp, order); + page = alloc_pages_node(node, gfp | gfp_flags_quirk, order); if (!page) return NULL; @@ -4887,6 +4889,17 @@ static bool __maybe_unused its_enable_quirk_hip09_162100801(void *data) return true; } +static bool __maybe_unused its_enable_rk3568002(void *data) +{ + if (!of_machine_is_compatible("rockchip,rk3566") && + !of_machine_is_compatible("rockchip,rk3568")) + return false; + + gfp_flags_quirk |= GFP_DMA32; + + return true; +} + static const struct gic_quirk its_quirks[] = { #ifdef CONFIG_CAVIUM_ERRATUM_22375 { @@ -4954,6 +4967,14 @@ static const struct gic_quirk its_quirks[] = { .property = "dma-noncoherent", .init = its_set_non_coherent, }, +#ifdef CONFIG_ROCKCHIP_ERRATUM_3568002 + { + .desc = "ITS: Rockchip erratum RK3568002", + .iidr = 0x0201743b, + .mask = 0xffffffff, + .init = its_enable_rk3568002, + }, +#endif { } };
Rockchip RK3566/RK3568 GIC600 integration has DDR addressing limited to first 4GB of DRAM. Rockchip assigned Erratum ID #3568002 for this issue. Add driver quirk for this Rockchip GIC Erratum. Note, that the 0x0201743b ID is not Rockchip 356x specific and thus there is an extra of_machine_is_compatible() check. Rockchip 3588 uses same ID and it is not affected by this errata. Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> --- Documentation/arch/arm64/silicon-errata.rst | 2 ++ arch/arm64/Kconfig | 9 ++++++++ drivers/irqchip/irq-gic-v3-its.c | 23 ++++++++++++++++++++- 3 files changed, 33 insertions(+), 1 deletion(-)