[Qemu-devel] [PATCH 0/7] Some more AHCI work v2

[Qemu-devel] [PATCH 0/7] Some more AHCI work v2

[Alexander Graf]

Clearly, AHCI as is is not perfect yet (intentionally, release early,
release often, remember?). This patch set makes it work with SeaBIOS
so booting Windows 7 works flawlessly for me. it also adds some speedups
and fixes a level based interrupts, rendering ahci useful on PPC targets.

In preparation of potential non-ich9 implementations, this set also
splits ahci code from concrete ich9 specific code. That way we can
later on create other AHCI adapters while reusing a lot of code.

Git tree (including BIOS patch to enable booting from AHCI):

 git://repo.or.cz/qemu/ahci.git ahci


v1 -> v2:

 - drop dma helper removal
 - drop "free dynamically allocated iovs" patch
 - add "add license header in ahci.h"
 - rephrase interrupt bugfix
 - add comment on d2h delay hack


Alexander Graf (6):
  ahci: add license header in ahci.h
  ahci: split ICH and AHCI even more
  ahci: send init d2h fis on fis enable
  ahci: Implement HBA reset
  ahci: make number of ports runtime determined
  ahci: work around bug with level interrupts

Sebastian Herbszt (1):
  ahci: split ICH9 from core

 Makefile.objs |    1 +
 hw/ide/ahci.c |  488 +++++++--------------------------------------------------
 hw/ide/ahci.h |  333 +++++++++++++++++++++++++++++++++++++++
 hw/ide/ich.c  |  148 +++++++++++++++++
 4 files changed, 540 insertions(+), 430 deletions(-)
 create mode 100644 hw/ide/ahci.h
 create mode 100644 hw/ide/ich.c

[Qemu-devel] [PATCH 1/7] ahci: split ICH9 from core

[Alexander Graf]

From: Sebastian Herbszt <herbszt@gmx.de>

There are multiple ahci devices out there. The currently implemented ich-9
is only one of the many. So let's split that one out into a separate file
to stress the difference.

Signed-off-by: Sebastian Herbszt <herbszt@gmx.de>
Signed-off-by: Alexander Graf <agraf@suse.de>
---
 Makefile.objs |    1 +
 hw/ide/ahci.c |  305 +-------------------------------------------------------
 hw/ide/ahci.h |  309 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/ide/ich.c  |   61 +++++++++++
 4 files changed, 375 insertions(+), 301 deletions(-)
 create mode 100644 hw/ide/ahci.h
 create mode 100644 hw/ide/ich.c

diff --git a/Makefile.objs b/Makefile.objs
index 93406ff..b15df5e 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -244,6 +244,7 @@ hw-obj-$(CONFIG_IDE_CMD646) += ide/cmd646.o
 hw-obj-$(CONFIG_IDE_MACIO) += ide/macio.o
 hw-obj-$(CONFIG_IDE_VIA) += ide/via.o
 hw-obj-$(CONFIG_AHCI) += ide/ahci.o
+hw-obj-$(CONFIG_AHCI) += ide/ich.o
 
 # SCSI layer
 hw-obj-$(CONFIG_LSI_SCSI_PCI) += lsi53c895a.o
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 671b4df..28412d0 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -72,6 +72,7 @@
 #include "cpu-common.h"
 #include "internal.h"
 #include <hw/ide/pci.h>
+#include <hw/ide/ahci.h>
 
 /* #define DEBUG_AHCI */
 
@@ -83,304 +84,6 @@ do { fprintf(stderr, "ahci: %s: [%d] ", __FUNCTION__, port); \
 #define DPRINTF(port, fmt, ...) do {} while(0)
 #endif
 
-#define AHCI_PCI_BAR              5
-#define AHCI_MAX_PORTS            32
-#define AHCI_MAX_SG               168 /* hardware max is 64K */
-#define AHCI_DMA_BOUNDARY         0xffffffff
-#define AHCI_USE_CLUSTERING       0
-#define AHCI_MAX_CMDS             32
-#define AHCI_CMD_SZ               32
-#define AHCI_CMD_SLOT_SZ          (AHCI_MAX_CMDS * AHCI_CMD_SZ)
-#define AHCI_RX_FIS_SZ            256
-#define AHCI_CMD_TBL_CDB          0x40
-#define AHCI_CMD_TBL_HDR_SZ       0x80
-#define AHCI_CMD_TBL_SZ           (AHCI_CMD_TBL_HDR_SZ + (AHCI_MAX_SG * 16))
-#define AHCI_CMD_TBL_AR_SZ        (AHCI_CMD_TBL_SZ * AHCI_MAX_CMDS)
-#define AHCI_PORT_PRIV_DMA_SZ     (AHCI_CMD_SLOT_SZ + AHCI_CMD_TBL_AR_SZ + \
-                                   AHCI_RX_FIS_SZ)
-
-#define AHCI_IRQ_ON_SG            (1 << 31)
-#define AHCI_CMD_ATAPI            (1 << 5)
-#define AHCI_CMD_WRITE            (1 << 6)
-#define AHCI_CMD_PREFETCH         (1 << 7)
-#define AHCI_CMD_RESET            (1 << 8)
-#define AHCI_CMD_CLR_BUSY         (1 << 10)
-
-#define RX_FIS_D2H_REG            0x40 /* offset of D2H Register FIS data */
-#define RX_FIS_SDB                0x58 /* offset of SDB FIS data */
-#define RX_FIS_UNK                0x60 /* offset of Unknown FIS data */
-
-/* global controller registers */
-#define HOST_CAP                  0x00 /* host capabilities */
-#define HOST_CTL                  0x04 /* global host control */
-#define HOST_IRQ_STAT             0x08 /* interrupt status */
-#define HOST_PORTS_IMPL           0x0c /* bitmap of implemented ports */
-#define HOST_VERSION              0x10 /* AHCI spec. version compliancy */
-
-/* HOST_CTL bits */
-#define HOST_CTL_RESET            (1 << 0)  /* reset controller; self-clear */
-#define HOST_CTL_IRQ_EN           (1 << 1)  /* global IRQ enable */
-#define HOST_CTL_AHCI_EN          (1 << 31) /* AHCI enabled */
-
-/* HOST_CAP bits */
-#define HOST_CAP_SSC              (1 << 14) /* Slumber capable */
-#define HOST_CAP_AHCI             (1 << 18) /* AHCI only */
-#define HOST_CAP_CLO              (1 << 24) /* Command List Override support */
-#define HOST_CAP_SSS              (1 << 27) /* Staggered Spin-up */
-#define HOST_CAP_NCQ              (1 << 30) /* Native Command Queueing */
-#define HOST_CAP_64               (1 << 31) /* PCI DAC (64-bit DMA) support */
-
-/* registers for each SATA port */
-#define PORT_LST_ADDR             0x00 /* command list DMA addr */
-#define PORT_LST_ADDR_HI          0x04 /* command list DMA addr hi */
-#define PORT_FIS_ADDR             0x08 /* FIS rx buf addr */
-#define PORT_FIS_ADDR_HI          0x0c /* FIS rx buf addr hi */
-#define PORT_IRQ_STAT             0x10 /* interrupt status */
-#define PORT_IRQ_MASK             0x14 /* interrupt enable/disable mask */
-#define PORT_CMD                  0x18 /* port command */
-#define PORT_TFDATA               0x20 /* taskfile data */
-#define PORT_SIG                  0x24 /* device TF signature */
-#define PORT_SCR_STAT             0x28 /* SATA phy register: SStatus */
-#define PORT_SCR_CTL              0x2c /* SATA phy register: SControl */
-#define PORT_SCR_ERR              0x30 /* SATA phy register: SError */
-#define PORT_SCR_ACT              0x34 /* SATA phy register: SActive */
-#define PORT_CMD_ISSUE            0x38 /* command issue */
-#define PORT_RESERVED             0x3c /* reserved */
-
-/* PORT_IRQ_{STAT,MASK} bits */
-#define PORT_IRQ_COLD_PRES        (1 << 31) /* cold presence detect */
-#define PORT_IRQ_TF_ERR           (1 << 30) /* task file error */
-#define PORT_IRQ_HBUS_ERR         (1 << 29) /* host bus fatal error */
-#define PORT_IRQ_HBUS_DATA_ERR    (1 << 28) /* host bus data error */
-#define PORT_IRQ_IF_ERR           (1 << 27) /* interface fatal error */
-#define PORT_IRQ_IF_NONFATAL      (1 << 26) /* interface non-fatal error */
-#define PORT_IRQ_OVERFLOW         (1 << 24) /* xfer exhausted available S/G */
-#define PORT_IRQ_BAD_PMP          (1 << 23) /* incorrect port multiplier */
-
-#define PORT_IRQ_PHYRDY           (1 << 22) /* PhyRdy changed */
-#define PORT_IRQ_DEV_ILCK         (1 << 7) /* device interlock */
-#define PORT_IRQ_CONNECT          (1 << 6) /* port connect change status */
-#define PORT_IRQ_SG_DONE          (1 << 5) /* descriptor processed */
-#define PORT_IRQ_UNK_FIS          (1 << 4) /* unknown FIS rx'd */
-#define PORT_IRQ_SDB_FIS          (1 << 3) /* Set Device Bits FIS rx'd */
-#define PORT_IRQ_DMAS_FIS         (1 << 2) /* DMA Setup FIS rx'd */
-#define PORT_IRQ_PIOS_FIS         (1 << 1) /* PIO Setup FIS rx'd */
-#define PORT_IRQ_D2H_REG_FIS      (1 << 0) /* D2H Register FIS rx'd */
-
-#define PORT_IRQ_FREEZE           (PORT_IRQ_HBUS_ERR | PORT_IRQ_IF_ERR |   \
-                                   PORT_IRQ_CONNECT | PORT_IRQ_PHYRDY |    \
-                                   PORT_IRQ_UNK_FIS)
-#define PORT_IRQ_ERROR            (PORT_IRQ_FREEZE | PORT_IRQ_TF_ERR |     \
-                                   PORT_IRQ_HBUS_DATA_ERR)
-#define DEF_PORT_IRQ              (PORT_IRQ_ERROR | PORT_IRQ_SG_DONE |     \
-                                   PORT_IRQ_SDB_FIS | PORT_IRQ_DMAS_FIS |  \
-                                   PORT_IRQ_PIOS_FIS | PORT_IRQ_D2H_REG_FIS)
-
-/* PORT_CMD bits */
-#define PORT_CMD_ATAPI            (1 << 24) /* Device is ATAPI */
-#define PORT_CMD_LIST_ON          (1 << 15) /* cmd list DMA engine running */
-#define PORT_CMD_FIS_ON           (1 << 14) /* FIS DMA engine running */
-#define PORT_CMD_FIS_RX           (1 << 4) /* Enable FIS receive DMA engine */
-#define PORT_CMD_CLO              (1 << 3) /* Command list override */
-#define PORT_CMD_POWER_ON         (1 << 2) /* Power up device */
-#define PORT_CMD_SPIN_UP          (1 << 1) /* Spin up device */
-#define PORT_CMD_START            (1 << 0) /* Enable port DMA engine */
-
-#define PORT_CMD_ICC_MASK         (0xf << 28) /* i/f ICC state mask */
-#define PORT_CMD_ICC_ACTIVE       (0x1 << 28) /* Put i/f in active state */
-#define PORT_CMD_ICC_PARTIAL      (0x2 << 28) /* Put i/f in partial state */
-#define PORT_CMD_ICC_SLUMBER      (0x6 << 28) /* Put i/f in slumber state */
-
-#define PORT_IRQ_STAT_DHRS        (1 << 0) /* Device to Host Register FIS */
-#define PORT_IRQ_STAT_PSS         (1 << 1) /* PIO Setup FIS */
-#define PORT_IRQ_STAT_DSS         (1 << 2) /* DMA Setup FIS */
-#define PORT_IRQ_STAT_SDBS        (1 << 3) /* Set Device Bits */
-#define PORT_IRQ_STAT_UFS         (1 << 4) /* Unknown FIS */
-#define PORT_IRQ_STAT_DPS         (1 << 5) /* Descriptor Processed */
-#define PORT_IRQ_STAT_PCS         (1 << 6) /* Port Connect Change Status */
-#define PORT_IRQ_STAT_DMPS        (1 << 7) /* Device Mechanical Presence
-                                              Status */
-#define PORT_IRQ_STAT_PRCS        (1 << 22) /* File Ready Status */
-#define PORT_IRQ_STAT_IPMS        (1 << 23) /* Incorrect Port Multiplier
-                                               Status */
-#define PORT_IRQ_STAT_OFS         (1 << 24) /* Overflow Status */
-#define PORT_IRQ_STAT_INFS        (1 << 26) /* Interface Non-Fatal Error
-                                               Status */
-#define PORT_IRQ_STAT_IFS         (1 << 27) /* Interface Fatal Error */
-#define PORT_IRQ_STAT_HBDS        (1 << 28) /* Host Bus Data Error Status */
-#define PORT_IRQ_STAT_HBFS        (1 << 29) /* Host Bus Fatal Error Status */
-#define PORT_IRQ_STAT_TFES        (1 << 30) /* Task File Error Status */
-#define PORT_IRQ_STAT_CPDS        (1 << 31) /* Code Port Detect Status */
-
-/* ap->flags bits */
-#define AHCI_FLAG_NO_NCQ                  (1 << 24)
-#define AHCI_FLAG_IGN_IRQ_IF_ERR          (1 << 25) /* ignore IRQ_IF_ERR */
-#define AHCI_FLAG_HONOR_PI                (1 << 26) /* honor PORTS_IMPL */
-#define AHCI_FLAG_IGN_SERR_INTERNAL       (1 << 27) /* ignore SERR_INTERNAL */
-#define AHCI_FLAG_32BIT_ONLY              (1 << 28) /* force 32bit */
-
-#define ATA_SRST                          (1 << 2)  /* software reset */
-
-#define STATE_RUN                         0
-#define STATE_RESET                       1
-
-#define SATA_SCR_SSTATUS_DET_NODEV        0x0
-#define SATA_SCR_SSTATUS_DET_DEV_PRESENT_PHY_UP 0x3
-
-#define SATA_SCR_SSTATUS_SPD_NODEV        0x00
-#define SATA_SCR_SSTATUS_SPD_GEN1         0x10
-
-#define SATA_SCR_SSTATUS_IPM_NODEV        0x000
-#define SATA_SCR_SSTATUS_IPM_ACTIVE       0X100
-
-#define AHCI_SCR_SCTL_DET                 0xf
-
-#define SATA_FIS_TYPE_REGISTER_H2D        0x27
-#define SATA_FIS_REG_H2D_UPDATE_COMMAND_REGISTER 0x80
-
-#define AHCI_CMD_HDR_CMD_FIS_LEN           0x1f
-#define AHCI_CMD_HDR_PRDT_LEN              16
-
-#define SATA_SIGNATURE_CDROM               0xeb140000
-#define SATA_SIGNATURE_DISK                0x00000101
-
-#define AHCI_GENERIC_HOST_CONTROL_REGS_MAX_ADDR 0x20
-                                            /* Shouldn't this be 0x2c? */
-
-#define SATA_PORTS                         4
-
-#define AHCI_PORT_REGS_START_ADDR          0x100
-#define AHCI_PORT_REGS_END_ADDR (AHCI_PORT_REGS_START_ADDR + SATA_PORTS * 0x80)
-#define AHCI_PORT_ADDR_OFFSET_MASK         0x7f
-
-#define AHCI_NUM_COMMAND_SLOTS             31
-#define AHCI_SUPPORTED_SPEED               20
-#define AHCI_SUPPORTED_SPEED_GEN1          1
-#define AHCI_VERSION_1_0                   0x10000
-
-#define AHCI_PROGMODE_MAJOR_REV_1          1
-
-#define AHCI_COMMAND_TABLE_ACMD            0x40
-
-#define IDE_FEATURE_DMA                    1
-
-#define READ_FPDMA_QUEUED                  0x60
-#define WRITE_FPDMA_QUEUED                 0x61
-
-#define RES_FIS_DSFIS                      0x00
-#define RES_FIS_PSFIS                      0x20
-#define RES_FIS_RFIS                       0x40
-#define RES_FIS_SDBFIS                     0x58
-#define RES_FIS_UFIS                       0x60
-
-typedef struct AHCIControlRegs {
-    uint32_t    cap;
-    uint32_t    ghc;
-    uint32_t    irqstatus;
-    uint32_t    impl;
-    uint32_t    version;
-} AHCIControlRegs;
-
-typedef struct AHCIPortRegs {
-    uint32_t    lst_addr;
-    uint32_t    lst_addr_hi;
-    uint32_t    fis_addr;
-    uint32_t    fis_addr_hi;
-    uint32_t    irq_stat;
-    uint32_t    irq_mask;
-    uint32_t    cmd;
-    uint32_t    unused0;
-    uint32_t    tfdata;
-    uint32_t    sig;
-    uint32_t    scr_stat;
-    uint32_t    scr_ctl;
-    uint32_t    scr_err;
-    uint32_t    scr_act;
-    uint32_t    cmd_issue;
-    uint32_t    reserved;
-} AHCIPortRegs;
-
-typedef struct AHCICmdHdr {
-    uint32_t    opts;
-    uint32_t    status;
-    uint64_t    tbl_addr;
-    uint32_t    reserved[4];
-} __attribute__ ((packed)) AHCICmdHdr;
-
-typedef struct AHCI_SG {
-    uint64_t    addr;
-    uint32_t    reserved;
-    uint32_t    flags_size;
-} __attribute__ ((packed)) AHCI_SG;
-
-typedef struct AHCIDevice AHCIDevice;
-
-typedef struct NCQTransferState {
-    AHCIDevice *drive;
-    BlockDriverAIOCB *aiocb;
-    QEMUSGList sglist;
-    int is_read;
-    uint16_t sector_count;
-    uint64_t lba;
-    uint8_t tag;
-    int slot;
-    int used;
-} NCQTransferState;
-
-struct AHCIDevice {
-    IDEDMA dma;
-    IDEBus port;
-    int port_no;
-    uint32_t port_state;
-    uint32_t finished;
-    AHCIPortRegs port_regs;
-    struct AHCIState *hba;
-    QEMUBH *check_bh;
-    uint8_t *lst;
-    uint8_t *res_fis;
-    int dma_status;
-    int done_atapi_packet;
-    int busy_slot;
-    BlockDriverCompletionFunc *dma_cb;
-    AHCICmdHdr *cur_cmd;
-    NCQTransferState ncq_tfs[AHCI_MAX_CMDS];
-};
-
-typedef struct AHCIState {
-    AHCIDevice dev[SATA_PORTS];
-    AHCIControlRegs control_regs;
-    int mem;
-    qemu_irq irq;
-} AHCIState;
-
-typedef struct AHCIPCIState {
-    PCIDevice card;
-    AHCIState ahci;
-} AHCIPCIState;
-
-typedef struct NCQFrame {
-    uint8_t fis_type;
-    uint8_t c;
-    uint8_t command;
-    uint8_t sector_count_low;
-    uint8_t lba0;
-    uint8_t lba1;
-    uint8_t lba2;
-    uint8_t fua;
-    uint8_t lba3;
-    uint8_t lba4;
-    uint8_t lba5;
-    uint8_t sector_count_high;
-    uint8_t tag;
-    uint8_t reserved5;
-    uint8_t reserved6;
-    uint8_t control;
-    uint8_t reserved7;
-    uint8_t reserved8;
-    uint8_t reserved9;
-    uint8_t reserved10;
-} __attribute__ ((packed)) NCQFrame;
-
 static void check_cmd(AHCIState *s, int port);
 static int handle_cmd(AHCIState *s,int port,int slot);
 static void ahci_reset_port(AHCIState *s, int port);
