[PATCH] hw/arm/smmuv3: Fix incorrect reserved mask for SMMU CR0 register

[PATCH] hw/arm/smmuv3: Fix incorrect reserved mask for SMMU CR0 register

[taotang2025]

From: Tao Tang <taotang2025@gmail.com>

The current definition of the SMMU_CR0_RESERVED mask is incorrect.
It mistakenly treats bit 10 (DPT_WALK_EN) as a reserved bit while
treating bit 9 (RES0) as an implemented bit.

According to the SMMU architecture specification, the layout for CR0 is:
| 31:11| RES0           |
| 10   | DPT_WALK_EN    |
| 9    | RES0           |
| 8:6  | VMW            |
| 5    | RES0           |
| 4    | ATSCHK         |
| 3    | CMDQEN         |
| 2    | EVENTQEN       |
| 1    | PRIQEN         |
| 0    | SMMUEN         |

Signed-off-by: Tao Tang <taotang2025@gmail.com>
---
 hw/arm/smmuv3-internal.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
index b6b7399347..42ac77e654 100644
--- a/hw/arm/smmuv3-internal.h
+++ b/hw/arm/smmuv3-internal.h
@@ -120,7 +120,7 @@ REG32(CR0,                 0x20)
     FIELD(CR0, EVENTQEN,      2, 1)
     FIELD(CR0, CMDQEN,        3, 1)
 
-#define SMMU_CR0_RESERVED 0xFFFFFC20
+#define SMMU_CR0_RESERVED 0xFFFFFA20
 
 REG32(CR0ACK,              0x24)
 REG32(CR1,                 0x28)
-- 
2.34.1

Re: [PATCH] hw/arm/smmuv3: Fix incorrect reserved mask for SMMU CR0 register

[Eric Auger]

Hi,

On 6/5/25 4:26 AM, taotang2025@gmail.com wrote:
> From: Tao Tang <taotang2025@gmail.com>
>
> The current definition of the SMMU_CR0_RESERVED mask is incorrect.
> It mistakenly treats bit 10 (DPT_WALK_EN) as a reserved bit while
> treating bit 9 (RES0) as an implemented bit.
>
> According to the SMMU architecture specification, the layout for CR0 is:
> | 31:11| RES0           |
> | 10   | DPT_WALK_EN    |
> | 9    | RES0           |
> | 8:6  | VMW            |
> | 5    | RES0           |
> | 4    | ATSCHK         |
> | 3    | CMDQEN         |
> | 2    | EVENTQEN       |
> | 1    | PRIQEN         |
> | 0    | SMMUEN         |
>
> Signed-off-by: Tao Tang <taotang2025@gmail.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks!

Eric

> ---
>  hw/arm/smmuv3-internal.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
> index b6b7399347..42ac77e654 100644
> --- a/hw/arm/smmuv3-internal.h
> +++ b/hw/arm/smmuv3-internal.h
> @@ -120,7 +120,7 @@ REG32(CR0,                 0x20)
>      FIELD(CR0, EVENTQEN,      2, 1)
>      FIELD(CR0, CMDQEN,        3, 1)
>  
> -#define SMMU_CR0_RESERVED 0xFFFFFC20
> +#define SMMU_CR0_RESERVED 0xFFFFFA20
>  
>  REG32(CR0ACK,              0x24)
>  REG32(CR1,                 0x28)

Re: [PATCH] hw/arm/smmuv3: Fix incorrect reserved mask for SMMU CR0 register

[Peter Maydell]

On Thu, 5 Jun 2025 at 03:26, <taotang2025@gmail.com> wrote:
>
> From: Tao Tang <taotang2025@gmail.com>
>
> The current definition of the SMMU_CR0_RESERVED mask is incorrect.
> It mistakenly treats bit 10 (DPT_WALK_EN) as a reserved bit

This is because our implementation pre-dates the revision
of the SMMUv3 spec which adds the DPT and this DPT_WALK_EN bit.
So for us it is/was legitimately reserved. We use this constant
value to update SMMU_CR0ACK when the guest writes to SMMU_CR0,
so ideally we shouldn't mark bits as non-reserved unless we
actually implement the feature that they correspond to.

We don't get this right for VMW and ATSCHK, which are both
"RES0 unless the ID register indicates presence of some
feature" fields, but which we allow the guest to write to
(and cause updates to CR0ACK) anyway.

> while
> treating bit 9 (RES0) as an implemented bit.

This part is a bug; nice catch.

thanks
-- PMM