Re: [PATCH v5 05/11] exec/ioport: Add portio_list_set_address()

[Philippe Mathieu-Daudé]

On 14/1/24 13:39, Bernhard Beschow wrote:
> Some SuperI/O devices such as the VIA south bridges or the PC87312 controller
> are able to relocate their SuperI/O functions. Add a convenience function for
> implementing this in the VIA south bridges.
>
> This convenience function relies on previous simplifications in exec/ioport
> which avoids some duplicate synchronization of I/O port base addresses. The
> naming of the function is inspired by its memory_region_set_address() pendant.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   docs/devel/migration.rst |  5 +++--
>   include/exec/ioport.h    |  2 ++
>   system/ioport.c          | 19 +++++++++++++++++++
>   3 files changed, 24 insertions(+), 2 deletions(-)


> +void portio_list_set_address(PortioList *piolist, uint32_t addr)
> +{
> +    MemoryRegionPortioList *mrpio;
> +    unsigned i, j;
> +

       memory_region_transaction_begin();

> +    for (i = 0; i < piolist->nr; ++i) {
> +        mrpio = container_of(piolist->regions[i], MemoryRegionPortioList, mr);

Should we check mrpio->mr is disabled before changing its base address?

> +        memory_region_set_address(&mrpio->mr,
> +                                  mrpio->mr.addr - piolist->addr + addr);
> +        for (j = 0; mrpio->ports[j].size; ++j) {
> +            mrpio->ports[j].offset += addr - piolist->addr;
> +        }

          memory_region_transaction_commit();

> +    }
> +
> +    piolist->addr = addr;
> +}
> +
>   static void memory_region_portio_list_finalize(Object *obj)
>   {
>       MemoryRegionPortioList *mrpio = MEMORY_REGION_PORTIO_LIST(obj);