@@ -1410,7 +1113,7 @@ static const IDEDMAOps ahci_dma_ops = {
     .reset = ahci_dma_reset,
 };
 
-static void ahci_init(AHCIState *s, DeviceState *qdev)
+void ahci_init(AHCIState *s, DeviceState *qdev)
 {
     qemu_irq *irqs;
     int i;
@@ -1434,7 +1137,7 @@ static void ahci_init(AHCIState *s, DeviceState *qdev)
     }
 }
 
-static void ahci_pci_map(PCIDevice *pci_dev, int region_num,
+void ahci_pci_map(PCIDevice *pci_dev, int region_num,
         pcibus_t addr, pcibus_t size, int type)
 {
     struct AHCIPCIState *d = (struct AHCIPCIState *)pci_dev;
@@ -1443,7 +1146,7 @@ static void ahci_pci_map(PCIDevice *pci_dev, int region_num,
     cpu_register_physical_memory(addr, size, s->mem);
 }
 
-static void ahci_reset(void *opaque)
+void ahci_reset(void *opaque)
 {
     struct AHCIPCIState *d = opaque;
     int i;
diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
new file mode 100644
index 0000000..63ef785
--- /dev/null
+++ b/hw/ide/ahci.h
@@ -0,0 +1,309 @@
+#ifndef HW_IDE_AHCI_H
+#define HW_IDE_AHCI_H
+
+#define AHCI_PCI_BAR              5
+#define AHCI_MAX_PORTS            32
+#define AHCI_MAX_SG               168 /* hardware max is 64K */
+#define AHCI_DMA_BOUNDARY         0xffffffff
+#define AHCI_USE_CLUSTERING       0
+#define AHCI_MAX_CMDS             32
+#define AHCI_CMD_SZ               32
+#define AHCI_CMD_SLOT_SZ          (AHCI_MAX_CMDS * AHCI_CMD_SZ)
+#define AHCI_RX_FIS_SZ            256
+#define AHCI_CMD_TBL_CDB          0x40
+#define AHCI_CMD_TBL_HDR_SZ       0x80
+#define AHCI_CMD_TBL_SZ           (AHCI_CMD_TBL_HDR_SZ + (AHCI_MAX_SG * 16))
+#define AHCI_CMD_TBL_AR_SZ        (AHCI_CMD_TBL_SZ * AHCI_MAX_CMDS)
+#define AHCI_PORT_PRIV_DMA_SZ     (AHCI_CMD_SLOT_SZ + AHCI_CMD_TBL_AR_SZ + \
+                                   AHCI_RX_FIS_SZ)
+
+#define AHCI_IRQ_ON_SG            (1 << 31)
+#define AHCI_CMD_ATAPI            (1 << 5)
+#define AHCI_CMD_WRITE            (1 << 6)
+#define AHCI_CMD_PREFETCH         (1 << 7)
+#define AHCI_CMD_RESET            (1 << 8)
+#define AHCI_CMD_CLR_BUSY         (1 << 10)
+
+#define RX_FIS_D2H_REG            0x40 /* offset of D2H Register FIS data */
+#define RX_FIS_SDB                0x58 /* offset of SDB FIS data */
+#define RX_FIS_UNK                0x60 /* offset of Unknown FIS data */
+
+/* global controller registers */
+#define HOST_CAP                  0x00 /* host capabilities */
+#define HOST_CTL                  0x04 /* global host control */
+#define HOST_IRQ_STAT             0x08 /* interrupt status */
+#define HOST_PORTS_IMPL           0x0c /* bitmap of implemented ports */
+#define HOST_VERSION              0x10 /* AHCI spec. version compliancy */
+
+/* HOST_CTL bits */
+#define HOST_CTL_RESET            (1 << 0)  /* reset controller; self-clear */
+#define HOST_CTL_IRQ_EN           (1 << 1)  /* global IRQ enable */
+#define HOST_CTL_AHCI_EN          (1 << 31) /* AHCI enabled */
+
+/* HOST_CAP bits */
+#define HOST_CAP_SSC              (1 << 14) /* Slumber capable */
+#define HOST_CAP_AHCI             (1 << 18) /* AHCI only */
+#define HOST_CAP_CLO              (1 << 24) /* Command List Override support */
+#define HOST_CAP_SSS              (1 << 27) /* Staggered Spin-up */
+#define HOST_CAP_NCQ              (1 << 30) /* Native Command Queueing */
+#define HOST_CAP_64               (1 << 31) /* PCI DAC (64-bit DMA) support */
+
+/* registers for each SATA port */
+#define PORT_LST_ADDR             0x00 /* command list DMA addr */
+#define PORT_LST_ADDR_HI          0x04 /* command list DMA addr hi */
+#define PORT_FIS_ADDR             0x08 /* FIS rx buf addr */
+#define PORT_FIS_ADDR_HI          0x0c /* FIS rx buf addr hi */
+#define PORT_IRQ_STAT             0x10 /* interrupt status */
+#define PORT_IRQ_MASK             0x14 /* interrupt enable/disable mask */
+#define PORT_CMD                  0x18 /* port command */
+#define PORT_TFDATA               0x20 /* taskfile data */
+#define PORT_SIG                  0x24 /* device TF signature */
+#define PORT_SCR_STAT             0x28 /* SATA phy register: SStatus */
+#define PORT_SCR_CTL              0x2c /* SATA phy register: SControl */
+#define PORT_SCR_ERR              0x30 /* SATA phy register: SError */
+#define PORT_SCR_ACT              0x34 /* SATA phy register: SActive */
+#define PORT_CMD_ISSUE            0x38 /* command issue */
+#define PORT_RESERVED             0x3c /* reserved */
+
+/* PORT_IRQ_{STAT,MASK} bits */
+#define PORT_IRQ_COLD_PRES        (1 << 31) /* cold presence detect */
+#define PORT_IRQ_TF_ERR           (1 << 30) /* task file error */
+#define PORT_IRQ_HBUS_ERR         (1 << 29) /* host bus fatal error */
+#define PORT_IRQ_HBUS_DATA_ERR    (1 << 28) /* host bus data error */
+#define PORT_IRQ_IF_ERR           (1 << 27) /* interface fatal error */
+#define PORT_IRQ_IF_NONFATAL      (1 << 26) /* interface non-fatal error */
+#define PORT_IRQ_OVERFLOW         (1 << 24) /* xfer exhausted available S/G */
+#define PORT_IRQ_BAD_PMP          (1 << 23) /* incorrect port multiplier */
+
+#define PORT_IRQ_PHYRDY           (1 << 22) /* PhyRdy changed */
+#define PORT_IRQ_DEV_ILCK         (1 << 7) /* device interlock */
+#define PORT_IRQ_CONNECT          (1 << 6) /* port connect change status */
+#define PORT_IRQ_SG_DONE          (1 << 5) /* descriptor processed */
+#define PORT_IRQ_UNK_FIS          (1 << 4) /* unknown FIS rx'd */
+#define PORT_IRQ_SDB_FIS          (1 << 3) /* Set Device Bits FIS rx'd */
+#define PORT_IRQ_DMAS_FIS         (1 << 2) /* DMA Setup FIS rx'd */
+#define PORT_IRQ_PIOS_FIS         (1 << 1) /* PIO Setup FIS rx'd */
+#define PORT_IRQ_D2H_REG_FIS      (1 << 0) /* D2H Register FIS rx'd */
+
+#define PORT_IRQ_FREEZE           (PORT_IRQ_HBUS_ERR | PORT_IRQ_IF_ERR |   \
+                                   PORT_IRQ_CONNECT | PORT_IRQ_PHYRDY |    \
+                                   PORT_IRQ_UNK_FIS)
+#define PORT_IRQ_ERROR            (PORT_IRQ_FREEZE | PORT_IRQ_TF_ERR |     \
+                                   PORT_IRQ_HBUS_DATA_ERR)
+#define DEF_PORT_IRQ              (PORT_IRQ_ERROR | PORT_IRQ_SG_DONE |     \
+                                   PORT_IRQ_SDB_FIS | PORT_IRQ_DMAS_FIS |  \
+                                   PORT_IRQ_PIOS_FIS | PORT_IRQ_D2H_REG_FIS)
+
+/* PORT_CMD bits */
+#define PORT_CMD_ATAPI            (1 << 24) /* Device is ATAPI */
+#define PORT_CMD_LIST_ON          (1 << 15) /* cmd list DMA engine running */
+#define PORT_CMD_FIS_ON           (1 << 14) /* FIS DMA engine running */
+#define PORT_CMD_FIS_RX           (1 << 4) /* Enable FIS receive DMA engine */
+#define PORT_CMD_CLO              (1 << 3) /* Command list override */
+#define PORT_CMD_POWER_ON         (1 << 2) /* Power up device */
+#define PORT_CMD_SPIN_UP          (1 << 1) /* Spin up device */
+#define PORT_CMD_START            (1 << 0) /* Enable port DMA engine */
+
+#define PORT_CMD_ICC_MASK         (0xf << 28) /* i/f ICC state mask */
+#define PORT_CMD_ICC_ACTIVE       (0x1 << 28) /* Put i/f in active state */
+#define PORT_CMD_ICC_PARTIAL      (0x2 << 28) /* Put i/f in partial state */
+#define PORT_CMD_ICC_SLUMBER      (0x6 << 28) /* Put i/f in slumber state */
+
+#define PORT_IRQ_STAT_DHRS        (1 << 0) /* Device to Host Register FIS */
+#define PORT_IRQ_STAT_PSS         (1 << 1) /* PIO Setup FIS */
+#define PORT_IRQ_STAT_DSS         (1 << 2) /* DMA Setup FIS */
+#define PORT_IRQ_STAT_SDBS        (1 << 3) /* Set Device Bits */
+#define PORT_IRQ_STAT_UFS         (1 << 4) /* Unknown FIS */
+#define PORT_IRQ_STAT_DPS         (1 << 5) /* Descriptor Processed */
+#define PORT_IRQ_STAT_PCS         (1 << 6) /* Port Connect Change Status */
+#define PORT_IRQ_STAT_DMPS        (1 << 7) /* Device Mechanical Presence
+                                              Status */
+#define PORT_IRQ_STAT_PRCS        (1 << 22) /* File Ready Status */
+#define PORT_IRQ_STAT_IPMS        (1 << 23) /* Incorrect Port Multiplier
+                                               Status */
+#define PORT_IRQ_STAT_OFS         (1 << 24) /* Overflow Status */
+#define PORT_IRQ_STAT_INFS        (1 << 26) /* Interface Non-Fatal Error
+                                               Status */
+#define PORT_IRQ_STAT_IFS         (1 << 27) /* Interface Fatal Error */
+#define PORT_IRQ_STAT_HBDS        (1 << 28) /* Host Bus Data Error Status */
+#define PORT_IRQ_STAT_HBFS        (1 << 29) /* Host Bus Fatal Error Status */
+#define PORT_IRQ_STAT_TFES        (1 << 30) /* Task File Error Status */
+#define PORT_IRQ_STAT_CPDS        (1 << 31) /* Code Port Detect Status */
+
+/* ap->flags bits */
+#define AHCI_FLAG_NO_NCQ                  (1 << 24)
+#define AHCI_FLAG_IGN_IRQ_IF_ERR          (1 << 25) /* ignore IRQ_IF_ERR */
+#define AHCI_FLAG_HONOR_PI                (1 << 26) /* honor PORTS_IMPL */
+#define AHCI_FLAG_IGN_SERR_INTERNAL       (1 << 27) /* ignore SERR_INTERNAL */
+#define AHCI_FLAG_32BIT_ONLY              (1 << 28) /* force 32bit */
+
+#define ATA_SRST                          (1 << 2)  /* software reset */
+
+#define STATE_RUN                         0
+#define STATE_RESET                       1
+
+#define SATA_SCR_SSTATUS_DET_NODEV        0x0
+#define SATA_SCR_SSTATUS_DET_DEV_PRESENT_PHY_UP 0x3
+
+#define SATA_SCR_SSTATUS_SPD_NODEV        0x00
+#define SATA_SCR_SSTATUS_SPD_GEN1         0x10
+
+#define SATA_SCR_SSTATUS_IPM_NODEV        0x000
+#define SATA_SCR_SSTATUS_IPM_ACTIVE       0X100
+
+#define AHCI_SCR_SCTL_DET                 0xf
+
+#define SATA_FIS_TYPE_REGISTER_H2D        0x27
+#define SATA_FIS_REG_H2D_UPDATE_COMMAND_REGISTER 0x80
+
+#define AHCI_CMD_HDR_CMD_FIS_LEN           0x1f
+#define AHCI_CMD_HDR_PRDT_LEN              16
+
+#define SATA_SIGNATURE_CDROM               0xeb140000
+#define SATA_SIGNATURE_DISK                0x00000101
+
+#define AHCI_GENERIC_HOST_CONTROL_REGS_MAX_ADDR 0x20
+                                            /* Shouldn't this be 0x2c? */
+
+#define SATA_PORTS                         4
+
+#define AHCI_PORT_REGS_START_ADDR          0x100
+#define AHCI_PORT_REGS_END_ADDR (AHCI_PORT_REGS_START_ADDR + SATA_PORTS * 0x80)
+#define AHCI_PORT_ADDR_OFFSET_MASK         0x7f
+
+#define AHCI_NUM_COMMAND_SLOTS             31
+#define AHCI_SUPPORTED_SPEED               20
+#define AHCI_SUPPORTED_SPEED_GEN1          1
+#define AHCI_VERSION_1_0                   0x10000
+
+#define AHCI_PROGMODE_MAJOR_REV_1          1
+
+#define AHCI_COMMAND_TABLE_ACMD            0x40
+
+#define IDE_FEATURE_DMA                    1
+
+#define READ_FPDMA_QUEUED                  0x60
+#define WRITE_FPDMA_QUEUED                 0x61
+
+#define RES_FIS_DSFIS                      0x00
+#define RES_FIS_PSFIS                      0x20
+#define RES_FIS_RFIS                       0x40
+#define RES_FIS_SDBFIS                     0x58
+#define RES_FIS_UFIS                       0x60
+
+typedef struct AHCIControlRegs {
+    uint32_t    cap;
+    uint32_t    ghc;
+    uint32_t    irqstatus;
+    uint32_t    impl;
+    uint32_t    version;
+} AHCIControlRegs;
+
+typedef struct AHCIPortRegs {
+    uint32_t    lst_addr;
+    uint32_t    lst_addr_hi;
+    uint32_t    fis_addr;
+    uint32_t    fis_addr_hi;
+    uint32_t    irq_stat;
+    uint32_t    irq_mask;
+    uint32_t    cmd;
+    uint32_t    unused0;
+    uint32_t    tfdata;
+    uint32_t    sig;
+    uint32_t    scr_stat;
+    uint32_t    scr_ctl;
+    uint32_t    scr_err;
+    uint32_t    scr_act;
+    uint32_t    cmd_issue;
+    uint32_t    reserved;
+} AHCIPortRegs;
+
+typedef struct AHCICmdHdr {
+    uint32_t    opts;
+    uint32_t    status;
+    uint64_t    tbl_addr;
+    uint32_t    reserved[4];
+} __attribute__ ((packed)) AHCICmdHdr;
+
+typedef struct AHCI_SG {
+    uint64_t    addr;
+    uint32_t    reserved;
+    uint32_t    flags_size;
+} __attribute__ ((packed)) AHCI_SG;
+
+typedef struct AHCIDevice AHCIDevice;
+
+typedef struct NCQTransferState {
+    AHCIDevice *drive;
+    BlockDriverAIOCB *aiocb;
+    QEMUSGList sglist;
+    int is_read;
+    uint16_t sector_count;
+    uint64_t lba;
+    uint8_t tag;
+    int slot;
+    int used;
+} NCQTransferState;
+
+struct AHCIDevice {
+    IDEDMA dma;
+    IDEBus port;
+    int port_no;
+    uint32_t port_state;
+    uint32_t finished;
+    AHCIPortRegs port_regs;
+    struct AHCIState *hba;
+    QEMUBH *check_bh;
+    uint8_t *lst;
+    uint8_t *res_fis;
+    int dma_status;
+    int done_atapi_packet;
+    int busy_slot;
+    BlockDriverCompletionFunc *dma_cb;
+    AHCICmdHdr *cur_cmd;
+    NCQTransferState ncq_tfs[AHCI_MAX_CMDS];
+};
+
+typedef struct AHCIState {
+    AHCIDevice dev[SATA_PORTS];
+    AHCIControlRegs control_regs;
+    int mem;
+    qemu_irq irq;
+} AHCIState;
+
+typedef struct AHCIPCIState {
+    PCIDevice card;
+    AHCIState ahci;
+} AHCIPCIState;
+
+typedef struct NCQFrame {
+    uint8_t fis_type;
+    uint8_t c;
+    uint8_t command;
+    uint8_t sector_count_low;
+    uint8_t lba0;
+    uint8_t lba1;
+    uint8_t lba2;
+    uint8_t fua;
+    uint8_t lba3;
+    uint8_t lba4;
+    uint8_t lba5;
+    uint8_t sector_count_high;
+    uint8_t tag;
+    uint8_t reserved5;
+    uint8_t reserved6;
+    uint8_t control;
+    uint8_t reserved7;
+    uint8_t reserved8;
+    uint8_t reserved9;
+    uint8_t reserved10;
+} __attribute__ ((packed)) NCQFrame;
+
+void ahci_init(AHCIState *s, DeviceState *qdev);
+
+void ahci_pci_map(PCIDevice *pci_dev, int region_num,
+        pcibus_t addr, pcibus_t size, int type);
+
+void ahci_reset(void *opaque);
+
+#endif /* HW_IDE_AHCI_H */
diff --git a/hw/ide/ich.c b/hw/ide/ich.c
new file mode 100644
index 0000000..9868b73
--- /dev/null
+++ b/hw/ide/ich.c
@@ -0,0 +1,61 @@
+#include <hw/hw.h>
+#include <hw/msi.h>
+#include <hw/pc.h>
+#include <hw/pci.h>
+#include <hw/isa.h>
+#include "block.h"
+#include "block_int.h"
+#include "sysemu.h"
+#include "dma.h"
+
+#include <hw/ide/pci.h>
+#include <hw/ide/ahci.h>
+
+static int pci_ich9_ahci_initfn(PCIDevice *dev)
+{
+    struct AHCIPCIState *d;
+    d = DO_UPCAST(struct AHCIPCIState, card, dev);
+
+    pci_config_set_vendor_id(d->card.config, PCI_VENDOR_ID_INTEL);
+    pci_config_set_device_id(d->card.config, PCI_DEVICE_ID_INTEL_82801IR);
+
+    pci_config_set_class(d->card.config, PCI_CLASS_STORAGE_SATA);
+    pci_config_set_revision(d->card.config, 0x02);
+    pci_config_set_prog_interface(d->card.config, AHCI_PROGMODE_MAJOR_REV_1);
+
+    d->card.config[PCI_CACHE_LINE_SIZE] = 0x08;  /* Cache line size */
+    d->card.config[PCI_LATENCY_TIMER]   = 0x00;  /* Latency timer */
+    pci_config_set_interrupt_pin(d->card.config, 1);
+
+    /* XXX Software should program this register */
+    d->card.config[0x90]   = 1 << 6; /* Address Map Register - AHCI mode */
+
+    qemu_register_reset(ahci_reset, d);
+
+    /* XXX BAR size should be 1k, but that breaks, so bump it to 4k for now */
+    pci_register_bar(&d->card, 5, 0x1000, PCI_BASE_ADDRESS_SPACE_MEMORY,
+                     ahci_pci_map);
+
+    msi_init(dev, 0x50, 1, true, false);
+
+    ahci_init(&d->ahci, &dev->qdev);
+    d->ahci.irq = d->card.irq[0];
+
+    return 0;
+}
+
+static PCIDeviceInfo ich_ahci_info[] = {
+    {
+        .qdev.name    = "ich9-ahci",
+        .qdev.size    = sizeof(AHCIPCIState),
+        .init         = pci_ich9_ahci_initfn,
+    },{
+        /* end of list */
+    }
+};
+
+static void ich_ahci_register(void)
+{
+    pci_qdev_register_many(ich_ahci_info);
+}
+device_init(ich_ahci_register);
-- 
1.6.0.2

