Re: [PATCH v2 5/9] pci: pcie_dw_rockchip: Hide BARs of the root complex

2023-07-13 Thread Jonas Karlman 
Hi Kever,
On 2023-06-29 12:25, Kever Yang wrote:
> Hi Jonas,
> 
>      Sorry for reply late for this patch.
> 
>      I have check internally again for this BAR issue, the BAR alloce 
> fail for RC would not affect other process, and it always works on 
> vendor U-Boot tree.

The issue is not that the RC BAR allocation fails and that it affect
other PCI devices BAR allocation after that, the issue is that with
updated ranges the RC BAR allocation is successful and the entire memory
region gets allocated and that in turn affect other PCI devices from a
successful probe.

There are multiple parts in play here:

1. Two BARs of 1 GiB each is reported for RC on RK3568, 0xC000 is read
   back instead of 0x000.
   As seen by "PCI Autoconfig: BAR X, Mem, size=0x4000" below.

2. With the ranges from "rockchip: rk356x: Update PCIe config, IO and
   memory regions", now also merged in linux, the first BAR for RC does
   allocate the entire memory region.
   As seen by "With a memory region of the entire 1 GiB this leads to".

   PCI Autoconfig: BAR 0, Mem, size=0x4000, address=0x4000 
bus_lower=0x8000

3. Any other BAR processed will fail with "No room in resource" and the
   PCI autoconfig fails and no device is discovered.
   As seen by "With a memory region of the entire 1 GiB this leads to".

   PCI Autoconfig: BAR 0, Mem64, size=0x4000, No room in resource, avail 
start=8000 / size=4000, need=4000
   PCI: Failed autoconfig bar 10

On downstream vendor U-Boot this is not an issue because the start
address is 0x0 instead of 0x4000. U-Boot skips first 0x1000 of the
range when the start address is 0x0, meaning the available range is less
then 1 GiB, and the BAR allocation for RC fails.

As seen by "With a memory region less than 1 GiB this was not a real
issue" this also happen until memory ranges are adjusted in device tree.

  PCI Autoconfig: BAR 0, Mem, size=0x4000, No room in resource, avail 
start=1000 / size=3ef0, need=4000

The issue with RC allocating the entire 1 GiB region would probably
affect vendor U-Boot if it would use 0x4000 as start address instead
of 0x0.

There has been reports on linux ML that the use of 0x0 as start address
caused issues for some PCI devices and that the use of 0x4000 as
start address fixed such issue.

> 
>      And if we have to handle this, we can disable the BAR instead of 
> this workaround.

Please provide details on how we can configure the HW to not report any
BARs when in RC mode.

Using a very similar approach that other pci drivers have used, e.g. the
Intel FPGA PCIe host controller driver, is the only way that I found
that worked consistently.
See 
https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/pci/pcie_intel_fpga.c#L87-103

Regards,
Jonas

> 
> 
> Thanks,
> 
> - Kever
> 
> On 2023/5/18 06:53, Jonas Karlman wrote:
>> PCI Autoconfig read the Root Complex BARs and try to claim the entire
>> 1 GiB memory region on RK3568, leaving no space for any attached device.
>>
>> With a memory region less than 1 GiB this was not a real issue:
>>
>>PCI Autoconfig: Bus Memory region: [0-3eef],
>>PCI Autoconfig: Bus I/O region: [3ef0-3eff],
>>PCI Autoconfig: Found P2P bridge, device 0
>>PCI Autoconfig: BAR 0, Mem, size=0x4000, No room in resource, avail 
>> start=1000 / size=3ef0, need=4000
>>PCI: Failed autoconfig bar 10
>>PCI Autoconfig: BAR 1, Mem, size=0x4000, No room in resource, avail 
>> start=1000 / size=3ef0, need=4000
>>PCI: Failed autoconfig bar 14
>>PCI Autoconfig: ROM, size=0x1, address=0x1 bus_lower=0x2
>>
>>PCI Autoconfig: BAR 0, Mem64, size=0x4000, address=0x10 
>> bus_lower=0x104000
>>
>> With a memory region of the entire 1 GiB this leads to:
>>
>>PCI Autoconfig: Bus Memory region: [4000-7fff],
>>PCI Autoconfig: Bus I/O region: [f010-f01f],
>>PCI Autoconfig: Found P2P bridge, device 0
>>PCI Autoconfig: BAR 0, Mem, size=0x4000, address=0x4000 
>> bus_lower=0x8000
>>PCI Autoconfig: BAR 1, Mem, size=0x4000, No room in resource, avail 
>> start=8000 / size=4000, need=4000
>>PCI: Failed autoconfig bar 14
>>PCI Autoconfig: ROM, size=0x1, No room in resource, avail 
>> start=8000 / size=4000, need=1
>>
>>PCI Autoconfig: BAR 0, Mem64, size=0x4000, No room in resource, avail 
>> start=8000 / size=4000, need=4000
>>PCI: Failed autoconfig bar 10
>>
>> After this change with a memory region of the entire 1 GiB:
>>
>>PCI Autoconfig: Bus Memory region: [4000-7fff],
>>PCI Autoconfig: Bus I/O region: [f010-f01f],
>>PCI Autoconfig: Found P2P bridge, device 0
>>PCI Autoconfig: ROM, size=0x1, address=0x4000 bus_lower=0x4001
>>
>>PCI Autoconfig: BAR 0, Mem64, size=0x4000, address=0x4010 
>> bus_lower=0x40104000
>>
>> Return an invalid value during config read of Root Complex

