[PATCH] hw/riscv/riscv-iommu: fix U-bit check to apply only to leaf S/VS-stage PTEs

Andrew Jones posted 1 patch 1 week, 1 day ago
Failed in applying to current master (apply log)
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Chao Liu <chao.liu@processmission.com>
hw/riscv/riscv-iommu.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
[PATCH] hw/riscv/riscv-iommu: fix U-bit check to apply only to leaf S/VS-stage PTEs
Posted by Andrew Jones 1 week, 1 day ago
Commit b795ea0ba471 ("hw/riscv/riscv-iommu.c: fault when !PTE_U and
no priv access") placed its check ahead of the leaf-vs-non-leaf
branch in riscv_iommu_spa_fetch(), so it fires on every PTE walked,
including non-leaf/table entries. Per the RISC-V privileged spec's
address translation algorithm (Sv39/Sv48/etc., the "leaf PTE has
been reached" step, followed separately by the U-bit permission
check), the U bit is only defined and checked for the leaf PTE
reached at the end of the walk -- non-leaf PTEs don't carry a
meaningful U bit at all.

Move the check after the leaf/non-leaf branch, alongside the other
leaf-only checks, mirroring how the G_STAGE U-bit check (added in
9158c900ab30) is already correctly placed.

Fixes: b795ea0ba471 ("hw/riscv/riscv-iommu.c: fault when !PTE_U and no priv access")
Signed-off-by: Andrew Jones <andrew.jones@oss.qualcomm.com>
---
 hw/riscv/riscv-iommu.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
index f6865d1e164e..ed9fb09f8bc7 100644
--- a/hw/riscv/riscv-iommu.c
+++ b/hw/riscv/riscv-iommu.c
@@ -476,13 +476,6 @@ static int riscv_iommu_spa_fetch(RISCVIOMMUState *s, RISCVIOMMUContext *ctx,
             break;                /* Invalid PTE */
         } else if (pte & PTE_RESERVED(false)) {
             break;                /* Reserved PTE bits set */
-        } else if (!(pte & PTE_U) && !pv) {
-            /*
-             * All accesses are assumed to be User mode unless
-             * process_id is valid (pv).  In case we have a
-             * non-user mode PTE and !pv we need to fault.
-             */
-            break;
         } else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
             base = PPN_PHYS(ppn); /* Inner PTE, continue walking */
         } else if ((pte & (PTE_R | PTE_W | PTE_X)) == PTE_W) {
@@ -491,6 +484,13 @@ static int riscv_iommu_spa_fetch(RISCVIOMMUState *s, RISCVIOMMUContext *ctx,
             break;                /* Reserved leaf PTE flags: PTE_W + PTE_X */
         } else if (ppn & ((1ULL << (va_skip - TARGET_PAGE_BITS)) - 1)) {
             break;                /* Misaligned PPN */
+        } else if (!(pte & PTE_U) && !pv) {
+            /*
+             * All accesses are assumed to be User mode unless
+             * process_id is valid (pv).  In case we have a
+             * non-user mode leaf PTE and !pv we need to fault.
+             */
+            break;
         } else if ((iotlb->perm & IOMMU_RO) && !(pte & PTE_R)) {
             break;                /* Read access check failed */
         } else if ((iotlb->perm & IOMMU_WO) && !(pte & PTE_W)) {
-- 
2.43.0
Re: [PATCH] hw/riscv/riscv-iommu: fix U-bit check to apply only to leaf S/VS-stage PTEs
Posted by Nutty.Liu 4 days, 4 hours ago
On 7/17/2026 7:23 PM, Andrew Jones wrote:
> Commit b795ea0ba471 ("hw/riscv/riscv-iommu.c: fault when !PTE_U and
> no priv access") placed its check ahead of the leaf-vs-non-leaf
> branch in riscv_iommu_spa_fetch(), so it fires on every PTE walked,
> including non-leaf/table entries. Per the RISC-V privileged spec's
> address translation algorithm (Sv39/Sv48/etc., the "leaf PTE has
> been reached" step, followed separately by the U-bit permission
> check), the U bit is only defined and checked for the leaf PTE
> reached at the end of the walk -- non-leaf PTEs don't carry a
> meaningful U bit at all.
>
> Move the check after the leaf/non-leaf branch, alongside the other
> leaf-only checks, mirroring how the G_STAGE U-bit check (added in
> 9158c900ab30) is already correctly placed.
>
> Fixes: b795ea0ba471 ("hw/riscv/riscv-iommu.c: fault when !PTE_U and no priv access")
> Signed-off-by: Andrew Jones <andrew.jones@oss.qualcomm.com>
Reviewed-by: Nutty Liu <nutty.liu@hotmail.com>

Thanks,
Nutty
> ---
>   hw/riscv/riscv-iommu.c | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
> index f6865d1e164e..ed9fb09f8bc7 100644
> --- a/hw/riscv/riscv-iommu.c
> +++ b/hw/riscv/riscv-iommu.c
> @@ -476,13 +476,6 @@ static int riscv_iommu_spa_fetch(RISCVIOMMUState *s, RISCVIOMMUContext *ctx,
>               break;                /* Invalid PTE */
>           } else if (pte & PTE_RESERVED(false)) {
>               break;                /* Reserved PTE bits set */
> -        } else if (!(pte & PTE_U) && !pv) {
> -            /*
> -             * All accesses are assumed to be User mode unless
> -             * process_id is valid (pv).  In case we have a
> -             * non-user mode PTE and !pv we need to fault.
> -             */
> -            break;
>           } else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
>               base = PPN_PHYS(ppn); /* Inner PTE, continue walking */
>           } else if ((pte & (PTE_R | PTE_W | PTE_X)) == PTE_W) {
> @@ -491,6 +484,13 @@ static int riscv_iommu_spa_fetch(RISCVIOMMUState *s, RISCVIOMMUContext *ctx,
>               break;                /* Reserved leaf PTE flags: PTE_W + PTE_X */
>           } else if (ppn & ((1ULL << (va_skip - TARGET_PAGE_BITS)) - 1)) {
>               break;                /* Misaligned PPN */
> +        } else if (!(pte & PTE_U) && !pv) {
> +            /*
> +             * All accesses are assumed to be User mode unless
> +             * process_id is valid (pv).  In case we have a
> +             * non-user mode leaf PTE and !pv we need to fault.
> +             */
> +            break;
>           } else if ((iotlb->perm & IOMMU_RO) && !(pte & PTE_R)) {
>               break;                /* Read access check failed */
>           } else if ((iotlb->perm & IOMMU_WO) && !(pte & PTE_W)) {
Re: [PATCH] hw/riscv/riscv-iommu: fix U-bit check to apply only to leaf S/VS-stage PTEs
Posted by Daniel Henrique Barboza 1 week, 1 day ago

On 7/17/2026 8:23 AM, Andrew Jones wrote:
> Commit b795ea0ba471 ("hw/riscv/riscv-iommu.c: fault when !PTE_U and
> no priv access") placed its check ahead of the leaf-vs-non-leaf
> branch in riscv_iommu_spa_fetch(), so it fires on every PTE walked,
> including non-leaf/table entries. Per the RISC-V privileged spec's
> address translation algorithm (Sv39/Sv48/etc., the "leaf PTE has
> been reached" step, followed separately by the U-bit permission
> check), the U bit is only defined and checked for the leaf PTE
> reached at the end of the walk -- non-leaf PTEs don't carry a
> meaningful U bit at all.
> 
> Move the check after the leaf/non-leaf branch, alongside the other
> leaf-only checks, mirroring how the G_STAGE U-bit check (added in
> 9158c900ab30) is already correctly placed.
> 
> Fixes: b795ea0ba471 ("hw/riscv/riscv-iommu.c: fault when !PTE_U and no priv access")
> Signed-off-by: Andrew Jones <andrew.jones@oss.qualcomm.com>
> ---

Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

>   hw/riscv/riscv-iommu.c | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
> index f6865d1e164e..ed9fb09f8bc7 100644
> --- a/hw/riscv/riscv-iommu.c
> +++ b/hw/riscv/riscv-iommu.c
> @@ -476,13 +476,6 @@ static int riscv_iommu_spa_fetch(RISCVIOMMUState *s, RISCVIOMMUContext *ctx,
>               break;                /* Invalid PTE */
>           } else if (pte & PTE_RESERVED(false)) {
>               break;                /* Reserved PTE bits set */
> -        } else if (!(pte & PTE_U) && !pv) {
> -            /*
> -             * All accesses are assumed to be User mode unless
> -             * process_id is valid (pv).  In case we have a
> -             * non-user mode PTE and !pv we need to fault.
> -             */
> -            break;
>           } else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
>               base = PPN_PHYS(ppn); /* Inner PTE, continue walking */
>           } else if ((pte & (PTE_R | PTE_W | PTE_X)) == PTE_W) {
> @@ -491,6 +484,13 @@ static int riscv_iommu_spa_fetch(RISCVIOMMUState *s, RISCVIOMMUContext *ctx,
>               break;                /* Reserved leaf PTE flags: PTE_W + PTE_X */
>           } else if (ppn & ((1ULL << (va_skip - TARGET_PAGE_BITS)) - 1)) {
>               break;                /* Misaligned PPN */
> +        } else if (!(pte & PTE_U) && !pv) {
> +            /*
> +             * All accesses are assumed to be User mode unless
> +             * process_id is valid (pv).  In case we have a
> +             * non-user mode leaf PTE and !pv we need to fault.
> +             */
> +            break;
>           } else if ((iotlb->perm & IOMMU_RO) && !(pte & PTE_R)) {
>               break;                /* Read access check failed */
>           } else if ((iotlb->perm & IOMMU_WO) && !(pte & PTE_W)) {