[+cc Alex, Leon, Jason]
On Wed, May 04, 2022 at 07:56:01PM +0000, Volodymyr Babchuk wrote:
>
> Hello,
>
> I have encountered issue when PCI code tries to use both fields in
>
> union {
> struct pci_sriov *sriov; /* PF: SR-IOV info */
> struct pci_dev *physfn; /* VF: related PF */
> };
>
> (which are part of struct pci_dev) at the same time.
>
> Symptoms are following:
>
> # echo 1 > /sys/bus/pci/devices/0000:01:00.0/sriov_numvfs
>
> pci 0000:01:00.2: reg 0x20c: [mem 0x30018000-0x3001ffff 64bit]
> pci 0000:01:00.2: VF(n) BAR0 space: [mem 0x30018000-0x30117fff 64bit] (contains BAR0 for 32 VFs)
> Unable to handle kernel paging request at virtual address 0001000200000010
> Mem abort info:
> ESR = 0x96000004
> EC = 0x25: DABT (current EL), IL = 32 bits
> SET = 0, FnV = 0
> EA = 0, S1PTW = 0
> Data abort info:
> ISV = 0, ISS = 0x00000004
> CM = 0, WnR = 0
> [0001000200000010] address between user and kernel address ranges
> Internal error: Oops: 96000004 [#1] PREEMPT SMP
> Modules linked in: xt_MASQUERADE iptable_nat nf_nat nf_conntrack nf_defrag_ipv6
> nf_defrag_ipv4 libcrc32c iptable_filter crct10dif_ce nvme nvme_core at24
> pci_endpoint_test bridge pdrv_genirq ip_tables x_tables ipv6
> CPU: 3 PID: 287 Comm: sh Not tainted 5.10.41-lorc+ #233
> Hardware name: XENVM-4.17 (DT)
> pstate: 60400005 (nZCv daif +PAN -UAO -TCO BTYPE=--)
> pc : pcie_aspm_get_link+0x90/0xcc
> lr : pcie_aspm_get_link+0x8c/0xcc
> sp : ffff8000130d39c0
> x29: ffff8000130d39c0 x28: 00000000000001a4
> x27: 00000000ffffee4b x26: ffff80001164f560
> x25: 0000000000000000 x24: 0000000000000000
> x23: ffff80001164f660 x22: 0000000000000000
> x23: ffff80001164f660 x22: 0000000000000000
> x21: ffff000003f08000 x20: ffff800010db37d8
> x19: ffff000004b8e780 x18: ffffffffffffffff
> x17: 0000000000000000 x16: 00000000deadbeef
> x15: ffff8000930d36c7 x14: 0000000000000006
> x13: ffff8000115c2710 x12: 000000000000081c
> x11: 00000000000002b4 x10: ffff8000115c2710
> x9 : ffff8000115c2710 x8 : 00000000ffffefff
> x7 : ffff80001161a710 x6 : ffff80001161a710
> x5 : ffff00003fdad900 x4 : 0000000000000000
> x3 : 0000000000000000 x2 : 0000000000000000
> x1 : ffff000003c51c80 x0 : 0001000200000000
> Call trace:
> pcie_aspm_get_link+0x90/0xcc
> aspm_ctrl_attrs_are_visible+0x30/0xc0
> internal_create_group+0xd0/0x3cc
> internal_create_groups.part.0+0x4c/0xc0
> sysfs_create_groups+0x20/0x34
> device_add+0x2b4/0x760
> pci_device_add+0x814/0x854
> pci_iov_add_virtfn+0x240/0x2f0
> sriov_enable+0x1f8/0x474
> pci_sriov_configure_simple+0x38/0x90
> sriov_numvfs_store+0xa4/0x1a0
> dev_attr_store+0x1c/0x30
> sysfs_kf_write+0x48/0x60
> kernfs_fop_write_iter+0x118/0x1ac
> new_sync_write+0xe8/0x184
> vfs_write+0x23c/0x2a0
> ksys_write+0x68/0xf4
> __arm64_sys_write+0x20/0x2c
> el0_svc_common.constprop.0+0x78/0x1a0
> do_el0_svc+0x28/0x94
> el0_svc+0x14/0x20
> el0_sync_handler+0xa4/0x130
> el0_sync+0x180/0x1c0
> Code: d0002120 9133e000 97ffef8e f9400a60 (f9400813)
>
>
> Debugging showed the following:
>
> pci_iov_add_virtfn() allocates new struct pci_dev:
>
> virtfn = pci_alloc_dev(bus);
> and sets physfn:
> virtfn->is_virtfn = 1;
> virtfn->physfn = pci_dev_get(dev);
>
> then we will get into sriov_init() via the following call path:
>
> pci_device_add(virtfn, virtfn->bus);
> pci_init_capabilities(dev);
> pci_iov_init(dev);
> sriov_init(dev, pos);
We called pci_device_add() with the VF. pci_iov_init() only calls
sriov_init() if it finds an SR-IOV capability on the device:
pci_iov_init(struct pci_dev *dev)
pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_SRIOV);
if (pos)
return sriov_init(dev, pos);
So this means the VF must have an SR-IOV capability, which sounds a
little dubious. From PCIe r6.0:
9.3.3 SR-IOV Extended Capability
The SR-IOV Extended Capability defined here is a PCIe extended
capability that must be implemented in each PF that supports SR-IOV.
This Capability is used to describe and control a PF’s SR-IOV
Capabilities.
For Multi-Function Devices, each PF that supports SR-IOV shall
provide the Capability structure defined in this section. This
Capability structure may be present in any Function with a Type 0
Configuration Space Header. This Capability must not be present in
Functions with a Type 1 Configuration Space Header.
Can you supply the output of "sudo lspci -vv" for your system?
It could be that the device has an SR-IOV capability when it
shouldn't. But even if it does, Linux could tolerate that better
than it does today.
> sriov_init() overwrites value in the union:
> dev->sriov = iov; <<<<<---- There
> dev->is_physfn = 1;
>
> Next, we will get into function that causes panic:
>
> pci_device_add(virtfn, virtfn->bus);
> device_add(&dev->dev)
> sysfs_create_groups()
> internal_create_group()
> aspm_ctrl_attrs_are_visible()
> pcie_aspm_get_link()
> pci_upstream_bridge()
> pci_physfn()
>
> which is
>
> static inline struct pci_dev *pci_physfn(struct pci_dev *dev)
> {
> #ifdef CONFIG_PCI_IOV
> if (dev->is_virtfn)
> dev = dev->physfn;
> #endif
> return dev;
> }
>
>
> as is_virtfn == 1 and dev->physfn was overwritten via write to
> dev->sriov, pci_physfn() will return pointer to struct pci_sriov
> allocated by sriov_init(). And then pci_upstream_bridge() will
> cause panic by acessing it as if it were pointer to struct pci_dev
>
> I encountered this issue on ARM64, Linux 5.10.41. Tried to test
> on master branch, but it is quite difficult to rebase platform
> code on the master. But I compared all relevant parts of PCI code
> and didn't found any differences.
>
> Looks like I am missing following, because how SR-IOV can be so broken?
> But what exactly?
>
> --
> Volodymyr Babchuk at EPAM
