On 2021/1/21 23:30, Bjorn Helgaas wrote:
[Alex is a reset expert, hoping he can chime in]
On Thu, Jan 21, 2021 at 08:53:12PM +0800, Chiqijun wrote:
On 2021/1/9 6:25, Bjorn Helgaas wrote:
On Fri, Dec 25, 2020 at 05:25:30PM +0800, Chiqijun wrote:
When multiple VFs do FLR at the same time, the firmware is
processed serially, resulting in some VF FLRs being delayed more
than 100ms, when the virtual machine restarts and the device
driver is loaded, the firmware is doing the corresponding VF
FLR, causing the driver to fail to load.
To solve this problem, add host and firmware status synchronization
during FLR.
Signed-off-by: Chiqijun <chiqi...@huawei.com>
...
+ * Get and check firmware capabilities.
+ */
+ val = readl(bar + HINIC_VF_FLR_TYPE);
+ if (!(val & (1UL << HINIC_VF_FLR_CAP_BIT_SHIFT))) {
+ pci_iounmap(pdev, bar);
+ return -ENOTTY;
+ }
+
+ /*
+ * Set the processing bit for the start of FLR, which will be cleared
+ * by the firmware after FLR is completed.
+ */
+ val = readl(bar + HINIC_VF_OP);
+ val = val | (1UL << HINIC_VF_FLR_PROC_BIT_SHIFT);
+ writel(val, bar + HINIC_VF_OP);
+
+ /* Perform the actual device function reset */
+ pcie_flr(pdev);
+
+ /*
+ * The device must learn BDF after FLR in order to respond to BAR's
+ * read request, therefore, we issue a configure write request to let
+ * the device capture BDF.
+ */
+ pci_read_config_word(pdev, PCI_COMMAND, &command);
+ pci_write_config_word(pdev, PCI_COMMAND, command);
I assume this is because of this requirement from PCIe r5.0, sec
2.2.9:
Functions must capture the Bus and Device Numbers supplied with all
Type 0 Configuration Write Requests completed by the Function, and
supply these numbers in the Bus and Device Number fields of the
Completer ID for all Completions generated by the Device/Function.
I'm a little concerned because it seems like this requirement should
apply to *all* resets, and I don't see where we do a similar write
following other resets. Can you help me out? Do we need this in
other cases? Do we do it?
This depends on the hardware device. The HINIC device clears the BDF
information of the VF during FLR, so it relies on Configuration
Write Requests to capture BDF. If other devices do not clear the DBF
information during FLR, this operation is not required.
If the spec says devices must keep the latched BDF during FLR, and the
HINIC doesn't comply with that, then it makes sense to do a config
write here in HINIC-specific code.
But if devices are allowed to clear the BDF during FLR, the OS has to
assume they all do, and the generic code for FLR (and probably other
resets) should do a config write so devices can latch the BDF again.
In addition, I did not find other devices directly access the BAR register
after FLR in resets.
I didn't catch your meaning here.
If a device loses the BDF during FLR and we don't do something to
allow it to latch the BDF again, any completions from the device will
have the wrong information. We will likely do *some* config write to
the device eventually, which will fix this, but we can't rely on some
unknown future write to do this. If it's a problem, we need to
explicitly do a write for this purpose.
Bjorn
.
The spec does not specify whether the BDF needs to be kept after FLR,
but the section 2.2.9 of the PCIe r5.0 has the following description:
If a Function must generate a Completion prior to the initial
device Configuration Write Request, 0's must be entered into the
Bus Number and Device Number fields
Does this mean that we should always get the expected completion
after initializing the device?
