Re: [PATCH v4 13/19] hw/arm/smmu: Introduce smmu_iotlb_inv_asid_vmid

[Eric Auger]

On 7/4/24 20:33, Jean-Philippe Brucker wrote:
> On Mon, Jul 01, 2024 at 11:02:35AM +0000, Mostafa Saleh wrote:
>> Soon, Instead of doing TLB invalidation by ASID only, VMID will be
>> also required.
>> Add smmu_iotlb_inv_asid_vmid() which invalidates by both ASID and VMID.
>>
>> However, at the moment this function is only used in SMMU_CMD_TLBI_NH_ASID
>> which is a stage-1 command, so passing VMID = -1 keeps the original
>> behaviour.
>>
>> Signed-off-by: Mostafa Saleh <smostafa@google.com>
> One small issue: the change to hw/arm/trace-events for
> trace_smmu_iotlb_inv_asid_vmid() should be in patch 13 rather than 14,
> otherwise the series isn't bisectable.
>
> Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
With Jean's comment addressed
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric
>
>> ---
>>  hw/arm/smmu-common.c         | 20 +++++++++++++-------
>>  hw/arm/smmuv3.c              |  2 +-
>>  include/hw/arm/smmu-common.h |  2 +-
>>  3 files changed, 15 insertions(+), 9 deletions(-)
>>
>> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
>> index 5bf9eadeff..d0309a95b2 100644
>> --- a/hw/arm/smmu-common.c
>> +++ b/hw/arm/smmu-common.c
>> @@ -159,13 +159,14 @@ void smmu_iotlb_inv_all(SMMUState *s)
>>      g_hash_table_remove_all(s->iotlb);
>>  }
>>  
>> -static gboolean smmu_hash_remove_by_asid(gpointer key, gpointer value,
>> -                                         gpointer user_data)
>> +static gboolean smmu_hash_remove_by_asid_vmid(gpointer key, gpointer value,
>> +                                              gpointer user_data)
>>  {
>> -    int asid = *(int *)user_data;
>> +    SMMUIOTLBPageInvInfo *info = (SMMUIOTLBPageInvInfo *)user_data;
>>      SMMUIOTLBKey *iotlb_key = (SMMUIOTLBKey *)key;
>>  
>> -    return SMMU_IOTLB_ASID(*iotlb_key) == asid;
>> +    return (SMMU_IOTLB_ASID(*iotlb_key) == info->asid) &&
>> +           (SMMU_IOTLB_VMID(*iotlb_key) == info->vmid);
>>  }
>>  
>>  static gboolean smmu_hash_remove_by_vmid(gpointer key, gpointer value,
>> @@ -270,10 +271,15 @@ void smmu_iotlb_inv_ipa(SMMUState *s, int vmid, 
>> dma_addr_t ipa, uint8_t tg,
>>                                  &info);
>>  }
>>  
>> -void smmu_iotlb_inv_asid(SMMUState *s, int asid)
>> +void smmu_iotlb_inv_asid_vmid(SMMUState *s, int asid, int vmid)
>>  {
>> -    trace_smmu_iotlb_inv_asid(asid);
>> -    g_hash_table_foreach_remove(s->iotlb, smmu_hash_remove_by_asid, &asid);
>> +    SMMUIOTLBPageInvInfo info = {
>> +        .asid = asid,
>> +        .vmid = vmid,
>> +    };
>> +
>> +    trace_smmu_iotlb_inv_asid_vmid(asid, vmid);
>> +    g_hash_table_foreach_remove(s->iotlb, smmu_hash_remove_by_asid_vmid, 
>> &info);
>>  }
>>  
>>  void smmu_iotlb_inv_vmid(SMMUState *s, int vmid)
>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
>> index e5ecd93258..928f125523 100644
>> --- a/hw/arm/smmuv3.c
>> +++ b/hw/arm/smmuv3.c
>> @@ -1329,7 +1329,7 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
>>  
>>              trace_smmuv3_cmdq_tlbi_nh_asid(asid);
>>              smmu_inv_notifiers_all(&s->smmu_state);
>> -            smmu_iotlb_inv_asid(bs, asid);
>> +            smmu_iotlb_inv_asid_vmid(bs, asid, -1);
>>              break;
>>          }
>>          case SMMU_CMD_TLBI_NH_ALL:
>> diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
>> index de032fdfd1..2bc9a03d47 100644
>> --- a/include/hw/arm/smmu-common.h
>> +++ b/include/hw/arm/smmu-common.h
>> @@ -212,7 +212,7 @@ void smmu_iotlb_insert(SMMUState *bs, SMMUTransCfg *cfg, 
>> SMMUTLBEntry *entry);
>>  SMMUIOTLBKey smmu_get_iotlb_key(int asid, int vmid, uint64_t iova,
>>                                  uint8_t tg, uint8_t level);
>>  void smmu_iotlb_inv_all(SMMUState *s);
>> -void smmu_iotlb_inv_asid(SMMUState *s, int asid);
>> +void smmu_iotlb_inv_asid_vmid(SMMUState *s, int asid, int vmid);
>>  void smmu_iotlb_inv_vmid(SMMUState *s, int vmid);
>>  void smmu_iotlb_inv_iova(SMMUState *s, int asid, int vmid, dma_addr_t iova,
>>                           uint8_t tg, uint64_t num_pages, uint8_t ttl);
>> -- 
>> 2.45.2.803.g4e1b14247a-goog
>>