Re: [PATCH v1] PCI: qcom: Avoid DBI and ATU register space mirror to BAR/MMIO region

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 6/22/2024 10:41 AM, Bjorn Helgaas wrote:
On Sat, Jun 22, 2024 at 09:24:44AM +0530, Manivannan Sadhasivam wrote:
On Thu, Jun 20, 2024 at 02:34:05PM -0700, Prudhvi Yarlagadda wrote:
PARF hardware block which is a wrapper on top of DWC PCIe controller
mirrors the DBI and ATU register space. It uses PARF_SLV_ADDR_SPACE_SIZE
register to get the size of the memory block to be mirrored and uses
PARF_DBI_BASE_ADDR, PARF_ATU_BASE_ADDR registers to determine the base
address of DBI and ATU space inside the memory block that is being
mirrored.

This PARF_SLV_ADDR_SPACE register is a mystery to me. I tried getting to the
bottom of it, but nobody could explain it to me clearly. Looks like you know
more about it...

 From your description, it seems like this register specifies the size of the
mirroring region (ATU + DBI), but the response from your colleague indicates
something different [1].

[1] https://lore.kernel.org/linux-pci/f42559f5-9d4c-4667-bf0e-7abfd9983c36@xxxxxxxxxxx/

When a memory region which is located above the SLV_ADDR_SPACE_SIZE
boundary is used for BAR region then there could be an overlap of DBI and
ATU address space that is getting mirrored and the BAR region. This
results in DBI and ATU address space contents getting updated when a PCIe
function driver tries updating the BAR/MMIO memory region. Reference
memory map of the PCIe memory region with DBI and ATU address space
overlapping BAR region is as below.

			|---------------|
			|		|
			|		|
	-------	--------|---------------|
	   |	   |	|---------------|
	   |	   |	|	DBI	|
	   |	   |	|---------------|---->DBI_BASE_ADDR
	   |	   |	|		|
	   |	   |	|		|
	   |	PCIe	|		|---->2*SLV_ADDR_SPACE_SIZE
	   |	BAR/MMIO|---------------|
	   |	Region	|	ATU	|
	   |	   |	|---------------|---->ATU_BASE_ADDR
	   |	   |	|		|
	PCIe	   |	|---------------|
	Memory	   |	|	DBI	|
	Region	   |	|---------------|---->DBI_BASE_ADDR
	   |	   |	|		|
	   |	--------|		|
	   |		|		|---->SLV_ADDR_SPACE_SIZE
	   |		|---------------|
	   |		|	ATU	|
	   |		|---------------|---->ATU_BASE_ADDR
	   |		|		|
	   |		|---------------|
	   |		|	DBI	|
	   |		|---------------|---->DBI_BASE_ADDR
	   |		|		|
	   |		|		|
	----------------|---------------|
			|		|
			|		|
			|		|
			|---------------|

Currently memory region beyond the SLV_ADDR_SPACE_SIZE boundary is
not used for BAR region which is why the above mentioned issue is
not encountered. This issue is discovered as part of internal
testing when we tried moving the BAR region beyond the
SLV_ADDR_SPACE_SIZE boundary. Hence we are trying to fix this.

I don't quite understand this. PoR value of SLV_ADDR_SPACE_SIZE is
16MB and most of the platforms have the size of whole PCIe region
defined in DT as 512MB (registers + I/O + MEM). So the range is
already crossing the SLV_ADDR_SPACE_SIZE boundary.

Ironically, the patch I pointed out above changes the value of this
register as 128MB, and the PCIe region size of that platform is also
128MB. The author of that patch pointed out that if the
SLV_ADDR_SPACE_SIZE is set to 256MB, then they are seeing
enumeration failures. If we go by your description of that register,
the SLV_ADDR_SPACE_SIZE of 256MB should be > PCIe region size of
128MB. So they should not see any issues, right?

As PARF hardware block mirrors DBI and ATU register space after
every PARF_SLV_ADDR_SPACE_SIZE (default 0x1000000) boundary
multiple, write U64_MAX to PARF_SLV_ADDR_SPACE_SIZE register to
avoid mirroring DBI and ATU to BAR/MMIO region.

Looks like you are trying to avoid this mirroring on a whole. First
of all, what is the reasoning behind this mirroring?

This sounds like what we usually call "aliasing" that happens when
some upper address bits are ignored.
Yes, here we are trying to disable altogether aliasing (mirroring of DBI/ATU) address space).

Regards,
Mayank




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux