[RFC PATCH 07/30] iommu/arm-smmu-v3: Add second level of context descriptor table

[Jean-Philippe Brucker]

On 15/05/17 13:47, Tomasz Nowicki wrote:
> Hi Jean,
> 
> On 27.02.2017 20:54, Jean-Philippe Brucker wrote:
>> @@ -1213,17 +1356,59 @@ static void arm_smmu_free_cd_tables(struct
>> arm_smmu_master_data *master)
>>  __maybe_unused
>>  static int arm_smmu_alloc_cd(struct arm_smmu_master_data *master)
>>  {
>> +    int ssid;
>> +    int i, ret;
>>      struct arm_smmu_cd_cfg *cfg = &master->ste.cd_cfg;
>>
>> -    return arm_smmu_bitmap_alloc(cfg->context_map,
>> ilog2(cfg->num_entries));
>> +    if (cfg->linear)
>> +        return arm_smmu_bitmap_alloc(cfg->table.context_map,
>> +                         ilog2(cfg->num_entries));
>> +
>> +    /* Find first leaf table with an empty slot, or allocate a new leaf */
>> +    for (i = cfg->l1.cur_table; i < cfg->num_entries; i++) {
>> +        struct arm_smmu_cd_table *table = &cfg->l1.tables[i];
>> +
>> +        if (!table->cdptr) {
>> +            __le64 *l1ptr = cfg->l1.ptr + i * CTXDESC_L1_DESC_DWORD;
>> +
>> +            ret = arm_smmu_alloc_cd_leaf_table(master->smmu, table,
>> +                               CTXDESC_NUM_L2_ENTRIES);
>> +            if (ret)
>> +                return ret;
>> +
>> +            arm_smmu_write_cd_l1_desc(l1ptr, table);
>> +            arm_smmu_sync_cd(master, i << CTXDESC_SPLIT, false);
>> +        }
>> +
>> +        ssid = arm_smmu_bitmap_alloc(table->context_map, CTXDESC_SPLIT);
>> +        if (ssid < 0)
>> +            continue;
>> +
>> +        cfg->l1.cur_table = i;
>> +        return i << CTXDESC_SPLIT | ssid;
>> +    }
>> +
>> +    return -ENOSPC;
>>  }
>>
>>  __maybe_unused
>>  static void arm_smmu_free_cd(struct arm_smmu_master_data *master, u32
>> ssid)
>>  {
>> +    unsigned long l1_idx, idx;
>>      struct arm_smmu_cd_cfg *cfg = &master->ste.cd_cfg;
>>
>> -    arm_smmu_bitmap_free(cfg->context_map, ssid);
>> +    if (cfg->linear) {
>> +        arm_smmu_bitmap_free(cfg->table.context_map, ssid);
>> +        return;
>> +    }
>> +
>> +    l1_idx = ssid >> CTXDESC_SPLIT;
>> +    idx = ssid & ((1 << CTXDESC_SPLIT) - 1);
>> +    arm_smmu_bitmap_free(cfg->l1.tables[l1_idx].context_map, idx);
>> +
>> +    /* Prepare next allocation */
>> +    if (cfg->l1.cur_table > idx)
>> +        cfg->l1.cur_table = idx;
>>  }
> 
> I am not sure what is the logic here. idx becomes index of l1.tables in
> arm_smmu_alloc_cd() which in turn may be out of allocated memory. I may
> miss something. Can you please elaborate on this?

The code is wrong, this should be:

+    if (cfg->l1.cur_table > l1_idx)
+        cfg->l1.cur_table = l1_idx;

Which tells alloc_cd() that there is a free slot in table l1_idx. It would
also look better with a min(). However this allocator will be replaced by
a global (IDR) allocator in next version, as requested by other users of
the IOMMU API.

Thanks,
Jean