Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
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
Thread-Topic: [PATCH] PCI : check if type 0 devices have all BARs of size zero
Thread-Index: AQHXAScblKz5CllRi0OQmPzb1ns9ZqpZvF8AgACyYbA=
Date:   Tue, 16 Feb 2021 07:52:08 +0000
Message-ID: <VE1PR04MB6702EE8C0FBAFDFD3B35199090879@VE1PR04MB6702.eurprd04.prod.outlook.com>
References: <20210212100856.473415-1-wasim.khan@oss.nxp.com>
 <20210215211300.GA748236@bjorn-Precision-5520>
In-Reply-To: <20210215211300.GA748236@bjorn-Precision-5520>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 3962e682-bdde-4cf6-b477-08d8d24fc321
X-MS-Exchange-CrossTenant-originalarrivaltime: 16 Feb 2021 07:52:08.9521
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: EakGL63COnK1NewKeXhbtEwPIDKno2EoTMSoOI0gQO7cagSGsdZaikBigDy6nZgaiKOuoN4Et5PSpUGdEsV+3w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VE1PR04MB7325
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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
>=20
> 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.
>=20
> 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 wer=
e
> missing, so I'm not sure what this would add.
>=20

Thank you for the review.
I observed this issue with an under development EP. Due to some logic probl=
em in EP's firmware, the BAR sizes were reported zero and crash was observe=
d sometime later in PCIe code.=20
I agree with you that such issues should have been caught in pre-silicon te=
sting, but not sure of pre-si testing details and if the issue was specific=
ally observed with real OS. Also, because the EP is in early stage of devel=
opment, device driver of EP is not available as of now.=20
So, I though it will be a good idea to print an information message only fo=
r *type 0* devices to give a quick hint if the zero BAR size is expected fo=
r 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, w=
hich
> means they would all be hardwired to zero.
>=20

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 u=
nknown
> to the PCI core.

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

>=20
> > 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 =3D 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 =3D &dev->resource[pos];
> >  		reg =3D PCI_BASE_ADDRESS_0 + (pos << 2);
> >  		pos +=3D __pci_read_base(dev, pci_bar_unknown, res, reg);
> > +		found |=3D 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 =3D &dev->resource[PCI_ROM_RESOURCE];
> >  		dev->rom_base_reg =3D rom;
> > --
> > 2.25.1
> >