LinuxLists.cc - [PATCH] PCI/IOV: Expose error return to dmesg

2022-12-13 08:41:31

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

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.

In addition, -ENODEV doesn't make much sense and is not returned
just like any other capabilities.

Signed-off-by: Liming Wu <[email protected]>
---
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);
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;

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

2022-12-13 09:14:39

by Leon Romanovsky

[permalink] [raw]

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.

>
> In addition, -ENODEV doesn't make much sense and is not returned
> just like any other capabilities.
>
> Signed-off-by: Liming Wu <[email protected]>
> ---
> 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.

> 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.

>
> 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
>

2022-12-13 11:51:36

by Liming Wu

[permalink] [raw]

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

HI,

Thanks for review it.

> -----Original Message-----
> From: Leon Romanovsky <[email protected]>
> Sent: Tuesday, December 13, 2022 5:02 PM
> To: Liming Wu <[email protected]>
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected]
> 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 <[email protected]>
> > ---
> > 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
> >

2022-12-13 12:37:28

by Leon Romanovsky

[permalink] [raw]

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

On Tue, Dec 13, 2022 at 11:33:27AM +0000, Liming Wu wrote:
> HI,
>
> Thanks for review it.
>
> > -----Original Message-----
> > From: Leon Romanovsky <[email protected]>
> > Sent: Tuesday, December 13, 2022 5:02 PM
> > To: Liming Wu <[email protected]>
> > Cc: [email protected]; [email protected]; linux-
> > [email protected]; [email protected]; [email protected]
> > 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.

It is not motivation, but summarizing HW bug found during device bringup.
Why should we (as users) see this print in upstream kernel?

>
> > >
> > > In addition, -ENODEV doesn't make much sense and is not returned just
> > > like any other capabilities.
> > >
> > > Signed-off-by: Liming Wu <[email protected]>
> > > ---
> > > 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.

You added print under if(!total) condition. The "SR-IOV capability is
enabled but has %d VFs" will always print "SR-IOV capability is enabled but has 0 VFs"

>
> 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.

Drop the patch and leave this code as is.

Thanks

2022-12-13 16:54:45

by kernel test robot

[permalink] [raw]

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

Attachments:

(No filename) (2.78 kB)
config (158.65 kB)
Download all attachments

2022-12-13 17:41:38

by kernel test robot

[permalink] [raw]

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

Hi Liming,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on helgaas-pci/next]
[also build test ERROR on helgaas-pci/for-linus linus/master v6.1 next-20221213]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Liming-Wu/PCI-IOV-Expose-error-return-to-dmesg/20221213-161738
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
patch link: https://lore.kernel.org/r/20221213081607.1641-1-liming.wu%40jaguarmicro.com
patch subject: [PATCH] PCI/IOV: Expose error return to dmesg
config: i386-randconfig-a013
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/c1d53738a3ff4c33e1c442c3b9ad02377be02eaa
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Liming-Wu/PCI-IOV-Expose-error-return-to-dmesg/20221213-161738
git checkout c1d53738a3ff4c33e1c442c3b9ad02377be02eaa
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

In file included from drivers/pci/access.c:8:
>> drivers/pci/pci.h:489:1: error: non-void function does not return a value [-Werror,-Wreturn-type]
}
^
1 error generated.
--
In file included from drivers/pci/msi/pcidev_msi.c:5:
>> drivers/pci/msi/../pci.h:489:1: error: non-void function does not return a value [-Werror,-Wreturn-type]
}
^
1 error generated.
--
In file included from drivers/pci/controller/pcie-xilinx.c:27:
>> drivers/pci/controller/../pci.h:489:1: error: non-void function does not return a value [-Werror,-Wreturn-type]
}
^
1 error generated.
--
In file included from drivers/pci/controller/dwc/pcie-designware.c:22:
>> drivers/pci/controller/dwc/../../pci.h:489:1: error: non-void function does not return a value [-Werror,-Wreturn-type]
}
^
1 error generated.

vim +489 drivers/pci/pci.h

