Skip to contentSkip to conversation

piix4: cleanup and improvements

PATCH Superseded Hervé Poussineau 5 participants 30 messages Dec 29, 2017 – Jan 5, 2018

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Hervé Poussineau

author
#0 lore raw

v3 cover piix4: cleanup and improvements

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
Signature · 1 line
2.11.0
author
#1 lore raw

PATCH v3 1/16 fdc: move object structures to header file

We are now able to embed floppy controllers in another object.

Signed-off-by Hervé Poussineau <[email protected]>
---
 hw/block/fdc.c         | 102 ------------------------------------------------
 include/hw/block/fdc.h | 103 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 103 insertions(+), 102 deletions(-)
hw/block/fdc.c +0 -102
index 7b7dd41296..c81e0313c8 100644
@@ -60,15 +60,8 @@
 #define TYPE_FLOPPY_BUS "floppy-bus"
 #define FLOPPY_BUS(obj) OBJECT_CHECK(FloppyBus, (obj), TYPE_FLOPPY_BUS)
 
-typedef struct FDCtrl FDCtrl;
-typedef struct FDrive FDrive;
 static FDrive *get_drv(FDCtrl *fdctrl, int unit);
 
-typedef struct FloppyBus {
-    BusState bus;
-    FDCtrl *fdc;
-} FloppyBus;
-
 static const TypeInfo floppy_bus_info = {
     .name = TYPE_FLOPPY_BUS,
     .parent = TYPE_BUS,

@@ -178,36 +171,6 @@ static FDriveSize drive_size(FloppyDriveType drive)
 #define FD_SECTOR_SC           2   /* Sector size code */
 #define FD_RESET_SENSEI_COUNT  4   /* Number of sense interrupts on RESET */
 
-/* Floppy disk drive emulation */
-typedef enum FDiskFlags {
-    FDISK_DBL_SIDES  = 0x01,
-} FDiskFlags;
-
-struct FDrive {
-    FDCtrl *fdctrl;
-    BlockBackend *blk;
-    BlockConf *conf;
-    /* Drive status */
-    FloppyDriveType drive;    /* CMOS drive type        */
-    uint8_t perpendicular;    /* 2.88 MB access mode    */
-    /* Position */
-    uint8_t head;
-    uint8_t track;
-    uint8_t sect;
-    /* Media */
-    FloppyDriveType disk;     /* Current disk type      */
-    FDiskFlags flags;
-    uint8_t last_sect;        /* Nb sector per track    */
-    uint8_t max_track;        /* Nb of tracks           */
-    uint16_t bps;             /* Bytes per sector       */
-    uint8_t ro;               /* Is read-only           */
-    uint8_t media_changed;    /* Is media changed       */
-    uint8_t media_rate;       /* Data rate of medium    */
-
-    bool media_validated;     /* Have we validated the media? */
-};
-
-
 static FloppyDriveType get_fallback_drive_type(FDrive *drv);
 
 /* Hack: FD_SEEK is expected to work on empty drives. However, QEMU

@@ -819,60 +782,6 @@ enum {
 #define FD_MULTI_TRACK(state) ((state) & FD_STATE_MULTI)
 #define FD_FORMAT_CMD(state) ((state) & FD_STATE_FORMAT)
 
-struct FDCtrl {
-    MemoryRegion iomem;
-    qemu_irq irq;
-    /* Controller state */
-    QEMUTimer *result_timer;
-    int dma_chann;
-    uint8_t phase;
-    IsaDma *dma;
-    /* Controller's identification */
-    uint8_t version;
-    /* HW */
-    uint8_t sra;
-    uint8_t srb;
-    uint8_t dor;
-    uint8_t dor_vmstate; /* only used as temp during vmstate */
-    uint8_t tdr;
-    uint8_t dsr;
-    uint8_t msr;
-    uint8_t cur_drv;
-    uint8_t status0;
-    uint8_t status1;
-    uint8_t status2;
-    /* Command FIFO */
-    uint8_t *fifo;
-    int32_t fifo_size;
-    uint32_t data_pos;
-    uint32_t data_len;
-    uint8_t data_state;
-    uint8_t data_dir;
-    uint8_t eot; /* last wanted sector */
-    /* States kept only to be returned back */
-    /* precompensation */
-    uint8_t precomp_trk;
-    uint8_t config;
-    uint8_t lock;
-    /* Power down config (also with status regB access mode */
-    uint8_t pwrd;
-    /* Floppy drives */
-    FloppyBus bus;
-    uint8_t num_floppies;
-    FDrive drives[MAX_FD];
-    struct {
-        BlockBackend *blk;
-        FloppyDriveType type;
-    } qdev_for_drives[MAX_FD];
-    int reset_sensei;
-    uint32_t check_media_rate;
-    FloppyDriveType fallback; /* type=auto failure fallback */
-    /* Timers state */
-    uint8_t timer0;
-    uint8_t timer1;
-    PortioList portio_list;
-};
-
 static FloppyDriveType get_fallback_drive_type(FDrive *drv)
 {
     return drv->fdctrl->fallback;

@@ -891,17 +800,6 @@ typedef struct FDCtrlSysBus {
 
 #define ISA_FDC(obj) OBJECT_CHECK(FDCtrlISABus, (obj), TYPE_ISA_FDC)
 
-typedef struct FDCtrlISABus {
-    ISADevice parent_obj;
-
-    uint32_t iobase;
-    uint32_t irq;
-    uint32_t dma;
-    struct FDCtrl state;
-    int32_t bootindexA;
-    int32_t bootindexB;
-} FDCtrlISABus;
-
 static uint32_t fdctrl_read (void *opaque, uint32_t reg)
 {
     FDCtrl *fdctrl = opaque;
include/hw/block/fdc.h +103 -0

115 lines not on this page · raw message

Signature · 1 line
2.11.0
#2 lore raw

Re: [Qemu-devel] [PATCH v3 01/16] fdc: move object structures to header file

Quoted 2 lines · Hervé Poussineau

We are now able to embed floppy controllers in another object.

Hi Hervé,

Are you sure we need to move all the struct definitions to the header file?

I looked at patch 11/16 and it seems only FDCtrlISABus definition is needed. And also only the typedef is needed and not the fields.

It may worth a look also patches 2/16 and 3/16.

Thanks,
Marcel

Quoted 263 lines
Signed-off-by Hervé Poussineau <[email protected]>
---
  hw/block/fdc.c         | 102 ------------------------------------------------
  include/hw/block/fdc.h | 103 +++++++++++++++++++++++++++++++++++++++++++++++++
  2 files changed, 103 insertions(+), 102 deletions(-)
hw/block/fdc.c +0 -102
index 7b7dd41296..c81e0313c8 100644
@@ -60,15 +60,8 @@
  #define TYPE_FLOPPY_BUS "floppy-bus"
  #define FLOPPY_BUS(obj) OBJECT_CHECK(FloppyBus, (obj), TYPE_FLOPPY_BUS)
 
-typedef struct FDCtrl FDCtrl;
-typedef struct FDrive FDrive;
  static FDrive *get_drv(FDCtrl *fdctrl, int unit);
 
-typedef struct FloppyBus {
-    BusState bus;
-    FDCtrl *fdc;
-} FloppyBus;
-
  static const TypeInfo floppy_bus_info = {
      .name = TYPE_FLOPPY_BUS,
      .parent = TYPE_BUS,

@@ -178,36 +171,6 @@ static FDriveSize drive_size(FloppyDriveType drive)
  #define FD_SECTOR_SC           2   /* Sector size code */
  #define FD_RESET_SENSEI_COUNT  4   /* Number of sense interrupts on RESET */
 
-/* Floppy disk drive emulation */
-typedef enum FDiskFlags {
-    FDISK_DBL_SIDES  = 0x01,
-} FDiskFlags;
-
-struct FDrive {
-    FDCtrl *fdctrl;
-    BlockBackend *blk;
-    BlockConf *conf;
-    /* Drive status */
-    FloppyDriveType drive;    /* CMOS drive type        */
-    uint8_t perpendicular;    /* 2.88 MB access mode    */
-    /* Position */
-    uint8_t head;
-    uint8_t track;
-    uint8_t sect;
-    /* Media */
-    FloppyDriveType disk;     /* Current disk type      */
-    FDiskFlags flags;
-    uint8_t last_sect;        /* Nb sector per track    */
-    uint8_t max_track;        /* Nb of tracks           */
-    uint16_t bps;             /* Bytes per sector       */
-    uint8_t ro;               /* Is read-only           */
-    uint8_t media_changed;    /* Is media changed       */
-    uint8_t media_rate;       /* Data rate of medium    */
-
-    bool media_validated;     /* Have we validated the media? */
-};
-
-
  static FloppyDriveType get_fallback_drive_type(FDrive *drv);
 
  /* Hack: FD_SEEK is expected to work on empty drives. However, QEMU

@@ -819,60 +782,6 @@ enum {
  #define FD_MULTI_TRACK(state) ((state) & FD_STATE_MULTI)
  #define FD_FORMAT_CMD(state) ((state) & FD_STATE_FORMAT)
 
-struct FDCtrl {
-    MemoryRegion iomem;
-    qemu_irq irq;
-    /* Controller state */
-    QEMUTimer *result_timer;
-    int dma_chann;
-    uint8_t phase;
-    IsaDma *dma;
-    /* Controller's identification */
-    uint8_t version;
-    /* HW */
-    uint8_t sra;
-    uint8_t srb;
-    uint8_t dor;
-    uint8_t dor_vmstate; /* only used as temp during vmstate */
-    uint8_t tdr;
-    uint8_t dsr;
-    uint8_t msr;
-    uint8_t cur_drv;
-    uint8_t status0;
-    uint8_t status1;
-    uint8_t status2;
-    /* Command FIFO */
-    uint8_t *fifo;
-    int32_t fifo_size;
-    uint32_t data_pos;
-    uint32_t data_len;
-    uint8_t data_state;
-    uint8_t data_dir;
-    uint8_t eot; /* last wanted sector */
-    /* States kept only to be returned back */
-    /* precompensation */
-    uint8_t precomp_trk;
-    uint8_t config;
-    uint8_t lock;
-    /* Power down config (also with status regB access mode */
-    uint8_t pwrd;
-    /* Floppy drives */
-    FloppyBus bus;
-    uint8_t num_floppies;
-    FDrive drives[MAX_FD];
-    struct {
-        BlockBackend *blk;
-        FloppyDriveType type;
-    } qdev_for_drives[MAX_FD];
-    int reset_sensei;
-    uint32_t check_media_rate;
-    FloppyDriveType fallback; /* type=auto failure fallback */
-    /* Timers state */
-    uint8_t timer0;
-    uint8_t timer1;
-    PortioList portio_list;
-};
-
  static FloppyDriveType get_fallback_drive_type(FDrive *drv)
  {
      return drv->fdctrl->fallback;

@@ -891,17 +800,6 @@ typedef struct FDCtrlSysBus {
 
  #define ISA_FDC(obj) OBJECT_CHECK(FDCtrlISABus, (obj), TYPE_ISA_FDC)
 
-typedef struct FDCtrlISABus {
-    ISADevice parent_obj;
-
-    uint32_t iobase;
-    uint32_t irq;
-    uint32_t dma;
-    struct FDCtrl state;
-    int32_t bootindexA;
-    int32_t bootindexB;
-} FDCtrlISABus;
-
  static uint32_t fdctrl_read (void *opaque, uint32_t reg)
  {
      FDCtrl *fdctrl = opaque;
include/hw/block/fdc.h +103 -0
index 1749dabf25..d076b2fc1a 100644
@@ -2,12 +2,115 @@
  #define HW_FDC_H
 
  #include "qemu-common.h"
+#include "hw/block/block.h"
+#include "hw/isa/isa.h"
 
  /* fdc.c */
  #define MAX_FD 2
 
+typedef struct FDCtrl FDCtrl;
+
+/* Floppy disk drive emulation */
+typedef enum FDiskFlags {
+    FDISK_DBL_SIDES  = 0x01,
+} FDiskFlags;
+
+typedef struct FDrive {
+    FDCtrl *fdctrl;
+    BlockBackend *blk;
+    BlockConf *conf;
+    /* Drive status */
+    FloppyDriveType drive;    /* CMOS drive type        */
+    uint8_t perpendicular;    /* 2.88 MB access mode    */
+    /* Position */
+    uint8_t head;
+    uint8_t track;
+    uint8_t sect;
+    /* Media */
+    FloppyDriveType disk;     /* Current disk type      */
+    FDiskFlags flags;
+    uint8_t last_sect;        /* Nb sector per track    */
+    uint8_t max_track;        /* Nb of tracks           */
+    uint16_t bps;             /* Bytes per sector       */
+    uint8_t ro;               /* Is read-only           */
+    uint8_t media_changed;    /* Is media changed       */
+    uint8_t media_rate;       /* Data rate of medium    */
+
+    bool media_validated;     /* Have we validated the media? */
+} FDrive;
+
+typedef struct FloppyBus {
+    BusState bus;
+    FDCtrl *fdc;
+} FloppyBus;
+
+struct FDCtrl {
+    MemoryRegion iomem;
+    qemu_irq irq;
+    /* Controller state */
+    QEMUTimer *result_timer;
+    int dma_chann;
+    uint8_t phase;
+    IsaDma *dma;
+    /* Controller's identification */
+    uint8_t version;
+    /* HW */
+    uint8_t sra;
+    uint8_t srb;
+    uint8_t dor;
+    uint8_t dor_vmstate; /* only used as temp during vmstate */
+    uint8_t tdr;
+    uint8_t dsr;
+    uint8_t msr;
+    uint8_t cur_drv;
+    uint8_t status0;
+    uint8_t status1;
+    uint8_t status2;
+    /* Command FIFO */
+    uint8_t *fifo;
+    int32_t fifo_size;
+    uint32_t data_pos;
+    uint32_t data_len;
+    uint8_t data_state;
+    uint8_t data_dir;
+    uint8_t eot; /* last wanted sector */
+    /* States kept only to be returned back */
+    /* precompensation */
+    uint8_t precomp_trk;
+    uint8_t config;
+    uint8_t lock;
+    /* Power down config (also with status regB access mode */
+    uint8_t pwrd;
+    /* Floppy drives */
+    FloppyBus bus;
+    uint8_t num_floppies;
+    FDrive drives[MAX_FD];
+    struct {
+        BlockBackend *blk;
+        FloppyDriveType type;
+    } qdev_for_drives[MAX_FD];
+    int reset_sensei;
+    uint32_t check_media_rate;
+    FloppyDriveType fallback; /* type=auto failure fallback */
+    /* Timers state */
+    uint8_t timer0;
+    uint8_t timer1;
+    PortioList portio_list;
+};
+
  #define TYPE_ISA_FDC "isa-fdc"
 
+typedef struct FDCtrlISABus {
+    ISADevice parent_obj;
+
+    uint32_t iobase;
+    uint32_t irq;
+    uint32_t dma;
+    struct FDCtrl state;
+    int32_t bootindexA;
+    int32_t bootindexB;
+} FDCtrlISABus;
+
  ISADevice *fdctrl_init_isa(ISABus *bus, DriveInfo **fds);
  void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
                          hwaddr mmio_base, DriveInfo **fds);
author
#3 lore raw

Re: [Qemu-devel] [PATCH v3 01/16] fdc: move object structures to header file

Le 04/01/2018 à 14:11, Marcel Apfelbaum a écrit :

Quoted 12 lines · Hervé Poussineau
Quoted 2 lines

We are now able to embed floppy controllers in another object.

Hi Hervé,

Are you sure we need to move all the struct definitions to the header file?

I looked at patch 11/16 and it seems only FDCtrlISABus definition is needed. And also only the typedef is needed and not the fields.

It may worth a look also patches 2/16 and 3/16.

Structure FDCtrlISABus contains a structure FDCtrl (state) which contains a structure FloppyBus and FDrive. So I think I need to move all these struct definitions to the header file, and that a typedef is not enough.

On patch 11, I include a FDCtrlISABus structure in the PIIX4State structure. So, FDCtrlISABus structure must be shared between fdc.c and piix4.c That's what I do when I move all these structures to fdc.h

Do you see another possibility?

Regards,

Hervé

Quoted 258 lines

Thanks,
Marcel

Quoted 252 lines
Signed-off-by Hervé Poussineau <[email protected]>

---
  hw/block/fdc.c         | 102 ------------------------------------------------ include/hw/block/fdc.h | 103 +++++++++++++++++++++++++++++++++++++++++++++++++

  2 files changed, 103 insertions(+), 102 deletions(-)
hw/block/fdc.c +0 -102
index 7b7dd41296..c81e0313c8 100644
@@ -60,15 +60,8 @@
   #define TYPE_FLOPPY_BUS "floppy-bus"
   #define FLOPPY_BUS(obj) OBJECT_CHECK(FloppyBus, (obj), TYPE_FLOPPY_BUS)
-typedef struct FDCtrl FDCtrl;
-typedef struct FDrive FDrive;
   static FDrive *get_drv(FDCtrl *fdctrl, int unit);
-typedef struct FloppyBus {
-    BusState bus;
-    FDCtrl *fdc;
-} FloppyBus;
-
   static const TypeInfo floppy_bus_info = {
       .name = TYPE_FLOPPY_BUS,
       .parent = TYPE_BUS,

@@ -178,36 +171,6 @@ static FDriveSize drive_size(FloppyDriveType drive)
   #define FD_SECTOR_SC           2   /* Sector size code */
   #define FD_RESET_SENSEI_COUNT  4   /* Number of sense interrupts on RESET */
-/* Floppy disk drive emulation */
-typedef enum FDiskFlags {
-    FDISK_DBL_SIDES  = 0x01,
-} FDiskFlags;
-
-struct FDrive {
-    FDCtrl *fdctrl;
-    BlockBackend *blk;
-    BlockConf *conf;
-    /* Drive status */
-    FloppyDriveType drive;    /* CMOS drive type        */
-    uint8_t perpendicular;    /* 2.88 MB access mode    */
-    /* Position */
-    uint8_t head;
-    uint8_t track;
-    uint8_t sect;
-    /* Media */
-    FloppyDriveType disk;     /* Current disk type      */
-    FDiskFlags flags;
-    uint8_t last_sect;        /* Nb sector per track    */
-    uint8_t max_track;        /* Nb of tracks           */
-    uint16_t bps;             /* Bytes per sector       */
-    uint8_t ro;               /* Is read-only           */
-    uint8_t media_changed;    /* Is media changed       */
-    uint8_t media_rate;       /* Data rate of medium    */
-
-    bool media_validated;     /* Have we validated the media? */
-};
-
-
   static FloppyDriveType get_fallback_drive_type(FDrive *drv);
   /* Hack: FD_SEEK is expected to work on empty drives. However, QEMU

@@ -819,60 +782,6 @@ enum {
   #define FD_MULTI_TRACK(state) ((state) & FD_STATE_MULTI)
   #define FD_FORMAT_CMD(state) ((state) & FD_STATE_FORMAT)
-struct FDCtrl {
-    MemoryRegion iomem;
-    qemu_irq irq;
-    /* Controller state */
-    QEMUTimer *result_timer;
-    int dma_chann;
-    uint8_t phase;
-    IsaDma *dma;
-    /* Controller's identification */
-    uint8_t version;
-    /* HW */
-    uint8_t sra;
-    uint8_t srb;
-    uint8_t dor;
-    uint8_t dor_vmstate; /* only used as temp during vmstate */
-    uint8_t tdr;
-    uint8_t dsr;
-    uint8_t msr;
-    uint8_t cur_drv;
-    uint8_t status0;
-    uint8_t status1;
-    uint8_t status2;
-    /* Command FIFO */
-    uint8_t *fifo;
-    int32_t fifo_size;
-    uint32_t data_pos;
-    uint32_t data_len;
-    uint8_t data_state;
-    uint8_t data_dir;
-    uint8_t eot; /* last wanted sector */
-    /* States kept only to be returned back */
-    /* precompensation */
-    uint8_t precomp_trk;
-    uint8_t config;
-    uint8_t lock;
-    /* Power down config (also with status regB access mode */
-    uint8_t pwrd;
-    /* Floppy drives */
-    FloppyBus bus;
-    uint8_t num_floppies;
-    FDrive drives[MAX_FD];
-    struct {
-        BlockBackend *blk;
-        FloppyDriveType type;
-    } qdev_for_drives[MAX_FD];
-    int reset_sensei;
-    uint32_t check_media_rate;
-    FloppyDriveType fallback; /* type=auto failure fallback */
-    /* Timers state */
-    uint8_t timer0;
-    uint8_t timer1;
-    PortioList portio_list;
-};
-
   static FloppyDriveType get_fallback_drive_type(FDrive *drv)
   {
       return drv->fdctrl->fallback;

@@ -891,17 +800,6 @@ typedef struct FDCtrlSysBus {
   #define ISA_FDC(obj) OBJECT_CHECK(FDCtrlISABus, (obj), TYPE_ISA_FDC)
-typedef struct FDCtrlISABus {
-    ISADevice parent_obj;
-
-    uint32_t iobase;
-    uint32_t irq;
-    uint32_t dma;
-    struct FDCtrl state;
-    int32_t bootindexA;
-    int32_t bootindexB;
-} FDCtrlISABus;
-
   static uint32_t fdctrl_read (void *opaque, uint32_t reg)
   {
       FDCtrl *fdctrl = opaque;
include/hw/block/fdc.h +103 -0
index 1749dabf25..d076b2fc1a 100644
@@ -2,12 +2,115 @@
   #define HW_FDC_H
   #include "qemu-common.h"
+#include "hw/block/block.h"
+#include "hw/isa/isa.h"
   /* fdc.c */
   #define MAX_FD 2
+typedef struct FDCtrl FDCtrl;
+
+/* Floppy disk drive emulation */
+typedef enum FDiskFlags {
+    FDISK_DBL_SIDES  = 0x01,
+} FDiskFlags;
+
+typedef struct FDrive {
+    FDCtrl *fdctrl;
+    BlockBackend *blk;
+    BlockConf *conf;
+    /* Drive status */
+    FloppyDriveType drive;    /* CMOS drive type        */
+    uint8_t perpendicular;    /* 2.88 MB access mode    */
+    /* Position */
+    uint8_t head;
+    uint8_t track;
+    uint8_t sect;
+    /* Media */
+    FloppyDriveType disk;     /* Current disk type      */
+    FDiskFlags flags;
+    uint8_t last_sect;        /* Nb sector per track    */
+    uint8_t max_track;        /* Nb of tracks           */
+    uint16_t bps;             /* Bytes per sector       */
+    uint8_t ro;               /* Is read-only           */
+    uint8_t media_changed;    /* Is media changed       */
+    uint8_t media_rate;       /* Data rate of medium    */
+
+    bool media_validated;     /* Have we validated the media? */
+} FDrive;
+
+typedef struct FloppyBus {
+    BusState bus;
+    FDCtrl *fdc;
+} FloppyBus;
+
+struct FDCtrl {
+    MemoryRegion iomem;
+    qemu_irq irq;
+    /* Controller state */
+    QEMUTimer *result_timer;
+    int dma_chann;
+    uint8_t phase;
+    IsaDma *dma;
+    /* Controller's identification */
+    uint8_t version;
+    /* HW */
+    uint8_t sra;
+    uint8_t srb;
+    uint8_t dor;
+    uint8_t dor_vmstate; /* only used as temp during vmstate */
+    uint8_t tdr;
+    uint8_t dsr;
+    uint8_t msr;
+    uint8_t cur_drv;
+    uint8_t status0;
+    uint8_t status1;
+    uint8_t status2;
+    /* Command FIFO */
+    uint8_t *fifo;
+    int32_t fifo_size;
+    uint32_t data_pos;
+    uint32_t data_len;
+    uint8_t data_state;
+    uint8_t data_dir;
+    uint8_t eot; /* last wanted sector */
+    /* States kept only to be returned back */
+    /* precompensation */
+    uint8_t precomp_trk;
+    uint8_t config;
+    uint8_t lock;
+    /* Power down config (also with status regB access mode */
+    uint8_t pwrd;
+    /* Floppy drives */
+    FloppyBus bus;
+    uint8_t num_floppies;
+    FDrive drives[MAX_FD];
+    struct {
+        BlockBackend *blk;
+        FloppyDriveType type;
+    } qdev_for_drives[MAX_FD];
+    int reset_sensei;
+    uint32_t check_media_rate;
+    FloppyDriveType fallback; /* type=auto failure fallback */
+    /* Timers state */
+    uint8_t timer0;
+    uint8_t timer1;
+    PortioList portio_list;
+};
+
   #define TYPE_ISA_FDC "isa-fdc"
+typedef struct FDCtrlISABus {
+    ISADevice parent_obj;
+
+    uint32_t iobase;
+    uint32_t irq;
+    uint32_t dma;
+    struct FDCtrl state;
+    int32_t bootindexA;
+    int32_t bootindexB;
+} FDCtrlISABus;
+
   ISADevice *fdctrl_init_isa(ISABus *bus, DriveInfo **fds);
   void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
                           hwaddr mmio_base, DriveInfo **fds);
author
#4 lore raw

PATCH v3 2/16 serial/parallel: move object structures to header file

In reply to Hervé Poussineau

We are now able to embed serial/parallel ports in another object.

Signed-off-by Hervé Poussineau <[email protected]>
---
 hw/char/parallel.c       | 31 +-----------------------------
 hw/char/serial-isa.c     | 13 +------------
 hw/ppc/pnv.c             |  2 +-
 include/hw/char/isa.h    | 50 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/char/serial.h |  1 -
 5 files changed, 53 insertions(+), 44 deletions(-)
 create mode 100644 include/hw/char/isa.h
hw/char/parallel.c +1 -30
index f79dc76543..6b36d425ff 100644
@@ -28,6 +28,7 @@
 #include "chardev/char-parallel.h"
 #include "chardev/char-fe.h"
 #include "hw/isa/isa.h"
+#include "hw/char/isa.h"
 #include "hw/i386/pc.h"
 #include "sysemu/sysemu.h"
 

@@ -67,36 +68,6 @@
 
 #define PARA_CTR_SIGNAL (PARA_CTR_SELECT|PARA_CTR_INIT|PARA_CTR_AUTOLF|PARA_CTR_STROBE)
 
-typedef struct ParallelState {
-    MemoryRegion iomem;
-    uint8_t dataw;
-    uint8_t datar;
-    uint8_t status;
-    uint8_t control;
-    qemu_irq irq;
-    int irq_pending;
-    CharBackend chr;
-    int hw_driver;
-    int epp_timeout;
-    uint32_t last_read_offset; /* For debugging */
-    /* Memory-mapped interface */
-    int it_shift;
-    PortioList portio_list;
-} ParallelState;
-
-#define TYPE_ISA_PARALLEL "isa-parallel"
-#define ISA_PARALLEL(obj) \
-    OBJECT_CHECK(ISAParallelState, (obj), TYPE_ISA_PARALLEL)
-
-typedef struct ISAParallelState {
-    ISADevice parent_obj;
-
-    uint32_t index;
-    uint32_t iobase;
-    uint32_t isairq;
-    ParallelState state;
-} ISAParallelState;
-
 static void parallel_update_irq(ParallelState *s)
 {
     if (s->irq_pending)
hw/char/serial-isa.c +1 -12
index d7c5cc11fe..2c6cf81790 100644
@@ -26,18 +26,7 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "hw/char/serial.h"
-#include "hw/isa/isa.h"
-
-#define ISA_SERIAL(obj) OBJECT_CHECK(ISASerialState, (obj), TYPE_ISA_SERIAL)
-
-typedef struct ISASerialState {
-    ISADevice parent_obj;
-
-    uint32_t index;
-    uint32_t iobase;
-    uint32_t isairq;
-    SerialState state;
-} ISASerialState;
+#include "hw/char/isa.h"
 
 static const int isa_serial_io[MAX_SERIAL_PORTS] = {
     0x3f8, 0x2f8, 0x3e8, 0x2e8
hw/ppc/pnv.c +1 -1
index 94ffc8e137..bf518d92a2 100644
@@ -41,7 +41,7 @@
 #include "hw/ppc/pnv_xscom.h"
 
 #include "hw/isa/isa.h"
-#include "hw/char/serial.h"
+#include "hw/char/isa.h"
 #include "hw/timer/mc146818rtc.h"
 
 #include <libfdt.h>
include/hw/char/isa.h new file +50 -0

50 lines not on this page · raw message

include/hw/char/serial.h +0 -1
index c4daf11a14..ec7da3d7f6 100644
@@ -95,7 +95,6 @@ SerialState *serial_mm_init(MemoryRegion *address_space,
                             Chardev *chr, enum device_endian end);
 
 /* serial-isa.c */
-#define TYPE_ISA_SERIAL "isa-serial"
 void serial_hds_isa_init(ISABus *bus, int from, int to);
 
 #endif
Signature · 1 line
2.11.0
author
#5 lore raw

PATCH v3 3/16 mc146818rtc: move structure to header file

In reply to Hervé Poussineau

We are now able to embed a timer in another object.

Signed-off-by Hervé Poussineau <[email protected]>
---
 hw/timer/mc146818rtc.c         | 30 ------------------------------
 include/hw/timer/mc146818rtc.h | 29 +++++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 30 deletions(-)
hw/timer/mc146818rtc.c +0 -30
index 35a05a64cc..3e8c0b7d33 100644
@@ -62,36 +62,6 @@
 #define RTC_CLOCK_RATE            32768
 #define UIP_HOLD_LENGTH           (8 * NANOSECONDS_PER_SECOND / 32768)
 
-#define MC146818_RTC(obj) OBJECT_CHECK(RTCState, (obj), TYPE_MC146818_RTC)
-
-typedef struct RTCState {
-    ISADevice parent_obj;
-
-    MemoryRegion io;
-    uint8_t cmos_data[128];
-    uint8_t cmos_index;
-    int32_t base_year;
-    uint64_t base_rtc;
-    uint64_t last_update;
-    int64_t offset;
-    qemu_irq irq;
-    int it_shift;
-    /* periodic timer */
-    QEMUTimer *periodic_timer;
-    int64_t next_periodic_time;
-    /* update-ended timer */
-    QEMUTimer *update_timer;
-    uint64_t next_alarm_time;
-    uint16_t irq_reinject_on_ack_count;
-    uint32_t irq_coalesced;
-    uint32_t period;
-    QEMUTimer *coalesced_timer;
-    Notifier clock_reset_notifier;
-    LostTickPolicy lost_tick_policy;
-    Notifier suspend_notifier;
-    QLIST_ENTRY(RTCState) link;
-} RTCState;
-
 static void rtc_set_time(RTCState *s);
 static void rtc_update_time(RTCState *s);
 static void rtc_set_cmos(RTCState *s, const struct tm *tm);
include/hw/timer/mc146818rtc.h +29 -0
index fe6ed63f71..7385622604 100644
@@ -5,6 +5,35 @@
 #include "hw/timer/mc146818rtc_regs.h"
 
 #define TYPE_MC146818_RTC "mc146818rtc"
+#define MC146818_RTC(obj) OBJECT_CHECK(RTCState, (obj), TYPE_MC146818_RTC)
+
+typedef struct RTCState {
+    ISADevice parent_obj;
+
+    MemoryRegion io;
+    uint8_t cmos_data[128];
+    uint8_t cmos_index;
+    int32_t base_year;
+    uint64_t base_rtc;
+    uint64_t last_update;
+    int64_t offset;
+    qemu_irq irq;
+    int it_shift;
+    /* periodic timer */
+    QEMUTimer *periodic_timer;
+    int64_t next_periodic_time;
+    /* update-ended timer */
+    QEMUTimer *update_timer;
+    uint64_t next_alarm_time;
+    uint16_t irq_reinject_on_ack_count;
+    uint32_t irq_coalesced;
+    uint32_t period;
+    QEMUTimer *coalesced_timer;
+    Notifier clock_reset_notifier;
+    LostTickPolicy lost_tick_policy;
+    Notifier suspend_notifier;
+    QLIST_ENTRY(RTCState) link;
+} RTCState;
 
 ISADevice *mc146818_rtc_init(ISABus *bus, int base_year,
                              qemu_irq intercept_irq);
Signature · 1 line
2.11.0
author
#6 lore raw

PATCH v3 4/16 mc146818rtc: always register rtc to rtc list

In reply to Hervé Poussineau

We are not required anymore to use rtc_init() function.

Signed-off-by Hervé Poussineau <[email protected]>
---
 hw/timer/mc146818rtc.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)
hw/timer/mc146818rtc.c +8 -5
index 3e8c0b7d33..0b0da691cc 100644
@@ -905,6 +905,13 @@ static void rtc_get_date(Object *obj, struct tm *current_tm, Error **errp)
     rtc_get_time(s, current_tm);
 }
 
+static int rtc_initfn(DeviceState *dev)
+{
+    RTCState *s = MC146818_RTC(dev);
+    QLIST_INSERT_HEAD(&rtc_devices, s, link);
+    return 0;
+}
+
 static void rtc_realizefn(DeviceState *dev, Error **errp)
 {
     ISADevice *isadev = ISA_DEVICE(dev);

@@ -973,11 +980,9 @@ ISADevice *mc146818_rtc_init(ISABus *bus, int base_year, qemu_irq intercept_irq)
 {
     DeviceState *dev;
     ISADevice *isadev;
-    RTCState *s;
 
     isadev = isa_create(bus, TYPE_MC146818_RTC);
     dev = DEVICE(isadev);
-    s = MC146818_RTC(isadev);
     qdev_prop_set_int32(dev, "base_year", base_year);
     qdev_init_nofail(dev);
     if (intercept_irq) {

@@ -985,7 +990,6 @@ ISADevice *mc146818_rtc_init(ISABus *bus, int base_year, qemu_irq intercept_irq)
     } else {
         isa_connect_gpio_out(isadev, 0, RTC_ISA_IRQ);
     }
-    QLIST_INSERT_HEAD(&rtc_devices, s, link);
 
     return isadev;
 }

@@ -1012,12 +1016,11 @@ static void rtc_class_initfn(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
+    dc->init = rtc_initfn;
     dc->realize = rtc_realizefn;
     dc->reset = rtc_resetdev;
     dc->vmsd = &vmstate_rtc;
     dc->props = mc146818rtc_properties;
-    /* Reason: needs to be wired up by rtc_init() */
-    dc->user_creatable = false;
 }
 
 static void rtc_finalize(Object *obj)
Signature · 1 line
2.11.0
#7 lore raw

Re: [Qemu-devel] [PATCH v3 04/16] mc146818rtc: always register rtc to rtc list

Quoted 46 lines · Hervé Poussineau

We are not required anymore to use rtc_init() function.

Signed-off-by Hervé Poussineau <[email protected]>
---
  hw/timer/mc146818rtc.c | 13 ++++++++-----
  1 file changed, 8 insertions(+), 5 deletions(-)
hw/timer/mc146818rtc.c +7 -3
index 3e8c0b7d33..0b0da691cc 100644
@@ -905,6 +905,13 @@ static void rtc_get_date(Object *obj, struct tm *current_tm, Error **errp)
      rtc_get_time(s, current_tm);
  }
 
+static int rtc_initfn(DeviceState *dev)
+{
+    RTCState *s = MC146818_RTC(dev);
+    QLIST_INSERT_HEAD(&rtc_devices, s, link);
+    return 0;
+}
+
  static void rtc_realizefn(DeviceState *dev, Error **errp)
  {
      ISADevice *isadev = ISA_DEVICE(dev);

@@ -973,11 +980,9 @@ ISADevice *mc146818_rtc_init(ISABus *bus, int base_year, qemu_irq intercept_irq)
  {
      DeviceState *dev;
      ISADevice *isadev;
-    RTCState *s;
 
      isadev = isa_create(bus, TYPE_MC146818_RTC);
      dev = DEVICE(isadev);
-    s = MC146818_RTC(isadev);
      qdev_prop_set_int32(dev, "base_year", base_year);
      qdev_init_nofail(dev);
      if (intercept_irq) {

@@ -985,7 +990,6 @@ ISADevice *mc146818_rtc_init(ISABus *bus, int base_year, qemu_irq intercept_irq)
      } else {
          isa_connect_gpio_out(isadev, 0, RTC_ISA_IRQ);
      }
-    QLIST_INSERT_HEAD(&rtc_devices, s, link);
 
      return isadev;
  }

hw/timer/mc146818rtc.c static void rtc_class_initfn(ObjectClass *klass, void *data) · line 1019

Quoted 4 lines
hw/timer/mc146818rtc.c +1 -0
@@ -1012,12 +1016,11 @@ static void rtc_class_initfn(ObjectClass *klass, void *data)
  {
      DeviceClass *dc = DEVICE_CLASS(klass);
 
+    dc->init = rtc_initfn;

I am not sure you want to use dc->init function.

I think you need TypeInfo's instance_init.

Thanks,
Marcel

Quoted 10 lines
hw/timer/mc146818rtc.c +0 -2
      dc->realize = rtc_realizefn;
      dc->reset = rtc_resetdev;
      dc->vmsd = &vmstate_rtc;
      dc->props = mc146818rtc_properties;
-    /* Reason: needs to be wired up by rtc_init() */
-    dc->user_creatable = false;
  }
 
  static void rtc_finalize(Object *obj)
author
#8 lore raw

Re: [Qemu-devel] [PATCH v3 04/16] mc146818rtc: always register rtc to rtc list

Le 04/01/2018 à 15:30, Marcel Apfelbaum a écrit :

Quoted 50 lines · Hervé Poussineau
Quoted 43 lines

We are not required anymore to use rtc_init() function.

Signed-off-by Hervé Poussineau <[email protected]>

---
  hw/timer/mc146818rtc.c | 13 ++++++++-----

  1 file changed, 8 insertions(+), 5 deletions(-)
hw/timer/mc146818rtc.c +7 -3
index 3e8c0b7d33..0b0da691cc 100644
@@ -905,6 +905,13 @@ static void rtc_get_date(Object *obj, struct tm *current_tm, Error **errp)
       rtc_get_time(s, current_tm);
   }
+static int rtc_initfn(DeviceState *dev)
+{
+    RTCState *s = MC146818_RTC(dev);
+    QLIST_INSERT_HEAD(&rtc_devices, s, link);
+    return 0;
+}
+
   static void rtc_realizefn(DeviceState *dev, Error **errp)
   {
       ISADevice *isadev = ISA_DEVICE(dev);

@@ -973,11 +980,9 @@ ISADevice *mc146818_rtc_init(ISABus *bus, int base_year, qemu_irq intercept_irq)
   {
       DeviceState *dev;
       ISADevice *isadev;
-    RTCState *s;
       isadev = isa_create(bus, TYPE_MC146818_RTC);
       dev = DEVICE(isadev);
-    s = MC146818_RTC(isadev);
       qdev_prop_set_int32(dev, "base_year", base_year);
       qdev_init_nofail(dev);
       if (intercept_irq) {

@@ -985,7 +990,6 @@ ISADevice *mc146818_rtc_init(ISABus *bus, int base_year, qemu_irq intercept_irq)
       } else {
           isa_connect_gpio_out(isadev, 0, RTC_ISA_IRQ);
       }
-    QLIST_INSERT_HEAD(&rtc_devices, s, link);
       return isadev;
   }

hw/timer/mc146818rtc.c static void rtc_class_initfn(ObjectClass *klass, void *data) · line 1018

Quoted 3 lines
hw/timer/mc146818rtc.c +1 -0
@@ -1012,12 +1016,11 @@ static void rtc_class_initfn(ObjectClass *klass, void *data)
   {
       DeviceClass *dc = DEVICE_CLASS(klass);
+    dc->init = rtc_initfn;

I am not sure you want to use dc->init function.

Quoted 1 line

I think you need TypeInfo's instance_init.

OK, I will move list insertion to existing function rtc_realizefn.

Quoted 15 lines

Thanks,
Marcel

Quoted 9 lines
hw/timer/mc146818rtc.c +0 -2
       dc->realize = rtc_realizefn;
       dc->reset = rtc_resetdev;
       dc->vmsd = &vmstate_rtc;
       dc->props = mc146818rtc_properties;
-    /* Reason: needs to be wired up by rtc_init() */
-    dc->user_creatable = false;
   }
   static void rtc_finalize(Object *obj)
author
#9 lore raw

PATCH v3 5/16 piix4: rename some variables in realize function

In reply to Hervé Poussineau

PIIX4 structure is now 's'
PCI device is now 'pci'
DeviceState is now 'dev'

Signed-off-by Hervé Poussineau <[email protected]>
---
 hw/isa/piix4.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
hw/isa/piix4.c +7 -6
index 6b8bc3faf0..4f476dc7e6 100644
@@ -87,16 +87,17 @@ static const VMStateDescription vmstate_piix4 = {
     }
 };
 
-static void piix4_realize(PCIDevice *dev, Error **errp)
+static void piix4_realize(PCIDevice *pci, Error **errp)
 {
-    PIIX4State *d = PIIX4_PCI_DEVICE(dev);
+    DeviceState *dev = DEVICE(pci);
+    PIIX4State *s = DO_UPCAST(PIIX4State, dev, pci);
 
-    if (!isa_bus_new(DEVICE(d), pci_address_space(dev),
-                     pci_address_space_io(dev), errp)) {
+    if (!isa_bus_new(dev, pci_address_space(pci),
+                     pci_address_space_io(pci), errp)) {
         return;
     }
-    piix4_dev = &d->dev;
-    qemu_register_reset(piix4_reset, d);
+    piix4_dev = pci;
+    qemu_register_reset(piix4_reset, s);
 }
 
 int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn)
Signature · 1 line
2.11.0
#10 lore raw

Re: [Qemu-devel] [PATCH v3 05/16] piix4: rename some variables in realize function

Quoted 2 lines · Hervé Poussineau

PIIX4 structure is now 's'
PCI device is now 'pci'

Please don't use 'pci'. Use pci_dev', the former is too wide,

Thanks,
Marcel

Quoted 36 lines

DeviceState is now 'dev'

Signed-off-by Hervé Poussineau <[email protected]>
---
  hw/isa/piix4.c | 13 +++++++------
  1 file changed, 7 insertions(+), 6 deletions(-)
hw/isa/piix4.c +7 -6
index 6b8bc3faf0..4f476dc7e6 100644
@@ -87,16 +87,17 @@ static const VMStateDescription vmstate_piix4 = {
      }
  };
 
-static void piix4_realize(PCIDevice *dev, Error **errp)
+static void piix4_realize(PCIDevice *pci, Error **errp)
  {
-    PIIX4State *d = PIIX4_PCI_DEVICE(dev);
+    DeviceState *dev = DEVICE(pci);
+    PIIX4State *s = DO_UPCAST(PIIX4State, dev, pci);
 
-    if (!isa_bus_new(DEVICE(d), pci_address_space(dev),
-                     pci_address_space_io(dev), errp)) {
+    if (!isa_bus_new(dev, pci_address_space(pci),
+                     pci_address_space_io(pci), errp)) {
          return;
      }
-    piix4_dev = &d->dev;
-    qemu_register_reset(piix4_reset, d);
+    piix4_dev = pci;
+    qemu_register_reset(piix4_reset, s);
  }
 
  int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn)
author
#11 lore raw

Re: [Qemu-devel] [PATCH v3 05/16] piix4: rename some variables in realize function

Le 04/01/2018 à 15:33, Marcel Apfelbaum a écrit :

Quoted 5 lines · Hervé Poussineau
Quoted 2 lines

PIIX4 structure is now 's'
PCI device is now 'pci'

Please don't use 'pci'. Use pci_dev', the former is too wide,

OK, will do.

Quoted 39 lines

Thanks,
Marcel

Quoted 33 lines

DeviceState is now 'dev'

Signed-off-by Hervé Poussineau <[email protected]>

---
  hw/isa/piix4.c | 13 +++++++------

  1 file changed, 7 insertions(+), 6 deletions(-)
hw/isa/piix4.c +7 -6
index 6b8bc3faf0..4f476dc7e6 100644
@@ -87,16 +87,17 @@ static const VMStateDescription vmstate_piix4 = {
       }
   };
-static void piix4_realize(PCIDevice *dev, Error **errp)
+static void piix4_realize(PCIDevice *pci, Error **errp)
   {
-    PIIX4State *d = PIIX4_PCI_DEVICE(dev);
+    DeviceState *dev = DEVICE(pci);
+    PIIX4State *s = DO_UPCAST(PIIX4State, dev, pci);
-    if (!isa_bus_new(DEVICE(d), pci_address_space(dev),
-                     pci_address_space_io(dev), errp)) {
+    if (!isa_bus_new(dev, pci_address_space(pci),
+                     pci_address_space_io(pci), errp)) {
           return;
       }
-    piix4_dev = &d->dev;
-    qemu_register_reset(piix4_reset, d);
+    piix4_dev = pci;
+    qemu_register_reset(piix4_reset, s);
   }
   int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn)
author
#12 lore raw

PATCH v3 6/16 piix4: add Reset Control Register

In reply to Hervé Poussineau

The RCR I/O port (0xcf9) is used to generate a hard reset or a soft reset.

Signed-off-by Hervé Poussineau <[email protected]>
---
 hw/isa/piix4.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)
hw/isa/piix4.c +35 -0
index 4f476dc7e6..7c83e7c23d 100644
@@ -2,6 +2,7 @@
  * QEMU PIIX4 PCI Bridge Emulation
  *
  * Copyright (c) 2006 Fabrice Bellard
+ * Copyright (c) 2016 Hervé Poussineau
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal

@@ -33,6 +34,10 @@ PCIDevice *piix4_dev;
 
 typedef struct PIIX4State {
     PCIDevice dev;
+
+    /* Reset Control Register */
+    MemoryRegion rcr_mem;
+    uint8_t rcr;
 } PIIX4State;
 
 #define TYPE_PIIX4_PCI_DEVICE "PIIX4"

@@ -87,6 +92,30 @@ static const VMStateDescription vmstate_piix4 = {
     }
 };
 
+static void piix4_rcr_write(void *opaque, hwaddr addr, uint64_t val,
+                            unsigned int len)
+{
+    PIIX4State *s = opaque;
+
+    if (val & 4) {
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
+        return;
+    }
+    s->rcr = val & 2; /* keep System Reset type only */
+}
+
+static uint64_t piix4_rcr_read(void *opaque, hwaddr addr, unsigned int len)
+{
+    PIIX4State *s = opaque;
+    return s->rcr;
+}
+
+static const MemoryRegionOps piix4_rcr_ops = {
+    .read = piix4_rcr_read,
+    .write = piix4_rcr_write,
+    .endianness = DEVICE_LITTLE_ENDIAN
+};
+
 static void piix4_realize(PCIDevice *pci, Error **errp)
 {
     DeviceState *dev = DEVICE(pci);

@@ -96,6 +125,12 @@ static void piix4_realize(PCIDevice *pci, Error **errp)
                      pci_address_space_io(pci), errp)) {
         return;
     }
+
+    memory_region_init_io(&s->rcr_mem, OBJECT(dev), &piix4_rcr_ops, s,
+                          "reset-control", 1);
+    memory_region_add_subregion_overlap(pci_address_space_io(pci), 0xcf9,
+                                        &s->rcr_mem, 1);
+
     piix4_dev = pci;
     qemu_register_reset(piix4_reset, s);
 }
Signature · 1 line
2.11.0
#13 lore raw

Re: [Qemu-devel] [PATCH v3 06/16] piix4: add Reset Control Register

Quoted 12 lines · Hervé Poussineau

The RCR I/O port (0xcf9) is used to generate a hard reset or a soft reset.

Signed-off-by Hervé Poussineau <[email protected]>
---
  hw/isa/piix4.c | 35 +++++++++++++++++++++++++++++++++++
  1 file changed, 35 insertions(+)
hw/isa/piix4.c +0 -0
index 4f476dc7e6..7c83e7c23d 100644

hw/isa/piix4.c line 5

Quoted 4 lines
hw/isa/piix4.c +1 -0
@@ -2,6 +2,7 @@
   * QEMU PIIX4 PCI Bridge Emulation
   *
   * Copyright (c) 2006 Fabrice Bellard
+ * Copyright (c) 2016 Hervé Poussineau

I think 2018 :)

