Re: [PATCH v5 2/4] hw/arm/smmuv3: Update implementation of CFGI commands based on device SID

[Eric Auger]

Hi Chunming

On 8/25/21 10:08 AM, chunming wrote:
> From: chunming <chunming.li@verisilicon.com>
>
> Replace "smmuv3_flush_config" with "g_hash_table_foreach_remove".
this replacement may have a potential negative impact on the performance
for PCIe support, which is the main use case: a unique
g_hash_table_remove() is replaced by an iteration over all the config
hash keys.

I wonder if you couldn't just adapt smmu_iommu_mr() and it case this
latter returns NULL for the current PCIe search, look up in the platform
device list:

peri_sdev_list?

Thanks

Eric



> "smmu_iommu_mr" function can't get MR according to SID for non PCI/PCIe 
> devices.
>
> Signed-off-by: chunming <chunming.li@verisilicon.com>
> ---
>  hw/arm/smmuv3.c              | 35 ++++++++++-------------------------
>  include/hw/arm/smmu-common.h |  5 ++++-
>  2 files changed, 14 insertions(+), 26 deletions(-)
>
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 11d7fe8423..9f3f13fb8e 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -613,14 +613,6 @@ static SMMUTransCfg *smmuv3_get_config(SMMUDevice *sdev, 
> SMMUEventInfo *event)
>      return cfg;
>  }
>  
> -static void smmuv3_flush_config(SMMUDevice *sdev)
> -{
> -    SMMUv3State *s = sdev->smmu;
> -    SMMUState *bc = &s->smmu_state;
> -
> -    trace_smmuv3_config_cache_inv(smmu_get_sid(sdev));
> -    g_hash_table_remove(bc->configs, sdev);
> -}
>  
>  static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
>                                        IOMMUAccessFlags flag, int iommu_idx)
> @@ -964,22 +956,18 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
>          case SMMU_CMD_CFGI_STE:
>          {
>              uint32_t sid = CMD_SID(&cmd);
> -            IOMMUMemoryRegion *mr = smmu_iommu_mr(bs, sid);
> -            SMMUDevice *sdev;
> +            SMMUSIDRange sid_range;
>  
>              if (CMD_SSEC(&cmd)) {
>                  cmd_error = SMMU_CERROR_ILL;
>                  break;
>              }
>  
> -            if (!mr) {
> -                break;
> -            }
> -
> +            sid_range.start = sid;
> +            sid_range.end = sid;
>              trace_smmuv3_cmdq_cfgi_ste(sid);
> -            sdev = container_of(mr, SMMUDevice, iommu);
> -            smmuv3_flush_config(sdev);
> -
> +            g_hash_table_foreach_remove(bs->configs, smmuv3_invalidate_ste,
> +                                        &sid_range);
>              break;
>          }
>          case SMMU_CMD_CFGI_STE_RANGE: /* same as SMMU_CMD_CFGI_ALL */
> @@ -1006,21 +994,18 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
>          case SMMU_CMD_CFGI_CD_ALL:
>          {
>              uint32_t sid = CMD_SID(&cmd);
> -            IOMMUMemoryRegion *mr = smmu_iommu_mr(bs, sid);
> -            SMMUDevice *sdev;
> +            SMMUSIDRange sid_range;
>  
>              if (CMD_SSEC(&cmd)) {
>                  cmd_error = SMMU_CERROR_ILL;
>                  break;
>              }
>  
> -            if (!mr) {
> -                break;
> -            }
> -
> +            sid_range.start = sid;
> +            sid_range.end = sid;
>              trace_smmuv3_cmdq_cfgi_cd(sid);
> -            sdev = container_of(mr, SMMUDevice, iommu);
> -            smmuv3_flush_config(sdev);
> +            g_hash_table_foreach_remove(bs->configs, smmuv3_invalidate_ste,
> +                                        &sid_range);
>              break;
>          }
>          case SMMU_CMD_TLBI_NH_ASID:
> diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
> index 95cd12a4b5..d016455d80 100644
> --- a/include/hw/arm/smmu-common.h
> +++ b/include/hw/arm/smmu-common.h
> @@ -159,7 +159,10 @@ int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, 
> IOMMUAccessFlags perm,
>   */
>  SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg, dma_addr_t iova);
>  
> -/* Return the iommu mr associated to @sid, or NULL if none */
> +/**
> + * Return the iommu mr associated to @sid, or NULL if none
> + * Only for PCI device, check smmu_find_peri_sdev for non PCI/PCIe device
> + */
>  IOMMUMemoryRegion *smmu_iommu_mr(SMMUState *s, uint32_t sid);
>  
>  #define SMMU_IOTLB_MAX_SIZE 256