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. > 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. What is the behaviour of the actual hardware here, both before and after 460A ? Which version are we trying to model? thanks -- PMM
