mbox series

[v4,00/11] hw/isa/vt82c686: Implement relocation and toggling of SuperI/O functions

Message ID 20240106210531.140542-1-shentey@gmail.com (mailing list archive)
Headers show
Series hw/isa/vt82c686: Implement relocation and toggling of SuperI/O functions | expand

Message

Bernhard Beschow Jan. 6, 2024, 9:05 p.m. UTC
This series implements relocation of the SuperI/O functions of the VIA south
bridges which resolves some FIXME's. It is part of my via-apollo-pro-133t
branch [1] which is an extension of bringing the VIA south bridges to the PC
machine [2]. This branch is able to run some real-world X86 BIOSes in the hope
that it allows us to form a better understanding of the real vt82c686b devices.
Implementing relocation and toggling of the SuperI/O functions is one step to
make these BIOSes run without error messages, so here we go.

The series is structured as follows: Patches 1-3 prepare the TYPE_ISA_FDC,
TYPE_ISA_PARALLEL and TYPE_ISA_SERIAL to relocate and toggle (enable/disable)
themselves without breaking encapsulation of their respective device states.
This is achieved by moving the MemoryRegions and PortioLists from the device
states into the encapsulating ISA devices since they will be relocated and
toggled.

Inspired by the memory API patches 4-6 add two convenience functions to the
portio_list API to toggle and relocate portio lists. Patch 5 is a preparation
for that which removes some redundancies which otherwise had to be dealt with
during relocation.

Patches 7-9 implement toggling and relocation for types TYPE_ISA_FDC,
TYPE_ISA_PARALLEL and TYPE_ISA_SERIAL. Patch 10 prepares the pegasos2 machine
which would end up with all SuperI/O functions disabled if no -bios argument is
given. Patch 11 finally implements the main feature which now relies on
firmware to configure the SuperI/O functions accordingly (except for pegasos2).

v4:
* Drop incomplete SuperI/O vmstate handling (Zoltan)

v3:
* Rework various commit messages (Zoltan)
* Drop patch "hw/char/serial: Free struct SerialState from MemoryRegion"
  (Zoltan)
* Generalize wording in migration.rst to include portio_list API (Zoltan)

v2:
* Improve commit messages (Zoltan)
* Split pegasos2 from vt82c686 patch (Zoltan)
* Avoid poking into device internals (Zoltan)

Testing done:
* `make check`
* `make check-avocado`
* Run MorphOS on pegasos2 with and without pegasos2.rom
* Run Linux on amigaone
* Run real-world BIOSes on via-apollo-pro-133t branch
* Start rescue-yl on fuloong2e

[1] https://github.com/shentok/qemu/tree/via-apollo-pro-133t
[2] https://github.com/shentok/qemu/tree/pc-via

Bernhard Beschow (11):
  hw/block/fdc-isa: Move portio_list from FDCtrl to FDCtrlISABus
  hw/block/fdc-sysbus: Move iomem from FDCtrl to FDCtrlSysBus
  hw/char/parallel: Move portio_list from ParallelState to
    ISAParallelState
  exec/ioport: Resolve redundant .base attribute in struct
    MemoryRegionPortio
  exec/ioport: Add portio_list_set_address()
  exec/ioport: Add portio_list_set_enabled()
  hw/block/fdc-isa: Implement relocation and enabling/disabling for
    TYPE_ISA_FDC
  hw/char/serial-isa: Implement relocation and enabling/disabling for
    TYPE_ISA_SERIAL
  hw/char/parallel-isa: Implement relocation and enabling/disabling for
    TYPE_ISA_PARALLEL
  hw/ppc/pegasos2: Let pegasos2 machine configure SuperI/O functions
  hw/isa/vt82c686: Implement relocation and toggling of SuperI/O
    functions

 docs/devel/migration.rst       |  6 ++--
 hw/block/fdc-internal.h        |  4 ---
 include/exec/ioport.h          |  4 ++-
 include/hw/block/fdc.h         |  3 ++
 include/hw/char/parallel-isa.h |  5 +++
 include/hw/char/parallel.h     |  2 --
 include/hw/char/serial.h       |  2 ++
 hw/block/fdc-isa.c             | 18 +++++++++-
 hw/block/fdc-sysbus.c          |  6 ++--
 hw/char/parallel-isa.c         | 14 ++++++++
 hw/char/parallel.c             |  2 +-
 hw/char/serial-isa.c           | 14 ++++++++
 hw/isa/vt82c686.c              | 66 ++++++++++++++++++++++++++++------
 hw/ppc/pegasos2.c              | 15 ++++++++
 system/ioport.c                | 41 +++++++++++++++++----
 15 files changed, 172 insertions(+), 30 deletions(-)