Quoted 18 lines
hw/isa/piix4.c +4 -0
   *
   * Permission is hereby granted, free of charge, to any person obtaining a copy
   * of this software and associated documentation files (the "Software"), to deal

@@ -33,6 +34,10 @@ PCIDevice *piix4_dev;
 
  typedef struct PIIX4State {
      PCIDevice dev;
+
+    /* Reset Control Register */
+    MemoryRegion rcr_mem;
+    uint8_t rcr;
  } PIIX4State;
 
  #define TYPE_PIIX4_PCI_DEVICE "PIIX4"

@@ -87,6 +92,30 @@ static const VMStateDescription vmstate_piix4 = {
      }
  };

hw/isa/piix4.c static const VMStateDescription vmstate_piix4 = { · line 104

Quoted 10 lines
hw/isa/piix4.c +10 -0
+static void piix4_rcr_write(void *opaque, hwaddr addr, uint64_t val,
+                            unsigned int len)
+{
+    PIIX4State *s = opaque;
+
+    if (val & 4) {
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
+        return;
+    }
+    s->rcr = val & 2; /* keep System Reset type only */

How does it work? When do we reset the value?

hw/isa/piix4.c static const VMStateDescription vmstate_piix4 = { · line 116

Quoted 12 lines
hw/isa/piix4.c +12 -0
+}
+
+static uint64_t piix4_rcr_read(void *opaque, hwaddr addr, unsigned int len)
+{
+    PIIX4State *s = opaque;
+    return s->rcr;
+}
+
+static const MemoryRegionOps piix4_rcr_ops = {
+    .read = piix4_rcr_read,
+    .write = piix4_rcr_write,
+    .endianness = DEVICE_LITTLE_ENDIAN

Maybe we need to specify the access_size.
Something like:

.impl = {
        .min_access_size = 1,
        .max_access_size = 1,
    },

Thanks,
Marcel

Quoted 19 lines
hw/isa/piix4.c +8 -0
+};
+
  static void piix4_realize(PCIDevice *pci, Error **errp)
  {
      DeviceState *dev = DEVICE(pci);

@@ -96,6 +125,12 @@ static void piix4_realize(PCIDevice *pci, Error **errp)
                       pci_address_space_io(pci), errp)) {
          return;
      }
+
+    memory_region_init_io(&s->rcr_mem, OBJECT(dev), &piix4_rcr_ops, s,
+                          "reset-control", 1);
+    memory_region_add_subregion_overlap(pci_address_space_io(pci), 0xcf9,
+                                        &s->rcr_mem, 1);
+
      piix4_dev = pci;
      qemu_register_reset(piix4_reset, s);
  }
