[v3] piix4: cleanup and improvements

[Qemu-devel] [PATCH v3 00/16] piix4: cleanup and improvements

Posted by Hervé Poussineau 7 years, 10 months ago

Hi,

This patchset is a cleanup of the PIIX4 PCI-ISA bridge. Lots of devices
are moved from MIPS Malta board (which has a PIIX4) to PIIX4, where devices
belong. This lets us reuse PIIX4 in other machines, while not loosing any
functionality.

Last patch allows adding a new PIIX4 device directly from command line.
Note that this will work only as long no other ISA bus already exist on the
machine.

Hervé

Changes v2 -> v3:
- rebased on master and fixed conflicts
- added patch 14 (convert reset function to QOM)
- use TYPE_PIIX4_PCI_DEVICE in Malta board (patch 15, Philippe Mathieu-Daudé)

Changes v1 -> v2:
- fixed compilation on CentOS 6 (patch 1)
- automatically create serial/parallel chardevs if not provided (patch 11)

Hervé Poussineau (16):
  fdc: move object structures to header file
  serial/parallel: move object structures to header file
  mc146818rtc: move structure to header file
  mc146818rtc: always register rtc to rtc list
  piix4: rename some variables in realize function
  piix4: add Reset Control Register
  piix4: add a i8259 interrupt controller as specified in datasheet
  piix4: add a i8257 dma controller as specified in datasheet
  piix4: add a i8254 pit controller as specified in datasheet
  piix4: add a i8042 keyboard/mouse controller as specified in datasheet
  piix4: add a floppy controller, 1 parallel port and 2 serial ports
  piix4: add a mc146818rtc controller as specified in datasheet
  piix4: add a speaker as specified in datasheet
  piix4: convert reset function to QOM
  piix4: rename PIIX4 object to piix4-isa
  piix4: we can now instanciate a PIIX4 with -device

 hw/block/fdc.c                 | 102 -----------------------
 hw/char/parallel.c             |  31 +------
 hw/char/serial-isa.c           |  13 +--
 hw/isa/piix4.c                 | 183 ++++++++++++++++++++++++++++++++++++-----
 hw/mips/mips_malta.c           |  74 +++++++----------
 hw/ppc/pnv.c                   |   2 +-
 hw/timer/mc146818rtc.c         |  43 ++--------
 include/hw/block/fdc.h         | 103 +++++++++++++++++++++++
 include/hw/char/isa.h          |  50 +++++++++++
 include/hw/char/serial.h       |   1 -
 include/hw/i386/pc.h           |   1 -
 include/hw/isa/isa.h           |   3 +
 include/hw/timer/mc146818rtc.h |  29 +++++++
 13 files changed, 389 insertions(+), 246 deletions(-)
 create mode 100644 include/hw/char/isa.h

-- 
2.11.0

Re: [Qemu-devel] [PATCH v3 00/16] piix4: cleanup and improvements

Posted by Michael S. Tsirkin 7 years, 9 months ago

On Fri, Dec 29, 2017 at 03:29:06PM +0100, Hervé Poussineau wrote:
> Hi,
> 
> This patchset is a cleanup of the PIIX4 PCI-ISA bridge. Lots of devices
> are moved from MIPS Malta board (which has a PIIX4) to PIIX4, where devices
> belong. This lets us reuse PIIX4 in other machines, while not loosing any
> functionality.
> 
> Last patch allows adding a new PIIX4 device directly from command line.
> Note that this will work only as long no other ISA bus already exist on the
> machine.
> 
> Hervé

Looks ok superficially

Acked-by: Michael S. Tsirkin <mst@redhat.com>

but really I think I'm marked as maintainer of this file for the sole
reason that it uses pc.h.

Once your patches are applied you can remove pc.h include from mips
and piix, so it's no longer pc related.

Sent a patch to move it to the mips section, pls feel free to include
in the patches.


> Changes v2 -> v3:
> - rebased on master and fixed conflicts
> - added patch 14 (convert reset function to QOM)
> - use TYPE_PIIX4_PCI_DEVICE in Malta board (patch 15, Philippe Mathieu-Daudé)
> 
> Changes v1 -> v2:
> - fixed compilation on CentOS 6 (patch 1)
> - automatically create serial/parallel chardevs if not provided (patch 11)
> 
> Hervé Poussineau (16):
>   fdc: move object structures to header file
>   serial/parallel: move object structures to header file
>   mc146818rtc: move structure to header file
>   mc146818rtc: always register rtc to rtc list
>   piix4: rename some variables in realize function
>   piix4: add Reset Control Register
>   piix4: add a i8259 interrupt controller as specified in datasheet
>   piix4: add a i8257 dma controller as specified in datasheet
>   piix4: add a i8254 pit controller as specified in datasheet
>   piix4: add a i8042 keyboard/mouse controller as specified in datasheet
>   piix4: add a floppy controller, 1 parallel port and 2 serial ports
>   piix4: add a mc146818rtc controller as specified in datasheet
>   piix4: add a speaker as specified in datasheet
>   piix4: convert reset function to QOM
>   piix4: rename PIIX4 object to piix4-isa
>   piix4: we can now instanciate a PIIX4 with -device
> 
>  hw/block/fdc.c                 | 102 -----------------------
>  hw/char/parallel.c             |  31 +------
>  hw/char/serial-isa.c           |  13 +--
>  hw/isa/piix4.c                 | 183 ++++++++++++++++++++++++++++++++++++-----
>  hw/mips/mips_malta.c           |  74 +++++++----------
>  hw/ppc/pnv.c                   |   2 +-
>  hw/timer/mc146818rtc.c         |  43 ++--------
>  include/hw/block/fdc.h         | 103 +++++++++++++++++++++++
>  include/hw/char/isa.h          |  50 +++++++++++
>  include/hw/char/serial.h       |   1 -
>  include/hw/i386/pc.h           |   1 -
>  include/hw/isa/isa.h           |   3 +
>  include/hw/timer/mc146818rtc.h |  29 +++++++
>  13 files changed, 389 insertions(+), 246 deletions(-)
>  create mode 100644 include/hw/char/isa.h
> 
> -- 
> 2.11.0