[Qemu-devel] [PATCH 2/7] ahci: add license header in ahci.h

[Alexander Graf]

Due to popular request, this patch adds a license header to ahci.h

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ide/ahci.h |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
index 63ef785..d65b5e3 100644
--- a/hw/ide/ahci.h
+++ b/hw/ide/ahci.h
@@ -1,3 +1,26 @@
+/*
+ * QEMU AHCI Emulation
+ *
+ * Copyright (c) 2010 qiaochong@loongson.cn
+ * Copyright (c) 2010 Roland Elek <elek.roland@gmail.com>
+ * Copyright (c) 2010 Sebastian Herbszt <herbszt@gmx.de>
+ * Copyright (c) 2010 Alexander Graf <agraf@suse.de>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
 #ifndef HW_IDE_AHCI_H
 #define HW_IDE_AHCI_H
 
-- 
1.6.0.2

[Qemu-devel] [PATCH 3/7] ahci: split ICH and AHCI even more

[Alexander Graf]

Sebastian's patch already did a pretty good job at splitting up ICH-9
AHCI code and the AHCI core. We need some more though. Copyright was missing,
the lspci dump belongs to ICH-9, we don't need the AHCI core to have its
own qdev device duplicate.

So let's split them a bit more in this patch, making things easier to
read an understand.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ide/ahci.c |  110 ---------------------------------------------------------
 hw/ide/ich.c  |   90 +++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 88 insertions(+), 112 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 28412d0..6822046 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -19,47 +19,6 @@
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  *
- *
- * lspci dump of a ICH-9 real device in IDE mode (hopefully close enough):
- *
- * 00:1f.2 SATA controller [0106]: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6 port SATA AHCI Controller [8086:2922] (rev 02) (prog-if 01 [AHCI 1.0])
- *         Subsystem: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6 port SATA AHCI Controller [8086:2922]
- *         Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+
- *         Status: Cap+ 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
- *         Latency: 0
- *         Interrupt: pin B routed to IRQ 222
- *         Region 0: I/O ports at d000 [size=8]
- *         Region 1: I/O ports at cc00 [size=4]
- *         Region 2: I/O ports at c880 [size=8]
- *         Region 3: I/O ports at c800 [size=4]
- *         Region 4: I/O ports at c480 [size=32]
- *         Region 5: Memory at febf9000 (32-bit, non-prefetchable) [size=2K]
- *         Capabilities: [80] Message Signalled Interrupts: Mask- 64bit- Count=1/16 Enable+
- *                 Address: fee0f00c  Data: 41d9
- *         Capabilities: [70] Power Management version 3
- *                 Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot+,D3cold-)
- *                 Status: D0 PME-Enable- DSel=0 DScale=0 PME-
- *         Capabilities: [a8] SATA HBA <?>
- *         Capabilities: [b0] Vendor Specific Information <?>
- *         Kernel driver in use: ahci
- *         Kernel modules: ahci
- * 00: 86 80 22 29 07 04 b0 02 02 01 06 01 00 00 00 00
- * 10: 01 d0 00 00 01 cc 00 00 81 c8 00 00 01 c8 00 00
- * 20: 81 c4 00 00 00 90 bf fe 00 00 00 00 86 80 22 29
- * 30: 00 00 00 00 80 00 00 00 00 00 00 00 0f 02 00 00
- * 40: 00 80 00 80 00 00 00 00 00 00 00 00 00 00 00 00
- * 50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
- * 60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
- * 70: 01 a8 03 40 08 00 00 00 00 00 00 00 00 00 00 00
- * 80: 05 70 09 00 0c f0 e0 fe d9 41 00 00 00 00 00 00
- * 90: 40 00 0f 82 93 01 00 00 00 00 00 00 00 00 00 00
- * a0: ac 00 00 00 0a 00 12 00 12 b0 10 00 48 00 00 00
- * b0: 09 00 06 20 00 00 00 00 00 00 00 00 00 00 00 00
- * c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
- * d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
- * e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
- * f0: 00 00 00 00 00 00 00 00 86 0f 02 00 00 00 00 00
- *
  */
 
 #include <hw/hw.h>
