On Thu, 11 Jun 2020 at 17:16, Eric Auger <eric.au...@redhat.com> wrote:
>
> At the moment each entry in the IOTLB corresponds to a page sized
> mapping (4K, 16K or 64K), even if the page belongs to a mapped
> block. In case of block mapping this unefficiently consume IOTLB
> entries.
>
> Change the value of the entry so that it reflects the actual
> mapping it belongs to (block or page start address and size).
>
> Also the level/tg of the entry is encoded in the key. In subsequent
> patches we will enable range invalidation. This latter is able
> to provide the level/tg of the entry.
>
> Signed-off-by: Eric Auger <eric.au...@redhat.com>
> -uint64_t smmu_get_iotlb_key(uint16_t asid, uint64_t iova)
> +uint64_t smmu_get_iotlb_key(uint16_t asid, uint64_t iova,
> + uint8_t tg, uint8_t level)
> {
> - return iova >> 12 | (uint64_t)(asid) << SMMU_IOTLB_ASID_SHIFT;
> + return iova >> 12 | (uint64_t)(asid) << SMMU_IOTLB_ASID_SHIFT |
> + (uint64_t)(level) << SMMU_IOTLB_LEVEL_SHIFT |
> + (uint64_t)(tg) << SMMU_IOTLB_TG_SHIFT;
> }
> SMMUTLBEntry *smmu_iotlb_lookup(SMMUState *bs, SMMUTransCfg *cfg,
> - hwaddr iova)
> + SMMUTransTableInfo *tt, hwaddr iova)
> {
> - uint64_t key = smmu_get_iotlb_key(cfg->asid, iova);
> - SMMUTLBEntry *entry = g_hash_table_lookup(bs->iotlb, &key);
> + uint8_t tg = (tt->granule_sz - 10) / 2;
> + int level = tt->starting_level;
> + SMMUTLBEntry *entry = NULL;
> +
> + while (level <= 3) {
> + uint64_t subpage_size = 1ULL << level_shift(level, tt->granule_sz);
> + uint64_t mask = subpage_size - 1;
> + uint64_t key;
> +
> + key = smmu_get_iotlb_key(cfg->asid, iova & ~mask, tg, level);
> + entry = g_hash_table_lookup(bs->iotlb, &key);
> + if (entry) {
> + break;
> + }
> + level++;
Rather than looping around doing multiple hash table lookups like
this, why not just avoid including the tg and level in the
key equality test?
If I understand the range-based-invalidation feature correctly,
the only time we care about the TG/LVL is if we're processing
an invalidate-range command that specifies them. But in that
case there should never be multiple entries in the bs->iotlb
with the same iova, so we can just check whether the entry
matches the requested TG/LVL once we've pulled it out of the
hash table. (Or we could architecturally validly just blow
it away regardless of requested TG/LVL -- they are only hints,
not required-to-match.)
thanks
-- PMM
