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  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]
Date:	Wed, 19 Mar 2014 16:14:57 +0100
From:	Tomasz Figa <>
To:	Cho KyongHo <>
Cc:	Linux ARM Kernel <>,
	Linux DeviceTree <>,
	Linux IOMMU <>,
	Linux Kernel <>,
	Linux Samsung SOC <>,
	Antonios Motakis <>,
	Grant Grundler <>,
	Joerg Roedel <>,
	Kukjin Kim <>,
	Prathyush <>,
	Rahul Sharma <>,
	Sachin Kamat <>,
	Sylwester Nawrocki <>,
	Varun Sethi <>
Subject: Re: [PATCH v11 20/27] iommu/exynos: allow having multiple System MMUs
 for a master H/W

On 19.03.2014 14:20, Tomasz Figa wrote:
> On 19.03.2014 01:39, Cho KyongHo wrote:
>> On Tue, 18 Mar 2014 15:26:48 +0100, Tomasz Figa wrote:
>>> On 18.03.2014 14:01, Cho KyongHo wrote:
>>>> On Fri, 14 Mar 2014 17:12:03 +0100, Tomasz Figa wrote:
>>>>> Hi KyongHo,
>>>>> On 14.03.2014 06:10, Cho KyongHo wrote:
>>>>>> Some master device descriptor like fimc-is which is an abstraction
>>>>>> of very complex H/W may have multiple System MMUs. For those devices,
>>>>>> the design of the link between System MMU and its master H/W is
>>>>>> needed
>>>>>> to be reconsidered.
>>>>>> A link structure, sysmmu_list_data is introduced that provides a link
>>>>>> to master H/W and that has a pointer to the device descriptor of a
>>>>>> System MMU. Given a device descriptor of a master H/W, it is possible
>>>>>> to traverse all System MMUs that must be controlled along with the
>>>>>> master H/W.
>>>>> NAK.
>>>>> A device driver should handle particular hardware instances
>>>>> separately,
>>>>> without abstracting a virtual hardware instance consisting of multiple
>>>>> physical ones.
>>>>> If such abstraction is needed, it should be done above the
>>>>> exynos-iommu
>>>>> driver, e.g. by something like iommu-composite driver that would
>>>>> aggregate several IOMMUs. Keep in mind that such IOMMUs in a group
>>>>> could
>>>>> be different, e.g. different Exynos SysMMU versions or even completely
>>>>> different IPs handled by different drivers.
>>>>> Still, I don't think there is a real need for such abstraction.
>>>>> Instead,
>>>>> related drivers shall be fixed to properly handle multiple memory
>>>>> masters and their IOMMUs.
>>>> G2D, Scalers and FIMD of Exynos5420 has 2 System MMUs while aother
>>>> SoC like
>>>> Exynos5250 does not.
>>>> I don't understand why you are negative to this approach.
>>>> This is the simplest than the others.
>>>> Let me show you an example.
>>>> FIMC-IS driver just controls MCU in FIMC-IS subsystem and the
>>>> firmware of
>>>> the MCU controls all other peripherals in the subsystem. Each
>>>> peripherals
>>>> have their own System MMU. Moreover, the configuration of the
>>>> peripherals
>>>> varies according to the SoCs.
>>>> If System MMU driver accepts multiple masters, everything is done in
>>>> DT.
>>>> But I worry that it is not easy if System MMU driver does not support
>>>> multiple masters.
>>> I believe I have stated enough reasons why this kind of implementation
>>> is bad. I'm not going to waste time repeating myself.
>>> Your concerns presented above are valid, however they are not related to
>>> what is wrong with this patch. I have given you two proper ways to
>>> handle this, none should be forced upon particular IOMMU master drivers
>>> - their authors should have the chance to select the method that works
>>> best for them.
>> I don't still understand why you think this patch is wrong.
>> I think this is the best way not to think for all the driver developers
>> about other things than their business logic.
> I agree, but one of the ways I proposed (an iommu-composite layer above
> the IOMMU low level drivers) doesn't add any extra responsibility of
> driver developers.
> Moreover, it's this kind of business logic in low level drivers that is
> adding more responsibility, because it introduces additional complexity
> and makes the driver harder to read, maintain and extend in future.
>> This does not hurt anyone and I think this is good enough.
> Well, it is barely good enough. It is a good practice to make a low
> level driver handle a single device instance and this is how Linux
> driver model is designed.
> Moreover, a single device tree node _must_ represent a single hardware
> block, so you can't group multiple SysMMUs into a single device tree node.

OK, you add nodes for single SysMMUs devices which is fine, sorry. I was 
under impression that one kernel device (struct device) corresponds to 
multiple SysMMUs, but this was before your patches, sorry. So one issue 
less, but it's still not good.

> Furthermore, if you force grouping of SysMMUs into a single virtual one,
> you enforce using the same address space for all masters of some
> particular hardware blocks, while potentially driver developers would
> like to separate them.

Probably some clarification is needed. Your other patch adds:

	sysmmu_fimd0w04: sysmmu@...40000 {
		compatible = "samsung,sysmmu-v3.3";
		reg = <0x14640000 0x1000>;
		interrupt-parent = <&combiner>;
		interrupts = <3 2>;
		clock-names = "sysmmu", "master";
		clocks = <&clock 422>, <&clock 421>;
		samsung,power-domain = <&disp_pd>;
		mmu-masters = <&fimd>;

	sysmmu_fimd0w123: sysmmu@...80000 {
		compatible = "samsung,sysmmu-v3.3";
		reg = <0x14680000 0x1000>;
		interrupt-parent = <&combiner>;
		interrupts = <3 0>;
		clock-names = "sysmmu", "master";
		clocks = <&clock 423>, <&clock 421>;
		samsung,power-domain = <&disp_pd>;
		mmu-masters = <&fimd>;

 From such description, in future FIMD driver won't be able to determine 
which SysMMU is used for windows 0 and 4 and which for windows 1, 2 and 
3. However it would be desirable to handle both SysMMUs separately, 
allowing each SysMMU to have only mappings for frame buffers needed by 
respective FIMD windows.

> A good interface design should not enforce any particular implementation
> and this is what we should stick to in this case as well.
>> If you want to provide another layer between master device and system mmu
>> as you mentioned, you do that. This patch does not restrict it.
> It does, as mentioned above. With a device tree listing multiple SysMMUs
> as one, you can't separate them.

What I mean is that based on DT description, drivers should be able to 
control SysMMUs separately if they want to.

Best regards,
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at
Please read the FAQ at

Powered by blists - more mailing lists