linux-kernel - Re: [PATCH v3 6/8] irqchip/gic-v3-its: Initialize its nodes later

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]

Date:   Mon, 21 Aug 2017 09:30:24 +0100
From:   Marc Zyngier <marc.zyngier@....com>
To:     Robert Richter <rrichter@...ium.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Jason Cooper <jason@...edaemon.net>
Cc:     Shanker Donthineni <shankerd@...eaurora.org>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Lorenzo Pieralisi <Lorenzo.Pieralisi@....com>
Subject: Re: [PATCH v3 6/8] irqchip/gic-v3-its: Initialize its nodes later

+Lorenzo

On 08/08/17 13:22, Robert Richter wrote:
> Use an initcall to initialize its. This allows us to use the device
> framework during initialization that is up at this point. We use
> subsys_initcall() here since we need the arch to be initialized
> first. It is before pci and platform device probe where devices are
> bound to msi interrupts.
> 
> Signed-off-by: Robert Richter <rrichter@...ium.com>
> ---
>  drivers/irqchip/irq-gic-v3-its.c   | 3 ++-
>  drivers/irqchip/irq-gic-v3.c       | 5 -----
>  include/linux/irqchip/arm-gic-v3.h | 1 -
>  3 files changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 5e2d4f2876d8..488f811d5978 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -1994,7 +1994,7 @@ int __init its_probe(struct fwnode_handle *handle, struct rdists *rdists,
>  	return 0;
>  }
>  
> -int __init its_init(void)
> +static int __init its_init(void)
>  {
>  	struct its_node *its, *tmp;
>  	int err = 0, err2;
> @@ -2036,3 +2036,4 @@ int __init its_init(void)
>  
>  	return 0;
>  }
> +subsys_initcall(its_init);

*ding*!

I'm sorry, but that's a total NAK. We're trading hard to maintain
hardcoded dependencies for even more difficult to deal with, link-order
driven dependencies.  That's exactly what Lorenzo and I have been
fighting against in the XGene ACPI case, and I'm not going to introduce
more of this.

We need to break the dependency between the HW and the associated MSI
domains, not making it tighter. Let the HW be probed at some time, and
the MSI domains lazily created as needed once the end-points are probed.

I'm also pretty worried that (as mentioned in my reply to patch #2) that
this "make it happen later" games with the initcalls are breaking stuff
(see how platform devices are instantiated on the back of an
arch_initcall_sync). As far as I can tell, non-PCI MSI is dead after
patch #2.

I think there is a number of things to fix in the core code before we
can start playing that kind of tricks. The major issue I see is that MSI
domains are expected to be available way too early, at device discovery
time rather than at driver probe time. This has a cascading effect which
we should solve first.

I'll try to prototype something...

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...