@@ -1155,72 +1114,3 @@ void ahci_reset(void *opaque)
         ahci_reset_port(&d->ahci, i);
     }
 }
-
-static int pci_ahci_init(PCIDevice *dev)
-{
-    struct AHCIPCIState *d;
-    d = DO_UPCAST(struct AHCIPCIState, card, dev);
-
-    pci_config_set_vendor_id(d->card.config, PCI_VENDOR_ID_INTEL);
-    pci_config_set_device_id(d->card.config, PCI_DEVICE_ID_INTEL_82801IR);
-
-    pci_config_set_class(d->card.config, PCI_CLASS_STORAGE_SATA);
-    pci_config_set_revision(d->card.config, 0x02);
-    pci_config_set_prog_interface(d->card.config, AHCI_PROGMODE_MAJOR_REV_1);
-
-    d->card.config[PCI_CACHE_LINE_SIZE] = 0x08;  /* Cache line size */
-    d->card.config[PCI_LATENCY_TIMER]   = 0x00;  /* Latency timer */
-    pci_config_set_interrupt_pin(d->card.config, 1);
-
-    /* XXX Software should program this register */
-    d->card.config[0x90]   = 1 << 6; /* Address Map Register - AHCI mode */
-
-    qemu_register_reset(ahci_reset, d);
-
-    /* XXX BAR size should be 1k, but that breaks, so bump it to 4k for now */
-    pci_register_bar(&d->card, 5, 0x1000, PCI_BASE_ADDRESS_SPACE_MEMORY,
-                     ahci_pci_map);
-
-    msi_init(dev, 0x50, 1, true, false);
-
-    ahci_init(&d->ahci, &dev->qdev);
-    d->ahci.irq = d->card.irq[0];
-
-    return 0;
-}
-
-static int pci_ahci_uninit(PCIDevice *dev)
-{
-    struct AHCIPCIState *d;
-    d = DO_UPCAST(struct AHCIPCIState, card, dev);
-
-    if (msi_enabled(dev)) {
-        msi_uninit(dev);
-    }
-
-    qemu_unregister_reset(ahci_reset, d);
-
-    return 0;
-}
-
-static void pci_ahci_write_config(PCIDevice *pci, uint32_t addr,
-                                  uint32_t val, int len)
-{
-    pci_default_write_config(pci, addr, val, len);
-    msi_write_config(pci, addr, val, len);
-}
-
-static PCIDeviceInfo ahci_info = {
-    .qdev.name  = "ahci",
-    .qdev.size  = sizeof(AHCIPCIState),
-    .init       = pci_ahci_init,
-    .exit       = pci_ahci_uninit,
-    .config_write = pci_ahci_write_config,
-};
-
-static void ahci_pci_register_devices(void)
-{
-    pci_qdev_register(&ahci_info);
-}
-
-device_init(ahci_pci_register_devices)
diff --git a/hw/ide/ich.c b/hw/ide/ich.c
index 9868b73..70cb766 100644
--- a/hw/ide/ich.c
+++ b/hw/ide/ich.c
@@ -1,3 +1,65 @@
+/*
+ * QEMU ICH Emulation
+ *
+ * Copyright (c) 2010 Sebastian Herbszt <herbszt@gmx.de>
+ * Copyright (c) 2010 Alexander Graf <agraf@suse.de>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ *
+ * lspci dump of a ICH-9 real device
+ *
+ * 00:1f.2 SATA controller [0106]: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6 port SATA AHCI Controller [8086:2922] (rev 02) (prog-if 01 [AHCI 1.0])
+ *         Subsystem: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6 port SATA AHCI Controller [8086:2922]
+ *         Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+
+ *         Status: Cap+ 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
+ *         Latency: 0
+ *         Interrupt: pin B routed to IRQ 222
+ *         Region 0: I/O ports at d000 [size=8]
+ *         Region 1: I/O ports at cc00 [size=4]
+ *         Region 2: I/O ports at c880 [size=8]
+ *         Region 3: I/O ports at c800 [size=4]
+ *         Region 4: I/O ports at c480 [size=32]
+ *         Region 5: Memory at febf9000 (32-bit, non-prefetchable) [size=2K]
+ *         Capabilities: [80] Message Signalled Interrupts: Mask- 64bit- Count=1/16 Enable+
+ *                 Address: fee0f00c  Data: 41d9
+ *         Capabilities: [70] Power Management version 3
+ *                 Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot+,D3cold-)
+ *                 Status: D0 PME-Enable- DSel=0 DScale=0 PME-
+ *         Capabilities: [a8] SATA HBA <?>
+ *         Capabilities: [b0] Vendor Specific Information <?>
+ *         Kernel driver in use: ahci
+ *         Kernel modules: ahci
+ * 00: 86 80 22 29 07 04 b0 02 02 01 06 01 00 00 00 00
+ * 10: 01 d0 00 00 01 cc 00 00 81 c8 00 00 01 c8 00 00
+ * 20: 81 c4 00 00 00 90 bf fe 00 00 00 00 86 80 22 29
+ * 30: 00 00 00 00 80 00 00 00 00 00 00 00 0f 02 00 00
+ * 40: 00 80 00 80 00 00 00 00 00 00 00 00 00 00 00 00
+ * 50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ * 60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ * 70: 01 a8 03 40 08 00 00 00 00 00 00 00 00 00 00 00
+ * 80: 05 70 09 00 0c f0 e0 fe d9 41 00 00 00 00 00 00
+ * 90: 40 00 0f 82 93 01 00 00 00 00 00 00 00 00 00 00
+ * a0: ac 00 00 00 0a 00 12 00 12 b0 10 00 48 00 00 00
+ * b0: 09 00 06 20 00 00 00 00 00 00 00 00 00 00 00 00
+ * c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ * d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ * e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ * f0: 00 00 00 00 00 00 00 00 86 0f 02 00 00 00 00 00
+ *
+ */
+
 #include <hw/hw.h>
 #include <hw/msi.h>
 #include <hw/pc.h>
@@ -11,7 +73,7 @@
 #include <hw/ide/pci.h>
 #include <hw/ide/ahci.h>
 
-static int pci_ich9_ahci_initfn(PCIDevice *dev)
+static int pci_ich9_ahci_init(PCIDevice *dev)
 {
     struct AHCIPCIState *d;
     d = DO_UPCAST(struct AHCIPCIState, card, dev);
@@ -44,11 +106,35 @@ static int pci_ich9_ahci_initfn(PCIDevice *dev)
     return 0;
 }
 
+static int pci_ich9_uninit(PCIDevice *dev)
+{
+    struct AHCIPCIState *d;
+    d = DO_UPCAST(struct AHCIPCIState, card, dev);
+
+    if (msi_enabled(dev)) {
+        msi_uninit(dev);
+    }
+
+    qemu_unregister_reset(ahci_reset, d);
+
+    return 0;
+}
+
+static void pci_ich9_write_config(PCIDevice *pci, uint32_t addr,
+                                  uint32_t val, int len)
+{
+    pci_default_write_config(pci, addr, val, len);
+    msi_write_config(pci, addr, val, len);
+}
+
 static PCIDeviceInfo ich_ahci_info[] = {
     {
         .qdev.name    = "ich9-ahci",
+        .qdev.alias   = "ahci",
         .qdev.size    = sizeof(AHCIPCIState),
-        .init         = pci_ich9_ahci_initfn,
+        .init         = pci_ich9_ahci_init,
+        .exit         = pci_ich9_uninit,
+        .config_write = pci_ich9_write_config,
     },{
         /* end of list */
     }
-- 
1.6.0.2

[Qemu-devel] [PATCH 4/7] ahci: send init d2h fis on fis enable

[Alexander Graf]

The drive sends a d2h init fis on initialization. Usually, the guest doesn't
receive fises yet at that point though, so the delivery is deferred.

Let's reflect that by sending the init fis on fis receive enablement.

Signed-off-by: Alexander Graf <agraf@suse.de>

---

v1 -> v2:

  - add comment on d2h delay hack
---
 hw/ide/ahci.c |   38 +++++++++++++++++++++++++++++++-------
 hw/ide/ahci.h |    1 +
 2 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 6822046..e6ac77c 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -47,6 +47,7 @@ static void check_cmd(AHCIState *s, int port);
 static int handle_cmd(AHCIState *s,int port,int slot);
 static void ahci_reset_port(AHCIState *s, int port);
 static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t *cmd_fis);