Comments

Mark Cave-Ayland Jan. 7, 2024, 2:13 p.m. UTC | #1
On 06/01/2024 21:05, Bernhard Beschow wrote:

> This series implements relocation of the SuperI/O functions of the VIA south
> bridges which resolves some FIXME's. It is part of my via-apollo-pro-133t
> branch [1] which is an extension of bringing the VIA south bridges to the PC
> machine [2]. This branch is able to run some real-world X86 BIOSes in the hope
> that it allows us to form a better understanding of the real vt82c686b devices.
> Implementing relocation and toggling of the SuperI/O functions is one step to
> make these BIOSes run without error messages, so here we go.
> 
> The series is structured as follows: Patches 1-3 prepare the TYPE_ISA_FDC,
> TYPE_ISA_PARALLEL and TYPE_ISA_SERIAL to relocate and toggle (enable/disable)
> themselves without breaking encapsulation of their respective device states.
> This is achieved by moving the MemoryRegions and PortioLists from the device
> states into the encapsulating ISA devices since they will be relocated and
> toggled.
> 
> Inspired by the memory API patches 4-6 add two convenience functions to the
> portio_list API to toggle and relocate portio lists. Patch 5 is a preparation
> for that which removes some redundancies which otherwise had to be dealt with
> during relocation.
> 
> Patches 7-9 implement toggling and relocation for types TYPE_ISA_FDC,
> TYPE_ISA_PARALLEL and TYPE_ISA_SERIAL. Patch 10 prepares the pegasos2 machine
> which would end up with all SuperI/O functions disabled if no -bios argument is
> given. Patch 11 finally implements the main feature which now relies on
> firmware to configure the SuperI/O functions accordingly (except for pegasos2).
> 
> v4:
> * Drop incomplete SuperI/O vmstate handling (Zoltan)
> 
> v3:
> * Rework various commit messages (Zoltan)
> * Drop patch "hw/char/serial: Free struct SerialState from MemoryRegion"
>    (Zoltan)
> * Generalize wording in migration.rst to include portio_list API (Zoltan)
> 
> v2:
> * Improve commit messages (Zoltan)
> * Split pegasos2 from vt82c686 patch (Zoltan)
> * Avoid poking into device internals (Zoltan)
> 
> Testing done:
> * `make check`
> * `make check-avocado`
> * Run MorphOS on pegasos2 with and without pegasos2.rom
> * Run Linux on amigaone
> * Run real-world BIOSes on via-apollo-pro-133t branch
> * Start rescue-yl on fuloong2e
> 
> [1] https://github.com/shentok/qemu/tree/via-apollo-pro-133t
> [2] https://github.com/shentok/qemu/tree/pc-via
> 
> Bernhard Beschow (11):
>    hw/block/fdc-isa: Move portio_list from FDCtrl to FDCtrlISABus
>    hw/block/fdc-sysbus: Move iomem from FDCtrl to FDCtrlSysBus
>    hw/char/parallel: Move portio_list from ParallelState to
>      ISAParallelState
>    exec/ioport: Resolve redundant .base attribute in struct
>      MemoryRegionPortio
>    exec/ioport: Add portio_list_set_address()
>    exec/ioport: Add portio_list_set_enabled()
>    hw/block/fdc-isa: Implement relocation and enabling/disabling for
>      TYPE_ISA_FDC
>    hw/char/serial-isa: Implement relocation and enabling/disabling for
>      TYPE_ISA_SERIAL
>    hw/char/parallel-isa: Implement relocation and enabling/disabling for
>      TYPE_ISA_PARALLEL
>    hw/ppc/pegasos2: Let pegasos2 machine configure SuperI/O functions
>    hw/isa/vt82c686: Implement relocation and toggling of SuperI/O
>      functions
> 
>   docs/devel/migration.rst       |  6 ++--
>   hw/block/fdc-internal.h        |  4 ---
>   include/exec/ioport.h          |  4 ++-
>   include/hw/block/fdc.h         |  3 ++
>   include/hw/char/parallel-isa.h |  5 +++
>   include/hw/char/parallel.h     |  2 --
>   include/hw/char/serial.h       |  2 ++
>   hw/block/fdc-isa.c             | 18 +++++++++-
>   hw/block/fdc-sysbus.c          |  6 ++--
>   hw/char/parallel-isa.c         | 14 ++++++++
>   hw/char/parallel.c             |  2 +-
>   hw/char/serial-isa.c           | 14 ++++++++
>   hw/isa/vt82c686.c              | 66 ++++++++++++++++++++++++++++------
>   hw/ppc/pegasos2.c              | 15 ++++++++
>   system/ioport.c                | 41 +++++++++++++++++----
>   15 files changed, 172 insertions(+), 30 deletions(-)

