* [Qemu-devel] [PATCH 00/10] AHCI emulation support v2
@ 2010-11-18 3:27 Alexander Graf
2010-11-18 3:27 ` [Qemu-devel] [PATCH 01/10] ide: split ide command interpretation off Alexander Graf
` (11 more replies)
0 siblings, 12 replies; 30+ messages in thread
From: Alexander Graf @ 2010-11-18 3:27 UTC (permalink / raw)
To: QEMU-devel Developers
Cc: Joerg Roedel, Gerd Hoffmann, Stefan Hajnoczi, tj, Roland Elek,
Sebastian Herbszt
This patch adds support for AHCI emulation. I have tested and verified it works
in Linux, OpenBSD, Windows Vista and Windows 7. This AHCI emulation supports
NCQ, so multiple read or write requests can be outstanding at the same time.
The code is however not fully optimized yet. I'm fairly sure that there are
low hanging performance fruits to be found still :). In my simple benchmarks
I achieved about 2/3rd of virtio performance.
Also, this AHCI emulation layer does not support legacy mode. So if you're
using a disk with this emulation, you do not get it exposed using the legacy
IDE interfaces.
Another nitpick is CD-ROM support in Windows. Somehow it doesn't detect a
CD-ROM drive attached to AHCI. At least it doesn't list it.
To attach an AHCI disk to your VM, please use
-drive file=...,if=sata
This should do the trick for x86. On other platforms, you might need to add
the ahci host controller using -device.
This patch set is based on work done during the Google Summer of Code. I was
mentoring a student, Roland Elek, who wrote most of the AHCI emulation code
based on a patch from Chong Qiao. A bunch of other people were also involved,
so everybody who I didn't mention - thanks a lot!
v1 -> v2:
- rename IDEExtender to IDEBusOps and make a pointer (kraxel)
- make dma hooks explicit by putting them into ops struct (stefanha)
- use qdev buses (kraxel)
- minor cleanups
- dprintf overhaul
- add reset function
Alex
Alexander Graf (8):
ide: split ide command interpretation off
ide: fix whitespace gap in ide_exec_cmd
ide: add DMA hooks to bus ops
pci: add storage class for sata
pci: add ich7 pci id
ahci: add ahci emulation
ahci: add -drive support
ahci: spawn controller on demand
Roland Elek (2):
ide: add support for ide bus ops
ide: add ncq identify data for ahci sata drives
Makefile.objs | 1 +
Makefile.target | 4 +
blockdev.c | 6 +-
blockdev.h | 1 +
default-configs/i386-softmmu.mak | 1 +
default-configs/x86_64-softmmu.mak | 1 +
hw/ahci.c | 1280 ++++++++++++++++++++++++++++++++++++
hw/ide/core.c | 800 ++++++++++++-----------
hw/ide/internal.h | 34 +-
hw/pc.c | 5 +
hw/pci.h | 1 +
hw/pci_ids.h | 1 +
qemu-common.h | 2 +-
13 files changed, 1754 insertions(+), 383 deletions(-)
create mode 100644 hw/ahci.c
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH 01/10] ide: split ide command interpretation off
2010-11-18 3:27 [Qemu-devel] [PATCH 00/10] AHCI emulation support v2 Alexander Graf
@ 2010-11-18 3:27 ` Alexander Graf
2010-11-18 3:27 ` [Qemu-devel] [PATCH 02/10] ide: fix whitespace gap in ide_exec_cmd Alexander Graf
` (10 subsequent siblings)
11 siblings, 0 replies; 30+ messages in thread
From: Alexander Graf @ 2010-11-18 3:27 UTC (permalink / raw)
To: QEMU-devel Developers
Cc: Joerg Roedel, Gerd Hoffmann, Stefan Hajnoczi, tj, Roland Elek,
Sebastian Herbszt
The ATA command interpretation code can be used for PATA and SATA
interfaces alike. So let's split it out into a separate function.
Signed-off-by: Alexander Graf <agraf@suse.de>
---
hw/ide/core.c | 20 ++++++++++++++------
hw/ide/internal.h | 2 ++
2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 484e0ca..ee551ac 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1790,9 +1790,6 @@ static void ide_clear_hob(IDEBus *bus)
void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val)
{
IDEBus *bus = opaque;
- IDEState *s;
- int n;
- int lba48 = 0;
#ifdef DEBUG_IDE
printf("IDE: write addr=0x%x val=0x%02x\n", addr, val);
@@ -1853,17 +1850,29 @@ void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val)
default:
case 7:
/* command */
+ ide_exec_cmd(opaque, val);
+ break;
+ }
+}
+
+
+void ide_exec_cmd(IDEBus *bus, uint32_t val)
+{
+ IDEState *s;
+ int n;
+ int lba48 = 0;
+
#if defined(DEBUG_IDE)
printf("ide: CMD=%02x\n", val);
#endif
s = idebus_active_if(bus);
/* ignore commands to non existant slave */
if (s != bus->ifs && !s->bs)
- break;
+ return;
/* Only DEVICE RESET is allowed while BSY or/and DRQ are set */
if ((s->status & (BUSY_STAT|DRQ_STAT)) && val != WIN_DEVICE_RESET)
- break;
+ return;
switch(val) {
case WIN_IDENTIFY:
@@ -2354,7 +2363,6 @@ void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val)
ide_set_irq(s->bus);
break;
}
- }
}
uint32_t ide_ioport_read(void *opaque, uint32_t addr1)
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index d652e06..e7e1f80 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -564,6 +564,8 @@ void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
DriveInfo *hd1, qemu_irq irq);
void ide_init_ioport(IDEBus *bus, int iobase, int iobase2);
+void ide_exec_cmd(IDEBus *bus, uint32_t val);
+
/* hw/ide/qdev.c */
void ide_bus_new(IDEBus *idebus, DeviceState *dev);
IDEDevice *ide_create_drive(IDEBus *bus, int unit, DriveInfo *drive);
--
1.6.0.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH 02/10] ide: fix whitespace gap in ide_exec_cmd
2010-11-18 3:27 [Qemu-devel] [PATCH 00/10] AHCI emulation support v2 Alexander Graf
2010-11-18 3:27 ` [Qemu-devel] [PATCH 01/10] ide: split ide command interpretation off Alexander Graf
@ 2010-11-18 3:27 ` Alexander Graf
2010-11-18 3:27 ` [Qemu-devel] [PATCH 03/10] ide: add support for ide bus ops Alexander Graf
` (9 subsequent siblings)
11 siblings, 0 replies; 30+ messages in thread
From: Alexander Graf @ 2010-11-18 3:27 UTC (permalink / raw)
To: QEMU-devel Developers
Cc: Joerg Roedel, Gerd Hoffmann, Stefan Hajnoczi, tj, Roland Elek,
Sebastian Herbszt
Now that we have the function split out, we have to reindent it.
In order to increase the readability of the actual functional change,
this is split out.
Signed-off-by: Alexander Graf <agraf@suse.de>
---
hw/ide/core.c | 734 ++++++++++++++++++++++++++++----------------------------
1 files changed, 367 insertions(+), 367 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index ee551ac..1849069 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1863,423 +1863,423 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
int lba48 = 0;
#if defined(DEBUG_IDE)
- printf("ide: CMD=%02x\n", val);
+ printf("ide: CMD=%02x\n", val);
#endif
- s = idebus_active_if(bus);
- /* ignore commands to non existant slave */
- if (s != bus->ifs && !s->bs)
- return;
+ s = idebus_active_if(bus);
+ /* ignore commands to non existant slave */
+ if (s != bus->ifs && !s->bs)
+ return;
- /* Only DEVICE RESET is allowed while BSY or/and DRQ are set */
- if ((s->status & (BUSY_STAT|DRQ_STAT)) && val != WIN_DEVICE_RESET)
- return;
+ /* Only DEVICE RESET is allowed while BSY or/and DRQ are set */
+ if ((s->status & (BUSY_STAT|DRQ_STAT)) && val != WIN_DEVICE_RESET)
+ return;
- switch(val) {
- case WIN_IDENTIFY:
- if (s->bs && s->drive_kind != IDE_CD) {
- if (s->drive_kind != IDE_CFATA)
- ide_identify(s);
- else
- ide_cfata_identify(s);
- s->status = READY_STAT | SEEK_STAT;
- ide_transfer_start(s, s->io_buffer, 512, ide_transfer_stop);
- } else {
- if (s->drive_kind == IDE_CD) {
- ide_set_signature(s);
- }
- ide_abort_command(s);
- }
- ide_set_irq(s->bus);
- break;
- case WIN_SPECIFY:
- case WIN_RECAL:
- s->error = 0;
+ switch(val) {
+ case WIN_IDENTIFY:
+ if (s->bs && s->drive_kind != IDE_CD) {
+ if (s->drive_kind != IDE_CFATA)
+ ide_identify(s);
+ else
+ ide_cfata_identify(s);
s->status = READY_STAT | SEEK_STAT;
- ide_set_irq(s->bus);
- break;
- case WIN_SETMULT:
- if (s->drive_kind == IDE_CFATA && s->nsector == 0) {
- /* Disable Read and Write Multiple */
- s->mult_sectors = 0;
- s->status = READY_STAT | SEEK_STAT;
- } else if ((s->nsector & 0xff) != 0 &&
- ((s->nsector & 0xff) > MAX_MULT_SECTORS ||
- (s->nsector & (s->nsector - 1)) != 0)) {
- ide_abort_command(s);
- } else {
- s->mult_sectors = s->nsector & 0xff;
- s->status = READY_STAT | SEEK_STAT;
+ ide_transfer_start(s, s->io_buffer, 512, ide_transfer_stop);
+ } else {
+ if (s->drive_kind == IDE_CD) {
+ ide_set_signature(s);
}
- ide_set_irq(s->bus);
- break;
- case WIN_VERIFY_EXT:
- lba48 = 1;
- case WIN_VERIFY:
- case WIN_VERIFY_ONCE:
- /* do sector number check ? */
- ide_cmd_lba48_transform(s, lba48);
+ ide_abort_command(s);
+ }
+ ide_set_irq(s->bus);
+ break;
+ case WIN_SPECIFY:
+ case WIN_RECAL:
+ s->error = 0;
+ s->status = READY_STAT | SEEK_STAT;
+ ide_set_irq(s->bus);
+ break;
+ case WIN_SETMULT:
+ if (s->drive_kind == IDE_CFATA && s->nsector == 0) {
+ /* Disable Read and Write Multiple */
+ s->mult_sectors = 0;
s->status = READY_STAT | SEEK_STAT;
- ide_set_irq(s->bus);
- break;
+ } else if ((s->nsector & 0xff) != 0 &&
+ ((s->nsector & 0xff) > MAX_MULT_SECTORS ||
+ (s->nsector & (s->nsector - 1)) != 0)) {
+ ide_abort_command(s);
+ } else {
+ s->mult_sectors = s->nsector & 0xff;
+ s->status = READY_STAT | SEEK_STAT;
+ }
+ ide_set_irq(s->bus);
+ break;
+ case WIN_VERIFY_EXT:
+ lba48 = 1;
+ case WIN_VERIFY:
+ case WIN_VERIFY_ONCE:
+ /* do sector number check ? */
+ ide_cmd_lba48_transform(s, lba48);
+ s->status = READY_STAT | SEEK_STAT;
+ ide_set_irq(s->bus);
+ break;
case WIN_READ_EXT:
- lba48 = 1;
- case WIN_READ:
- case WIN_READ_ONCE:
- if (!s->bs)
- goto abort_cmd;
- ide_cmd_lba48_transform(s, lba48);
- s->req_nb_sectors = 1;
- ide_sector_read(s);
- break;
+ lba48 = 1;
+ case WIN_READ:
+ case WIN_READ_ONCE:
+ if (!s->bs)
+ goto abort_cmd;
+ ide_cmd_lba48_transform(s, lba48);
+ s->req_nb_sectors = 1;
+ ide_sector_read(s);
+ break;
case WIN_WRITE_EXT:
- lba48 = 1;
- case WIN_WRITE:
- case WIN_WRITE_ONCE:
- case CFA_WRITE_SECT_WO_ERASE:
- case WIN_WRITE_VERIFY:
- ide_cmd_lba48_transform(s, lba48);
- s->error = 0;
- s->status = SEEK_STAT | READY_STAT;
- s->req_nb_sectors = 1;
- ide_transfer_start(s, s->io_buffer, 512, ide_sector_write);
- s->media_changed = 1;
- break;
+ lba48 = 1;
+ case WIN_WRITE:
+ case WIN_WRITE_ONCE:
+ case CFA_WRITE_SECT_WO_ERASE:
+ case WIN_WRITE_VERIFY:
+ ide_cmd_lba48_transform(s, lba48);
+ s->error = 0;
+ s->status = SEEK_STAT | READY_STAT;
+ s->req_nb_sectors = 1;
+ ide_transfer_start(s, s->io_buffer, 512, ide_sector_write);
+ s->media_changed = 1;
+ break;
case WIN_MULTREAD_EXT:
- lba48 = 1;
- case WIN_MULTREAD:
- if (!s->mult_sectors)
- goto abort_cmd;
- ide_cmd_lba48_transform(s, lba48);
- s->req_nb_sectors = s->mult_sectors;
- ide_sector_read(s);
- break;
- case WIN_MULTWRITE_EXT:
- lba48 = 1;
- case WIN_MULTWRITE:
- case CFA_WRITE_MULTI_WO_ERASE:
- if (!s->mult_sectors)
- goto abort_cmd;
- ide_cmd_lba48_transform(s, lba48);
- s->error = 0;
- s->status = SEEK_STAT | READY_STAT;
- s->req_nb_sectors = s->mult_sectors;
- n = s->nsector;
- if (n > s->req_nb_sectors)
- n = s->req_nb_sectors;
- ide_transfer_start(s, s->io_buffer, 512 * n, ide_sector_write);
- s->media_changed = 1;
- break;
+ lba48 = 1;
+ case WIN_MULTREAD:
+ if (!s->mult_sectors)
+ goto abort_cmd;
+ ide_cmd_lba48_transform(s, lba48);
+ s->req_nb_sectors = s->mult_sectors;
+ ide_sector_read(s);
+ break;
+ case WIN_MULTWRITE_EXT:
+ lba48 = 1;
+ case WIN_MULTWRITE:
+ case CFA_WRITE_MULTI_WO_ERASE:
+ if (!s->mult_sectors)
+ goto abort_cmd;
+ ide_cmd_lba48_transform(s, lba48);
+ s->error = 0;
+ s->status = SEEK_STAT | READY_STAT;
+ s->req_nb_sectors = s->mult_sectors;
+ n = s->nsector;
+ if (n > s->req_nb_sectors)
+ n = s->req_nb_sectors;
+ ide_transfer_start(s, s->io_buffer, 512 * n, ide_sector_write);
+ s->media_changed = 1;
+ break;
case WIN_READDMA_EXT:
- lba48 = 1;
- case WIN_READDMA:
- case WIN_READDMA_ONCE:
- if (!s->bs)
- goto abort_cmd;
- ide_cmd_lba48_transform(s, lba48);
- ide_sector_read_dma(s);
- break;
+ lba48 = 1;
+ case WIN_READDMA:
+ case WIN_READDMA_ONCE:
+ if (!s->bs)
+ goto abort_cmd;
+ ide_cmd_lba48_transform(s, lba48);
+ ide_sector_read_dma(s);
+ break;
case WIN_WRITEDMA_EXT:
- lba48 = 1;
- case WIN_WRITEDMA:
- case WIN_WRITEDMA_ONCE:
- if (!s->bs)
- goto abort_cmd;
- ide_cmd_lba48_transform(s, lba48);
- ide_sector_write_dma(s);
- s->media_changed = 1;
- break;
- case WIN_READ_NATIVE_MAX_EXT:
- lba48 = 1;
- case WIN_READ_NATIVE_MAX:
- ide_cmd_lba48_transform(s, lba48);
- ide_set_sector(s, s->nb_sectors - 1);
- s->status = READY_STAT | SEEK_STAT;
- ide_set_irq(s->bus);
- break;
- case WIN_CHECKPOWERMODE1:
- case WIN_CHECKPOWERMODE2:
- s->nsector = 0xff; /* device active or idle */
+ lba48 = 1;
+ case WIN_WRITEDMA:
+ case WIN_WRITEDMA_ONCE:
+ if (!s->bs)
+ goto abort_cmd;
+ ide_cmd_lba48_transform(s, lba48);
+ ide_sector_write_dma(s);
+ s->media_changed = 1;
+ break;
+ case WIN_READ_NATIVE_MAX_EXT:
+ lba48 = 1;
+ case WIN_READ_NATIVE_MAX:
+ ide_cmd_lba48_transform(s, lba48);
+ ide_set_sector(s, s->nb_sectors - 1);
+ s->status = READY_STAT | SEEK_STAT;
+ ide_set_irq(s->bus);
+ break;
+ case WIN_CHECKPOWERMODE1:
+ case WIN_CHECKPOWERMODE2:
+ s->nsector = 0xff; /* device active or idle */
+ s->status = READY_STAT | SEEK_STAT;
+ ide_set_irq(s->bus);
+ break;
+ case WIN_SETFEATURES:
+ if (!s->bs)
+ goto abort_cmd;
+ /* XXX: valid for CDROM ? */
+ switch(s->feature) {
+ case 0xcc: /* reverting to power-on defaults enable */
+ case 0x66: /* reverting to power-on defaults disable */
+ case 0x02: /* write cache enable */
+ case 0x82: /* write cache disable */
+ case 0xaa: /* read look-ahead enable */
+ case 0x55: /* read look-ahead disable */
+ case 0x05: /* set advanced power management mode */
+ case 0x85: /* disable advanced power management mode */
+ case 0x69: /* NOP */
+ case 0x67: /* NOP */
+ case 0x96: /* NOP */
+ case 0x9a: /* NOP */
+ case 0x42: /* enable Automatic Acoustic Mode */
+ case 0xc2: /* disable Automatic Acoustic Mode */
s->status = READY_STAT | SEEK_STAT;
ide_set_irq(s->bus);
break;
- case WIN_SETFEATURES:
- if (!s->bs)
- goto abort_cmd;
- /* XXX: valid for CDROM ? */
- switch(s->feature) {
- case 0xcc: /* reverting to power-on defaults enable */
- case 0x66: /* reverting to power-on defaults disable */
- case 0x02: /* write cache enable */
- case 0x82: /* write cache disable */
- case 0xaa: /* read look-ahead enable */
- case 0x55: /* read look-ahead disable */
- case 0x05: /* set advanced power management mode */
- case 0x85: /* disable advanced power management mode */
- case 0x69: /* NOP */
- case 0x67: /* NOP */
- case 0x96: /* NOP */
- case 0x9a: /* NOP */
- case 0x42: /* enable Automatic Acoustic Mode */
- case 0xc2: /* disable Automatic Acoustic Mode */
- s->status = READY_STAT | SEEK_STAT;
- ide_set_irq(s->bus);
- break;
- case 0x03: { /* set transfer mode */
+ case 0x03: { /* set transfer mode */
uint8_t val = s->nsector & 0x07;
- uint16_t *identify_data = (uint16_t *)s->identify_data;
+ uint16_t *identify_data = (uint16_t *)s->identify_data;
switch (s->nsector >> 3) {
- case 0x00: /* pio default */
- case 0x01: /* pio mode */
+ case 0x00: /* pio default */
+ case 0x01: /* pio mode */
put_le16(identify_data + 62,0x07);
put_le16(identify_data + 63,0x07);
put_le16(identify_data + 88,0x3f);
break;
- case 0x02: /* sigle word dma mode*/
+ case 0x02: /* sigle word dma mode*/
put_le16(identify_data + 62,0x07 | (1 << (val + 8)));
put_le16(identify_data + 63,0x07);
put_le16(identify_data + 88,0x3f);
break;
- case 0x04: /* mdma mode */
+ case 0x04: /* mdma mode */
put_le16(identify_data + 62,0x07);
put_le16(identify_data + 63,0x07 | (1 << (val + 8)));
put_le16(identify_data + 88,0x3f);
break;
- case 0x08: /* udma mode */
+ case 0x08: /* udma mode */
put_le16(identify_data + 62,0x07);
put_le16(identify_data + 63,0x07);
put_le16(identify_data + 88,0x3f | (1 << (val + 8)));
break;
- default:
+ default:
goto abort_cmd;
}
- s->status = READY_STAT | SEEK_STAT;
- ide_set_irq(s->bus);
- break;
- }
- default:
- goto abort_cmd;
- }
- break;
- case WIN_FLUSH_CACHE:
- case WIN_FLUSH_CACHE_EXT:
- ide_flush_cache(s);
- break;
- case WIN_STANDBY:
- case WIN_STANDBY2:
- case WIN_STANDBYNOW1:
- case WIN_STANDBYNOW2:
- case WIN_IDLEIMMEDIATE:
- case CFA_IDLEIMMEDIATE:
- case WIN_SETIDLE1:
- case WIN_SETIDLE2:
- case WIN_SLEEPNOW1:
- case WIN_SLEEPNOW2:
- s->status = READY_STAT;
- ide_set_irq(s->bus);
- break;
- case WIN_SEEK:
- if(s->drive_kind == IDE_CD)
- goto abort_cmd;
- /* XXX: Check that seek is within bounds */
s->status = READY_STAT | SEEK_STAT;
ide_set_irq(s->bus);
break;
- /* ATAPI commands */
- case WIN_PIDENTIFY:
- if (s->drive_kind == IDE_CD) {
- ide_atapi_identify(s);
- s->status = READY_STAT | SEEK_STAT;
- ide_transfer_start(s, s->io_buffer, 512, ide_transfer_stop);
- } else {
- ide_abort_command(s);
- }
- ide_set_irq(s->bus);
- break;
- case WIN_DIAGNOSE:
- ide_set_signature(s);
- if (s->drive_kind == IDE_CD)
- s->status = 0; /* ATAPI spec (v6) section 9.10 defines packet
- * devices to return a clear status register
- * with READY_STAT *not* set. */
- else
- s->status = READY_STAT | SEEK_STAT;
- s->error = 0x01; /* Device 0 passed, Device 1 passed or not
- * present.
- */
- ide_set_irq(s->bus);
- break;
- case WIN_SRST:
- if (s->drive_kind != IDE_CD)
- goto abort_cmd;
- ide_set_signature(s);
- s->status = 0x00; /* NOTE: READY is _not_ set */
- s->error = 0x01;
- break;
- case WIN_PACKETCMD:
- if (s->drive_kind != IDE_CD)
- goto abort_cmd;
- /* overlapping commands not supported */
- if (s->feature & 0x02)
- goto abort_cmd;
+ }
+ default:
+ goto abort_cmd;
+ }
+ break;
+ case WIN_FLUSH_CACHE:
+ case WIN_FLUSH_CACHE_EXT:
+ ide_flush_cache(s);
+ break;
+ case WIN_STANDBY:
+ case WIN_STANDBY2:
+ case WIN_STANDBYNOW1:
+ case WIN_STANDBYNOW2:
+ case WIN_IDLEIMMEDIATE:
+ case CFA_IDLEIMMEDIATE:
+ case WIN_SETIDLE1:
+ case WIN_SETIDLE2:
+ case WIN_SLEEPNOW1:
+ case WIN_SLEEPNOW2:
+ s->status = READY_STAT;
+ ide_set_irq(s->bus);
+ break;
+ case WIN_SEEK:
+ if(s->drive_kind == IDE_CD)
+ goto abort_cmd;
+ /* XXX: Check that seek is within bounds */
+ s->status = READY_STAT | SEEK_STAT;
+ ide_set_irq(s->bus);
+ break;
+ /* ATAPI commands */
+ case WIN_PIDENTIFY:
+ if (s->drive_kind == IDE_CD) {
+ ide_atapi_identify(s);
s->status = READY_STAT | SEEK_STAT;
- s->atapi_dma = s->feature & 1;
- s->nsector = 1;
- ide_transfer_start(s, s->io_buffer, ATAPI_PACKET_SIZE,
- ide_atapi_cmd);
- break;
- /* CF-ATA commands */
- case CFA_REQ_EXT_ERROR_CODE:
- if (s->drive_kind != IDE_CFATA)
- goto abort_cmd;
- s->error = 0x09; /* miscellaneous error */
+ ide_transfer_start(s, s->io_buffer, 512, ide_transfer_stop);
+ } else {
+ ide_abort_command(s);
+ }
+ ide_set_irq(s->bus);
+ break;
+ case WIN_DIAGNOSE:
+ ide_set_signature(s);
+ if (s->drive_kind == IDE_CD)
+ s->status = 0; /* ATAPI spec (v6) section 9.10 defines packet
+ * devices to return a clear status register
+ * with READY_STAT *not* set. */
+ else
s->status = READY_STAT | SEEK_STAT;
- ide_set_irq(s->bus);
+ s->error = 0x01; /* Device 0 passed, Device 1 passed or not
+ * present.
+ */
+ ide_set_irq(s->bus);
+ break;
+ case WIN_SRST:
+ if (s->drive_kind != IDE_CD)
+ goto abort_cmd;
+ ide_set_signature(s);
+ s->status = 0x00; /* NOTE: READY is _not_ set */
+ s->error = 0x01;
+ break;
+ case WIN_PACKETCMD:
+ if (s->drive_kind != IDE_CD)
+ goto abort_cmd;
+ /* overlapping commands not supported */
+ if (s->feature & 0x02)
+ goto abort_cmd;
+ s->status = READY_STAT | SEEK_STAT;
+ s->atapi_dma = s->feature & 1;
+ s->nsector = 1;
+ ide_transfer_start(s, s->io_buffer, ATAPI_PACKET_SIZE,
+ ide_atapi_cmd);
+ break;
+ /* CF-ATA commands */
+ case CFA_REQ_EXT_ERROR_CODE:
+ if (s->drive_kind != IDE_CFATA)
+ goto abort_cmd;
+ s->error = 0x09; /* miscellaneous error */
+ s->status = READY_STAT | SEEK_STAT;
+ ide_set_irq(s->bus);
+ break;
+ case CFA_ERASE_SECTORS:
+ case CFA_WEAR_LEVEL:
+ if (s->drive_kind != IDE_CFATA)
+ goto abort_cmd;
+ if (val == CFA_WEAR_LEVEL)
+ s->nsector = 0;
+ if (val == CFA_ERASE_SECTORS)
+ s->media_changed = 1;
+ s->error = 0x00;
+ s->status = READY_STAT | SEEK_STAT;
+ ide_set_irq(s->bus);
+ break;
+ case CFA_TRANSLATE_SECTOR:
+ if (s->drive_kind != IDE_CFATA)
+ goto abort_cmd;
+ s->error = 0x00;
+ s->status = READY_STAT | SEEK_STAT;
+ memset(s->io_buffer, 0, 0x200);
+ s->io_buffer[0x00] = s->hcyl; /* Cyl MSB */
+ s->io_buffer[0x01] = s->lcyl; /* Cyl LSB */
+ s->io_buffer[0x02] = s->select; /* Head */
+ s->io_buffer[0x03] = s->sector; /* Sector */
+ s->io_buffer[0x04] = ide_get_sector(s) >> 16; /* LBA MSB */
+ s->io_buffer[0x05] = ide_get_sector(s) >> 8; /* LBA */
+ s->io_buffer[0x06] = ide_get_sector(s) >> 0; /* LBA LSB */
+ s->io_buffer[0x13] = 0x00; /* Erase flag */
+ s->io_buffer[0x18] = 0x00; /* Hot count */
+ s->io_buffer[0x19] = 0x00; /* Hot count */
+ s->io_buffer[0x1a] = 0x01; /* Hot count */
+ ide_transfer_start(s, s->io_buffer, 0x200, ide_transfer_stop);
+ ide_set_irq(s->bus);
+ break;
+ case CFA_ACCESS_METADATA_STORAGE:
+ if (s->drive_kind != IDE_CFATA)
+ goto abort_cmd;
+ switch (s->feature) {
+ case 0x02: /* Inquiry Metadata Storage */
+ ide_cfata_metadata_inquiry(s);
break;
- case CFA_ERASE_SECTORS:
- case CFA_WEAR_LEVEL:
- if (s->drive_kind != IDE_CFATA)
- goto abort_cmd;
- if (val == CFA_WEAR_LEVEL)
- s->nsector = 0;
- if (val == CFA_ERASE_SECTORS)
- s->media_changed = 1;
- s->error = 0x00;
- s->status = READY_STAT | SEEK_STAT;
- ide_set_irq(s->bus);
+ case 0x03: /* Read Metadata Storage */
+ ide_cfata_metadata_read(s);
break;
- case CFA_TRANSLATE_SECTOR:
- if (s->drive_kind != IDE_CFATA)
- goto abort_cmd;
- s->error = 0x00;
- s->status = READY_STAT | SEEK_STAT;
- memset(s->io_buffer, 0, 0x200);
- s->io_buffer[0x00] = s->hcyl; /* Cyl MSB */
- s->io_buffer[0x01] = s->lcyl; /* Cyl LSB */
- s->io_buffer[0x02] = s->select; /* Head */
- s->io_buffer[0x03] = s->sector; /* Sector */
- s->io_buffer[0x04] = ide_get_sector(s) >> 16; /* LBA MSB */
- s->io_buffer[0x05] = ide_get_sector(s) >> 8; /* LBA */
- s->io_buffer[0x06] = ide_get_sector(s) >> 0; /* LBA LSB */
- s->io_buffer[0x13] = 0x00; /* Erase flag */
- s->io_buffer[0x18] = 0x00; /* Hot count */
- s->io_buffer[0x19] = 0x00; /* Hot count */
- s->io_buffer[0x1a] = 0x01; /* Hot count */
- ide_transfer_start(s, s->io_buffer, 0x200, ide_transfer_stop);
- ide_set_irq(s->bus);
+ case 0x04: /* Write Metadata Storage */
+ ide_cfata_metadata_write(s);
break;
- case CFA_ACCESS_METADATA_STORAGE:
- if (s->drive_kind != IDE_CFATA)
- goto abort_cmd;
- switch (s->feature) {
- case 0x02: /* Inquiry Metadata Storage */
- ide_cfata_metadata_inquiry(s);
- break;
- case 0x03: /* Read Metadata Storage */
- ide_cfata_metadata_read(s);
- break;
- case 0x04: /* Write Metadata Storage */
- ide_cfata_metadata_write(s);
- break;
- default:
- goto abort_cmd;
- }
- ide_transfer_start(s, s->io_buffer, 0x200, ide_transfer_stop);
- s->status = 0x00; /* NOTE: READY is _not_ set */
- ide_set_irq(s->bus);
- break;
- case IBM_SENSE_CONDITION:
- if (s->drive_kind != IDE_CFATA)
- goto abort_cmd;
- switch (s->feature) {
- case 0x01: /* sense temperature in device */
- s->nsector = 0x50; /* +20 C */
- break;
- default:
- goto abort_cmd;
- }
- s->status = READY_STAT | SEEK_STAT;
- ide_set_irq(s->bus);
+ default:
+ goto abort_cmd;
+ }
+ ide_transfer_start(s, s->io_buffer, 0x200, ide_transfer_stop);
+ s->status = 0x00; /* NOTE: READY is _not_ set */
+ ide_set_irq(s->bus);
+ break;
+ case IBM_SENSE_CONDITION:
+ if (s->drive_kind != IDE_CFATA)
+ goto abort_cmd;
+ switch (s->feature) {
+ case 0x01: /* sense temperature in device */
+ s->nsector = 0x50; /* +20 C */
break;
+ default:
+ goto abort_cmd;
+ }
+ s->status = READY_STAT | SEEK_STAT;
+ ide_set_irq(s->bus);
+ break;
case WIN_SMART:
- if (s->drive_kind == IDE_CD)
+ if (s->drive_kind == IDE_CD)
goto abort_cmd;
- if (s->hcyl != 0xc2 || s->lcyl != 0x4f)
+ if (s->hcyl != 0xc2 || s->lcyl != 0x4f)
goto abort_cmd;
- if (!s->smart_enabled && s->feature != SMART_ENABLE)
+ if (!s->smart_enabled && s->feature != SMART_ENABLE)
goto abort_cmd;
- switch (s->feature) {
- case SMART_DISABLE:
+ switch (s->feature) {
+ case SMART_DISABLE:
s->smart_enabled = 0;
s->status = READY_STAT | SEEK_STAT;
ide_set_irq(s->bus);
break;
- case SMART_ENABLE:
+ case SMART_ENABLE:
s->smart_enabled = 1;
s->status = READY_STAT | SEEK_STAT;
ide_set_irq(s->bus);
break;
- case SMART_ATTR_AUTOSAVE:
+ case SMART_ATTR_AUTOSAVE:
switch (s->sector) {
case 0x00:
- s->smart_autosave = 0;
- break;
+ s->smart_autosave = 0;
+ break;
case 0xf1:
- s->smart_autosave = 1;
- break;
+ s->smart_autosave = 1;
+ break;
default:
- goto abort_cmd;
+ goto abort_cmd;
}
s->status = READY_STAT | SEEK_STAT;
ide_set_irq(s->bus);
break;
- case SMART_STATUS:
+ case SMART_STATUS:
if (!s->smart_errors) {
- s->hcyl = 0xc2;
- s->lcyl = 0x4f;
+ s->hcyl = 0xc2;
+ s->lcyl = 0x4f;
} else {
- s->hcyl = 0x2c;
- s->lcyl = 0xf4;
+ s->hcyl = 0x2c;
+ s->lcyl = 0xf4;
}
s->status = READY_STAT | SEEK_STAT;
ide_set_irq(s->bus);
break;
- case SMART_READ_THRESH:
+ case SMART_READ_THRESH:
memset(s->io_buffer, 0, 0x200);
s->io_buffer[0] = 0x01; /* smart struct version */
for (n=0; n<30; n++) {
- if (smart_attributes[n][0] == 0)
+ if (smart_attributes[n][0] == 0)
break;
- s->io_buffer[2+0+(n*12)] = smart_attributes[n][0];
- s->io_buffer[2+1+(n*12)] = smart_attributes[n][4];
+ s->io_buffer[2+0+(n*12)] = smart_attributes[n][0];
+ s->io_buffer[2+1+(n*12)] = smart_attributes[n][4];
}
for (n=0; n<511; n++) /* checksum */
- s->io_buffer[511] += s->io_buffer[n];
+ s->io_buffer[511] += s->io_buffer[n];
s->io_buffer[511] = 0x100 - s->io_buffer[511];
s->status = READY_STAT | SEEK_STAT;
ide_transfer_start(s, s->io_buffer, 0x200, ide_transfer_stop);
ide_set_irq(s->bus);
break;
- case SMART_READ_DATA:
+ case SMART_READ_DATA:
memset(s->io_buffer, 0, 0x200);
s->io_buffer[0] = 0x01; /* smart struct version */
for (n=0; n<30; n++) {
- if (smart_attributes[n][0] == 0)
+ if (smart_attributes[n][0] == 0)
break;
- s->io_buffer[2+0+(n*12)] = smart_attributes[n][0];
- s->io_buffer[2+1+(n*12)] = smart_attributes[n][1];
- s->io_buffer[2+3+(n*12)] = smart_attributes[n][2];
- s->io_buffer[2+4+(n*12)] = smart_attributes[n][3];
+ s->io_buffer[2+0+(n*12)] = smart_attributes[n][0];
+ s->io_buffer[2+1+(n*12)] = smart_attributes[n][1];
+ s->io_buffer[2+3+(n*12)] = smart_attributes[n][2];
+ s->io_buffer[2+4+(n*12)] = smart_attributes[n][3];
}
s->io_buffer[362] = 0x02 | (s->smart_autosave?0x80:0x00);
if (s->smart_selftest_count == 0) {
- s->io_buffer[363] = 0;
+ s->io_buffer[363] = 0;
} else {
- s->io_buffer[363] =
+ s->io_buffer[363] =
s->smart_selftest_data[3 +
- (s->smart_selftest_count - 1) *
- 24];
+ (s->smart_selftest_count - 1) *
+ 24];
}
s->io_buffer[364] = 0x20;
s->io_buffer[365] = 0x01;
@@ -2293,76 +2293,76 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
s->io_buffer[374] = 0x01; /* minutes for poll conveyance */
for (n=0; n<511; n++)
- s->io_buffer[511] += s->io_buffer[n];
+ s->io_buffer[511] += s->io_buffer[n];
s->io_buffer[511] = 0x100 - s->io_buffer[511];
s->status = READY_STAT | SEEK_STAT;
ide_transfer_start(s, s->io_buffer, 0x200, ide_transfer_stop);
ide_set_irq(s->bus);
break;
- case SMART_READ_LOG:
+ case SMART_READ_LOG:
switch (s->sector) {
case 0x01: /* summary smart error log */
- memset(s->io_buffer, 0, 0x200);
- s->io_buffer[0] = 0x01;
- s->io_buffer[1] = 0x00; /* no error entries */
- s->io_buffer[452] = s->smart_errors & 0xff;
- s->io_buffer[453] = (s->smart_errors & 0xff00) >> 8;
+ memset(s->io_buffer, 0, 0x200);
+ s->io_buffer[0] = 0x01;
+ s->io_buffer[1] = 0x00; /* no error entries */
+ s->io_buffer[452] = s->smart_errors & 0xff;
+ s->io_buffer[453] = (s->smart_errors & 0xff00) >> 8;
- for (n=0; n<511; n++)
+ for (n=0; n<511; n++)
s->io_buffer[511] += s->io_buffer[n];
- s->io_buffer[511] = 0x100 - s->io_buffer[511];
- break;
+ s->io_buffer[511] = 0x100 - s->io_buffer[511];
+ break;
case 0x06: /* smart self test log */
- memset(s->io_buffer, 0, 0x200);
- s->io_buffer[0] = 0x01;
- if (s->smart_selftest_count == 0) {
+ memset(s->io_buffer, 0, 0x200);
+ s->io_buffer[0] = 0x01;
+ if (s->smart_selftest_count == 0) {
s->io_buffer[508] = 0;
- } else {
+ } else {
s->io_buffer[508] = s->smart_selftest_count;
for (n=2; n<506; n++)
- s->io_buffer[n] = s->smart_selftest_data[n];
- }
- for (n=0; n<511; n++)
+ s->io_buffer[n] = s->smart_selftest_data[n];
+ }
+ for (n=0; n<511; n++)
s->io_buffer[511] += s->io_buffer[n];
- s->io_buffer[511] = 0x100 - s->io_buffer[511];
- break;
+ s->io_buffer[511] = 0x100 - s->io_buffer[511];
+ break;
default:
- goto abort_cmd;
+ goto abort_cmd;
}
s->status = READY_STAT | SEEK_STAT;
ide_transfer_start(s, s->io_buffer, 0x200, ide_transfer_stop);
ide_set_irq(s->bus);
break;
- case SMART_EXECUTE_OFFLINE:
+ case SMART_EXECUTE_OFFLINE:
switch (s->sector) {
case 0: /* off-line routine */
case 1: /* short self test */
case 2: /* extended self test */
- s->smart_selftest_count++;
- if(s->smart_selftest_count > 21)
+ s->smart_selftest_count++;
+ if(s->smart_selftest_count > 21)
s->smart_selftest_count = 0;
- n = 2 + (s->smart_selftest_count - 1) * 24;
- s->smart_selftest_data[n] = s->sector;
- s->smart_selftest_data[n+1] = 0x00; /* OK and finished */
- s->smart_selftest_data[n+2] = 0x34; /* hour count lsb */
- s->smart_selftest_data[n+3] = 0x12; /* hour count msb */
- s->status = READY_STAT | SEEK_STAT;
- ide_set_irq(s->bus);
- break;
+ n = 2 + (s->smart_selftest_count - 1) * 24;
+ s->smart_selftest_data[n] = s->sector;
+ s->smart_selftest_data[n+1] = 0x00; /* OK and finished */
+ s->smart_selftest_data[n+2] = 0x34; /* hour count lsb */
+ s->smart_selftest_data[n+3] = 0x12; /* hour count msb */
+ s->status = READY_STAT | SEEK_STAT;
+ ide_set_irq(s->bus);
+ break;
default:
- goto abort_cmd;
+ goto abort_cmd;
}
break;
- default:
+ default:
goto abort_cmd;
- }
- break;
- default:
- abort_cmd:
- ide_abort_command(s);
- ide_set_irq(s->bus);
- break;
- }
+ }
+ break;
+ default:
+ abort_cmd:
+ ide_abort_command(s);
+ ide_set_irq(s->bus);
+ break;
+ }
}
uint32_t ide_ioport_read(void *opaque, uint32_t addr1)
--
1.6.0.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH 03/10] ide: add support for ide bus ops
2010-11-18 3:27 [Qemu-devel] [PATCH 00/10] AHCI emulation support v2 Alexander Graf
2010-11-18 3:27 ` [Qemu-devel] [PATCH 01/10] ide: split ide command interpretation off Alexander Graf
2010-11-18 3:27 ` [Qemu-devel] [PATCH 02/10] ide: fix whitespace gap in ide_exec_cmd Alexander Graf
@ 2010-11-18 3:27 ` Alexander Graf
2010-11-18 3:27 ` [Qemu-devel] [PATCH 04/10] ide: add DMA hooks to " Alexander Graf
` (8 subsequent siblings)
11 siblings, 0 replies; 30+ messages in thread
From: Alexander Graf @ 2010-11-18 3:27 UTC (permalink / raw)
To: QEMU-devel Developers
Cc: Joerg Roedel, Gerd Hoffmann, Stefan Hajnoczi, tj, Roland Elek,
Sebastian Herbszt
From: Roland Elek <elek.roland@gmail.com>
We need to hook into some of the core IDE functionality for AHCI. To
do that, the easiest way is to make explicit functions calls be implicit
through a function call struct.
Signed-off-by: Roland Elek <elek.roland@gmail.com>
Signed-off-by: Alexander Graf <agraf@suse.de>
---
v1 -> v2:
- rename IDEExtender to IDEBusOps and make a pointer (kraxel)
---
hw/ide/core.c | 36 +++++++++++++++++++++++++++++++++++-
hw/ide/internal.h | 26 +++++++++++++++++++-------
2 files changed, 54 insertions(+), 8 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 1849069..c8d7810 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -67,6 +67,8 @@ static void ide_atapi_cmd_read_dma_cb(void *opaque, int ret);
static int ide_handle_rw_error(IDEState *s, int error, int op);
static void ide_flush_cache(IDEState *s);
+static void pata_dma_start(IDEState *s, BlockDriverCompletionFunc *dma_cb);
+
static void padstr(char *str, const char *src, int len)
{
int i, v;
@@ -325,6 +327,12 @@ static inline void ide_dma_submit_check(IDEState *s,
static void ide_transfer_start(IDEState *s, uint8_t *buf, int size,
EndTransferFunc *end_transfer_func)
{
+ s->bus->ops->transfer_start_fn(s,buf,size,end_transfer_func);
+}
+
+static void pata_transfer_start(IDEState *s, uint8_t *buf, int size,
+ EndTransferFunc *end_transfer_func)
+{
s->end_transfer_func = end_transfer_func;
s->data_ptr = buf;
s->data_end = buf + size;
@@ -2580,6 +2588,18 @@ static void ide_dummy_transfer_stop(IDEState *s)
s->io_buffer[3] = 0xff;
}
+static void pata_set_irq(IDEBus *bus)
+{
+ BMDMAState *bm = bus->bmdma;
+
+ if (!(bus->cmd & IDE_CMD_DISABLE_IRQ)) {
+ if (bm) {
+ bm->status |= BM_STATUS_INT;
+ }
+ qemu_irq_raise(bus->irq);
+ }
+}
+
static void ide_reset(IDEState *s)
{
#ifdef DEBUG_IDE
@@ -2716,6 +2736,12 @@ static void ide_init1(IDEBus *bus, int unit)
ide_sector_write_timer_cb, s);
}
+static IDEBusOps ide_bus_ops = {
+ .transfer_start_fn = pata_transfer_start,
+ .irq_set_fn = pata_set_irq,
+ .dma_start_fn = pata_dma_start,
+};
+
void ide_init2(IDEBus *bus, qemu_irq irq)
{
int i;
@@ -2725,6 +2751,7 @@ void ide_init2(IDEBus *bus, qemu_irq irq)
ide_reset(&bus->ifs[i]);
}
bus->irq = irq;
+ bus->ops = &ide_bus_ops;
}
/* TODO convert users to qdev and remove */
@@ -2748,6 +2775,7 @@ void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
}
}
bus->irq = irq;
+ bus->ops = &ide_bus_ops;
}
void ide_init_ioport(IDEBus *bus, int iobase, int iobase2)
@@ -2919,9 +2947,10 @@ const VMStateDescription vmstate_ide_bus = {
/***********************************************************/
/* PCI IDE definitions */
-static void ide_dma_start(IDEState *s, BlockDriverCompletionFunc *dma_cb)
+static void pata_dma_start(IDEState *s, BlockDriverCompletionFunc *dma_cb)
{
BMDMAState *bm = s->bus->bmdma;
+
if(!bm)
return;
bm->unit = s->unit;
@@ -2936,6 +2965,11 @@ static void ide_dma_start(IDEState *s, BlockDriverCompletionFunc *dma_cb)
}
}
+static void ide_dma_start(IDEState *s, BlockDriverCompletionFunc *dma_cb)
+{
+ s->bus->ops->dma_start_fn(s,dma_cb);
+}
+
static void ide_dma_restart(IDEState *s, int is_read)
{
BMDMAState *bm = s->bus->bmdma;
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index e7e1f80..ee7e13e 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -20,6 +20,7 @@ typedef struct IDEDevice IDEDevice;
typedef struct IDEDeviceInfo IDEDeviceInfo;
typedef struct IDEState IDEState;
typedef struct BMDMAState BMDMAState;
+typedef struct IDEBusOps IDEBusOps;
/* Bits of HD_STATUS */
#define ERR_STAT 0x01
@@ -366,6 +367,14 @@ typedef enum { IDE_HD, IDE_CD, IDE_CFATA } IDEDriveKind;
typedef void EndTransferFunc(IDEState *);
+
+typedef void TransferStartFunc(IDEState *,
+ uint8_t *,
+ int,
+ EndTransferFunc *);
+typedef void IRQSetFunc(IDEBus *);
+typedef void DMAStartFunc(IDEState *, BlockDriverCompletionFunc *);
+
/* NOTE: IDEState represents in fact one drive */
struct IDEState {
IDEBus *bus;
@@ -442,12 +451,21 @@ struct IDEState {
uint8_t *smart_selftest_data;
};
+/* This struct represents a device that uses an IDE bus, but requires
+ * modifications to how it works. An example is AHCI. */
+struct IDEBusOps {
+ TransferStartFunc *transfer_start_fn;
+ IRQSetFunc *irq_set_fn;
+ DMAStartFunc *dma_start_fn;
+};
+
struct IDEBus {
BusState qbus;
IDEDevice *master;
IDEDevice *slave;
BMDMAState *bmdma;
IDEState ifs[2];
+ IDEBusOps *ops;
uint8_t unit;
uint8_t cmd;
qemu_irq irq;
@@ -512,13 +530,7 @@ static inline IDEState *bmdma_active_if(BMDMAState *bmdma)
static inline void ide_set_irq(IDEBus *bus)
{
- BMDMAState *bm = bus->bmdma;
- if (!(bus->cmd & IDE_CMD_DISABLE_IRQ)) {
- if (bm) {
- bm->status |= BM_STATUS_INT;
- }
- qemu_irq_raise(bus->irq);
- }
+ bus->ops->irq_set_fn(bus);
}
/* hw/ide/core.c */
--
1.6.0.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH 04/10] ide: add DMA hooks to bus ops
2010-11-18 3:27 [Qemu-devel] [PATCH 00/10] AHCI emulation support v2 Alexander Graf
` (2 preceding siblings ...)
2010-11-18 3:27 ` [Qemu-devel] [PATCH 03/10] ide: add support for ide bus ops Alexander Graf
@ 2010-11-18 3:27 ` Alexander Graf
2010-11-18 3:27 ` [Qemu-devel] [PATCH 05/10] ide: add ncq identify data for ahci sata drives Alexander Graf
` (7 subsequent siblings)
11 siblings, 0 replies; 30+ messages in thread
From: Alexander Graf @ 2010-11-18 3:27 UTC (permalink / raw)
To: QEMU-devel Developers
Cc: Joerg Roedel, Gerd Hoffmann, Stefan Hajnoczi, tj, Roland Elek,
Sebastian Herbszt
For DMA operations, we need to hook into even more IDE functionality.
This patch adds the respective hooking points, allowing us to handle
SG lists ourselves in the AHCI code.
Signed-off-by: Roland Elek <elek.roland@gmail.com>
Signed-off-by: Alexander Graf <agraf@suse.de>
---
v1 -> v2:
- make dma hooks explicit by putting them into ops struct (stefanha)
---
hw/ide/core.c | 9 ++++++---
hw/ide/internal.h | 4 ++++
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index c8d7810..04190d2 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -607,7 +607,7 @@ static void ide_read_dma_cb(void *opaque, int ret)
n = s->nsector;
s->io_buffer_index = 0;
s->io_buffer_size = n * 512;
- if (dma_buf_prepare(bm, 1) == 0)
+ if (s->bus->ops->dma_prepare_fn(bm, 1) == 0)
goto eot;
#ifdef DEBUG_AIO
printf("aio_read: sector_num=%" PRId64 " n=%d\n", sector_num, n);
@@ -752,7 +752,7 @@ static void ide_write_dma_cb(void *opaque, int ret)
n = s->nsector;
s->io_buffer_size = n * 512;
/* launch next transfer */
- if (dma_buf_prepare(bm, 0) == 0)
+ if (s->bus->ops->dma_prepare_fn(bm, 0) == 0)
goto eot;
#ifdef DEBUG_AIO
printf("aio_write: sector_num=%" PRId64 " n=%d\n", sector_num, n);
@@ -1060,7 +1060,7 @@ static void ide_atapi_cmd_read_dma_cb(void *opaque, int ret)
s->lba += n;
}
s->packet_transfer_size -= s->io_buffer_size;
- if (dma_buf_rw(bm, 1) == 0)
+ if (s->bus->ops->dma_rw_fn(bm, 1) == 0)
goto eot;
}
@@ -2715,6 +2715,7 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs,
} else {
pstrcpy(s->version, sizeof(s->version), QEMU_VERSION);
}
+
ide_reset(s);
bdrv_set_removable(bs, s->drive_kind == IDE_CD);
return 0;
@@ -2740,6 +2741,8 @@ static IDEBusOps ide_bus_ops = {
.transfer_start_fn = pata_transfer_start,
.irq_set_fn = pata_set_irq,
.dma_start_fn = pata_dma_start,
+ .dma_prepare_fn = dma_buf_prepare,
+ .dma_rw_fn = dma_buf_rw,
};
void ide_init2(IDEBus *bus, qemu_irq irq)
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index ee7e13e..4bee636 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -374,6 +374,8 @@ typedef void TransferStartFunc(IDEState *,
EndTransferFunc *);
typedef void IRQSetFunc(IDEBus *);
typedef void DMAStartFunc(IDEState *, BlockDriverCompletionFunc *);
+typedef int DMAPrepareFunc(BMDMAState *, int);
+typedef int DMARWFunc(BMDMAState *, int);
/* NOTE: IDEState represents in fact one drive */
struct IDEState {
@@ -457,6 +459,8 @@ struct IDEBusOps {
TransferStartFunc *transfer_start_fn;
IRQSetFunc *irq_set_fn;
DMAStartFunc *dma_start_fn;
+ DMAPrepareFunc *dma_prepare_fn;
+ DMARWFunc *dma_rw_fn;
};
struct IDEBus {
--
1.6.0.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH 05/10] ide: add ncq identify data for ahci sata drives
2010-11-18 3:27 [Qemu-devel] [PATCH 00/10] AHCI emulation support v2 Alexander Graf
` (3 preceding siblings ...)
2010-11-18 3:27 ` [Qemu-devel] [PATCH 04/10] ide: add DMA hooks to " Alexander Graf
@ 2010-11-18 3:27 ` Alexander Graf
2010-11-18 3:27 ` [Qemu-devel] [PATCH 06/10] pci: add storage class for sata Alexander Graf
` (6 subsequent siblings)
11 siblings, 0 replies; 30+ messages in thread
From: Alexander Graf @ 2010-11-18 3:27 UTC (permalink / raw)
To: QEMU-devel Developers
Cc: Joerg Roedel, Gerd Hoffmann, Stefan Hajnoczi, tj, Roland Elek,
Sebastian Herbszt
From: Roland Elek <elek.roland@gmail.com>
I modified ide_identify() to include the zero-based queue length
value in word 75, and set bit 8 in word 76 to signal NCQ support
in the identify data for AHCI SATA drives.
Signed-off-by: Roland Elek <elek.roland@gmail.com>
---
hw/ide/core.c | 7 +++++++
hw/ide/internal.h | 2 ++
2 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 04190d2..073c038 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -147,6 +147,13 @@ static void ide_identify(IDEState *s)
put_le16(p + 66, 120);
put_le16(p + 67, 120);
put_le16(p + 68, 120);
+
+ if (s->ncq_queues) {
+ put_le16(p + 75, s->ncq_queues - 1);
+ /* NCQ supported */
+ put_le16(p + 76, (1 << 8));
+ }
+
put_le16(p + 80, 0xf0); /* ata3 -> ata6 supported */
put_le16(p + 81, 0x16); /* conforms to ata5 */
/* 14=NOP supported, 5=WCACHE supported, 0=SMART supported */
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 4bee636..1261eea 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -451,6 +451,8 @@ struct IDEState {
int smart_errors;
uint8_t smart_selftest_count;
uint8_t *smart_selftest_data;
+ /* AHCI */
+ int ncq_queues;
};
/* This struct represents a device that uses an IDE bus, but requires
--
1.6.0.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH 06/10] pci: add storage class for sata
2010-11-18 3:27 [Qemu-devel] [PATCH 00/10] AHCI emulation support v2 Alexander Graf
` (4 preceding siblings ...)
2010-11-18 3:27 ` [Qemu-devel] [PATCH 05/10] ide: add ncq identify data for ahci sata drives Alexander Graf
@ 2010-11-18 3:27 ` Alexander Graf
2010-11-18 3:27 ` [Qemu-devel] [PATCH 07/10] pci: add ich7 pci id Alexander Graf
` (5 subsequent siblings)
11 siblings, 0 replies; 30+ messages in thread
From: Alexander Graf @ 2010-11-18 3:27 UTC (permalink / raw)
To: QEMU-devel Developers
Cc: Joerg Roedel, Gerd Hoffmann, Stefan Hajnoczi, tj, Roland Elek,
Sebastian Herbszt
This patch adds the storage sata class id.
Signed-off-by: Alexander Graf <agraf@suse.de>
---
hw/pci_ids.h | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/hw/pci_ids.h b/hw/pci_ids.h
index 82cba7e..ea3418c 100644
--- a/hw/pci_ids.h
+++ b/hw/pci_ids.h
@@ -15,6 +15,7 @@
#define PCI_CLASS_STORAGE_SCSI 0x0100
#define PCI_CLASS_STORAGE_IDE 0x0101
+#define PCI_CLASS_STORAGE_SATA 0x0106
#define PCI_CLASS_STORAGE_OTHER 0x0180
#define PCI_CLASS_NETWORK_ETHERNET 0x0200
--
1.6.0.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH 07/10] pci: add ich7 pci id
2010-11-18 3:27 [Qemu-devel] [PATCH 00/10] AHCI emulation support v2 Alexander Graf
` (5 preceding siblings ...)
2010-11-18 3:27 ` [Qemu-devel] [PATCH 06/10] pci: add storage class for sata Alexander Graf
@ 2010-11-18 3:27 ` Alexander Graf
2010-11-18 3:27 ` [Qemu-devel] [PATCH 08/10] ahci: add ahci emulation Alexander Graf
` (4 subsequent siblings)
11 siblings, 0 replies; 30+ messages in thread
From: Alexander Graf @ 2010-11-18 3:27 UTC (permalink / raw)
To: QEMU-devel Developers
Cc: Joerg Roedel, Gerd Hoffmann, Stefan Hajnoczi, tj, Roland Elek,
Sebastian Herbszt
We need a PCI ID for our new AHCI adapter. I just picked an ICH-7M
because that's the one built into the first Macbooks.
This patch adds a PCI ID define for an ICH-7M AHCI adapter.
Signed-off-by: Alexander Graf <agraf@suse.de>
---
hw/pci.h | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/hw/pci.h b/hw/pci.h
index 7100804..be0f9c2 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -62,6 +62,7 @@
/* Intel (0x8086) */
#define PCI_DEVICE_ID_INTEL_82551IT 0x1209
#define PCI_DEVICE_ID_INTEL_82557 0x1229
+#define PCI_DEVICE_ID_INTEL_ICH7M_AHCI 0x27c5
/* Red Hat / Qumranet (for QEMU) -- see pci-ids.txt */
#define PCI_VENDOR_ID_REDHAT_QUMRANET 0x1af4
--
1.6.0.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH 08/10] ahci: add ahci emulation
2010-11-18 3:27 [Qemu-devel] [PATCH 00/10] AHCI emulation support v2 Alexander Graf
` (6 preceding siblings ...)
2010-11-18 3:27 ` [Qemu-devel] [PATCH 07/10] pci: add ich7 pci id Alexander Graf
@ 2010-11-18 3:27 ` Alexander Graf
2010-11-18 8:01 ` [Qemu-devel] " Gerd Hoffmann
2010-11-18 3:27 ` [Qemu-devel] [PATCH 09/10] ahci: add -drive support Alexander Graf
` (3 subsequent siblings)
11 siblings, 1 reply; 30+ messages in thread
From: Alexander Graf @ 2010-11-18 3:27 UTC (permalink / raw)
To: QEMU-devel Developers
Cc: Joerg Roedel, Gerd Hoffmann, Stefan Hajnoczi, tj, Roland Elek,
Sebastian Herbszt
This patch adds an emulation layer for an ICH-7M AHCI controller. For now
this controller does not do IDE legacy emulation. It is a pure AHCI controller.
Signed-off-by: Alexander Graf <agraf@suse.de>
---
v1 -> v2:
- rename IDEExtender to IDEBusOps and make a pointer (kraxel)
- make dma hooks explicit by putting them into ops struct (stefanha)
- use qdev buses (kraxel)
- minor cleanups
- dprintf overhaul
- add reset function
---
Makefile.objs | 1 +
Makefile.target | 4 +
default-configs/i386-softmmu.mak | 1 +
default-configs/x86_64-softmmu.mak | 1 +
hw/ahci.c | 1280 ++++++++++++++++++++++++++++++++++++
5 files changed, 1287 insertions(+), 0 deletions(-)
create mode 100644 hw/ahci.c
diff --git a/Makefile.objs b/Makefile.objs
index 15569af..8dfcefa 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -229,6 +229,7 @@ hw-obj-$(CONFIG_IDE_PIIX) += ide/piix.o
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) += ahci.o
# SCSI layer
hw-obj-y += lsi53c895a.o
diff --git a/Makefile.target b/Makefile.target
index 91e6e74..bd899cd 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -195,6 +195,10 @@ obj-y += e1000.o
# Inter-VM PCI shared memory
obj-$(CONFIG_KVM) += ivshmem.o
+# ahci
+#
+obj-$(CONFIG_AHCI) += ahci.o
+
# Hardware support
obj-i386-y += vga.o
obj-i386-y += mc146818rtc.o i8259.o pc.o
diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index ed00471..66b92af 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -19,6 +19,7 @@ CONFIG_IDE_QDEV=y
CONFIG_IDE_PCI=y
CONFIG_IDE_ISA=y
CONFIG_IDE_PIIX=y
+CONFIG_AHCI=y
CONFIG_NE2000_ISA=y
CONFIG_PIIX_PCI=y
CONFIG_SOUND=y
diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
index 5183203..508e843 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -19,6 +19,7 @@ CONFIG_IDE_QDEV=y
CONFIG_IDE_PCI=y
CONFIG_IDE_ISA=y
CONFIG_IDE_PIIX=y
+CONFIG_AHCI=y
CONFIG_NE2000_ISA=y
CONFIG_PIIX_PCI=y
CONFIG_SOUND=y
diff --git a/hw/ahci.c b/hw/ahci.c
new file mode 100644
index 0000000..0ee021e
--- /dev/null
+++ b/hw/ahci.c
@@ -0,0 +1,1280 @@
+/*
+ * 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/>.
+ *
+ */
+#include "hw.h"
+#include "monitor.h"
+#include "pci.h"
+#include "dma.h"
+#include "cpu-common.h"
+#include "scsi-defs.h"
+#include "scsi.h"
+#include "blockdev.h"
+#include <hw/ide/internal.h>
+#include <hw/ide/pci.h>
+
+/* #define DEBUG_AHCI */
+
+#ifdef DEBUG_AHCI
+#define DPRINTF(port, fmt, ...) \
+do { fprintf(stderr, "ahci: %s: [%d] ", __FUNCTION__, port); \
+ fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
+#else
+#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;
+} __attribute__ ((packed)) 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;
+} __attribute__ ((packed)) 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 {
+ uint32_t addr;
+ uint32_t addr_hi;
+ uint32_t reserved;
+ uint32_t flags_size;
+} __attribute__ ((packed)) AHCI_SG;
+
+typedef struct AHCIDevice AHCIDevice;
+
+struct AHCIDevice {
+ IDEBus port;
+ BMDMAState bmdma;
+ int port_no;
+ uint32_t port_state;
+ uint32_t finished;
+ AHCIPortRegs port_regs;
+ struct AHCIState *hba;
+ uint8_t *lst;
+ uint8_t *res_fis;
+ uint8_t *cmd_fis;
+ int cmd_fis_len;
+ AHCICmdHdr *cur_cmd;
+};
+
+typedef struct NCQTransferState {
+ AHCIDevice *drive;
+ QEMUSGList sglist;
+ int is_read;
+ uint16_t sector_count;
+ uint64_t lba;
+ uint8_t tag;
+ int slot;
+ int used;
+} NCQTransferState;
+
+typedef struct AHCIState{
+ AHCIDevice dev[SATA_PORTS];
+ AHCIControlRegs control_regs;
+ int mem;
+ qemu_irq irq;
+ NCQTransferState ncq_tfs[AHCI_MAX_CMDS];
+} AHCIState;
+
+typedef struct AHCIPciState {
+ PCIDevice card;
+ AHCIState ahci;
+} AHCIPciState;
+
+typedef struct H2D_NCQ_FIS {
+ 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)) H2D_NCQ_FIS;
+
+static void ahci_irq_set_fn(IDEBus *s);
+
+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(AHCIState *s, int port, uint8_t *cmd_fis);
+
+static uint32_t ahci_port_read(AHCIState *s, int port, int offset)
+{
+ uint32_t val;
+ uint32_t *p;
+ AHCIPortRegs *pr;
+ pr = &s->dev[port].port_regs;
+
+ switch (offset) {
+ case PORT_SCR_STAT:
+ if (s->dev[port].port.ifs[0].bs) {
+ val = SATA_SCR_SSTATUS_DET_DEV_PRESENT_PHY_UP |
+ SATA_SCR_SSTATUS_SPD_GEN1 | SATA_SCR_SSTATUS_IPM_ACTIVE;
+ } else {
+ val = SATA_SCR_SSTATUS_DET_NODEV;
+ }
+ break;
+ case PORT_IRQ_STAT:
+ val = pr->irq_stat;
+ break;
+ case PORT_CMD_ISSUE:
+ val = 0;
+ break;
+ case PORT_SCR_ACT:
+ pr->scr_act &= ~s->dev[port].finished;
+ s->dev[port].finished = 0;
+ val = pr->scr_act;
+ break;
+ case PORT_TFDATA:
+ case PORT_SIG:
+ case PORT_SCR_CTL:
+ case PORT_SCR_ERR:
+ default:
+ p = (uint32_t *)&s->dev[port].port_regs;
+ val = p[offset / sizeof(*p)];
+ break;
+ }
+ DPRINTF(port, "offset: 0x%x val: 0x%x\n", offset, val);
+ return val;
+
+}
+
+static void ahci_trigger_irq(AHCIState *s, AHCIDevice *d,
+ int irq_type)
+{
+ DPRINTF(d->port_no, "trigger irq %#x -> %x\n",
+ irq_type, d->port_regs.irq_mask & irq_type);
+
+ d->port_regs.irq_stat |= irq_type;
+
+ /* Only trigger an interrupt if unmasked */
+ if (d->port_regs.irq_mask & irq_type) {
+ s->control_regs.irqstatus |= (1 << d->port_no);
+ if (s->control_regs.ghc & HOST_CTL_IRQ_EN) {
+ qemu_irq_raise(s->irq);
+ }
+ }
+}
+
+static void ahci_check_irq(AHCIState *s)
+{
+ DPRINTF(-1, "check irq %#x\n", s->control_regs.irqstatus);
+
+ if (s->control_regs.irqstatus &&
+ (s->control_regs.ghc & HOST_CTL_IRQ_EN)) {
+ qemu_irq_raise(s->irq);
+ }
+}
+
+static void map_page(uint8_t **ptr, uint64_t addr)
+{
+ target_phys_addr_t len = 4096;
+
+ if (*ptr) {
+ cpu_physical_memory_unmap(*ptr, 1, len, len);
+ }
+
+ *ptr = cpu_physical_memory_map(addr, &len, 1);
+ if (len < 4096) {
+ *ptr = NULL;
+ }
+}
+
+static void ahci_port_write(AHCIState *s, int port, int offset, uint32_t val)
+{
+ AHCIPortRegs *pr = &s->dev[port].port_regs;
+ uint32_t *p;
+
+ DPRINTF(port, "offset: 0x%x val: 0x%x\n", offset, val);
+ switch (offset) {
+ case PORT_LST_ADDR:
+ pr->lst_addr = val;
+ map_page(&s->dev[port].lst,
+ ((uint64_t)pr->lst_addr_hi << 32) | pr->lst_addr);
+ break;
+ case PORT_LST_ADDR_HI:
+ pr->lst_addr_hi = val;
+ map_page(&s->dev[port].lst,
+ ((uint64_t)pr->lst_addr_hi << 32) | pr->lst_addr);
+ break;
+ case PORT_FIS_ADDR:
+ pr->fis_addr = val;
+ map_page(&s->dev[port].res_fis,
+ ((uint64_t)pr->fis_addr_hi << 32) | pr->fis_addr);
+ break;
+ case PORT_FIS_ADDR_HI:
+ pr->fis_addr_hi = val;
+ map_page(&s->dev[port].res_fis,
+ ((uint64_t)pr->fis_addr_hi << 32) | pr->fis_addr);
+ break;
+ case PORT_IRQ_STAT:
+ pr->irq_stat &= ~val;
+ break;
+ case PORT_IRQ_MASK:
+ pr->irq_mask = val & 0xfdc000ff;
+ break;
+ case PORT_CMD:
+ pr->cmd = val & ~(PORT_CMD_LIST_ON | PORT_CMD_FIS_ON);
+
+ if (pr->cmd & PORT_CMD_START) {
+ pr->cmd |= PORT_CMD_LIST_ON;
+ }
+
+ if (pr->cmd & PORT_CMD_FIS_RX) {
+ pr->cmd |= PORT_CMD_FIS_ON;
+ }
+
+ check_cmd(s, port);
+ break;
+ case PORT_CMD_ISSUE:
+ pr->cmd_issue |= val;
+ check_cmd(s, port);
+ break;
+ case PORT_SCR_ERR:
+ pr->scr_err &= ~val;
+ break;
+ case PORT_SCR_ACT:
+ /* RW1 */
+ pr->scr_act |= val;
+ break;
+ case PORT_SCR_CTL:
+ if (((pr->scr_ctl & AHCI_SCR_SCTL_DET) == 1) &&
+ ((val & AHCI_SCR_SCTL_DET) == 0)) {
+ ahci_reset_port(s, port);
+ }
+ pr->scr_ctl = val;
+ break;
+ case PORT_TFDATA:
+ case PORT_SIG:
+ case PORT_SCR_STAT:
+ default:
+ p = (uint32_t *)pr;
+ p[offset / sizeof(*p)] = val;
+ break;
+ }
+
+}
+
+static uint32_t ahci_mem_readl(void *ptr, target_phys_addr_t addr)
+{
+ AHCIState *s = ptr;
+ uint32_t val;
+ uint32_t *p;
+ addr = addr & 0xfff;
+ if (addr < AHCI_GENERIC_HOST_CONTROL_REGS_MAX_ADDR) {
+ switch (addr) {
+ case HOST_IRQ_STAT:
+ default:
+ /* genernal host control */
+ p = (uint32_t *)&s->control_regs;
+ val = p[addr / sizeof(*p)];
+ }
+ } else if((addr >= AHCI_PORT_REGS_START_ADDR) &&
+ (addr < AHCI_PORT_REGS_END_ADDR)) {
+ val = ahci_port_read(s, (addr - AHCI_PORT_REGS_START_ADDR) >> 7,
+ addr & AHCI_PORT_ADDR_OFFSET_MASK);
+ } else {
+ val = 0;
+ }
+
+ DPRINTF(-1, "(addr 0x%08X), val 0x%08X\n", (unsigned) addr, val);
+
+ return val;
+}
+
+
+
+static void ahci_mem_writel(void *ptr, target_phys_addr_t addr, uint32_t val)
+{
+ AHCIState *s = ptr;
+ addr = addr & 0xfff;
+ int i;
+
+ /* Only aligned reads are allowed on AHCI */
+ if (addr & 3) {
+ fprintf(stderr, "ahci: Mis-aligned write to addr 0x"
+ TARGET_FMT_plx "\n", addr);
+ return;
+ }
+
+ if (addr < AHCI_GENERIC_HOST_CONTROL_REGS_MAX_ADDR) {
+ switch (addr) {
+ case HOST_CAP: /* R/WO, RO */
+ /* FIXME handle R/WO */
+ break;
+ case HOST_CTL: /* R/W */
+ if (val & HOST_CTL_RESET) {
+ DPRINTF(-1, "HBA Reset\n");
+ /* FIXME reset? */
+ } else {
+ s->control_regs.ghc = (val & 0x3) | HOST_CTL_AHCI_EN;
+ ahci_check_irq(s);
+ }
+ break;
+ case HOST_IRQ_STAT: /* R/WC, RO */
+ s->control_regs.irqstatus &= ~val;
+ for (i = 0; i < SATA_PORTS; i++) {
+ if (s->dev[i].port_regs.irq_stat) {
+ s->control_regs.irqstatus |= (1 << i);
+ qemu_irq_lower(s->irq);
+ qemu_irq_raise(s->irq);
+ }
+ }
+ if (!s->control_regs.irqstatus) {
+ qemu_irq_lower(s->irq);
+ }
+ break;
+ case HOST_PORTS_IMPL: /* R/WO, RO */
+ /* FIXME handle R/WO */
+ break;
+ case HOST_VERSION: /* RO */
+ /* FIXME report write? */
+ break;
+ default:
+ 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)) {
+ ahci_port_write(s, (addr - AHCI_PORT_REGS_START_ADDR) >> 7,
+ addr & AHCI_PORT_ADDR_OFFSET_MASK, val);
+ }
+
+ DPRINTF(-1, "(addr 0x%08X), val 0x%08X\n", (unsigned) addr, val);
+
+}
+
+static CPUReadMemoryFunc *ahci_readfn[3]={
+ ahci_mem_readl,
+ ahci_mem_readl,
+ ahci_mem_readl
+};
+
+static CPUWriteMemoryFunc *ahci_writefn[3]={
+ ahci_mem_writel,
+ ahci_mem_writel,
+ ahci_mem_writel
+};
+
+static void ahci_reg_init(AHCIState *s)
+{
+ int i;
+
+ s->control_regs.cap = (SATA_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.version = AHCI_VERSION_1_0;
+
+ for (i = 0; i < SATA_PORTS; i++) {
+ s->dev[i].port_state = STATE_RUN;
+ }
+}
+
+#ifndef min
+#define min(a, b) ((a) < (b) ? (a) : (b))
+#endif
+
+static uint32_t write_to_sglist(uint8_t *buffer, uint32_t len,
+ QEMUSGList *sglist)
+{
+ uint32_t i = 0;
+ uint32_t total = 0, once;
+ ScatterGatherEntry *cur_prd;
+ uint32_t sgcount;
+
+ cur_prd = sglist->sg;
+ sgcount = sglist->nsg;
+ for (i = 0; len && sgcount; i++) {
+ once = min(cur_prd->len + 1, len);
+ cpu_physical_memory_write(cur_prd->base, buffer, once);
+ cur_prd++;
+ sgcount--;
+ len -= once;
+ buffer += once;
+ total += once;
+ }
+
+ return total;
+}
+
+static void check_cmd(AHCIState *s, int port)
+{
+ AHCIPortRegs *pr = &s->dev[port].port_regs;
+ int slot;
+
+ if (pr->cmd & PORT_CMD_START) {
+ for (slot = 0; (slot < 32) && pr->cmd_issue; slot++) {
+ if ((pr->cmd_issue & (1 << slot)) &&
+ !handle_cmd(s, port, slot)) {
+ pr->cmd_issue &= ~(1 << slot);
+ }
+ }
+ }
+}
+
+static void ahci_reset_port(AHCIState *s, int port)
+{
+ IDEState *ide_state;
+ uint8_t init_fis[0x20];
+ uint32_t tfd;
+
+ DPRINTF(port, "reset port\n");
+
+ ide_state = &s->dev[port].port.ifs[0];
+ if (!ide_state->bs) {
+ return;
+ }
+
+ 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;
+ tfd = (1 << 8) | SEEK_STAT | WRERR_STAT;
+ } else if (ide_state->drive_kind == IDE_CD) {
+ s->dev[port].port_regs.sig = SATA_SIGNATURE_CDROM;
+ 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;
+ tfd = (1 << 8) | SEEK_STAT | WRERR_STAT | READY_STAT;
+ } else {
+ s->dev[port].port_regs.sig = SATA_SIGNATURE_DISK;
+ tfd = (1 << 8) | SEEK_STAT | WRERR_STAT;
+ }
+
+ ide_state->error = 1;
+ ide_state->status = 0;
+ init_fis[4] = 1;
+ init_fis[12] = 1;
+ ahci_write_fis_d2h(s, port, init_fis);
+
+ s->dev[port].port_regs.tfdata = tfd;
+}
+
+static void debug_print_fis(uint8_t *fis, int cmd_len)
+{
+#ifdef DEBUG_AHCI
+ int i;
+
+ fprintf(stderr, "fis:");
+ for (i = 0; i < cmd_len; i++) {
+ if ((i & 0xf) == 0) {
+ fprintf(stderr, "\n%02x:",i);
+ }
+ fprintf(stderr, "%02x ",fis[i]);
+ }
+ fprintf(stderr, "\n");
+#endif
+}
+
+static void ahci_write_fis_sdb(AHCIState *s, int port, uint32_t finished)
+{
+ AHCIPortRegs *pr = &s->dev[port].port_regs;
+ IDEState *ide_state;
+ uint8_t *sdb_fis;
+
+ if (!s->dev[port].res_fis ||
+ !(pr->cmd & PORT_CMD_FIS_RX)) {
+ return;
+ }
+
+ sdb_fis = &s->dev[port].res_fis[RES_FIS_SDBFIS];
+ ide_state = &s->dev[port].port.ifs[0];
+
+ pr->tfdata = (uint16_t)ide_state->error << 8 | ide_state->status;
+
+ /* clear memory */
+ *(uint32_t*)sdb_fis = 0;
+
+ /* write values */
+ sdb_fis[0] = ide_state->error;
+ sdb_fis[2] = ide_state->status & 0x77;
+ s->dev[port].finished |= finished;
+ *(uint32_t*)(sdb_fis + 4) = cpu_to_le32(s->dev[port].finished);
+
+ ahci_trigger_irq(s, &s->dev[port], PORT_IRQ_STAT_SDBS);
+}
+
+static void ahci_write_fis_d2h(AHCIState *s, int port, uint8_t *cmd_fis)
+{
+ AHCIPortRegs *pr = &s->dev[port].port_regs;
+ uint8_t *d2h_fis;
+ int i;
+
+ if (!s->dev[port].res_fis ||
+ !(pr->cmd & PORT_CMD_FIS_RX)) {
+ return;
+ }
+
+ d2h_fis = &s->dev[port].res_fis[RES_FIS_RFIS];
+
+ d2h_fis[0] = 0x34;
+ d2h_fis[1] = (s->control_regs.irqstatus ? (1 << 6) : 0);
+ d2h_fis[2] = s->dev[port].port.ifs[0].status;
+ d2h_fis[3] = s->dev[port].port.ifs[0].error;
+
+ d2h_fis[4] = cmd_fis[4];
+ d2h_fis[5] = cmd_fis[5];
+ d2h_fis[6] = cmd_fis[6];
+ d2h_fis[7] = cmd_fis[7];
+ d2h_fis[8] = cmd_fis[8];
+ d2h_fis[9] = cmd_fis[9];
+ d2h_fis[10] = cmd_fis[10];
+ d2h_fis[11] = cmd_fis[11];
+ d2h_fis[12] = cmd_fis[12];
+ d2h_fis[13] = cmd_fis[13];
+ for (i = 14; i < 0x20; i++) {
+ d2h_fis[i] = 0;
+ }
+
+ pr->tfdata = (uint16_t)d2h_fis[3] << 8 | d2h_fis[2];
+
+ if (d2h_fis[2] & ERR_STAT) {
+ ahci_trigger_irq(s, &s->dev[port], PORT_IRQ_STAT_TFES);
+ }
+
+ ahci_trigger_irq(s, &s->dev[port], PORT_IRQ_D2H_REG_FIS);
+}
+
+static void ncq_cb(void *opaque, int ret)
+{
+ NCQTransferState *ncq_tfs = (NCQTransferState *)opaque;
+ IDEState *ide_state;
+
+ if (ret < 0) {
+ /* error */
+ }
+
+ /* Clear bit for this tag in SActive */
+ ncq_tfs->drive->port_regs.scr_act &= ~(1 << ncq_tfs->tag);
+
+ ide_state = &ncq_tfs->drive->port.ifs[0];
+ ide_state->status = READY_STAT | SEEK_STAT;
+
+ /* XXX do we send a d2h fis here? */
+ ahci_write_fis_d2h(ncq_tfs->drive->hba, ncq_tfs->drive->port_no,
+ ncq_tfs->drive->cmd_fis);
+
+ ahci_write_fis_sdb(ncq_tfs->drive->hba, ncq_tfs->drive->port_no,
+ (1 << ncq_tfs->tag));
+
+ DPRINTF(ncq_tfs->drive->port_no, "NCQ transfer tag %d finished\n",
+ ncq_tfs->tag);
+
+ qemu_sglist_destroy(&ncq_tfs->sglist);
+ cpu_physical_memory_unmap(ncq_tfs->drive->cmd_fis, 1,
+ ncq_tfs->drive->cmd_fis_len,
+ ncq_tfs->drive->cmd_fis_len);
+
+ ncq_tfs->used = 0;
+}
+
+static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis,
+ int slot, QEMUSGList *sg)
+{
+ H2D_NCQ_FIS *ncq_fis = (H2D_NCQ_FIS*)cmd_fis;
+ uint8_t tag = ncq_fis->tag >> 3;
+ NCQTransferState *ncq_tfs = &s->ncq_tfs[tag];
+
+ if (ncq_tfs->used) {
+ /* error - already in use */
+ fprintf(stderr, "%s: tag %d already used\n", __FUNCTION__, tag);
+ return;
+ }
+
+ ncq_tfs->used = 1;
+ ncq_tfs->drive = &s->dev[port];
+ ncq_tfs->drive->cmd_fis = cmd_fis;
+ ncq_tfs->drive->cmd_fis_len = 0x20;
+ ncq_tfs->slot = slot;
+ ncq_tfs->lba = ((uint64_t)ncq_fis->lba5 << 40) |
+ ((uint64_t)ncq_fis->lba4 << 32) |
+ ((uint64_t)ncq_fis->lba3 << 24) |
+ ((uint64_t)ncq_fis->lba2 << 16) |
+ ((uint64_t)ncq_fis->lba1 << 8) |
+ (uint64_t)ncq_fis->lba0;
+
+ /* Note: We calculate the sector count, but don't currently rely on it.
+ * The total size of the DMA buffer tells us the transfer size instead. */
+ ncq_tfs->sector_count = ((uint16_t)ncq_fis->sector_count_high << 8) |
+ ncq_fis->sector_count_low;
+
+ DPRINTF(port, "NCQ transfer LBA from %ld to %ld, drive max %ld\n",
+ ncq_tfs->lba, ncq_tfs->lba + ncq_tfs->sector_count - 2,
+ s->dev[port].port.ifs[0].nb_sectors - 1);
+
+ ncq_tfs->sglist = *sg;
+ ncq_tfs->tag = tag;
+
+ switch(ncq_fis->command) {
+ case READ_FPDMA_QUEUED:
+ DPRINTF(port, "NCQ reading %d sectors from LBA %ld, tag %d\n",
+ ncq_tfs->sector_count-1, ncq_tfs->lba, ncq_tfs->tag);
+ ncq_tfs->is_read = 1;
+
+ /* XXX: The specification is unclear about whether the DMA Setup
+ * FIS here should have the I bit set, but it suggest that it should
+ * not. Linux works without this interrupt, so I disabled it.
+ * If someone knows if it is needed, please tell me, or fix this. */
+
+ /* ahci_trigger_irq(s,s->dev[port],PORT_IRQ_STAT_DSS); */
+ DPRINTF(port, "tag %d aio read %ld\n", ncq_tfs->tag, ncq_tfs->lba);
+ dma_bdrv_read(ncq_tfs->drive->port.ifs[0].bs, &ncq_tfs->sglist,
+ ncq_tfs->lba, ncq_cb, ncq_tfs);
+ break;
+ case WRITE_FPDMA_QUEUED:
+ DPRINTF(port, "NCQ writing %d sectors to LBA %ld, tag %d\n",
+ ncq_tfs->sector_count-1, ncq_tfs->lba, ncq_tfs->tag);
+ ncq_tfs->is_read = 0;
+ /* ahci_trigger_irq(s,s->dev[port],PORT_IRQ_STAT_DSS); */
+ DPRINTF(port, "tag %d aio write %ld\n", ncq_tfs->tag, ncq_tfs->lba);
+ dma_bdrv_write(ncq_tfs->drive->port.ifs[0].bs, &ncq_tfs->sglist,
+ ncq_tfs->lba, ncq_cb, ncq_tfs);
+ break;
+ default:
+ hw_error("ahci: tried to process non-NCQ command as NCQ\n");
+ break;
+ }
+}
+
+static int handle_cmd(AHCIState *s, int port, int slot)
+{
+ IDEState *ide_state;
+
+ int sglist_alloc_hint;
+ QEMUSGList sglist;
+ int atapi_packet_len = 0;
+ AHCIPortRegs *pr;
+ uint32_t opts;
+ uint64_t tbl_addr;
+ AHCICmdHdr *cmd;
+ uint8_t *cmd_fis;
+
+ target_phys_addr_t cmd_len;
+ int i;
+
+ pr = &s->dev[port].port_regs;
+ cmd = (AHCICmdHdr *)&s->dev[port].lst[slot * 32];
+
+ if (!s->dev[port].lst) {
+ hw_error("%s: lst not given but cmd handled", __FUNCTION__);
+ }
+
+ opts = le32_to_cpu(cmd->opts);
+ tbl_addr = le64_to_cpu(cmd->tbl_addr);
+
+ cmd_len = (opts & AHCI_CMD_HDR_CMD_FIS_LEN) * 4;
+ cmd_fis = cpu_physical_memory_map(tbl_addr, &cmd_len, 1);
+
+ /* The device we are working for */
+ ide_state = &s->dev[port].port.ifs[0];
+
+ if (!ide_state->bs) {
+ hw_error("%s: guest accessed unused port", __FUNCTION__);
+ }
+
+ /* Get number of entries in the PRDT, init a qemu sglist accordingly */
+ sglist_alloc_hint = opts >> AHCI_CMD_HDR_PRDT_LEN;
+ memset(&sglist, 0, sizeof(sglist));
+
+ if (sglist_alloc_hint > 0) {
+ qemu_sglist_init(&sglist, sglist_alloc_hint);
+ /* Parse the PRDs and create qemu sglist entries accordingly */
+ for (i = 0; i < sglist_alloc_hint; i++) {
+ target_phys_addr_t cur_prd_addr;
+
+ cur_prd_addr = tbl_addr + 0x80 + i * sizeof(AHCI_SG);
+ /* flags_size is zero-based */
+ qemu_sglist_add(&sglist,
+ ldl_phys(cur_prd_addr + offsetof(AHCI_SG, addr)),
+ ldl_phys(cur_prd_addr + offsetof(AHCI_SG, flags_size)) + 1);
+ }
+ }
+
+ debug_print_fis(cmd_fis, cmd_len);
+
+ switch (cmd_fis[0]) {
+ case SATA_FIS_TYPE_REGISTER_H2D:
+ break;
+ default:
+ hw_error("unknown command cmd_fis[0]=%02x cmd_fis[1]=%02x cmd_fis[2]=%02x\n",
+ cmd_fis[0], cmd_fis[1], cmd_fis[2]);
+ break;
+ }
+
+ switch (cmd_fis[1]) {
+ case SATA_FIS_REG_H2D_UPDATE_COMMAND_REGISTER:
+ break;
+ case 0:
+ break;
+ default:
+ hw_error("unknown command cmd_fis[0]=%02x cmd_fis[1]=%02x cmd_fis[2]=%02x\n",
+ cmd_fis[0], cmd_fis[1], cmd_fis[2]);
+ break;
+ }
+
+ switch (s->dev[port].port_state) {
+ case STATE_RUN:
+ if (cmd_fis[15] & ATA_SRST) {
+ s->dev[port].port_state = STATE_RESET;
+ }
+ break;
+ case STATE_RESET:
+ if (!(cmd_fis[15] & ATA_SRST)) {
+ ahci_reset_port(s, port);
+ }
+ break;
+ }
+
+ if (cmd_fis[1] == SATA_FIS_REG_H2D_UPDATE_COMMAND_REGISTER) {
+
+ /* Check for NCQ command */
+ if ((cmd_fis[2] == READ_FPDMA_QUEUED) ||
+ (cmd_fis[2] == WRITE_FPDMA_QUEUED)) {
+ process_ncq_command(s, port, cmd_fis, slot, &sglist);
+ goto out;
+ }
+
+ /* If the command is not NCQ, the sglist is needed in the core */
+ ide_state->sg = sglist;
+
+ /* Decompose the FIS */
+ ide_state->nsector = (int64_t)((cmd_fis[13] << 8) | cmd_fis[12]);
+ if (!ide_state->nsector) {
+ ide_state->nsector = 256;
+ }
+
+ if (ide_state->drive_kind != IDE_CD) {
+ ide_set_sector(ide_state, (cmd_fis[6] << 16) | (cmd_fis[5] << 8) |
+ cmd_fis[4]);
+ }
+
+ /* Copy the ACMD field (ATAPI packet, if any) from the AHCI command
+ * table to ide_state->io_buffer
+ */
+ if (opts & AHCI_CMD_ATAPI) {
+ atapi_packet_len = ((ide_state->hcyl) << 8) + ide_state->lcyl;
+ cpu_physical_memory_read(tbl_addr + AHCI_COMMAND_TABLE_ACMD,
+ ide_state->io_buffer, 0x10);
+ }
+
+ ide_state->error = 0;
+ s->dev[port].cur_cmd = cmd;
+
+ /* We're ready to process the command in FIS byte 2. */
+ ide_exec_cmd(&s->dev[port].port, cmd_fis[2]);
+
+ /* we're DMA'ing, so we're not ready yet, postpone cleanup to later */
+ if (s->dev[port].bmdma.status & BM_STATUS_DMAING) {
+ cmd->status = 0;
+ s->dev[port].cmd_fis = cmd_fis;
+ s->dev[port].cmd_fis_len = cmd_len;
+ return 0;
+ }
+
+ ahci_write_fis_d2h(s, port, cmd_fis);
+ }
+
+out:
+ cpu_physical_memory_unmap(cmd_fis, 1, cmd_len, cmd_len);
+
+ return 0;
+}
+
+static void ahci_transfer_start(IDEState *s, uint8_t *buf, int size,
+ EndTransferFunc *end_transfer_func)
+{
+ AHCIDevice *ad;
+ AHCIState *as;
+
+ s->end_transfer_func = end_transfer_func;
+
+ ad = DO_UPCAST(AHCIDevice, port, s->bus);
+ as = ad->hba;
+
+ write_to_sglist(buf, (uint32_t)size, &s->sg);
+
+ /* update number of transferred bytes */
+ ad->cur_cmd->status = cpu_to_le32(le32_to_cpu(ad->cur_cmd->status) + size);
+
+ end_transfer_func(s);
+}
+
+static void ahci_dma_start_fn(IDEState *s, BlockDriverCompletionFunc *dma_cb)
+{
+ AHCIDevice *ad;
+ AHCIState *as;
+ BMDMAState *bm = s->bus->bmdma;
+
+ ad = DO_UPCAST(AHCIDevice, port, s->bus);
+ as = ad->hba;
+
+ if (!bm) {
+ return;
+ }
+
+ bm->unit = s->unit;
+ bm->dma_cb = dma_cb;
+ bm->cur_prd_last = 0;
+ bm->cur_prd_addr = 0;
+ bm->cur_prd_len = 0;
+ bm->cur_addr = 0;
+ bm->sector_num = ide_get_sector(s);
+ bm->nsector = s->nsector;
+ bmdma_cmd_writeb(bm, 0, 1);
+}
+
+static int ahci_dma_buf_prepare(BMDMAState *bm, int is_write)
+{
+ IDEState *s = bmdma_active_if(bm);
+ int i;
+
+ s->io_buffer_size = 0;
+ for (i = 0; i < s->sg.nsg; i++) {
+ s->io_buffer_size += s->sg.sg[i].len;
+ }
+
+ DPRINTF(-1, "len=%#x\n", s->io_buffer_size);
+ return s->io_buffer_size != 0;
+}
+
+static int ahci_dma_buf_rw(BMDMAState *bm, int is_write)
+{
+ IDEState *s = bmdma_active_if(bm);
+ int l, len;
+
+ for (;;) {
+ l = s->io_buffer_size - s->io_buffer_index;
+
+ DPRINTF(-1, "size=%#x idx=%#x l=%#x\n",
+ s->io_buffer_size, s->io_buffer_index, l);
+
+ if (l <= 0) {
+ break;
+ }
+
+ if (bm->cur_prd_len == 0) {
+ /* end of table */
+ if (bm->cur_prd_last) {
+ bm->cur_addr = 0;
+ return 0;
+ }
+
+ len = s->sg.sg[bm->cur_addr].len;
+ bm->cur_prd_len = len;
+ bm->cur_prd_addr = s->sg.sg[bm->cur_addr].base;
+
+ DPRINTF(-1, "[%d] base=%#x len=%#x\n",
+ bm->cur_addr, bm->cur_prd_addr, len);
+
+ bm->cur_addr++;
+ bm->cur_prd_last = (bm->cur_addr == s->sg.nsg);
+ }
+
+ if (l > bm->cur_prd_len) {
+ l = bm->cur_prd_len;
+ }
+
+ if (l > 0) {
+ if (is_write) {
+ cpu_physical_memory_write(bm->cur_prd_addr,
+ s->io_buffer + s->io_buffer_index, l);
+ } else {
+ cpu_physical_memory_read(bm->cur_prd_addr,
+ s->io_buffer + s->io_buffer_index, l);
+ }
+ bm->cur_prd_addr += l;
+ bm->cur_prd_len -= l;
+ s->io_buffer_index += l;
+ }
+ }
+
+ return 1;
+}
+
+static void ahci_irq_set_fn(IDEBus *s)
+{
+ AHCIDevice *ad;
+ AHCIState *as;
+
+ ad = DO_UPCAST(AHCIDevice, port, s);
+ as = ad->hba;
+
+ /* error interrupts will be triggered later */
+ if (ad->port.ifs[0].status & ERR_STAT) {
+ return;
+ }
+
+ /* DMA is done */
+ /* XXX find actual end point of a DMA and only do then */
+ if (!(ad->bmdma.status & BM_STATUS_DMAING)) {
+ ahci_trigger_irq(as, ad, PORT_IRQ_STAT_DSS);
+ }
+
+ /* update d2h status */
+ if (ad->cmd_fis) {
+ ahci_write_fis_d2h(as, ad->port_no, ad->cmd_fis);
+ cpu_physical_memory_unmap(ad->cmd_fis, 1, ad->cmd_fis_len, ad->cmd_fis_len);
+ ad->cmd_fis = NULL;
+ }
+}
+
+static IDEBusOps ahci_bus_ops = {
+ .transfer_start_fn = ahci_transfer_start,
+ .irq_set_fn = ahci_irq_set_fn,
+ .dma_start_fn = ahci_dma_start_fn,
+ .dma_prepare_fn = ahci_dma_buf_prepare,
+ .dma_rw_fn = ahci_dma_buf_rw,
+};
+
+static void ahci_init(AHCIState *s, DeviceState *qdev)
+{
+ int i;
+
+ ahci_reg_init(s);
+ s->mem = cpu_register_io_memory(ahci_readfn, ahci_writefn, s);
+
+ for (i = 0; i < SATA_PORTS; i++) {
+ DriveInfo *dinfo = drive_get(IF_SATA, 0, i);
+ AHCIDevice *ad = &s->dev[i];
+
+ ide_bus_new(&ad->port, qdev);
+ ide_init2(&ad->port, 0);
+
+ if (dinfo) {
+ ide_create_drive(&ad->port, 0, dinfo);
+ }
+
+ ad->hba = s;
+ ad->port_no = i;
+ ad->port.bmdma = &ad->bmdma;
+ ad->bmdma.bus = &ad->port;
+ ad->port.ops = &ahci_bus_ops;
+ ad->port_regs.cmd = PORT_CMD_SPIN_UP | PORT_CMD_POWER_ON;
+ }
+}
+
+static 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;
+ AHCIState *s = &d->ahci;
+
+ cpu_register_physical_memory(addr, size, s->mem);
+}
+
+static void ahci_reset(void *opaque)
+{
+ struct AHCIPciState *d = opaque;
+ int i;
+
+ for (i = 0; i < SATA_PORTS; i++) {
+ AHCIDevice *ad = &d->ahci.dev[i];
+
+ ide_bus_reset(&d->ahci.dev[i].port);
+ ide_dma_reset(&d->ahci.dev[i].bmdma);
+
+ ad->port.ifs[0].feature |= IDE_FEATURE_DMA;
+ ad->port.ifs[0].ncq_queues = AHCI_MAX_CMDS;
+ }
+
+ pci_config_set_vendor_id(d->card.config, PCI_VENDOR_ID_INTEL);
+ pci_config_set_device_id(d->card.config, PCI_DEVICE_ID_INTEL_ICH7M_AHCI);
+ d->card.config[PCI_COMMAND] = PCI_COMMAND_IO | PCI_COMMAND_MEMORY |
+ PCI_COMMAND_MASTER;
+
+ pci_config_set_class(d->card.config, PCI_CLASS_STORAGE_SATA);
+ 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 */
+ d->card.config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL;
+ d->card.config[PCI_INTERRUPT_PIN] = 1; /* interrupt pin 0 */
+}
+
+static int pci_ahci_init(PCIDevice *dev)
+{
+ struct AHCIPciState *d;
+ d = DO_UPCAST(struct AHCIPciState, card, dev);
+
+ qemu_register_reset(ahci_reset, d);
+
+ pci_register_bar(&d->card, 5, 0x400, PCI_BASE_ADDRESS_SPACE_MEMORY,
+ ahci_pci_map);
+
+ ahci_init(&d->ahci, &dev->qdev);
+ d->ahci.irq = d->card.irq[0];
+
+ return 0;
+}
+
+static int pci_ahci_uninit(PCIDevice *dev)
+{
+ return 0;
+}
+
+static PCIDeviceInfo ahci_info = {
+ .qdev.name = "ahci",
+ .qdev.size = sizeof(AHCIPciState),
+ .init = pci_ahci_init,
+ .exit = pci_ahci_uninit,
+ .qdev.props = (Property[]) {
+ DEFINE_PROP_END_OF_LIST(),
+ }
+};
+
+static void ahci_pci_register_devices(void)
+{
+ pci_qdev_register(&ahci_info);
+}
+
+device_init(ahci_pci_register_devices)
--
1.6.0.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH 09/10] ahci: add -drive support
2010-11-18 3:27 [Qemu-devel] [PATCH 00/10] AHCI emulation support v2 Alexander Graf
` (7 preceding siblings ...)
2010-11-18 3:27 ` [Qemu-devel] [PATCH 08/10] ahci: add ahci emulation Alexander Graf
@ 2010-11-18 3:27 ` Alexander Graf
2010-11-18 3:27 ` [Qemu-devel] [PATCH 10/10] ahci: spawn controller on demand Alexander Graf
` (2 subsequent siblings)
11 siblings, 0 replies; 30+ messages in thread
From: Alexander Graf @ 2010-11-18 3:27 UTC (permalink / raw)
To: QEMU-devel Developers
Cc: Joerg Roedel, Gerd Hoffmann, Stefan Hajnoczi, tj, Roland Elek,
Sebastian Herbszt
We need to be able to spawn new AHCI drives, so let's add AHCI support
to the -drive option.
Signed-off-by: Alexander Graf <agraf@suse.de>
---
blockdev.c | 6 +++++-
blockdev.h | 1 +
qemu-common.h | 2 +-
3 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 6cb179a..5ce90cc 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -209,6 +209,9 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
} else if (!strcmp(buf, "xen")) {
type = IF_XEN;
max_devs = 0;
+ } else if (!strcmp(buf, "sata")) {
+ type = IF_SATA;
+ max_devs = MAX_SATA_DEVS;
} else if (!strcmp(buf, "none")) {
type = IF_NONE;
max_devs = 0;
@@ -402,7 +405,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
} else {
/* no id supplied -> create one */
dinfo->id = qemu_mallocz(32);
- if (type == IF_IDE || type == IF_SCSI)
+ if (type == IF_IDE || type == IF_SCSI || type == IF_SATA)
mediastr = (media == MEDIA_CDROM) ? "-cd" : "-hd";
if (max_devs)
snprintf(dinfo->id, 32, "%s%i%s%i",
@@ -427,6 +430,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
case IF_IDE:
case IF_SCSI:
case IF_XEN:
+ case IF_SATA:
case IF_NONE:
switch(media) {
case MEDIA_DISK:
diff --git a/blockdev.h b/blockdev.h
index 653affc..f50a15e 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -33,6 +33,7 @@ struct DriveInfo {
#define MAX_IDE_DEVS 2
#define MAX_SCSI_DEVS 7
+#define MAX_SATA_DEVS 4
DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit);
int drive_get_max_bus(BlockInterfaceType type);
diff --git a/qemu-common.h b/qemu-common.h
index b3957f1..ae1cfca 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -257,7 +257,7 @@ typedef uint64_t pcibus_t;
typedef enum {
IF_NONE,
IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
- IF_COUNT
+ IF_SATA, IF_COUNT
} BlockInterfaceType;
void cpu_exec_init_all(unsigned long tb_size);
--
1.6.0.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH 10/10] ahci: spawn controller on demand
2010-11-18 3:27 [Qemu-devel] [PATCH 00/10] AHCI emulation support v2 Alexander Graf
` (8 preceding siblings ...)
2010-11-18 3:27 ` [Qemu-devel] [PATCH 09/10] ahci: add -drive support Alexander Graf
@ 2010-11-18 3:27 ` Alexander Graf
2010-11-18 10:00 ` [Qemu-devel] Re: [PATCH 00/10] AHCI emulation support v2 Stefan Hajnoczi
2010-11-18 13:26 ` [Qemu-devel] " Kevin Wolf
11 siblings, 0 replies; 30+ messages in thread
From: Alexander Graf @ 2010-11-18 3:27 UTC (permalink / raw)
To: QEMU-devel Developers
Cc: Joerg Roedel, Gerd Hoffmann, Stefan Hajnoczi, tj, Roland Elek,
Sebastian Herbszt
When we add a device using -drive to the guest, we also need to create a
new SATA bus to handle the device. This patch does it the same way SCSI
does it today.
Signed-off-by: Alexander Graf <agraf@suse.de>
---
hw/pc.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/hw/pc.c b/hw/pc.c
index 69b13bf..d0320c4 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1075,4 +1075,9 @@ void pc_pci_device_init(PCIBus *pci_bus)
for (bus = 0; bus <= max_bus; bus++) {
pci_create_simple(pci_bus, -1, "lsi53c895a");
}
+
+ max_bus = drive_get_max_bus(IF_SATA);
+ for (bus = 0; bus <= max_bus; bus++) {
+ pci_create_simple(pci_bus, -1, "ahci");
+ }
}
--
1.6.0.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Qemu-devel] Re: [PATCH 08/10] ahci: add ahci emulation
2010-11-18 3:27 ` [Qemu-devel] [PATCH 08/10] ahci: add ahci emulation Alexander Graf
@ 2010-11-18 8:01 ` Gerd Hoffmann
2010-11-18 18:05 ` Alexander Graf
0 siblings, 1 reply; 30+ messages in thread
From: Gerd Hoffmann @ 2010-11-18 8:01 UTC (permalink / raw)
To: Alexander Graf
Cc: Joerg Roedel, QEMU-devel Developers, Stefan Hajnoczi, tj,
Roland Elek, Sebastian Herbszt
Hi,
> +static void ahci_trigger_irq(AHCIState *s, AHCIDevice *d,
> + int irq_type)
> +static void ahci_check_irq(AHCIState *s)
MSI support would be nice to have.
> +#ifndef min
> +#define min(a, b) ((a)< (b) ? (a) : (b))
> +#endif
osdep.h has MIN/MAX macros.
> +static void ahci_init(AHCIState *s, DeviceState *qdev)
> +{
> + ide_bus_new(&ad->port, qdev);
> + ide_init2(&ad->port, 0);
Good.
> + if (dinfo) {
> + ide_create_drive(&ad->port, 0, dinfo);
> + }
That doesn't belong into the qdev init function.
Look how ide/isa.c handles it: The qdev init function (isa_ide_initfn)
doesn't create ide-drives at all. And there is a convinience function
(isa_ide_init) which first creates the controller, then attaches the
drives. The later is called from pc_init() with the drives specified
via -drive if=ide (or -hda).
Does AHCI support drive hotplug btw?
> +static void ahci_reset(void *opaque)
> +{
> + pci_config_set_vendor_id(d->card.config, PCI_VENDOR_ID_INTEL);
> + pci_config_set_device_id(d->card.config, PCI_DEVICE_ID_INTEL_ICH7M_AHCI);
> + pci_config_set_class(d->card.config, PCI_CLASS_STORAGE_SATA);
> + pci_config_set_prog_interface(d->card.config, AHCI_PROGMODE_MAJOR_REV_1);
> + d->card.config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL;
> + d->card.config[PCI_INTERRUPT_PIN] = 1; /* interrupt pin 0 */
Why is that in the reset function instead of init? It should never ever
change, should it?
> +static PCIDeviceInfo ahci_info = {
> + .qdev.name = "ahci",
> + .qdev.size = sizeof(AHCIPciState),
> + .init = pci_ahci_init,
> + .exit = pci_ahci_uninit,
> + .qdev.props = (Property[]) {
> + DEFINE_PROP_END_OF_LIST(),
> + }
If there are no properties you can zap the last tree lines altogether ;)
cheers,
Gerd
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] Re: [PATCH 00/10] AHCI emulation support v2
2010-11-18 3:27 [Qemu-devel] [PATCH 00/10] AHCI emulation support v2 Alexander Graf
` (9 preceding siblings ...)
2010-11-18 3:27 ` [Qemu-devel] [PATCH 10/10] ahci: spawn controller on demand Alexander Graf
@ 2010-11-18 10:00 ` Stefan Hajnoczi
2010-11-18 13:26 ` [Qemu-devel] " Kevin Wolf
11 siblings, 0 replies; 30+ messages in thread
From: Stefan Hajnoczi @ 2010-11-18 10:00 UTC (permalink / raw)
To: Alexander Graf
Cc: Joerg Roedel, QEMU-devel Developers, Gerd Hoffmann, tj,
Roland Elek, Sebastian Herbszt
The changes make sense. Looks good.
Stefan
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH 00/10] AHCI emulation support v2
2010-11-18 3:27 [Qemu-devel] [PATCH 00/10] AHCI emulation support v2 Alexander Graf
` (10 preceding siblings ...)
2010-11-18 10:00 ` [Qemu-devel] Re: [PATCH 00/10] AHCI emulation support v2 Stefan Hajnoczi
@ 2010-11-18 13:26 ` Kevin Wolf
2010-11-18 18:43 ` Alexander Graf
11 siblings, 1 reply; 30+ messages in thread
From: Kevin Wolf @ 2010-11-18 13:26 UTC (permalink / raw)
To: Alexander Graf
Cc: Stefan Hajnoczi, QEMU-devel Developers, Gerd Hoffmann,
Joerg Roedel, tj, Sebastian Herbszt, Roland Elek
Hi Alex,
Am 18.11.2010 04:27, schrieb Alexander Graf:
> This patch adds support for AHCI emulation. I have tested and verified it works
> in Linux, OpenBSD, Windows Vista and Windows 7. This AHCI emulation supports
> NCQ, so multiple read or write requests can be outstanding at the same time.
>
> The code is however not fully optimized yet. I'm fairly sure that there are
> low hanging performance fruits to be found still :). In my simple benchmarks
> I achieved about 2/3rd of virtio performance.
>
> Also, this AHCI emulation layer does not support legacy mode. So if you're
> using a disk with this emulation, you do not get it exposed using the legacy
> IDE interfaces.
>
> Another nitpick is CD-ROM support in Windows. Somehow it doesn't detect a
> CD-ROM drive attached to AHCI. At least it doesn't list it.
>
> To attach an AHCI disk to your VM, please use
>
> -drive file=...,if=sata
>
> This should do the trick for x86. On other platforms, you might need to add
> the ahci host controller using -device.
>
>
> This patch set is based on work done during the Google Summer of Code. I was
> mentoring a student, Roland Elek, who wrote most of the AHCI emulation code
> based on a patch from Chong Qiao. A bunch of other people were also involved,
> so everybody who I didn't mention - thanks a lot!
I'm not completely sure about the relationship between the AHCI
emulation and our existing IDE emulation. First thing I noticed is that
AHCI wants to be independent and resides in hw/ instead of hw/ide/, but
it still include ide/internal.h. Do you think it would make sense to
move AHCI into hw/ide?
Then I believe that core.c is now a mixture of some generic ATA code
(that is also used by SATA) and the Legacy IDE code. SATA doesn't seem
to interact with the generic code through clean interfaces, but by
accessing internal data structures and calls to somewhere in the middle
of the existing IDE emultion. I think we should get a clean abstraction
there and have a clean split between SATA, PATA and common code, with
each of them sitting in its own file in hw/ide.
I haven't reviewed the patches in detail but just had a quick look at
them, so my impressions might be wrong. If so, please correct me.
Kevin
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] Re: [PATCH 08/10] ahci: add ahci emulation
2010-11-18 8:01 ` [Qemu-devel] " Gerd Hoffmann
@ 2010-11-18 18:05 ` Alexander Graf
2010-11-19 9:12 ` Gerd Hoffmann
0 siblings, 1 reply; 30+ messages in thread
From: Alexander Graf @ 2010-11-18 18:05 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Joerg Roedel, QEMU-devel Developers, Stefan Hajnoczi, tj,
Roland Elek, Sebastian Herbszt
Gerd Hoffmann wrote:
> Hi,
>
>> +static void ahci_trigger_irq(AHCIState *s, AHCIDevice *d,
>> + int irq_type)
>
>> +static void ahci_check_irq(AHCIState *s)
>
> MSI support would be nice to have.
Alrighty, I added MSI support. But I only have a single vector in use
atm as nvec > 0 doesn't get written to the PCI config space so the code
trips on that later on.
[...]
>
>> + if (dinfo) {
>> + ide_create_drive(&ad->port, 0, dinfo);
>> + }
>
> That doesn't belong into the qdev init function.
>
> Look how ide/isa.c handles it: The qdev init function
> (isa_ide_initfn) doesn't create ide-drives at all. And there is a
> convinience function (isa_ide_init) which first creates the
> controller, then attaches the drives. The later is called from
> pc_init() with the drives specified via -drive if=ide (or -hda).
Hrm. I guess I'll just put it into the PC specific initialization
function :).
>
> Does AHCI support drive hotplug btw?
AHCI does, my code does not yet :). Feel free to add support for it! :D
Alex
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH 00/10] AHCI emulation support v2
2010-11-18 13:26 ` [Qemu-devel] " Kevin Wolf
@ 2010-11-18 18:43 ` Alexander Graf
2010-11-18 20:06 ` Ryan Harper
2010-11-19 9:15 ` Kevin Wolf
0 siblings, 2 replies; 30+ messages in thread
From: Alexander Graf @ 2010-11-18 18:43 UTC (permalink / raw)
To: Kevin Wolf
Cc: Stefan Hajnoczi, QEMU-devel Developers, Gerd Hoffmann,
Joerg Roedel, tj, Sebastian Herbszt, Roland Elek
On 18.11.2010, at 14:26, Kevin Wolf <kwolf@redhat.com> wrote:
> Hi Alex,
>
> Am 18.11.2010 04:27, schrieb Alexander Graf:
>> This patch adds support for AHCI emulation. I have tested and verified it works
>> in Linux, OpenBSD, Windows Vista and Windows 7. This AHCI emulation supports
>> NCQ, so multiple read or write requests can be outstanding at the same time.
>>
>> The code is however not fully optimized yet. I'm fairly sure that there are
>> low hanging performance fruits to be found still :). In my simple benchmarks
>> I achieved about 2/3rd of virtio performance.
>>
>> Also, this AHCI emulation layer does not support legacy mode. So if you're
>> using a disk with this emulation, you do not get it exposed using the legacy
>> IDE interfaces.
>>
>> Another nitpick is CD-ROM support in Windows. Somehow it doesn't detect a
>> CD-ROM drive attached to AHCI. At least it doesn't list it.
>>
>> To attach an AHCI disk to your VM, please use
>>
>> -drive file=...,if=sata
>>
>> This should do the trick for x86. On other platforms, you might need to add
>> the ahci host controller using -device.
>>
>>
>> This patch set is based on work done during the Google Summer of Code. I was
>> mentoring a student, Roland Elek, who wrote most of the AHCI emulation code
>> based on a patch from Chong Qiao. A bunch of other people were also involved,
>> so everybody who I didn't mention - thanks a lot!
>
> I'm not completely sure about the relationship between the AHCI
> emulation and our existing IDE emulation. First thing I noticed is that
> AHCI wants to be independent and resides in hw/ instead of hw/ide/, but
> it still include ide/internal.h. Do you think it would make sense to
> move AHCI into hw/ide?
Both ahci and ide implement ata. I guess the best thing to do would be to completely refactor all ide code into ata and pata code, then add ahci (sata) to the game. Estimated working time: ~1 month. :)
As I would rather have something working we can base on in the tree, so whoever volunteers for the refactoring (hint!) knows how to design the interfaces, I am not sure how much is reasonable within this patch set.
Moving the file to ide/ does sound like a good idea however.
>
> Then I believe that core.c is now a mixture of some generic ATA code
> (that is also used by SATA) and the Legacy IDE code. SATA doesn't seem
> to interact with the generic code through clean interfaces, but by
> accessing internal data structures and calls to somewhere in the middle
> of the existing IDE emultion. I think we should get a clean abstraction
> there and have a clean split between SATA, PATA and common code, with
> each of them sitting in its own file in hw/ide.
>
> I haven't reviewed the patches in detail but just had a quick look at
> them, so my impressions might be wrong. If so, please correct me.
No, you're completely right. We're in a chicken and egg situation. We don't have ahci, but the ide code is ugly. We would probably do a bad job at refactoring the ata code if we don't know which interfaces to design for.
So IMHO the only way we can really go is to implement sata, take the uglyness it adds to the already ugly ide code and use all of that for a working state we can start to refactor from.
So yes, while I agree with your obversations, I do not believe we will get there until at least 0.16 or so. And I'd rather like to see a fast default block driver in gueast sooner than later :)
Alex
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH 00/10] AHCI emulation support v2
2010-11-18 18:43 ` Alexander Graf
@ 2010-11-18 20:06 ` Ryan Harper
2010-11-18 23:24 ` Alexander Graf
2010-11-19 9:15 ` Kevin Wolf
1 sibling, 1 reply; 30+ messages in thread
From: Ryan Harper @ 2010-11-18 20:06 UTC (permalink / raw)
To: Alexander Graf
Cc: Kevin Wolf, Stefan Hajnoczi, QEMU-devel Developers,
Gerd Hoffmann, Joerg Roedel, tj, Roland Elek, Sebastian Herbszt
* Alexander Graf <agraf@suse.de> [2010-11-18 12:49]:
>
> On 18.11.2010, at 14:26, Kevin Wolf <kwolf@redhat.com> wrote:
>
> > Hi Alex,
> >
> > Am 18.11.2010 04:27, schrieb Alexander Graf:
> >> This patch adds support for AHCI emulation. I have tested and verified it works
> >> in Linux, OpenBSD, Windows Vista and Windows 7. This AHCI emulation supports
> >> NCQ, so multiple read or write requests can be outstanding at the same time.
> >>
> >> The code is however not fully optimized yet. I'm fairly sure that there are
> >> low hanging performance fruits to be found still :). In my simple benchmarks
> >> I achieved about 2/3rd of virtio performance.
> >>
> >> Also, this AHCI emulation layer does not support legacy mode. So if you're
> >> using a disk with this emulation, you do not get it exposed using the legacy
> >> IDE interfaces.
> >>
> >> Another nitpick is CD-ROM support in Windows. Somehow it doesn't detect a
> >> CD-ROM drive attached to AHCI. At least it doesn't list it.
> >>
> >> To attach an AHCI disk to your VM, please use
> >>
> >> -drive file=...,if=sata
> >>
> >> This should do the trick for x86. On other platforms, you might need to add
> >> the ahci host controller using -device.
> >>
> >>
> >> This patch set is based on work done during the Google Summer of Code. I was
> >> mentoring a student, Roland Elek, who wrote most of the AHCI emulation code
> >> based on a patch from Chong Qiao. A bunch of other people were also involved,
> >> so everybody who I didn't mention - thanks a lot!
> >
> > I'm not completely sure about the relationship between the AHCI
> > emulation and our existing IDE emulation. First thing I noticed is that
> > AHCI wants to be independent and resides in hw/ instead of hw/ide/, but
> > it still include ide/internal.h. Do you think it would make sense to
> > move AHCI into hw/ide?
>
> Both ahci and ide implement ata. I guess the best thing to do would be to completely refactor all ide code into ata and pata code, then add ahci (sata) to the game. Estimated working time: ~1 month. :)
>
> As I would rather have something working we can base on in the tree, so whoever volunteers for the refactoring (hint!) knows how to design the interfaces, I am not sure how much is reasonable within this patch set.
>
> Moving the file to ide/ does sound like a good idea however.
>
> >
> > Then I believe that core.c is now a mixture of some generic ATA code
> > (that is also used by SATA) and the Legacy IDE code. SATA doesn't seem
> > to interact with the generic code through clean interfaces, but by
> > accessing internal data structures and calls to somewhere in the middle
> > of the existing IDE emultion. I think we should get a clean abstraction
> > there and have a clean split between SATA, PATA and common code, with
> > each of them sitting in its own file in hw/ide.
> >
> > I haven't reviewed the patches in detail but just had a quick look at
> > them, so my impressions might be wrong. If so, please correct me.
>
> No, you're completely right. We're in a chicken and egg situation. We don't have ahci, but the ide code is ugly. We would probably do a bad job at refactoring the ata code if we don't know which interfaces to design for.
>
> So IMHO the only way we can really go is to implement sata, take the uglyness it adds to the already ugly ide code and use all of that for a working state we can start to refactor from.
>
> So yes, while I agree with your obversations, I do not believe we will
> get there until at least 0.16 or so. And I'd rather like to see a fast
> default block driver in gueast sooner than later :)
>
Speaking of fast, do you have any numbers around ACHI vs IDE (not that I
need any convincing that we can do better than IDE); just curious.
>
> Alex
>
> >
--
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ryanh@us.ibm.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH 00/10] AHCI emulation support v2
2010-11-18 20:06 ` Ryan Harper
@ 2010-11-18 23:24 ` Alexander Graf
2010-11-19 9:12 ` Stefan Hajnoczi
0 siblings, 1 reply; 30+ messages in thread
From: Alexander Graf @ 2010-11-18 23:24 UTC (permalink / raw)
To: Ryan Harper
Cc: Kevin Wolf, Stefan Hajnoczi, QEMU-devel Developers,
Gerd Hoffmann, Joerg Roedel, tj, Roland Elek, Sebastian Herbszt
On 18.11.2010, at 21:06, Ryan Harper wrote:
> Speaking of fast, do you have any numbers around ACHI vs IDE (not that I
> need any convincing that we can do better than IDE); just curious.
To test the raw link speed, I usually take a tmpfs backed sparse raw file and pass it to the guest. Inside the guest, I then dd the device to /dev/null with iflag=direct, so we don't get the guest page cache involved.
ide:
linux-uztg:~ # dd if=/dev/sdc of=/dev/null bs=10M count=300 iflag=direct
300+0 records in
300+0 records out
3145728000 bytes (3.1 GB) copied, 2.06424 s, 1.5 GB/s
ahci:
linux-uztg:~ # dd if=/dev/sda of=/dev/null bs=10M count=300 iflag=direct
300+0 records in
300+0 records out
3145728000 bytes (3.1 GB) copied, 1.40743 s, 2.2 GB/s
virtio:
linux-uztg:~ # dd if=/dev/vda of=/dev/null bs=10M count=300 iflag=direct
300+0 records in
300+0 records out
3145728000 bytes (3.1 GB) copied, 0.851306 s, 3.7 GB/s
Obviously, the main benefit of ahci over ide is that ahci can have multiple outstanding read/write requests at the same time. So even with the same link speed, you would see improvements because the backend can be partially cached or striped over different volumes which make simultaneous requests crucial for performance.
Alex
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH 00/10] AHCI emulation support v2
2010-11-18 23:24 ` Alexander Graf
@ 2010-11-19 9:12 ` Stefan Hajnoczi
2010-11-21 2:32 ` Alexander Graf
0 siblings, 1 reply; 30+ messages in thread
From: Stefan Hajnoczi @ 2010-11-19 9:12 UTC (permalink / raw)
To: Alexander Graf
Cc: Kevin Wolf, Joerg Roedel, QEMU-devel Developers, Ryan Harper,
Gerd Hoffmann, tj, Roland Elek, Sebastian Herbszt
On Thu, Nov 18, 2010 at 11:24 PM, Alexander Graf <agraf@suse.de> wrote:
> linux-uztg:~ # dd if=/dev/sdc of=/dev/null bs=10M count=300 iflag=direct
That's a big block size. bs=8k is interesting too because we see the
per-request overhead. Since IDE, SATA, and virtio have different
hardware interfaces that the guest drives we may see an interesting
picture with small block sizes.
Stefan
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] Re: [PATCH 08/10] ahci: add ahci emulation
2010-11-18 18:05 ` Alexander Graf
@ 2010-11-19 9:12 ` Gerd Hoffmann
2010-11-19 10:08 ` Roedel, Joerg
0 siblings, 1 reply; 30+ messages in thread
From: Gerd Hoffmann @ 2010-11-19 9:12 UTC (permalink / raw)
To: Alexander Graf
Cc: Joerg Roedel, QEMU-devel Developers, Stefan Hajnoczi, tj,
Roland Elek, Sebastian Herbszt
>>> +static void ahci_check_irq(AHCIState *s)
>>
>> MSI support would be nice to have.
>
> Alrighty, I added MSI support. But I only have a single vector in use
> atm as nvec> 0 doesn't get written to the PCI config space so the code
> trips on that later on.
For multiple vectors MSI-X is the best choice as it fixes a few design
issues MSI has when it comes to multiple vectors. I think linux never
ever uses MSI with multiple vectors anyway.
cheers,
Gerd
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH 00/10] AHCI emulation support v2
2010-11-18 18:43 ` Alexander Graf
2010-11-18 20:06 ` Ryan Harper
@ 2010-11-19 9:15 ` Kevin Wolf
2010-11-19 11:56 ` Gerd Hoffmann
2010-11-19 13:08 ` Alexander Graf
1 sibling, 2 replies; 30+ messages in thread
From: Kevin Wolf @ 2010-11-19 9:15 UTC (permalink / raw)
To: Alexander Graf
Cc: Stefan Hajnoczi, QEMU-devel Developers, Gerd Hoffmann,
Joerg Roedel, tj, Sebastian Herbszt, Roland Elek
Am 18.11.2010 19:43, schrieb Alexander Graf:
>
> On 18.11.2010, at 14:26, Kevin Wolf <kwolf@redhat.com> wrote:
>
>> Hi Alex,
>>
>> Am 18.11.2010 04:27, schrieb Alexander Graf:
>>> This patch adds support for AHCI emulation. I have tested and verified it works
>>> in Linux, OpenBSD, Windows Vista and Windows 7. This AHCI emulation supports
>>> NCQ, so multiple read or write requests can be outstanding at the same time.
>>>
>>> The code is however not fully optimized yet. I'm fairly sure that there are
>>> low hanging performance fruits to be found still :). In my simple benchmarks
>>> I achieved about 2/3rd of virtio performance.
>>>
>>> Also, this AHCI emulation layer does not support legacy mode. So if you're
>>> using a disk with this emulation, you do not get it exposed using the legacy
>>> IDE interfaces.
>>>
>>> Another nitpick is CD-ROM support in Windows. Somehow it doesn't detect a
>>> CD-ROM drive attached to AHCI. At least it doesn't list it.
>>>
>>> To attach an AHCI disk to your VM, please use
>>>
>>> -drive file=...,if=sata
>>>
>>> This should do the trick for x86. On other platforms, you might need to add
>>> the ahci host controller using -device.
>>>
>>>
>>> This patch set is based on work done during the Google Summer of Code. I was
>>> mentoring a student, Roland Elek, who wrote most of the AHCI emulation code
>>> based on a patch from Chong Qiao. A bunch of other people were also involved,
>>> so everybody who I didn't mention - thanks a lot!
>>
>> I'm not completely sure about the relationship between the AHCI
>> emulation and our existing IDE emulation. First thing I noticed is that
>> AHCI wants to be independent and resides in hw/ instead of hw/ide/, but
>> it still include ide/internal.h. Do you think it would make sense to
>> move AHCI into hw/ide?
>
> Both ahci and ide implement ata. I guess the best thing to do would be to completely refactor all ide code into ata and pata code, then add ahci (sata) to the game. Estimated working time: ~1 month. :)
>
> As I would rather have something working we can base on in the tree, so whoever volunteers for the refactoring (hint!) knows how to design the interfaces, I am not sure how much is reasonable within this patch set.
I guess I have to read this as: You want to drop the code into the
repository, no matter what mess it creates, and leave the cleanup to
some volunteer that will never appear. So whoever is in the unfortunate
position of having to touch the IDE code afterwards will have the pain
because this series is doing only the half of what should be done.
I'm sure that with a working time of only a few days the result could be
substantially improved, even if a month would be needed for perfection.
> Moving the file to ide/ does sound like a good idea however.
>
>>
>> Then I believe that core.c is now a mixture of some generic ATA code
>> (that is also used by SATA) and the Legacy IDE code. SATA doesn't seem
>> to interact with the generic code through clean interfaces, but by
>> accessing internal data structures and calls to somewhere in the middle
>> of the existing IDE emultion. I think we should get a clean abstraction
>> there and have a clean split between SATA, PATA and common code, with
>> each of them sitting in its own file in hw/ide.
>>
>> I haven't reviewed the patches in detail but just had a quick look at
>> them, so my impressions might be wrong. If so, please correct me.
>
> No, you're completely right. We're in a chicken and egg situation. We don't have ahci, but the ide code is ugly. We would probably do a bad job at refactoring the ata code if we don't know which interfaces to design for.
That problem is solved. You have posted patches, so you're aware what
interfaces you need for AHCI. This awareness doesn't come from putting
the code into git master.
> So IMHO the only way we can really go is to implement sata, take the uglyness it adds to the already ugly ide code and use all of that for a working state we can start to refactor from.
>
> So yes, while I agree with your obversations, I do not believe we will get there until at least 0.16 or so. And I'd rather like to see a fast default block driver in gueast sooner than later :)
Note that not doing the refactoring in this patch series means not only
that it won't happen, but also that your patches are very hard to
review. The order in which I would expect changes is to do the
refactoring and then base the SATA code on these clean interfaces - not
first creating a mess and then (maybe some time) trying to clean it up.
You always say "we" will refactor and "we" will clean up, so who is this
"we"? Will you do it or are you rather trying to put it on my todo list?
I must admit that I wouldn't be happy about the latter.
Kevin
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] Re: [PATCH 08/10] ahci: add ahci emulation
2010-11-19 9:12 ` Gerd Hoffmann
@ 2010-11-19 10:08 ` Roedel, Joerg
0 siblings, 0 replies; 30+ messages in thread
From: Roedel, Joerg @ 2010-11-19 10:08 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Stefan Hajnoczi, Herbszt, Alexander Graf, QEMU-devel Developers,
tj, Sebastian, Roland Elek
On Fri, Nov 19, 2010 at 04:12:43AM -0500, Gerd Hoffmann wrote:
> >>> +static void ahci_check_irq(AHCIState *s)
> >>
> >> MSI support would be nice to have.
> >
> > Alrighty, I added MSI support. But I only have a single vector in use
> > atm as nvec> 0 doesn't get written to the PCI config space so the code
> > trips on that later on.
>
> For multiple vectors MSI-X is the best choice as it fixes a few design
> issues MSI has when it comes to multiple vectors. I think linux never
> ever uses MSI with multiple vectors anyway.
Linux doesn't even support MSI with multiple vectors today.
Joerg
--
AMD Operating System Research Center
Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH 00/10] AHCI emulation support v2
2010-11-19 9:15 ` Kevin Wolf
@ 2010-11-19 11:56 ` Gerd Hoffmann
2010-11-19 12:27 ` Kevin Wolf
2010-11-19 13:08 ` Alexander Graf
1 sibling, 1 reply; 30+ messages in thread
From: Gerd Hoffmann @ 2010-11-19 11:56 UTC (permalink / raw)
To: Kevin Wolf
Cc: Stefan Hajnoczi, Alexander Graf, QEMU-devel Developers,
Joerg Roedel, tj, Sebastian Herbszt, Roland Elek
Hi,
>> As I would rather have something working we can base on in the
>> tree, so whoever volunteers for the refactoring (hint!) knows how
>> to design the interfaces, I am not sure how much is reasonable
>> within this patch set.
>
> I guess I have to read this as: You want to drop the code into the
> repository, no matter what mess it creates, and leave the cleanup to
> some volunteer that will never appear. So whoever is in the
> unfortunate position of having to touch the IDE code afterwards will
> have the pain because this series is doing only the half of what
> should be done.
Well, this is how IDE gets cleaned up right now. I was one of the
victims ;)
IDE is in a noticeable better state than it used to be two years ago.
It is still quite messy though.
> I'm sure that with a working time of only a few days the result could
> be substantially improved, even if a month would be needed for
> perfection.
[ disclaimer: didn't look at v3 yet ]
Adding ahci should at least not make it more messy that it already is.
Doing the controller-depending dispatching through a function pointer
table (aka IDEBusOps) looks sane to me. SCSI does the same, although
without a Ops struct as it needs a single function pointer only.
Moving code shared between sata/ahci and ide to another source file
(ata.c ?) makes sense and would be a good start to make clear which code
is used for what.
IDEBus must learn some things about sata. For example there is no such
thing as a slave device, so a IDEBus representing a sata port must not
allow slave devices being attached.
Fixing the IDEState mess would be great. Right now IDEBus carries a
array of IDEStates for master and slave for historical reasons: The ide
code (used to?) assume that the IDEStates for master and slave are
allocated together and that it can easily jump from one to the other and
back using pointer arithmetics on the IDEState struct. IDEState doesn't
belong there though, it should be part of IDEDrive. Also having a
unused slave state doesn't make sense at all for sata. Fixing that
without breaking migration could be non-trivial though.
cheers,
Gerd
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH 00/10] AHCI emulation support v2
2010-11-19 11:56 ` Gerd Hoffmann
@ 2010-11-19 12:27 ` Kevin Wolf
0 siblings, 0 replies; 30+ messages in thread
From: Kevin Wolf @ 2010-11-19 12:27 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Stefan Hajnoczi, Alexander Graf, QEMU-devel Developers,
Joerg Roedel, tj, Sebastian Herbszt, Roland Elek
Am 19.11.2010 12:56, schrieb Gerd Hoffmann:
> Hi,
>
>>> As I would rather have something working we can base on in the
>>> tree, so whoever volunteers for the refactoring (hint!) knows how
>>> to design the interfaces, I am not sure how much is reasonable
>>> within this patch set.
>>
>> I guess I have to read this as: You want to drop the code into the
>> repository, no matter what mess it creates, and leave the cleanup to
>> some volunteer that will never appear. So whoever is in the
>> unfortunate position of having to touch the IDE code afterwards will
>> have the pain because this series is doing only the half of what
>> should be done.
>
> Well, this is how IDE gets cleaned up right now. I was one of the
> victims ;)
>
> IDE is in a noticeable better state than it used to be two years ago.
> It is still quite messy though.
Sure that's how it works today. We have no choice because we can't
change the mistakes that were made in the past. But we can avoid to
contribute new mess to it or we'll need even more victims. ;-)
>> I'm sure that with a working time of only a few days the result could
>> be substantially improved, even if a month would be needed for
>> perfection.
>
> [ disclaimer: didn't look at v3 yet ]
>
> Adding ahci should at least not make it more messy that it already is.
>
> Doing the controller-depending dispatching through a function pointer
> table (aka IDEBusOps) looks sane to me. SCSI does the same, although
> without a Ops struct as it needs a single function pointer only.
Yes, I suppose (still haven't looked at it in detail) these interfaces
are generally sane, based on your comments and a quick look.
This is also what makes me think that doing the next step shouldn't be
too hard. The abstraction is basically in place, the rest should be
mostly code movement and maybe splitting up some IDE data structures to
make things look more symmetric between PATA and SATA.
> Moving code shared between sata/ahci and ide to another source file
> (ata.c ?) makes sense and would be a good start to make clear which code
> is used for what.
I agree (and ata.c would have been my suggestion, too).
> IDEBus must learn some things about sata. For example there is no such
> thing as a slave device, so a IDEBus representing a sata port must not
> allow slave devices being attached.
Hm, do the generic functions (i.e. what would end up in ata.c) even need
access to the IDEBus? If not, we could leave IDEBus as it is for Legacy
IDE and introduce a completely new SATABus.
Most occurrences in core.c seem to use the bus only for ide_set_irq.
> Fixing the IDEState mess would be great. Right now IDEBus carries a
> array of IDEStates for master and slave for historical reasons: The ide
> code (used to?) assume that the IDEStates for master and slave are
> allocated together and that it can easily jump from one to the other and
> back using pointer arithmetics on the IDEState struct. IDEState doesn't
> belong there though, it should be part of IDEDrive.
Okay, that makes sense.
Kevin
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH 00/10] AHCI emulation support v2
2010-11-19 9:15 ` Kevin Wolf
2010-11-19 11:56 ` Gerd Hoffmann
@ 2010-11-19 13:08 ` Alexander Graf
2010-11-19 13:46 ` Kevin Wolf
2010-11-19 14:36 ` Gerd Hoffmann
1 sibling, 2 replies; 30+ messages in thread
From: Alexander Graf @ 2010-11-19 13:08 UTC (permalink / raw)
To: Kevin Wolf
Cc: Stefan Hajnoczi, QEMU-devel Developers, Gerd Hoffmann,
Joerg Roedel, tj, Sebastian Herbszt, Roland Elek
On 19.11.2010, at 10:15, Kevin Wolf wrote:
> Am 18.11.2010 19:43, schrieb Alexander Graf:
>>
>> On 18.11.2010, at 14:26, Kevin Wolf <kwolf@redhat.com> wrote:
>>
>>> Hi Alex,
>>>
>>> Am 18.11.2010 04:27, schrieb Alexander Graf:
>>>> This patch adds support for AHCI emulation. I have tested and verified it works
>>>> in Linux, OpenBSD, Windows Vista and Windows 7. This AHCI emulation supports
>>>> NCQ, so multiple read or write requests can be outstanding at the same time.
>>>>
>>>> The code is however not fully optimized yet. I'm fairly sure that there are
>>>> low hanging performance fruits to be found still :). In my simple benchmarks
>>>> I achieved about 2/3rd of virtio performance.
>>>>
>>>> Also, this AHCI emulation layer does not support legacy mode. So if you're
>>>> using a disk with this emulation, you do not get it exposed using the legacy
>>>> IDE interfaces.
>>>>
>>>> Another nitpick is CD-ROM support in Windows. Somehow it doesn't detect a
>>>> CD-ROM drive attached to AHCI. At least it doesn't list it.
>>>>
>>>> To attach an AHCI disk to your VM, please use
>>>>
>>>> -drive file=...,if=sata
>>>>
>>>> This should do the trick for x86. On other platforms, you might need to add
>>>> the ahci host controller using -device.
>>>>
>>>>
>>>> This patch set is based on work done during the Google Summer of Code. I was
>>>> mentoring a student, Roland Elek, who wrote most of the AHCI emulation code
>>>> based on a patch from Chong Qiao. A bunch of other people were also involved,
>>>> so everybody who I didn't mention - thanks a lot!
>>>
>>> I'm not completely sure about the relationship between the AHCI
>>> emulation and our existing IDE emulation. First thing I noticed is that
>>> AHCI wants to be independent and resides in hw/ instead of hw/ide/, but
>>> it still include ide/internal.h. Do you think it would make sense to
>>> move AHCI into hw/ide?
>>
>> Both ahci and ide implement ata. I guess the best thing to do would be to completely refactor all ide code into ata and pata code, then add ahci (sata) to the game. Estimated working time: ~1 month. :)
>>
>> As I would rather have something working we can base on in the tree, so whoever volunteers for the refactoring (hint!) knows how to design the interfaces, I am not sure how much is reasonable within this patch set.
>
> I guess I have to read this as: You want to drop the code into the
> repository, no matter what mess it creates, and leave the cleanup to
> some volunteer that will never appear. So whoever is in the unfortunate
> position of having to touch the IDE code afterwards will have the pain
> because this series is doing only the half of what should be done.
>
> I'm sure that with a working time of only a few days the result could be
> substantially improved, even if a month would be needed for perfection.
>
>> Moving the file to ide/ does sound like a good idea however.
>>
>>>
>>> Then I believe that core.c is now a mixture of some generic ATA code
>>> (that is also used by SATA) and the Legacy IDE code. SATA doesn't seem
>>> to interact with the generic code through clean interfaces, but by
>>> accessing internal data structures and calls to somewhere in the middle
>>> of the existing IDE emultion. I think we should get a clean abstraction
>>> there and have a clean split between SATA, PATA and common code, with
>>> each of them sitting in its own file in hw/ide.
>>>
>>> I haven't reviewed the patches in detail but just had a quick look at
>>> them, so my impressions might be wrong. If so, please correct me.
>>
>> No, you're completely right. We're in a chicken and egg situation. We don't have ahci, but the ide code is ugly. We would probably do a bad job at refactoring the ata code if we don't know which interfaces to design for.
>
> That problem is solved. You have posted patches, so you're aware what
> interfaces you need for AHCI. This awareness doesn't come from putting
> the code into git master.
I guess you should go back and read the "this doesn't work yet" list. There is a lot of stuff that I'm not sure we have all correctly sorted out. The most intrusive one on that side is the legacy IDE compatibility. I don't know what interfaces and what changes we will need for that to become realistic.
Also to catch up on Gerd's point - whatever refactoring we do, we will basically have to break migration. There is no way we can change all the internal state and structure and maintain binary compatibility with the old save states.
Alex
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH 00/10] AHCI emulation support v2
2010-11-19 13:08 ` Alexander Graf
@ 2010-11-19 13:46 ` Kevin Wolf
2010-11-21 2:19 ` Alexander Graf
2010-11-19 14:36 ` Gerd Hoffmann
1 sibling, 1 reply; 30+ messages in thread
From: Kevin Wolf @ 2010-11-19 13:46 UTC (permalink / raw)
To: Alexander Graf
Cc: Juan Quintela, Stefan Hajnoczi, QEMU-devel Developers,
Gerd Hoffmann, Joerg Roedel, tj, Sebastian Herbszt, Roland Elek
Am 19.11.2010 14:08, schrieb Alexander Graf:
>
> On 19.11.2010, at 10:15, Kevin Wolf wrote:
>
>> Am 18.11.2010 19:43, schrieb Alexander Graf:
>>>> Then I believe that core.c is now a mixture of some generic ATA code
>>>> (that is also used by SATA) and the Legacy IDE code. SATA doesn't seem
>>>> to interact with the generic code through clean interfaces, but by
>>>> accessing internal data structures and calls to somewhere in the middle
>>>> of the existing IDE emultion. I think we should get a clean abstraction
>>>> there and have a clean split between SATA, PATA and common code, with
>>>> each of them sitting in its own file in hw/ide.
>>>>
>>>> I haven't reviewed the patches in detail but just had a quick look at
>>>> them, so my impressions might be wrong. If so, please correct me.
>>>
>>> No, you're completely right. We're in a chicken and egg situation. We don't have ahci, but the ide code is ugly. We would probably do a bad job at refactoring the ata code if we don't know which interfaces to design for.
>>
>> That problem is solved. You have posted patches, so you're aware what
>> interfaces you need for AHCI. This awareness doesn't come from putting
>> the code into git master.
>
> I guess you should go back and read the "this doesn't work yet" list. There is a lot of stuff that I'm not sure we have all correctly sorted out. The most intrusive one on that side is the legacy IDE compatibility. I don't know what interfaces and what changes we will need for that to become realistic.
Fair enough. I'll accept that we can't get it sorted out now, but let's
try to do the part that we can do. Let's change the split to SATA
(ahci.c), Legacy IDE (ide.c?), common code (ata.c) and "don't know yet"
(core.c).
A start for that would be if in Patch 2 you moved the function to ata.c
instead of just reindenting. We're also probably pretty sure that, for
example, the I/O port handling won't need to be shared and can be moved
to ide.c. Whenever it becomes clear for a part in which category it
belongs, we would move it out of core.c and eventually, I hope, core.c
could be removed.
> Also to catch up on Gerd's point - whatever refactoring we do, we will basically have to break migration. There is no way we can change all the internal state and structure and maintain binary compatibility with the old save states.
Hm, breaking migration isn't really an option. I'm not familiar with
migration code, but maybe Juan can contribute some magic?
Speaking of migration, that seems to be missing for the AHCI yet, too.
Are you planning to complete the functionality first before you start
with that?
Kevin
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH 00/10] AHCI emulation support v2
2010-11-19 13:08 ` Alexander Graf
2010-11-19 13:46 ` Kevin Wolf
@ 2010-11-19 14:36 ` Gerd Hoffmann
1 sibling, 0 replies; 30+ messages in thread
From: Gerd Hoffmann @ 2010-11-19 14:36 UTC (permalink / raw)
To: Alexander Graf
Cc: Kevin Wolf, Juan Quintela, Joerg Roedel, QEMU-devel Developers,
Stefan Hajnoczi, tj, Sebastian Herbszt, Roland Elek
Hi,
> Also to catch up on Gerd's point - whatever refactoring we do, we
> will basically have to break migration. There is no way we can change
> all the internal state and structure and maintain binary
> compatibility with the old save states.
On the other hand it would be a *real* pity to drag the ide migration
mess into ahci ...
Ideally each ide-drive should save its state itself, and likewise each
ide/ahci controller. Maybe it would be a good first step to move to
such a model in the savevm data format, but keep the current ide data
structures for a while so we can easily accept migrations in the old
savevm format for the time being.
Then a relase or two away from today we'll drop support for the old ide
savevm format, so we can finally tranform the ide data structures to
something sane?
Juan? Comments?
cheers,
Gerd
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH 00/10] AHCI emulation support v2
2010-11-19 13:46 ` Kevin Wolf
@ 2010-11-21 2:19 ` Alexander Graf
2010-11-22 8:45 ` Kevin Wolf
0 siblings, 1 reply; 30+ messages in thread
From: Alexander Graf @ 2010-11-21 2:19 UTC (permalink / raw)
To: Kevin Wolf
Cc: Juan Quintela, Stefan Hajnoczi, QEMU-devel Developers,
Gerd Hoffmann, Joerg Roedel, tj, Sebastian Herbszt, Roland Elek
On 19.11.2010, at 14:46, Kevin Wolf wrote:
> Am 19.11.2010 14:08, schrieb Alexander Graf:
>>
>> On 19.11.2010, at 10:15, Kevin Wolf wrote:
>>
>>> Am 18.11.2010 19:43, schrieb Alexander Graf:
>>>>> Then I believe that core.c is now a mixture of some generic ATA code
>>>>> (that is also used by SATA) and the Legacy IDE code. SATA doesn't seem
>>>>> to interact with the generic code through clean interfaces, but by
>>>>> accessing internal data structures and calls to somewhere in the middle
>>>>> of the existing IDE emultion. I think we should get a clean abstraction
>>>>> there and have a clean split between SATA, PATA and common code, with
>>>>> each of them sitting in its own file in hw/ide.
>>>>>
>>>>> I haven't reviewed the patches in detail but just had a quick look at
>>>>> them, so my impressions might be wrong. If so, please correct me.
>>>>
>>>> No, you're completely right. We're in a chicken and egg situation. We don't have ahci, but the ide code is ugly. We would probably do a bad job at refactoring the ata code if we don't know which interfaces to design for.
>>>
>>> That problem is solved. You have posted patches, so you're aware what
>>> interfaces you need for AHCI. This awareness doesn't come from putting
>>> the code into git master.
>>
>> I guess you should go back and read the "this doesn't work yet" list. There is a lot of stuff that I'm not sure we have all correctly sorted out. The most intrusive one on that side is the legacy IDE compatibility. I don't know what interfaces and what changes we will need for that to become realistic.
>
> Fair enough. I'll accept that we can't get it sorted out now, but let's
> try to do the part that we can do. Let's change the split to SATA
> (ahci.c), Legacy IDE (ide.c?), common code (ata.c) and "don't know yet"
> (core.c).
>
> A start for that would be if in Patch 2 you moved the function to ata.c
> instead of just reindenting. We're also probably pretty sure that, for
> example, the I/O port handling won't need to be shared and can be moved
> to ide.c. Whenever it becomes clear for a part in which category it
> belongs, we would move it out of core.c and eventually, I hope, core.c
> could be removed.
I can certainly move out obviously pata specific pieces to a new file called pata.c. As for the split between ata.c and core.c, I don't think it's useful. Once we moved everything pata specific out or core.c, that file will essentially be ata.c.
>
>> Also to catch up on Gerd's point - whatever refactoring we do, we will basically have to break migration. There is no way we can change all the internal state and structure and maintain binary compatibility with the old save states.
>
> Hm, breaking migration isn't really an option. I'm not familiar with
> migration code, but maybe Juan can contribute some magic?
>
> Speaking of migration, that seems to be missing for the AHCI yet, too.
> Are you planning to complete the functionality first before you start
> with that?
I'm planning to take baby steps so others can contribute and I don't keep a patch set lying around become more invasive and thus more prone to bitrot every day :). I myself just don't scale well enough for a feature this intrusive and important.
I had the code bitrot for quite a bit already btw. GSoC ended a couple months ago and I just didn't get around to polish the code well enough for upstream submission. And believe me, it rots very fast.
Vmstate is an issue we need to solve. I'm not sure what the right way forward for that would be. I certainly would not recommend declaring the migration protocol for sata even remotely stable for the time being, as we're missing crucial pieces still that might require major restructuring or even duplicating of core ide code. And as long as that's the case, I'm not sure how much sense it really makes to have any at all.
Basically, if there was a CONFIG_EXPERIMENTAL flag, I would set it on ahci. The code is not and will not be 100% stable and well structures and reliable within the next probably 1/2 year to year. But we need to start walking into a direction where it can finally end up being there some day, and that only works by multiple people working together on this, preferably upstream, so we don't collide with other possible ide work.
Of course there's some chance that it won't get there. If there is interest in it though, it will. And from what I've gathered so far, there is interest, as it's a speedup for a lot of guests without the need for special drivers. If it just lies around without getting improved, rip it out again :). If nobody works on it, nobody uses it.
Alex
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH 00/10] AHCI emulation support v2
2010-11-19 9:12 ` Stefan Hajnoczi
@ 2010-11-21 2:32 ` Alexander Graf
0 siblings, 0 replies; 30+ messages in thread
From: Alexander Graf @ 2010-11-21 2:32 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Kevin Wolf, Joerg Roedel, QEMU-devel Developers, Ryan Harper,
Gerd Hoffmann, tj, Roland Elek, Sebastian Herbszt
On 19.11.2010, at 10:12, Stefan Hajnoczi wrote:
> On Thu, Nov 18, 2010 at 11:24 PM, Alexander Graf <agraf@suse.de> wrote:
>> linux-uztg:~ # dd if=/dev/sdc of=/dev/null bs=10M count=300 iflag=direct
>
> That's a big block size. bs=8k is interesting too because we see the
> per-request overhead. Since IDE, SATA, and virtio have different
> hardware interfaces that the guest drives we may see an interesting
> picture with small block sizes.
IDE: 60MB/s
SATA: 100MB/s
virtio: 120 MB/s
I had a pretty big jitter on that one though, with SATA going between 80MB and 120MB :). But overall, it's the same picture as the one with big block sizes.
Alex
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH 00/10] AHCI emulation support v2
2010-11-21 2:19 ` Alexander Graf
@ 2010-11-22 8:45 ` Kevin Wolf
0 siblings, 0 replies; 30+ messages in thread
From: Kevin Wolf @ 2010-11-22 8:45 UTC (permalink / raw)
To: Alexander Graf
Cc: Juan Quintela, Stefan Hajnoczi, QEMU-devel Developers,
Gerd Hoffmann, Joerg Roedel, tj, Sebastian Herbszt, Roland Elek
Am 21.11.2010 03:19, schrieb Alexander Graf:
>
> On 19.11.2010, at 14:46, Kevin Wolf wrote:
>
>> Am 19.11.2010 14:08, schrieb Alexander Graf:
>>>
>>> On 19.11.2010, at 10:15, Kevin Wolf wrote:
>>>
>>>> Am 18.11.2010 19:43, schrieb Alexander Graf:
>>>>>> Then I believe that core.c is now a mixture of some generic ATA code
>>>>>> (that is also used by SATA) and the Legacy IDE code. SATA doesn't seem
>>>>>> to interact with the generic code through clean interfaces, but by
>>>>>> accessing internal data structures and calls to somewhere in the middle
>>>>>> of the existing IDE emultion. I think we should get a clean abstraction
>>>>>> there and have a clean split between SATA, PATA and common code, with
>>>>>> each of them sitting in its own file in hw/ide.
>>>>>>
>>>>>> I haven't reviewed the patches in detail but just had a quick look at
>>>>>> them, so my impressions might be wrong. If so, please correct me.
>>>>>
>>>>> No, you're completely right. We're in a chicken and egg situation. We don't have ahci, but the ide code is ugly. We would probably do a bad job at refactoring the ata code if we don't know which interfaces to design for.
>>>>
>>>> That problem is solved. You have posted patches, so you're aware what
>>>> interfaces you need for AHCI. This awareness doesn't come from putting
>>>> the code into git master.
>>>
>>> I guess you should go back and read the "this doesn't work yet" list. There is a lot of stuff that I'm not sure we have all correctly sorted out. The most intrusive one on that side is the legacy IDE compatibility. I don't know what interfaces and what changes we will need for that to become realistic.
>>
>> Fair enough. I'll accept that we can't get it sorted out now, but let's
>> try to do the part that we can do. Let's change the split to SATA
>> (ahci.c), Legacy IDE (ide.c?), common code (ata.c) and "don't know yet"
>> (core.c).
>>
>> A start for that would be if in Patch 2 you moved the function to ata.c
>> instead of just reindenting. We're also probably pretty sure that, for
>> example, the I/O port handling won't need to be shared and can be moved
>> to ide.c. Whenever it becomes clear for a part in which category it
>> belongs, we would move it out of core.c and eventually, I hope, core.c
>> could be removed.
>
> I can certainly move out obviously pata specific pieces to a new file called pata.c. As for the split between ata.c and core.c, I don't think it's useful. Once we moved everything pata specific out or core.c, that file will essentially be ata.c.
The reason why I suggested ata.c is that core.c would serve as kind of a
todo list. But I don't really mind if you wan to keep it in core.c, the
important part is getting the split between core/ata and pata.
>>> Also to catch up on Gerd's point - whatever refactoring we do, we will basically have to break migration. There is no way we can change all the internal state and structure and maintain binary compatibility with the old save states.
>>
>> Hm, breaking migration isn't really an option. I'm not familiar with
>> migration code, but maybe Juan can contribute some magic?
>>
>> Speaking of migration, that seems to be missing for the AHCI yet, too.
>> Are you planning to complete the functionality first before you start
>> with that?
>
> I'm planning to take baby steps so others can contribute and I don't keep a patch set lying around become more invasive and thus more prone to bitrot every day :). I myself just don't scale well enough for a feature this intrusive and important.
>
> I had the code bitrot for quite a bit already btw. GSoC ended a couple months ago and I just didn't get around to polish the code well enough for upstream submission. And believe me, it rots very fast.
>
> Vmstate is an issue we need to solve. I'm not sure what the right way forward for that would be. I certainly would not recommend declaring the migration protocol for sata even remotely stable for the time being, as we're missing crucial pieces still that might require major restructuring or even duplicating of core ide code. And as long as that's the case, I'm not sure how much sense it really makes to have any at all.
Okay, I think that's a good point.
I was asking because I'm not sure if it wouldn't be easier to have
migration working early and then incrementally change it with whatever
is added, compared to developing everything and adding migration as the
last thing. I haven't done either yet, so I might be wrong.
> Basically, if there was a CONFIG_EXPERIMENTAL flag, I would set it on ahci. The code is not and will not be 100% stable and well structures and reliable within the next probably 1/2 year to year. But we need to start walking into a direction where it can finally end up being there some day, and that only works by multiple people working together on this, preferably upstream, so we don't collide with other possible ide work.
>
> Of course there's some chance that it won't get there. If there is interest in it though, it will. And from what I've gathered so far, there is interest, as it's a speedup for a lot of guests without the need for special drivers. If it just lies around without getting improved, rip it out again :). If nobody works on it, nobody uses it.
Not disagreeing with that. Also, we have a lot of hardware that has only
a few users. That's never been a reason for dropping it.
The one important point to me is that AHCI touches IDE and shouldn't
leave it in a worse state than before. If AHCI itself in its current
state really works well all the time is secondary for me, because, as
you say, there's still time to fix and enhance it when it's in.
Kevin
^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2010-11-22 8:44 UTC | newest]
Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-18 3:27 [Qemu-devel] [PATCH 00/10] AHCI emulation support v2 Alexander Graf
2010-11-18 3:27 ` [Qemu-devel] [PATCH 01/10] ide: split ide command interpretation off Alexander Graf
2010-11-18 3:27 ` [Qemu-devel] [PATCH 02/10] ide: fix whitespace gap in ide_exec_cmd Alexander Graf
2010-11-18 3:27 ` [Qemu-devel] [PATCH 03/10] ide: add support for ide bus ops Alexander Graf
2010-11-18 3:27 ` [Qemu-devel] [PATCH 04/10] ide: add DMA hooks to " Alexander Graf
2010-11-18 3:27 ` [Qemu-devel] [PATCH 05/10] ide: add ncq identify data for ahci sata drives Alexander Graf
2010-11-18 3:27 ` [Qemu-devel] [PATCH 06/10] pci: add storage class for sata Alexander Graf
2010-11-18 3:27 ` [Qemu-devel] [PATCH 07/10] pci: add ich7 pci id Alexander Graf
2010-11-18 3:27 ` [Qemu-devel] [PATCH 08/10] ahci: add ahci emulation Alexander Graf
2010-11-18 8:01 ` [Qemu-devel] " Gerd Hoffmann
2010-11-18 18:05 ` Alexander Graf
2010-11-19 9:12 ` Gerd Hoffmann
2010-11-19 10:08 ` Roedel, Joerg
2010-11-18 3:27 ` [Qemu-devel] [PATCH 09/10] ahci: add -drive support Alexander Graf
2010-11-18 3:27 ` [Qemu-devel] [PATCH 10/10] ahci: spawn controller on demand Alexander Graf
2010-11-18 10:00 ` [Qemu-devel] Re: [PATCH 00/10] AHCI emulation support v2 Stefan Hajnoczi
2010-11-18 13:26 ` [Qemu-devel] " Kevin Wolf
2010-11-18 18:43 ` Alexander Graf
2010-11-18 20:06 ` Ryan Harper
2010-11-18 23:24 ` Alexander Graf
2010-11-19 9:12 ` Stefan Hajnoczi
2010-11-21 2:32 ` Alexander Graf
2010-11-19 9:15 ` Kevin Wolf
2010-11-19 11:56 ` Gerd Hoffmann
2010-11-19 12:27 ` Kevin Wolf
2010-11-19 13:08 ` Alexander Graf
2010-11-19 13:46 ` Kevin Wolf
2010-11-21 2:19 ` Alexander Graf
2010-11-22 8:45 ` Kevin Wolf
2010-11-19 14:36 ` Gerd Hoffmann
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.