+static void ahci_init_d2h(AHCIDevice *ad);
 
 static uint32_t  ahci_port_read(AHCIState *s, int port, int offset)
 {
@@ -230,6 +231,16 @@ static void  ahci_port_write(AHCIState *s, int port, int offset, uint32_t val)
                 pr->cmd |= PORT_CMD_FIS_ON;
             }
 
+            /* XXX usually the FIS would be pending on the bus here and
+                   issuing deferred until the OS enables FIS receival.
+                   Instead, we only submit it once - which works in most
+                   cases, but is a hack. */
+            if ((pr->cmd & PORT_CMD_FIS_ON) &&
+                !s->dev[port].init_d2h_sent) {
+                ahci_init_d2h(&s->dev[port]);
+                s->dev[port].init_d2h_sent = 1;
+            }
+
             check_cmd(s, port);
             break;
         case PORT_TFDATA:
@@ -462,12 +473,29 @@ static void ahci_check_cmd_bh(void *opaque)
     check_cmd(ad->hba, ad->port_no);
 }
 
+static void ahci_init_d2h(AHCIDevice *ad)
+{
+    uint8_t init_fis[0x20];
+    IDEState *ide_state = &ad->port.ifs[0];
+
+    memset(init_fis, 0, sizeof(init_fis));
+
+    init_fis[4] = 1;
+    init_fis[12] = 1;
+
+    if (ide_state->drive_kind == IDE_CD) {
+        init_fis[5] = ide_state->lcyl;
+        init_fis[6] = ide_state->hcyl;
+    }
+
+    ahci_write_fis_d2h(ad, init_fis);
+}
+
 static void ahci_reset_port(AHCIState *s, int port)
 {
     AHCIDevice *d = &s->dev[port];
     AHCIPortRegs *pr = &d->port_regs;
     IDEState *ide_state = &d->port.ifs[0];
-    uint8_t init_fis[0x20];
     int i;
 
     DPRINTF(port, "reset port\n");
@@ -482,6 +510,7 @@ static void ahci_reset_port(AHCIState *s, int port)
     pr->scr_err = 0;
     pr->scr_act = 0;
     d->busy_slot = -1;
+    d->init_d2h_sent = 0;
 
     ide_state = &s->dev[port].port.ifs[0];
     if (!ide_state->bs) {
@@ -504,7 +533,6 @@ static void ahci_reset_port(AHCIState *s, int port)
         ncq_tfs->used = 0;
     }
 
-    memset(init_fis, 0, sizeof(init_fis));
     s->dev[port].port_state = STATE_RUN;
     if (!ide_state->bs) {
         s->dev[port].port_regs.sig = 0;
@@ -514,8 +542,6 @@ static void ahci_reset_port(AHCIState *s, int port)
         ide_state->lcyl = 0x14;
         ide_state->hcyl = 0xeb;
         DPRINTF(port, "set lcyl = %d\n", ide_state->lcyl);
-        init_fis[5] = ide_state->lcyl;
-        init_fis[6] = ide_state->hcyl;
         ide_state->status = SEEK_STAT | WRERR_STAT | READY_STAT;
     } else {
         s->dev[port].port_regs.sig = SATA_SIGNATURE_DISK;
@@ -523,9 +549,7 @@ static void ahci_reset_port(AHCIState *s, int port)
     }
 
     ide_state->error = 1;
-    init_fis[4] = 1;
-    init_fis[12] = 1;
-    ahci_write_fis_d2h(d, init_fis);
+    ahci_init_d2h(d);
 }
 
 static void debug_print_fis(uint8_t *fis, int cmd_len)
diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
index d65b5e3..b2786d1 100644
--- a/hw/ide/ahci.h
+++ b/hw/ide/ahci.h
@@ -282,6 +282,7 @@ struct AHCIDevice {
     int dma_status;
     int done_atapi_packet;
     int busy_slot;
+    int init_d2h_sent;
     BlockDriverCompletionFunc *dma_cb;
     AHCICmdHdr *cur_cmd;
     NCQTransferState ncq_tfs[AHCI_MAX_CMDS];
-- 
1.6.0.2

[Qemu-devel] [PATCH 5/7] ahci: Implement HBA reset

[Alexander Graf]

The ahci code was missing its soft reset functionality. This wasn't really an
issue for Linux guests, but Windows gets confused when the controller doesn't
reset when it tells it so.

Using this patch I can now successfully boot Windows 7 from AHCI using AHCI
enabled SeaBIOS.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ide/ahci.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index e6ac77c..105dd53 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -335,7 +335,7 @@ static void ahci_mem_writel(void *ptr, target_phys_addr_t addr, uint32_t val)
             case HOST_CTL: /* R/W */
                 if (val & HOST_CTL_RESET) {
                     DPRINTF(-1, "HBA Reset\n");
-                    /* FIXME reset? */
+                    ahci_reset(container_of(s, AHCIPCIState, ahci));
                 } else {
                     s->control_regs.ghc = (val & 0x3) | HOST_CTL_AHCI_EN;
                     ahci_check_irq(s);
@@ -1134,6 +1134,9 @@ void ahci_reset(void *opaque)
     struct AHCIPCIState *d = opaque;
     int i;
 
+    d->ahci.control_regs.irqstatus = 0;
+    d->ahci.control_regs.ghc = 0;
+
     for (i = 0; i < SATA_PORTS; i++) {
         ahci_reset_port(&d->ahci, i);
     }
-- 
1.6.0.2

[Qemu-devel] [PATCH 6/7] ahci: make number of ports runtime determined

[Alexander Graf]

Different AHCI controllers have a different number of ports, so the core
shouldn't care about the amount of ports available.

This patch makes the number of ports available to the AHCI core runtime
configurable, allowing us to have multiple different AHCI implementations
with different amounts of ports.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ide/ahci.c |   29 +++++++++++++++++++----------
 hw/ide/ahci.h |   10 +++++-----
 hw/ide/ich.c  |    3 ++-
 3 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 105dd53..98bdf70 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -145,7 +145,7 @@ static void ahci_check_irq(AHCIState *s)
 
     DPRINTF(-1, "check irq %#x\n", s->control_regs.irqstatus);
 
-    for (i = 0; i < SATA_PORTS; i++) {
+    for (i = 0; i < s->ports; i++) {
         AHCIPortRegs *pr = &s->dev[i].port_regs;
         if (pr->irq_stat & pr->irq_mask) {
             s->control_regs.irqstatus |= (1 << i);
@@ -303,7 +303,8 @@ static uint32_t ahci_mem_readl(void *ptr, target_phys_addr_t addr)
 
         DPRINTF(-1, "(addr 0x%08X), val 0x%08X\n", (unsigned) addr, val);
     } else if ((addr >= AHCI_PORT_REGS_START_ADDR) &&
-               (addr < AHCI_PORT_REGS_END_ADDR)) {
+               (addr < (AHCI_PORT_REGS_START_ADDR +
+                (s->ports * AHCI_PORT_ADDR_OFFSET_LEN)))) {
         val = ahci_port_read(s, (addr - AHCI_PORT_REGS_START_ADDR) >> 7,
                              addr & AHCI_PORT_ADDR_OFFSET_MASK);
     }
@@ -355,7 +356,8 @@ static void ahci_mem_writel(void *ptr, target_phys_addr_t addr, uint32_t val)
                 DPRINTF(-1, "write to unknown register 0x%x\n", (unsigned)addr);
         }
     } else if ((addr >= AHCI_PORT_REGS_START_ADDR) &&
-               (addr < AHCI_PORT_REGS_END_ADDR)) {
+               (addr < (AHCI_PORT_REGS_START_ADDR +
+                (s->ports * AHCI_PORT_ADDR_OFFSET_LEN)))) {
         ahci_port_write(s, (addr - AHCI_PORT_REGS_START_ADDR) >> 7,
                         addr & AHCI_PORT_ADDR_OFFSET_MASK, val);
     }
@@ -378,16 +380,16 @@ static void ahci_reg_init(AHCIState *s)
 {
     int i;
 
-    s->control_regs.cap = (SATA_PORTS - 1) |
+    s->control_regs.cap = (s->ports - 1) |
                           (AHCI_NUM_COMMAND_SLOTS << 8) |
                           (AHCI_SUPPORTED_SPEED_GEN1 << AHCI_SUPPORTED_SPEED) |
                           HOST_CAP_NCQ | HOST_CAP_AHCI;
 
-    s->control_regs.impl = (1 << SATA_PORTS) - 1;
+    s->control_regs.impl = (1 << s->ports) - 1;
 
     s->control_regs.version = AHCI_VERSION_1_0;
 
-    for (i = 0; i < SATA_PORTS; i++) {
+    for (i = 0; i < s->ports; i++) {
         s->dev[i].port_state = STATE_RUN;
     }
 }
@@ -1096,17 +1098,19 @@ static const IDEDMAOps ahci_dma_ops = {
     .reset = ahci_dma_reset,
 };
 
-void ahci_init(AHCIState *s, DeviceState *qdev)
+void ahci_init(AHCIState *s, DeviceState *qdev, int ports)
 {
     qemu_irq *irqs;
     int i;
 
+    s->ports = ports;
+    s->dev = qemu_mallocz(sizeof(AHCIDevice) * ports);
     ahci_reg_init(s);
     s->mem = cpu_register_io_memory(ahci_readfn, ahci_writefn, s,
                                     DEVICE_LITTLE_ENDIAN);
-    irqs = qemu_allocate_irqs(ahci_irq_set, s, SATA_PORTS);
+    irqs = qemu_allocate_irqs(ahci_irq_set, s, s->ports);
 
-    for (i = 0; i < SATA_PORTS; i++) {
+    for (i = 0; i < s->ports; i++) {
         AHCIDevice *ad = &s->dev[i];
 
         ide_bus_new(&ad->port, qdev, i);
@@ -1120,6 +1124,11 @@ void ahci_init(AHCIState *s, DeviceState *qdev)
     }
 }
 
+void ahci_uninit(AHCIState *s)
+{
+    qemu_free(s->dev);
+}
+
 void ahci_pci_map(PCIDevice *pci_dev, int region_num,
         pcibus_t addr, pcibus_t size, int type)
 {
@@ -1137,7 +1146,7 @@ void ahci_reset(void *opaque)
     d->ahci.control_regs.irqstatus = 0;
     d->ahci.control_regs.ghc = 0;
 
-    for (i = 0; i < SATA_PORTS; i++) {
+    for (i = 0; i < d->ahci.ports; i++) {
         ahci_reset_port(&d->ahci, i);
     }
 }
diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
index b2786d1..a4560c4 100644
--- a/hw/ide/ahci.h
+++ b/hw/ide/ahci.h
@@ -188,11 +188,9 @@
 #define AHCI_GENERIC_HOST_CONTROL_REGS_MAX_ADDR 0x20
                                             /* Shouldn't this be 0x2c? */
 
-#define SATA_PORTS                         4
-
 #define AHCI_PORT_REGS_START_ADDR          0x100
-#define AHCI_PORT_REGS_END_ADDR (AHCI_PORT_REGS_START_ADDR + SATA_PORTS * 0x80)
 #define AHCI_PORT_ADDR_OFFSET_MASK         0x7f
+#define AHCI_PORT_ADDR_OFFSET_LEN          0x80
 
 #define AHCI_NUM_COMMAND_SLOTS             31
 #define AHCI_SUPPORTED_SPEED               20
@@ -289,9 +287,10 @@ struct AHCIDevice {
 };
 
 typedef struct AHCIState {
-    AHCIDevice dev[SATA_PORTS];
+    AHCIDevice *dev;
     AHCIControlRegs control_regs;
     int mem;
+    int ports;
     qemu_irq irq;
 } AHCIState;
 
@@ -323,7 +322,8 @@ typedef struct NCQFrame {
     uint8_t reserved10;
 } __attribute__ ((packed)) NCQFrame;
 
-void ahci_init(AHCIState *s, DeviceState *qdev);
+void ahci_init(AHCIState *s, DeviceState *qdev, int ports);
+void ahci_uninit(AHCIState *s);
 
 void ahci_pci_map(PCIDevice *pci_dev, int region_num,
         pcibus_t addr, pcibus_t size, int type);
diff --git a/hw/ide/ich.c b/hw/ide/ich.c
index 70cb766..f242d7a 100644
--- a/hw/ide/ich.c
+++ b/hw/ide/ich.c
@@ -100,7 +100,7 @@ static int pci_ich9_ahci_init(PCIDevice *dev)
 
     msi_init(dev, 0x50, 1, true, false);
 
-    ahci_init(&d->ahci, &dev->qdev);
+    ahci_init(&d->ahci, &dev->qdev, 6);
     d->ahci.irq = d->card.irq[0];
 
     return 0;
@@ -116,6 +116,7 @@ static int pci_ich9_uninit(PCIDevice *dev)
     }
 
     qemu_unregister_reset(ahci_reset, d);
+    ahci_uninit(&d->ahci);
 
     return 0;
 }
-- 
1.6.0.2

[Qemu-devel] [PATCH 7/7] ahci: work around bug with level interrupts

[Alexander Graf]

When using level based interrupts, the interrupt is treated the same as an
edge triggered one: leaving the line up does not retrigger the interrupt.

In fact, when not lowering the line, we won't ever get a new interrupt inside
the guest. So let's always retrigger an interrupt as soon as the OS ack'ed
something on the device. This way we're sure the guest doesn't starve on
interrupts until someone fixes the actual interrupt path.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ide/ahci.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 98bdf70..bce7fba 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -152,11 +152,10 @@ static void ahci_check_irq(AHCIState *s)
         }
     }
 
+    ahci_irq_lower(s, NULL);
     if (s->control_regs.irqstatus &&
         (s->control_regs.ghc & HOST_CTL_IRQ_EN)) {
             ahci_irq_raise(s, NULL);
-    } else {
-        ahci_irq_lower(s, NULL);
     }
 }
 
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH 7/7] ahci: work around bug with level interrupts

