linux-kernel - Re: [PATCH] PCI/DPC: Fix INT legacy interrupt in dpc

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: <d759fe9efb8e5ece4097375e97772565@codeaurora.org>
Date:   Thu, 01 Mar 2018 21:17:47 +0530
From:   poza@...eaurora.org
To:     Bjorn Helgaas <helgaas@...nel.org>
Cc:     Bjorn Helgaas <bhelgaas@...gle.com>,
        Philippe Ombredanne <pombredanne@...b.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Kate Stewart <kstewart@...uxfoundation.org>,
        linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
        Dongdong Liu <liudongdong3@...wei.com>,
        Keith Busch <keith.busch@...el.com>, Wei Zhang <wzhang@...com>,
        Sinan Kaya <okaya@...eaurora.org>,
        Timur Tabi <timur@...eaurora.org>
Subject: Re: [PATCH] PCI/DPC: Fix INT legacy interrupt in dpc_irq

On 2018-03-01 21:08, Bjorn Helgaas wrote:
> Hi Oza,
> 
> On Wed, Jan 31, 2018 at 09:48:55PM +0530, Oza Pawandeep wrote:
>> Current dpc driver acknowledge the interrupt in deferred work, which 
>> works
>> okay since LPI are edge triggered.
>> But when RP does not have MSI support, port service driver falls back 
>> to
>> legacy GIC SPI interrupts, and with current code we do not acknowledge 
>> the
>> interrupt and we get dpc interrupt storm.
>> This patch acknowledges the interrupt in interrupt handler.
> 
> Would you mind rephrasing this changelog in generic PCIe terms?  E.g.,
> "LPI" and "GIC SPI" are meaningless to me (I assume they're
> arm64-specific things), but I don't think it's any arm64-specific
> thing that motivates this change.  IIUC, the issue should occur any
> time we're not using MSI, so we should be able to describe it in
> purely PCIe terms.
> 
> s/dpc/DPC/ in text.
> 
> It looks like this changelog is intended to be multiple paragraphs; if
> so, please leave a blank line between them.  It's a little awkward to
> infer "this is a new paragraph" from the fact that "the previous line
> didn't look full" :)
> 
> Please rebase to my "master" branch (v4.16-rc1).  There was some minor
> change that made this not apply cleanly.
> 

sure will do these changes and re-post the same.

Regards,
Oza.

>> Signed-off-by: Oza Pawandeep <poza@...eaurora.org>
>> 
>> diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c
>> index 2d976a6..570b561 100644
>> --- a/drivers/pci/pcie/pcie-dpc.c
>> +++ b/drivers/pci/pcie/pcie-dpc.c
>> @@ -134,7 +134,7 @@ static void interrupt_event_handler(struct 
>> work_struct *work)
>>  	}
>> 
>>  	pci_write_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_STATUS,
>> -		PCI_EXP_DPC_STATUS_TRIGGER | PCI_EXP_DPC_STATUS_INTERRUPT);
>> +			      PCI_EXP_DPC_STATUS_TRIGGER);
>>  }
>> 
>>  static void dpc_rp_pio_print_tlp_header(struct device *dev,
>> @@ -277,6 +277,8 @@ static irqreturn_t dpc_irq(int irq, void *context)
>> 
>>  		schedule_work(&dpc->work);
>>  	}
>> +	pci_write_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_STATUS,
>> +			      PCI_EXP_DPC_STATUS_INTERRUPT);
>>  	return IRQ_HANDLED;
>>  }
>> 
>> --
>> Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm 
>> Technologies, Inc.,
>> a Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a 
>> Linux Foundation Collaborative Project.
>>