author
#14 lore raw

Re: [Qemu-devel] [PATCH v3 06/16] piix4: add Reset Control Register

Le 04/01/2018 à 16:50, Marcel Apfelbaum a écrit :

Quoted 8 lines · Hervé Poussineau
Quoted 7 lines

The RCR I/O port (0xcf9) is used to generate a hard reset or a soft reset.

Signed-off-by Hervé Poussineau <[email protected]>

---
  hw/isa/piix4.c | 35 +++++++++++++++++++++++++++++++++++

  1 file changed, 35 insertions(+)
Quoted 11 lines

hw/isa/piix4.c line 5

Quoted 9 lines
hw/isa/piix4.c +1 -0
index 4f476dc7e6..7c83e7c23d 100644
@@ -2,6 +2,7 @@
    * QEMU PIIX4 PCI Bridge Emulation
    *
    * Copyright (c) 2006 Fabrice Bellard
+ * Copyright (c) 2016 Hervé Poussineau

I think 2018 :)

Oops :) Initial work was already done 2 years ago...

Quoted 20 lines
Quoted 20 lines
hw/isa/piix4.c +8 -0
    *
    * Permission is hereby granted, free of charge, to any person obtaining a copy
    * of this software and associated documentation files (the "Software"), to deal

@@ -33,6 +34,10 @@ PCIDevice *piix4_dev;
   typedef struct PIIX4State {
       PCIDevice dev;
+
+    /* Reset Control Register */
+    MemoryRegion rcr_mem;
+    uint8_t rcr;
   } PIIX4State;
   #define TYPE_PIIX4_PCI_DEVICE "PIIX4"

