On 2017-12-29 22:53, Keith Busch wrote:
On Fri, Dec 29, 2017 at 12:54:17PM +0530, Oza Pawandeep wrote:
This patch addresses the race condition between AER and DPC for
recovery.
Current DPC driver does not do recovery, e.g. calling end-point's
driver's
callbacks, which sanitize the device.
DPC driver implements link_reset callback, and calls pci_do_recovery.
I'm not sure I see why any of this is necessary for two reasons:
1. A downstream port containment event disables the link. How can a
driver
sanitize an end device when all the end devices below the containment
are
physically inaccessible? Any attempt to access such devices will just
end with either CA or UR (depending on DPC control settings). Since we
already know the failed outcome from attempting to access such devices,
why do you want the drivers to do anything?
Ok I think my statement was misleading, not device sanitation, but the
device driver making
SW sanitize.
for e.g. have a look at e1000_io_error_detected which is called say in
case of AER ERR_FATAL msg.
which sanitizes sw stack, interrupts management (synchronize_irq),
delete timers etc..
yes, DPC would have made the link state disabled, and HW would have
reset the internal logic with
quiescence activities so yes, any transaction on will end with CA or UR.
well but device driver
has to handle rest of the possible things as I mentioned (error
callbacks)
2. A DPC event suppresses the error message required for the Linux
AER driver to run. How can AER and DPC run concurrently?
I afraid I could not grasp the first line completely.
but they way it is triggering AER and DPC on our platform concurrently
is, we have same MSIx registered
for both AER and DPC, and linux calls the shared handlers to handle both
the triggers anyway.
otherwise also if ERR_FATAL msg occurs, the Root port should trigger
both AER and DPC
(assuming both are enabled, and no FW first for AER/DPC)
the problem with the current framework of AER and DPC in Linux is:
both try to act independently, while we know that (for e.g. ERR_FATAL
msg) is responsible for triggering
both AER and DPC depending on the configuration. (currently DPC is
configured for both FATAL and NONFATAL in linux anyway)
It does not make sense that AER goes ahead and attempts to sanitize with
the device driver's callbacks as I mentioned.
and DPC being unaware, asynchronously disables the link (although this
is all HW)
but DPC service driver should adapt to some kind of error handling and
error resume which AER has adapted.
Hence this whole design changes proposed with respect to error handling.
Let me give you another problem statement on the same line:
when DPC is active, AER does not need to act at all...because it doesnt
make sense for AER to act independently.,
without knowing what DPC service driver is upto!
which is handled in one of the patches.
the point I am trying to make is: DPC should not rely on AER to call
error callbacks, and AER should not be doing it without knowing that
DPC is active and it is also going to some course of action (be in HW or
SW)
Regards,
Oza.