I think this series generally looks good: the only thing I think it's worth checking 
is whether portio lists are considered exclusive to ISA devices or not? (Paolo?).

The portio_list_set_enabled() API looks interesting, and could be considered for use 
by my PCI IDE mode-switching changes too.

Apologies I don't have a huge amount of time for review right now, but I wanted to 
feed back that generally these patches look good, and if people are happy with the 
portio list changes then this series should be considered for merge.


ATB,

Mark.
Bernhard Beschow Jan. 8, 2024, 8:07 p.m. UTC | #2
Am 7. Januar 2024 14:13:44 UTC schrieb Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>:
>On 06/01/2024 21:05, Bernhard Beschow wrote:
>
>> This series implements relocation of the SuperI/O functions of the VIA south
>> bridges which resolves some FIXME's. It is part of my via-apollo-pro-133t
>> branch [1] which is an extension of bringing the VIA south bridges to the PC
>> machine [2]. This branch is able to run some real-world X86 BIOSes in the hope
>> that it allows us to form a better understanding of the real vt82c686b devices.
>> Implementing relocation and toggling of the SuperI/O functions is one step to
>> make these BIOSes run without error messages, so here we go.
>> 
>> The series is structured as follows: Patches 1-3 prepare the TYPE_ISA_FDC,
>> TYPE_ISA_PARALLEL and TYPE_ISA_SERIAL to relocate and toggle (enable/disable)
>> themselves without breaking encapsulation of their respective device states.
>> This is achieved by moving the MemoryRegions and PortioLists from the device
>> states into the encapsulating ISA devices since they will be relocated and
>> toggled.
>> 
>> Inspired by the memory API patches 4-6 add two convenience functions to the
>> portio_list API to toggle and relocate portio lists. Patch 5 is a preparation
>> for that which removes some redundancies which otherwise had to be dealt with
>> during relocation.
>> 
>> Patches 7-9 implement toggling and relocation for types TYPE_ISA_FDC,
>> TYPE_ISA_PARALLEL and TYPE_ISA_SERIAL. Patch 10 prepares the pegasos2 machine
>> which would end up with all SuperI/O functions disabled if no -bios argument is
>> given. Patch 11 finally implements the main feature which now relies on
>> firmware to configure the SuperI/O functions accordingly (except for pegasos2).
>> 
>> v4:
>> * Drop incomplete SuperI/O vmstate handling (Zoltan)
>> 
>> v3:
>> * Rework various commit messages (Zoltan)
>> * Drop patch "hw/char/serial: Free struct SerialState from MemoryRegion"
>>    (Zoltan)
>> * Generalize wording in migration.rst to include portio_list API (Zoltan)
>> 
>> v2:
>> * Improve commit messages (Zoltan)
>> * Split pegasos2 from vt82c686 patch (Zoltan)
>> * Avoid poking into device internals (Zoltan)
>> 
>> Testing done:
>> * `make check`
>> * `make check-avocado`
>> * Run MorphOS on pegasos2 with and without pegasos2.rom
>> * Run Linux on amigaone
>> * Run real-world BIOSes on via-apollo-pro-133t branch
>> * Start rescue-yl on fuloong2e
>> 
>> [1] https://github.com/shentok/qemu/tree/via-apollo-pro-133t
>> [2] https://github.com/shentok/qemu/tree/pc-via
>> 
>> Bernhard Beschow (11):
>>    hw/block/fdc-isa: Move portio_list from FDCtrl to FDCtrlISABus
>>    hw/block/fdc-sysbus: Move iomem from FDCtrl to FDCtrlSysBus
>>    hw/char/parallel: Move portio_list from ParallelState to
>>      ISAParallelState
>>    exec/ioport: Resolve redundant .base attribute in struct
>>      MemoryRegionPortio
>>    exec/ioport: Add portio_list_set_address()
>>    exec/ioport: Add portio_list_set_enabled()
>>    hw/block/fdc-isa: Implement relocation and enabling/disabling for
>>      TYPE_ISA_FDC
>>    hw/char/serial-isa: Implement relocation and enabling/disabling for
>>      TYPE_ISA_SERIAL
>>    hw/char/parallel-isa: Implement relocation and enabling/disabling for
>>      TYPE_ISA_PARALLEL
>>    hw/ppc/pegasos2: Let pegasos2 machine configure SuperI/O functions
>>    hw/isa/vt82c686: Implement relocation and toggling of SuperI/O
>>      functions
>> 
>>   docs/devel/migration.rst       |  6 ++--
>>   hw/block/fdc-internal.h        |  4 ---
>>   include/exec/ioport.h          |  4 ++-
>>   include/hw/block/fdc.h         |  3 ++
>>   include/hw/char/parallel-isa.h |  5 +++
>>   include/hw/char/parallel.h     |  2 --
>>   include/hw/char/serial.h       |  2 ++
>>   hw/block/fdc-isa.c             | 18 +++++++++-
>>   hw/block/fdc-sysbus.c          |  6 ++--
>>   hw/char/parallel-isa.c         | 14 ++++++++
>>   hw/char/parallel.c             |  2 +-
>>   hw/char/serial-isa.c           | 14 ++++++++
>>   hw/isa/vt82c686.c              | 66 ++++++++++++++++++++++++++++------
>>   hw/ppc/pegasos2.c              | 15 ++++++++
>>   system/ioport.c                | 41 +++++++++++++++++----
>>   15 files changed, 172 insertions(+), 30 deletions(-)
>
>I think this series generally looks good: the only thing I think it's worth checking is whether portio lists are considered exclusive to ISA devices or not? (Paolo?).