[Aurelien Jarno]

On Tue, Feb 01, 2011 at 03:51:32PM +0100, Alexander Graf wrote:
> When using level based interrupts, the interrupt is treated the same as an
> edge triggered one: leaving the line up does not retrigger the interrupt.
> 
> In fact, when not lowering the line, we won't ever get a new interrupt inside
> the guest. So let's always retrigger an interrupt as soon as the OS ack'ed
> something on the device. This way we're sure the guest doesn't starve on
> interrupts until someone fixes the actual interrupt path.

Given this issue mostly concerns x86 and not other architectures where
the SATA emulation can probably be used, what about putting the two
versions of the codes like in i8259.c:

| * all targets should do this rather than acking the IRQ in the cpu */
| #if defined(TARGET_MIPS) || defined(TARGET_PPC) || defined(TARGET_ALPHA)

The list of architectures here is reduced given the few architectures 
that actually use the i8259, so for ahci.c it should probably be #if not
defined(TARGET_I386) instead.

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH 7/7] ahci: work around bug with level interrupts

[Alexander Graf]

On 01.02.2011, at 17:34, Aurelien Jarno wrote:

> On Tue, Feb 01, 2011 at 03:51:32PM +0100, Alexander Graf wrote:
>> When using level based interrupts, the interrupt is treated the same as an
>> edge triggered one: leaving the line up does not retrigger the interrupt.
>> 
>> In fact, when not lowering the line, we won't ever get a new interrupt inside
>> the guest. So let's always retrigger an interrupt as soon as the OS ack'ed
>> something on the device. This way we're sure the guest doesn't starve on
>> interrupts until someone fixes the actual interrupt path.
> 
> Given this issue mostly concerns x86 and not other architectures where
> the SATA emulation can probably be used, what about putting the two
> versions of the codes like in i8259.c:
> 
> | * all targets should do this rather than acking the IRQ in the cpu */
> | #if defined(TARGET_MIPS) || defined(TARGET_PPC) || defined(TARGET_ALPHA)
> 
> The list of architectures here is reduced given the few architectures 
> that actually use the i8259, so for ahci.c it should probably be #if not
> defined(TARGET_I386) instead.

Because then we'd have to build the ahci code conditionally on the architecture. Right now it builds into libhw :)


Alex

Re: [Qemu-devel] [PATCH 7/7] ahci: work around bug with level interrupts

[Aurelien Jarno]

On Tue, Feb 01, 2011 at 05:53:43PM +0100, Alexander Graf wrote:
> 
> On 01.02.2011, at 17:34, Aurelien Jarno wrote:
> 
> > On Tue, Feb 01, 2011 at 03:51:32PM +0100, Alexander Graf wrote:
> >> When using level based interrupts, the interrupt is treated the same as an
> >> edge triggered one: leaving the line up does not retrigger the interrupt.
> >> 
> >> In fact, when not lowering the line, we won't ever get a new interrupt inside
> >> the guest. So let's always retrigger an interrupt as soon as the OS ack'ed
> >> something on the device. This way we're sure the guest doesn't starve on
> >> interrupts until someone fixes the actual interrupt path.
> > 
> > Given this issue mostly concerns x86 and not other architectures where
> > the SATA emulation can probably be used, what about putting the two
> > versions of the codes like in i8259.c:
> > 
> > | * all targets should do this rather than acking the IRQ in the cpu */
> > | #if defined(TARGET_MIPS) || defined(TARGET_PPC) || defined(TARGET_ALPHA)
> > 
> > The list of architectures here is reduced given the few architectures 
> > that actually use the i8259, so for ahci.c it should probably be #if not
> > defined(TARGET_I386) instead.
> 
> Because then we'd have to build the ahci code conditionally on the architecture. Right now it builds into libhw :)

If we want to keep it in libhw, it's probably better to disable it on
other architectures than i386.

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH 7/7] ahci: work around bug with level interrupts

[Alexander Graf]

On 01.02.2011, at 18:06, Aurelien Jarno wrote:

> On Tue, Feb 01, 2011 at 05:53:43PM +0100, Alexander Graf wrote:
>> 
>> On 01.02.2011, at 17:34, Aurelien Jarno wrote:
>> 
>>> On Tue, Feb 01, 2011 at 03:51:32PM +0100, Alexander Graf wrote:
>>>> When using level based interrupts, the interrupt is treated the same as an
>>>> edge triggered one: leaving the line up does not retrigger the interrupt.
>>>> 
>>>> In fact, when not lowering the line, we won't ever get a new interrupt inside
>>>> the guest. So let's always retrigger an interrupt as soon as the OS ack'ed
>>>> something on the device. This way we're sure the guest doesn't starve on
>>>> interrupts until someone fixes the actual interrupt path.
>>> 
>>> Given this issue mostly concerns x86 and not other architectures where
>>> the SATA emulation can probably be used, what about putting the two
>>> versions of the codes like in i8259.c:
>>> 
>>> | * all targets should do this rather than acking the IRQ in the cpu */
>>> | #if defined(TARGET_MIPS) || defined(TARGET_PPC) || defined(TARGET_ALPHA)
>>> 
>>> The list of architectures here is reduced given the few architectures 
>>> that actually use the i8259, so for ahci.c it should probably be #if not
>>> defined(TARGET_I386) instead.
>> 
>> Because then we'd have to build the ahci code conditionally on the architecture. Right now it builds into libhw :)
> 
> If we want to keep it in libhw, it's probably better to disable it on
> other architectures than i386.

How so? The workaround simply triggers a superfluous interrupt event, but shouldn't break anything for other architectures. In fact, last time I checked it worked just fine on ppc.


Alex

Re: [Qemu-devel] [PATCH 7/7] ahci: work around bug with level interrupts

[Aurelien Jarno]

On Tue, Feb 01, 2011 at 06:10:56PM +0100, Alexander Graf wrote:
> 
> On 01.02.2011, at 18:06, Aurelien Jarno wrote:
> 
> > On Tue, Feb 01, 2011 at 05:53:43PM +0100, Alexander Graf wrote:
> >> 
> >> On 01.02.2011, at 17:34, Aurelien Jarno wrote:
> >> 
> >>> On Tue, Feb 01, 2011 at 03:51:32PM +0100, Alexander Graf wrote:
> >>>> When using level based interrupts, the interrupt is treated the same as an
> >>>> edge triggered one: leaving the line up does not retrigger the interrupt.
> >>>> 
> >>>> In fact, when not lowering the line, we won't ever get a new interrupt inside
> >>>> the guest. So let's always retrigger an interrupt as soon as the OS ack'ed
> >>>> something on the device. This way we're sure the guest doesn't starve on
> >>>> interrupts until someone fixes the actual interrupt path.
> >>> 
> >>> Given this issue mostly concerns x86 and not other architectures where
> >>> the SATA emulation can probably be used, what about putting the two
> >>> versions of the codes like in i8259.c:
> >>> 
> >>> | * all targets should do this rather than acking the IRQ in the cpu */
> >>> | #if defined(TARGET_MIPS) || defined(TARGET_PPC) || defined(TARGET_ALPHA)
> >>> 
> >>> The list of architectures here is reduced given the few architectures 
> >>> that actually use the i8259, so for ahci.c it should probably be #if not
> >>> defined(TARGET_I386) instead.
> >> 
> >> Because then we'd have to build the ahci code conditionally on the architecture. Right now it builds into libhw :)
> > 
> > If we want to keep it in libhw, it's probably better to disable it on
> > other architectures than i386.
> 
> How so? The workaround simply triggers a superfluous interrupt event, but shouldn't break anything for other architectures. In fact, last time I checked it worked just fine on ppc.
> 

Superfluous interrupt events can have a lot of consequences, it's often
a kernel panic (see recent zilog serial port issues for example), and
when it comes to disk controllers, it might mean data loss.

Anyway the way to go there is cleary to fix the x86 interrupt model,
it's the only one broken among all targets. If we can't find anyone to
do that, we can also declare x86 unsupported :)

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

[Qemu-devel] [PATCH 7/7] ahci: work around bug with level interrupts

[Alexander Graf]

When using level based interrupts, the interrupt is treated the same as an
edge triggered one: leaving the line up does not retrigger the interrupt.

In fact, when not lowering the line, we won't ever get a new interrupt inside
the guest. So let's always retrigger an interrupt as soon as the OS ack'ed
something on the device. This way we're sure the guest doesn't starve on
interrupts until someone fixes the actual interrupt path.

Signed-off-by: Alexander Graf <agraf@suse.de>

---

v2 -> v3:

  - add comment about the interrupt hack
---
 hw/ide/ahci.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 98bdf70..95e1cf7 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -152,11 +152,15 @@ static void ahci_check_irq(AHCIState *s)
         }
     }
 
+    /* XXX We lower the interrupt line here because of a bug with interrupt
+           handling in Qemu. When leaving a line up, the interrupt does
+           not get retriggered automatically currently. Once that bug is fixed,
+           this workaround is not necessary anymore and we only need to lower
+           in the else branch. */
+    ahci_irq_lower(s, NULL);
     if (s->control_regs.irqstatus &&
         (s->control_regs.ghc & HOST_CTL_IRQ_EN)) {
             ahci_irq_raise(s, NULL);
-    } else {
-        ahci_irq_lower(s, NULL);
     }
 }
 
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH 7/7] ahci: work around bug with level interrupts

[Aurelien Jarno]

On Tue, Feb 01, 2011 at 07:35:01PM +0100, Alexander Graf wrote:
> When using level based interrupts, the interrupt is treated the same as an
> edge triggered one: leaving the line up does not retrigger the interrupt.
> 
> In fact, when not lowering the line, we won't ever get a new interrupt inside
> the guest. So let's always retrigger an interrupt as soon as the OS ack'ed
> something on the device. This way we're sure the guest doesn't starve on
> interrupts until someone fixes the actual interrupt path.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> 
> ---
> 
> v2 -> v3:
> 
>   - add comment about the interrupt hack
> ---
>  hw/ide/ahci.c |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 98bdf70..95e1cf7 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -152,11 +152,15 @@ static void ahci_check_irq(AHCIState *s)
>          }
>      }
>  
> +    /* XXX We lower the interrupt line here because of a bug with interrupt
> +           handling in Qemu. When leaving a line up, the interrupt does
> +           not get retriggered automatically currently. Once that bug is fixed,
> +           this workaround is not necessary anymore and we only need to lower
> +           in the else branch. */
> +    ahci_irq_lower(s, NULL);
>      if (s->control_regs.irqstatus &&
>          (s->control_regs.ghc & HOST_CTL_IRQ_EN)) {
>              ahci_irq_raise(s, NULL);
> -    } else {
> -        ahci_irq_lower(s, NULL);
>      }
>  }
>  

It's a lot better that way, however I think we should still keep the
correct code. Also given this problem only concerns the i386 target (ppc
code is actually a bit strange, but at the end does the correct thing),
it's something we should probably mention.

What about something like that?

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 671b4df..0b153f0 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -489,12 +489,25 @@ static void ahci_check_irq(AHCIState *s)
         }
     }
 
+#if 0
     if (s->control_regs.irqstatus &&
         (s->control_regs.ghc & HOST_CTL_IRQ_EN)) {
             ahci_irq_raise(s, NULL);
     } else {
         ahci_irq_lower(s, NULL);
     }
+#else
+    /* XXX We lower the interrupt line here because of a bug with interrupt
+           handling in Qemu on the i386 target. When leaving a line up, the
+           interrupt does not get retriggered automatically currently. Once
+           that bug is fixed, this workaround is not necessary anymore and
+           we only need to lower in the else branch. */
+    ahci_irq_lower(s, NULL);
+    if (s->control_regs.irqstatus &&
+        (s->control_regs.ghc & HOST_CTL_IRQ_EN)) {
+            ahci_irq_raise(s, NULL);
+    }
+#endif
 }
 
 static void ahci_trigger_irq(AHCIState *s, AHCIDevice *d,


-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH 7/7] ahci: work around bug with level interrupts

[Alexander Graf]

On 01.02.2011, at 20:58, Aurelien Jarno wrote:

> On Tue, Feb 01, 2011 at 07:35:01PM +0100, Alexander Graf wrote:
>> When using level based interrupts, the interrupt is treated the same as an
>> edge triggered one: leaving the line up does not retrigger the interrupt.
>> 
>> In fact, when not lowering the line, we won't ever get a new interrupt inside
>> the guest. So let's always retrigger an interrupt as soon as the OS ack'ed
>> something on the device. This way we're sure the guest doesn't starve on
>> interrupts until someone fixes the actual interrupt path.
>> 
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> 
>> ---
>> 
>> v2 -> v3:
>> 
>>  - add comment about the interrupt hack
>> ---
>> hw/ide/ahci.c |    8 ++++++--
>> 1 files changed, 6 insertions(+), 2 deletions(-)
>> 
>> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
>> index 98bdf70..95e1cf7 100644
>> --- a/hw/ide/ahci.c
>> +++ b/hw/ide/ahci.c
>> @@ -152,11 +152,15 @@ static void ahci_check_irq(AHCIState *s)
>>         }
>>     }
>> 
>> +    /* XXX We lower the interrupt line here because of a bug with interrupt
>> +           handling in Qemu. When leaving a line up, the interrupt does
>> +           not get retriggered automatically currently. Once that bug is fixed,
>> +           this workaround is not necessary anymore and we only need to lower
>> +           in the else branch. */
>> +    ahci_irq_lower(s, NULL);
>>     if (s->control_regs.irqstatus &&
>>         (s->control_regs.ghc & HOST_CTL_IRQ_EN)) {
>>             ahci_irq_raise(s, NULL);
>> -    } else {
>> -        ahci_irq_lower(s, NULL);
>>     }
>> }
>> 
> 
> It's a lot better that way, however I think we should still keep the
> correct code. Also given this problem only concerns the i386 target (ppc
> code is actually a bit strange, but at the end does the correct thing),
> it's something we should probably mention.
> 
> What about something like that?

While I dislike #if 0s in released code in general, it's fine with me. I know what I meant based on the comment, but for others this might make it more explicit. How would we go about committing this? Kevin, will you just change the code inside your tree?


Alex

Re: [Qemu-devel] [PATCH 7/7] ahci: work around bug with level interrupts

[Kevin Wolf]

Am 01.02.2011 21:10, schrieb Alexander Graf:
> 
> On 01.02.2011, at 20:58, Aurelien Jarno wrote:
> 
>> On Tue, Feb 01, 2011 at 07:35:01PM +0100, Alexander Graf wrote:
>>> When using level based interrupts, the interrupt is treated the same as an
>>> edge triggered one: leaving the line up does not retrigger the interrupt.
>>>
>>> In fact, when not lowering the line, we won't ever get a new interrupt inside
>>> the guest. So let's always retrigger an interrupt as soon as the OS ack'ed
>>> something on the device. This way we're sure the guest doesn't starve on
>>> interrupts until someone fixes the actual interrupt path.
>>>
>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>
>>> ---
>>>
>>> v2 -> v3:
>>>
>>>  - add comment about the interrupt hack
>>> ---
>>> hw/ide/ahci.c |    8 ++++++--
>>> 1 files changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
>>> index 98bdf70..95e1cf7 100644
>>> --- a/hw/ide/ahci.c
>>> +++ b/hw/ide/ahci.c
>>> @@ -152,11 +152,15 @@ static void ahci_check_irq(AHCIState *s)
>>>         }
>>>     }
>>>
>>> +    /* XXX We lower the interrupt line here because of a bug with interrupt
>>> +           handling in Qemu. When leaving a line up, the interrupt does
>>> +           not get retriggered automatically currently. Once that bug is fixed,
>>> +           this workaround is not necessary anymore and we only need to lower
>>> +           in the else branch. */
>>> +    ahci_irq_lower(s, NULL);
>>>     if (s->control_regs.irqstatus &&
>>>         (s->control_regs.ghc & HOST_CTL_IRQ_EN)) {
>>>             ahci_irq_raise(s, NULL);
>>> -    } else {
>>> -        ahci_irq_lower(s, NULL);
>>>     }
>>> }
>>>
>>
>> It's a lot better that way, however I think we should still keep the
>> correct code. Also given this problem only concerns the i386 target (ppc
>> code is actually a bit strange, but at the end does the correct thing),
>> it's something we should probably mention.
>>
>> What about something like that?
> 
> While I dislike #if 0s in released code in general, it's fine with me. I know what I meant based on the comment, but for others this might make it more explicit. How would we go about committing this? Kevin, will you just change the code inside your tree?

I would prefer if you sent a new version of this patch.

Kevin

[Qemu-devel] [PATCH 7/7] ahci: work around bug with level interrupts

[Alexander Graf]

When using level based interrupts, the interrupt is treated the same as an
edge triggered one: leaving the line up does not retrigger the interrupt.

In fact, when not lowering the line, we won't ever get a new interrupt inside
the guest. So let's always retrigger an interrupt as soon as the OS ack'ed
something on the device. This way we're sure the guest doesn't starve on
interrupts until someone fixes the actual interrupt path.

Signed-off-by: Alexander Graf <agraf@suse.de>

---

v2 -> v3:

  - add comment about the interrupt hack

v3 -> v4:

  - embed non-workaround version in the code
---
 hw/ide/ahci.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 98bdf70..10377ca 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -152,12 +152,25 @@ static void ahci_check_irq(AHCIState *s)
         }
     }
 
+    /* XXX We always lower the interrupt line here because of a bug with
+           interrupt handling in Qemu. When leaving a line up, the interrupt
+           does not get retriggered automatically currently. Once that bug is
+           fixed, this workaround is not necessary anymore and we only need to
+           lower in the else branch. */
+#if 0
     if (s->control_regs.irqstatus &&
         (s->control_regs.ghc & HOST_CTL_IRQ_EN)) {
             ahci_irq_raise(s, NULL);
     } else {
         ahci_irq_lower(s, NULL);
     }
+#else
+    ahci_irq_lower(s, NULL);
+    if (s->control_regs.irqstatus &&
+        (s->control_regs.ghc & HOST_CTL_IRQ_EN)) {
+            ahci_irq_raise(s, NULL);
+    }
+#endif
 }
 
 static void ahci_trigger_irq(AHCIState *s, AHCIDevice *d,
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH 7/7] ahci: work around bug with level interrupts

[Alexander Graf]

Kevin Wolf wrote:
> Am 01.02.2011 21:10, schrieb Alexander Graf:
>   
>> On 01.02.2011, at 20:58, Aurelien Jarno wrote:
>>
>>     
>>> On Tue, Feb 01, 2011 at 07:35:01PM +0100, Alexander Graf wrote:
>>>       
>>>> When using level based interrupts, the interrupt is treated the same as an
>>>> edge triggered one: leaving the line up does not retrigger the interrupt.
>>>>
>>>> In fact, when not lowering the line, we won't ever get a new interrupt inside
>>>> the guest. So let's always retrigger an interrupt as soon as the OS ack'ed
>>>> something on the device. This way we're sure the guest doesn't starve on
>>>> interrupts until someone fixes the actual interrupt path.
>>>>
>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>>
>>>> ---
>>>>
>>>> v2 -> v3:
>>>>
>>>>  - add comment about the interrupt hack
>>>> ---
>>>> hw/ide/ahci.c |    8 ++++++--
>>>> 1 files changed, 6 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
>>>> index 98bdf70..95e1cf7 100644
>>>> --- a/hw/ide/ahci.c
>>>> +++ b/hw/ide/ahci.c
>>>> @@ -152,11 +152,15 @@ static void ahci_check_irq(AHCIState *s)
>>>>         }
>>>>     }
>>>>
>>>> +    /* XXX We lower the interrupt line here because of a bug with interrupt
>>>> +           handling in Qemu. When leaving a line up, the interrupt does
>>>> +           not get retriggered automatically currently. Once that bug is fixed,
>>>> +           this workaround is not necessary anymore and we only need to lower
>>>> +           in the else branch. */
>>>> +    ahci_irq_lower(s, NULL);
>>>>     if (s->control_regs.irqstatus &&
>>>>         (s->control_regs.ghc & HOST_CTL_IRQ_EN)) {
>>>>             ahci_irq_raise(s, NULL);
>>>> -    } else {
>>>> -        ahci_irq_lower(s, NULL);
>>>>     }
>>>> }
>>>>
>>>>         
>>> It's a lot better that way, however I think we should still keep the
>>> correct code. Also given this problem only concerns the i386 target (ppc
>>> code is actually a bit strange, but at the end does the correct thing),
>>> it's something we should probably mention.
>>>
>>> What about something like that?
>>>       
>> While I dislike #if 0s in released code in general, it's fine with me. I know what I meant based on the comment, but for others this might make it more explicit. How would we go about committing this? Kevin, will you just change the code inside your tree?
>>     
>
> I would prefer if you sent a new version of this patch.
>
>   

Ok, done :)


Alex

[Qemu-devel] Re: [PATCH 7/7] ahci: work around bug with level interrupts

[Jan Kiszka]

On 2011-02-02 15:39, Alexander Graf wrote:
> When using level based interrupts, the interrupt is treated the same as an
> edge triggered one: leaving the line up does not retrigger the interrupt.
> 
> In fact, when not lowering the line, we won't ever get a new interrupt inside
> the guest. So let's always retrigger an interrupt as soon as the OS ack'ed
> something on the device. This way we're sure the guest doesn't starve on
> interrupts until someone fixes the actual interrupt path.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> 
> ---
> 
> v2 -> v3:
> 
>   - add comment about the interrupt hack
> 
> v3 -> v4:
> 
>   - embed non-workaround version in the code
> ---
>  hw/ide/ahci.c |   13 +++++++++++++
>  1 files changed, 13 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 98bdf70..10377ca 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -152,12 +152,25 @@ static void ahci_check_irq(AHCIState *s)
>          }
>      }
>  
> +    /* XXX We always lower the interrupt line here because of a bug with
> +           interrupt handling in Qemu. When leaving a line up, the interrupt
> +           does not get retriggered automatically currently. Once that bug is
> +           fixed, this workaround is not necessary anymore and we only need to
> +           lower in the else branch. */
> +#if 0
>      if (s->control_regs.irqstatus &&
>          (s->control_regs.ghc & HOST_CTL_IRQ_EN)) {
>              ahci_irq_raise(s, NULL);
>      } else {
>          ahci_irq_lower(s, NULL);
>      }
> +#else
> +    ahci_irq_lower(s, NULL);
> +    if (s->control_regs.irqstatus &&
> +        (s->control_regs.ghc & HOST_CTL_IRQ_EN)) {
> +            ahci_irq_raise(s, NULL);
> +    }
> +#endif
>  }
>  
>  static void ahci_trigger_irq(AHCIState *s, AHCIDevice *d,

Could you check if this quick-hack obsoletes the above hack?

I was hoping it has some influence on our 64-bit Windows issue, but it
hasn't, or it's still buggy, or both. However, its intention is to
reassert still pending level-triggered IRQs on APIC EOI. This logic is
missing in the user space model but exists in KVM's kernel model (I was
asking for a test against the latter but unfortunately did not receive
an answer - I bet you already don't need your patch over qemu-kvm).

Thanks,
Jan

---
 hw/apic.c   |    9 ++++++---
 hw/ioapic.c |   43 +++++++++++++++++++++++++++++++++++++++++--
 hw/ioapic.h |   20 ++++++++++++++++++++
 3 files changed, 67 insertions(+), 5 deletions(-)
 create mode 100644 hw/ioapic.h

diff --git a/hw/apic.c b/hw/apic.c
index ff581f0..05a115f 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -18,6 +18,7 @@
  */
 #include "hw.h"
 #include "apic.h"
+#include "ioapic.h"
 #include "qemu-timer.h"
 #include "host-utils.h"
 #include "sysbus.h"
@@ -57,7 +58,8 @@
 
 #define ESR_ILLEGAL_ADDRESS (1 << 7)
 
-#define APIC_SV_ENABLE (1 << 8)
+#define APIC_SV_DIRECTED_IO             (1<<12)
+#define APIC_SV_ENABLE                  (1<<8)
 
 #define MAX_APICS 255
 #define MAX_APIC_WORDS 8
@@ -420,8 +422,9 @@ static void apic_eoi(APICState *s)
     if (isrv < 0)
         return;
     reset_bit(s->isr, isrv);
-    /* XXX: send the EOI packet to the APIC bus to allow the I/O APIC to
-            set the remote IRR bit for level triggered interrupts. */
+    if (!(s->spurious_vec & APIC_SV_DIRECTED_IO) && get_bit(s->tmr, isrv)) {
+        ioapic_eio_broadcast(isrv);
+    }
     apic_update_irq(s);
 }
 
diff --git a/hw/ioapic.c b/hw/ioapic.c
index 2109568..1d23474 100644
--- a/hw/ioapic.c
+++ b/hw/ioapic.c
@@ -23,6 +23,7 @@
 #include "hw.h"
 #include "pc.h"
 #include "apic.h"
