On Tue, Sep 20, 2011 at 6:48 PM, Nishanth Aravamudan <nacc@xxxxxxxxxx> wrote:
> Hi Jesse,
>
> We've gotten a few reports of the following situation: a SR-IOV capable
> adapter in a ppc64 server (in some cases driven by the lpfc driver, in
> others by the be2net driver, but I don't think it is driver specific)
> fails to initialize due to a collision on BAR 7 (the first IOV
> resource), e.g.:
>
> 0000:98:00.1: device not available (can't reserve [mem 0xfffe0000-0x1001dffff 64bit])
>
> I'm testing the following change:
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 4e84fd4..17b651e 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -1126,9 +1126,14 @@ static int __pci_enable_device_flags(struct pci_dev *dev,
> if (atomic_add_return(1, &dev->enable_cnt) > 1)
> return 0; /* already enabled */
>
> - for (i = 0; i < DEVICE_COUNT_RESOURCE; i++)
> + for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
> +#ifdef CONFIG_PCI_IOV
> + if (i >= PCI_IOV_RESOURCES && i <= PCI_IOV_RESOURCE_END)
> + continue;
> +#endif
> if (dev->resource[i].flags & flags)
> bars |= (1 << i);
> + }
>
> err = do_pci_enable_device(dev, bars);
> if (err < 0)
>
> With this change, the driver does load, although there do still appear
> to be problems with upstream at that point that I'm still tracking down.
>
> The thinking is that it shouldn't be an error at this point in the code
> if we fail to enable the IOV BARs as we're not enabling IOV here in the
> first place. The failure point should be when the driver attempts to
> create VFs if we can't use the IOV BARs.
>
> I have a few questions:
>
> 1) Does this make sense to you? :)
>
> 2) Presuming the fix above *isn't* ok, do you have thoughts on
> a better approach? Keeping in mind that on power, we don't
> control the device resource assignment, so we are a little more
> stuck here, arguably.
>
> 3) pci_select_bars seems like it could be used by
> __pci_enable_device_flags as a cleanup? Would the above change
> be good to put there as well?
I'm not Jesse (who's on vacation for a couple weeks), but this does
make sense to me.
The VF BARs don't consume resources until we set VF Enable and VF MSE,
which happens in pci_enable_sriov(). I agree that the
pci_enable_device() for the PF should succeed and that the PF should
work as a normal non-SR-IOV device until the driver enables SR-IOV.
It seems a bit weird that the IOV resources leaked out into struct
pci_dev, resulting in this problem, #ifdefs like this to fix it, and
wasting space in the pci_dev for every non-SR-IOV device. I suppose
there's some reason they can't live in the struct pci_sriov?
Good point about pci_select_bars(), too. That looks like another
problem waiting to happen -- if a driver claiming the PF uses
pci_select_bars(), then pci_request_selected_regions(), it will
attempt to request the VF BARs, which it shouldn't.
I think we should refactor so __pci_enable_device_flags() calls
pci_select_bars(). If it's feasible to move the IOV resources out of
the struct pci_dev, that would solve both problems. Otherwise, maybe
just put your #ifdef in pci_select_bars().
I'm assuming that this is post-3.1 material, right?
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