The modifications preserve the current design, so how is this question related to this series?

I'd appreciate feedback from the maintainers indeed since this part hasn't received any comments so far. Thanks :)

>
>The portio_list_set_enabled() API looks interesting, and could be considered for use by my PCI IDE mode-switching changes too.
>
>Apologies I don't have a huge amount of time for review right now, but I wanted to feed back that generally these patches look good, and if people are happy with the portio list changes then this series should be considered for merge.

Never mind, it's still nice getting some confirmation from your side!

Best regards,
Bernhard

>
>
>ATB,
>
>Mark.
>
Mark Cave-Ayland Jan. 8, 2024, 10:12 p.m. UTC | #3
On 08/01/2024 20:07, Bernhard Beschow wrote:

> Am 7. Januar 2024 14:13:44 UTC schrieb Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>:
>> On 06/01/2024 21:05, Bernhard Beschow wrote:
>>
>>> This series implements relocation of the SuperI/O functions of the VIA south
>>> bridges which resolves some FIXME's. It is part of my via-apollo-pro-133t
>>> branch [1] which is an extension of bringing the VIA south bridges to the PC
>>> machine [2]. This branch is able to run some real-world X86 BIOSes in the hope
>>> that it allows us to form a better understanding of the real vt82c686b devices.
>>> Implementing relocation and toggling of the SuperI/O functions is one step to
>>> make these BIOSes run without error messages, so here we go.
>>>
>>> The series is structured as follows: Patches 1-3 prepare the TYPE_ISA_FDC,
>>> TYPE_ISA_PARALLEL and TYPE_ISA_SERIAL to relocate and toggle (enable/disable)
>>> themselves without breaking encapsulation of their respective device states.
>>> This is achieved by moving the MemoryRegions and PortioLists from the device
>>> states into the encapsulating ISA devices since they will be relocated and
>>> toggled.
>>>
>>> Inspired by the memory API patches 4-6 add two convenience functions to the
>>> portio_list API to toggle and relocate portio lists. Patch 5 is a preparation
>>> for that which removes some redundancies which otherwise had to be dealt with
>>> during relocation.
>>>
>>> Patches 7-9 implement toggling and relocation for types TYPE_ISA_FDC,
>>> TYPE_ISA_PARALLEL and TYPE_ISA_SERIAL. Patch 10 prepares the pegasos2 machine
>>> which would end up with all SuperI/O functions disabled if no -bios argument is
>>> given. Patch 11 finally implements the main feature which now relies on
>>> firmware to configure the SuperI/O functions accordingly (except for pegasos2).
>>>
>>> v4:
>>> * Drop incomplete SuperI/O vmstate handling (Zoltan)
>>>
>>> v3:
>>> * Rework various commit messages (Zoltan)
>>> * Drop patch "hw/char/serial: Free struct SerialState from MemoryRegion"
>>>     (Zoltan)
>>> * Generalize wording in migration.rst to include portio_list API (Zoltan)
>>>
>>> v2:
>>> * Improve commit messages (Zoltan)
>>> * Split pegasos2 from vt82c686 patch (Zoltan)
>>> * Avoid poking into device internals (Zoltan)
>>>
>>> Testing done:
>>> * `make check`
>>> * `make check-avocado`
>>> * Run MorphOS on pegasos2 with and without pegasos2.rom
>>> * Run Linux on amigaone
>>> * Run real-world BIOSes on via-apollo-pro-133t branch
>>> * Start rescue-yl on fuloong2e
>>>
>>> [1] https://github.com/shentok/qemu/tree/via-apollo-pro-133t
>>> [2] https://github.com/shentok/qemu/tree/pc-via
>>>
>>> Bernhard Beschow (11):
>>>     hw/block/fdc-isa: Move portio_list from FDCtrl to FDCtrlISABus
>>>     hw/block/fdc-sysbus: Move iomem from FDCtrl to FDCtrlSysBus
>>>     hw/char/parallel: Move portio_list from ParallelState to
>>>       ISAParallelState
>>>     exec/ioport: Resolve redundant .base attribute in struct
>>>       MemoryRegionPortio
>>>     exec/ioport: Add portio_list_set_address()
>>>     exec/ioport: Add portio_list_set_enabled()
>>>     hw/block/fdc-isa: Implement relocation and enabling/disabling for
>>>       TYPE_ISA_FDC
>>>     hw/char/serial-isa: Implement relocation and enabling/disabling for
>>>       TYPE_ISA_SERIAL
>>>     hw/char/parallel-isa: Implement relocation and enabling/disabling for
>>>       TYPE_ISA_PARALLEL
>>>     hw/ppc/pegasos2: Let pegasos2 machine configure SuperI/O functions
>>>     hw/isa/vt82c686: Implement relocation and toggling of SuperI/O
>>>       functions
>>>
>>>    docs/devel/migration.rst       |  6 ++--
>>>    hw/block/fdc-internal.h        |  4 ---
>>>    include/exec/ioport.h          |  4 ++-
>>>    include/hw/block/fdc.h         |  3 ++
>>>    include/hw/char/parallel-isa.h |  5 +++
>>>    include/hw/char/parallel.h     |  2 --
>>>    include/hw/char/serial.h       |  2 ++
>>>    hw/block/fdc-isa.c             | 18 +++++++++-
>>>    hw/block/fdc-sysbus.c          |  6 ++--
>>>    hw/char/parallel-isa.c         | 14 ++++++++
>>>    hw/char/parallel.c             |  2 +-
>>>    hw/char/serial-isa.c           | 14 ++++++++
>>>    hw/isa/vt82c686.c              | 66 ++++++++++++++++++++++++++++------
>>>    hw/ppc/pegasos2.c              | 15 ++++++++
>>>    system/ioport.c                | 41 +++++++++++++++++----
>>>    15 files changed, 172 insertions(+), 30 deletions(-)
>>
>> I think this series generally looks good: the only thing I think it's worth checking is whether portio lists are considered exclusive to ISA devices or not? (Paolo?).
> 
> The modifications preserve the current design, so how is this question related to this series?

