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: <alpine.DEB.2.11.1510111335150.6097@nanos>
Date:	Sun, 11 Oct 2015 18:45:48 +0200 (CEST)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Marc Zyngier <marc.zyngier@....com>
cc:	"majun (F)" <majun258@...wei.com>, 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.

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.

Thanks,

	tglx

--
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/