On Tue, May 19, 2020 at 10:08:54PM +0000, Gustavo Pimentel wrote:
On Tue, May 19, 2020 at 15:58:16, Lorenzo Pieralisi
<lorenzo.pieralisi@xxxxxxx> wrote:
On Tue, May 19, 2020 at 07:25:02PM +0530, Vidya Sagar wrote:
On 18-May-20 9:24 PM, Lorenzo Pieralisi wrote:
External email: Use caution opening links or attachments
On Wed, May 13, 2020 at 05:35:08PM -0500, Bjorn Helgaas wrote:
[+cc Alan; please cc authors of relevant commits,
updated Andrew's email address]
On Thu, May 14, 2020 at 12:38:55AM +0530, Vidya Sagar wrote:
commit 9e73fa02aa009 ("PCI: dwc: Warn if MEM resource size
exceeds max for
32-bits") enables warning for MEM resources of size >4GB
but prefetchable
memory resources also come under this category where
sizes can go beyond
4GB. Avoid logging a warning for prefetchable memory
resources.
Signed-off-by: Vidya Sagar <vidyas@xxxxxxxxxx>
---
drivers/pci/controller/dwc/pcie-designware-host.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git
a/drivers/pci/controller/dwc/pcie-designware-host.c
b/drivers/pci/controller/dwc/pcie-designware-host.c
index 42fbfe2a1b8f..a29396529ea4 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -366,7 +366,8 @@ int dw_pcie_host_init(struct pcie_port
*pp)
pp->mem = win->res;
pp->mem->name = "MEM";
mem_size = resource_size(pp->mem);
- if (upper_32_bits(mem_size))
+ if (upper_32_bits(mem_size) &&
+ !(win->res->flags &
IORESOURCE_PREFETCH))
dev_warn(dev, "MEM resource
size exceeds max for 32 bits\n");
pp->mem_size = mem_size;
pp->mem_bus_addr = pp->mem->start -
win->offset;
That warning was added for a reason - why should not we log
legitimate
warnings ? AFAIU having resources larger than 4GB can lead to
undefined
behaviour given the current ATU programming API.
Yeah. I'm all for a warning if the size is larger than 4GB in
case of
non-prefetchable window as one of the ATU outbound translation
channels is being used,
Is it true for all DWC host controllers ? Or there may be another
exception whereby we would be forced to disable this warning
altogether
?
but, we are not employing any ATU outbound translation channel
for
What does this mean ? "we are not employing any ATU
outbound...", is
this the tegra driver ? And what guarantees that this warning
is not
legitimate on DWC host controllers that do use the ATU outbound
translation for prefetchable windows ?
Can DWC maintainers chime in and clarify please ?
Before this code section, there is the following function call
pci_parse_request_of_pci_ranges(), which performs a simple
validation for
the IORESOURCE_MEM resource type.
This validation checks if the resource is marked as
prefetchable, if so,
an error message "non-prefetchable memory resource required" is
given and
a return code with the -EINVAL value.
That's not what the code is doing.
pci_parse_request_of_pci_range() will
traverse over the whole list of resources that it can find for
the given
host controller and checks whether one of the resources defines
prefetch
memory (note the res_valid |= ...). The error will only be
returned if
no prefetchable memory region was found.
dw_pcie_host_init() will then again traverse the list of
resources and
it will typically encounter two resource of type IORESOURCE_MEM,
one for
non-prefetchable memory and another for prefetchable memory.
Vidya's patch is to differentiate between these two resources and
allow
prefetchable memory regions to exceed sizes of 4 GiB.
That said, I wonder if there isn't a bigger problem at hand here.
From
looking at the code it doesn't seem like the DWC driver makes any
distinction between prefetchable and non-prefetchable memory. Or at
least it doesn't allow both to be stored in struct pcie_port.
Am I missing something? Or can anyone explain how we're
programming the
apertures for prefetchable vs. non-prefetchable memory? Perhaps
this is
what Vidya was referring to when he said: "we are not using an
outbound
ATU translation channel for prefetchable memory".
It looks to me like we're also getting partially lucky, or
perhaps that
is by design, in that Tegra194 defines PCI regions in the following
order: I/O, prefetchable memory, non-prefetchable memory. That means
that the DWC core code will overwrite prefetchable memory data
with that
of non-prefetchable memory and hence the non-prefetchable region
ends up
stored in struct pcie_port and is then used to program the ATU
outbound
channel.
