2023-10-30 07:01:47

by Fang Xiang

[permalink] [raw]
Subject: [PATCH v3] irqchip/gic-v3-its: Flush ITS tables before writing GITS_BASER<n> registers in non-coherent GIC designs.

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.

Signed-off-by: Marc Zyngier <[email protected]>
Signed-off-by: Fang Xiang <[email protected]>
Tested-by: Fang Xiang <[email protected]>

Link to v2:
https://lore.kernel.org/all/[email protected]/

Link to v1:
https://lore.kernel.org/all/[email protected]/

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


2023-10-30 07:48:52

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v3] irqchip/gic-v3-its: Flush ITS tables before writing GITS_BASER<n> registers in non-coherent GIC designs.

On Mon, 30 Oct 2023 07:00:20 +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.
>
> Signed-off-by: Marc Zyngier <[email protected]>

Please read the documentation[1]. You cannot use my own SoB yourself.
*I* put it on patches *I* author, but you don't get to do that unless
you put me as the author (which isn't the case here). The original
suggestion from Thomas was 'Originally-by:', and 'Suggested-by:' would
work as well.

> Signed-off-by: Fang Xiang <[email protected]>
> Tested-by: Fang Xiang <[email protected]>
>
> Link to v2:
> https://lore.kernel.org/all/[email protected]/
>
> Link to v1:
> https://lore.kernel.org/all/[email protected]/
>
> 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

All this needs to go as a note *under* the '---' line.

Thanks,

M.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst#n396

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