linux-kernel - Re: [PATCH v18 03/11] PCI/DPC: Fix DPC recovery issue in non hotplug case

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: <621ef46b-3556-e667-3982-16f3c908a793@linux.intel.com>
Date:   Sat, 28 Mar 2020 15:40:23 -0700
From:   "Kuppuswamy, Sathyanarayanan" 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>
To:     Bjorn Helgaas <helgaas@...nel.org>
Cc:     linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
        ashok.raj@...el.com
Subject: Re: [PATCH v18 03/11] PCI/DPC: Fix DPC recovery issue in non hotplug
 case



On 3/28/20 3:21 PM, Bjorn Helgaas wrote:
>>> OK, thanks.  I'm still uncomfortable with this issue, so I think I'm
>>> going to apply this series but omit this patch.  Here's why:
>>>
>>> 1) The fact that resets may cause hotplug events isn't specific to
>>> DPC, so I don't think dpc_reset_link() is the right place.  For
>>> instance, aer_root_reset() ultimately does a secondary bus reset.
>> Agree. Reset is common for pci_channel_io_frozen errors. I did not
>> look into aer_root_reset() implementation. So if state
>> is pci_channel_io_frozen then we can assume the slot has been
>> reseted.
>>   The
>>> pci_slot_reset() -> pciehp_reset_slot() path goes to some trouble to
>>> ignore the resulting hotplug event, but the pci_bus_reset() path does
>>> not.
>>>
>>> 2) I'm not convinced that "hotplug_is_native()" is the correct test.
>>> Even if we're using ACPI hotplug (acpiphp), that will detach the
>>> drivers and remove the devices, won't it?
>> Yes, agreed. It does not handle ACPI hotplug case. In case of
>> ACPI hotplug, native_pcie_hotplug = 0. May be we need a new helper
>> function. hotplug_is_enabled() ?
> I'm not proposing the patch below to be applied.  I only included it
> as an idea of where the hotplug testing should be.
> 
> I'm proposing to merge the pci/edr branch as-is, without these two
> patches:
> 
>    PCI: move {pciehp,shpchp}_is_native() definitions to pci.c
>    PCI/DPC: Fix DPC recovery issue in non hotplug case
> 
> accepting that we still have some issues in the non-hotplug case that
> we can fix later.
Ok. I am fine with it. Thanks for working on it.
>