751035b8dc061a Kuppuswamy Sathyanarayanan 2019-09-05 475
d1b054da8f5999 Yu Zhao 2009-03-20 476 #ifdef CONFIG_PCI_IOV
f39d5b72913e2a Bjorn Helgaas 2013-04-12 477 int pci_iov_init(struct pci_dev *dev);
f39d5b72913e2a Bjorn Helgaas 2013-04-12 478 void pci_iov_release(struct pci_dev *dev);
38972375ef7bdc Jakub Kicinski 2018-06-29 479 void pci_iov_remove(struct pci_dev *dev);
6ffa2489c51da7 Bjorn Helgaas 2016-11-28 480 void pci_iov_update_resource(struct pci_dev *dev, int resno);
f39d5b72913e2a Bjorn Helgaas 2013-04-12 481 resource_size_t pci_sriov_resource_alignment(struct pci_dev *dev, int resno);
f39d5b72913e2a Bjorn Helgaas 2013-04-12 482 void pci_restore_iov_state(struct pci_dev *dev);
f39d5b72913e2a Bjorn Helgaas 2013-04-12 483 int pci_iov_bus_range(struct pci_bus *bus);
c3d5c2d96d69f2 Leon Romanovsky 2021-04-04 484 extern const struct attribute_group sriov_pf_dev_attr_group;
c3d5c2d96d69f2 Leon Romanovsky 2021-04-04 485 extern const struct attribute_group sriov_vf_dev_attr_group;
d1b054da8f5999 Yu Zhao 2009-03-20 486 #else
d1b054da8f5999 Yu Zhao 2009-03-20 487 static inline int pci_iov_init(struct pci_dev *dev)
d1b054da8f5999 Yu Zhao 2009-03-20 488 {
d1b054da8f5999 Yu Zhao 2009-03-20 @489 }
d1b054da8f5999 Yu Zhao 2009-03-20 490 static inline void pci_iov_release(struct pci_dev *dev)
38972375ef7bdc Jakub Kicinski 2018-06-29 491 {
38972375ef7bdc Jakub Kicinski 2018-06-29 492 }
38972375ef7bdc Jakub Kicinski 2018-06-29 493 static inline void pci_iov_remove(struct pci_dev *dev)
d1b054da8f5999 Yu Zhao 2009-03-20 494 {
d1b054da8f5999 Yu Zhao 2009-03-20 495 }
8c5cdb6adc6688 Yu Zhao 2009-03-20 496 static inline void pci_restore_iov_state(struct pci_dev *dev)
8c5cdb6adc6688 Yu Zhao 2009-03-20 497 {
8c5cdb6adc6688 Yu Zhao 2009-03-20 498 }
a28724b0fb909d Yu Zhao 2009-03-20 499 static inline int pci_iov_bus_range(struct pci_bus *bus)
a28724b0fb909d Yu Zhao 2009-03-20 500 {
a28724b0fb909d Yu Zhao 2009-03-20 501 return 0;
a28724b0fb909d Yu Zhao 2009-03-20 502 }
302b4215daa0a7 Yu Zhao 2009-05-18 503

--
0-DAY CI Kernel Test Service
https://01.org/lkp

Attachments:

(No filename) (5.17 kB)
config (157.98 kB)
Download all attachments

2022-12-14 03:10:23

by Liming Wu

[permalink] [raw]

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

> -----Original Message-----
> From: Leon Romanovsky <[email protected]>
> Sent: Tuesday, December 13, 2022 7:43 PM
> To: Liming Wu <[email protected]>
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCH] PCI/IOV: Expose error return to dmesg
>
> On Tue, Dec 13, 2022 at 11:33:27AM +0000, Liming Wu wrote:
> > HI,
> >
> > Thanks for review it.
> >
> > > -----Original Message-----
> > > From: Leon Romanovsky <[email protected]>
> > > Sent: Tuesday, December 13, 2022 5:02 PM
> > > To: Liming Wu <[email protected]>
> > > Cc: [email protected]; [email protected]; linux-
> > > [email protected]; [email protected]; [email protected]
> > > 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.
>
> It is not motivation, but summarizing HW bug found during device bringup.
> Why should we (as users) see this print in upstream kernel?
Agreed.

> >
> > > >
> > > > In addition, -ENODEV doesn't make much sense and is not returned
> > > > just like any other capabilities.
> > > >
> > > > Signed-off-by: Liming Wu <[email protected]>
> > > > ---
> > > > 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.
>
> You added print under if(!total) condition. The "SR-IOV capability is enabled but
> has %d VFs" will always print "SR-IOV capability is enabled but has 0 VFs"
Yes?? I just want to show this to users. VFs of 0 is not normal.
A message is displayed indicating that the hardware Settings may be incorrect.

> >
> > 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]
> > --
> > [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.
>
> Drop the patch and leave this code as is.
OK, Thanks