In non-coherent GIC design, ITS tables should be clean and flushed
to the PoV of the ITS before writing GITS_BASER<n> registers, otherwise
the ITS would read dirty tables and lead to UNPREDICTABLE behaviors.
The ITS always got clean tables in initialization with this fix, by
observing the signals from GIC.
Furthermore, hoist the quirked non-shareable attributes earlier to
save effort in tables setup.
Suggested-by: Marc Zyngier <[email protected]>
Signed-off-by: Fang Xiang <[email protected]>
Tested-by: Fang Xiang <[email protected]>
---
Link to v3:
https://lore.kernel.org/all/[email protected]/
Link to v2:
https://lore.kernel.org/all/[email protected]/
Link to v1:
https://lore.kernel.org/all/[email protected]/
v3-> v4:
- Change 'Signed-off-by' trailer to 'Suggested-by' for original
author Marc
- Fix the format of commit message
v2 -> v3:
- Add 'Signed-off-by' trailer for original author Marc
- Expand commit message with testing result
v1 -> v2:
- Flush ITS tables before writing GITS_BASER<n> registers
- Hoist the quirked non-shareable attributes earlier
---
drivers/irqchip/irq-gic-v3-its.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 75a2dd550625..d76d44ea2de1 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -2379,12 +2379,12 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
break;
}
+ if (!shr)
+ gic_flush_dcache_to_poc(base, PAGE_ORDER_TO_SIZE(order));
+
its_write_baser(its, baser, val);
tmp = baser->val;
- if (its->flags & ITS_FLAGS_FORCE_NON_SHAREABLE)
- tmp &= ~GITS_BASER_SHAREABILITY_MASK;
-
if ((val ^ tmp) & GITS_BASER_SHAREABILITY_MASK) {
/*
* Shareability didn't stick. Just use
@@ -2394,10 +2394,9 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
* non-cacheable as well.
*/
shr = tmp & GITS_BASER_SHAREABILITY_MASK;
- if (!shr) {
+ if (!shr)
cache = GITS_BASER_nC;
- gic_flush_dcache_to_poc(base, PAGE_ORDER_TO_SIZE(order));
- }
+
goto retry_baser;
}
@@ -2609,6 +2608,11 @@ static int its_alloc_tables(struct its_node *its)
/* erratum 24313: ignore memory access type */
cache = GITS_BASER_nCnB;
+ if (its->flags & ITS_FLAGS_FORCE_NON_SHAREABLE) {
+ cache = GITS_BASER_nC;
+ shr = 0;
+ }
+
for (i = 0; i < GITS_BASER_NR_REGS; i++) {
struct its_baser *baser = its->tables + i;
u64 val = its_read_baser(its, baser);
--
2.34.1
On Mon, 30 Oct 2023 08:32:56 +0000,
Fang Xiang <[email protected]> wrote:
>
> In non-coherent GIC design, ITS tables should be clean and flushed
> to the PoV of the ITS before writing GITS_BASER<n> registers, otherwise
> the ITS would read dirty tables and lead to UNPREDICTABLE behaviors.
>
> The ITS always got clean tables in initialization with this fix, by
> observing the signals from GIC.
>
> Furthermore, hoist the quirked non-shareable attributes earlier to
> save effort in tables setup.
>
> Suggested-by: Marc Zyngier <[email protected]>
> Signed-off-by: Fang Xiang <[email protected]>
> Tested-by: Fang Xiang <[email protected]>
Reviewed-by: Marc Zyngier <[email protected]>
M.
--
Without deviation from the norm, progress is not possible.
On Sat, Nov 04 2023 at 09:56, Marc Zyngier wrote:
> On Mon, 30 Oct 2023 08:32:56 +0000,
> Fang Xiang <[email protected]> wrote:
>>
>> In non-coherent GIC design, ITS tables should be clean and flushed
>> to the PoV of the ITS before writing GITS_BASER<n> registers, otherwise
>> the ITS would read dirty tables and lead to UNPREDICTABLE behaviors.
>>
>> The ITS always got clean tables in initialization with this fix, by
>> observing the signals from GIC.
>>
>> Furthermore, hoist the quirked non-shareable attributes earlier to
>> save effort in tables setup.
>>
>> Suggested-by: Marc Zyngier <[email protected]>
>> Signed-off-by: Fang Xiang <[email protected]>
>> Tested-by: Fang Xiang <[email protected]>
>
> Reviewed-by: Marc Zyngier <[email protected]>
Shouldn't this have a Fixes tag? My guess is:
a8707f553884 ("irqchip/gic-v3: Add Rockchip 3588001 erratum workaround")
On Sun, 05 Nov 2023 08:55:11 +0000,
Thomas Gleixner <[email protected]> wrote:
>
> On Sat, Nov 04 2023 at 09:56, Marc Zyngier wrote:
> > On Mon, 30 Oct 2023 08:32:56 +0000,
> > Fang Xiang <[email protected]> wrote:
> >>
> >> In non-coherent GIC design, ITS tables should be clean and flushed
> >> to the PoV of the ITS before writing GITS_BASER<n> registers, otherwise
> >> the ITS would read dirty tables and lead to UNPREDICTABLE behaviors.
> >>
> >> The ITS always got clean tables in initialization with this fix, by
> >> observing the signals from GIC.
> >>
> >> Furthermore, hoist the quirked non-shareable attributes earlier to
> >> save effort in tables setup.
> >>
> >> Suggested-by: Marc Zyngier <[email protected]>
> >> Signed-off-by: Fang Xiang <[email protected]>
> >> Tested-by: Fang Xiang <[email protected]>
> >
> > Reviewed-by: Marc Zyngier <[email protected]>
>
> Shouldn't this have a Fixes tag? My guess is:
>
> a8707f553884 ("irqchip/gic-v3: Add Rockchip 3588001 erratum workaround")
Yes, that's indeed the point where the out of sequence programming can
occur.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
The following commit has been merged into the irq/urgent branch of tip:
Commit-ID: d3badb15613c14dd35d3495b1dde5c90fcd616dd
Gitweb: https://git.kernel.org/tip/d3badb15613c14dd35d3495b1dde5c90fcd616dd
Author: Fang Xiang <[email protected]>
AuthorDate: Mon, 30 Oct 2023 16:32:56 +08:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Mon, 06 Nov 2023 01:16:33 +01:00
irqchip/gic-v3-its: Flush ITS tables correctly in non-coherent GIC designs
In non-coherent GIC designs, the ITS tables must be flushed before writing
to the GITS_BASER<n> registers, otherwise the ITS could read dirty tables,
which results in unpredictable behavior.
Flush the tables right at the begin of its_setup_baser() to prevent that.
[ tglx: Massage changelog ]
Fixes: a8707f553884 ("irqchip/gic-v3: Add Rockchip 3588001 erratum workaround")
Suggested-by: Marc Zyngier <[email protected]>
Signed-off-by: Fang Xiang <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Reviewed-by: Marc Zyngier <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/irqchip/irq-gic-v3-its.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index a8c89df..9a7a742 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -2379,12 +2379,12 @@ retry_baser:
break;
}
+ if (!shr)
+ gic_flush_dcache_to_poc(base, PAGE_ORDER_TO_SIZE(order));
+
its_write_baser(its, baser, val);
tmp = baser->val;
- if (its->flags & ITS_FLAGS_FORCE_NON_SHAREABLE)
- tmp &= ~GITS_BASER_SHAREABILITY_MASK;
-
if ((val ^ tmp) & GITS_BASER_SHAREABILITY_MASK) {
/*
* Shareability didn't stick. Just use
@@ -2394,10 +2394,9 @@ retry_baser:
* non-cacheable as well.
*/
shr = tmp & GITS_BASER_SHAREABILITY_MASK;
- if (!shr) {
+ if (!shr)
cache = GITS_BASER_nC;
- gic_flush_dcache_to_poc(base, PAGE_ORDER_TO_SIZE(order));
- }
+
goto retry_baser;
}
@@ -2609,6 +2608,11 @@ static int its_alloc_tables(struct its_node *its)
/* erratum 24313: ignore memory access type */
cache = GITS_BASER_nCnB;
+ if (its->flags & ITS_FLAGS_FORCE_NON_SHAREABLE) {
+ cache = GITS_BASER_nC;
+ shr = 0;
+ }
+
for (i = 0; i < GITS_BASER_NR_REGS; i++) {
struct its_baser *baser = its->tables + i;
u64 val = its_read_baser(its, baser);