On Mon, Jul 13, 2020 at 8:32 PM Alexey Kardashevskiy <a...@ozlabs.ru> wrote:
>
> > #ifdef CONFIG_PCI_IOV
> > int pseries_send_allow_unfreeze(struct pci_dn *pdn,
> > u16 *vf_pe_array, int cur_vfs)
> > @@ -848,7 +824,7 @@ static struct eeh_ops pseries_eeh_ops = {
> > .read_config = pseries_eeh_read_config,
> > .write_config = pseries_eeh_write_config,
> > .next_error = NULL,
> > - .restore_config = pseries_eeh_restore_config,
> > + .restore_config = NULL, /* NB: configure_bridge() does this */
>
>
> configure_bridge() calls rtas_call(ibm_configure_pe, 3, 1, NULL...)
> which reconfigures the PE and which is quite different from what
> pseries_eeh_restore_config() used to do although the comment suggests it
> is about the same thing. I am pretty sure the new code produces a better
> result so I suggest ditching this comment and adding a note to the
> commit log may be. Thanks,
I put the comment there largely because the EEH core seems to think
that restore_config() is what should be called to reset the device's
config space to the defaults set be firmware. On PowerNV it does
actually do that and configure_bridge is this:
static int pnv_eeh_configure_bridge(struct eeh_pe *pe)
{
return 0;
}
So... there's definitely something strange going on there. I don't
remember the exact details, but I think the generic EEH code calls
into RTAS to collect debug data and apparently that requires the
device to be accessible via MMIO (i.e BARs need to be restored) which
is why the pseries .configure_bridge() calls configure_pe. It might
work out better, but having something called "restore_config" that
doesn't actually restore the config is uh... modern. It's something
that probably needs a rework at some point. Anyway, I think the
comment is more helpful than it is misleading. Especially if you
consider the PowerNV behaviour.
> > #ifdef CONFIG_PCI_IOV
> > .notify_resume = pseries_notify_resume
> > #endif
> >
>
> --
> Alexey
