mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/3] irqchip/gic-v3-its: Fix how zeroed table pages reach the GIC
@ 2026-11-06 12:02 Fuad Tabba
  2026-11-06 12:02 ` [PATCH 1/3] irqchip/gic-v3-its: Zero ITS pages before sharing them with the host Fuad Tabba
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Fuad Tabba @ 2026-11-06 12:02 UTC (permalink / raw)
  To: maz, tglx
  Cc: radu, lpieralisi, steven.price, suzuki.poulose, linux-arm-kernel,
	linux-kernel, will, tabba

Hi folks,

Three fixes to how the ITS driver allocates the tables it hands to the
GIC, which turned up while I was working on the pKVM ITS series [1].

The first zeroes every page in its_alloc_pages_node() before
set_memory_decrypted() can share it with the host. Every caller
already did, except for the LPI property table, which was allocated
unzeroed and filled after the share. The second orders the zeroing, in
the same function, before the store that exposes the page to the GIC.
On a coherent table it wasn't ordered at all. The third cleans the
GICv4.1 vPE table to PoC when the redistributor is non-shareable, as
the driver already does for its other tables.

Based on Linux 7.4-rc1 (6628e7d4c6b25).

Cheers,
/fuad

[1] https://lore.kernel.org/all/20261102130133.3684226-1-fuad.tabba@linux.dev/

Fuad Tabba (3):
  irqchip/gic-v3-its: Zero ITS pages before sharing them with the host
  irqchip/gic-v3-its: Order table zeroing before the GIC can see it
  irqchip/gic-v3-its: Clean a non-shareable vPE table to PoC

 drivers/irqchip/irq-gic-v3-its.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)


base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
-- 
2.47.3


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/3] irqchip/gic-v3-its: Zero ITS pages before sharing them with the host
  2026-11-06 12:02 [PATCH 0/3] irqchip/gic-v3-its: Fix how zeroed table pages reach the GIC Fuad Tabba
@ 2026-11-06 12:02 ` Fuad Tabba
  2026-11-06 14:31   ` Suzuki K Poulose
  2026-11-06 12:02 ` [PATCH 2/3] irqchip/gic-v3-its: Order table zeroing before the GIC can see it Fuad Tabba
  2026-11-06 12:02 ` [PATCH 3/3] irqchip/gic-v3-its: Clean a non-shareable vPE table to PoC Fuad Tabba
  2 siblings, 1 reply; 6+ messages in thread
From: Fuad Tabba @ 2026-11-06 12:02 UTC (permalink / raw)
  To: maz, tglx
  Cc: radu, lpieralisi, steven.price, suzuki.poulose, linux-arm-kernel,
	linux-kernel, will, tabba, stable

In a pKVM protected guest, the ITS driver shares every page it
allocates for the GIC with the host, through set_memory_decrypted() in
its_alloc_pages_node(). its_allocate_prop_table() is the only caller
that doesn't pass __GFP_ZERO. It fills the table after the call
returns, so until then the host can read the page's previous contents,
at least 64KiB of guest memory.

Zero every page in its_alloc_pages_node() before it is shared. Every
caller except the property table already passes __GFP_ZERO.

Fixes: b08e2f42e86b5 ("irqchip/gic-v3-its: Share ITS tables with a non-trusted hypervisor")
Cc: stable@vger.kernel.org
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 drivers/irqchip/irq-gic-v3-its.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 282e4bafb084f..de1f1b1f26623 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -218,7 +218,8 @@ static struct page *its_alloc_pages_node(int node, gfp_t gfp,
 	struct page *page;
 	int ret = 0;
 
-	page = alloc_pages_node(node, gfp | gfp_flags_quirk, order);
+	/* Zeroed, as set_memory_decrypted() may share the page with the host */
+	page = alloc_pages_node(node, gfp | gfp_flags_quirk | __GFP_ZERO, order);
 
 	if (!page)
 		return NULL;
-- 
2.47.3


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/3] irqchip/gic-v3-its: Order table zeroing before the GIC can see it
  2026-11-06 12:02 [PATCH 0/3] irqchip/gic-v3-its: Fix how zeroed table pages reach the GIC Fuad Tabba
  2026-11-06 12:02 ` [PATCH 1/3] irqchip/gic-v3-its: Zero ITS pages before sharing them with the host Fuad Tabba
@ 2026-11-06 12:02 ` Fuad Tabba
  2026-11-06 12:02 ` [PATCH 3/3] irqchip/gic-v3-its: Clean a non-shareable vPE table to PoC Fuad Tabba
  2 siblings, 0 replies; 6+ messages in thread
From: Fuad Tabba @ 2026-11-06 12:02 UTC (permalink / raw)
  To: maz, tglx
  Cc: radu, lpieralisi, steven.price, suzuki.poulose, linux-arm-kernel,
	linux-kernel, will, tabba

