Re: [RFC][PATCH] Reset PCIe devices to address DMA problem on kdump with iommu

[Takao Indoh <indou.takao@jp.fujitsu.com>]

(2012/09/15 0:48), Vivek Goyal wrote:
> On Wed, Sep 12, 2012 at 06:00:55PM +0900, Takao Indoh wrote:
>> (2012/09/11 23:43), Vivek Goyal wrote:
>>> On Tue, Sep 11, 2012 at 07:32:35PM +0900, Takao Indoh wrote:
>>>
>>> [..]
>>>> I'll post new patch which clears bus master bit and resets devices in
>>>> second kernel.
>>>>
>>>> As to the boot parameter to enable this function, you suggested using
>>>> reset_devices. I found that on a certain platform resetting devices
>>>> caused PCIe error due to a hardware bug. Therefore I think we need
>>>> new parameter apart from reset_devices to disable this function on
>>>> such a machine.
>>>
>>> Can you explain a bit more how the error happens. I still don't think
>>> that because of a bug in a platform somewhere we should be introducing
>>> a separate command line parameter and not reuse the exisiting one. Also
>>> you have not explained what's the bug and how a new parameter will
>>> avoid the bug.
>>
>> The bug I mentioned is that ACS Violation occurs at PCIe switch when
>> reading PCI configuration after device reset. I got information that
>> this violation is caused by PCIe switch bug. The machine becomes fatal
>> status by this error.
>>
>> The reason why I try to introduce new parameter is that I want to avoid
>> regression by this patch. Let's say this patch was included in kernel
>> and its reset function was enabled by reset_devices as you said. AFAIK
>> reset_devices is always needed for kdump, so it means that devices are
>> always reset at kdump boot time. It causes a regression that system
>> always becomes abnormal status when we run kdump on the machine which has
>> a bug I mentioned.
>>
>> To avoid this regression, I want to separate reset_devices from this
>> reset function. Or how about this?
>> - if user specify reset_devices, devices are reset by this patch, as you
>>    said.
>> - To avoid a regression I said, add new parameter like "pci=noreset".
>>    If this parameter is specified, the reset function I add is disabled
>>    and we can avoid regression.
>
> Can we identify that particular switch in code and not reset it in code.
> Introducing new paramenters to avoid bugs really feels odd.

Maybe we can do it using PCI quirk or DMI quirk as Konrad and Don said.
But I'm still thinking whether I can add boot parameter or something to
disable reset function so that we can use it as workaround untill we add
quirk when we find such a buggy hardware.

> Also, what was the conclusion to avoid double reset. I am assuming that
> we don't want to do bus level reset as well as driver level reset based
> on reset_devices.

I don't have a good idea to do it yet. Maybe we need to write bus:slot:func
information to somewhere when we reset device.

Thanks,
Takao Indoh