[PATCH] hw/arm/smmuv3: Emulate CFGI_STE_RANGE for an aligned range of StreamIDs

[PATCH] hw/arm/smmuv3: Emulate CFGI_STE_RANGE for an aligned range of StreamIDs

[Zenghui Yu]

In emulation of the CFGI_STE_RANGE command, we now take StreamID as the
start of the invalidation range, regardless of whatever the Range is,
whilst the spec clearly states that

 - "Invalidation is performed for an *aligned* range of 2^(Range+1)
    StreamIDs."

 - "The bottom Range+1 bits of the StreamID parameter are IGNORED,
    aligning the range to its size."

Take CFGI_ALL (where Range == 31) as an example, if there are some random
bits in the StreamID field, we'll fail to perform the full invalidation but
get a strange range (e.g., SMMUSIDRange={.start=1, .end=0}) instead. Rework
the emulation a bit to get rid of the discrepancy with the spec.

Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
---
 hw/arm/smmuv3.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 3b87324ce2..8705612535 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -980,16 +980,20 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
         }
         case SMMU_CMD_CFGI_STE_RANGE: /* same as SMMU_CMD_CFGI_ALL */
         {
-            uint32_t start = CMD_SID(&cmd);
+            uint32_t sid = CMD_SID(&cmd), mask;
             uint8_t range = CMD_STE_RANGE(&cmd);
-            uint64_t end = start + (1ULL << (range + 1)) - 1;
-            SMMUSIDRange sid_range = {start, end};
+            SMMUSIDRange sid_range;
 
             if (CMD_SSEC(&cmd)) {
                 cmd_error = SMMU_CERROR_ILL;
                 break;
             }
-            trace_smmuv3_cmdq_cfgi_ste_range(start, end);
+
+            mask = (1ULL << (range + 1)) - 1;
+            sid_range.start = sid & ~mask;
+            sid_range.end = sid_range.start + mask;
+
+            trace_smmuv3_cmdq_cfgi_ste_range(sid_range.start, sid_range.end);
             g_hash_table_foreach_remove(bs->configs, smmuv3_invalidate_ste,
                                         &sid_range);
             break;
-- 
2.19.1

Re: [PATCH] hw/arm/smmuv3: Emulate CFGI_STE_RANGE for an aligned range of StreamIDs

[Auger Eric]

Hi Zenghui,

On 4/2/21 12:04 PM, Zenghui Yu wrote:
> In emulation of the CFGI_STE_RANGE command, we now take StreamID as the
> start of the invalidation range, regardless of whatever the Range is,
> whilst the spec clearly states that
> 
>  - "Invalidation is performed for an *aligned* range of 2^(Range+1)
>     StreamIDs."
> 
>  - "The bottom Range+1 bits of the StreamID parameter are IGNORED,
>     aligning the range to its size."
> 
> Take CFGI_ALL (where Range == 31) as an example, if there are some random
> bits in the StreamID field, we'll fail to perform the full invalidation but
> get a strange range (e.g., SMMUSIDRange={.start=1, .end=0}) instead. Rework
> the emulation a bit to get rid of the discrepancy with the spec.
> 
> Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
Acked-by: Eric Auger <eric.auger@redhat.com>

Thanks!

Eric
> ---
>  hw/arm/smmuv3.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 3b87324ce2..8705612535 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -980,16 +980,20 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
>          }
>          case SMMU_CMD_CFGI_STE_RANGE: /* same as SMMU_CMD_CFGI_ALL */
>          {
> -            uint32_t start = CMD_SID(&cmd);
> +            uint32_t sid = CMD_SID(&cmd), mask;
>              uint8_t range = CMD_STE_RANGE(&cmd);
> -            uint64_t end = start + (1ULL << (range + 1)) - 1;
> -            SMMUSIDRange sid_range = {start, end};
> +            SMMUSIDRange sid_range;
>  
>              if (CMD_SSEC(&cmd)) {
>                  cmd_error = SMMU_CERROR_ILL;
>                  break;
>              }
> -            trace_smmuv3_cmdq_cfgi_ste_range(start, end);
> +
> +            mask = (1ULL << (range + 1)) - 1;
> +            sid_range.start = sid & ~mask;
> +            sid_range.end = sid_range.start + mask;
> +
> +            trace_smmuv3_cmdq_cfgi_ste_range(sid_range.start, sid_range.end);
>              g_hash_table_foreach_remove(bs->configs, smmuv3_invalidate_ste,
>                                          &sid_range);
>              break;
> 

Re: [PATCH] hw/arm/smmuv3: Emulate CFGI_STE_RANGE for an aligned range of StreamIDs

[Peter Maydell]

On Fri, 2 Apr 2021 at 11:05, Zenghui Yu <yuzenghui@huawei.com> wrote:
>
> In emulation of the CFGI_STE_RANGE command, we now take StreamID as the
> start of the invalidation range, regardless of whatever the Range is,
> whilst the spec clearly states that
>
>  - "Invalidation is performed for an *aligned* range of 2^(Range+1)
>     StreamIDs."
>
>  - "The bottom Range+1 bits of the StreamID parameter are IGNORED,
>     aligning the range to its size."
>
> Take CFGI_ALL (where Range == 31) as an example, if there are some random
> bits in the StreamID field, we'll fail to perform the full invalidation but
> get a strange range (e.g., SMMUSIDRange={.start=1, .end=0}) instead. Rework
> the emulation a bit to get rid of the discrepancy with the spec.
>
> Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
> ---



Applied to target-arm.next, thanks.

-- PMM