From: Roque Arcudia Hernandez <roqueh@google.com>

Current implementation checks the StreamIDs against STRTAB_BASE_CFG.LOG2SIZE
register field value and a constant SMMU_IDR1_SIDSIZE which is also used as
initial value for field SMMU_IDR1.SIDSIZE.

This limits the possibility of extending the SMMUv3 by inheritance and
redefining the value of SMMU_IDR1.SIDSIZE because the check is hardcoded to the
constant SMMU_IDR1_SIDSIZE rather than the register value.

Signed-off-by: Roque Arcudia Hernandez <roqueh@google.com>
Signed-off-by: Nabih Estefan <nabihestefan@google.com>
---
 hw/arm/smmuv3.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 9eb56a70f3..a01031821a 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -580,15 +580,17 @@ static int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste,
 {
     dma_addr_t addr, strtab_base;
     uint32_t log2size;
+    uint32_t idr1_sidsize;
     int strtab_size_shift;
     int ret;
 
     trace_smmuv3_find_ste(sid, s->features, s->sid_split);
     log2size = FIELD_EX32(s->strtab_base_cfg, STRTAB_BASE_CFG, LOG2SIZE);
+    idr1_sidsize = FIELD_EX32(s->idr[1], IDR1, SIDSIZE);
     /*
      * Check SID range against both guest-configured and implementation limits
      */
-    if (sid >= (1 << MIN(log2size, SMMU_IDR1_SIDSIZE))) {
+    if (sid >= (1 << MIN(log2size, idr1_sidsize))) {
         event->type = SMMU_EVT_C_BAD_STREAMID;
         return -EINVAL;
     }
-- 
2.44.0.rc0.258.g7320e95886-goog

Hi,

On 2/21/24 18:17, Nabih Estefan wrote:
> From: Roque Arcudia Hernandez <roqueh@google.com>
>
> Current implementation checks the StreamIDs against STRTAB_BASE_CFG.LOG2SIZE
> register field value and a constant SMMU_IDR1_SIDSIZE which is also used as
> initial value for field SMMU_IDR1.SIDSIZE.
>
> This limits the possibility of extending the SMMUv3 by inheritance and
> redefining the value of SMMU_IDR1.SIDSIZE because the check is hardcoded to the
> constant SMMU_IDR1_SIDSIZE rather than the register value.
>
> Signed-off-by: Roque Arcudia Hernandez <roqueh@google.com>
> Signed-off-by: Nabih Estefan <nabihestefan@google.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric
> ---
>  hw/arm/smmuv3.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 9eb56a70f3..a01031821a 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -580,15 +580,17 @@ static int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste,
>  {
>      dma_addr_t addr, strtab_base;
>      uint32_t log2size;
> +    uint32_t idr1_sidsize;
>      int strtab_size_shift;
>      int ret;
>  
>      trace_smmuv3_find_ste(sid, s->features, s->sid_split);
>      log2size = FIELD_EX32(s->strtab_base_cfg, STRTAB_BASE_CFG, LOG2SIZE);
> +    idr1_sidsize = FIELD_EX32(s->idr[1], IDR1, SIDSIZE);
>      /*
>       * Check SID range against both guest-configured and implementation limits
>       */
> -    if (sid >= (1 << MIN(log2size, SMMU_IDR1_SIDSIZE))) {
> +    if (sid >= (1 << MIN(log2size, idr1_sidsize))) {
>          event->type = SMMU_EVT_C_BAD_STREAMID;
>          return -EINVAL;
>      }