Question about ARI + SR-VIO

[David Rubin <david@vortan.dev>]

Hello,

I had a question about ARI and SR-VIO interactions, and I hope this is the
right place to ask.

As I understand it, when ARI is disabled it is impossible to address a 
function
number larger than 7 (since the function takes up 3 bits in the ID).

Currently, when creating VF through SR-VIO and sysfs in an environment
where the first VFis placed at function number 8+ (such as when we have
a vf offset of 8, and a stride of 1), there is no check in place for 
ensuring
that ARI is enabled.

The kernel will go and ask the device to create this VF, and I believe the
devfn will just be garbage (although I haven't checked this). The device
comes back to the kernel with an "OK, everything is good", however when the
kernel then asks for the config on that VF, it gets complete garbage
and fails after trying to interpret the header type. Doing the
`early_dump_pci_device` just shows all 0xFF.

Example:
[   45.017386] ice 0000:01:00.0: Enabling 1 VFs
[   45.121403] pci 0000:01:01.0: config space:
[   45.121499] 00000000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[   45.121501] 00000010: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
... (cut for brevity)
    45.121508] 000000e0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[   45.121509] 000000f0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[   45.122040] pci 0000:01:01.0: [8086:1889] type 7f class 0xffffff 
conventional PCI
[   45.122049] pci 0000:01:01.0: unknown header type 7f, ignoring device
[   46.169410] ice 0000:01:00.0: Failed to enable SR-IOV: -5

I had a rough time debugging this issue, since I am not very familiar 
with how
PCI works in general, although I've been learning a lot recently. I finally
did figure out that enabling ARI solves it!

I would like to propose we add a check in `pci_enable_sriov`, that if the
"effective" vf function is larger than or equal to 8, we produce an "early"
error and a more helpful pci error message to dmesg. This may be a naive 
idea,
and I'd like to hear some feedback on it.

I would also love to understand the process of submitting patches to the
Linux kernel better and if possible write this patch myself.

Below is an explanatory patch on what I'm trying to propose. Please 
understand
that it is merely trying to demonstrate my idea; we would come up with 
better
wording and fix issues that I likely don't see.

Thank you and happy holidays,
David Rubin

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 77dee43b7..e9ff5e892 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -672,6 +672,13 @@ static int sriov_enable(struct pci_dev *dev, int 
nr_virtfn)
             (!(iov->cap & PCI_SRIOV_CAP_VFM) && (nr_virtfn > initial)))
                 return -EINVAL;

+       int effective_virtfn = dev->devfn + dev->sriov->offset +
+                                       dev->sriov->stride * (nr_virtfn 
- 1);
+       if (effective_virtfn >= 8 && !pci_ari_enabled(dev->bus)) {
+               pci_err(dev, "cannot enable %d VF without ARI enabled", 
nr_virtfn);
+               return -EINVAL;
+       }
+
         nres = 0;
         for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
                 int idx = pci_resource_num_from_vf_bar(i);