On Wed, Oct 06, 2021 at 09:27:49PM +0000, Kelvin.Cao@microchip.com wrote:

On Wed, 2021-10-06 at 15:20 -0500, Bjorn Helgaas wrote:

quoted

On Wed, Oct 06, 2021 at 07:00:55PM +0000, Kelvin.Cao@microchip.com
wrote:

quoted

So wait, you mean you just intentionally ask the firmware to
reset, knowing that the device will be unusable until the user
reboots or does a manual rescan?  And the way to improve this is
for the driver to report an error to the user instead of hanging?
I *guess* that might be some sort of improvement, but seems like a
pretty small one.

Yes, however, I believe it's something our users really like to
have...  With this, they can do their user space
programming/scripting more easily in a synchronous fashion.

quoted

quoted

  - The firwmare crashes and doesn't respond, which normally is
  the reason for users to issue a firmware reset command to try
  to recover it via either the driver or a sideband interface.
  The firmware may not be able to recover by a reset in some
  extream situations like hardware errors, so that an error
  return is probably all the users can get before another level
  of recovery happens.

So I'd think this patch is still making the driver better in
some way.

OK.  I still think the fact that all these different mechanisms can
reset the device behind your back and make the switch and anything on
the other side of it just stop working is ..., well, let's just say
it's quite surprising to me.

Well, at least this isn't quite so much a mystery anymore and maybe we
can improve the commit log.  E.g., maybe something like this:

  A firmware hard reset may be initiated by various mechanisms
  including a UART interface, TWI sideband interface from BMC, MRPC
  command from userspace, etc.  The switchtec management driver is
  unaware of these resets.

  The reset clears PCI state including the BARs and Memory Space
  Enable bits, so the device no longer responds to the MMIO accesses
  the driver uses to operate it.

  MMIO reads to the device will fail with a PCIe error.  When the root
  complex handles that error, it typically fabricates ~0 data to
  complete the CPU read.

  Check for this sort of error by reading the device ID from MMIO
  space.  This ID can never be ~0, so if we see that value, it
  probably means the PCIe Memory Read failed and we should return an
  error indication to the application using the switchtec driver.