Thank you.
Please see comments inline.

On Fri, Jan 12, 2024 at 7:03 PM Peter Maydell <peter.mayd...@linaro.org> wrote:
On Tue, 9 Jan 2024 at 12:45, Shlomo Pongratz <shlomopongr...@gmail.com> wrote:

Hi; thanks for this patch.

Hanlde wrap around caused by the fact that perior to version 460A
Is this "460A" version number a version of the hardware
we're modelling ?

the limit was 32bit quantity.
See Linux kernel code in:
drivers/pci/controllers/dwc/pcie-designware.c
"/controller/"

function: __dw_pcie_prog_outbound_atu
There don't seem to be any comments in this kernel function
that say anything about wrap-around:

https://elixir.bootlin.com/linux/latest/source/drivers/pci/controller/dwc/pcie-designware.c#L468

so I'm not sure what you're trying to explain by referring to it.
This is just to give some  context.
If you look at the original submission of the controller patch d64e5eabc4c7e20c
pci: Add support for Designware IP block by Andrey Smirnov it is written there
"Add code needed to get a functional PCI subsytem when using in
     conjunction with upstream Linux guest (4.13+)."
Now in a 64bit system the range can be above 4G but as long as
the limit itself is less then 4G the overflow is avoided

Signed-off-by: Shlomo Pongratz <shlo...@pliops.com>

----

Changes since v1:
  * Seperate subject and description
---
  hw/pci-host/designware.c | 15 ++++++++++++++-
  1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
index dd9e389c07..7ce4a6b64d 100644
--- a/hw/pci-host/designware.c
+++ b/hw/pci-host/designware.c
@@ -269,11 +269,24 @@ static void 
designware_pcie_update_viewport(DesignwarePCIERoot *root,
  {
      const uint64_t target = viewport->target;
      const uint64_t base   = viewport->base;
-    const uint64_t size   = (uint64_t)viewport->limit - base + 1;
      const bool enabled    = viewport->cr[1] & DESIGNWARE_PCIE_ATU_ENABLE;
+    uint64_t tbase, tlimit, size;

      MemoryRegion *current, *other;

+    /*
+     * Hanlde wrap around caused by the fact that perior to version 460A
+     * the limit was 32bit quantity.
+     * See Linux kernel code in:
+     * drivers/pci/controllers/dwc/pcie-designware.c
+     * function: __dw_pcie_prog_outbound_atu
+     * Now in a 64bit system the range can be above 4G but as long as
+     * the limit itself is less then 4G the overflow is avoided
+     */
+    tbase = base & 0xffffffff;
+    tlimit = 0x100000000 + (uint64_t)viewport->limit;
+    size = ((tlimit - tbase) & 0xffffffff) + 1;
+
I find this patch a bit confusing, partly because the comment
seems to be written from the perspective of what the kernel
driver is doing, not from the perspective of the hardware
behaviour.

Again I refer to the original patch comment.
The original patch was written to support Linux kernel 4.13+ and a
specific implementation i.MX6 Applications Processor.
I've looked at the i.MX6 reference manual and it was silent regarding
the wrap-around case.
There is no reference to the  relevant Synopsys' Designware's specification.
Note that the pci version of the QEMU is 0, therefore I assume that
the implementation was tailored
to a specific implementation.
What is the behaviour of the actual hardware here, both before
and after 460A ? Which version are we trying to model?
I don't have access to the pantora of Synopsys' Designware's root port.
I can only conclude from the Linux kernel code that although the base
address was always 64 bit quantity,
then before version 460A that the limit quantity was 32 bit quantity
and from version 460A it is 64 bit quantity.
And the document that the original patch was based on didn't say what
is the behavior in case of wrap around.
I don't want to model any specific version, I just want to work with
device tree definitions that has regions above 4G,
which are possible since the base address is 64 bit quantity as long
as the regions size are
less teh 4G.
thanks
-- PMM

Reply via email to