I was thinking about patches 1 and 3 where the portio_list variable is moved from the 
core object to the ISA-specific child objects.

> I'd appreciate feedback from the maintainers indeed since this part hasn't received any comments so far. Thanks :)

Agreed. I *think* the portio_lists are ISA-specific as far as QEMU is concerned, but 
a quick nod from an x86 maintainer would be a great help :)

>> The portio_list_set_enabled() API looks interesting, and could be considered for use by my PCI IDE mode-switching changes too.
>>
>> Apologies I don't have a huge amount of time for review right now, but I wanted to feed back that generally these patches look good, and if people are happy with the portio list changes then this series should be considered for merge.
> 
> Never mind, it's still nice getting some confirmation from your side!

No worries!


ATB,

Mark.
Bernhard Beschow Jan. 9, 2024, 10:09 p.m. UTC | #4
Am 8. Januar 2024 22:12:12 UTC schrieb Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>:
>On 08/01/2024 20:07, Bernhard Beschow wrote:
>
>> Am 7. Januar 2024 14:13:44 UTC schrieb Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>:
>>> On 06/01/2024 21:05, Bernhard Beschow wrote:
>>> 
>>>> This series implements relocation of the SuperI/O functions of the VIA south
>>>> bridges which resolves some FIXME's. It is part of my via-apollo-pro-133t
>>>> branch [1] which is an extension of bringing the VIA south bridges to the PC
>>>> machine [2]. This branch is able to run some real-world X86 BIOSes in the hope
>>>> that it allows us to form a better understanding of the real vt82c686b devices.
>>>> Implementing relocation and toggling of the SuperI/O functions is one step to
>>>> make these BIOSes run without error messages, so here we go.
>>>> 
>>>> The series is structured as follows: Patches 1-3 prepare the TYPE_ISA_FDC,
>>>> TYPE_ISA_PARALLEL and TYPE_ISA_SERIAL to relocate and toggle (enable/disable)
>>>> themselves without breaking encapsulation of their respective device states.
>>>> This is achieved by moving the MemoryRegions and PortioLists from the device
>>>> states into the encapsulating ISA devices since they will be relocated and
>>>> toggled.
>>>> 
>>>> Inspired by the memory API patches 4-6 add two convenience functions to the
>>>> portio_list API to toggle and relocate portio lists. Patch 5 is a preparation
>>>> for that which removes some redundancies which otherwise had to be dealt with
>>>> during relocation.
>>>> 
>>>> Patches 7-9 implement toggling and relocation for types TYPE_ISA_FDC,
>>>> TYPE_ISA_PARALLEL and TYPE_ISA_SERIAL. Patch 10 prepares the pegasos2 machine
>>>> which would end up with all SuperI/O functions disabled if no -bios argument is
>>>> given. Patch 11 finally implements the main feature which now relies on
>>>> firmware to configure the SuperI/O functions accordingly (except for pegasos2).
>>>> 
>>>> v4:
>>>> * Drop incomplete SuperI/O vmstate handling (Zoltan)
>>>> 
>>>> v3:
>>>> * Rework various commit messages (Zoltan)
>>>> * Drop patch "hw/char/serial: Free struct SerialState from MemoryRegion"
>>>>     (Zoltan)
>>>> * Generalize wording in migration.rst to include portio_list API (Zoltan)
>>>> 
>>>> v2:
>>>> * Improve commit messages (Zoltan)
>>>> * Split pegasos2 from vt82c686 patch (Zoltan)
>>>> * Avoid poking into device internals (Zoltan)
>>>> 
>>>> Testing done:
>>>> * `make check`
>>>> * `make check-avocado`
>>>> * Run MorphOS on pegasos2 with and without pegasos2.rom
>>>> * Run Linux on amigaone
>>>> * Run real-world BIOSes on via-apollo-pro-133t branch
>>>> * Start rescue-yl on fuloong2e
>>>> 
>>>> [1] https://github.com/shentok/qemu/tree/via-apollo-pro-133t
>>>> [2] https://github.com/shentok/qemu/tree/pc-via
>>>> 
>>>> Bernhard Beschow (11):
>>>>     hw/block/fdc-isa: Move portio_list from FDCtrl to FDCtrlISABus
>>>>     hw/block/fdc-sysbus: Move iomem from FDCtrl to FDCtrlSysBus
>>>>     hw/char/parallel: Move portio_list from ParallelState to
>>>>       ISAParallelState
>>>>     exec/ioport: Resolve redundant .base attribute in struct
>>>>       MemoryRegionPortio
>>>>     exec/ioport: Add portio_list_set_address()
>>>>     exec/ioport: Add portio_list_set_enabled()
>>>>     hw/block/fdc-isa: Implement relocation and enabling/disabling for
>>>>       TYPE_ISA_FDC
>>>>     hw/char/serial-isa: Implement relocation and enabling/disabling for
>>>>       TYPE_ISA_SERIAL
>>>>     hw/char/parallel-isa: Implement relocation and enabling/disabling for
>>>>       TYPE_ISA_PARALLEL
>>>>     hw/ppc/pegasos2: Let pegasos2 machine configure SuperI/O functions
>>>>     hw/isa/vt82c686: Implement relocation and toggling of SuperI/O
>>>>       functions
>>>> 
>>>>    docs/devel/migration.rst       |  6 ++--
>>>>    hw/block/fdc-internal.h        |  4 ---
>>>>    include/exec/ioport.h          |  4 ++-
>>>>    include/hw/block/fdc.h         |  3 ++
>>>>    include/hw/char/parallel-isa.h |  5 +++
>>>>    include/hw/char/parallel.h     |  2 --
>>>>    include/hw/char/serial.h       |  2 ++
>>>>    hw/block/fdc-isa.c             | 18 +++++++++-
>>>>    hw/block/fdc-sysbus.c          |  6 ++--
>>>>    hw/char/parallel-isa.c         | 14 ++++++++
>>>>    hw/char/parallel.c             |  2 +-
>>>>    hw/char/serial-isa.c           | 14 ++++++++
>>>>    hw/isa/vt82c686.c              | 66 ++++++++++++++++++++++++++++------
>>>>    hw/ppc/pegasos2.c              | 15 ++++++++
>>>>    system/ioport.c                | 41 +++++++++++++++++----
>>>>    15 files changed, 172 insertions(+), 30 deletions(-)
>>> 
>>> I think this series generally looks good: the only thing I think it's worth checking is whether portio lists are considered exclusive to ISA devices or not? (Paolo?).
>> 
>> The modifications preserve the current design, so how is this question related to this series?
>
>I was thinking about patches 1 and 3 where the portio_list variable is moved from the core object to the ISA-specific child objects.

I think of patches 1 - 3 more as cleanups where an attribute unused in the core is moved one level up to where it is needed. In addition, the floppy core had two attributes where only one was ever used in a specific device. From that perspective I think the question of whether the portio_list API is ISA specific or not is unrelated to this series, i.e. should be discussed separately.

>
>> I'd appreciate feedback from the maintainers indeed since this part hasn't received any comments so far. Thanks :)
>
>Agreed. I *think* the portio_lists are ISA-specific as far as QEMU is concerned, but a quick nod from an x86 maintainer would be a great help :)

A quick nod shall be in scope ;)

Best regards,
Bernhard

>
>>> The portio_list_set_enabled() API looks interesting, and could be considered for use by my PCI IDE mode-switching changes too.
>>> 
>>> Apologies I don't have a huge amount of time for review right now, but I wanted to feed back that generally these patches look good, and if people are happy with the portio list changes then this series should be considered for merge.
>> 
>> Never mind, it's still nice getting some confirmation from your side!
>
>No worries!
>
>
>ATB,
>
>Mark.
>