2020-08-14 09:29:06

by Zhiqiang Hou

[permalink] [raw]
Subject: [PATCH] PCI: designware-ep: Fix the Header Type check

From: Hou Zhiqiang <[email protected]>

The current check will result in the multiple function device
fails to initialize. So fix the check by masking out the
multiple function bit.

Fixes: 0b24134f7888 ("PCI: dwc: Add validation that PCIe core is set to correct mode")
Signed-off-by: Hou Zhiqiang <[email protected]>
---
drivers/pci/controller/dwc/pcie-designware-ep.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 4680a51c49c0..4b7abfb1e669 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -654,7 +654,7 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
int i;

hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE);
- if (hdr_type != PCI_HEADER_TYPE_NORMAL) {
+ if (hdr_type & 0x7f != PCI_HEADER_TYPE_NORMAL) {
dev_err(pci->dev,
"PCIe controller is not set to EP mode (hdr_type:0x%x)!\n",
hdr_type);
--
2.17.1


2020-08-14 16:22:12

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH] PCI: designware-ep: Fix the Header Type check

On Fri, Aug 14, 2020 at 2:15 AM Zhiqiang Hou <[email protected]> wrote:
>
> From: Hou Zhiqiang <[email protected]>
>
> The current check will result in the multiple function device
> fails to initialize. So fix the check by masking out the
> multiple function bit.
>
> Fixes: 0b24134f7888 ("PCI: dwc: Add validation that PCIe core is set to correct mode")
> Signed-off-by: Hou Zhiqiang <[email protected]>
> ---
> drivers/pci/controller/dwc/pcie-designware-ep.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 4680a51c49c0..4b7abfb1e669 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -654,7 +654,7 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
> int i;
>
> hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE);
> - if (hdr_type != PCI_HEADER_TYPE_NORMAL) {
> + if (hdr_type & 0x7f != PCI_HEADER_TYPE_NORMAL) {

Should have () around 'hdr_type & 0x7f'.

> dev_err(pci->dev,
> "PCIe controller is not set to EP mode (hdr_type:0x%x)!\n",
> hdr_type);

However, shouldn't the printed value be masked too? I'd just do:

hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE) & 0x7f;

Perhaps add a #define too. '0x7f' is used in several places.

Rob

2020-08-16 09:13:24

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] PCI: designware-ep: Fix the Header Type check

Hi Zhiqiang,

I love your patch! Perhaps something to improve:

[auto build test WARNING on pci/next]
[also build test WARNING on linux/master linus/master v5.8 next-20200814]
[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]

url: https://github.com/0day-ci/linux/commits/Zhiqiang-Hou/PCI-designware-ep-Fix-the-Header-Type-check/20200814-161623
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: alpha-allyesconfig (attached as .config)
compiler: alpha-linux-gcc (GCC) 9.3.0
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
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=alpha

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

All warnings (new ones prefixed by >>):

drivers/pci/controller/dwc/pcie-designware-ep.c: In function 'dw_pcie_ep_init_complete':
>> drivers/pci/controller/dwc/pcie-designware-ep.c:509:15: warning: suggest parentheses around comparison in operand of '&' [-Wparentheses]
509 | if (hdr_type & 0x7f != PCI_HEADER_TYPE_NORMAL) {
| ^

vim +509 drivers/pci/controller/dwc/pcie-designware-ep.c

498
499 int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
500 {
501 struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
502 unsigned int offset;
503 unsigned int nbars;
504 u8 hdr_type;
505 u32 reg;
506 int i;
507
508 hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE);
> 509 if (hdr_type & 0x7f != PCI_HEADER_TYPE_NORMAL) {
510 dev_err(pci->dev,
511 "PCIe controller is not set to EP mode (hdr_type:0x%x)!\n",
512 hdr_type);
513 return -EIO;
514 }
515
516 ep->msi_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
517
518 ep->msix_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSIX);
519
520 offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
521 if (offset) {
522 reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
523 nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >>
524 PCI_REBAR_CTRL_NBAR_SHIFT;
525
526 dw_pcie_dbi_ro_wr_en(pci);
527 for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
528 dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0);
529 dw_pcie_dbi_ro_wr_dis(pci);
530 }
531
532 dw_pcie_setup(pci);
533
534 return 0;
535 }
536 EXPORT_SYMBOL_GPL(dw_pcie_ep_init_complete);
537

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (2.89 kB)
.config.gz (63.51 kB)
Download all attachments

2020-08-16 16:39:34

by Zhiqiang Hou

[permalink] [raw]
Subject: RE: [PATCH] PCI: designware-ep: Fix the Header Type check

Hi Rob,

Thanks a lot for your comments!