@@ -87,6 +92,30 @@ static const VMStateDescription vmstate_piix4 = {
       }
   };
+static void piix4_rcr_write(void *opaque, hwaddr addr, uint64_t val,
+                            unsigned int len)
+{
+    PIIX4State *s = opaque;
Quoted 8 lines

hw/isa/piix4.c static const VMStateDescription vmstate_piix4 = { · line 104

Quoted 6 lines
hw/isa/piix4.c +6 -0
+
+    if (val & 4) {
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
+        return;
+    }
+    s->rcr = val & 2; /* keep System Reset type only */

How does it work? When do we reset the value?

The value is never reset. According to datasheet:
- bits 7:3 and 0 are reserved. We don't keep those bits in s->rcr.
- bit 2 ("Reset CPU") resets the system when transitioning from 0 to 1. It is never read as 0, so we don't keep it in s->rcr.
- bit 1 ("System Reset") tells if we need to do a hard reset (1) or a soft reset (0). This bit is kept accross reboots.

Note that piix3 also does the same thing in hw/pci-host/piix.c in rcr_write()

Quoted 12 lines
Quoted 12 lines
hw/isa/piix4.c +11 -0
+}
+
+static uint64_t piix4_rcr_read(void *opaque, hwaddr addr, unsigned int len)
+{
+    PIIX4State *s = opaque;
+    return s->rcr;
+}
+
+static const MemoryRegionOps piix4_rcr_ops = {
+    .read = piix4_rcr_read,
+    .write = piix4_rcr_write,
Quoted 9 lines

hw/isa/piix4.c static const VMStateDescription vmstate_piix4 = { · line 116

Quoted 1 line
hw/isa/piix4.c +1 -0
+    .endianness = DEVICE_LITTLE_ENDIAN

Maybe we need to specify the access_size.
Something like:
.impl = {
        .min_access_size = 1,
        .max_access_size = 1,
    },

Not a problem, I will add it.

Quoted 24 lines

Thanks,
Marcel

Quoted 19 lines
hw/isa/piix4.c +8 -0
+};
+
   static void piix4_realize(PCIDevice *pci, Error **errp)
   {
       DeviceState *dev = DEVICE(pci);

@@ -96,6 +125,12 @@ static void piix4_realize(PCIDevice *pci, Error **errp)
                        pci_address_space_io(pci), errp)) {
           return;
       }
+
+    memory_region_init_io(&s->rcr_mem, OBJECT(dev), &piix4_rcr_ops, s,
+                          "reset-control", 1);
+    memory_region_add_subregion_overlap(pci_address_space_io(pci), 0xcf9,
+                                        &s->rcr_mem, 1);
+
       piix4_dev = pci;
       qemu_register_reset(piix4_reset, s);
   }
