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  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, 16 Jan 2017 22:23:16 +0800
From:   Hanjun Guo <>
To:     Lorenzo Pieralisi <>,
        "Rafael J. Wysocki" <>
CC:     Hanjun Guo <>, <>,
        Kefeng Wang <>, <>,
        Jon Masters <>,
        Marc Zyngier <>,
        Greg KH <>,
        <>, <>,
        Sinan Kaya <>,
        Xinwei Kong <>,
        Matthias Brugger <>,
        Tomasz Nowicki <>,
        Thomas Gleixner <>,
        Agustin Vega-Frias <>,
        Ma Jun <>
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.


Powered by blists - more mailing lists