Re: [PATCH v2] PCI: Add quirk for HiSilicon NP 5896 devices

[Bjorn Helgaas]

On Wed, Dec 18, 2019 at 05:16:03PM +0800, Xiongfeng Wang wrote:
> On 2019/12/11 12:10, Bjorn Helgaas wrote:
> > On Tue, Dec 10, 2019 at 9:28 PM Xiongfeng Wang
> > <wangxiongfeng2@xxxxxxxxxx> wrote:
> >> On 2019/12/7 2:10, Bjorn Helgaas wrote:
> >>> On Fri, Dec 06, 2019 at 03:01:45PM +0800, Xiongfeng Wang wrote:
> >>>> HiSilicon PCI Network Processor 5896 devices misreport the
> >>>> class type as 'NOT_DEFINED', but it is actually a network
> >>>> device. Also the size of BAR3 is reported as 265T, but this BAR
> >>>> is actually unused.  This patch modify the class type to
> >>>> 'CLASS_NETWORK' and disable the unused BAR3.

> >>> The question is not whether the BAR is used by the driver; the
> >>> question is whether the device responds to accesses to the
> >>> region described by the BAR when PCI_COMMAND_MEMORY is turned
> >>> on.
> >>
> >> I asked the hardware engineer. He said I can not write an address
> >> into that BAR.
> > 
> > If the BAR is not writable, I think sizing should fail, so I
> > suspect some of the bits are actually writable.
> 
> Sorry for the delayed response. It's not so convenient for me to get
> to the hardware guys.  BAR0 BAR1 BAR2 are 32-bit and can be used to
> access the registers and memory within 5896 devices. These three
> BARs can meet the need for most scenario.  BAR3 is 64-bit and can be
> used to access all the registers and memory within 5896 devices.
> (BAR3 is writable. Sorry for the non-confirmed information before.)
> But BAR3 is not used by the driver and the size is very
> largeïlarger than 100G, still didn't get the precise sizeï.  So I
> think maybe we can disable this BAR for now, otherwise the
> unassigned resource will cause 'pci_enable_device()' returning
> failure.

Here's the problem: the proposed patch (below) clears the struct
resource corresponding to BAR 3, but that doesn't actually disable the
BAR.  It hides the BAR from Linux, so Linux will pretend it doesn't
exist, but it's still there in the hardware.

The hardware BAR 3 still contains some value (possibly zero), and if
PCI_COMMAND_MEMORY is set (which you need to do if you want to use
*any* memory BARs on the device), the device will respond to any
transactions in the BAR 3 range.  Depending on the topology and all
the other BAR and window assignments, this may cause address
conflicts.

+ * HiSilicon NP 5896 devices BAR3 size is reported as 256T and causes problem
+ * when assigning the resources. But this BAR is actually unused by the driver,
+ * so let's disable it.
+ */
+static void quirk_hisi_fixup_np_bar(struct pci_dev *pdev)
+{
+       struct resource *r = &pdev->resource[3];
+
+       r->start = 0;
+       r->end = 0;
+       r->flags = 0;
+
+       pci_info(pdev, "Disabling invalid BAR 3\n");