author
#15 lore raw

PATCH v3 7/16 piix4: add a i8259 interrupt controller as specified in datasheet

In reply to Hervé Poussineau

Add ISA irqs as piix4 gpio in, and CPU interrupt request as piix4 gpio out. Remove i8259 instanciated in malta board, to not have it twice.

We can also remove the now unused piix4_init() function.

Signed-off-by Hervé Poussineau <[email protected]>
---
 hw/isa/piix4.c       | 40 +++++++++++++++++++++++++++++-----------
 hw/mips/mips_malta.c | 28 ++++++++++++----------------
 include/hw/i386/pc.h |  1 -
 3 files changed, 41 insertions(+), 28 deletions(-)
hw/isa/piix4.c +29 -11
index 7c83e7c23d..eb2f730fff 100644
@@ -34,6 +34,8 @@ PCIDevice *piix4_dev;
 
 typedef struct PIIX4State {
     PCIDevice dev;
+    qemu_irq cpu_intr;
+    qemu_irq *isa;
 
     /* Reset Control Register */
     MemoryRegion rcr_mem;

@@ -92,6 +94,18 @@ static const VMStateDescription vmstate_piix4 = {
     }
 };
 
+static void piix4_request_i8259_irq(void *opaque, int irq, int level)
+{
+    PIIX4State *s = opaque;
+    qemu_set_irq(s->cpu_intr, level);
+}
+
+static void piix4_set_i8259_irq(void *opaque, int irq, int level)
+{
+    PIIX4State *s = opaque;
+    qemu_set_irq(s->isa[irq], level);
+}
+
 static void piix4_rcr_write(void *opaque, hwaddr addr, uint64_t val,
                             unsigned int len)
 {

@@ -120,28 +134,32 @@ static void piix4_realize(PCIDevice *pci, Error **errp)
 {
     DeviceState *dev = DEVICE(pci);
     PIIX4State *s = DO_UPCAST(PIIX4State, dev, pci);
+    ISABus *isa_bus;
+    qemu_irq *i8259_out_irq;
 
-    if (!isa_bus_new(dev, pci_address_space(pci),
-                     pci_address_space_io(pci), errp)) {
+    isa_bus = isa_bus_new(dev, pci_address_space(pci),
+                          pci_address_space_io(pci), errp);
+    if (!isa_bus) {
         return;
     }
 
+    qdev_init_gpio_in_named(dev, piix4_set_i8259_irq, "isa", ISA_NUM_IRQS);
+    qdev_init_gpio_out_named(dev, &s->cpu_intr, "intr", 1);
+
     memory_region_init_io(&s->rcr_mem, OBJECT(dev), &piix4_rcr_ops, s,
                           "reset-control", 1);
     memory_region_add_subregion_overlap(pci_address_space_io(pci), 0xcf9,
                                         &s->rcr_mem, 1);
 
-    piix4_dev = pci;
-    qemu_register_reset(piix4_reset, s);
-}
+    /* initialize i8259 pic */
+    i8259_out_irq = qemu_allocate_irqs(piix4_request_i8259_irq, s, 1);
+    s->isa = i8259_init(isa_bus, *i8259_out_irq);
 
-int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn)
-{
-    PCIDevice *d;
+    /* initialize ISA irqs */
+    isa_bus_irqs(isa_bus, s->isa);
 
-    d = pci_create_simple_multifunction(bus, devfn, true, "PIIX4");
-    *isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(d), "isa.0"));
-    return d->devfn;
+    piix4_dev = pci;
+    qemu_register_reset(piix4_reset, s);
 }
 
 static void piix4_class_init(ObjectClass *klass, void *data)
hw/mips/mips_malta.c +12 -16
index 37f19428d6..043fe40bce 100644
@@ -96,7 +96,7 @@ typedef struct {
     SysBusDevice parent_obj;
 
     MIPSCPSState *cps;
-    qemu_irq *i8259;
+    qemu_irq i8259[16];
 } MaltaState;
 
 static ISADevice *pit;

@@ -998,8 +998,8 @@ void mips_malta_init(MachineState *machine)
     int64_t kernel_entry, bootloader_run_addr;
     PCIBus *pci_bus;
     ISABus *isa_bus;
-    qemu_irq *isa_irq;
     qemu_irq cbus_irq, i8259_irq;
+    PCIDevice *pci;
     int piix4_devfn;
     I2CBus *smbus;
     int i;

@@ -1180,28 +1180,24 @@ void mips_malta_init(MachineState *machine)
     /* Board ID = 0x420 (Malta Board with CoreLV) */
     stl_p(memory_region_get_ram_ptr(bios_copy) + 0x10, 0x00000420);
 
