linux-kernel - Re: [PATCH v5 1/3] initialize each mbigen device node as a interrupt controller.

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: <561CA58C.2060207@huawei.com>
Date:	Tue, 13 Oct 2015 14:32:44 +0800
From:	"majun (F)" <majun258@...wei.com>
To:	Thomas Gleixner <tglx@...utronix.de>,
	Marc Zyngier <marc.zyngier@....com>
CC:	<Catalin.Marinas@....com>, <linux-kernel@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>, <Will.Deacon@....com>,
	<mark.rutland@....com>, <jason@...edaemon.net>,
	<lizefan@...wei.com>, <huxinwei@...wei.com>,
	<dingtianhong@...wei.com>, <zhaojunhua@...ilicon.com>,
	<liguozhu@...ilicon.com>, <xuwei5@...ilicon.com>,
	<wei.chenwei@...ilicon.com>, <guohanjun@...wei.com>,
	<wuyun.wu@...wei.com>, <guodong.xu@...aro.org>,
	<haojian.zhuang@...aro.org>, <zhangfei.gao@...aro.org>,
	<usman.ahmad@...aro.org>, <klimov.linux@...il.com>
Subject: Re: [PATCH v5 1/3] initialize each mbigen device node as a interrupt
 controller.

Hi Thomas:

在 2015/10/12 0:45, Thomas Gleixner 写道:
> On Sun, 11 Oct 2015, Marc Zyngier wrote:
>> On Sun, 11 Oct 2015 11:54:49 +0200
>> Thomas Gleixner <tglx@...utronix.de> wrote:
>>
>>> On Sat, 10 Oct 2015, Marc Zyngier wrote:
>>>> On Sat, 10 Oct 2015 17:01:32 +0800
>>>> "majun (F)" <majun258@...wei.com> wrote:
>>>>> But there is a problem If i make the structure like you said.
>>>>>
>>>>> For example, my hardware structure likes below:
>>>>>
>>>>> uart ------> mbigen --> ITS-pMSI --> ITS --> GIC
>>>>>      virq1
>>>>>
>>>>> virq1 means the virq number allocted by irq_of_parse_and_map() function
>>>>> when system parse the uart dts node in initializing  stage.
>>>>>
>>>>> To create a ITS device, I need to call msi_domain_alloc_irqs() function
>>>>> in my mbigen alloc function.
>>>>>
>>>>> In this function, a new virq number(named as virq2 ) which different from
>>>>> virq1 is allocated.
>>>>> So, this is a big problem.
>>>>
>>>> I think I see what your problem is:
>>>> - The wired interrupt (uart -> mbigen) is allocated through DT (and
>>>>   must be available early, because of of_platform_populate),
>>>> - The MSI (mgigen -> ITS) is dynamic (and allocated much later,
>>>>   because the device model kicks in after irqchip init, and we cannot
>>>>   allocate MSIs without a device).
>>>
>>> Why do we need that wired interrupt at all? 
>>>
>>> We can make mbigen the 'msi-parent' of the device and let the
>>> msi_domain_ops::msi_prepare() callback figure out the actual wiring
>>> through device->fwnode.
>>
>> That's because the device behind the mbigen can't do any MSI at all.
>> Think of a 8250 uart, for example.
>>
>> If we make the mbigen the msi-parent of the uart, then we need to teach
>> the 8250 driver to request MSIs.
> 
> I really do not understand why of_platform_populate cares about
> interrupt allocations. That's outright stupid. We should care about
> that at device probe time, i.e. at the point where the driver is
> registered and probed if there is matching platform data. If we do it
> in of_platform_populate then we allocate interrupts even for devices
> which do not have a driver, i.e. we just waste memory.
> 
> So we better teach a couple of drivers to handle that instead of
> inventing horrible workarounds.
> 
>> It also means that the DT doesn't represent the HW anymore (this
>> wired interrupt actually exists).
> 
> I think the abstraction here is wrong. If it would be correct, then
> PCI-MSI would be wrong. The MSI part of PCI is a MSI producer, mbigen
> is as well. Technically MSI is not integral part of the PCI device, it
> just happens to have it's configuration registers in the PCI
> configuration space of the device:
> 
>     [PCI-BUS]------[Interrupt mode selector]
>     	      	|        |
>     	      	|        |
> 		------[Legacy irq gate]<-----
> 		|        |                  |
> 		|        |                  |---[Device interrupt]
> 		|        |                  |
> 		------[MSI unit]<------------
> 
> So you have a 'wire' from the device to the MSI unit, but we do not
> care about that 'wire'. All we care about are the MSI configuration
> registers. We find them via the PCI device which gives us the address
> of the PCI configuration space.
> 
> So now in the mbigen case this looks like this:
> 
>     [MSI-BUS] ----- [MBIGEN]<-------------------[Device interrupt]
> 
> Again, you have a 'wire' from the device to the MSI unit (MBIGEN) and
> we do not care about that 'wire' either. What we care about is how we
> find the MSI (mbigen) configuration registers for a particular
> device. So we need a DT/ACPI entry which describes those configuration
> registers and whatever supplementary information is required. That
> will make the mbigen driver extremly simple.
> 

According to your suggestions, I tried to make the hardware structure likes below:

device(8250 uart) -> mbigne -> ITS-pMSI --> ITS --> GIC

And  8250 uart dts node is:

 8250_uart {
	compatible = "xxx";
	msi-parent = < &mbigen>;
	config_addr = <xxxxx> ; /* configuration register */
	interrupts = <x>;
	interrupt-parent = ?
}

My question is what's the interrupt-parent should be?



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/