Re: [Qemu-devel] [PATCH v3 00/16] piix4: cleanup and improvements

Posted by Paolo Bonzini 7 years, 9 months ago

On 29/12/2017 15:29, Hervé Poussineau wrote:
> Hi,
> 
> This patchset is a cleanup of the PIIX4 PCI-ISA bridge. Lots of devices
> are moved from MIPS Malta board (which has a PIIX4) to PIIX4, where devices
> belong. This lets us reuse PIIX4 in other machines, while not loosing any
> functionality.
> 
> Last patch allows adding a new PIIX4 device directly from command line.
> Note that this will work only as long no other ISA bus already exist on the
> machine.

Thanks, I think this is the right place to draw the line.  The more
"correct" one would be like pc87312, where the serial and parallel ports
are e.g. also included in the SuperI/O chip, but then you get

            /* FIXME use a qdev chardev prop instead of serial_hds[] */
            chr = serial_hds[i];

so you have to mark the bridge as not user-creatable.

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo

> Hervé
> 
> Changes v2 -> v3:
> - rebased on master and fixed conflicts
> - added patch 14 (convert reset function to QOM)
> - use TYPE_PIIX4_PCI_DEVICE in Malta board (patch 15, Philippe Mathieu-Daudé)
> 
> Changes v1 -> v2:
> - fixed compilation on CentOS 6 (patch 1)
> - automatically create serial/parallel chardevs if not provided (patch 11)
> 
> Hervé Poussineau (16):
>   fdc: move object structures to header file
>   serial/parallel: move object structures to header file
>   mc146818rtc: move structure to header file
>   mc146818rtc: always register rtc to rtc list
>   piix4: rename some variables in realize function
>   piix4: add Reset Control Register
>   piix4: add a i8259 interrupt controller as specified in datasheet
>   piix4: add a i8257 dma controller as specified in datasheet
>   piix4: add a i8254 pit controller as specified in datasheet
>   piix4: add a i8042 keyboard/mouse controller as specified in datasheet
>   piix4: add a floppy controller, 1 parallel port and 2 serial ports
>   piix4: add a mc146818rtc controller as specified in datasheet
>   piix4: add a speaker as specified in datasheet
>   piix4: convert reset function to QOM
>   piix4: rename PIIX4 object to piix4-isa
>   piix4: we can now instanciate a PIIX4 with -device
> 
>  hw/block/fdc.c                 | 102 -----------------------
>  hw/char/parallel.c             |  31 +------
>  hw/char/serial-isa.c           |  13 +--
>  hw/isa/piix4.c                 | 183 ++++++++++++++++++++++++++++++++++++-----
>  hw/mips/mips_malta.c           |  74 +++++++----------
>  hw/ppc/pnv.c                   |   2 +-
>  hw/timer/mc146818rtc.c         |  43 ++--------
>  include/hw/block/fdc.h         | 103 +++++++++++++++++++++++
>  include/hw/char/isa.h          |  50 +++++++++++
>  include/hw/char/serial.h       |   1 -
>  include/hw/i386/pc.h           |   1 -
>  include/hw/isa/isa.h           |   3 +
>  include/hw/timer/mc146818rtc.h |  29 +++++++
>  13 files changed, 389 insertions(+), 246 deletions(-)
>  create mode 100644 include/hw/char/isa.h
>

Re: [Qemu-devel] [PATCH v3 00/16] piix4: cleanup and improvements

Posted by Philippe Mathieu-Daudé 7 years, 9 months ago

>> This patchset is a cleanup of the PIIX4 PCI-ISA bridge. Lots of devices
>> are moved from MIPS Malta board (which has a PIIX4) to PIIX4, where devices
>> belong. This lets us reuse PIIX4 in other machines, while not loosing any
>> functionality.
>>
>> Last patch allows adding a new PIIX4 device directly from command line.
>> Note that this will work only as long no other ISA bus already exist on the
>> machine.
>
> Thanks, I think this is the right place to draw the line.  The more
> "correct" one would be like pc87312, where the serial and parallel ports
> are e.g. also included in the SuperI/O chip, but then you get

Yes, this is how I did in my "remove 'i386/pc.h' dependency" series ...

>
>             /* FIXME use a qdev chardev prop instead of serial_hds[] */
>             chr = serial_hds[i];
>
> so you have to mark the bridge as not user-creatable.

... using qdev chardev and getting ride of this serial_hds[] limitation.

I suggested Hervé we have an IRC meeting today at 7PM GMT+1 to talk
about sharing common efforts.