-    /*
-     * We have a circular dependency problem: pci_bus depends on isa_irq,
-     * isa_irq is provided by i8259, i8259 depends on ISA, ISA depends
-     * on piix4, and piix4 depends on pci_bus.  To stop the cycle we have
-     * qemu_irq_proxy() adds an extra bit of indirection, allowing us
-     * to resolve the isa_irq -> i8259 dependency after i8259 is initialized.
-     */
-    isa_irq = qemu_irq_proxy(&s->i8259, 16);
-
     /* Northbridge */
-    pci_bus = gt64120_register(isa_irq);
+    pci_bus = gt64120_register(s->i8259);
 
     /* Southbridge */
     ide_drive_get(hd, ARRAY_SIZE(hd));
 
-    piix4_devfn = piix4_init(pci_bus, &isa_bus, 80);
+    pci = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(10, 0),
+                                          true, "PIIX4");
+    dev = DEVICE(pci);
+    isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
+    piix4_devfn = pci->devfn;
 
     /* Interrupt controller */
-    /* The 8259 is attached to the MIPS CPU INT0 pin, ie interrupt 2 */
-    s->i8259 = i8259_init(isa_bus, i8259_irq);
+    qdev_connect_gpio_out_named(dev, "intr", 0, i8259_irq);
+    for (i = 0; i < 16; i++) {
+        s->i8259[i] = qdev_get_gpio_in_named(dev, "isa", i);
+    }
 
-    isa_bus_irqs(isa_bus, s->i8259);
     pci_piix4_ide_init(pci_bus, hd, piix4_devfn + 1);
     pci_create_simple(pci_bus, piix4_devfn + 2, "piix4-usb-uhci");
     smbus = piix4_pm_init(pci_bus, piix4_devfn + 3, 0x1100,
include/hw/i386/pc.h +0 -1
index 6f77eb0665..3308ab4b93 100644
@@ -300,7 +300,6 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
 PCIBus *find_i440fx(void);
 /* piix4.c */
 extern PCIDevice *piix4_dev;
-int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn);
 
 /* pc_sysfw.c */
 void pc_system_firmware_init(MemoryRegion *rom_memory,
Signature · 1 line
2.11.0
#16 lore raw

Re: [Qemu-devel] [PATCH v3 07/16] piix4: add a i8259 interrupt controller as specified in datasheet

Quoted 155 lines · Hervé Poussineau

Add ISA irqs as piix4 gpio in, and CPU interrupt request as piix4 gpio out. Remove i8259 instanciated in malta board, to not have it twice.

We can also remove the now unused piix4_init() function.

Signed-off-by Hervé Poussineau <[email protected]>
---
 hw/isa/piix4.c       | 40 +++++++++++++++++++++++++++++-----------
 hw/mips/mips_malta.c | 28 ++++++++++++----------------
 include/hw/i386/pc.h |  1 -
 3 files changed, 41 insertions(+), 28 deletions(-)
hw/isa/piix4.c +29 -11
index 7c83e7c23d..eb2f730fff 100644
@@ -34,6 +34,8 @@ PCIDevice *piix4_dev;
 
 typedef struct PIIX4State {
     PCIDevice dev;
+    qemu_irq cpu_intr;
+    qemu_irq *isa;
 
     /* Reset Control Register */
     MemoryRegion rcr_mem;

@@ -92,6 +94,18 @@ static const VMStateDescription vmstate_piix4 = {
     }
 };
 
+static void piix4_request_i8259_irq(void *opaque, int irq, int level)
+{
+    PIIX4State *s = opaque;
+    qemu_set_irq(s->cpu_intr, level);
+}
+
+static void piix4_set_i8259_irq(void *opaque, int irq, int level)
+{
+    PIIX4State *s = opaque;
+    qemu_set_irq(s->isa[irq], level);
+}
+
 static void piix4_rcr_write(void *opaque, hwaddr addr, uint64_t val,
                             unsigned int len)
 {

@@ -120,28 +134,32 @@ static void piix4_realize(PCIDevice *pci, Error **errp)
 {
     DeviceState *dev = DEVICE(pci);
     PIIX4State *s = DO_UPCAST(PIIX4State, dev, pci);
+    ISABus *isa_bus;
+    qemu_irq *i8259_out_irq;
 
-    if (!isa_bus_new(dev, pci_address_space(pci),
-                     pci_address_space_io(pci), errp)) {
+    isa_bus = isa_bus_new(dev, pci_address_space(pci),
+                          pci_address_space_io(pci), errp);
+    if (!isa_bus) {
         return;
     }
 
+    qdev_init_gpio_in_named(dev, piix4_set_i8259_irq, "isa", ISA_NUM_IRQS);
+    qdev_init_gpio_out_named(dev, &s->cpu_intr, "intr", 1);
+
     memory_region_init_io(&s->rcr_mem, OBJECT(dev), &piix4_rcr_ops, s,
                           "reset-control", 1);
     memory_region_add_subregion_overlap(pci_address_space_io(pci), 0xcf9,
                                         &s->rcr_mem, 1);
 
-    piix4_dev = pci;
-    qemu_register_reset(piix4_reset, s);
-}
+    /* initialize i8259 pic */
+    i8259_out_irq = qemu_allocate_irqs(piix4_request_i8259_irq, s, 1);
+    s->isa = i8259_init(isa_bus, *i8259_out_irq);
 
-int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn)
-{
-    PCIDevice *d;
+    /* initialize ISA irqs */
+    isa_bus_irqs(isa_bus, s->isa);
 
-    d = pci_create_simple_multifunction(bus, devfn, true, "PIIX4");
-    *isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(d), "isa.0"));
-    return d->devfn;
+    piix4_dev = pci;
+    qemu_register_reset(piix4_reset, s);
 }
 
 static void piix4_class_init(ObjectClass *klass, void *data)
hw/mips/mips_malta.c +12 -16
index 37f19428d6..043fe40bce 100644
@@ -96,7 +96,7 @@ typedef struct {
     SysBusDevice parent_obj;
 
     MIPSCPSState *cps;
-    qemu_irq *i8259;
+    qemu_irq i8259[16];
 } MaltaState;
 
 static ISADevice *pit;

@@ -998,8 +998,8 @@ void mips_malta_init(MachineState *machine)
     int64_t kernel_entry, bootloader_run_addr;
     PCIBus *pci_bus;
     ISABus *isa_bus;
-    qemu_irq *isa_irq;
     qemu_irq cbus_irq, i8259_irq;
+    PCIDevice *pci;
     int piix4_devfn;
     I2CBus *smbus;
     int i;

@@ -1180,28 +1180,24 @@ void mips_malta_init(MachineState *machine)
     /* Board ID = 0x420 (Malta Board with CoreLV) */
     stl_p(memory_region_get_ram_ptr(bios_copy) + 0x10, 0x00000420);
 
-    /*
-     * We have a circular dependency problem: pci_bus depends on isa_irq,
-     * isa_irq is provided by i8259, i8259 depends on ISA, ISA depends
-     * on piix4, and piix4 depends on pci_bus.  To stop the cycle we have
-     * qemu_irq_proxy() adds an extra bit of indirection, allowing us
-     * to resolve the isa_irq -> i8259 dependency after i8259 is initialized.
-     */
-    isa_irq = qemu_irq_proxy(&s->i8259, 16);
-
     /* Northbridge */
-    pci_bus = gt64120_register(isa_irq);
+    pci_bus = gt64120_register(s->i8259);
 
     /* Southbridge */
     ide_drive_get(hd, ARRAY_SIZE(hd));
 
-    piix4_devfn = piix4_init(pci_bus, &isa_bus, 80);
+    pci = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(10, 0),
+                                          true, "PIIX4");
+    dev = DEVICE(pci);
+    isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
+    piix4_devfn = pci->devfn;
 
     /* Interrupt controller */
-    /* The 8259 is attached to the MIPS CPU INT0 pin, ie interrupt 2 */
-    s->i8259 = i8259_init(isa_bus, i8259_irq);
+    qdev_connect_gpio_out_named(dev, "intr", 0, i8259_irq);
+    for (i = 0; i < 16; i++) {
+        s->i8259[i] = qdev_get_gpio_in_named(dev, "isa", i);
+    }
 
-    isa_bus_irqs(isa_bus, s->i8259);
     pci_piix4_ide_init(pci_bus, hd, piix4_devfn + 1);
     pci_create_simple(pci_bus, piix4_devfn + 2, "piix4-usb-uhci");
     smbus = piix4_pm_init(pci_bus, piix4_devfn + 3, 0x1100,
include/hw/i386/pc.h +0 -0
index 6f77eb0665..3308ab4b93 100644

include/hw/i386/pc.h PCIBus *i440fx_init(const char *host_type, const char *pci_type, · line 305

Quoted 7 lines
include/hw/i386/pc.h +0 -1
@@ -300,7 +300,6 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
 PCIBus *find_i440fx(void);
 /* piix4.c */
 extern PCIDevice *piix4_dev;
-int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn);
 
 /* pc_sysfw.c */
 void pc_system_firmware_init(MemoryRegion *rom_memory,

And please drop the pc.h include from mips_malta.c and piix.c

Quoted 2 lines
Signature · 1 line
2.11.0
author
#17 lore raw

Re: [Qemu-devel] [PATCH v3 07/16] piix4: add a i8259 interrupt controller as specified in datasheet

Le 05/01/2018 à 00:21, Michael S. Tsirkin a écrit :

Quoted 161 lines · Hervé Poussineau
Quoted 160 lines

Add ISA irqs as piix4 gpio in, and CPU interrupt request as piix4 gpio out. Remove i8259 instanciated in malta board, to not have it twice.

We can also remove the now unused piix4_init() function.

Signed-off-by Hervé Poussineau <[email protected]>
---
  hw/isa/piix4.c       | 40 +++++++++++++++++++++++++++++-----------
  hw/mips/mips_malta.c | 28 ++++++++++++----------------
  include/hw/i386/pc.h |  1 -
  3 files changed, 41 insertions(+), 28 deletions(-)
hw/isa/piix4.c +29 -11
index 7c83e7c23d..eb2f730fff 100644
@@ -34,6 +34,8 @@ PCIDevice *piix4_dev;
 
  typedef struct PIIX4State {
      PCIDevice dev;
+    qemu_irq cpu_intr;
+    qemu_irq *isa;
 
      /* Reset Control Register */
      MemoryRegion rcr_mem;

@@ -92,6 +94,18 @@ static const VMStateDescription vmstate_piix4 = {
      }
  };
 
+static void piix4_request_i8259_irq(void *opaque, int irq, int level)
+{
+    PIIX4State *s = opaque;
+    qemu_set_irq(s->cpu_intr, level);
+}
+
+static void piix4_set_i8259_irq(void *opaque, int irq, int level)
+{
+    PIIX4State *s = opaque;
+    qemu_set_irq(s->isa[irq], level);
+}
+
  static void piix4_rcr_write(void *opaque, hwaddr addr, uint64_t val,
                              unsigned int len)
  {

@@ -120,28 +134,32 @@ static void piix4_realize(PCIDevice *pci, Error **errp)
  {
      DeviceState *dev = DEVICE(pci);
      PIIX4State *s = DO_UPCAST(PIIX4State, dev, pci);
+    ISABus *isa_bus;
+    qemu_irq *i8259_out_irq;
 
-    if (!isa_bus_new(dev, pci_address_space(pci),
-                     pci_address_space_io(pci), errp)) {
+    isa_bus = isa_bus_new(dev, pci_address_space(pci),
+                          pci_address_space_io(pci), errp);
+    if (!isa_bus) {
          return;
      }
 
+    qdev_init_gpio_in_named(dev, piix4_set_i8259_irq, "isa", ISA_NUM_IRQS);
+    qdev_init_gpio_out_named(dev, &s->cpu_intr, "intr", 1);
+
      memory_region_init_io(&s->rcr_mem, OBJECT(dev), &piix4_rcr_ops, s,
                            "reset-control", 1);
      memory_region_add_subregion_overlap(pci_address_space_io(pci), 0xcf9,
                                          &s->rcr_mem, 1);
 
-    piix4_dev = pci;
-    qemu_register_reset(piix4_reset, s);
-}
+    /* initialize i8259 pic */
+    i8259_out_irq = qemu_allocate_irqs(piix4_request_i8259_irq, s, 1);
+    s->isa = i8259_init(isa_bus, *i8259_out_irq);
 
-int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn)
-{
-    PCIDevice *d;
+    /* initialize ISA irqs */
+    isa_bus_irqs(isa_bus, s->isa);
 
-    d = pci_create_simple_multifunction(bus, devfn, true, "PIIX4");
-    *isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(d), "isa.0"));
-    return d->devfn;
+    piix4_dev = pci;
+    qemu_register_reset(piix4_reset, s);
  }
 
  static void piix4_class_init(ObjectClass *klass, void *data)
hw/mips/mips_malta.c +12 -16
index 37f19428d6..043fe40bce 100644
@@ -96,7 +96,7 @@ typedef struct {
      SysBusDevice parent_obj;
 
      MIPSCPSState *cps;
-    qemu_irq *i8259;
+    qemu_irq i8259[16];
  } MaltaState;
 
  static ISADevice *pit;

@@ -998,8 +998,8 @@ void mips_malta_init(MachineState *machine)
      int64_t kernel_entry, bootloader_run_addr;
      PCIBus *pci_bus;
      ISABus *isa_bus;
-    qemu_irq *isa_irq;
      qemu_irq cbus_irq, i8259_irq;
+    PCIDevice *pci;
      int piix4_devfn;
      I2CBus *smbus;
      int i;

@@ -1180,28 +1180,24 @@ void mips_malta_init(MachineState *machine)
      /* Board ID = 0x420 (Malta Board with CoreLV) */
      stl_p(memory_region_get_ram_ptr(bios_copy) + 0x10, 0x00000420);
 
-    /*
-     * We have a circular dependency problem: pci_bus depends on isa_irq,
-     * isa_irq is provided by i8259, i8259 depends on ISA, ISA depends
-     * on piix4, and piix4 depends on pci_bus.  To stop the cycle we have
-     * qemu_irq_proxy() adds an extra bit of indirection, allowing us
-     * to resolve the isa_irq -> i8259 dependency after i8259 is initialized.
-     */
-    isa_irq = qemu_irq_proxy(&s->i8259, 16);
-
      /* Northbridge */
-    pci_bus = gt64120_register(isa_irq);
+    pci_bus = gt64120_register(s->i8259);
 
      /* Southbridge */
      ide_drive_get(hd, ARRAY_SIZE(hd));
 
-    piix4_devfn = piix4_init(pci_bus, &isa_bus, 80);
+    pci = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(10, 0),
+                                          true, "PIIX4");
+    dev = DEVICE(pci);
+    isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
+    piix4_devfn = pci->devfn;
 
      /* Interrupt controller */
