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 <smost...@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-phili...@linaro.org>
With Jean's comment addressed
Reviewed-by: Eric Auger <eric.au...@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
>>
