On Tue, 2024-01-23 at 11:58 +0200, Shlomo Pongratz wrote: > 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 <[email protected]> > > ---- > > 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; >
Hi all, just a gentle ping on this patc from 2024. It is in Patchwork ever since but haven't been merged yet. Is there anything I need to do to get these into a future pull request?"