-    /* The 8259 is attached to the MIPS CPU INT0 pin, ie interrupt 2 */
-    s->i8259 = i8259_init(isa_bus, i8259_irq);
+    qdev_connect_gpio_out_named(dev, "intr", 0, i8259_irq);
+    for (i = 0; i < 16; i++) {
+        s->i8259[i] = qdev_get_gpio_in_named(dev, "isa", i);
+    }
 
-    isa_bus_irqs(isa_bus, s->i8259);
      pci_piix4_ide_init(pci_bus, hd, piix4_devfn + 1);
      pci_create_simple(pci_bus, piix4_devfn + 2, "piix4-usb-uhci");
      smbus = piix4_pm_init(pci_bus, piix4_devfn + 3, 0x1100,
include/hw/i386/pc.h +0 -1
index 6f77eb0665..3308ab4b93 100644
@@ -300,7 +300,6 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
  PCIBus *find_i440fx(void);
  /* piix4.c */
  extern PCIDevice *piix4_dev;
-int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn);
Quoted 4 lines

include/hw/i386/pc.h PCIBus *i440fx_init(const char *host_type, const char *pci_type, · line 305

Quoted 2 lines
include/hw/i386/pc.h +0 -0
  /* pc_sysfw.c */
  void pc_system_firmware_init(MemoryRegion *rom_memory,

And please drop the pc.h include from mips_malta.c and piix.c

I can't remove it from piix4.c due to a call to i8259_init() and I can't remove it from mips_malta.c due to a call to piix4_pm_init(). I suppose that's a work for later.

Quoted 4 lines
Quoted 3 lines
Signature · 2 lines
-- 
2.11.0
author
#18 lore raw

PATCH v3 8/16 piix4: add a i8257 dma controller as specified in datasheet

In reply to Hervé Poussineau

Remove i8257 instanciated in malta board, to not have it twice.

Signed-off-by Hervé Poussineau <[email protected]>
---
 hw/isa/piix4.c       | 3 +++
 hw/mips/mips_malta.c | 1 -
 2 files changed, 3 insertions(+), 1 deletion(-)
hw/isa/piix4.c +3 -0
index eb2f730fff..66fdfe25e2 100644
@@ -158,6 +158,9 @@ static void piix4_realize(PCIDevice *pci, Error **errp)
     /* initialize ISA irqs */
     isa_bus_irqs(isa_bus, s->isa);
 
+    /* DMA */
+    DMA_init(isa_bus, 0);
+
     piix4_dev = pci;
     qemu_register_reset(piix4_reset, s);
 }
hw/mips/mips_malta.c +0 -1
index 043fe40bce..647688c58a 100644
@@ -1205,7 +1205,6 @@ void mips_malta_init(MachineState *machine)
     smbus_eeprom_init(smbus, 8, smbus_eeprom_buf, smbus_eeprom_size);
     g_free(smbus_eeprom_buf);
     pit = i8254_pit_init(isa_bus, 0x40, 0, NULL);
-    DMA_init(isa_bus, 0);
 
     /* Super I/O */
     isa_create_simple(isa_bus, "i8042");
Signature · 1 line
2.11.0
author
#19 lore raw

PATCH v3 9/16 piix4: add a i8254 pit controller as specified in datasheet

In reply to Hervé Poussineau

Remove i8254 instanciated in malta board, to not have it twice.

Signed-off-by Hervé Poussineau <[email protected]>
---
 hw/isa/piix4.c       | 4 ++++
 hw/mips/mips_malta.c | 3 ---
 2 files changed, 4 insertions(+), 3 deletions(-)
hw/isa/piix4.c +4 -0
index 66fdfe25e2..1fd9f4f330 100644
@@ -29,6 +29,7 @@
 #include "hw/pci/pci.h"
 #include "hw/isa/isa.h"
 #include "hw/sysbus.h"
+#include "hw/timer/i8254.h"
 
 PCIDevice *piix4_dev;
 

@@ -158,6 +159,9 @@ static void piix4_realize(PCIDevice *pci, Error **errp)
     /* initialize ISA irqs */
     isa_bus_irqs(isa_bus, s->isa);
 
+    /* initialize pit */
+    i8254_pit_init(isa_bus, 0x40, 0, NULL);
+
     /* DMA */
     DMA_init(isa_bus, 0);
hw/mips/mips_malta.c +0 -3
index 647688c58a..afe13c684f 100644
@@ -99,8 +99,6 @@ typedef struct {
     qemu_irq i8259[16];
 } MaltaState;
 
-static ISADevice *pit;
-
 static struct _loaderparams {
     int ram_size, ram_low_size;
     const char *kernel_filename;

@@ -1204,7 +1202,6 @@ void mips_malta_init(MachineState *machine)
                           isa_get_irq(NULL, 9), NULL, 0, NULL);
     smbus_eeprom_init(smbus, 8, smbus_eeprom_buf, smbus_eeprom_size);
     g_free(smbus_eeprom_buf);
-    pit = i8254_pit_init(isa_bus, 0x40, 0, NULL);
 
     /* Super I/O */
     isa_create_simple(isa_bus, "i8042");
Signature · 1 line
2.11.0
author
#20 lore raw

PATCH v3 10/16 piix4: add a i8042 keyboard/mouse controller as specified in datasheet

In reply to Hervé Poussineau

Remove i8042 instanciated in malta board, to not have it twice.

Signed-off-by Hervé Poussineau <[email protected]>
---
 hw/isa/piix4.c       | 3 +++
 hw/mips/mips_malta.c | 2 --
 2 files changed, 3 insertions(+), 2 deletions(-)
hw/isa/piix4.c +3 -0
index 1fd9f4f330..c5639f7640 100644
@@ -165,6 +165,9 @@ static void piix4_realize(PCIDevice *pci, Error **errp)
     /* DMA */
     DMA_init(isa_bus, 0);
 
+    /* Super I/O */
+    isa_create_simple(isa_bus, "i8042");
+
     piix4_dev = pci;
     qemu_register_reset(piix4_reset, s);
 }
hw/mips/mips_malta.c +0 -2
index afe13c684f..7498fad006 100644
@@ -1204,8 +1204,6 @@ void mips_malta_init(MachineState *machine)
     g_free(smbus_eeprom_buf);
 
     /* Super I/O */
-    isa_create_simple(isa_bus, "i8042");
-
     mc146818_rtc_init(isa_bus, 2000, NULL);
     serial_hds_isa_init(isa_bus, 0, 2);
     parallel_hds_isa_init(isa_bus, 1);
Signature · 1 line
2.11.0
author
#21 lore raw

PATCH v3 11/16 piix4: add a floppy controller, 1 parallel port and 2 serial ports

In reply to Hervé Poussineau

Remove their instanciation from malta board, to not have them twice. Automatically create serial/parallel ports in PIIX4 if not provided.

Signed-off-by Hervé Poussineau <[email protected]>
---
 hw/isa/piix4.c       | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/mips/mips_malta.c | 41 +++++++++++++++++---------------
 2 files changed, 89 insertions(+), 19 deletions(-)
hw/isa/piix4.c +67 -0
index c5639f7640..f672af21cd 100644
@@ -28,8 +28,10 @@
 #include "hw/i386/pc.h"
 #include "hw/pci/pci.h"
 #include "hw/isa/isa.h"
+#include "hw/char/isa.h"
 #include "hw/sysbus.h"
 #include "hw/timer/i8254.h"
+#include "qapi/error.h"
 
 PCIDevice *piix4_dev;
 