> -----Original Message-----
> From: Rob Herring [mailto:[email protected]]
> Sent: 2020年8月14日 23:51
> To: Z.q. Hou <[email protected]>
> Cc: [email protected]; PCI <[email protected]>; Lorenzo
> Pieralisi <[email protected]>; Bjorn Helgaas
> <[email protected]>; Andrew Murray <[email protected]>;
> Jingoo Han <[email protected]>; Gustavo Pimentel
> <[email protected]>
> Subject: Re: [PATCH] PCI: designware-ep: Fix the Header Type check
>
> On Fri, Aug 14, 2020 at 2:15 AM Zhiqiang Hou <[email protected]>
> wrote:
> >
> > From: Hou Zhiqiang <[email protected]>
> >
> > The current check will result in the multiple function device fails to
> > initialize. So fix the check by masking out the multiple function bit.
> >
> > Fixes: 0b24134f7888 ("PCI: dwc: Add validation that PCIe core is set
> > to correct mode")
> > Signed-off-by: Hou Zhiqiang <[email protected]>
> > ---
> > drivers/pci/controller/dwc/pcie-designware-ep.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > index 4680a51c49c0..4b7abfb1e669 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > @@ -654,7 +654,7 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep
> *ep)
> > int i;
> >
> > hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE);
> > - if (hdr_type != PCI_HEADER_TYPE_NORMAL) {
> > + if (hdr_type & 0x7f != PCI_HEADER_TYPE_NORMAL) {
>
> Should have () around 'hdr_type & 0x7f'.
>
> > dev_err(pci->dev,
> > "PCIe controller is not set to EP mode
> (hdr_type:0x%x)!\n",
> > hdr_type);
>
> However, shouldn't the printed value be masked too? I'd just do:
>
> hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE) & 0x7f;
>
> Perhaps add a #define too. '0x7f' is used in several places.

All these are good suggestions, will make it in next version.

Thanks,
Zhiqiang

>
> Rob

2020-08-17 12:08:49

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] PCI: designware-ep: Fix the Header Type check

Hi Zhiqiang,

I love your patch! Perhaps something to improve:

[auto build test WARNING on pci/next]
[also build test WARNING on linux/master linus/master v5.9-rc1 next-20200817]
[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]

url: https://github.com/0day-ci/linux/commits/Zhiqiang-Hou/PCI-designware-ep-Fix-the-Header-Type-check/20200814-161623
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: x86_64-randconfig-r013-20200817 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project de71b46a519db014ce906a39f8a0e1b235ef1568)
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
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64

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

All warnings (new ones prefixed by >>):

>> drivers/pci/controller/dwc/pcie-designware-ep.c:509:15: warning: & has lower precedence than !=; != will be evaluated first [-Wparentheses]
if (hdr_type & 0x7f != PCI_HEADER_TYPE_NORMAL) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/pci/controller/dwc/pcie-designware-ep.c:509:15: note: place parentheses around the '!=' expression to silence this warning
if (hdr_type & 0x7f != PCI_HEADER_TYPE_NORMAL) {
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/pci/controller/dwc/pcie-designware-ep.c:509:15: note: place parentheses around the & expression to evaluate it first
if (hdr_type & 0x7f != PCI_HEADER_TYPE_NORMAL) {
^
( )
1 warning generated.

# https://github.com/0day-ci/linux/commit/819b7848e81d962a90377b2988abf5862ab35d01
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Zhiqiang-Hou/PCI-designware-ep-Fix-the-Header-Type-check/20200814-161623
git checkout 819b7848e81d962a90377b2988abf5862ab35d01
vim +509 drivers/pci/controller/dwc/pcie-designware-ep.c

498
499 int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
500 {
501 struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
502 unsigned int offset;
503 unsigned int nbars;
504 u8 hdr_type;
505 u32 reg;
506 int i;
507
508 hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE);
> 509 if (hdr_type & 0x7f != PCI_HEADER_TYPE_NORMAL) {
510 dev_err(pci->dev,
511 "PCIe controller is not set to EP mode (hdr_type:0x%x)!\n",
512 hdr_type);
513 return -EIO;
514 }
515
516 ep->msi_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
517
518 ep->msix_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSIX);
519
520 offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
521 if (offset) {
522 reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
523 nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >>
524 PCI_REBAR_CTRL_NBAR_SHIFT;
525
526 dw_pcie_dbi_ro_wr_en(pci);
527 for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
528 dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0);
529 dw_pcie_dbi_ro_wr_dis(pci);
530 }
531
532 dw_pcie_setup(pci);
533
534 return 0;
535 }
536 EXPORT_SYMBOL_GPL(dw_pcie_ep_init_complete);
537

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (3.85 kB)
.config.gz (32.59 kB)
Download all attachments