The ITS driver allocates the GIC's tables as zeroed pages. It exposes
the ITS tables and the vPE tables with a single store: a relaxed
GITS_BASER or GICR_VPROPBASER write with Valid set, the relaxed
GITS_CTLR write that enables the ITS, or a valid level-1 entry for a
level-2 page. However, on a coherent table the zeroing isn't ordered
before that store, so the GIC can read the page's previous contents as
table entries (UNPREDICTABLE: IHI 0069H.b 5.2.1, and GICR_VPROPBASER.Z
in 12.11). A level-2 page is published at runtime, while the GIC may be
walking the same level-1 table for other IDs.

Add a dma_wmb() at the end of its_alloc_pages_node(). It orders the
zeroing before later stores to memory and to MMIO alike.

Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---

Notes:
    herd7 -model aarch64.cat, the ITS modelled as an observer PE that
    reads a published entry (e), then through it the zeroed page (p):
    with no barrier the stale read is Sometimes, with dma_wmb()'s
    DMB OSHST it is Never.
    
    AArch64 MP+dmb.st+addr-its-l1
    {
    0:X0=p; 0:X1=e; 0:X2=0; 0:X3=1;
    1:X1=e; 1:X0=p;
    p=1;
    }
     P0          | P1                ;
     STR X2,[X0] | LDR X4,[X1]       ;
     DMB OSHST   |                   ;
     STR X3,[X1] | EOR X5,X4,X4      ;
                 | LDR X6,[X0,X5]    ;
    exists (1:X4=1 /\ 1:X6=1)
    
    The model puts both PEs in one Inner Shareable domain, so it does
    not separate ISH from OSH. OSH reaches the GIC whichever domain it
    is in: DDI0487 B2.10.1.1 says Outer Shareable "applies across both
    the Inner Shareable and the Outer Shareable shareability domains".
    
    The register publishes (GITS_BASER, GITS_CTLR, GICR_VPROPBASER) are
    the writel() case: the same DMB OSHST ahead of a Device write.

 drivers/irqchip/irq-gic-v3-its.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index de1f1b1f26623..badc3843d90ca 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -234,6 +234,9 @@ static struct page *its_alloc_pages_node(int node, gfp_t gfp,
 	if (ret)
 		return NULL;
 
+	/* Order the zeroing before any store that exposes the page to the GIC */
+	dma_wmb();
+
 	return page;
 }
 
-- 
2.47.3


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 3/3] irqchip/gic-v3-its: Clean a non-shareable vPE table to PoC
  2026-11-06 12:02 [PATCH 0/3] irqchip/gic-v3-its: Fix how zeroed table pages reach the GIC Fuad Tabba
  2026-11-06 12:02 ` [PATCH 1/3] irqchip/gic-v3-its: Zero ITS pages before sharing them with the host Fuad Tabba
  2026-11-06 12:02 ` [PATCH 2/3] irqchip/gic-v3-its: Order table zeroing before the GIC can see it Fuad Tabba
@ 2026-11-06 12:02 ` Fuad Tabba
  2 siblings, 0 replies; 6+ messages in thread
From: Fuad Tabba @ 2026-11-06 12:02 UTC (permalink / raw)
  To: maz, tglx
  Cc: radu, lpieralisi, steven.price, suzuki.poulose, linux-arm-kernel,
	linux-kernel, will, tabba

On a GICv4.1 redistributor that doesn't support shareability,
allocate_vpe_l1_table() points GICR_VPROPBASER at a freshly zeroed
vPE table with Z and Valid set, but leaves the zeroes in the CPU's
cache. The redistributor reads the table from memory, where it may
not be zero, and the spec makes that UNPREDICTABLE (GICR_VPROPBASER.Z,
IHI 0069H.b 12.11).

Clean the table to PoC when the redistributor is non-shareable, as the
driver does for its other tables.

Fixes: 846297e11e8ae ("irqchip/gic-v3-its: Handle non-coherent GICv4 redistributors")
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 drivers/irqchip/irq-gic-v3-its.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index badc3843d90ca..5b3273e0dfc79 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -3012,6 +3012,8 @@ static int allocate_vpe_l1_table(void)
 	if (rdists_support_shareable()) {
 		val |= GICR_VPROPBASER_RaWb;
 		val |= GICR_VPROPBASER_InnerShareable;
+	} else {
+		gic_flush_dcache_to_poc(page_address(page), np * PAGE_SIZE);
 	}
 	val |= GICR_VPROPBASER_4_1_Z;
 	val |= GICR_VPROPBASER_4_1_VALID;
-- 
2.47.3


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] irqchip/gic-v3-its: Zero ITS pages before sharing them with the host
  2026-11-06 12:02 ` [PATCH 1/3] irqchip/gic-v3-its: Zero ITS pages before sharing them with the host Fuad Tabba
