linux-kernel - Re: [PATCH] irqchip: fix the config HISILICON_IRQ

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: <57692C7C.1070703@hisilicon.com>
Date:	Tue, 21 Jun 2016 20:01:00 +0800
From:	Jiancheng Xue <xuejiancheng@...ilicon.com>
To:	Marc Zyngier <marc.zyngier@....com>, <tglx@...utronix.de>,
	<jason@...edaemon.net>, <majun258@...wei.com>
CC:	<linux-kernel@...r.kernel.org>, <yanhaifeng@...ilicon.com>
Subject: Re: [PATCH] irqchip: fix the config HISILICON_IRQ_MBIGEN dependency
 error.



On 2016/6/21 19:30, Jiancheng Xue wrote:
> Hi Marc，
> 
> On 2016/6/21 18:36, Marc Zyngier wrote:
>> On 21/06/16 10:26, Jiancheng Xue wrote:
>>> This patch fixes the compiling error caused when
>>> config HISILICON_IRQ_MBIGEN is selected but
>>> PCI_MSI is not seleted.
>>>
>>> Signed-off-by: Jiancheng Xue <xuejiancheng@...ilicon.com>
>>> ---
>>>  drivers/irqchip/Kconfig | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
>>> index fa33c50..23dcf3e 100644
>>> --- a/drivers/irqchip/Kconfig
>>> +++ b/drivers/irqchip/Kconfig
>>> @@ -110,7 +110,7 @@ config DW_APB_ICTL
>>>  config HISILICON_IRQ_MBIGEN
>>>  	bool
>>>  	select ARM_GIC_V3
>>> -	select ARM_GIC_V3_ITS
>>> +	select ARM_GIC_V3_ITS if PCI_MSI
>>>  	select GENERIC_MSI_IRQ_DOMAIN
>>
>> How can this be correct? The MBIGEN uses platform MSI (not PCI) and
>> relies on the ITS (it doesn't work without it). It seems that you're
>> papering over another issue.
>>
> Sorry. I am not familiar with this part. But I encountered errors when
> I compiled. I think there may be some problems about dependency.
> 
> In this Kconfig file
> 
> config ARM_GIC_V3_ITS
>         bool
>         select PCI_MSI_IRQ_DOMAIN
> 
> config HISILICON_IRQ_MBIGEN
>   	bool
>  	select ARM_GIC_V3
> 	select ARM_GIC_V3_ITS
>   	select GENERIC_MSI_IRQ_DOMAIN
> 
> In the file drivers/pci/Kconfig
> 
> config PCI_MSI_IRQ_DOMAIN
>         bool
>         depends on PCI_MSI
>         select GENERIC_MSI_IRQ_DOMAIN
> 
> We can see if the HISILICON_IRQ_MBIGEN is selected, the ARM_GIC_V3_ITS and
> PCI_MSI_IRQ_DOMAIN will be selected. But PCI_MSI_IRQ_DOMAIN depends on
> PCI_MSI. If PCI_MSI is not selected, it will cause a compiling error like this
> "drivers/irqchip/irq-gic-v3-its-pci-msi.c:52:12: error: implicit declaration of function 'pci_msi_vec_count' [-Werror=implicit-function-declaration]
>   msi = max(pci_msi_vec_count(pdev), 0);"
> 
> I found many other options which need ARM_GIC_V3_ITS or PCI_MSI_IRQ_DOMAIN
> were configured like below:
> select ARM_GIC_V3_ITS if PCI_MSI
> or
> select GENERIC_MSI_IRQ_DOMAIN if PCI_MSI
> 

This patch is really not correct. It will cause new problems.

In drivers/irqchip/Makefile

obj-$(CONFIG_ARM_GIC_V3_ITS)            += irq-gic-v3-its.o irq-gic-v3-its-pci-msi.o irq-gic-v3-its-platform-msi.o

if CONFIG_ARM_GIC_V3_ITS is selected, irq-gic-v3-its-pci-msi.c will be compiled. But it depends on config PCI_MSI.

if irq-gic-v3-its-pci-msi.c is not relied on by irq-gic-v3-its.c, maybe it should be controlled by another config
item instead of CONFIG_ARM_GIC_V3_ITS. Otherwise, CONFIG_ARM_GIC_V3_ITS should depend on PCI_MSI.

> Regards,
> Jiancheng
> 
> 
>