@@ -38,6 +40,10 @@ typedef struct PIIX4State {
     qemu_irq cpu_intr;
     qemu_irq *isa;
 
+    FDCtrlISABus floppy;
+    ISASerialState serial[2];
+    ISAParallelState parallel;
+
     /* Reset Control Register */
     MemoryRegion rcr_mem;
     uint8_t rcr;

@@ -137,6 +143,8 @@ static void piix4_realize(PCIDevice *pci, Error **errp)
     PIIX4State *s = DO_UPCAST(PIIX4State, dev, pci);
     ISABus *isa_bus;
     qemu_irq *i8259_out_irq;
+    int i;
+    Error *err = NULL;
 
     isa_bus = isa_bus_new(dev, pci_address_space(pci),
                           pci_address_space_io(pci), errp);

@@ -168,10 +176,68 @@ static void piix4_realize(PCIDevice *pci, Error **errp)
     /* Super I/O */
     isa_create_simple(isa_bus, "i8042");
 
+    /* floppy */
+    qdev_set_parent_bus(DEVICE(&s->floppy), BUS(isa_bus));
+    object_property_set_bool(OBJECT(&s->floppy), true, "realized", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+
+    /* serial ports */
+    for (i = 0; i < 2; i++) {
+        qdev_set_parent_bus(DEVICE(&s->serial[i]), BUS(isa_bus));
+        if (!qemu_chr_fe_backend_connected(&s->serial[i].state.chr)) {
+            char prop[] = "serial?";
+            char label[] = "piix4.serial?";
+            prop[6] = i + '0';
+            label[12] = i + '0';
+            qdev_prop_set_chr(dev, prop, qemu_chr_new(label, "null"));
+        }
+        object_property_set_bool(OBJECT(&s->serial[i]), true, "realized", &err);
+        if (err) {
+            error_propagate(errp, err);
+            return;
+        }
+    }
+
+    /* parallel port */
+    qdev_set_parent_bus(DEVICE(&s->parallel), BUS(isa_bus));
+    if (!qemu_chr_fe_backend_connected(&s->parallel.state.chr)) {
+        qdev_prop_set_chr(dev, "parallel",
+                          qemu_chr_new("pii4x.parallel", "null"));
+    }
+    object_property_set_bool(OBJECT(&s->parallel), true, "realized", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+
     piix4_dev = pci;
     qemu_register_reset(piix4_reset, s);
 }
 
+static void piix4_init(Object *obj)
+{
+    PIIX4State *s = PIIX4_PCI_DEVICE(obj);
+    int i;
+
+    object_initialize(&s->floppy, sizeof(s->floppy), TYPE_ISA_FDC);
+    for (i = 0; i < 2; i++) {
+        object_initialize(&s->serial[i], sizeof(s->serial[i]), TYPE_ISA_SERIAL);
+    }
+    object_initialize(&s->parallel, sizeof(s->parallel), TYPE_ISA_PARALLEL);
+
+    object_property_add_alias(obj, "floppy", OBJECT(&s->floppy), "driveA",
+                              &error_abort);
+    object_property_add_alias(obj, "serial0", OBJECT(&s->serial[0]), "chardev",
+                              &error_abort);
+    object_property_add_alias(obj, "serial1", OBJECT(&s->serial[1]), "chardev",
+                              &error_abort);
+    object_property_add_alias(obj, "parallel", OBJECT(&s->parallel), "chardev",
+                              &error_abort);
+}
+
 static void piix4_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);

@@ -195,6 +261,7 @@ static const TypeInfo piix4_info = {
     .name          = TYPE_PIIX4_PCI_DEVICE,
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(PIIX4State),
+    .instance_init = piix4_init,
     .class_init    = piix4_class_init,
     .interfaces = (InterfaceInfo[]) {
         { INTERFACE_CONVENTIONAL_PCI_DEVICE },
hw/mips/mips_malta.c +22 -19

72 lines not on this page · raw message

Signature · 1 line
2.11.0
author
#22 lore raw

PATCH v3 12/16 piix4: add a mc146818rtc controller as specified in datasheet

In reply to Hervé Poussineau

Remove mc146818rtc instanciated in malta board, to not have it twice.

Signed-off-by Hervé Poussineau <[email protected]>
---
 hw/isa/piix4.c       | 12 ++++++++++++
 hw/mips/mips_malta.c |  5 -----
 2 files changed, 12 insertions(+), 5 deletions(-)
hw/isa/piix4.c +12 -0
index f672af21cd..09388ff286 100644
@@ -31,6 +31,7 @@
 #include "hw/char/isa.h"
 #include "hw/sysbus.h"
 #include "hw/timer/i8254.h"
+#include "hw/timer/mc146818rtc.h"
 #include "qapi/error.h"
 
 PCIDevice *piix4_dev;

@@ -43,6 +44,7 @@ typedef struct PIIX4State {
     FDCtrlISABus floppy;
     ISASerialState serial[2];
     ISAParallelState parallel;
+    RTCState rtc;
 
     /* Reset Control Register */
     MemoryRegion rcr_mem;

@@ -213,6 +215,15 @@ static void piix4_realize(PCIDevice *pci, Error **errp)
         return;
     }
 
+    /* timer */
+    qdev_set_parent_bus(DEVICE(&s->rtc), BUS(isa_bus));
+    object_property_set_bool(OBJECT(&s->rtc), true, "realized", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+    isa_init_irq(ISA_DEVICE(&s->rtc), &s->rtc.irq, RTC_ISA_IRQ);
+
     piix4_dev = pci;
     qemu_register_reset(piix4_reset, s);
 }

@@ -227,6 +238,7 @@ static void piix4_init(Object *obj)
         object_initialize(&s->serial[i], sizeof(s->serial[i]), TYPE_ISA_SERIAL);
     }
     object_initialize(&s->parallel, sizeof(s->parallel), TYPE_ISA_PARALLEL);
+    object_initialize(&s->rtc, sizeof(s->rtc), TYPE_MC146818_RTC);
 
     object_property_add_alias(obj, "floppy", OBJECT(&s->floppy), "driveA",
                               &error_abort);
hw/mips/mips_malta.c +0 -5
index 30fb30fc0e..3d304a6e0a 100644
@@ -995,7 +995,6 @@ void mips_malta_init(MachineState *machine)
     uint8_t *smbus_eeprom_buf = g_malloc0(smbus_eeprom_size);
     int64_t kernel_entry, bootloader_run_addr;
     PCIBus *pci_bus;
-    ISABus *isa_bus;
     qemu_irq cbus_irq, i8259_irq;
     PCIDevice *pci;
     int piix4_devfn;

@@ -1197,7 +1196,6 @@ void mips_malta_init(MachineState *machine)
     qdev_prop_set_chr(dev, "parallel", parallel_hds[0]);
 
     qdev_init_nofail(dev);
-    isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
     piix4_devfn = pci->devfn;
 
     /* Interrupt controller */

@@ -1213,9 +1211,6 @@ void mips_malta_init(MachineState *machine)
     smbus_eeprom_init(smbus, 8, smbus_eeprom_buf, smbus_eeprom_size);
     g_free(smbus_eeprom_buf);
 
-    /* Super I/O */
-    mc146818_rtc_init(isa_bus, 2000, NULL);
-
     /* Network card */
     network_init(pci_bus);
Signature · 1 line
2.11.0
author
#23 lore raw

PATCH v3 13/16 piix4: add a speaker as specified in datasheet

In reply to Hervé Poussineau
Signed-off-by Hervé Poussineau <[email protected]>
---
 hw/isa/piix4.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
hw/isa/piix4.c +6 -1
index 09388ff286..64526e1bb8 100644
@@ -30,6 +30,7 @@
 #include "hw/isa/isa.h"
 #include "hw/char/isa.h"
 #include "hw/sysbus.h"
+#include "hw/audio/pcspk.h"
 #include "hw/timer/i8254.h"
 #include "hw/timer/mc146818rtc.h"
 #include "qapi/error.h"

@@ -144,6 +145,7 @@ static void piix4_realize(PCIDevice *pci, Error **errp)
     DeviceState *dev = DEVICE(pci);
     PIIX4State *s = DO_UPCAST(PIIX4State, dev, pci);
     ISABus *isa_bus;
+    ISADevice *pit;
     qemu_irq *i8259_out_irq;
     int i;
     Error *err = NULL;

@@ -170,7 +172,10 @@ static void piix4_realize(PCIDevice *pci, Error **errp)
     isa_bus_irqs(isa_bus, s->isa);
 
     /* initialize pit */
-    i8254_pit_init(isa_bus, 0x40, 0, NULL);
+    pit = i8254_pit_init(isa_bus, 0x40, 0, NULL);
+
+    /* speaker */
+    pcspk_init(isa_bus, pit);
 
     /* DMA */
     DMA_init(isa_bus, 0);
Signature · 1 line
2.11.0
author
#24 lore raw

PATCH v3 14/16 piix4: convert reset function to QOM

In reply to Hervé Poussineau
Signed-off-by Hervé Poussineau <[email protected]>
---
 hw/isa/piix4.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
hw/isa/piix4.c +4 -4
index 64526e1bb8..c6691a6914 100644
@@ -56,10 +56,10 @@ typedef struct PIIX4State {
 #define PIIX4_PCI_DEVICE(obj) \
     OBJECT_CHECK(PIIX4State, (obj), TYPE_PIIX4_PCI_DEVICE)
 
-static void piix4_reset(void *opaque)
+static void piix4_reset(DeviceState *dev)
 {
-    PIIX4State *d = opaque;
-    uint8_t *pci_conf = d->dev.config;
+    PIIX4State *s = PIIX4_PCI_DEVICE(dev);
+    uint8_t *pci_conf = s->dev.config;
 
     pci_conf[0x04] = 0x07; // master, memory and I/O
     pci_conf[0x05] = 0x00;

@@ -230,7 +230,6 @@ static void piix4_realize(PCIDevice *pci, Error **errp)
     isa_init_irq(ISA_DEVICE(&s->rtc), &s->rtc.irq, RTC_ISA_IRQ);
 
     piix4_dev = pci;
-    qemu_register_reset(piix4_reset, s);
 }
 
 static void piix4_init(Object *obj)

@@ -264,6 +263,7 @@ static void piix4_class_init(ObjectClass *klass, void *data)
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->device_id = PCI_DEVICE_ID_INTEL_82371AB_0;
     k->class_id = PCI_CLASS_BRIDGE_ISA;
+    dc->reset = piix4_reset;
     dc->desc = "ISA bridge";
     dc->vmsd = &vmstate_piix4;
     /*
Signature · 1 line
2.11.0
author
#25 lore raw

PATCH v3 15/16 piix4: rename PIIX4 object to piix4-isa

In reply to Hervé Poussineau

Other piix4 parts are already named piix4-ide and piix4-usb-uhci.

Reviewed-by Philippe Mathieu-Daudé <[email protected]>
Signed-off-by Hervé Poussineau <[email protected]>
---
 hw/isa/piix4.c       | 1 -
 hw/mips/mips_malta.c | 2 +-
 include/hw/isa/isa.h | 3 +++
 3 files changed, 4 insertions(+), 2 deletions(-)
hw/isa/piix4.c +0 -1
index c6691a6914..8e99087c21 100644
@@ -52,7 +52,6 @@ typedef struct PIIX4State {
     uint8_t rcr;
 } PIIX4State;
 
-#define TYPE_PIIX4_PCI_DEVICE "PIIX4"
 #define PIIX4_PCI_DEVICE(obj) \
     OBJECT_CHECK(PIIX4State, (obj), TYPE_PIIX4_PCI_DEVICE)
hw/mips/mips_malta.c +1 -1
index 3d304a6e0a..93ec013ea2 100644
@@ -1178,7 +1178,7 @@ void mips_malta_init(MachineState *machine)
     ide_drive_get(hd, ARRAY_SIZE(hd));
 
     pci = pci_create_multifunction(pci_bus, PCI_DEVFN(10, 0),
-                                   true, "PIIX4");
+                                   true, TYPE_PIIX4_PCI_DEVICE);
     dev = DEVICE(pci);
 
     /* Floppy */
include/hw/isa/isa.h +3 -0
index 95593408ef..1f47692447 100644
@@ -153,4 +153,7 @@ static inline ISABus *isa_bus_from_device(ISADevice *d)
 
 /* i8257.c */
 void DMA_init(ISABus *bus, int high_page_enable);
+
+#define TYPE_PIIX4_PCI_DEVICE "piix4-isa"
+
 #endif
Signature · 1 line
2.11.0
author
#26 lore raw

PATCH v3 16/16 piix4: we can now instanciate a PIIX4 with -device

In reply to Hervé Poussineau

Note that the PIC master can't be connected to CPU using the command line, but it's not necessary to have a working ISA bus (for I/O, memory and DMA).

Signed-off-by Hervé Poussineau <[email protected]>
---
 hw/isa/piix4.c | 6 ------
 1 file changed, 6 deletions(-)
hw/isa/piix4.c +0 -6
index 8e99087c21..50a45d8f98 100644
@@ -265,12 +265,6 @@ static void piix4_class_init(ObjectClass *klass, void *data)
     dc->reset = piix4_reset;
     dc->desc = "ISA bridge";
     dc->vmsd = &vmstate_piix4;
-    /*
-     * Reason: part of PIIX4 southbridge, needs to be wired up,
-     * e.g. by mips_malta_init()
-     */
-    dc->user_creatable = false;
-    dc->hotpluggable = false;
 }
 
 static const TypeInfo piix4_info = {
Signature · 1 line
2.11.0
Acked-by on the v3 series
#27 lore raw
In reply to Hervé Poussineau
Quoted 7 lines · Hervé Poussineau

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.

Quoted 5 lines

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 <[email protected]>

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.

Quoted 45 lines

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
Signature · 1 line
2.11.0
Acked-by on the v3 series
#28 lore raw
In reply to Hervé Poussineau
Quoted 10 lines · Hervé Poussineau

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 <[email protected]>

Paolo

Quoted 45 lines

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
#29 lore raw
Quoted 5 lines
Quoted 5 lines

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.

Quoted 7 lines
Quoted 3 lines

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 ...

Quoted 5 lines
            /* 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.