linux-kernel - Re: [PATCH v5 08/13] iommu/rockchip: Control clocks needed to access the IOMMU

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <34a60ab2-a3af-3302-6612-740cba5460db@arm.com>
Date:   Wed, 28 Feb 2018 15:06:19 +0000
From:   Robin Murphy <robin.murphy@....com>
To:     JeffyChen <jeffy.chen@...k-chips.com>,
        Rob Herring <robh@...nel.org>, Tomasz Figa <tfiga@...omium.org>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Ricky Liang <jcliang@...omium.org>,
        simon xue <xxm@...k-chips.com>, devicetree@...r.kernel.org,
        Heiko Stuebner <heiko@...ech.de>,
        "open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
        "list@....net:IOMMU DRIVERS" <iommu@...ts.linux-foundation.org>,
        Joerg Roedel <joro@...tes.org>,
        Mark Rutland <mark.rutland@....com>,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v5 08/13] iommu/rockchip: Control clocks needed to access
 the IOMMU

On 28/02/18 13:00, JeffyChen wrote:
> Hi Robin,
> 
> Thanks for your reply.
> 
> On 02/28/2018 12:59 AM, Robin Murphy wrote:
>>>> the rockchip IOMMU is part of the master block in hardware, so it needs
>>>> to control the master's power domain and some of the master's clocks
>>>> when access it's registers.
>>>>
>>>> and the number of clocks needed here, might be different between each
>>>> IOMMUs(according to which master block it belongs), it's a little like
>>>> our power domain:
>>>> https://elixir.free-electrons.com/linux/latest/source/arch/arm64/boot/dts/rockchip/rk3399.dtsi#L935 
>>>>
>>>>
>>>>
>>>>
>>>> i'm not sure how to describe this correctly, is it ok use something 
>>>> like
>>>> "the same as it's master block"?
>>>
>>> would it make sense to add a property to specify the master who owns
>>> the iommu, and we can get all clocks(only some of those clocks are
>>> actually needed) from it in the of_xlate()? and we can also reuse the
>>> clock-names of that master to build clk_bulk_data and log errors in
>>> clk_bulk_get.
>>
>> I'm inclined to agree with Rob here - if we're to add anything to the
>> binding, it should only be whatever clock inputs are defined for the
>> IOMMU IP block itself. If Linux doesn't properly handle the interconnect
>> clock hierarchy external to a particular integration, that's a separate
>> issue and it's not the binding's problem.
>>
>> I actually quite like the hack of "borrowing" the clocks from
>> dev->of_node in of_xlate() - you shouldn't need any DT changes for that,
>> because you already know that each IOMMU instance only has the one
>> master device anyway.
> 
> Thanks:) but actually we are going to support sharing IOMMU between 
> multiple masters(one of them is the main master i think) in the newer 
> chips(not yet supported on upstream kernel)...

Ha! OK, fair enough, back to the first point then...

> So we might have to get all clocks from all masters, or find a way to 
> specify the main master...and for the multiple masters case, do it in 
> of_xlate() turns out to be a little racy...maybe we can add a property 
> to specify main master, and get it's clocks in probe()?

I notice that the 4.4 BSP kernel consistently specifies "aclk" and 
"hclk" for the IOMMU instances - it feels unusual to say "why don't we 
follow the downstream binding?", but it does look a lot like what I 
would expect (I'd guess at one for the register slave interface and one 
for the master interface/general operation?)

If we can implement conceptually-correct clock handling based on an 
accurate binding, which should cover most cases, and *then* look at 
hacking around those where it doesn't quite work in practice due to 
shortcomings elsewhere, that would be ideal, and of course a lot nicer 
than just jumping straight into piles of hacks.

Robin.