| Message ID | 20240121164754.47367-1-shlomop@pliops.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] Handle wrap around in limit calculation | expand |
Hi Shlomo, On 21/1/24 17:47, Shlomo Pongratz wrote: > From: Shlomo Pongratz <shlomopongratz@gmail.com> > > 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 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 | 19 ++++++++++++++----- > include/hw/pci-host/designware.h | 2 +- > 2 files changed, 15 insertions(+), 6 deletions(-) > > diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c > index dd9e389c07..43cba9432f 100644 > --- a/hw/pci-host/designware.c > +++ b/hw/pci-host/designware.c > @@ -269,7 +269,7 @@ 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 uint64_t size = viewport->limit - base + 1; > const bool enabled = viewport->cr[1] & DESIGNWARE_PCIE_ATU_ENABLE; > > MemoryRegion *current, *other; > @@ -351,6 +351,14 @@ static void designware_pcie_root_config_write(PCIDevice *d, uint32_t address, > case DESIGNWARE_PCIE_ATU_UPPER_BASE: > viewport->base &= 0x00000000FFFFFFFFULL; > viewport->base |= (uint64_t)val << 32; > + /* The documentatoin states that the value of this register > + * "Forms bits [63:32] of the start (and end) address > + * of the address region to be translated. > + * Note that from version 406A there is a sperate > + * register fot the upper end address > + */ > + viewport->limit &= 0x00000000FFFFFFFFULL; > + viewport->limit |= (uint64_t)val << 32; This code is easier to review using: viewport->limit = deposit64(viewport->limit, 32, 32, val); > break; > > case DESIGNWARE_PCIE_ATU_LOWER_TARGET: > @@ -364,7 +372,8 @@ static void designware_pcie_root_config_write(PCIDevice *d, uint32_t address, > break; > > case DESIGNWARE_PCIE_ATU_LIMIT: > - viewport->limit = val; > + viewport->limit &= 0xFFFFFFFF00000000ULL; > + viewport->limit |= val; Here: viewport->limit = deposit64(viewport->limit, 0, 32, val); > break; > > case DESIGNWARE_PCIE_ATU_CR1: > @@ -429,7 +438,7 @@ static void designware_pcie_root_realize(PCIDevice *dev, Error **errp) > viewport->inbound = true; > viewport->base = 0x0000000000000000ULL; > viewport->target = 0x0000000000000000ULL; > - viewport->limit = UINT32_MAX; > + viewport->limit = 0x00000000FFFFFFFFULL; Previous code is easier to review IMHO. > viewport->cr[0] = DESIGNWARE_PCIE_ATU_TYPE_MEM; > > source = &host->pci.address_space_root; > @@ -453,7 +462,7 @@ static void designware_pcie_root_realize(PCIDevice *dev, Error **errp) > viewport->inbound = false; > viewport->base = 0x0000000000000000ULL; > viewport->target = 0x0000000000000000ULL; > - viewport->limit = UINT32_MAX; > + viewport->limit = 0x00000000FFFFFFFFULL; Ditto. > viewport->cr[0] = DESIGNWARE_PCIE_ATU_TYPE_MEM; > > destination = &host->pci.memory; > @@ -560,7 +569,7 @@ static const VMStateDescription vmstate_designware_pcie_viewport = { > .fields = (const VMStateField[]) { > VMSTATE_UINT64(base, DesignwarePCIEViewport), > VMSTATE_UINT64(target, DesignwarePCIEViewport), > - VMSTATE_UINT32(limit, DesignwarePCIEViewport), > + VMSTATE_UINT64(limit, DesignwarePCIEViewport), Unfortunately this breaks the migration stream. I'm not sure what is the best way to deal with it (Cc'ing migration maintainers). > VMSTATE_UINT32_ARRAY(cr, DesignwarePCIEViewport, 2), > VMSTATE_END_OF_LIST() > } Regards, Phil.
On Mon, Jan 22, 2024 at 12:17:24AM +0100, Philippe Mathieu-Daudé wrote: > > @@ -560,7 +569,7 @@ static const VMStateDescription vmstate_designware_pcie_viewport = { > > .fields = (const VMStateField[]) { > > VMSTATE_UINT64(base, DesignwarePCIEViewport), > > VMSTATE_UINT64(target, DesignwarePCIEViewport), > > - VMSTATE_UINT32(limit, DesignwarePCIEViewport), > > + VMSTATE_UINT64(limit, DesignwarePCIEViewport), > > Unfortunately this breaks the migration stream. I'm not sure > what is the best way to deal with it (Cc'ing migration > maintainers). My understanding is that we can have at least two ways to do this, one relying on machine type properties, the other one can be VMSD versioning. Frankly I don't have a solid mind either on which is the best approach. I never had a talk with either Juan / Dave before on this, but my understanding is that VMSD versioning is just less-flexible, because it doesn't support backward migrations (only forward). While machine-type property based solution can support both (forward + backward). I decided to draft a doc update for this, to put my thoughts here: https://lore.kernel.org/qemu-devel/20240122070600.16681-1-peterx@redhat.com It can be seen as a reference, or review comments also welcomed. This device seems to be only supported by CONFIG_FSL_IMX7. Maybe vmsd versioning would be good enough here, then? If so, instead of VMSTATE_UINT64(), we may want a new VMSTATE_UINT32_V() with a boosted version. Thanks,
Please see inline On 22/01/2024 01:17, Philippe Mathieu-Daudé wrote: > Hi Shlomo, > > On 21/1/24 17:47, Shlomo Pongratz wrote: >> From: Shlomo Pongratz <shlomopongratz@gmail.com> >> >> 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 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 | 19 ++++++++++++++----- >> include/hw/pci-host/designware.h | 2 +- >> 2 files changed, 15 insertions(+), 6 deletions(-) >> >> diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c >> index dd9e389c07..43cba9432f 100644 >> --- a/hw/pci-host/designware.c >> +++ b/hw/pci-host/designware.c >> @@ -269,7 +269,7 @@ 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 uint64_t size = viewport->limit - base + 1; >> const bool enabled = viewport->cr[1] & >> DESIGNWARE_PCIE_ATU_ENABLE; >> MemoryRegion *current, *other; >> @@ -351,6 +351,14 @@ static void >> designware_pcie_root_config_write(PCIDevice *d, uint32_t address, >> case DESIGNWARE_PCIE_ATU_UPPER_BASE: >> viewport->base &= 0x00000000FFFFFFFFULL; >> viewport->base |= (uint64_t)val << 32; >> + /* The documentatoin states that the value of this register >> + * "Forms bits [63:32] of the start (and end) address >> + * of the address region to be translated. >> + * Note that from version 406A there is a sperate >> + * register fot the upper end address >> + */ >> + viewport->limit &= 0x00000000FFFFFFFFULL; >> + viewport->limit |= (uint64_t)val << 32; > > This code is easier to review using: > > viewport->limit = deposit64(viewport->limit, 32, 32, val); > It will be strange to have viewport->base &= 0x00000000FFFFFFFFULL; viewport->base |= (uint64_t)val << 32; and then viewport->limit = deposit64(viewport->limit, 32, 32, val); I think that the code for base and limit should be the same. And I don't think the original for base should be change to viewport->base = deposit64(viewport->base, 32, 32, val); SO everything will look the same. >> break; >> case DESIGNWARE_PCIE_ATU_LOWER_TARGET: >> @@ -364,7 +372,8 @@ static void >> designware_pcie_root_config_write(PCIDevice *d, uint32_t address, >> break; >> case DESIGNWARE_PCIE_ATU_LIMIT: >> - viewport->limit = val; >> + viewport->limit &= 0xFFFFFFFF00000000ULL; >> + viewport->limit |= val; > > Here: > > viewport->limit = deposit64(viewport->limit, 0, 32, val); My opinion is that the code should be identical to case DESIGNWARE_PCIE_ATU_LOWER_BASE: viewport->base &= 0xFFFFFFFF00000000ULL; viewport->base |= val; break; I don't think it is good to mix two styles. >> break; >> case DESIGNWARE_PCIE_ATU_CR1: >> @@ -429,7 +438,7 @@ static void >> designware_pcie_root_realize(PCIDevice *dev, Error **errp) >> viewport->inbound = true; >> viewport->base = 0x0000000000000000ULL; >> viewport->target = 0x0000000000000000ULL; >> - viewport->limit = UINT32_MAX; >> + viewport->limit = 0x00000000FFFFFFFFULL; > > Previous code is easier to review IMHO. Just want to make it clear that this is 64 bit values and that the upper value is the same as the base's upper value, and according to spec. >> viewport->cr[0] = DESIGNWARE_PCIE_ATU_TYPE_MEM; >> source = &host->pci.address_space_root; >> @@ -453,7 +462,7 @@ static void >> designware_pcie_root_realize(PCIDevice *dev, Error **errp) >> viewport->inbound = false; >> viewport->base = 0x0000000000000000ULL; >> viewport->target = 0x0000000000000000ULL; >> - viewport->limit = UINT32_MAX; >> + viewport->limit = 0x00000000FFFFFFFFULL; > > Ditto. Ditto. > >> viewport->cr[0] = DESIGNWARE_PCIE_ATU_TYPE_MEM; >> destination = &host->pci.memory; >> @@ -560,7 +569,7 @@ static const VMStateDescription >> vmstate_designware_pcie_viewport = { >> .fields = (const VMStateField[]) { >> VMSTATE_UINT64(base, DesignwarePCIEViewport), >> VMSTATE_UINT64(target, DesignwarePCIEViewport), >> - VMSTATE_UINT32(limit, DesignwarePCIEViewport), >> + VMSTATE_UINT64(limit, DesignwarePCIEViewport), > > Unfortunately this breaks the migration stream. I'm not sure > what is the best way to deal with it (Cc'ing migration > maintainers). My bad forgot to update version_id and minimum_version_id. I'll consult Peter Xu's document. > >> VMSTATE_UINT32_ARRAY(cr, DesignwarePCIEViewport, 2), >> VMSTATE_END_OF_LIST() >> } > Regards, > > Phil. ------------------------------------------------------------------------ *From:* Philippe Mathieu-Daudé [mailto:philmd@linaro.org] *Sent:* Monday, January 22, 2024, 1:17 AM *To:* Shlomo Pongratz; qemu-devel@nongnu.org *Cc:* andrew.sminov@gmail.com, peter.maydell@linaro.com, shlomop@pliops.com, Peter Xu; Fabiano Rosas *Subject:* [PATCH v3] Handle wrap around in limit calculation > Hi Shlomo, > > On 21/1/24 17:47, Shlomo Pongratz wrote: >> From: Shlomo Pongratz <shlomopongratz@gmail.com> >> >> 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 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 | 19 ++++++++++++++----- >> include/hw/pci-host/designware.h | 2 +- >> 2 files changed, 15 insertions(+), 6 deletions(-) >> >> diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c >> index dd9e389c07..43cba9432f 100644 >> --- a/hw/pci-host/designware.c >> +++ b/hw/pci-host/designware.c >> @@ -269,7 +269,7 @@ 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 uint64_t size = viewport->limit - base + 1; >> const bool enabled = viewport->cr[1] & >> DESIGNWARE_PCIE_ATU_ENABLE; >> MemoryRegion *current, *other; >> @@ -351,6 +351,14 @@ static void >> designware_pcie_root_config_write(PCIDevice *d, uint32_t address, >> case DESIGNWARE_PCIE_ATU_UPPER_BASE: >> viewport->base &= 0x00000000FFFFFFFFULL; >> viewport->base |= (uint64_t)val << 32; >> + /* The documentatoin states that the value of this register >> + * "Forms bits [63:32] of the start (and end) address >> + * of the address region to be translated. >> + * Note that from version 406A there is a sperate >> + * register fot the upper end address >> + */ >> + viewport->limit &= 0x00000000FFFFFFFFULL; >> + viewport->limit |= (uint64_t)val << 32; > > This code is easier to review using: > > viewport->limit = deposit64(viewport->limit, 32, 32, val); > >> break; >> case DESIGNWARE_PCIE_ATU_LOWER_TARGET: >> @@ -364,7 +372,8 @@ static void >> designware_pcie_root_config_write(PCIDevice *d, uint32_t address, >> break; >> case DESIGNWARE_PCIE_ATU_LIMIT: >> - viewport->limit = val; >> + viewport->limit &= 0xFFFFFFFF00000000ULL; >> + viewport->limit |= val; > > Here: > > viewport->limit = deposit64(viewport->limit, 0, 32, val); > >> break; >> case DESIGNWARE_PCIE_ATU_CR1: >> @@ -429,7 +438,7 @@ static void >> designware_pcie_root_realize(PCIDevice *dev, Error **errp) >> viewport->inbound = true; >> viewport->base = 0x0000000000000000ULL; >> viewport->target = 0x0000000000000000ULL; >> - viewport->limit = UINT32_MAX; >> + viewport->limit = 0x00000000FFFFFFFFULL; > > Previous code is easier to review IMHO. > >> viewport->cr[0] = DESIGNWARE_PCIE_ATU_TYPE_MEM; >> source = &host->pci.address_space_root; >> @@ -453,7 +462,7 @@ static void >> designware_pcie_root_realize(PCIDevice *dev, Error **errp) >> viewport->inbound = false; >> viewport->base = 0x0000000000000000ULL; >> viewport->target = 0x0000000000000000ULL; >> - viewport->limit = UINT32_MAX; >> + viewport->limit = 0x00000000FFFFFFFFULL; > > Ditto. > >> viewport->cr[0] = DESIGNWARE_PCIE_ATU_TYPE_MEM; >> destination = &host->pci.memory; >> @@ -560,7 +569,7 @@ static const VMStateDescription >> vmstate_designware_pcie_viewport = { >> .fields = (const VMStateField[]) { >> VMSTATE_UINT64(base, DesignwarePCIEViewport), >> VMSTATE_UINT64(target, DesignwarePCIEViewport), >> - VMSTATE_UINT32(limit, DesignwarePCIEViewport), >> + VMSTATE_UINT64(limit, DesignwarePCIEViewport), > > Unfortunately this breaks the migration stream. I'm not sure > what is the best way to deal with it (Cc'ing migration > maintainers). > >> VMSTATE_UINT32_ARRAY(cr, DesignwarePCIEViewport, 2), >> VMSTATE_END_OF_LIST() >> } > Regards, > > Phil.
On Sun, 21 Jan 2024 at 16:48, Shlomo Pongratz <shlomopongratz@gmail.com> wrote: > > From: Shlomo Pongratz <shlomopongratz@gmail.com> > > 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 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 | 19 ++++++++++++++----- > include/hw/pci-host/designware.h | 2 +- > 2 files changed, 15 insertions(+), 6 deletions(-) > > diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c > index dd9e389c07..43cba9432f 100644 > --- a/hw/pci-host/designware.c > +++ b/hw/pci-host/designware.c > @@ -269,7 +269,7 @@ 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 uint64_t size = viewport->limit - base + 1; > const bool enabled = viewport->cr[1] & DESIGNWARE_PCIE_ATU_ENABLE; > > MemoryRegion *current, *other; > @@ -351,6 +351,14 @@ static void designware_pcie_root_config_write(PCIDevice *d, uint32_t address, > case DESIGNWARE_PCIE_ATU_UPPER_BASE: > viewport->base &= 0x00000000FFFFFFFFULL; > viewport->base |= (uint64_t)val << 32; > + /* The documentatoin states that the value of this register "documentation". Multiline comments should have the /* on a line of its own. > + * "Forms bits [63:32] of the start (and end) address > + * of the address region to be translated. > + * Note that from version 406A there is a sperate > + * register fot the upper end address > + */ > + viewport->limit &= 0x00000000FFFFFFFFULL; > + viewport->limit |= (uint64_t)val << 32; > break; Better to calculate the effective limit address when we need it, rather than when the register is written. It's not a very expensive calculation. > diff --git a/include/hw/pci-host/designware.h b/include/hw/pci-host/designware.h > index 908f3d946b..51052683b7 100644 > --- a/include/hw/pci-host/designware.h > +++ b/include/hw/pci-host/designware.h > @@ -41,7 +41,7 @@ typedef struct DesignwarePCIEViewport { > > uint64_t base; > uint64_t target; > - uint32_t limit; > + uint64_t limit; > uint32_t cr[2]; Making this field 64 bits makes the code to read and write the register more complicated, and introduces a migration compat break that we need to deal with. Why bother? You can fix the problem that you're trying to solve in the way that I suggested to you without introducing this extra complication to this patch. thanks -- PMM
diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c index dd9e389c07..43cba9432f 100644 --- a/hw/pci-host/designware.c +++ b/hw/pci-host/designware.c @@ -269,7 +269,7 @@ 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 uint64_t size = viewport->limit - base + 1; const bool enabled = viewport->cr[1] & DESIGNWARE_PCIE_ATU_ENABLE; MemoryRegion *current, *other; @@ -351,6 +351,14 @@ static void designware_pcie_root_config_write(PCIDevice *d, uint32_t address, case DESIGNWARE_PCIE_ATU_UPPER_BASE: viewport->base &= 0x00000000FFFFFFFFULL; viewport->base |= (uint64_t)val << 32; + /* The documentatoin states that the value of this register + * "Forms bits [63:32] of the start (and end) address + * of the address region to be translated. + * Note that from version 406A there is a sperate + * register fot the upper end address + */ + viewport->limit &= 0x00000000FFFFFFFFULL; + viewport->limit |= (uint64_t)val << 32; break; case DESIGNWARE_PCIE_ATU_LOWER_TARGET: @@ -364,7 +372,8 @@ static void designware_pcie_root_config_write(PCIDevice *d, uint32_t address, break; case DESIGNWARE_PCIE_ATU_LIMIT: - viewport->limit = val; + viewport->limit &= 0xFFFFFFFF00000000ULL; + viewport->limit |= val; break; case DESIGNWARE_PCIE_ATU_CR1: @@ -429,7 +438,7 @@ static void designware_pcie_root_realize(PCIDevice *dev, Error **errp) viewport->inbound = true; viewport->base = 0x0000000000000000ULL; viewport->target = 0x0000000000000000ULL; - viewport->limit = UINT32_MAX; + viewport->limit = 0x00000000FFFFFFFFULL; viewport->cr[0] = DESIGNWARE_PCIE_ATU_TYPE_MEM; source = &host->pci.address_space_root; @@ -453,7 +462,7 @@ static void designware_pcie_root_realize(PCIDevice *dev, Error **errp) viewport->inbound = false; viewport->base = 0x0000000000000000ULL; viewport->target = 0x0000000000000000ULL; - viewport->limit = UINT32_MAX; + viewport->limit = 0x00000000FFFFFFFFULL; viewport->cr[0] = DESIGNWARE_PCIE_ATU_TYPE_MEM; destination = &host->pci.memory; @@ -560,7 +569,7 @@ static const VMStateDescription vmstate_designware_pcie_viewport = { .fields = (const VMStateField[]) { VMSTATE_UINT64(base, DesignwarePCIEViewport), VMSTATE_UINT64(target, DesignwarePCIEViewport), - VMSTATE_UINT32(limit, DesignwarePCIEViewport), + VMSTATE_UINT64(limit, DesignwarePCIEViewport), VMSTATE_UINT32_ARRAY(cr, DesignwarePCIEViewport, 2), VMSTATE_END_OF_LIST() } diff --git a/include/hw/pci-host/designware.h b/include/hw/pci-host/designware.h index 908f3d946b..51052683b7 100644 --- a/include/hw/pci-host/designware.h +++ b/include/hw/pci-host/designware.h @@ -41,7 +41,7 @@ typedef struct DesignwarePCIEViewport { uint64_t base; uint64_t target; - uint32_t limit; + uint64_t limit; uint32_t cr[2]; bool inbound;