Re: [PATCH] PCI: Init NumVFs register to zero in sriov_init()

[Ethan Zhao]

On Fri, Nov 15, 2013 at 2:59 PM, Yinghai Lu <yinghai@xxxxxxxxxx> wrote:
> On Wed, Nov 13, 2013 at 5:57 PM, Ethan Zhao <ethan.kernel@xxxxxxxxx> wrote:
>> On Wed, Nov 6, 2013 at 10:49 PM, ethan.zhao <ethan.kernel@xxxxxxxxx> wrote:
>>> Though no specification about NumVFs register initial value after POST, to void the confusion
>>> lspci output as following before VF was enabled, we should clear the NumVFs value left by BIOS
>>> to zero:
>
> Does BIOS need to clear it?

No, don't think BIOS need to clear it, according to the SR-IOV and PCI
specification, BIOS hasn't the responsibility to clear NumVFs register
 or not.  "The initial value of NumVFs is undefined." (SINGLE ROOT I/O
VIRTUALIZATION AND SHARING SPECIFICATION, REV. 1.1 chapter 3.3.7 ).
In fact, Linux doesn't trust the PCI_SRIOV_XX registers initial value
left by BIOS, exactly, it did in iov.c , Linux will re-initialize the
PCI_SRIOV_XX registers' value whatever their original value left by
BIOS.

static int sriov_init(struct pci_dev *dev, int pos)
{
  ... ...
  pci_read_config_word(dev, pos + PCI_SRIOV_CTRL, &ctrl);
  if (ctrl & PCI_SRIOV_CTRL_VFE) {
  pci_write_config_word(dev, pos + PCI_SRIOV_CTRL, 0);
  ssleep(1);
  }

... ...
ctrl = 0;
... ...
found:
pci_write_config_word(dev, pos + PCI_SRIOV_CTRL, ctrl);
... ...
}

If so, why leave  value of NumVFs register un-initialized, till be
exposed to user via lspci etc.

>
>
>>>
>>> $lspci -vvv -s 03:00.0
>>> Ethernet controller: Intel Corporation 82599EB 10-Gigabit SFI/SFP+ Network Connection (rev 01)
>>> ~
>>> Capabilities: [160 v1] Single Root I/O Virtualization (SR-IOV)
>>>                 IOVCap: Migration-, Interrupt Message Number: 000
>>>                 IOVCtl: Enable+ Migration- Interrupt- MSE+ ARIHierarchy+
>>>                 IOVSta: Migration-
>>>                 Initial VFs: 64, Total VFs: 64, Number of VFs: 64, Function Dependency Link: 00
>>>                                                                            ^dazed !
>>> ~
>
> just display problem?

So far, only catch the issue via lspci, "seems' only confuses user,
the VFs are enabled to 64. in fact, they are disabled. but it is not
bug of lspci, binary dump of lspci could prove it.


Thanks,
Ethan
>
>>> Signed-off-by: ethan.zhao <ethan.kernel@xxxxxxxxx>
>>> ---
>>>  drivers/pci/iov.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
>>> index de8ffac..a4941ad 100644
>>> --- a/drivers/pci/iov.c
>>> +++ b/drivers/pci/iov.c
>>> @@ -439,6 +439,8 @@ static int sriov_init(struct pci_dev *dev, int pos)
>>>
>>>  found:
>>>         pci_write_config_word(dev, pos + PCI_SRIOV_CTRL, ctrl);
>>> +       /* VF Enable is cleared, so we could init the NumVFs register to 0 */
>>> +       pci_write_config_word(dev, pos + PCI_SRIOV_NUM_VF, 0);
>>>         pci_read_config_word(dev, pos + PCI_SRIOV_VF_OFFSET, &offset);
>>>         pci_read_config_word(dev, pos + PCI_SRIOV_VF_STRIDE, &stride);
>>>         if (!offset || (total > 1 && !stride))
>
> Thanks
>
> Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/