Re: [PATCH v2 5/9] pci: pcie_dw_rockchip: Hide BARs of the root complex

2023-06-29 Thread Kever Yang 

Hi Jonas,

    Sorry for reply late for this patch.

    I have check internally again for this BAR issue, the BAR alloce 
fail for RC would not affect other process, and it always works on 
vendor U-Boot tree.


    And if we have to handle this, we can disable the BAR instead of 
this workaround.



Thanks,

- Kever

On 2023/5/18 06:53, Jonas Karlman wrote:

PCI Autoconfig read the Root Complex BARs and try to claim the entire
1 GiB memory region on RK3568, leaving no space for any attached device.

With a memory region less than 1 GiB this was not a real issue:

   PCI Autoconfig: Bus Memory region: [0-3eef],
   PCI Autoconfig: Bus I/O region: [3ef0-3eff],
   PCI Autoconfig: Found P2P bridge, device 0
   PCI Autoconfig: BAR 0, Mem, size=0x4000, No room in resource, avail 
start=1000 / size=3ef0, need=4000
   PCI: Failed autoconfig bar 10
   PCI Autoconfig: BAR 1, Mem, size=0x4000, No room in resource, avail 
start=1000 / size=3ef0, need=4000
   PCI: Failed autoconfig bar 14
   PCI Autoconfig: ROM, size=0x1, address=0x1 bus_lower=0x2

   PCI Autoconfig: BAR 0, Mem64, size=0x4000, address=0x10 
bus_lower=0x104000

With a memory region of the entire 1 GiB this leads to:

   PCI Autoconfig: Bus Memory region: [4000-7fff],
   PCI Autoconfig: Bus I/O region: [f010-f01f],
   PCI Autoconfig: Found P2P bridge, device 0
   PCI Autoconfig: BAR 0, Mem, size=0x4000, address=0x4000 
bus_lower=0x8000
   PCI Autoconfig: BAR 1, Mem, size=0x4000, No room in resource, avail 
start=8000 / size=4000, need=4000
   PCI: Failed autoconfig bar 14
   PCI Autoconfig: ROM, size=0x1, No room in resource, avail start=8000 
/ size=4000, need=1

   PCI Autoconfig: BAR 0, Mem64, size=0x4000, No room in resource, avail 
start=8000 / size=4000, need=4000
   PCI: Failed autoconfig bar 10

After this change with a memory region of the entire 1 GiB:

   PCI Autoconfig: Bus Memory region: [4000-7fff],
   PCI Autoconfig: Bus I/O region: [f010-f01f],
   PCI Autoconfig: Found P2P bridge, device 0
   PCI Autoconfig: ROM, size=0x1, address=0x4000 bus_lower=0x4001

   PCI Autoconfig: BAR 0, Mem64, size=0x4000, address=0x4010 
bus_lower=0x40104000

Return an invalid value during config read of Root Complex BARs during
autoconfig to work around such issue.

Signed-off-by: Jonas Karlman 
---
v2:
- Update commit message

  drivers/pci/pcie_dw_rockchip.c | 28 +++-
  1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pcie_dw_rockchip.c b/drivers/pci/pcie_dw_rockchip.c
index 82a8b9c96e2b..f56773c2e58c 100644
--- a/drivers/pci/pcie_dw_rockchip.c
+++ b/drivers/pci/pcie_dw_rockchip.c
@@ -146,6 +146,32 @@ static inline void rk_pcie_writel_apb(struct rk_pcie 
*rk_pcie, u32 reg,
__rk_pcie_write_apb(rk_pcie, rk_pcie->apb_base, reg, 0x4, val);
  }
  
