* [PATCH v2 0/2] Add Rockchip RK3588 GIC ITS support @ 2023-04-17 15:00 Sebastian Reichel 2023-04-17 15:00 ` [PATCH v2 1/2] irqchip/gic-v3: Add Rockchip 3588001 errata workaround Sebastian Reichel 2023-04-17 15:00 ` [PATCH v2 2/2] arm64: dts: rockchip: rk3588: add MSI support Sebastian Reichel 0 siblings, 2 replies; 8+ messages in thread From: Sebastian Reichel @ 2023-04-17 15:00 UTC (permalink / raw) To: Marc Zyngier, Heiko Stuebner Cc: Rob Herring, Krzysztof Kozlowski, Thomas Gleixner, Peng Fan, Robin Murphy, Peter Geis, XiaoDong Huang, Kever Yang, linux-rockchip, devicetree, linux-kernel, Sebastian Reichel, kernel Hi, We got an Errata # from Rockchip for the GIC600 integration issue in RK3588, so here is another try adding upstream ITS support for that platform. I noticed https://lore.kernel.org/lkml/DU0PR04MB9417388F9BDD73080294FA8E88889@DU0PR04MB9417.eurprd04.prod.outlook.com/ so I kept the flag name itself generic. P.S.: I'm taking over from Lucas Tanure, who no longer works for Collabora. List of previous attempts / changelog: * PATCHv1 - https://lore.kernel.org/lkml/20230227151847.207922-1-lucas.tanure@collabora.com/ - uses of_dma_is_coherent() instead of providing errata info from kernel * RFCv1 - https://lore.kernel.org/lkml/20230310080518.78054-1-lucas.tanure@collabora.com/ - uses 0x0201743b IIDR for quirk detection and misses errata # Greetings, -- Sebastian Sebastian Reichel (2): irqchip/gic-v3: Add Rockchip 3588001 errata workaround arm64: dts: rockchip: rk3588: add MSI support Documentation/arm64/silicon-errata.rst | 3 +++ arch/arm64/Kconfig | 10 +++++++++ arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 17 +++++++++++++++ drivers/irqchip/irq-gic-v3-its.c | 25 +++++++++++++++++++++++ 4 files changed, 55 insertions(+) -- 2.39.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/2] irqchip/gic-v3: Add Rockchip 3588001 errata workaround 2023-04-17 15:00 [PATCH v2 0/2] Add Rockchip RK3588 GIC ITS support Sebastian Reichel @ 2023-04-17 15:00 ` Sebastian Reichel 2023-04-17 16:20 ` Marc Zyngier 2023-04-17 15:00 ` [PATCH v2 2/2] arm64: dts: rockchip: rk3588: add MSI support Sebastian Reichel 1 sibling, 1 reply; 8+ messages in thread From: Sebastian Reichel @ 2023-04-17 15:00 UTC (permalink / raw) To: Marc Zyngier, Heiko Stuebner Cc: Rob Herring, Krzysztof Kozlowski, Thomas Gleixner, Peng Fan, Robin Murphy, Peter Geis, XiaoDong Huang, Kever Yang, linux-rockchip, devicetree, linux-kernel, Sebastian Reichel, kernel Rockchip RK3588/RK3588s GIC600 integration is not cache coherent. Thus even though the GIC600 itself supports the sharability feature, it may not be used. Rockchip assigned Errata ID #3588001 for this issue. The RK3588's GIC600 IP is using ARM's ID in the IIDR register (0x0201743b), so it cannot be used to apply the quirk. Thus I used the machine compatible instead. I named the flag ITS_FLAGS_BROKEN_SHAREABILITY to be vendor agnostic, since apparently similar integration design errors exist in other platforms and they can reuse the same flag. Co-developed-by: XiaoDong Huang <derrick.huang@rock-chips.com> Signed-off-by: XiaoDong Huang <derrick.huang@rock-chips.com> Co-developed-by: Kever Yang <kever.yang@rock-chips.com> Signed-off-by: Kever Yang <kever.yang@rock-chips.com> Co-developed-by: Lucas Tanure <lucas.tanure@collabora.com> Signed-off-by: Lucas Tanure <lucas.tanure@collabora.com> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> --- Documentation/arm64/silicon-errata.rst | 3 +++ arch/arm64/Kconfig | 10 ++++++++++ drivers/irqchip/irq-gic-v3-its.c | 25 +++++++++++++++++++++++++ 3 files changed, 38 insertions(+) diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst index ec5f889d7681..46d06ed3e4f4 100644 --- a/Documentation/arm64/silicon-errata.rst +++ b/Documentation/arm64/silicon-errata.rst @@ -205,6 +205,9 @@ stable kernels. +----------------+-----------------+-----------------+-----------------------------+ | Qualcomm Tech. | Kryo4xx Gold | N/A | ARM64_ERRATUM_1286807 | +----------------+-----------------+-----------------+-----------------------------+ ++----------------+-----------------+-----------------+-----------------------------+ +| Rockchip | RK3588 | #3588001 | ROCKCHIP_ERRATUM_3588001 | ++----------------+-----------------+-----------------+-----------------------------+ +----------------+-----------------+-----------------+-----------------------------+ | Fujitsu | A64FX | E#010001 | FUJITSU_ERRATUM_010001 | diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 1023e896d46b..640619459648 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -1150,6 +1150,16 @@ config NVIDIA_CARMEL_CNP_ERRATUM If unsure, say Y. +config ROCKCHIP_ERRATUM_3588001 + bool "Rockchip 3588001: GIC600 can not support shareable attribute" + default y + help + The Rockchip RK3588 GIC600 SoC integration does not support ACE/ACE-lite + and thus is not cache coherent. This means, that the GIC600 may not use + the sharability feature even though it is supported by the IP itself. + + If unsure, say Y. + config SOCIONEXT_SYNQUACER_PREITS bool "Socionext Synquacer: Workaround for GICv3 pre-ITS" default y diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c index 586271b8aa39..0701d0b67690 100644 --- a/drivers/irqchip/irq-gic-v3-its.c +++ b/drivers/irqchip/irq-gic-v3-its.c @@ -42,6 +42,7 @@ #define ITS_FLAGS_CMDQ_NEEDS_FLUSHING (1ULL << 0) #define ITS_FLAGS_WORKAROUND_CAVIUM_22375 (1ULL << 1) #define ITS_FLAGS_WORKAROUND_CAVIUM_23144 (1ULL << 2) +#define ITS_FLAGS_BROKEN_SHAREABILITY (1ULL << 3) #define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING (1 << 0) #define RDIST_FLAGS_RD_TABLES_PREALLOCATED (1 << 1) @@ -2359,6 +2360,13 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser, its_write_baser(its, baser, val); tmp = baser->val; + if (its->flags & ITS_FLAGS_BROKEN_SHAREABILITY) { + if (tmp & GITS_BASER_SHAREABILITY_MASK) + tmp &= ~GITS_BASER_SHAREABILITY_MASK; + else + gic_flush_dcache_to_poc(base, PAGE_ORDER_TO_SIZE(order)); + } + if ((val ^ tmp) & GITS_BASER_SHAREABILITY_MASK) { /* * Shareability didn't stick. Just use @@ -3055,6 +3063,7 @@ static u64 its_clear_vpend_valid(void __iomem *vlpi_base, u64 clr, u64 set) static void its_cpu_init_lpis(void) { + struct its_node *its = list_first_entry(&its_nodes, struct its_node, entry); void __iomem *rbase = gic_data_rdist_rd_base(); struct page *pend_page; phys_addr_t paddr; @@ -3096,6 +3105,9 @@ static void its_cpu_init_lpis(void) gicr_write_propbaser(val, rbase + GICR_PROPBASER); tmp = gicr_read_propbaser(rbase + GICR_PROPBASER); + if (its->flags & ITS_FLAGS_BROKEN_SHAREABILITY) + tmp &= ~GICR_PROPBASER_SHAREABILITY_MASK; + if ((tmp ^ val) & GICR_PROPBASER_SHAREABILITY_MASK) { if (!(tmp & GICR_PROPBASER_SHAREABILITY_MASK)) { /* @@ -3120,6 +3132,9 @@ static void its_cpu_init_lpis(void) gicr_write_pendbaser(val, rbase + GICR_PENDBASER); tmp = gicr_read_pendbaser(rbase + GICR_PENDBASER); + if (its->flags & ITS_FLAGS_BROKEN_SHAREABILITY) + tmp &= ~GICR_PENDBASER_SHAREABILITY_MASK; + if (!(tmp & GICR_PENDBASER_SHAREABILITY_MASK)) { /* * The HW reports non-shareable, we must remove the @@ -4765,6 +4780,13 @@ static void its_enable_quirks(struct its_node *its) u32 iidr = readl_relaxed(its->base + GITS_IIDR); gic_enable_quirks(iidr, its_quirks, its); + +#ifdef CONFIG_ROCKCHIP_ERRATUM_3588001 + if (of_machine_is_compatible("rockchip,rk3588")) { + its->flags |= ITS_FLAGS_BROKEN_SHAREABILITY; + pr_info("GIC: enabling workaround for Rockchip #3588001\n"); + } +#endif } static int its_save_disable(void) @@ -5096,6 +5118,9 @@ static int __init its_probe_one(struct resource *res, gits_write_cbaser(baser, its->base + GITS_CBASER); tmp = gits_read_cbaser(its->base + GITS_CBASER); + if (its->flags & ITS_FLAGS_BROKEN_SHAREABILITY) + tmp &= ~GITS_CBASER_SHAREABILITY_MASK; + if ((tmp ^ baser) & GITS_CBASER_SHAREABILITY_MASK) { if (!(tmp & GITS_CBASER_SHAREABILITY_MASK)) { /* -- 2.39.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] irqchip/gic-v3: Add Rockchip 3588001 errata workaround 2023-04-17 15:00 ` [PATCH v2 1/2] irqchip/gic-v3: Add Rockchip 3588001 errata workaround Sebastian Reichel @ 2023-04-17 16:20 ` Marc Zyngier 2023-04-17 21:40 ` Sebastian Reichel 0 siblings, 1 reply; 8+ messages in thread From: Marc Zyngier @ 2023-04-17 16:20 UTC (permalink / raw) To: Sebastian Reichel Cc: Heiko Stuebner, Rob Herring, Krzysztof Kozlowski, Thomas Gleixner, Peng Fan, Robin Murphy, Peter Geis, XiaoDong Huang, Kever Yang, linux-rockchip, devicetree, linux-kernel, kernel On Mon, 17 Apr 2023 16:00:37 +0100, Sebastian Reichel <sebastian.reichel@collabora.com> wrote: > > Rockchip RK3588/RK3588s GIC600 integration is not cache coherent. Thus > even though the GIC600 itself supports the sharability feature, it may > not be used. Rockchip assigned Errata ID #3588001 for this issue. > > The RK3588's GIC600 IP is using ARM's ID in the IIDR register > (0x0201743b), so it cannot be used to apply the quirk. Thus I used the > machine compatible instead. These are not incompatible requirements, see below. > > I named the flag ITS_FLAGS_BROKEN_SHAREABILITY to be vendor agnostic, > since apparently similar integration design errors exist in other > platforms and they can reuse the same flag. > > Co-developed-by: XiaoDong Huang <derrick.huang@rock-chips.com> > Signed-off-by: XiaoDong Huang <derrick.huang@rock-chips.com> > Co-developed-by: Kever Yang <kever.yang@rock-chips.com> > Signed-off-by: Kever Yang <kever.yang@rock-chips.com> > Co-developed-by: Lucas Tanure <lucas.tanure@collabora.com> > Signed-off-by: Lucas Tanure <lucas.tanure@collabora.com> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> > --- > Documentation/arm64/silicon-errata.rst | 3 +++ > arch/arm64/Kconfig | 10 ++++++++++ > drivers/irqchip/irq-gic-v3-its.c | 25 +++++++++++++++++++++++++ > 3 files changed, 38 insertions(+) > > diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst > index ec5f889d7681..46d06ed3e4f4 100644 > --- a/Documentation/arm64/silicon-errata.rst > +++ b/Documentation/arm64/silicon-errata.rst > @@ -205,6 +205,9 @@ stable kernels. > +----------------+-----------------+-----------------+-----------------------------+ > | Qualcomm Tech. | Kryo4xx Gold | N/A | ARM64_ERRATUM_1286807 | > +----------------+-----------------+-----------------+-----------------------------+ > ++----------------+-----------------+-----------------+-----------------------------+ > +| Rockchip | RK3588 | #3588001 | ROCKCHIP_ERRATUM_3588001 | > ++----------------+-----------------+-----------------+-----------------------------+ Finally. It only took two years... :-/ > > +----------------+-----------------+-----------------+-----------------------------+ > | Fujitsu | A64FX | E#010001 | FUJITSU_ERRATUM_010001 | > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index 1023e896d46b..640619459648 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -1150,6 +1150,16 @@ config NVIDIA_CARMEL_CNP_ERRATUM > > If unsure, say Y. > > +config ROCKCHIP_ERRATUM_3588001 > + bool "Rockchip 3588001: GIC600 can not support shareable attribute" s/shareable attributes/shareability attributes/ > + default y > + help > + The Rockchip RK3588 GIC600 SoC integration does not support ACE/ACE-lite > + and thus is not cache coherent. This means, that the GIC600 may not use > + the sharability feature even though it is supported by the IP itself. Shareability and cache coherence are not the same thing. The latter derive from the former, but not the other way around. So please drop any reference to cache coherency, because that's not the problem at hand. > + > + If unsure, say Y. > + > config SOCIONEXT_SYNQUACER_PREITS > bool "Socionext Synquacer: Workaround for GICv3 pre-ITS" > default y > diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c > index 586271b8aa39..0701d0b67690 100644 > --- a/drivers/irqchip/irq-gic-v3-its.c > +++ b/drivers/irqchip/irq-gic-v3-its.c > @@ -42,6 +42,7 @@ > #define ITS_FLAGS_CMDQ_NEEDS_FLUSHING (1ULL << 0) > #define ITS_FLAGS_WORKAROUND_CAVIUM_22375 (1ULL << 1) > #define ITS_FLAGS_WORKAROUND_CAVIUM_23144 (1ULL << 2) > +#define ITS_FLAGS_BROKEN_SHAREABILITY (1ULL << 3) > > #define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING (1 << 0) > #define RDIST_FLAGS_RD_TABLES_PREALLOCATED (1 << 1) > @@ -2359,6 +2360,13 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser, > its_write_baser(its, baser, val); > tmp = baser->val; > > + if (its->flags & ITS_FLAGS_BROKEN_SHAREABILITY) { > + if (tmp & GITS_BASER_SHAREABILITY_MASK) > + tmp &= ~GITS_BASER_SHAREABILITY_MASK; > + else > + gic_flush_dcache_to_poc(base, PAGE_ORDER_TO_SIZE(order)); > + } Hmmm. This doesn't really make much sense: why only clear the shareability if it is set? You know, by definition, that it is not working anyway. Also, why the clean to PoC? We already have it a few lines below... > + > if ((val ^ tmp) & GITS_BASER_SHAREABILITY_MASK) { > /* > * Shareability didn't stick. Just use > @@ -3055,6 +3063,7 @@ static u64 its_clear_vpend_valid(void __iomem *vlpi_base, u64 clr, u64 set) > > static void its_cpu_init_lpis(void) > { > + struct its_node *its = list_first_entry(&its_nodes, struct its_node, entry); > void __iomem *rbase = gic_data_rdist_rd_base(); > struct page *pend_page; > phys_addr_t paddr; > @@ -3096,6 +3105,9 @@ static void its_cpu_init_lpis(void) > gicr_write_propbaser(val, rbase + GICR_PROPBASER); > tmp = gicr_read_propbaser(rbase + GICR_PROPBASER); > > + if (its->flags & ITS_FLAGS_BROKEN_SHAREABILITY) > + tmp &= ~GICR_PROPBASER_SHAREABILITY_MASK; > + Why treat ITS and redistributor the same way? We have redistributor flags already. > if ((tmp ^ val) & GICR_PROPBASER_SHAREABILITY_MASK) { > if (!(tmp & GICR_PROPBASER_SHAREABILITY_MASK)) { > /* > @@ -3120,6 +3132,9 @@ static void its_cpu_init_lpis(void) > gicr_write_pendbaser(val, rbase + GICR_PENDBASER); > tmp = gicr_read_pendbaser(rbase + GICR_PENDBASER); > > + if (its->flags & ITS_FLAGS_BROKEN_SHAREABILITY) > + tmp &= ~GICR_PENDBASER_SHAREABILITY_MASK; > + > if (!(tmp & GICR_PENDBASER_SHAREABILITY_MASK)) { > /* > * The HW reports non-shareable, we must remove the > @@ -4765,6 +4780,13 @@ static void its_enable_quirks(struct its_node *its) > u32 iidr = readl_relaxed(its->base + GITS_IIDR); > > gic_enable_quirks(iidr, its_quirks, its); > + > +#ifdef CONFIG_ROCKCHIP_ERRATUM_3588001 > + if (of_machine_is_compatible("rockchip,rk3588")) { > + its->flags |= ITS_FLAGS_BROKEN_SHAREABILITY; > + pr_info("GIC: enabling workaround for Rockchip #3588001\n"); > + } > +#endif What prevents you from implementing this as a standard quirk and apply further filtering inside the callback? > } > > static int its_save_disable(void) > @@ -5096,6 +5118,9 @@ static int __init its_probe_one(struct resource *res, > gits_write_cbaser(baser, its->base + GITS_CBASER); > tmp = gits_read_cbaser(its->base + GITS_CBASER); > > + if (its->flags & ITS_FLAGS_BROKEN_SHAREABILITY) > + tmp &= ~GITS_CBASER_SHAREABILITY_MASK; > + And here, you correctly deal with the masking of the attributes. Why the lack of consistency? Please see below for an untested diff against your patch, addressing most of the issues mentioned here. Also, I don't see anything here addressing the *other* bug this platform suffers from, which is the 32bit limit to the allocations. Without a fix for it, this patch is pointless as the GIC may end-up with memory it cannot reach. What;s the plan for that? M. diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c index 0701d0b67690..dcdcffb4ca86 100644 --- a/drivers/irqchip/irq-gic-v3-its.c +++ b/drivers/irqchip/irq-gic-v3-its.c @@ -46,6 +46,7 @@ #define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING (1 << 0) #define RDIST_FLAGS_RD_TABLES_PREALLOCATED (1 << 1) +#define RDIST_FLAGS_BROKEN_SHAREABILITY (1 << 2) #define RD_LOCAL_LPI_ENABLED BIT(0) #define RD_LOCAL_PENDTABLE_PREALLOCATED BIT(1) @@ -2360,12 +2361,8 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser, its_write_baser(its, baser, val); tmp = baser->val; - if (its->flags & ITS_FLAGS_BROKEN_SHAREABILITY) { - if (tmp & GITS_BASER_SHAREABILITY_MASK) - tmp &= ~GITS_BASER_SHAREABILITY_MASK; - else - gic_flush_dcache_to_poc(base, PAGE_ORDER_TO_SIZE(order)); - } + if (its->flags & ITS_FLAGS_BROKEN_SHAREABILITY) + tmp &= ~GITS_BASER_SHAREABILITY_MASK; if ((val ^ tmp) & GITS_BASER_SHAREABILITY_MASK) { /* @@ -3063,7 +3060,6 @@ static u64 its_clear_vpend_valid(void __iomem *vlpi_base, u64 clr, u64 set) static void its_cpu_init_lpis(void) { - struct its_node *its = list_first_entry(&its_nodes, struct its_node, entry); void __iomem *rbase = gic_data_rdist_rd_base(); struct page *pend_page; phys_addr_t paddr; @@ -3105,7 +3101,7 @@ static void its_cpu_init_lpis(void) gicr_write_propbaser(val, rbase + GICR_PROPBASER); tmp = gicr_read_propbaser(rbase + GICR_PROPBASER); - if (its->flags & ITS_FLAGS_BROKEN_SHAREABILITY) + if (gic_rdists->flags & RDIST_FLAGS_BROKEN_SHAREABILITY) tmp &= ~GICR_PROPBASER_SHAREABILITY_MASK; if ((tmp ^ val) & GICR_PROPBASER_SHAREABILITY_MASK) { @@ -3132,7 +3128,7 @@ static void its_cpu_init_lpis(void) gicr_write_pendbaser(val, rbase + GICR_PENDBASER); tmp = gicr_read_pendbaser(rbase + GICR_PENDBASER); - if (its->flags & ITS_FLAGS_BROKEN_SHAREABILITY) + if (gic_rdists->flags & RDIST_FLAGS_BROKEN_SHAREABILITY) tmp &= ~GICR_PENDBASER_SHAREABILITY_MASK; if (!(tmp & GICR_PENDBASER_SHAREABILITY_MASK)) { @@ -4725,6 +4721,19 @@ static bool __maybe_unused its_enable_quirk_hip07_161600802(void *data) return true; } +static bool __maybe_unused its_enable_rk3388001(void *data) +{ + struct its_node *its = data; + + if (!of_machine_is_compatible("rockchip,rk3588")) + return false; + + its->flags |= ITS_FLAGS_BROKEN_SHAREABILITY; + gic_rdists->flags |= RDIST_FLAGS_BROKEN_SHAREABILITY; + + return true; +} + static const struct gic_quirk its_quirks[] = { #ifdef CONFIG_CAVIUM_ERRATUM_22375 { @@ -4770,6 +4779,14 @@ static const struct gic_quirk its_quirks[] = { .mask = 0xffffffff, .init = its_enable_quirk_hip07_161600802, }, +#endif +#ifdef CONFIG_ROCKCHIP_ERRATUM_3588001 + { + .desc = "ITS: Rockchip erratum RK3588001", + .iidr = 0x0201743b, + .mask = 0xffffffff, + .init = its_enable_rk3388001, + }, #endif { } @@ -4780,13 +4797,6 @@ static void its_enable_quirks(struct its_node *its) u32 iidr = readl_relaxed(its->base + GITS_IIDR); gic_enable_quirks(iidr, its_quirks, its); - -#ifdef CONFIG_ROCKCHIP_ERRATUM_3588001 - if (of_machine_is_compatible("rockchip,rk3588")) { - its->flags |= ITS_FLAGS_BROKEN_SHAREABILITY; - pr_info("GIC: enabling workaround for Rockchip #3588001\n"); - } -#endif } static int its_save_disable(void) -- Without deviation from the norm, progress is not possible. ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] irqchip/gic-v3: Add Rockchip 3588001 errata workaround 2023-04-17 16:20 ` Marc Zyngier @ 2023-04-17 21:40 ` Sebastian Reichel 2023-04-18 8:58 ` Marc Zyngier 0 siblings, 1 reply; 8+ messages in thread From: Sebastian Reichel @ 2023-04-17 21:40 UTC (permalink / raw) To: Marc Zyngier Cc: Heiko Stuebner, Rob Herring, Krzysztof Kozlowski, Thomas Gleixner, Peng Fan, Robin Murphy, Peter Geis, XiaoDong Huang, Kever Yang, linux-rockchip, devicetree, linux-kernel, kernel [-- Attachment #1: Type: text/plain, Size: 567 bytes --] Hi, On Mon, Apr 17, 2023 at 05:20:08PM +0100, Marc Zyngier wrote: >> [...] > Please see below for an untested diff against your patch, addressing > most of the issues mentioned here. Thanks, looks good. I integrated the changes into v3. > Also, I don't see anything here addressing the *other* bug this > platform suffers from, which is the 32bit limit to the allocations. > Without a fix for it, this patch is pointless as the GIC may end-up > with memory it cannot reach. > > What;s the plan for that? It got fixed in RK3588. > [...] Greetings, -- Sebastian [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] irqchip/gic-v3: Add Rockchip 3588001 errata workaround 2023-04-17 21:40 ` Sebastian Reichel @ 2023-04-18 8:58 ` Marc Zyngier 2023-04-18 14:34 ` Sebastian Reichel 0 siblings, 1 reply; 8+ messages in thread From: Marc Zyngier @ 2023-04-18 8:58 UTC (permalink / raw) To: Sebastian Reichel Cc: Heiko Stuebner, Rob Herring, Krzysztof Kozlowski, Thomas Gleixner, Peng Fan, Robin Murphy, Peter Geis, XiaoDong Huang, Kever Yang, linux-rockchip, devicetree, linux-kernel, kernel On Mon, 17 Apr 2023 22:40:33 +0100, Sebastian Reichel <sebastian.reichel@collabora.com> wrote: > > Hi, > > On Mon, Apr 17, 2023 at 05:20:08PM +0100, Marc Zyngier wrote: > >> [...] > > Please see below for an untested diff against your patch, addressing > > most of the issues mentioned here. > > Thanks, looks good. I integrated the changes into v3. > > > Also, I don't see anything here addressing the *other* bug this > > platform suffers from, which is the 32bit limit to the allocations. > > Without a fix for it, this patch is pointless as the GIC may end-up > > with memory it cannot reach. > > > > What;s the plan for that? > > It got fixed in RK3588. So what's the plan for the affected SoCs? It might be a good idea to address it too, unless there are none of them in the wild? M. -- Without deviation from the norm, progress is not possible. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] irqchip/gic-v3: Add Rockchip 3588001 errata workaround 2023-04-18 8:58 ` Marc Zyngier @ 2023-04-18 14:34 ` Sebastian Reichel 0 siblings, 0 replies; 8+ messages in thread From: Sebastian Reichel @ 2023-04-18 14:34 UTC (permalink / raw) To: Marc Zyngier Cc: Heiko Stuebner, Rob Herring, Krzysztof Kozlowski, Thomas Gleixner, Peng Fan, Robin Murphy, Peter Geis, XiaoDong Huang, Kever Yang, linux-rockchip, devicetree, linux-kernel, kernel [-- Attachment #1: Type: text/plain, Size: 1185 bytes --] Hi, On Tue, Apr 18, 2023 at 09:58:17AM +0100, Marc Zyngier wrote: > On Mon, 17 Apr 2023 22:40:33 +0100, > Sebastian Reichel <sebastian.reichel@collabora.com> wrote: > > On Mon, Apr 17, 2023 at 05:20:08PM +0100, Marc Zyngier wrote: > > > Also, I don't see anything here addressing the *other* bug this > > > platform suffers from, which is the 32bit limit to the allocations. > > > Without a fix for it, this patch is pointless as the GIC may end-up > > > with memory it cannot reach. > > > > > > What;s the plan for that? > > > > It got fixed in RK3588. > > So what's the plan for the affected SoCs? It might be a good idea to > address it too, unless there are none of them in the wild? The previous generation (RK3566 and RK3568) is affected by the 32 bit issue as well as the shareability issue. I'm currently focusing on the RK3588 and that's the only Rockchip platform I have on my desk. Missing PCIe support is a lot more critical on RK3588, because multiple board designers decided to use a 2.5G PCIe network card instead of using the 1G network MAC from the SoC. Anyways, I will try to help out with RK356x once I find some time. -- Sebastian [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] arm64: dts: rockchip: rk3588: add MSI support 2023-04-17 15:00 [PATCH v2 0/2] Add Rockchip RK3588 GIC ITS support Sebastian Reichel 2023-04-17 15:00 ` [PATCH v2 1/2] irqchip/gic-v3: Add Rockchip 3588001 errata workaround Sebastian Reichel @ 2023-04-17 15:00 ` Sebastian Reichel 2023-04-17 16:39 ` Marc Zyngier 1 sibling, 1 reply; 8+ messages in thread From: Sebastian Reichel @ 2023-04-17 15:00 UTC (permalink / raw) To: Marc Zyngier, Heiko Stuebner Cc: Rob Herring, Krzysztof Kozlowski, Thomas Gleixner, Peng Fan, Robin Murphy, Peter Geis, XiaoDong Huang, Kever Yang, linux-rockchip, devicetree, linux-kernel, Sebastian Reichel, kernel Add the two Interrupt Translation Service (ITS) IPs that are part of the GIC-600, which are required for message signalled interrupts (MSI). This is required for PCIe, which fully relies on MSI for interrupts. Enabling the ITS nodes requires an operating system, that has a workaround for Rockchip errata #3588001 (GIC600 can not support shareable attribute). Co-developed-by: Kever Yang <kever.yang@rock-chips.com> Signed-off-by: Kever Yang <kever.yang@rock-chips.com> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> --- arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi index 2124c654f665..62204b96b0b4 100644 --- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi +++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi @@ -1741,7 +1741,24 @@ gic: interrupt-controller@fe600000 { mbi-alias = <0x0 0xfe610000>; mbi-ranges = <424 56>; msi-controller; + ranges; + #address-cells = <2>; #interrupt-cells = <4>; + #size-cells = <2>; + + its0: msi-controller@fe640000 { + compatible = "arm,gic-v3-its"; + msi-controller; + #msi-cells = <1>; + reg = <0x0 0xfe640000 0x0 0x20000>; + }; + + its1: msi-controller@fe660000 { + compatible = "arm,gic-v3-its"; + msi-controller; + #msi-cells = <1>; + reg = <0x0 0xfe660000 0x0 0x20000>; + }; ppi-partitions { ppi_partition0: interrupt-partition-0 { -- 2.39.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] arm64: dts: rockchip: rk3588: add MSI support 2023-04-17 15:00 ` [PATCH v2 2/2] arm64: dts: rockchip: rk3588: add MSI support Sebastian Reichel @ 2023-04-17 16:39 ` Marc Zyngier 0 siblings, 0 replies; 8+ messages in thread From: Marc Zyngier @ 2023-04-17 16:39 UTC (permalink / raw) To: Sebastian Reichel Cc: Heiko Stuebner, Rob Herring, Krzysztof Kozlowski, Thomas Gleixner, Peng Fan, Robin Murphy, Peter Geis, XiaoDong Huang, Kever Yang, linux-rockchip, devicetree, linux-kernel, kernel On Mon, 17 Apr 2023 16:00:38 +0100, Sebastian Reichel <sebastian.reichel@collabora.com> wrote: > > Add the two Interrupt Translation Service (ITS) IPs that are part of the > GIC-600, which are required for message signalled interrupts (MSI). This > is required for PCIe, which fully relies on MSI for interrupts. > > Enabling the ITS nodes requires an operating system, that has a > workaround for Rockchip errata #3588001 (GIC600 can not support > shareable attribute). $SUBJECT is inaccurate. This adds the descriptions of the ITSs, but doesn't wire any MSI support at all (nobody even refers these nodes, so the whole thing is pretty pointless). M. -- Without deviation from the norm, progress is not possible. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-04-18 14:34 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-04-17 15:00 [PATCH v2 0/2] Add Rockchip RK3588 GIC ITS support Sebastian Reichel 2023-04-17 15:00 ` [PATCH v2 1/2] irqchip/gic-v3: Add Rockchip 3588001 errata workaround Sebastian Reichel 2023-04-17 16:20 ` Marc Zyngier 2023-04-17 21:40 ` Sebastian Reichel 2023-04-18 8:58 ` Marc Zyngier 2023-04-18 14:34 ` Sebastian Reichel 2023-04-17 15:00 ` [PATCH v2 2/2] arm64: dts: rockchip: rk3588: add MSI support Sebastian Reichel 2023-04-17 16:39 ` Marc Zyngier
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).