[PATCH v3 2/2] pseries/eeh: Add Pseries pcibios_bus_add_device

[Bjorn Helgaas]

On Thu, Oct 12, 2017 at 03:09:53PM +1100, Michael Ellerman wrote:
> Bjorn Helgaas <helgaas at kernel.org> writes:
> 
> > On Fri, Sep 22, 2017 at 09:19:28AM -0500, Bryant G. Ly wrote:
> >> This patch adds the machine dependent call for
> >> pcibios_bus_add_device, since the previous patch
> >> separated the calls out between the PowerNV and PowerVM.
> >> 
> >> The difference here is that for the PowerVM environment
> >> we do not want match_driver set because in this environment
> >> we do not want the VF device drivers to load immediately, due to
> >> firmware loading the device node when VF device is assigned to the
> >> logical partition.
> >> 
> >> This patch will depend on the patch linked below, which is under
> >> review.
> >> 
> >> https://patchwork.kernel.org/patch/9882915/
> >> 
> >> Signed-off-by: Bryant G. Ly <bryantly at linux.vnet.ibm.com>
> >> Signed-off-by: Juan J. Alvarez <jjalvare at us.ibm.com>
> >> ---
> >>  arch/powerpc/platforms/pseries/eeh_pseries.c | 24 ++++++++++++++++++++++++
> >>  1 file changed, 24 insertions(+)
> >> 
> >> diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
> >> index 6b812ad990e4..45946ee90985 100644
> >> --- a/arch/powerpc/platforms/pseries/eeh_pseries.c
> >> +++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
> >> @@ -64,6 +64,27 @@ static unsigned char slot_errbuf[RTAS_ERROR_LOG_MAX];
> >>  static DEFINE_SPINLOCK(slot_errbuf_lock);
> >>  static int eeh_error_buf_size;
> >>  
> >> +void pseries_pcibios_bus_add_device(struct pci_dev *pdev)
> >> +{
> >> +	struct pci_dn *pdn = pci_get_pdn(pdev);
> >> +
> >> +	if (!pdev->is_virtfn)
> >> +		return;
> >> +
> >> +	pdn->device_id  =  pdev->device;
> >> +	pdn->vendor_id  =  pdev->vendor;
> >> +	pdn->class_code =  pdev->class;
> >> +
> >> +	/*
> >> +	 * The following operations will fail if VF's sysfs files
> >> +	 * aren't created or its resources aren't finalized.
> >> +	 */
> >> +	eeh_add_device_early(pdn);
> >> +	eeh_add_device_late(pdev);
> >> +	eeh_sysfs_add_device(pdev);
> >> +	pdev->match_driver = -1;
> >
> > match_driver is a bool, which should be assigned "true" or "false".
> 
> Above he mentioned a dependency on:
> 
>   [04/10] PCI: extend pci device match_driver state
>   https://patchwork.kernel.org/patch/9882915/
> 
> 
> Which makes it an int.

Oh, right, I missed that, thanks.

> Or has that patch been rejected or something?

I haven't *rejected* it, but it's low on my priority list, so you
shouldn't depend on it unless it adds functionality you really need.
If I did apply that particular patch, I would want some rework because
it currently obfuscates the match_driver logic.  There's no clue when
reading the code what -1/0/1 mean.

Apparently here you *do* want the "-1 means the PCI core will never
set match_driver to 1" functionality, so maybe you do depend on it.

If that's the case, how to you ever bind a driver to these VFs?  The
changelog says you don't want VF drivers to load *immediately*, so I
assume you do want them to load eventually.

Bjorn