+#include "ioapic.h"
 #include "qemu-timer.h"
 #include "host-utils.h"
 #include "sysbus.h"
@@ -36,7 +37,10 @@
 #define DPRINTF(fmt, ...)
 #endif
 
-#define IOAPIC_LVT_MASKED 		(1<<16)
+#define MAX_IOAPICS                     1
+
+#define IOAPIC_LVT_MASKED               (1ULL << 16)
+#define IOAPIC_LVT_REMOTE_IRR           (1ULL << 14)
 
 #define IOAPIC_TRIGGER_EDGE		0
 #define IOAPIC_TRIGGER_LEVEL		1
@@ -61,6 +65,8 @@ struct IOAPICState {
     uint64_t ioredtbl[IOAPIC_NUM_PINS];
 };
 
+static IOAPICState *ioapics[MAX_IOAPICS];
+
 static void ioapic_service(IOAPICState *s)
 {
     uint8_t i;
@@ -83,8 +89,11 @@ static void ioapic_service(IOAPICState *s)
                 dest_mode = (entry >> 11) & 1;
                 delivery_mode = (entry >> 8) & 7;
                 polarity = (entry >> 13) & 1;
-                if (trig_mode == IOAPIC_TRIGGER_EDGE)
+                if (trig_mode == IOAPIC_TRIGGER_EDGE) {
                     s->irr &= ~mask;
+                } else {
+                    s->ioredtbl[i] |= IOAPIC_LVT_REMOTE_IRR;
+                }
                 if (delivery_mode == IOAPIC_DM_EXTINT)
                     vector = pic_read_irq(isa_pic);
                 else
@@ -131,6 +140,29 @@ static void ioapic_set_irq(void *opaque, int vector, int level)
     }
 }
 
+void ioapic_eio_broadcast(int vector)
+{
+    IOAPICState *s;
+    uint64_t entry;
+    int i, n;
+
+    for (i = 0; i < MAX_IOAPICS; i++) {
+        s = ioapics[i];
+        if (!s) {
+            continue;
+        }
+        for (n = 0; n < IOAPIC_NUM_PINS; n++) {
+            entry = s->ioredtbl[n];
+            if ((entry & IOAPIC_LVT_REMOTE_IRR) && (entry & 0xff) == vector) {
+                s->ioredtbl[n] = entry & ~IOAPIC_LVT_REMOTE_IRR;
+                if (!(entry & IOAPIC_LVT_MASKED) && (s->irr & (1 << n))) {
+                    ioapic_service(s);
+                }
+            }
+        }
+    }
+}
+
 static uint32_t ioapic_mem_readl(void *opaque, target_phys_addr_t addr)
 {
     IOAPICState *s = opaque;
@@ -240,6 +272,11 @@ static int ioapic_init1(SysBusDevice *dev)
 {
     IOAPICState *s = FROM_SYSBUS(IOAPICState, dev);
     int io_memory;
+    static int ioapic_no;
+
+    if (ioapic_no >= MAX_IOAPICS) {
+        return -1;
+    }
 
     io_memory = cpu_register_io_memory(ioapic_mem_read,
                                        ioapic_mem_write, s,
@@ -248,6 +285,8 @@ static int ioapic_init1(SysBusDevice *dev)
 
     qdev_init_gpio_in(&dev->qdev, ioapic_set_irq, IOAPIC_NUM_PINS);
 
+    ioapics[ioapic_no++] = s;
+
     return 0;
 }
 
diff --git a/hw/ioapic.h b/hw/ioapic.h
new file mode 100644
index 0000000..c441567
--- /dev/null
+++ b/hw/ioapic.h
@@ -0,0 +1,20 @@
+/*
+ *  ioapic.c IOAPIC emulation logic
+ *
+ *  Copyright (c) 2011 Jan Kiszka, Siemens AG
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+void ioapic_eio_broadcast(int vector);
-- 
1.7.1

[Qemu-devel] Re: [PATCH 7/7] ahci: work around bug with level interrupts

[Alexander Graf]

On 03.02.2011, at 11:30, Jan Kiszka wrote:

> On 2011-02-02 15:39, Alexander Graf wrote:
>> When using level based interrupts, the interrupt is treated the same as an
>> edge triggered one: leaving the line up does not retrigger the interrupt.
>> 
>> In fact, when not lowering the line, we won't ever get a new interrupt inside
>> the guest. So let's always retrigger an interrupt as soon as the OS ack'ed
>> something on the device. This way we're sure the guest doesn't starve on
>> interrupts until someone fixes the actual interrupt path.
>> 
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> 
>> ---
>> 
>> v2 -> v3:
>> 
>>  - add comment about the interrupt hack
>> 
>> v3 -> v4:
>> 
>>  - embed non-workaround version in the code
>> ---
>> hw/ide/ahci.c |   13 +++++++++++++
>> 1 files changed, 13 insertions(+), 0 deletions(-)
>> 
>> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
>> index 98bdf70..10377ca 100644
>> --- a/hw/ide/ahci.c
>> +++ b/hw/ide/ahci.c
>> @@ -152,12 +152,25 @@ static void ahci_check_irq(AHCIState *s)
>>         }
>>     }
>> 
>> +    /* XXX We always lower the interrupt line here because of a bug with
>> +           interrupt handling in Qemu. When leaving a line up, the interrupt
>> +           does not get retriggered automatically currently. Once that bug is
>> +           fixed, this workaround is not necessary anymore and we only need to
>> +           lower in the else branch. */
>> +#if 0
>>     if (s->control_regs.irqstatus &&
>>         (s->control_regs.ghc & HOST_CTL_IRQ_EN)) {
>>             ahci_irq_raise(s, NULL);
>>     } else {
>>         ahci_irq_lower(s, NULL);
>>     }
>> +#else
>> +    ahci_irq_lower(s, NULL);
>> +    if (s->control_regs.irqstatus &&
>> +        (s->control_regs.ghc & HOST_CTL_IRQ_EN)) {
>> +            ahci_irq_raise(s, NULL);
>> +    }
>> +#endif
>> }
>> 
>> static void ahci_trigger_irq(AHCIState *s, AHCIDevice *d,
> 
> Could you check if this quick-hack obsoletes the above hack?
> 
> I was hoping it has some influence on our 64-bit Windows issue, but it
> hasn't, or it's still buggy, or both. However, its intention is to
> reassert still pending level-triggered IRQs on APIC EOI. This logic is
> missing in the user space model but exists in KVM's kernel model (I was
> asking for a test against the latter but unfortunately did not receive
> an answer - I bet you already don't need your patch over qemu-kvm).

Oh, sorry for not replying there. I work on qemu.git, so the in-kernel apic is out of question for testing. I tried merging qemu-kvm.git and qemu.git several times and every time just wasted a few hours of my life, so I gave up on testing things on qemu-kvm.git.

The patch works though. If everybody agrees to take it, we can drop patch 7/7 of my ahci set.


Alex

[Qemu-devel] Re: [PATCH 7/7] ahci: work around bug with level interrupts

[Jan Kiszka]

On 2011-02-03 11:38, Alexander Graf wrote:
> 
> On 03.02.2011, at 11:30, Jan Kiszka wrote:
> 
>> On 2011-02-02 15:39, Alexander Graf wrote:
>>> When using level based interrupts, the interrupt is treated the same as an
>>> edge triggered one: leaving the line up does not retrigger the interrupt.
>>>
>>> In fact, when not lowering the line, we won't ever get a new interrupt inside
>>> the guest. So let's always retrigger an interrupt as soon as the OS ack'ed
>>> something on the device. This way we're sure the guest doesn't starve on
>>> interrupts until someone fixes the actual interrupt path.
>>>
>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>
>>> ---
>>>
>>> v2 -> v3:
>>>
>>>  - add comment about the interrupt hack
>>>
>>> v3 -> v4:
>>>
>>>  - embed non-workaround version in the code
>>> ---
>>> hw/ide/ahci.c |   13 +++++++++++++
>>> 1 files changed, 13 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
>>> index 98bdf70..10377ca 100644
>>> --- a/hw/ide/ahci.c
>>> +++ b/hw/ide/ahci.c
>>> @@ -152,12 +152,25 @@ static void ahci_check_irq(AHCIState *s)
>>>         }
>>>     }
>>>
>>> +    /* XXX We always lower the interrupt line here because of a bug with
>>> +           interrupt handling in Qemu. When leaving a line up, the interrupt
>>> +           does not get retriggered automatically currently. Once that bug is
>>> +           fixed, this workaround is not necessary anymore and we only need to
>>> +           lower in the else branch. */
>>> +#if 0
>>>     if (s->control_regs.irqstatus &&
>>>         (s->control_regs.ghc & HOST_CTL_IRQ_EN)) {
>>>             ahci_irq_raise(s, NULL);
>>>     } else {
>>>         ahci_irq_lower(s, NULL);
>>>     }
>>> +#else
>>> +    ahci_irq_lower(s, NULL);
>>> +    if (s->control_regs.irqstatus &&
>>> +        (s->control_regs.ghc & HOST_CTL_IRQ_EN)) {
>>> +            ahci_irq_raise(s, NULL);
>>> +    }
>>> +#endif
>>> }
>>>
>>> static void ahci_trigger_irq(AHCIState *s, AHCIDevice *d,
>>
>> Could you check if this quick-hack obsoletes the above hack?
>>
>> I was hoping it has some influence on our 64-bit Windows issue, but it
>> hasn't, or it's still buggy, or both. However, its intention is to
>> reassert still pending level-triggered IRQs on APIC EOI. This logic is
>> missing in the user space model but exists in KVM's kernel model (I was
>> asking for a test against the latter but unfortunately did not receive
>> an answer - I bet you already don't need your patch over qemu-kvm).
> 
> Oh, sorry for not replying there. I work on qemu.git, so the in-kernel apic is out of question for testing. I tried merging qemu-kvm.git and qemu.git several times and every time just wasted a few hours of my life, so I gave up on testing things on qemu-kvm.git.

Ah, I see. Marcelo recently merged back, resolving the tricky bits, and
now it should be much easier. Anyway.

> 
> The patch works though. If everybody agrees to take it, we can drop patch 7/7 of my ahci set.

Cool! I've a few more minor things to clean up here and will sent that
as a series later today, also for 0.14.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

[Qemu-devel] Re: [PATCH 0/7] Some more AHCI work v2

[Kevin Wolf]

Am 01.02.2011 15:51, schrieb Alexander Graf:
> Clearly, AHCI as is is not perfect yet (intentionally, release early,
> release often, remember?). This patch set makes it work with SeaBIOS
> so booting Windows 7 works flawlessly for me. it also adds some speedups
> and fixes a level based interrupts, rendering ahci useful on PPC targets.
> 
> In preparation of potential non-ich9 implementations, this set also
> splits ahci code from concrete ich9 specific code. That way we can
> later on create other AHCI adapters while reusing a lot of code.
> 
> Git tree (including BIOS patch to enable booting from AHCI):
> 
>  git://repo.or.cz/qemu/ahci.git ahci

Thanks, applied all to the block branch.

Kevin

[Qemu-devel] Re: [PATCH 0/7] Some more AHCI work v2

[Jan Kiszka]

On 2011-02-07 11:56, Kevin Wolf wrote:
> Am 01.02.2011 15:51, schrieb Alexander Graf:
>> Clearly, AHCI as is is not perfect yet (intentionally, release early,
>> release often, remember?). This patch set makes it work with SeaBIOS
>> so booting Windows 7 works flawlessly for me. it also adds some speedups
>> and fixes a level based interrupts, rendering ahci useful on PPC targets.
>>
>> In preparation of potential non-ich9 implementations, this set also
>> splits ahci code from concrete ich9 specific code. That way we can
>> later on create other AHCI adapters while reusing a lot of code.
>>
>> Git tree (including BIOS patch to enable booting from AHCI):
>>
>>  git://repo.or.cz/qemu/ahci.git ahci
> 
> Thanks, applied all to the block branch.

Please drop patch 7, 0280b571c1 obsoleted that workaround.

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

[Qemu-devel] Re: [PATCH 0/7] Some more AHCI work v2

[Kevin Wolf]

Am 07.02.2011 12:44, schrieb Jan Kiszka:
> On 2011-02-07 11:56, Kevin Wolf wrote:
>> Am 01.02.2011 15:51, schrieb Alexander Graf:
>>> Clearly, AHCI as is is not perfect yet (intentionally, release early,
>>> release often, remember?). This patch set makes it work with SeaBIOS
>>> so booting Windows 7 works flawlessly for me. it also adds some speedups
>>> and fixes a level based interrupts, rendering ahci useful on PPC targets.
>>>
>>> In preparation of potential non-ich9 implementations, this set also
>>> splits ahci code from concrete ich9 specific code. That way we can
>>> later on create other AHCI adapters while reusing a lot of code.
>>>
>>> Git tree (including BIOS patch to enable booting from AHCI):
>>>
>>>  git://repo.or.cz/qemu/ahci.git ahci
>>
>> Thanks, applied all to the block branch.
> 
> Please drop patch 7, 0280b571c1 obsoleted that workaround.

Oh, it's already in? Thanks for catching this, I'll drop patch 7.

Kevin