[PATCH v4] Handle wrap around in limit calculation

Shlomo Pongratz posted 1 patch 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20240123095821.59625-1-shlomop@pliops.com
Maintainers: Peter Maydell <peter.maydell@linaro.org>, Andrey Smirnov <andrew.smirnov@gmail.com>
hw/pci-host/designware.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
[PATCH v4] Handle wrap around in limit calculation
Posted by Shlomo Pongratz 8 months ago
    Hanlde wrap around when calculating the viewport size
    caused by the fact that perior to version 460A the limit variable
    was 32bit quantity and not 64 bits quantity.
    In the i.MX 6Dual/6Quad Applications Processor Reference Manual
    document on which the original code was based upon in the
    description of the iATU Region Upper Base Address Register it is
    written:
    Forms bits [63:32] of the start (and end) address of the address region to be
    translated.
    That is in this register is the upper of both base and the limit.
    In the current implementation this value was ignored for the limit
    which caused a wrap around of the size calculaiton.
    Using the documnet example:
    Base HI: 0x80000000 Base LO: 0xd0000000 Limit LO: 0xd000ffff
    The correct size is 0x80000000d000ffff - 0x80000000d0000000 + 1 =
0x010000
    The wrong result is 0xd000ffff - 0x80000000d0000000 + 1 = 0x8000000000010000

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

    ----

    Changes since v3:
     * Use Peter Maydell's suggestion for fixing the bug,
       that is compute a 64 bits limit from the upper 32 bits of the base
       and the given 32 bits limit.

    Changes since v2:
     * Don't try to fix the calculation.
     * Change the limit variable from 32bit to 64 bit
     * Set the limit bits [63:32] same as the base according to
       the specification on which the original code was base upon.

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

diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
index dd9e389c07..f81dbe3975 100644
--- a/hw/pci-host/designware.c
+++ b/hw/pci-host/designware.c
@@ -269,8 +269,20 @@ 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;
+    uint64_t size;
     const bool enabled    = viewport->cr[1] & DESIGNWARE_PCIE_ATU_ENABLE;
+    uint64_t limit;
+
+    /*
+     * Versions 460A and above of this PCI controller have a
+     * 64-bit limit register. We currently model the older type
+     * where the limit register is 32 bits, and the upper 32 bits
+     * of the end address are the same as the upper 32 bits of
+     * the start address.
+     */
+
+    limit = deposit64(viewport->limit, 32, 32, extract64(base, 32, 32));
+    size = limit - base + 1;
 
     MemoryRegion *current, *other;
 
-- 
2.25.1