@ 2026-11-06 14:31   ` Suzuki K Poulose
  2026-11-06 15:57     ` Fuad Tabba
  0 siblings, 1 reply; 6+ messages in thread
From: Suzuki K Poulose @ 2026-11-06 14:31 UTC (permalink / raw)
  To: Fuad Tabba, maz, tglx
  Cc: radu, lpieralisi, steven.price, linux-arm-kernel, linux-kernel,
	will, tabba, stable

Hi Fuad

On 06/11/2026 12:02, Fuad Tabba wrote:
> In a pKVM protected guest, the ITS driver shares every page it
> allocates for the GIC with the host, through set_memory_decrypted() in
> its_alloc_pages_node(). its_allocate_prop_table() is the only caller
> that doesn't pass __GFP_ZERO. It fills the table after the call
> returns, so until then the host can read the page's previous contents,
> at least 64KiB of guest memory.
> 
> Zero every page in its_alloc_pages_node() before it is shared. Every
> caller except the property table already passes __GFP_ZERO.
> 
> Fixes: b08e2f42e86b5 ("irqchip/gic-v3-its: Share ITS tables with a non-trusted hypervisor")
> Cc: stable@vger.kernel.org
> Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>

The patch, as such looks good to me and fixes the problem on pKVM.

We hit a similar problem with CCA, but not from a security perspective,
but a functionality perspective (crashing the Realm VM). There was a
patch proposed here [0]. The general consensus was to fix this for
all "shared allocations" and Aneesh has (had different attempts at this, 
but the latest) series is here [1]. Do you think something like this
would be preferred than fixing every caller like this ?

[0] https://lore.kernel.org/all/20260820150034.88729-2-steven.price@arm.com
[1] 
https://lore.kernel.org/all/20261104164555.3555725-1-aneesh.kumar@kernel.org

Cheers
SUzuki

> ---
>   drivers/irqchip/irq-gic-v3-its.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 282e4bafb084f..de1f1b1f26623 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -218,7 +218,8 @@ static struct page *its_alloc_pages_node(int node, gfp_t gfp,
>   	struct page *page;
>   	int ret = 0;
>   
> -	page = alloc_pages_node(node, gfp | gfp_flags_quirk, order);
> +	/* Zeroed, as set_memory_decrypted() may share the page with the host */
> +	page = alloc_pages_node(node, gfp | gfp_flags_quirk | __GFP_ZERO, order);
>   
>   	if (!page)
>   		return NULL;


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] irqchip/gic-v3-its: Zero ITS pages before sharing them with the host
  2026-11-06 14:31   ` Suzuki K Poulose
@ 2026-11-06 15:57     ` Fuad Tabba
  0 siblings, 0 replies; 6+ messages in thread
From: Fuad Tabba @ 2026-11-06 15:57 UTC (permalink / raw)
  To: Suzuki K Poulose
  Cc: maz, tglx, radu, lpieralisi, steven.price, linux-arm-kernel,
	linux-kernel, will, stable

Hi Suzuki,

On Fri, 06 Nov 2026 14:31:56 +0000, Suzuki K Poulose
<suzuki.poulose@arm.com> wrote:
[...]
> We hit a similar problem with CCA, but not from a security perspective,
> but a functionality perspective (crashing the Realm VM). There was a
> patch proposed here [0]. The general consensus was to fix this for
> all "shared allocations" and Aneesh has (had different attempts at this,
> but the latest) series is here [1]. Do you think something like this
> would be preferred than fixing every caller like this ?
>
> [0] https://lore.kernel.org/all/20260820150034.88729-2-steven.price@arm.com
> [1]
> https://lore.kernel.org/all/20261104164555.3555725-1-aneesh.kumar@kernel.org

For mainline, yes. Patch 2 of [1] zeroes the memory inside
set_memory_decrypted(), before the share on pKVM and after the
conversion on CCA, so every caller gets the order its platform
requires.

That said, [1] isn't stable material, so I'd still like this patch to
go in as the stable fix. Patch 4 of [1] would then replace it.

Cheers,
/fuad

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-11-06 15:58 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-06 12:02 [PATCH 0/3] irqchip/gic-v3-its: Fix how zeroed table pages reach the GIC Fuad Tabba
2026-11-06 12:02 ` [PATCH 1/3] irqchip/gic-v3-its: Zero ITS pages before sharing them with the host Fuad Tabba
2026-11-06 14:31   ` Suzuki K Poulose
2026-11-06 15:57     ` Fuad Tabba
2026-11-06 12:02 ` [PATCH 2/3] irqchip/gic-v3-its: Order table zeroing before the GIC can see it Fuad Tabba
2026-11-06 12:02 ` [PATCH 3/3] irqchip/gic-v3-its: Clean a non-shareable vPE table to PoC Fuad Tabba

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®