Re: [PATCH] PCI: hisi: fix the hisi_pcie_cfg_read bug when size=4

[Bjorn Helgaas <helgaas@kernel.org>]

On Thu, Dec 17, 2015 at 06:52:35PM +0800, Zhou Wang wrote:
> On 2015/12/10 4:36, Zhou Wang wrote:
> > On 2015/12/10 0:16, Bjorn Helgaas wrote:
> >> On Fri, Dec 04, 2015 at 04:43:34PM -0600, Bjorn Helgaas wrote:
> >>> On Fri, Nov 27, 2015 at 12:58:00PM +0800, Dongdong Liu wrote:
> >>>> Current hisi_pcie_cfg_read code read the RC configuration space,
> >>>> this has a problem that the output parameter "*val" have not been
> >>>> assigned when size=4, so we fix the bug by "*val = reg_val" when size=4.
> >>>>
> >>>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> >>>
> >>> Wow, that's a pretty egregious bug.  Was the hisi driver ever tested
> >>> at all?  We use dword reads in pci_bus_read_dev_vendor_id() to
> >>> enumerate devices, so if those didn't return valid data, nothing at
> >>> all should have worked.
> >>>
> >>> I tentatively put this on my for-linus branch for v4.4, but I wonder
> >>> whether it might be smarter to revert the driver completely until I
> >>> have more confidence in it.
> >>>
> >>> What sort of testing have you done on this driver?
> >>
> >> Since I haven't heard anything about whether this driver was ever
> >> tested, I dropped this patch from my for-linus branch and I reverted
> >> the entire hisi driver.
> >>
> >> Let me know when you have some testing results, and I'll take another
> >> look at it.
> >>
> >> Bjorn
> >>
> > 
> > Hi Bjorn,
> > 
> > Very sorry for late, I am on business trip at the moment so I have restricted
> > access to emails and my replies can be late.
> > 
> > The driver has been tested extensively based on internal version, but I did this
> > mistake when upstreaming :(. I will send a test log ASAP.
> >
> 
> Hi Bjorn,
> 
> I made a test based on Intel 82599 networking card, this is the bootup log which
> contains PCIe host and Intel 82599 networking card parts.

Thanks!  I updated my for-linus branch to remove the revert and add
the 32-bit config read fix.  I'll ask Linus to pull this before v4.4.

Bjorn