lkml.org 
[lkml]   [2021]   [Oct]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 1/5] PCI/switchtec: Error out MRPC execution when no GAS access
Date
On Thu, 2021-10-07 at 16:23 -0500, Bjorn Helgaas wrote:
> 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:
> > > On Wed, Oct 06, 2021 at 07:00:55PM +0000,
> > > Kelvin.Cao@microchip.com
> > > wrote:
> > > 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.
> >
> > > > - 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.

Actually there're mechanisms like permission control to limit what
people can do in the firmware, so I guess it's not as bad as it sounds
like.
>
> 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.

It looks good to me, the commit log removes the ambiguity. Let me know
if you prefer a v2 patchset with the updated commit log and naming
issue fix.

Thank you Bjorn for your patience and time!

Kelvin
\
 
 \ /
  Last update: 2021-10-08 02:07    [W:0.104 / U:0.556 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site