+/**

+ * The BARs of bridge should be hidden during enumeration to avoid
+ * allocation of the entire memory region by PCIe core on RK3568.
+ */
+static bool rk_pcie_hide_rc_bar(struct pcie_dw *pcie, pci_dev_t bdf,
+   uint offset)
+{
+   int bus = PCI_BUS(bdf) - pcie->first_busno;
+
+   return bus == 0 && PCI_DEV(bdf) == 0 && PCI_FUNC(bdf) == 0 &&
+  offset >= PCI_BASE_ADDRESS_0 && offset <= PCI_BASE_ADDRESS_1;
+}
+
+static int rk_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
+  uint offset, ulong *valuep,
+  enum pci_size_t size)
+{
+   struct pcie_dw *pcie = dev_get_priv(bus);
+   int ret = pcie_dw_read_config(bus, bdf, offset, valuep, size);
+
+   if (!ret && rk_pcie_hide_rc_bar(pcie, bdf, offset))
+   *valuep = pci_get_ff(size);
+
+   return ret;
+}
+
  /**
   * rk_pcie_configure() - Configure link capabilities and speed
   *
@@ -476,7 +502,7 @@ rockchip_pcie_probe_err_init_port:
  }
  
  static const struct dm_pci_ops rockchip_pcie_ops = {

-   .read_config= pcie_dw_read_config,
+   .read_config= rk_pcie_read_config,
.write_config   = pcie_dw_write_config,
  };

[PATCH v2 5/9] pci: pcie_dw_rockchip: Hide BARs of the root complex

2023-05-17 Thread Jonas Karlman 
PCI Autoconfig read the Root Complex BARs and try to claim the entire
1 GiB memory region on RK3568, leaving no space for any attached device.

With a memory region less than 1 GiB this was not a real issue:

  PCI Autoconfig: Bus Memory region: [0-3eef],
  PCI Autoconfig: Bus I/O region: [3ef0-3eff],
  PCI Autoconfig: Found P2P bridge, device 0
  PCI Autoconfig: BAR 0, Mem, size=0x4000, No room in resource, avail 
start=1000 / size=3ef0, need=4000
  PCI: Failed autoconfig bar 10
  PCI Autoconfig: BAR 1, Mem, size=0x4000, No room in resource, avail 
start=1000 / size=3ef0, need=4000
  PCI: Failed autoconfig bar 14
  PCI Autoconfig: ROM, size=0x1, address=0x1 bus_lower=0x2

  PCI Autoconfig: BAR 0, Mem64, size=0x4000, address=0x10 bus_lower=0x104000

With a memory region of the entire 1 GiB this leads to:

  PCI Autoconfig: Bus Memory region: [4000-7fff],
  PCI Autoconfig: Bus I/O region: [f010-f01f],
  PCI Autoconfig: Found P2P bridge, device 0
  PCI Autoconfig: BAR 0, Mem, size=0x4000, address=0x4000 
bus_lower=0x8000
  PCI Autoconfig: BAR 1, Mem, size=0x4000, No room in resource, avail 
start=8000 / size=4000, need=4000
  PCI: Failed autoconfig bar 14
  PCI Autoconfig: ROM, size=0x1, No room in resource, avail start=8000 
/ size=4000, need=1

  PCI Autoconfig: BAR 0, Mem64, size=0x4000, No room in resource, avail 
start=8000 / size=4000, need=4000
  PCI: Failed autoconfig bar 10

After this change with a memory region of the entire 1 GiB:

  PCI Autoconfig: Bus Memory region: [4000-7fff],
  PCI Autoconfig: Bus I/O region: [f010-f01f],
  PCI Autoconfig: Found P2P bridge, device 0
  PCI Autoconfig: ROM, size=0x1, address=0x4000 bus_lower=0x4001

  PCI Autoconfig: BAR 0, Mem64, size=0x4000, address=0x4010 
bus_lower=0x40104000

Return an invalid value during config read of Root Complex BARs during
autoconfig to work around such issue.

Signed-off-by: Jonas Karlman 
---
v2:
- Update commit message

 drivers/pci/pcie_dw_rockchip.c | 28 +++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pcie_dw_rockchip.c b/drivers/pci/pcie_dw_rockchip.c
index 82a8b9c96e2b..f56773c2e58c 100644
--- a/drivers/pci/pcie_dw_rockchip.c
+++ b/drivers/pci/pcie_dw_rockchip.c
@@ -146,6 +146,32 @@ static inline void rk_pcie_writel_apb(struct rk_pcie 
*rk_pcie, u32 reg,
__rk_pcie_write_apb(rk_pcie, rk_pcie->apb_base, reg, 0x4, val);
 }
 
+/**
+ * The BARs of bridge should be hidden during enumeration to avoid
+ * allocation of the entire memory region by PCIe core on RK3568.
+ */
+static bool rk_pcie_hide_rc_bar(struct pcie_dw *pcie, pci_dev_t bdf,
+   uint offset)
+{
+   int bus = PCI_BUS(bdf) - pcie->first_busno;
+
+   return bus == 0 && PCI_DEV(bdf) == 0 && PCI_FUNC(bdf) == 0 &&
+  offset >= PCI_BASE_ADDRESS_0 && offset <= PCI_BASE_ADDRESS_1;
+}
+
+static int rk_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
+  uint offset, ulong *valuep,
+  enum pci_size_t size)
+{
+   struct pcie_dw *pcie = dev_get_priv(bus);
+   int ret = pcie_dw_read_config(bus, bdf, offset, valuep, size);
+
+   if (!ret && rk_pcie_hide_rc_bar(pcie, bdf, offset))
+   *valuep = pci_get_ff(size);
+
+   return ret;
+}
+
 /**
  * rk_pcie_configure() - Configure link capabilities and speed
  *
@@ -476,7 +502,7 @@ rockchip_pcie_probe_err_init_port:
 }
 
 static const struct dm_pci_ops rockchip_pcie_ops = {
-   .read_config= pcie_dw_read_config,
+   .read_config= rk_pcie_read_config,
.write_config   = pcie_dw_write_config,
 };
 
-- 
2.40.1