Re: [PATCH V11 07/17] powerpc/pci: Don't unset pci resources for VFs

[Wei Yang <weiyang@linux.vnet.ibm.com>]

On Fri, Feb 20, 2015 at 05:47:09PM -0600, Bjorn Helgaas wrote:
>On Tue, Feb 10, 2015 at 07:14:45PM +1100, Benjamin Herrenschmidt wrote:
>> On Tue, 2015-02-10 at 14:25 +0800, Wei Yang wrote:
>> > PF's resource will be assigned first, including normal BARs and IOV
>> > BARs.
>> > 
>> > Then PF's driver will create VFs, in virtfn_add(). In this function,
>> > VF's
>> > resources is calculated from its PF's IOV BAR.
>> > 
>> > If you reset VF's resource as PFs, no one will try to assign it again.
>> 
>> So the problem is that the flag indicating VF is lost ? IE. We should
>> still mark them unset, but preserve that flag ?
>
>I think the problem is that the normal path for PCI_REASSIGN_ALL_RSRC is
>at boot-time, where we do this:
>
>    pcibios_init
>      pcibios_scan_phb
>        pci_scan_child_bus
>          ...
>            pci_device_add
>              pci_fixup_device(pci_fixup_header)
>		pcibios_fixup_resources                       # header fixup
>		  for (i = 0; i < DEVICE_COUNT_RESOURCE; i++)
>		    dev->resource[i].start = 0
>      pcibios_resource_survey
>        pcibios_allocate_resources
>
>and we assign dev->resource[] for everything in
>pcibios_allocate_resources().
>
>But VFs are enumerated later, when they are enabled by the PF driver after
>boot, so we have this path:
>
>    pci_enable_sriov
>      sriov_enable
>        virtfn_add(vf_id)
>          for (i = 0; i < 6; i++)
>            vf->resource[i].start = pf->resource[IOV + i].start + (size * vf_id)
>          pci_device_add
>            pci_fixup_device(pci_fixup_header)
>              pcibios_fixup_resources                   # header fixup
>                for (i = 0; i < DEVICE_COUNT_RESOURCE; i++)
>                  vf->resource[i].start = 0
>
>Here, we clear out vf->resource[0..5] in the header fixup, but we're not
>going to call pcibios_allocate_resources() again to reassign them.
>
>So I think the *intent* of PCI_REASSIGN_ALL_RSRC is preserved if
>pcibios_fixup_resources() leaves the VF resources alone, because the VF
>resources are completely determined by the PF resources, and the PF
>resources have already been reassigned.
>
>If my understanding is correct, I think the patch is reasonable, and I
>would try to put some of this explanation into the changelog.

Yep, it is correct, thanks for your explanation.

I did a chat on IRC with Ben, I guess he has got the idea :-)

>
>Bjorn

-- 
Richard Yang
Help you, Help me