linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Wasim Khan <wasim.khan@nxp.com>
To: Bjorn Helgaas <helgaas@kernel.org>,
	"Wasim Khan (OSS)" <wasim.khan@oss.nxp.com>
Cc: "bhelgaas@google.com" <bhelgaas@google.com>,
	"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] PCI : check if type 0 devices have all BARs of size zero
Date: Tue, 16 Feb 2021 07:52:08 +0000	[thread overview]
Message-ID: <VE1PR04MB6702EE8C0FBAFDFD3B35199090879@VE1PR04MB6702.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <20210215211300.GA748236@bjorn-Precision-5520>

Hi Bjorn,


> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: Tuesday, February 16, 2021 2:43 AM
> To: Wasim Khan (OSS) <wasim.khan@oss.nxp.com>
> Cc: bhelgaas@google.com; linux-pci@vger.kernel.org; linux-
> kernel@vger.kernel.org; Wasim Khan <wasim.khan@nxp.com>
> Subject: Re: [PATCH] PCI : check if type 0 devices have all BARs of size zero
> 
> On Fri, Feb 12, 2021 at 11:08:56AM +0100, Wasim Khan wrote:
> > From: Wasim Khan <wasim.khan@nxp.com>
> >
> > Log a message if all BARs of type 0 devices are of size zero. This can
> > help detecting type 0 devices not reporting BAR size correctly.
> 
> I could be missing something, but I don't think we can do this.  I would think the
> simplest possible presilicon testing would find errors like this, and the first
> attempt to have a driver claim the device would fail if required BARs were
> missing, so I'm not sure what this would add.
> 

Thank you for the review.
I observed this issue with an under development EP. Due to some logic problem in EP's firmware, the BAR sizes were reported zero and crash was observed sometime later in PCIe code. 
I agree with you that such issues should have been caught in pre-silicon testing, but not sure of pre-si testing details and if the issue was specifically observed with real OS. Also, because the EP is in early stage of development, device driver of EP is not available as of now. 
So, I though it will be a good idea to print an information message only for *type 0* devices to give a quick hint if the zero BAR size is expected for the given EP or not. So that SW can contribute to identify HW problem.

> While the subject line says "type 0 devices," this code path is also used for type
> 1 devices (bridges), and it's quite common for bridges to have no BARs, which
> means they would all be hardwired to zero.
> 

Yes, for type 1 devices, it is common to have zero BAR size, so I added log msg for type 0 devices only , which are in-general expected to have valid BARs.


> It is also legal for even type 0 devices to implement no BARs.  They may be
> operated entirely via config space or via device-specific BARs that are unknown
> to the PCI core.

OK, I did not know this . Thank you for sharing this.

> 
> > Signed-off-by: Wasim Khan <wasim.khan@nxp.com>
> > ---
> >  drivers/pci/probe.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index
> > 953f15abc850..6438d6d56777 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -321,6 +321,7 @@ int __pci_read_base(struct pci_dev *dev, enum
> > pci_bar_type type,  static void pci_read_bases(struct pci_dev *dev,
> > unsigned int howmany, int rom)  {
> >  	unsigned int pos, reg;
> > +	bool found = false;
> >
> >  	if (dev->non_compliant_bars)
> >  		return;
> > @@ -333,8 +334,12 @@ static void pci_read_bases(struct pci_dev *dev,
> unsigned int howmany, int rom)
> >  		struct resource *res = &dev->resource[pos];
> >  		reg = PCI_BASE_ADDRESS_0 + (pos << 2);
> >  		pos += __pci_read_base(dev, pci_bar_unknown, res, reg);
> > +		found |= res->flags ? 1 : 0;
> >  	}
> >
> > +	if (!dev->hdr_type && !found)
> > +		pci_info(dev, "BAR size is 0 for BAR[0..%d]\n", howmany - 1);
> > +
> >  	if (rom) {
> >  		struct resource *res = &dev->resource[PCI_ROM_RESOURCE];
> >  		dev->rom_base_reg = rom;
> > --
> > 2.25.1
> >

  reply	other threads:[~2021-02-16  7:53 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-12 10:08 Wasim Khan
2021-02-15 21:13 ` Bjorn Helgaas
2021-02-16  7:52   ` Wasim Khan [this message]
2021-02-16 14:16     ` Bjorn Helgaas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=VE1PR04MB6702EE8C0FBAFDFD3B35199090879@VE1PR04MB6702.eurprd04.prod.outlook.com \
    --to=wasim.khan@nxp.com \
    --cc=bhelgaas@google.com \
    --cc=helgaas@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=wasim.khan@oss.nxp.com \
    --subject='RE: [PATCH] PCI : check if type 0 devices have all BARs of size zero' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).