Re: [PATCH 1/6] iommu/qcom: Use the asid read from device-tree if specified

Konrad Dybcio
2022-06-03T18:03:42Z

On 31.05.2022 22:57, Rob Clark wrote:
> On Tue, May 31, 2022 at 9:19 AM Will Deacon <will@kernel.org> wrote:
>>
>> On Tue, May 31, 2022 at 09:15:22AM -0700, Rob Clark wrote:
>>> On Tue, May 31, 2022 at 8:46 AM Will Deacon <will@kernel.org> wrote:
>>>>
>>>> On Fri, May 27, 2022 at 11:28:56PM +0200, Konrad Dybcio wrote:
>>>>> From: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
>>>>>
>>>>> As specified in this driver, the context banks are 0x1000 apart.
>>>>> Problem is that sometimes the context number (our asid) does not
>>>>> match this logic and we end up using the wrong one: this starts
>>>>> being a problem in the case that we need to send TZ commands
>>>>> to do anything on a specific context.
>>>>
>>>> I don't understand this. The ASID is a software construct, so it shouldn't
>>>> matter what we use. If it does matter, then please can you explain why? The
>>>> fact that the context banks are 0x1000 apart seems unrelated.
>>>
>>> I think the connection is that mapping from ctx bank to ASID is 1:1
>>
>> But in what sense? How is the ASID used beyond a tag in the TLB? The commit
>> message hints at "TZ commands" being a problem.
>>
>> I'm not doubting that this is needed to make the thing work, I just don't
>> understand why.
>
> (disclaimer, it has been quite a while since I've looked at the smmu
> setup with earlier tz, ie. things that use qcom_iommu, but from
> memory...)
>
> We cannot actually assign the context banks ourselves, so in the dt
> bindings the "ASID" is actually the context bank index.
I think so.

I don't
> remember exactly if this was a limitation of the tz interface, or
> result of not being able to program the smmu's global registers
> ourselves.

As far as I understand, it's the latter, as changing the defaults is not allowed by the security policy on consumer devices.

Qualcomm arbitrarily chose some numbers that may or may have not aligned with their usual index-is-offset-divided-by-0x1000 and hardcoded them in the BSP, and now the secure side (if required, and well, it is..) expects precisely that configuration.

Konrad