| Message ID | 20241104121143.2169264-1-wangzhou1@hisilicon.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | irqchip/gicv3-its: Add workaround for hip09 ITS erratum 162100801 | expand |
On Mon, 04 Nov 2024 12:11:43 +0000, Zhou Wang <wangzhou1@hisilicon.com> wrote: > > When enabled GICv4.1 in hip09, VMAPP will fail to clear some caches nit: enabling. > during unmapping operation, which will cause some vSGIs interrupts lost. > > To fix the issue, it needs to send vinvall command after vmovp. nit: commands need to be in capital letters (VINVALL, VMOVP). drop 'interrupts'. > > Signed-off-by: Nianyao Tang <tangnianyao@huawei.com> > Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com> > --- > Documentation/arch/arm64/silicon-errata.rst | 2 ++ > arch/arm64/Kconfig | 10 ++++++ > drivers/irqchip/irq-gic-v3-its.c | 36 ++++++++++++++++----- > 3 files changed, 40 insertions(+), 8 deletions(-) > > diff --git a/Documentation/arch/arm64/silicon-errata.rst b/Documentation/arch/arm64/silicon-errata.rst > index 65bfab1b1861..77db10e944f0 100644 > --- a/Documentation/arch/arm64/silicon-errata.rst > +++ b/Documentation/arch/arm64/silicon-errata.rst > @@ -258,6 +258,8 @@ stable kernels. > | Hisilicon | Hip{08,09,10,10C| #162001900 | N/A | > | | ,11} SMMU PMCG | | | > +----------------+-----------------+-----------------+-----------------------------+ > +| Hisilicon | Hip09 | #162100801 | HISILICON_ERRATUM_162100801 | > ++----------------+-----------------+-----------------+-----------------------------+ > +----------------+-----------------+-----------------+-----------------------------+ > | Qualcomm Tech. | Kryo/Falkor v1 | E1003 | QCOM_FALKOR_ERRATUM_1003 | > +----------------+-----------------+-----------------+-----------------------------+ > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index fd9df6dcc593..27082e075d1a 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -1232,6 +1232,16 @@ config HISILICON_ERRATUM_161600802 > > If unsure, say Y. > > +config HISILICON_ERRATUM_162100801 > + bool "Hip09 162100801 erratum support" > + default y > + help > + When enabled GICv4.1 in hip09, VMAPP will fail to clear some caches > + during unmapping operation, which will cause some vSGIs interrupts > + lost. So fix it by sending vinvall commands after vmovp. Same remarks as above. Now, the workaround seems odd. I understand from the description that VMAPP with V=0 results in leftover cached entries influencing the behaviour of a newly created VPE. But if it is a VINVALL on each CPU that cures it, why don't we do that upfront, instead of on every single VMOVP? Or is the problem that VMAPP(V=0) can result in *existing* VPEs to observe an unrelated (or corrupted) state? > + > + If unsure, say Y. > + > config QCOM_FALKOR_ERRATUM_1003 > bool "Falkor E1003: Incorrect translation due to ASID change" > default y > diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c > index 52f625e07658..69b09072d24d 100644 > --- a/drivers/irqchip/irq-gic-v3-its.c > +++ b/drivers/irqchip/irq-gic-v3-its.c > @@ -44,6 +44,7 @@ > #define ITS_FLAGS_WORKAROUND_CAVIUM_22375 (1ULL << 1) > #define ITS_FLAGS_WORKAROUND_CAVIUM_23144 (1ULL << 2) > #define ITS_FLAGS_FORCE_NON_SHAREABLE (1ULL << 3) > +#define ITS_FLAGS_WORKAROUND_HISILICON_162100801 (1ULL << 4) > > #define RD_LOCAL_LPI_ENABLED BIT(0) > #define RD_LOCAL_PENDTABLE_PREALLOCATED BIT(1) > @@ -1314,6 +1315,14 @@ static void its_send_vmapp(struct its_node *its, > its_send_single_vcommand(its, its_build_vmapp_cmd, &desc); > } > > +static void its_send_vinvall(struct its_node *its, struct its_vpe *vpe) > +{ > + struct its_cmd_desc desc; > + > + desc.its_vinvall_cmd.vpe = vpe; > + its_send_single_vcommand(its, its_build_vinvall_cmd, &desc); > +} > + > static void its_send_vmovp(struct its_vpe *vpe) > { > struct its_cmd_desc desc = {}; > @@ -1351,17 +1360,12 @@ static void its_send_vmovp(struct its_vpe *vpe) > > desc.its_vmovp_cmd.col = &its->collections[col_id]; > its_send_single_vcommand(its, its_build_vmovp_cmd, &desc); > + if (is_v4_1(its) && (its->flags & > + ITS_FLAGS_WORKAROUND_HISILICON_162100801)) > + its_send_vinvall(its, vpe); > } > } Please don't bury workarounds inside the command primitives (and VMOVP is complicated enough). There is a single place where we send VMOVP, and I'd rather you place the workaround there. But more importantly, this code isn't supposed to get executed with a GICv4.1 implementation, as the driver does not expect it to use an ITSList. So what is your system implementing? Do you have such thing as GICv4.1 with ITSList? If so, I'm pretty sure the driver is broken on with such mix... Thanks, M.
On 2024/11/5 16:24, Marc Zyngier wrote: > On Mon, 04 Nov 2024 12:11:43 +0000, > Zhou Wang <wangzhou1@hisilicon.com> wrote: >> >> When enabled GICv4.1 in hip09, VMAPP will fail to clear some caches > > nit: enabling. Will fix, thanks. > >> during unmapping operation, which will cause some vSGIs interrupts lost. >> >> To fix the issue, it needs to send vinvall command after vmovp. > > nit: commands need to be in capital letters (VINVALL, VMOVP). drop > 'interrupts'. Will fix, thanks. > >> >> Signed-off-by: Nianyao Tang <tangnianyao@huawei.com> >> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com> >> --- >> Documentation/arch/arm64/silicon-errata.rst | 2 ++ >> arch/arm64/Kconfig | 10 ++++++ >> drivers/irqchip/irq-gic-v3-its.c | 36 ++++++++++++++++----- >> 3 files changed, 40 insertions(+), 8 deletions(-) >> >> diff --git a/Documentation/arch/arm64/silicon-errata.rst b/Documentation/arch/arm64/silicon-errata.rst >> index 65bfab1b1861..77db10e944f0 100644 >> --- a/Documentation/arch/arm64/silicon-errata.rst >> +++ b/Documentation/arch/arm64/silicon-errata.rst >> @@ -258,6 +258,8 @@ stable kernels. >> | Hisilicon | Hip{08,09,10,10C| #162001900 | N/A | >> | | ,11} SMMU PMCG | | | >> +----------------+-----------------+-----------------+-----------------------------+ >> +| Hisilicon | Hip09 | #162100801 | HISILICON_ERRATUM_162100801 | >> ++----------------+-----------------+-----------------+-----------------------------+ >> +----------------+-----------------+-----------------+-----------------------------+ >> | Qualcomm Tech. | Kryo/Falkor v1 | E1003 | QCOM_FALKOR_ERRATUM_1003 | >> +----------------+-----------------+-----------------+-----------------------------+ >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig >> index fd9df6dcc593..27082e075d1a 100644 >> --- a/arch/arm64/Kconfig >> +++ b/arch/arm64/Kconfig >> @@ -1232,6 +1232,16 @@ config HISILICON_ERRATUM_161600802 >> >> If unsure, say Y. >> >> +config HISILICON_ERRATUM_162100801 >> + bool "Hip09 162100801 erratum support" >> + default y >> + help >> + When enabled GICv4.1 in hip09, VMAPP will fail to clear some caches >> + during unmapping operation, which will cause some vSGIs interrupts >> + lost. So fix it by sending vinvall commands after vmovp. > > Same remarks as above. OK. > > Now, the workaround seems odd. I understand from the description that > VMAPP with V=0 results in leftover cached entries influencing the > behaviour of a newly created VPE. > VMAPP(V=0) only effects the cached entries of current VPE, the hardware problem is that VMAPP(V=0) will fail to clean cached entries in other CPU dies. > But if it is a VINVALL on each CPU that cures it, why don't we do that > upfront, instead of on every single VMOVP? As the reason above, even we add VINVALL before VMAPP(V=0), it can only clean cached entries in current CPU die, there are still cached entries left in other part of the system. > > Or is the problem that VMAPP(V=0) can result in *existing* VPEs to > observe an unrelated (or corrupted) state? > >> + >> + If unsure, say Y. >> + >> config QCOM_FALKOR_ERRATUM_1003 >> bool "Falkor E1003: Incorrect translation due to ASID change" >> default y >> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c >> index 52f625e07658..69b09072d24d 100644 >> --- a/drivers/irqchip/irq-gic-v3-its.c >> +++ b/drivers/irqchip/irq-gic-v3-its.c >> @@ -44,6 +44,7 @@ >> #define ITS_FLAGS_WORKAROUND_CAVIUM_22375 (1ULL << 1) >> #define ITS_FLAGS_WORKAROUND_CAVIUM_23144 (1ULL << 2) >> #define ITS_FLAGS_FORCE_NON_SHAREABLE (1ULL << 3) >> +#define ITS_FLAGS_WORKAROUND_HISILICON_162100801 (1ULL << 4) >> >> #define RD_LOCAL_LPI_ENABLED BIT(0) >> #define RD_LOCAL_PENDTABLE_PREALLOCATED BIT(1) >> @@ -1314,6 +1315,14 @@ static void its_send_vmapp(struct its_node *its, >> its_send_single_vcommand(its, its_build_vmapp_cmd, &desc); >> } >> >> +static void its_send_vinvall(struct its_node *its, struct its_vpe *vpe) >> +{ >> + struct its_cmd_desc desc; >> + >> + desc.its_vinvall_cmd.vpe = vpe; >> + its_send_single_vcommand(its, its_build_vinvall_cmd, &desc); >> +} >> + >> static void its_send_vmovp(struct its_vpe *vpe) >> { >> struct its_cmd_desc desc = {}; >> @@ -1351,17 +1360,12 @@ static void its_send_vmovp(struct its_vpe *vpe) >> >> desc.its_vmovp_cmd.col = &its->collections[col_id]; >> its_send_single_vcommand(its, its_build_vmovp_cmd, &desc); >> + if (is_v4_1(its) && (its->flags & >> + ITS_FLAGS_WORKAROUND_HISILICON_162100801)) >> + its_send_vinvall(its, vpe); >> } >> } > > Please don't bury workarounds inside the command primitives (and VMOVP > is complicated enough). There is a single place where we send VMOVP, > and I'd rather you place the workaround there. OK, will add VINVALL after its_send_vmovp. > > But more importantly, this code isn't supposed to get executed with a > GICv4.1 implementation, as the driver does not expect it to use an > ITSList. > > So what is your system implementing? Do you have such thing as GICv4.1 > with ITSList? If so, I'm pretty sure the driver is broken on with such > mix... I don't get the idea here, our system is as below, let's see if there is a problem here. Our system is GICv4.1 with multiple ITSs, GITS_TYPER.vmovp is 0, so VMOVP should be sent to each ITS. In its_send_vmovp, its_list_map will not be 0. We are GICv4.1(GICR_TYPER.RVPEID is 1), so require_its_list_vmovp will be true. It seems VMOVP can be called here. Thanks, Zhou > > Thanks, > > M. >
diff --git a/Documentation/arch/arm64/silicon-errata.rst b/Documentation/arch/arm64/silicon-errata.rst index 65bfab1b1861..77db10e944f0 100644 --- a/Documentation/arch/arm64/silicon-errata.rst +++ b/Documentation/arch/arm64/silicon-errata.rst @@ -258,6 +258,8 @@ stable kernels. | Hisilicon | Hip{08,09,10,10C| #162001900 | N/A | | | ,11} SMMU PMCG | | | +----------------+-----------------+-----------------+-----------------------------+ +| Hisilicon | Hip09 | #162100801 | HISILICON_ERRATUM_162100801 | ++----------------+-----------------+-----------------+-----------------------------+ +----------------+-----------------+-----------------+-----------------------------+ | Qualcomm Tech. | Kryo/Falkor v1 | E1003 | QCOM_FALKOR_ERRATUM_1003 | +----------------+-----------------+-----------------+-----------------------------+ diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index fd9df6dcc593..27082e075d1a 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -1232,6 +1232,16 @@ config HISILICON_ERRATUM_161600802 If unsure, say Y. +config HISILICON_ERRATUM_162100801 + bool "Hip09 162100801 erratum support" + default y + help + When enabled GICv4.1 in hip09, VMAPP will fail to clear some caches + during unmapping operation, which will cause some vSGIs interrupts + lost. So fix it by sending vinvall commands after vmovp. + + If unsure, say Y. + config QCOM_FALKOR_ERRATUM_1003 bool "Falkor E1003: Incorrect translation due to ASID change" default y diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c index 52f625e07658..69b09072d24d 100644 --- a/drivers/irqchip/irq-gic-v3-its.c +++ b/drivers/irqchip/irq-gic-v3-its.c @@ -44,6 +44,7 @@ #define ITS_FLAGS_WORKAROUND_CAVIUM_22375 (1ULL << 1) #define ITS_FLAGS_WORKAROUND_CAVIUM_23144 (1ULL << 2) #define ITS_FLAGS_FORCE_NON_SHAREABLE (1ULL << 3) +#define ITS_FLAGS_WORKAROUND_HISILICON_162100801 (1ULL << 4) #define RD_LOCAL_LPI_ENABLED BIT(0) #define RD_LOCAL_PENDTABLE_PREALLOCATED BIT(1) @@ -1314,6 +1315,14 @@ static void its_send_vmapp(struct its_node *its, its_send_single_vcommand(its, its_build_vmapp_cmd, &desc); } +static void its_send_vinvall(struct its_node *its, struct its_vpe *vpe) +{ + struct its_cmd_desc desc; + + desc.its_vinvall_cmd.vpe = vpe; + its_send_single_vcommand(its, its_build_vinvall_cmd, &desc); +} + static void its_send_vmovp(struct its_vpe *vpe) { struct its_cmd_desc desc = {}; @@ -1351,17 +1360,12 @@ static void its_send_vmovp(struct its_vpe *vpe) desc.its_vmovp_cmd.col = &its->collections[col_id]; its_send_single_vcommand(its, its_build_vmovp_cmd, &desc); + if (is_v4_1(its) && (its->flags & + ITS_FLAGS_WORKAROUND_HISILICON_162100801)) + its_send_vinvall(its, vpe); } } -static void its_send_vinvall(struct its_node *its, struct its_vpe *vpe) -{ - struct its_cmd_desc desc; - - desc.its_vinvall_cmd.vpe = vpe; - its_send_single_vcommand(its, its_build_vinvall_cmd, &desc); -} - static void its_send_vinv(struct its_device *dev, u32 event_id) { struct its_cmd_desc desc; @@ -4781,6 +4785,14 @@ static bool its_set_non_coherent(void *data) return true; } +static bool __maybe_unused its_enable_quirk_hip09_162100801(void *data) +{ + struct its_node *its = data; + + its->flags |= ITS_FLAGS_WORKAROUND_HISILICON_162100801; + return true; +} + static const struct gic_quirk its_quirks[] = { #ifdef CONFIG_CAVIUM_ERRATUM_22375 { @@ -4827,6 +4839,14 @@ static const struct gic_quirk its_quirks[] = { .init = its_enable_quirk_hip07_161600802, }, #endif +#ifdef CONFIG_HISILICON_ERRATUM_162100801 + { + .desc = "ITS: Hip09 erratum 162100801", + .iidr = 0x00051736, + .mask = 0xffffffff, + .init = its_enable_quirk_hip09_162100801, + }, +#endif #ifdef CONFIG_ROCKCHIP_ERRATUM_3588001 { .desc = "ITS: Rockchip erratum RK3588001",