linux-kernel - Re: [PATCH v7 15/15] irqchip: mbigen: Add ACPI support

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: <587CD754.1050808@huawei.com>
Date:   Mon, 16 Jan 2017 22:23:16 +0800
From:   Hanjun Guo <guohanjun@...wei.com>
To:     Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        "Rafael J. Wysocki" <rafael@...nel.org>
CC:     Hanjun Guo <hanjun.guo@...aro.org>, <huxinwei@...wei.com>,
        Kefeng Wang <wangkefeng.wang@...wei.com>, <yimin@...wei.com>,
        Jon Masters <jcm@...hat.com>,
        Marc Zyngier <marc.zyngier@....com>,
        Greg KH <gregkh@...uxfoundation.org>,
        <linux-kernel@...r.kernel.org>, <linuxarm@...wei.com>,
        Sinan Kaya <okaya@...eaurora.org>,
        <linux-acpi@...r.kernel.org>,
        Xinwei Kong <kong.kongxinwei@...ilicon.com>,
        Matthias Brugger <mbrugger@...e.com>,
        Tomasz Nowicki <tn@...ihalf.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Agustin Vega-Frias <agustinv@...eaurora.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        Ma Jun <majun258@...wei.com>
Subject: Re: [PATCH v7 15/15] irqchip: mbigen: Add ACPI support

Hi Lorenzo,

On 2017/1/16 19:38, Lorenzo Pieralisi wrote:
> On Sat, Jan 14, 2017 at 10:56:54AM +0800, Hanjun Guo wrote:
>> Hi Lorenzo,
>>
>> On 2017/1/13 18:21, Lorenzo Pieralisi wrote:
>>> On Wed, Jan 11, 2017 at 11:06:39PM +0800, Hanjun Guo wrote:
>>>> With the preparation of platform msi support and interrupt producer
>>>> in DSDT, we can add mbigen ACPI support now.
>>>>
>>>> We are using _PRS methd to indicate number of irq pins instead
>>>> of num_pins in DT to avoid _DSD usage in this case.
>>>>
>>>> For mbi-gen,
>>>>     Device(MBI0) {
>>>>           Name(_HID, "HISI0152")
>>>>           Name(_UID, Zero)
>>>>           Name(_CRS, ResourceTemplate() {
>>>>                   Memory32Fixed(ReadWrite, 0xa0080000, 0x10000)
>>>>           })
>>>>
>>>>           Name (_PRS, ResourceTemplate() {
>>>> 		  Interrupt(ResourceProducer,...) {12,14,....}
>>> I still do not understand why you are using _PRS for this, I think
>>> the MBIgen configuration is static and if it is so the Interrupt
>>> resource should be part of the _CRS unless there is something I am
>>> missing here.
>> Sorry for not clear in the commit message. MBIgen is an interrupt producer
>> which produces irq resource to devices connecting to it, and MBIgen itself
>> don't consume wired interrupts.
> That's why you mark it as ResourceProducer, but that's not a reason to
> put it in the _PRS instead of _CRS.

If using _CRS for the interrupt resource, the irq number represented will be mapped
(i.e acpi_register_gsi()), then will conflict with the irq number of devices consuming
it (mbigen is producing the interrupts), but I agree with you that let's ask Rafael's
point of view.

>
> IIUC _PRS is there to provide a way to define the possible resource
> settings of a _configurable_ device (ie programmable) so that the actual
> resource value you would programme with a call to its _SRS is sane (ie
> the OS has a way, through the _PRS, to detect what possible resource
> settings are available for the device).
>
> I think Rafael has more insights into how the _PRS is used on x86
> systems so I would ask his point of view here before merrily merging
> this code.

OK, Rafael is traveling now, hope he will have time to take a look.

How about updating this patch set then sending a new version for review
with this patch unchanged? if Rafael have comments on this one, I will
send a single updated one for this patch (if no other changes).

>
>> Also devices connecting MBIgen may not consume all the interrupts produced
>> by MBIgen, for example, MBIgen may produce 128 interrupts but only half of
>> them are currently used, so _PRS here means "provide interrupt resources
>> may consumed by devices connecting to it".
> See above.

Thanks
Hanjun