RE: [PATCH] PCI/IOV: Expose error return to dmesg

From: Liming Wu
Date: Tue Dec 13 2022 - 06:33:41 EST


HI,

Thanks for review it.

> -----Original Message-----
> From: Leon Romanovsky <leon@xxxxxxxxxx>
> Sent: Tuesday, December 13, 2022 5:02 PM
> To: Liming Wu <liming.wu@xxxxxxxxxxxxxxx>
> Cc: bhelgaas@xxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; alex.williamson@xxxxxxxxxx; 398776277@xxxxxx
> Subject: Re: [PATCH] PCI/IOV: Expose error return to dmesg
>
> On Tue, Dec 13, 2022 at 04:16:07PM +0800, Liming Wu wrote:
> > There are many errors returned during the initialization of sriov,
> > such as -EIO/-ENOMEM, but they are not exposed to dmesg.
> > Let's expose the real errors to the user.
>
> Please provide motivation. It is pretty easy to see what went wrong even
> without info print in dmesg.
The background is that we use our smat nic in the ARM64 architecture server
The following code in the sriov_init() function threw an exception

if (resource_size(res) & (PAGE_SIZE - 1)) {

The resource size obtained from smat nic is 4096(it's incorrectly set to a fixed value in nic).
But the PAGE_SIZE is 65536,
so sriov_init() exits, but the relevant exception information is not found in dmesg.

> >
> > In addition, -ENODEV doesn't make much sense and is not returned just
> > like any other capabilities.
> >
> > Signed-off-by: Liming Wu <liming.wu@xxxxxxxxxxxxxxx>
> > ---
> > drivers/pci/iov.c | 9 ++++++---
> > drivers/pci/pci.h | 2 --
> > drivers/pci/probe.c | 6 +++++-
> > 3 files changed, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c index
> > 952217572113..519aa2b48236 100644
> > --- a/drivers/pci/iov.c
> > +++ b/drivers/pci/iov.c
> > @@ -767,8 +767,11 @@ static int sriov_init(struct pci_dev *dev, int pos)
> > pci_write_config_word(dev, pos + PCI_SRIOV_CTRL, ctrl);
> >
> > pci_read_config_word(dev, pos + PCI_SRIOV_TOTAL_VF, &total);
> > - if (!total)
> > + if (!total) {
> > + pci_info(dev, "SR-IOV capability is enabled but has %d VFs)\n",
> > + total);
>
> total is always 0 in this print.
Spec describe PCI_SRIOV_TOTAL_VF reg (Total Virtual Functions) as below:
Indicates the maximum number of Virtual Functions (VFs) that can be associated
With the Physical Function (PF).
This values is HWInit in Single Root mode and must contain the same values as InitialVFs
In Multi-Root mode, the Multi-Root PCI Manager(MR-PCIM) can change this values.

I don't think total is always 0 in this print for it has been confirmed to have SR-IOV capability Enabled.

My arm64 Server dmesg as follow:
# dmesg -T |grep -B 1 -i total_vf
[Tue Dec 13 04:02:34 2022] pci 0000:07:00.0: reg 0x18: [mem 0x80001c00000-0x80001c00fff 64bit pref]
[Tue Dec 13 04:02:34 2022] drivers/pci/iov.c: 632 [info]: read PCI_SRIOV_TOTAL_VF 255
--
[Tue Dec 13 04:02:34 2022] pci 0000:08:00.0: reg 0x18: [mem 0x80001a00000-0x80001a00fff 64bit pref]
[Tue Dec 13 04:02:34 2022] drivers/pci/iov.c: 632 [info]: read PCI_SRIOV_TOTAL_VF 255
--
[Tue Dec 13 04:02:34 2022] pci 0000:20:00.0: reg 0x18: [mem 0x80000200000-0x80000200fff 64bit pref]
[Tue Dec 13 04:02:34 2022] drivers/pci/iov.c: 632 [info]: read PCI_SRIOV_TOTAL_VF 255
--
[Tue Dec 13 04:02:34 2022] pci 0000:21:00.0: reg 0x18: [mem 0x80000000000-0x80000000fff 64bit pref]
[Tue Dec 13 04:02:34 2022] drivers/pci/iov.c: 632 [info]: read PCI_SRIOV_TOTAL_VF 255
--
[Tue Dec 13 04:02:34 2022] pci 0000:7d:00.0: reg 0x18: [mem 0x120f00000-0x120ffffff 64bit pref]
[Tue Dec 13 04:02:34 2022] drivers/pci/iov.c: 632 [info]: read PCI_SRIOV_TOTAL_VF 3
--
[Tue Dec 13 04:02:34 2022] pci 0000:7d:00.1: reg 0x18: [mem 0x120b00000-0x120bfffff 64bit pref]
[Tue Dec 13 04:02:34 2022] drivers/pci/iov.c: 632 [info]: read PCI_SRIOV_TOTAL_VF 3
--
[Tue Dec 13 04:02:34 2022] pci 0000:7d:00.2: reg 0x18: [mem 0x120700000-0x1207fffff 64bit pref]
[Tue Dec 13 04:02:34 2022] drivers/pci/iov.c: 632 [info]: read PCI_SRIOV_TOTAL_VF 3
--
[Tue Dec 13 04:02:34 2022] pci 0000:7d:00.3: reg 0x18: [mem 0x120300000-0x1203fffff 64bit pref]
[Tue Dec 13 04:02:34 2022] drivers/pci/iov.c: 632 [info]: read PCI_SRIOV_TOTAL_VF 3
--
[Tue Dec 13 04:02:34 2022] pci 0000:83:00.0: PME# supported from D3cold
[Tue Dec 13 04:02:34 2022] drivers/pci/iov.c: 632 [info]: read PCI_SRIOV_TOTAL_VF 8
--
[Tue Dec 13 04:02:34 2022] pci 0000:83:00.1: PME# supported from D3cold
[Tue Dec 13 04:02:34 2022] drivers/pci/iov.c: 632 [info]: read PCI_SRIOV_TOTAL_VF 8
--
[Tue Dec 13 04:02:34 2022] pci 0000:dd:00.0: reg 0x18: [mem 0x400120000000-0x4001200fffff 64bit pref]
[Tue Dec 13 04:02:34 2022] drivers/pci/iov.c: 632 [info]: read PCI_SRIOV_TOTAL_VF 0

>
> > return 0;
> > + }
> >
> > pci_read_config_dword(dev, pos + PCI_SRIOV_SUP_PGSIZE, &pgsz);
> > i = PAGE_SHIFT > 12 ? PAGE_SHIFT - 12 : 0; @@ -899,13 +902,13 @@ int
> > pci_iov_init(struct pci_dev *dev)
> > int pos;
> >
> > if (!pci_is_pcie(dev))
> > - return -ENODEV;
> > + return;
>
> Please at least compile patches before you send them.
OK, thanks.
How about return 0, or any other suggestions.
>
> >
> > pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_SRIOV);
> > if (pos)
> > return sriov_init(dev, pos);
> >
> > - return -ENODEV;
> > + return;
> > }
> >
> > /**
> > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index
> > b1ebb7ab8805..c4836104f697 100644
> > --- a/drivers/pci/pci.h
> > +++ b/drivers/pci/pci.h
> > @@ -485,10 +485,8 @@ extern const struct attribute_group
> > sriov_vf_dev_attr_group; #else static inline int pci_iov_init(struct
> > pci_dev *dev) {
> > - return -ENODEV;
> > }
> > static inline void pci_iov_release(struct pci_dev *dev)
> > -
> > {
> > }
> > static inline void pci_iov_remove(struct pci_dev *dev) diff --git
> > a/drivers/pci/probe.c b/drivers/pci/probe.c index
> > b66fa42c4b1f..c951e0a50388 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -2449,6 +2449,7 @@ void pcie_report_downtraining(struct pci_dev
> > *dev)
> >
> > static void pci_init_capabilities(struct pci_dev *dev) {
> > + int ret;
> > pci_ea_init(dev); /* Enhanced Allocation */
> > pci_msi_init(dev); /* Disable MSI */
> > pci_msix_init(dev); /* Disable MSI-X */
> > @@ -2459,7 +2460,10 @@ static void pci_init_capabilities(struct pci_dev *dev)
> > pci_pm_init(dev); /* Power Management */
> > pci_vpd_init(dev); /* Vital Product Data */
> > pci_configure_ari(dev); /* Alternative Routing-ID Forwarding
> */
> > - pci_iov_init(dev); /* Single Root I/O Virtualization */
> > + ret = pci_iov_init(dev); /* Single Root I/O Virtualization */
> > + if (ret)
> > + pci_err(dev, "SR-IOV: init failed (%d)\n", ret);
> > +
> > pci_ats_init(dev); /* Address Translation Services */
> > pci_pri_init(dev); /* Page Request Interface */
> > pci_pasid_init(dev); /* Process Address Space ID */
> > --
> > 2.25.1
> >