* [Qemu-devel] [PATCH 01/23] ide: Add handler to ide_cmd_table
2013-06-24 9:10 [Qemu-devel] [PULL 00/23] Block patches Stefan Hajnoczi
@ 2013-06-24 9:10 ` Stefan Hajnoczi
2013-06-24 9:10 ` [Qemu-devel] [PATCH 02/23] ide: Convert WIN_DSM to ide_cmd_table handler Stefan Hajnoczi
` (21 subsequent siblings)
22 siblings, 0 replies; 35+ messages in thread
From: Stefan Hajnoczi @ 2013-06-24 9:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi
From: Kevin Wolf <kwolf@redhat.com>
As a preparation for moving all IDE commands into their own function
like in the ATAPI code, introduce a 'handler' callback to ide_cmd_table.
Commands using this new infrastructure get some things handled
automatically:
* The BSY flag is set before calling the handler (in order to avoid bugs
like the one fixed in f68ec837) and reset on completion.
* The (obsolete) DSC flag in the status register is set on completion if
the command is flagged with SET_DSC in the command table
* An IRQ is triggered on completion.
* The error register and the ERR flag in the status register are cleared
before calling the handler and on completion it is asserted that
either none or both of them are set.
No commands are converted at this point.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/ide/core.c | 144 +++++++++++++++++++++++++++++++++++-----------------------
1 file changed, 86 insertions(+), 58 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 9926d92..cd9de14 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1010,71 +1010,78 @@ void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val)
#define HD_CFA_OK (HD_OK | CFA_OK)
#define ALL_OK (HD_OK | CD_OK | CFA_OK)
+/* Set the Disk Seek Completed status bit during completion */
+#define SET_DSC (1u << 8)
+
/* See ACS-2 T13/2015-D Table B.2 Command codes */
-static const uint8_t ide_cmd_table[0x100] = {
+static const struct {
+ /* Returns true if the completion code should be run */
+ bool (*handler)(IDEState *s, uint8_t cmd);
+ int flags;
+} ide_cmd_table[0x100] = {
/* NOP not implemented, mandatory for CD */
- [CFA_REQ_EXT_ERROR_CODE] = CFA_OK,
- [WIN_DSM] = ALL_OK,
- [WIN_DEVICE_RESET] = CD_OK,
- [WIN_RECAL] = HD_CFA_OK,
- [WIN_READ] = ALL_OK,
- [WIN_READ_ONCE] = ALL_OK,
- [WIN_READ_EXT] = HD_CFA_OK,
- [WIN_READDMA_EXT] = HD_CFA_OK,
- [WIN_READ_NATIVE_MAX_EXT] = HD_CFA_OK,
- [WIN_MULTREAD_EXT] = HD_CFA_OK,
- [WIN_WRITE] = HD_CFA_OK,
- [WIN_WRITE_ONCE] = HD_CFA_OK,
- [WIN_WRITE_EXT] = HD_CFA_OK,
- [WIN_WRITEDMA_EXT] = HD_CFA_OK,
- [CFA_WRITE_SECT_WO_ERASE] = CFA_OK,
- [WIN_MULTWRITE_EXT] = HD_CFA_OK,
- [WIN_WRITE_VERIFY] = HD_CFA_OK,
- [WIN_VERIFY] = HD_CFA_OK,
- [WIN_VERIFY_ONCE] = HD_CFA_OK,
- [WIN_VERIFY_EXT] = HD_CFA_OK,
- [WIN_SEEK] = HD_CFA_OK,
- [CFA_TRANSLATE_SECTOR] = CFA_OK,
- [WIN_DIAGNOSE] = ALL_OK,
- [WIN_SPECIFY] = HD_CFA_OK,
- [WIN_STANDBYNOW2] = ALL_OK,
- [WIN_IDLEIMMEDIATE2] = ALL_OK,
- [WIN_STANDBY2] = ALL_OK,
- [WIN_SETIDLE2] = ALL_OK,
- [WIN_CHECKPOWERMODE2] = ALL_OK,
- [WIN_SLEEPNOW2] = ALL_OK,
- [WIN_PACKETCMD] = CD_OK,
- [WIN_PIDENTIFY] = CD_OK,
- [WIN_SMART] = HD_CFA_OK,
- [CFA_ACCESS_METADATA_STORAGE] = CFA_OK,
- [CFA_ERASE_SECTORS] = CFA_OK,
- [WIN_MULTREAD] = HD_CFA_OK,
- [WIN_MULTWRITE] = HD_CFA_OK,
- [WIN_SETMULT] = HD_CFA_OK,
- [WIN_READDMA] = HD_CFA_OK,
- [WIN_READDMA_ONCE] = HD_CFA_OK,
- [WIN_WRITEDMA] = HD_CFA_OK,
- [WIN_WRITEDMA_ONCE] = HD_CFA_OK,
- [CFA_WRITE_MULTI_WO_ERASE] = CFA_OK,
- [WIN_STANDBYNOW1] = ALL_OK,
- [WIN_IDLEIMMEDIATE] = ALL_OK,
- [WIN_STANDBY] = ALL_OK,
- [WIN_SETIDLE1] = ALL_OK,
- [WIN_CHECKPOWERMODE1] = ALL_OK,
- [WIN_SLEEPNOW1] = ALL_OK,
- [WIN_FLUSH_CACHE] = ALL_OK,
- [WIN_FLUSH_CACHE_EXT] = HD_CFA_OK,
- [WIN_IDENTIFY] = ALL_OK,
- [WIN_SETFEATURES] = ALL_OK,
- [IBM_SENSE_CONDITION] = CFA_OK,
- [CFA_WEAR_LEVEL] = HD_CFA_OK,
- [WIN_READ_NATIVE_MAX] = ALL_OK,
+ [CFA_REQ_EXT_ERROR_CODE] = { NULL, CFA_OK },
+ [WIN_DSM] = { NULL, ALL_OK },
+ [WIN_DEVICE_RESET] = { NULL, CD_OK },
+ [WIN_RECAL] = { NULL, HD_CFA_OK },
+ [WIN_READ] = { NULL, ALL_OK },
+ [WIN_READ_ONCE] = { NULL, ALL_OK },
+ [WIN_READ_EXT] = { NULL, HD_CFA_OK },
+ [WIN_READDMA_EXT] = { NULL, HD_CFA_OK },
+ [WIN_READ_NATIVE_MAX_EXT] = { NULL, HD_CFA_OK },
+ [WIN_MULTREAD_EXT] = { NULL, HD_CFA_OK },
+ [WIN_WRITE] = { NULL, HD_CFA_OK },
+ [WIN_WRITE_ONCE] = { NULL, HD_CFA_OK },
+ [WIN_WRITE_EXT] = { NULL, HD_CFA_OK },
+ [WIN_WRITEDMA_EXT] = { NULL, HD_CFA_OK },
+ [CFA_WRITE_SECT_WO_ERASE] = { NULL, CFA_OK },
+ [WIN_MULTWRITE_EXT] = { NULL, HD_CFA_OK },
+ [WIN_WRITE_VERIFY] = { NULL, HD_CFA_OK },
+ [WIN_VERIFY] = { NULL, HD_CFA_OK },
+ [WIN_VERIFY_ONCE] = { NULL, HD_CFA_OK },
+ [WIN_VERIFY_EXT] = { NULL, HD_CFA_OK },
+ [WIN_SEEK] = { NULL, HD_CFA_OK },
+ [CFA_TRANSLATE_SECTOR] = { NULL, CFA_OK },
+ [WIN_DIAGNOSE] = { NULL, ALL_OK },
+ [WIN_SPECIFY] = { NULL, HD_CFA_OK },
+ [WIN_STANDBYNOW2] = { NULL, ALL_OK },
+ [WIN_IDLEIMMEDIATE2] = { NULL, ALL_OK },
+ [WIN_STANDBY2] = { NULL, ALL_OK },
+ [WIN_SETIDLE2] = { NULL, ALL_OK },
+ [WIN_CHECKPOWERMODE2] = { NULL, ALL_OK },
+ [WIN_SLEEPNOW2] = { NULL, ALL_OK },
+ [WIN_PACKETCMD] = { NULL, CD_OK },
+ [WIN_PIDENTIFY] = { NULL, CD_OK },
+ [WIN_SMART] = { NULL, HD_CFA_OK },
+ [CFA_ACCESS_METADATA_STORAGE] = { NULL, CFA_OK },
+ [CFA_ERASE_SECTORS] = { NULL, CFA_OK },
+ [WIN_MULTREAD] = { NULL, HD_CFA_OK },
+ [WIN_MULTWRITE] = { NULL, HD_CFA_OK },
+ [WIN_SETMULT] = { NULL, HD_CFA_OK },
+ [WIN_READDMA] = { NULL, HD_CFA_OK },
+ [WIN_READDMA_ONCE] = { NULL, HD_CFA_OK },
+ [WIN_WRITEDMA] = { NULL, HD_CFA_OK },
+ [WIN_WRITEDMA_ONCE] = { NULL, HD_CFA_OK },
+ [CFA_WRITE_MULTI_WO_ERASE] = { NULL, CFA_OK },
+ [WIN_STANDBYNOW1] = { NULL, ALL_OK },
+ [WIN_IDLEIMMEDIATE] = { NULL, ALL_OK },
+ [WIN_STANDBY] = { NULL, ALL_OK },
+ [WIN_SETIDLE1] = { NULL, ALL_OK },
+ [WIN_CHECKPOWERMODE1] = { NULL, ALL_OK },
+ [WIN_SLEEPNOW1] = { NULL, ALL_OK },
+ [WIN_FLUSH_CACHE] = { NULL, ALL_OK },
+ [WIN_FLUSH_CACHE_EXT] = { NULL, HD_CFA_OK },
+ [WIN_IDENTIFY] = { NULL, ALL_OK },
+ [WIN_SETFEATURES] = { NULL, ALL_OK },
+ [IBM_SENSE_CONDITION] = { NULL, CFA_OK },
+ [CFA_WEAR_LEVEL] = { NULL, HD_CFA_OK },
+ [WIN_READ_NATIVE_MAX] = { NULL, ALL_OK },
};
static bool ide_cmd_permitted(IDEState *s, uint32_t cmd)
{
return cmd < ARRAY_SIZE(ide_cmd_table)
- && (ide_cmd_table[cmd] & (1u << s->drive_kind));
+ && (ide_cmd_table[cmd].flags & (1u << s->drive_kind));
}
void ide_exec_cmd(IDEBus *bus, uint32_t val)
@@ -1100,6 +1107,27 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
goto abort_cmd;
}
+ if (ide_cmd_table[val].handler != NULL) {
+ bool complete;
+
+ s->status = READY_STAT | BUSY_STAT;
+ s->error = 0;
+
+ complete = ide_cmd_table[val].handler(s, val);
+ if (complete) {
+ s->status &= ~BUSY_STAT;
+ assert(!!s->error == !!(s->status & ERR_STAT));
+
+ if ((ide_cmd_table[val].flags & SET_DSC) && !s->error) {
+ s->status |= SEEK_STAT;
+ }
+
+ ide_set_irq(s->bus);
+ }
+
+ return;
+ }
+
switch(val) {
case WIN_DSM:
switch (s->feature) {
--
1.8.1.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [Qemu-devel] [PATCH 02/23] ide: Convert WIN_DSM to ide_cmd_table handler
2013-06-24 9:10 [Qemu-devel] [PULL 00/23] Block patches Stefan Hajnoczi
2013-06-24 9:10 ` [Qemu-devel] [PATCH 01/23] ide: Add handler to ide_cmd_table Stefan Hajnoczi
@ 2013-06-24 9:10 ` Stefan Hajnoczi
2013-06-24 9:10 ` [Qemu-devel] [PATCH 03/23] ide: Convert WIN_IDENTIFY " Stefan Hajnoczi
` (20 subsequent siblings)
22 siblings, 0 replies; 35+ messages in thread
From: Stefan Hajnoczi @ 2013-06-24 9:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi
From: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/ide/core.c | 29 ++++++++++++++++-------------
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index cd9de14..567515e 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1004,6 +1004,21 @@ void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val)
}
}
+static bool cmd_data_set_management(IDEState *s, uint8_t cmd)
+{
+ switch (s->feature) {
+ case DSM_TRIM:
+ if (s->bs) {
+ ide_sector_start_dma(s, IDE_DMA_TRIM);
+ return false;
+ }
+ break;
+ }
+
+ ide_abort_command(s);
+ return true;
+}
+
#define HD_OK (1u << IDE_HD)
#define CD_OK (1u << IDE_CD)
#define CFA_OK (1u << IDE_CFATA)
@@ -1021,7 +1036,7 @@ static const struct {
} ide_cmd_table[0x100] = {
/* NOP not implemented, mandatory for CD */
[CFA_REQ_EXT_ERROR_CODE] = { NULL, CFA_OK },
- [WIN_DSM] = { NULL, ALL_OK },
+ [WIN_DSM] = { cmd_data_set_management, ALL_OK },
[WIN_DEVICE_RESET] = { NULL, CD_OK },
[WIN_RECAL] = { NULL, HD_CFA_OK },
[WIN_READ] = { NULL, ALL_OK },
@@ -1129,18 +1144,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
}
switch(val) {
- case WIN_DSM:
- switch (s->feature) {
- case DSM_TRIM:
- if (!s->bs) {
- goto abort_cmd;
- }
- ide_sector_start_dma(s, IDE_DMA_TRIM);
- break;
- default:
- goto abort_cmd;
- }
- break;
case WIN_IDENTIFY:
if (s->bs && s->drive_kind != IDE_CD) {
if (s->drive_kind != IDE_CFATA)
--
1.8.1.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [Qemu-devel] [PATCH 03/23] ide: Convert WIN_IDENTIFY to ide_cmd_table handler
2013-06-24 9:10 [Qemu-devel] [PULL 00/23] Block patches Stefan Hajnoczi
2013-06-24 9:10 ` [Qemu-devel] [PATCH 01/23] ide: Add handler to ide_cmd_table Stefan Hajnoczi
2013-06-24 9:10 ` [Qemu-devel] [PATCH 02/23] ide: Convert WIN_DSM to ide_cmd_table handler Stefan Hajnoczi
@ 2013-06-24 9:10 ` Stefan Hajnoczi
2013-06-24 9:10 ` [Qemu-devel] [PATCH 04/23] ide: Convert cmd_nop commands " Stefan Hajnoczi
` (19 subsequent siblings)
22 siblings, 0 replies; 35+ messages in thread
From: Stefan Hajnoczi @ 2013-06-24 9:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi
From: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/ide/core.c | 40 +++++++++++++++++++++++-----------------
1 file changed, 23 insertions(+), 17 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 567515e..2df078b 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1019,6 +1019,28 @@ static bool cmd_data_set_management(IDEState *s, uint8_t cmd)
return true;
}
+static bool cmd_identify(IDEState *s, uint8_t cmd)
+{
+ 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);
+ ide_set_irq(s->bus);
+ return false;
+ } else {
+ if (s->drive_kind == IDE_CD) {
+ ide_set_signature(s);
+ }
+ ide_abort_command(s);
+ }
+
+ return true;
+}
+
#define HD_OK (1u << IDE_HD)
#define CD_OK (1u << IDE_CD)
#define CFA_OK (1u << IDE_CFATA)
@@ -1086,7 +1108,7 @@ static const struct {
[WIN_SLEEPNOW1] = { NULL, ALL_OK },
[WIN_FLUSH_CACHE] = { NULL, ALL_OK },
[WIN_FLUSH_CACHE_EXT] = { NULL, HD_CFA_OK },
- [WIN_IDENTIFY] = { NULL, ALL_OK },
+ [WIN_IDENTIFY] = { cmd_identify, ALL_OK },
[WIN_SETFEATURES] = { NULL, ALL_OK },
[IBM_SENSE_CONDITION] = { NULL, CFA_OK },
[CFA_WEAR_LEVEL] = { NULL, HD_CFA_OK },
@@ -1144,22 +1166,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
}
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;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [Qemu-devel] [PATCH 04/23] ide: Convert cmd_nop commands to ide_cmd_table handler
2013-06-24 9:10 [Qemu-devel] [PULL 00/23] Block patches Stefan Hajnoczi
` (2 preceding siblings ...)
2013-06-24 9:10 ` [Qemu-devel] [PATCH 03/23] ide: Convert WIN_IDENTIFY " Stefan Hajnoczi
@ 2013-06-24 9:10 ` Stefan Hajnoczi
2013-06-24 9:10 ` [Qemu-devel] [PATCH 05/23] ide: Convert verify " Stefan Hajnoczi
` (18 subsequent siblings)
22 siblings, 0 replies; 35+ messages in thread
From: Stefan Hajnoczi @ 2013-06-24 9:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi
From: Kevin Wolf <kwolf@redhat.com>
cmd_nop handles all commands that don't really do anything in our
implementation except setting status register flags.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/ide/core.c | 48 +++++++++++++++++-------------------------------
1 file changed, 17 insertions(+), 31 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 2df078b..057662d 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1004,6 +1004,11 @@ void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val)
}
}
+static bool cmd_nop(IDEState *s, uint8_t cmd)
+{
+ return true;
+}
+
static bool cmd_data_set_management(IDEState *s, uint8_t cmd)
{
switch (s->feature) {
@@ -1060,7 +1065,7 @@ static const struct {
[CFA_REQ_EXT_ERROR_CODE] = { NULL, CFA_OK },
[WIN_DSM] = { cmd_data_set_management, ALL_OK },
[WIN_DEVICE_RESET] = { NULL, CD_OK },
- [WIN_RECAL] = { NULL, HD_CFA_OK },
+ [WIN_RECAL] = { cmd_nop, HD_CFA_OK | SET_DSC},
[WIN_READ] = { NULL, ALL_OK },
[WIN_READ_ONCE] = { NULL, ALL_OK },
[WIN_READ_EXT] = { NULL, HD_CFA_OK },
@@ -1080,13 +1085,13 @@ static const struct {
[WIN_SEEK] = { NULL, HD_CFA_OK },
[CFA_TRANSLATE_SECTOR] = { NULL, CFA_OK },
[WIN_DIAGNOSE] = { NULL, ALL_OK },
- [WIN_SPECIFY] = { NULL, HD_CFA_OK },
- [WIN_STANDBYNOW2] = { NULL, ALL_OK },
- [WIN_IDLEIMMEDIATE2] = { NULL, ALL_OK },
- [WIN_STANDBY2] = { NULL, ALL_OK },
- [WIN_SETIDLE2] = { NULL, ALL_OK },
+ [WIN_SPECIFY] = { cmd_nop, HD_CFA_OK | SET_DSC },
+ [WIN_STANDBYNOW2] = { cmd_nop, ALL_OK },
+ [WIN_IDLEIMMEDIATE2] = { cmd_nop, ALL_OK },
+ [WIN_STANDBY2] = { cmd_nop, ALL_OK },
+ [WIN_SETIDLE2] = { cmd_nop, ALL_OK },
[WIN_CHECKPOWERMODE2] = { NULL, ALL_OK },
- [WIN_SLEEPNOW2] = { NULL, ALL_OK },
+ [WIN_SLEEPNOW2] = { cmd_nop, ALL_OK },
[WIN_PACKETCMD] = { NULL, CD_OK },
[WIN_PIDENTIFY] = { NULL, CD_OK },
[WIN_SMART] = { NULL, HD_CFA_OK },
@@ -1100,12 +1105,12 @@ static const struct {
[WIN_WRITEDMA] = { NULL, HD_CFA_OK },
[WIN_WRITEDMA_ONCE] = { NULL, HD_CFA_OK },
[CFA_WRITE_MULTI_WO_ERASE] = { NULL, CFA_OK },
- [WIN_STANDBYNOW1] = { NULL, ALL_OK },
- [WIN_IDLEIMMEDIATE] = { NULL, ALL_OK },
- [WIN_STANDBY] = { NULL, ALL_OK },
- [WIN_SETIDLE1] = { NULL, ALL_OK },
+ [WIN_STANDBYNOW1] = { cmd_nop, ALL_OK },
+ [WIN_IDLEIMMEDIATE] = { cmd_nop, ALL_OK },
+ [WIN_STANDBY] = { cmd_nop, ALL_OK },
+ [WIN_SETIDLE1] = { cmd_nop, ALL_OK },
[WIN_CHECKPOWERMODE1] = { NULL, ALL_OK },
- [WIN_SLEEPNOW1] = { NULL, ALL_OK },
+ [WIN_SLEEPNOW1] = { cmd_nop, ALL_OK },
[WIN_FLUSH_CACHE] = { NULL, ALL_OK },
[WIN_FLUSH_CACHE_EXT] = { NULL, HD_CFA_OK },
[WIN_IDENTIFY] = { cmd_identify, ALL_OK },
@@ -1166,12 +1171,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
}
switch(val) {
- 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 */
@@ -1391,19 +1390,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
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 WIN_IDLEIMMEDIATE2:
- 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:
/* XXX: Check that seek is within bounds */
s->status = READY_STAT | SEEK_STAT;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [Qemu-devel] [PATCH 05/23] ide: Convert verify commands to ide_cmd_table handler
2013-06-24 9:10 [Qemu-devel] [PULL 00/23] Block patches Stefan Hajnoczi
` (3 preceding siblings ...)
2013-06-24 9:10 ` [Qemu-devel] [PATCH 04/23] ide: Convert cmd_nop commands " Stefan Hajnoczi
@ 2013-06-24 9:10 ` Stefan Hajnoczi
2013-06-24 9:10 ` [Qemu-devel] [PATCH 06/23] ide: Convert read/write multiple " Stefan Hajnoczi
` (17 subsequent siblings)
22 siblings, 0 replies; 35+ messages in thread
From: Stefan Hajnoczi @ 2013-06-24 9:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi
From: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/ide/core.c | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 057662d..bf2007a 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1046,6 +1046,16 @@ static bool cmd_identify(IDEState *s, uint8_t cmd)
return true;
}
+static bool cmd_verify(IDEState *s, uint8_t cmd)
+{
+ bool lba48 = (cmd == WIN_VERIFY_EXT);
+
+ /* do sector number check ? */
+ ide_cmd_lba48_transform(s, lba48);
+
+ return true;
+}
+
#define HD_OK (1u << IDE_HD)
#define CD_OK (1u << IDE_CD)
#define CFA_OK (1u << IDE_CFATA)
@@ -1079,9 +1089,9 @@ static const struct {
[CFA_WRITE_SECT_WO_ERASE] = { NULL, CFA_OK },
[WIN_MULTWRITE_EXT] = { NULL, HD_CFA_OK },
[WIN_WRITE_VERIFY] = { NULL, HD_CFA_OK },
- [WIN_VERIFY] = { NULL, HD_CFA_OK },
- [WIN_VERIFY_ONCE] = { NULL, HD_CFA_OK },
- [WIN_VERIFY_EXT] = { NULL, HD_CFA_OK },
+ [WIN_VERIFY] = { cmd_verify, HD_CFA_OK | SET_DSC },
+ [WIN_VERIFY_ONCE] = { cmd_verify, HD_CFA_OK | SET_DSC },
+ [WIN_VERIFY_EXT] = { cmd_verify, HD_CFA_OK | SET_DSC },
[WIN_SEEK] = { NULL, HD_CFA_OK },
[CFA_TRANSLATE_SECTOR] = { NULL, CFA_OK },
[WIN_DIAGNOSE] = { NULL, ALL_OK },
@@ -1187,17 +1197,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
ide_set_irq(s->bus);
break;
- case WIN_VERIFY_EXT:
- lba48 = 1;
- /* fall through */
- 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;
/* fall through */
--
1.8.1.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [Qemu-devel] [PATCH 06/23] ide: Convert read/write multiple commands to ide_cmd_table handler
2013-06-24 9:10 [Qemu-devel] [PULL 00/23] Block patches Stefan Hajnoczi
` (4 preceding siblings ...)
2013-06-24 9:10 ` [Qemu-devel] [PATCH 05/23] ide: Convert verify " Stefan Hajnoczi
@ 2013-06-24 9:10 ` Stefan Hajnoczi
2013-06-24 9:10 ` [Qemu-devel] [PATCH 07/23] ide: Convert PIO read/write " Stefan Hajnoczi
` (16 subsequent siblings)
22 siblings, 0 replies; 35+ messages in thread
From: Stefan Hajnoczi @ 2013-06-24 9:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi
From: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/ide/core.c | 119 +++++++++++++++++++++++++++++-----------------------------
1 file changed, 60 insertions(+), 59 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index bf2007a..e6cd7b8 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1056,6 +1056,60 @@ static bool cmd_verify(IDEState *s, uint8_t cmd)
return true;
}
+static bool cmd_set_multiple_mode(IDEState *s, uint8_t cmd)
+{
+ if (s->drive_kind == IDE_CFATA && s->nsector == 0) {
+ /* Disable Read and Write Multiple */
+ s->mult_sectors = 0;
+ } 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;
+ }
+
+ return true;
+}
+
+static bool cmd_read_multiple(IDEState *s, uint8_t cmd)
+{
+ bool lba48 = (cmd == WIN_MULTREAD_EXT);
+
+ if (!s->bs || !s->mult_sectors) {
+ ide_abort_command(s);
+ return true;
+ }
+
+ ide_cmd_lba48_transform(s, lba48);
+ s->req_nb_sectors = s->mult_sectors;
+ ide_sector_read(s);
+ return false;
+}
+
+static bool cmd_write_multiple(IDEState *s, uint8_t cmd)
+{
+ bool lba48 = (cmd == WIN_MULTWRITE_EXT);
+ int n;
+
+ if (!s->bs || !s->mult_sectors) {
+ ide_abort_command(s);
+ return true;
+ }
+
+ ide_cmd_lba48_transform(s, lba48);
+
+ s->req_nb_sectors = s->mult_sectors;
+ n = MIN(s->nsector, s->req_nb_sectors);
+
+ s->status = SEEK_STAT | READY_STAT;
+ ide_transfer_start(s, s->io_buffer, 512 * n, ide_sector_write);
+
+ s->media_changed = 1;
+
+ return false;
+}
+
#define HD_OK (1u << IDE_HD)
#define CD_OK (1u << IDE_CD)
#define CFA_OK (1u << IDE_CFATA)
@@ -1081,13 +1135,13 @@ static const struct {
[WIN_READ_EXT] = { NULL, HD_CFA_OK },
[WIN_READDMA_EXT] = { NULL, HD_CFA_OK },
[WIN_READ_NATIVE_MAX_EXT] = { NULL, HD_CFA_OK },
- [WIN_MULTREAD_EXT] = { NULL, HD_CFA_OK },
+ [WIN_MULTREAD_EXT] = { cmd_read_multiple, HD_CFA_OK },
[WIN_WRITE] = { NULL, HD_CFA_OK },
[WIN_WRITE_ONCE] = { NULL, HD_CFA_OK },
[WIN_WRITE_EXT] = { NULL, HD_CFA_OK },
[WIN_WRITEDMA_EXT] = { NULL, HD_CFA_OK },
[CFA_WRITE_SECT_WO_ERASE] = { NULL, CFA_OK },
- [WIN_MULTWRITE_EXT] = { NULL, HD_CFA_OK },
+ [WIN_MULTWRITE_EXT] = { cmd_write_multiple, HD_CFA_OK },
[WIN_WRITE_VERIFY] = { NULL, HD_CFA_OK },
[WIN_VERIFY] = { cmd_verify, HD_CFA_OK | SET_DSC },
[WIN_VERIFY_ONCE] = { cmd_verify, HD_CFA_OK | SET_DSC },
@@ -1107,14 +1161,14 @@ static const struct {
[WIN_SMART] = { NULL, HD_CFA_OK },
[CFA_ACCESS_METADATA_STORAGE] = { NULL, CFA_OK },
[CFA_ERASE_SECTORS] = { NULL, CFA_OK },
- [WIN_MULTREAD] = { NULL, HD_CFA_OK },
- [WIN_MULTWRITE] = { NULL, HD_CFA_OK },
- [WIN_SETMULT] = { NULL, HD_CFA_OK },
+ [WIN_MULTREAD] = { cmd_read_multiple, HD_CFA_OK },
+ [WIN_MULTWRITE] = { cmd_write_multiple, HD_CFA_OK },
+ [WIN_SETMULT] = { cmd_set_multiple_mode, HD_CFA_OK | SET_DSC },
[WIN_READDMA] = { NULL, HD_CFA_OK },
[WIN_READDMA_ONCE] = { NULL, HD_CFA_OK },
[WIN_WRITEDMA] = { NULL, HD_CFA_OK },
[WIN_WRITEDMA_ONCE] = { NULL, HD_CFA_OK },
- [CFA_WRITE_MULTI_WO_ERASE] = { NULL, CFA_OK },
+ [CFA_WRITE_MULTI_WO_ERASE] = { cmd_write_multiple, CFA_OK },
[WIN_STANDBYNOW1] = { cmd_nop, ALL_OK },
[WIN_IDLEIMMEDIATE] = { cmd_nop, ALL_OK },
[WIN_STANDBY] = { cmd_nop, ALL_OK },
@@ -1181,22 +1235,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
}
switch(val) {
- 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_set_irq(s->bus);
- break;
-
case WIN_READ_EXT:
lba48 = 1;
/* fall through */
@@ -1232,43 +1270,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
s->media_changed = 1;
break;
- case WIN_MULTREAD_EXT:
- lba48 = 1;
- /* fall through */
- case WIN_MULTREAD:
- if (!s->bs) {
- goto abort_cmd;
- }
- 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;
- /* fall through */
- case WIN_MULTWRITE:
- case CFA_WRITE_MULTI_WO_ERASE:
- if (!s->bs) {
- goto abort_cmd;
- }
- 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;
/* fall through */
--
1.8.1.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [Qemu-devel] [PATCH 07/23] ide: Convert PIO read/write commands to ide_cmd_table handler
2013-06-24 9:10 [Qemu-devel] [PULL 00/23] Block patches Stefan Hajnoczi
` (5 preceding siblings ...)
2013-06-24 9:10 ` [Qemu-devel] [PATCH 06/23] ide: Convert read/write multiple " Stefan Hajnoczi
@ 2013-06-24 9:10 ` Stefan Hajnoczi
2013-06-24 9:10 ` [Qemu-devel] [PATCH 08/23] ide: Convert DMA " Stefan Hajnoczi
` (15 subsequent siblings)
22 siblings, 0 replies; 35+ messages in thread
From: Stefan Hajnoczi @ 2013-06-24 9:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi
From: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/ide/core.c | 93 ++++++++++++++++++++++++++++++++---------------------------
1 file changed, 50 insertions(+), 43 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index e6cd7b8..86af4b0 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1110,6 +1110,48 @@ static bool cmd_write_multiple(IDEState *s, uint8_t cmd)
return false;
}
+static bool cmd_read_pio(IDEState *s, uint8_t cmd)
+{
+ bool lba48 = (cmd == WIN_READ_EXT);
+
+ if (s->drive_kind == IDE_CD) {
+ ide_set_signature(s); /* odd, but ATA4 8.27.5.2 requires it */
+ ide_abort_command(s);
+ return true;
+ }
+
+ if (!s->bs) {
+ ide_abort_command(s);
+ return true;
+ }
+
+ ide_cmd_lba48_transform(s, lba48);
+ s->req_nb_sectors = 1;
+ ide_sector_read(s);
+
+ return false;
+}
+
+static bool cmd_write_pio(IDEState *s, uint8_t cmd)
+{
+ bool lba48 = (cmd == WIN_WRITE_EXT);
+
+ if (!s->bs) {
+ ide_abort_command(s);
+ return true;
+ }
+
+ ide_cmd_lba48_transform(s, lba48);
+
+ s->req_nb_sectors = 1;
+ s->status = SEEK_STAT | READY_STAT;
+ ide_transfer_start(s, s->io_buffer, 512, ide_sector_write);
+
+ s->media_changed = 1;
+
+ return false;
+}
+
#define HD_OK (1u << IDE_HD)
#define CD_OK (1u << IDE_CD)
#define CFA_OK (1u << IDE_CFATA)
@@ -1130,19 +1172,19 @@ static const struct {
[WIN_DSM] = { cmd_data_set_management, ALL_OK },
[WIN_DEVICE_RESET] = { NULL, CD_OK },
[WIN_RECAL] = { cmd_nop, HD_CFA_OK | SET_DSC},
- [WIN_READ] = { NULL, ALL_OK },
- [WIN_READ_ONCE] = { NULL, ALL_OK },
- [WIN_READ_EXT] = { NULL, HD_CFA_OK },
+ [WIN_READ] = { cmd_read_pio, ALL_OK },
+ [WIN_READ_ONCE] = { cmd_read_pio, ALL_OK },
+ [WIN_READ_EXT] = { cmd_read_pio, HD_CFA_OK },
[WIN_READDMA_EXT] = { NULL, HD_CFA_OK },
[WIN_READ_NATIVE_MAX_EXT] = { NULL, HD_CFA_OK },
[WIN_MULTREAD_EXT] = { cmd_read_multiple, HD_CFA_OK },
- [WIN_WRITE] = { NULL, HD_CFA_OK },
- [WIN_WRITE_ONCE] = { NULL, HD_CFA_OK },
- [WIN_WRITE_EXT] = { NULL, HD_CFA_OK },
+ [WIN_WRITE] = { cmd_write_pio, HD_CFA_OK },
+ [WIN_WRITE_ONCE] = { cmd_write_pio, HD_CFA_OK },
+ [WIN_WRITE_EXT] = { cmd_write_pio, HD_CFA_OK },
[WIN_WRITEDMA_EXT] = { NULL, HD_CFA_OK },
- [CFA_WRITE_SECT_WO_ERASE] = { NULL, CFA_OK },
+ [CFA_WRITE_SECT_WO_ERASE] = { cmd_write_pio, CFA_OK },
[WIN_MULTWRITE_EXT] = { cmd_write_multiple, HD_CFA_OK },
- [WIN_WRITE_VERIFY] = { NULL, HD_CFA_OK },
+ [WIN_WRITE_VERIFY] = { cmd_write_pio, HD_CFA_OK },
[WIN_VERIFY] = { cmd_verify, HD_CFA_OK | SET_DSC },
[WIN_VERIFY_ONCE] = { cmd_verify, HD_CFA_OK | SET_DSC },
[WIN_VERIFY_EXT] = { cmd_verify, HD_CFA_OK | SET_DSC },
@@ -1235,41 +1277,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
}
switch(val) {
- case WIN_READ_EXT:
- lba48 = 1;
- /* fall through */
- case WIN_READ:
- case WIN_READ_ONCE:
- if (s->drive_kind == IDE_CD) {
- ide_set_signature(s); /* odd, but ATA4 8.27.5.2 requires it */
- goto abort_cmd;
- }
- 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;
- /* fall through */
- case WIN_WRITE:
- case WIN_WRITE_ONCE:
- case CFA_WRITE_SECT_WO_ERASE:
- case WIN_WRITE_VERIFY:
- if (!s->bs) {
- goto abort_cmd;
- }
- 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_READDMA_EXT:
lba48 = 1;
/* fall through */
--
1.8.1.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [Qemu-devel] [PATCH 08/23] ide: Convert DMA read/write commands to ide_cmd_table handler
2013-06-24 9:10 [Qemu-devel] [PULL 00/23] Block patches Stefan Hajnoczi
` (6 preceding siblings ...)
2013-06-24 9:10 ` [Qemu-devel] [PATCH 07/23] ide: Convert PIO read/write " Stefan Hajnoczi
@ 2013-06-24 9:10 ` Stefan Hajnoczi
2013-06-24 9:10 ` [Qemu-devel] [PATCH 09/23] ide: Convert READ NATIVE MAX ADDRESS " Stefan Hajnoczi
` (14 subsequent siblings)
22 siblings, 0 replies; 35+ messages in thread
From: Stefan Hajnoczi @ 2013-06-24 9:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi
From: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/ide/core.c | 69 ++++++++++++++++++++++++++++++++---------------------------
1 file changed, 38 insertions(+), 31 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 86af4b0..2c8a0ff 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1152,6 +1152,38 @@ static bool cmd_write_pio(IDEState *s, uint8_t cmd)
return false;
}
+static bool cmd_read_dma(IDEState *s, uint8_t cmd)
+{
+ bool lba48 = (cmd == WIN_READDMA_EXT);
+
+ if (!s->bs) {
+ ide_abort_command(s);
+ return true;
+ }
+
+ ide_cmd_lba48_transform(s, lba48);
+ ide_sector_start_dma(s, IDE_DMA_READ);
+
+ return false;
+}
+
+static bool cmd_write_dma(IDEState *s, uint8_t cmd)
+{
+ bool lba48 = (cmd == WIN_WRITEDMA_EXT);
+
+ if (!s->bs) {
+ ide_abort_command(s);
+ return true;
+ }
+
+ ide_cmd_lba48_transform(s, lba48);
+ ide_sector_start_dma(s, IDE_DMA_WRITE);
+
+ s->media_changed = 1;
+
+ return false;
+}
+
#define HD_OK (1u << IDE_HD)
#define CD_OK (1u << IDE_CD)
#define CFA_OK (1u << IDE_CFATA)
@@ -1175,13 +1207,13 @@ static const struct {
[WIN_READ] = { cmd_read_pio, ALL_OK },
[WIN_READ_ONCE] = { cmd_read_pio, ALL_OK },
[WIN_READ_EXT] = { cmd_read_pio, HD_CFA_OK },
- [WIN_READDMA_EXT] = { NULL, HD_CFA_OK },
+ [WIN_READDMA_EXT] = { cmd_read_dma, HD_CFA_OK },
[WIN_READ_NATIVE_MAX_EXT] = { NULL, HD_CFA_OK },
[WIN_MULTREAD_EXT] = { cmd_read_multiple, HD_CFA_OK },
[WIN_WRITE] = { cmd_write_pio, HD_CFA_OK },
[WIN_WRITE_ONCE] = { cmd_write_pio, HD_CFA_OK },
[WIN_WRITE_EXT] = { cmd_write_pio, HD_CFA_OK },
- [WIN_WRITEDMA_EXT] = { NULL, HD_CFA_OK },
+ [WIN_WRITEDMA_EXT] = { cmd_write_dma, HD_CFA_OK },
[CFA_WRITE_SECT_WO_ERASE] = { cmd_write_pio, CFA_OK },
[WIN_MULTWRITE_EXT] = { cmd_write_multiple, HD_CFA_OK },
[WIN_WRITE_VERIFY] = { cmd_write_pio, HD_CFA_OK },
@@ -1206,10 +1238,10 @@ static const struct {
[WIN_MULTREAD] = { cmd_read_multiple, HD_CFA_OK },
[WIN_MULTWRITE] = { cmd_write_multiple, HD_CFA_OK },
[WIN_SETMULT] = { cmd_set_multiple_mode, HD_CFA_OK | SET_DSC },
- [WIN_READDMA] = { NULL, HD_CFA_OK },
- [WIN_READDMA_ONCE] = { NULL, HD_CFA_OK },
- [WIN_WRITEDMA] = { NULL, HD_CFA_OK },
- [WIN_WRITEDMA_ONCE] = { NULL, HD_CFA_OK },
+ [WIN_READDMA] = { cmd_read_dma, HD_CFA_OK },
+ [WIN_READDMA_ONCE] = { cmd_read_dma, HD_CFA_OK },
+ [WIN_WRITEDMA] = { cmd_write_dma, HD_CFA_OK },
+ [WIN_WRITEDMA_ONCE] = { cmd_write_dma, HD_CFA_OK },
[CFA_WRITE_MULTI_WO_ERASE] = { cmd_write_multiple, CFA_OK },
[WIN_STANDBYNOW1] = { cmd_nop, ALL_OK },
[WIN_IDLEIMMEDIATE] = { cmd_nop, ALL_OK },
@@ -1277,31 +1309,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
}
switch(val) {
- case WIN_READDMA_EXT:
- lba48 = 1;
- /* fall through */
- case WIN_READDMA:
- case WIN_READDMA_ONCE:
- if (!s->bs) {
- goto abort_cmd;
- }
- ide_cmd_lba48_transform(s, lba48);
- ide_sector_start_dma(s, IDE_DMA_READ);
- break;
-
- case WIN_WRITEDMA_EXT:
- lba48 = 1;
- /* fall through */
- case WIN_WRITEDMA:
- case WIN_WRITEDMA_ONCE:
- if (!s->bs) {
- goto abort_cmd;
- }
- ide_cmd_lba48_transform(s, lba48);
- ide_sector_start_dma(s, IDE_DMA_WRITE);
- s->media_changed = 1;
- break;
-
case WIN_READ_NATIVE_MAX_EXT:
lba48 = 1;
/* fall through */
--
1.8.1.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [Qemu-devel] [PATCH 09/23] ide: Convert READ NATIVE MAX ADDRESS to ide_cmd_table handler
2013-06-24 9:10 [Qemu-devel] [PULL 00/23] Block patches Stefan Hajnoczi
` (7 preceding siblings ...)
2013-06-24 9:10 ` [Qemu-devel] [PATCH 08/23] ide: Convert DMA " Stefan Hajnoczi
@ 2013-06-24 9:10 ` Stefan Hajnoczi
2013-06-24 9:10 ` [Qemu-devel] [PATCH 10/23] ide: Convert CHECK POWER MDOE " Stefan Hajnoczi
` (13 subsequent siblings)
22 siblings, 0 replies; 35+ messages in thread
From: Stefan Hajnoczi @ 2013-06-24 9:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi
From: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/ide/core.c | 35 ++++++++++++++++++-----------------
1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 2c8a0ff..3064e2e 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1184,6 +1184,22 @@ static bool cmd_write_dma(IDEState *s, uint8_t cmd)
return false;
}
+static bool cmd_read_native_max(IDEState *s, uint8_t cmd)
+{
+ bool lba48 = (cmd == WIN_READ_NATIVE_MAX_EXT);
+
+ /* Refuse if no sectors are addressable (e.g. medium not inserted) */
+ if (s->nb_sectors == 0) {
+ ide_abort_command(s);
+ return true;
+ }
+
+ ide_cmd_lba48_transform(s, lba48);
+ ide_set_sector(s, s->nb_sectors - 1);
+
+ return true;
+}
+
#define HD_OK (1u << IDE_HD)
#define CD_OK (1u << IDE_CD)
#define CFA_OK (1u << IDE_CFATA)
@@ -1208,7 +1224,7 @@ static const struct {
[WIN_READ_ONCE] = { cmd_read_pio, ALL_OK },
[WIN_READ_EXT] = { cmd_read_pio, HD_CFA_OK },
[WIN_READDMA_EXT] = { cmd_read_dma, HD_CFA_OK },
- [WIN_READ_NATIVE_MAX_EXT] = { NULL, HD_CFA_OK },
+ [WIN_READ_NATIVE_MAX_EXT] = { cmd_read_native_max, HD_CFA_OK | SET_DSC },
[WIN_MULTREAD_EXT] = { cmd_read_multiple, HD_CFA_OK },
[WIN_WRITE] = { cmd_write_pio, HD_CFA_OK },
[WIN_WRITE_ONCE] = { cmd_write_pio, HD_CFA_OK },
@@ -1255,7 +1271,7 @@ static const struct {
[WIN_SETFEATURES] = { NULL, ALL_OK },
[IBM_SENSE_CONDITION] = { NULL, CFA_OK },
[CFA_WEAR_LEVEL] = { NULL, HD_CFA_OK },
- [WIN_READ_NATIVE_MAX] = { NULL, ALL_OK },
+ [WIN_READ_NATIVE_MAX] = { cmd_read_native_max, ALL_OK | SET_DSC },
};
static bool ide_cmd_permitted(IDEState *s, uint32_t cmd)
@@ -1269,7 +1285,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
uint16_t *identify_data;
IDEState *s;
int n;
- int lba48 = 0;
#if defined(DEBUG_IDE)
printf("ide: CMD=%02x\n", val);
@@ -1309,20 +1324,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
}
switch(val) {
- case WIN_READ_NATIVE_MAX_EXT:
- lba48 = 1;
- /* fall through */
- case WIN_READ_NATIVE_MAX:
- /* Refuse if no sectors are addressable (e.g. medium not inserted) */
- if (s->nb_sectors == 0) {
- goto abort_cmd;
- }
- 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->error = 0;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [Qemu-devel] [PATCH 10/23] ide: Convert CHECK POWER MDOE to ide_cmd_table handler
2013-06-24 9:10 [Qemu-devel] [PULL 00/23] Block patches Stefan Hajnoczi
` (8 preceding siblings ...)
2013-06-24 9:10 ` [Qemu-devel] [PATCH 09/23] ide: Convert READ NATIVE MAX ADDRESS " Stefan Hajnoczi
@ 2013-06-24 9:10 ` Stefan Hajnoczi
2013-06-24 9:10 ` [Qemu-devel] [PATCH 11/23] ide: Convert SET FEATURES " Stefan Hajnoczi
` (12 subsequent siblings)
22 siblings, 0 replies; 35+ messages in thread
From: Stefan Hajnoczi @ 2013-06-24 9:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi
From: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/ide/core.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 3064e2e..a7f8445 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1200,6 +1200,12 @@ static bool cmd_read_native_max(IDEState *s, uint8_t cmd)
return true;
}
+static bool cmd_check_power_mode(IDEState *s, uint8_t cmd)
+{
+ s->nsector = 0xff; /* device active or idle */
+ return true;
+}
+
#define HD_OK (1u << IDE_HD)
#define CD_OK (1u << IDE_CD)
#define CFA_OK (1u << IDE_CFATA)
@@ -1244,7 +1250,7 @@ static const struct {
[WIN_IDLEIMMEDIATE2] = { cmd_nop, ALL_OK },
[WIN_STANDBY2] = { cmd_nop, ALL_OK },
[WIN_SETIDLE2] = { cmd_nop, ALL_OK },
- [WIN_CHECKPOWERMODE2] = { NULL, ALL_OK },
+ [WIN_CHECKPOWERMODE2] = { cmd_check_power_mode, ALL_OK | SET_DSC },
[WIN_SLEEPNOW2] = { cmd_nop, ALL_OK },
[WIN_PACKETCMD] = { NULL, CD_OK },
[WIN_PIDENTIFY] = { NULL, CD_OK },
@@ -1263,7 +1269,7 @@ static const struct {
[WIN_IDLEIMMEDIATE] = { cmd_nop, ALL_OK },
[WIN_STANDBY] = { cmd_nop, ALL_OK },
[WIN_SETIDLE1] = { cmd_nop, ALL_OK },
- [WIN_CHECKPOWERMODE1] = { NULL, ALL_OK },
+ [WIN_CHECKPOWERMODE1] = { cmd_check_power_mode, ALL_OK | SET_DSC },
[WIN_SLEEPNOW1] = { cmd_nop, ALL_OK },
[WIN_FLUSH_CACHE] = { NULL, ALL_OK },
[WIN_FLUSH_CACHE_EXT] = { NULL, HD_CFA_OK },
@@ -1324,13 +1330,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
}
switch(val) {
- case WIN_CHECKPOWERMODE1:
- case WIN_CHECKPOWERMODE2:
- s->error = 0;
- 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;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [Qemu-devel] [PATCH 11/23] ide: Convert SET FEATURES to ide_cmd_table handler
2013-06-24 9:10 [Qemu-devel] [PULL 00/23] Block patches Stefan Hajnoczi
` (9 preceding siblings ...)
2013-06-24 9:10 ` [Qemu-devel] [PATCH 10/23] ide: Convert CHECK POWER MDOE " Stefan Hajnoczi
@ 2013-06-24 9:10 ` Stefan Hajnoczi
2013-06-24 9:10 ` [Qemu-devel] [PATCH 12/23] ide: Convert FLUSH CACHE " Stefan Hajnoczi
` (11 subsequent siblings)
22 siblings, 0 replies; 35+ messages in thread
From: Stefan Hajnoczi @ 2013-06-24 9:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi
From: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/ide/core.c | 147 ++++++++++++++++++++++++++++++----------------------------
1 file changed, 75 insertions(+), 72 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index a7f8445..8789758 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1206,6 +1206,80 @@ static bool cmd_check_power_mode(IDEState *s, uint8_t cmd)
return true;
}
+static bool cmd_set_features(IDEState *s, uint8_t cmd)
+{
+ uint16_t *identify_data;
+
+ if (!s->bs) {
+ ide_abort_command(s);
+ return true;
+ }
+
+ /* XXX: valid for CDROM ? */
+ switch (s->feature) {
+ case 0x02: /* write cache enable */
+ bdrv_set_enable_write_cache(s->bs, true);
+ identify_data = (uint16_t *)s->identify_data;
+ put_le16(identify_data + 85, (1 << 14) | (1 << 5) | 1);
+ return true;
+ case 0x82: /* write cache disable */
+ bdrv_set_enable_write_cache(s->bs, false);
+ identify_data = (uint16_t *)s->identify_data;
+ put_le16(identify_data + 85, (1 << 14) | 1);
+ ide_flush_cache(s);
+ return false;
+ case 0xcc: /* reverting to power-on defaults enable */
+ case 0x66: /* reverting to power-on defaults 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 */
+ return true;
+ case 0x03: /* set transfer mode */
+ {
+ uint8_t val = s->nsector & 0x07;
+ identify_data = (uint16_t *)s->identify_data;
+
+ switch (s->nsector >> 3) {
+ 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*/
+ 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 */
+ 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 */
+ put_le16(identify_data + 62, 0x07);
+ put_le16(identify_data + 63, 0x07);
+ put_le16(identify_data + 88, 0x3f | (1 << (val + 8)));
+ break;
+ default:
+ goto abort_cmd;
+ }
+ return true;
+ }
+ }
+
+abort_cmd:
+ ide_abort_command(s);
+ return true;
+}
+
#define HD_OK (1u << IDE_HD)
#define CD_OK (1u << IDE_CD)
#define CFA_OK (1u << IDE_CFATA)
@@ -1274,7 +1348,7 @@ static const struct {
[WIN_FLUSH_CACHE] = { NULL, ALL_OK },
[WIN_FLUSH_CACHE_EXT] = { NULL, HD_CFA_OK },
[WIN_IDENTIFY] = { cmd_identify, ALL_OK },
- [WIN_SETFEATURES] = { NULL, ALL_OK },
+ [WIN_SETFEATURES] = { cmd_set_features, ALL_OK | SET_DSC },
[IBM_SENSE_CONDITION] = { NULL, CFA_OK },
[CFA_WEAR_LEVEL] = { NULL, HD_CFA_OK },
[WIN_READ_NATIVE_MAX] = { cmd_read_native_max, ALL_OK | SET_DSC },
@@ -1288,7 +1362,6 @@ static bool ide_cmd_permitted(IDEState *s, uint32_t cmd)
void ide_exec_cmd(IDEBus *bus, uint32_t val)
{
- uint16_t *identify_data;
IDEState *s;
int n;
@@ -1330,76 +1403,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
}
switch(val) {
- case WIN_SETFEATURES:
- if (!s->bs)
- goto abort_cmd;
- /* XXX: valid for CDROM ? */
- switch(s->feature) {
- case 0x02: /* write cache enable */
- bdrv_set_enable_write_cache(s->bs, true);
- identify_data = (uint16_t *)s->identify_data;
- put_le16(identify_data + 85, (1 << 14) | (1 << 5) | 1);
- s->status = READY_STAT | SEEK_STAT;
- ide_set_irq(s->bus);
- break;
- case 0x82: /* write cache disable */
- bdrv_set_enable_write_cache(s->bs, false);
- identify_data = (uint16_t *)s->identify_data;
- put_le16(identify_data + 85, (1 << 14) | 1);
- ide_flush_cache(s);
- break;
- case 0xcc: /* reverting to power-on defaults enable */
- case 0x66: /* reverting to power-on defaults 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 */
- uint8_t val = s->nsector & 0x07;
- identify_data = (uint16_t *)s->identify_data;
-
- switch (s->nsector >> 3) {
- 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*/
- 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 */
- 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 */
- put_le16(identify_data + 62,0x07);
- put_le16(identify_data + 63,0x07);
- put_le16(identify_data + 88,0x3f | (1 << (val + 8)));
- break;
- 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);
--
1.8.1.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [Qemu-devel] [PATCH 12/23] ide: Convert FLUSH CACHE to ide_cmd_table handler
2013-06-24 9:10 [Qemu-devel] [PULL 00/23] Block patches Stefan Hajnoczi
` (10 preceding siblings ...)
2013-06-24 9:10 ` [Qemu-devel] [PATCH 11/23] ide: Convert SET FEATURES " Stefan Hajnoczi
@ 2013-06-24 9:10 ` Stefan Hajnoczi
2013-07-03 21:41 ` Alex Williamson
2013-06-24 9:10 ` [Qemu-devel] [PATCH 13/23] ide: Convert SEEK " Stefan Hajnoczi
` (10 subsequent siblings)
22 siblings, 1 reply; 35+ messages in thread
From: Stefan Hajnoczi @ 2013-06-24 9:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi
From: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/ide/core.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 8789758..83e86aa 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1184,6 +1184,12 @@ static bool cmd_write_dma(IDEState *s, uint8_t cmd)
return false;
}
+static bool cmd_flush_cache(IDEState *s, uint8_t cmd)
+{
+ ide_flush_cache(s);
+ return false;
+}
+
static bool cmd_read_native_max(IDEState *s, uint8_t cmd)
{
bool lba48 = (cmd == WIN_READ_NATIVE_MAX_EXT);
@@ -1345,8 +1351,8 @@ static const struct {
[WIN_SETIDLE1] = { cmd_nop, ALL_OK },
[WIN_CHECKPOWERMODE1] = { cmd_check_power_mode, ALL_OK | SET_DSC },
[WIN_SLEEPNOW1] = { cmd_nop, ALL_OK },
- [WIN_FLUSH_CACHE] = { NULL, ALL_OK },
- [WIN_FLUSH_CACHE_EXT] = { NULL, HD_CFA_OK },
+ [WIN_FLUSH_CACHE] = { cmd_flush_cache, ALL_OK },
+ [WIN_FLUSH_CACHE_EXT] = { cmd_flush_cache, HD_CFA_OK },
[WIN_IDENTIFY] = { cmd_identify, ALL_OK },
[WIN_SETFEATURES] = { cmd_set_features, ALL_OK | SET_DSC },
[IBM_SENSE_CONDITION] = { NULL, CFA_OK },
@@ -1403,10 +1409,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
}
switch(val) {
- case WIN_FLUSH_CACHE:
- case WIN_FLUSH_CACHE_EXT:
- ide_flush_cache(s);
- break;
case WIN_SEEK:
/* XXX: Check that seek is within bounds */
s->status = READY_STAT | SEEK_STAT;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [Qemu-devel] [PATCH 12/23] ide: Convert FLUSH CACHE to ide_cmd_table handler
2013-06-24 9:10 ` [Qemu-devel] [PATCH 12/23] ide: Convert FLUSH CACHE " Stefan Hajnoczi
@ 2013-07-03 21:41 ` Alex Williamson
2013-07-03 21:51 ` Alex Williamson
0 siblings, 1 reply; 35+ messages in thread
From: Alex Williamson @ 2013-07-03 21:41 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: Kevin Wolf, Anthony Liguori, qemu-devel
On Mon, 2013-06-24 at 11:10 +0200, Stefan Hajnoczi wrote:
> From: Kevin Wolf <kwolf@redhat.com>
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> hw/ide/core.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index 8789758..83e86aa 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -1184,6 +1184,12 @@ static bool cmd_write_dma(IDEState *s, uint8_t cmd)
> return false;
> }
>
> +static bool cmd_flush_cache(IDEState *s, uint8_t cmd)
> +{
> + ide_flush_cache(s);
> + return false;
> +}
> +
> static bool cmd_read_native_max(IDEState *s, uint8_t cmd)
> {
> bool lba48 = (cmd == WIN_READ_NATIVE_MAX_EXT);
> @@ -1345,8 +1351,8 @@ static const struct {
> [WIN_SETIDLE1] = { cmd_nop, ALL_OK },
> [WIN_CHECKPOWERMODE1] = { cmd_check_power_mode, ALL_OK | SET_DSC },
> [WIN_SLEEPNOW1] = { cmd_nop, ALL_OK },
> - [WIN_FLUSH_CACHE] = { NULL, ALL_OK },
> - [WIN_FLUSH_CACHE_EXT] = { NULL, HD_CFA_OK },
> + [WIN_FLUSH_CACHE] = { cmd_flush_cache, ALL_OK },
> + [WIN_FLUSH_CACHE_EXT] = { cmd_flush_cache, HD_CFA_OK },
> [WIN_IDENTIFY] = { cmd_identify, ALL_OK },
> [WIN_SETFEATURES] = { cmd_set_features, ALL_OK | SET_DSC },
> [IBM_SENSE_CONDITION] = { NULL, CFA_OK },
> @@ -1403,10 +1409,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
> }
>
> switch(val) {
> - case WIN_FLUSH_CACHE:
> - case WIN_FLUSH_CACHE_EXT:
> - ide_flush_cache(s);
> - break;
> case WIN_SEEK:
> /* XXX: Check that seek is within bounds */
> s->status = READY_STAT | SEEK_STAT;
This also breaks win7 x64 q35 IDE. Note that while this change looks
like a no-op, filling in a handler now means that we do:
s->status = READY_STAT | BUSY_STAT;
before calling the handler and don't clear it on the way out since the
function statically returns false. This then introduces the same bug as
f68ec837. Thanks,
Alex
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [Qemu-devel] [PATCH 12/23] ide: Convert FLUSH CACHE to ide_cmd_table handler
2013-07-03 21:41 ` Alex Williamson
@ 2013-07-03 21:51 ` Alex Williamson
2013-07-04 7:58 ` Kevin Wolf
0 siblings, 1 reply; 35+ messages in thread
From: Alex Williamson @ 2013-07-03 21:51 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: Kevin Wolf, Anthony Liguori, qemu-devel
On Wed, 2013-07-03 at 15:41 -0600, Alex Williamson wrote:
> On Mon, 2013-06-24 at 11:10 +0200, Stefan Hajnoczi wrote:
> > From: Kevin Wolf <kwolf@redhat.com>
> >
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> > hw/ide/core.c | 14 ++++++++------
> > 1 file changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git a/hw/ide/core.c b/hw/ide/core.c
> > index 8789758..83e86aa 100644
> > --- a/hw/ide/core.c
> > +++ b/hw/ide/core.c
> > @@ -1184,6 +1184,12 @@ static bool cmd_write_dma(IDEState *s, uint8_t cmd)
> > return false;
> > }
> >
> > +static bool cmd_flush_cache(IDEState *s, uint8_t cmd)
> > +{
> > + ide_flush_cache(s);
> > + return false;
> > +}
> > +
> > static bool cmd_read_native_max(IDEState *s, uint8_t cmd)
> > {
> > bool lba48 = (cmd == WIN_READ_NATIVE_MAX_EXT);
> > @@ -1345,8 +1351,8 @@ static const struct {
> > [WIN_SETIDLE1] = { cmd_nop, ALL_OK },
> > [WIN_CHECKPOWERMODE1] = { cmd_check_power_mode, ALL_OK | SET_DSC },
> > [WIN_SLEEPNOW1] = { cmd_nop, ALL_OK },
> > - [WIN_FLUSH_CACHE] = { NULL, ALL_OK },
> > - [WIN_FLUSH_CACHE_EXT] = { NULL, HD_CFA_OK },
> > + [WIN_FLUSH_CACHE] = { cmd_flush_cache, ALL_OK },
> > + [WIN_FLUSH_CACHE_EXT] = { cmd_flush_cache, HD_CFA_OK },
> > [WIN_IDENTIFY] = { cmd_identify, ALL_OK },
> > [WIN_SETFEATURES] = { cmd_set_features, ALL_OK | SET_DSC },
> > [IBM_SENSE_CONDITION] = { NULL, CFA_OK },
> > @@ -1403,10 +1409,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
> > }
> >
> > switch(val) {
> > - case WIN_FLUSH_CACHE:
> > - case WIN_FLUSH_CACHE_EXT:
> > - ide_flush_cache(s);
> > - break;
> > case WIN_SEEK:
> > /* XXX: Check that seek is within bounds */
> > s->status = READY_STAT | SEEK_STAT;
>
> This also breaks win7 x64 q35 IDE. Note that while this change looks
> like a no-op, filling in a handler now means that we do:
>
> s->status = READY_STAT | BUSY_STAT;
>
> before calling the handler and don't clear it on the way out since the
> function statically returns false. This then introduces the same bug as
> f68ec837. Thanks,
This seems to work around the bug, but I'll leave it to those of you who
actually know how IDE works for a proper fix:
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 96b468c..8893849 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1186,6 +1186,7 @@ static bool cmd_write_dma(IDEState *s, uint8_t cmd)
static bool cmd_flush_cache(IDEState *s, uint8_t cmd)
{
ide_flush_cache(s);
+ s->status &= ~BUSY_STAT;
return false;
}
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [Qemu-devel] [PATCH 12/23] ide: Convert FLUSH CACHE to ide_cmd_table handler
2013-07-03 21:51 ` Alex Williamson
@ 2013-07-04 7:58 ` Kevin Wolf
0 siblings, 0 replies; 35+ messages in thread
From: Kevin Wolf @ 2013-07-04 7:58 UTC (permalink / raw)
To: Alex Williamson; +Cc: Anthony Liguori, qemu-devel, Stefan Hajnoczi
Am 03.07.2013 um 23:51 hat Alex Williamson geschrieben:
> On Wed, 2013-07-03 at 15:41 -0600, Alex Williamson wrote:
> > On Mon, 2013-06-24 at 11:10 +0200, Stefan Hajnoczi wrote:
> > > From: Kevin Wolf <kwolf@redhat.com>
> > >
> > > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > > ---
> > > hw/ide/core.c | 14 ++++++++------
> > > 1 file changed, 8 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/hw/ide/core.c b/hw/ide/core.c
> > > index 8789758..83e86aa 100644
> > > --- a/hw/ide/core.c
> > > +++ b/hw/ide/core.c
> > > @@ -1184,6 +1184,12 @@ static bool cmd_write_dma(IDEState *s, uint8_t cmd)
> > > return false;
> > > }
> > >
> > > +static bool cmd_flush_cache(IDEState *s, uint8_t cmd)
> > > +{
> > > + ide_flush_cache(s);
> > > + return false;
> > > +}
> > > +
> > > static bool cmd_read_native_max(IDEState *s, uint8_t cmd)
> > > {
> > > bool lba48 = (cmd == WIN_READ_NATIVE_MAX_EXT);
> > > @@ -1345,8 +1351,8 @@ static const struct {
> > > [WIN_SETIDLE1] = { cmd_nop, ALL_OK },
> > > [WIN_CHECKPOWERMODE1] = { cmd_check_power_mode, ALL_OK | SET_DSC },
> > > [WIN_SLEEPNOW1] = { cmd_nop, ALL_OK },
> > > - [WIN_FLUSH_CACHE] = { NULL, ALL_OK },
> > > - [WIN_FLUSH_CACHE_EXT] = { NULL, HD_CFA_OK },
> > > + [WIN_FLUSH_CACHE] = { cmd_flush_cache, ALL_OK },
> > > + [WIN_FLUSH_CACHE_EXT] = { cmd_flush_cache, HD_CFA_OK },
> > > [WIN_IDENTIFY] = { cmd_identify, ALL_OK },
> > > [WIN_SETFEATURES] = { cmd_set_features, ALL_OK | SET_DSC },
> > > [IBM_SENSE_CONDITION] = { NULL, CFA_OK },
> > > @@ -1403,10 +1409,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
> > > }
> > >
> > > switch(val) {
> > > - case WIN_FLUSH_CACHE:
> > > - case WIN_FLUSH_CACHE_EXT:
> > > - ide_flush_cache(s);
> > > - break;
> > > case WIN_SEEK:
> > > /* XXX: Check that seek is within bounds */
> > > s->status = READY_STAT | SEEK_STAT;
> >
> > This also breaks win7 x64 q35 IDE. Note that while this change looks
> > like a no-op, filling in a handler now means that we do:
> >
> > s->status = READY_STAT | BUSY_STAT;
> >
> > before calling the handler and don't clear it on the way out since the
> > function statically returns false. This then introduces the same bug as
> > f68ec837. Thanks,
>
> This seems to work around the bug, but I'll leave it to those of you who
> actually know how IDE works for a proper fix:
>
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index 96b468c..8893849 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -1186,6 +1186,7 @@ static bool cmd_write_dma(IDEState *s, uint8_t cmd)
> static bool cmd_flush_cache(IDEState *s, uint8_t cmd)
> {
> ide_flush_cache(s);
> + s->status &= ~BUSY_STAT;
> return false;
> }
This is wrong, the BSY bit must remain set while the FLUSH command is
running. As I said in the other thread, the real problem is that AHCI
isn't notified about the command completion for flushes.
Kevin
^ permalink raw reply [flat|nested] 35+ messages in thread
* [Qemu-devel] [PATCH 13/23] ide: Convert SEEK to ide_cmd_table handler
2013-06-24 9:10 [Qemu-devel] [PULL 00/23] Block patches Stefan Hajnoczi
` (11 preceding siblings ...)
2013-06-24 9:10 ` [Qemu-devel] [PATCH 12/23] ide: Convert FLUSH CACHE " Stefan Hajnoczi
@ 2013-06-24 9:10 ` Stefan Hajnoczi
2013-06-24 9:10 ` [Qemu-devel] [PATCH 14/23] ide: Convert ATAPI commands " Stefan Hajnoczi
` (9 subsequent siblings)
22 siblings, 0 replies; 35+ messages in thread
From: Stefan Hajnoczi @ 2013-06-24 9:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi
From: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/ide/core.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 83e86aa..76a3fdf 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1190,6 +1190,12 @@ static bool cmd_flush_cache(IDEState *s, uint8_t cmd)
return false;
}
+static bool cmd_seek(IDEState *s, uint8_t cmd)
+{
+ /* XXX: Check that seek is within bounds */
+ return true;
+}
+
static bool cmd_read_native_max(IDEState *s, uint8_t cmd)
{
bool lba48 = (cmd == WIN_READ_NATIVE_MAX_EXT);
@@ -1322,7 +1328,7 @@ static const struct {
[WIN_VERIFY] = { cmd_verify, HD_CFA_OK | SET_DSC },
[WIN_VERIFY_ONCE] = { cmd_verify, HD_CFA_OK | SET_DSC },
[WIN_VERIFY_EXT] = { cmd_verify, HD_CFA_OK | SET_DSC },
- [WIN_SEEK] = { NULL, HD_CFA_OK },
+ [WIN_SEEK] = { cmd_seek, HD_CFA_OK | SET_DSC },
[CFA_TRANSLATE_SECTOR] = { NULL, CFA_OK },
[WIN_DIAGNOSE] = { NULL, ALL_OK },
[WIN_SPECIFY] = { cmd_nop, HD_CFA_OK | SET_DSC },
@@ -1409,11 +1415,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
}
switch(val) {
- case WIN_SEEK:
- /* XXX: Check that seek is within bounds */
- s->status = READY_STAT | SEEK_STAT;
- ide_set_irq(s->bus);
- break;
/* ATAPI commands */
case WIN_PIDENTIFY:
ide_atapi_identify(s);
--
1.8.1.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [Qemu-devel] [PATCH 14/23] ide: Convert ATAPI commands to ide_cmd_table handler
2013-06-24 9:10 [Qemu-devel] [PULL 00/23] Block patches Stefan Hajnoczi
` (12 preceding siblings ...)
2013-06-24 9:10 ` [Qemu-devel] [PATCH 13/23] ide: Convert SEEK " Stefan Hajnoczi
@ 2013-06-24 9:10 ` Stefan Hajnoczi
2013-06-24 9:10 ` [Qemu-devel] [PATCH 15/23] ide: Convert CF-ATA " Stefan Hajnoczi
` (8 subsequent siblings)
22 siblings, 0 replies; 35+ messages in thread
From: Stefan Hajnoczi @ 2013-06-24 9:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi
From: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/ide/core.c | 100 +++++++++++++++++++++++++++++++++++-----------------------
1 file changed, 61 insertions(+), 39 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 76a3fdf..eebd5d9 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1292,6 +1292,63 @@ abort_cmd:
return true;
}
+
+/*** ATAPI commands ***/
+
+static bool cmd_identify_packet(IDEState *s, uint8_t cmd)
+{
+ ide_atapi_identify(s);
+ s->status = READY_STAT | SEEK_STAT;
+ ide_transfer_start(s, s->io_buffer, 512, ide_transfer_stop);
+ ide_set_irq(s->bus);
+ return false;
+}
+
+static bool cmd_exec_dev_diagnostic(IDEState *s, uint8_t cmd)
+{
+ 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;
+ /* The bits of the error register are not as usual for this command!
+ * They are part of the regular output (this is why ERR_STAT isn't set)
+ * Device 0 passed, Device 1 passed or not present. */
+ s->error = 0x01;
+ ide_set_irq(s->bus);
+ }
+
+ return false;
+}
+
+static bool cmd_device_reset(IDEState *s, uint8_t cmd)
+{
+ ide_set_signature(s);
+ s->status = 0x00; /* NOTE: READY is _not_ set */
+ s->error = 0x01;
+
+ return false;
+}
+
+static bool cmd_packet(IDEState *s, uint8_t cmd)
+{
+ /* overlapping commands not supported */
+ if (s->feature & 0x02) {
+ ide_abort_command(s);
+ return true;
+ }
+
+ 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);
+ return false;
+}
+
#define HD_OK (1u << IDE_HD)
#define CD_OK (1u << IDE_CD)
#define CFA_OK (1u << IDE_CFATA)
@@ -1310,7 +1367,7 @@ static const struct {
/* NOP not implemented, mandatory for CD */
[CFA_REQ_EXT_ERROR_CODE] = { NULL, CFA_OK },
[WIN_DSM] = { cmd_data_set_management, ALL_OK },
- [WIN_DEVICE_RESET] = { NULL, CD_OK },
+ [WIN_DEVICE_RESET] = { cmd_device_reset, CD_OK },
[WIN_RECAL] = { cmd_nop, HD_CFA_OK | SET_DSC},
[WIN_READ] = { cmd_read_pio, ALL_OK },
[WIN_READ_ONCE] = { cmd_read_pio, ALL_OK },
@@ -1330,7 +1387,7 @@ static const struct {
[WIN_VERIFY_EXT] = { cmd_verify, HD_CFA_OK | SET_DSC },
[WIN_SEEK] = { cmd_seek, HD_CFA_OK | SET_DSC },
[CFA_TRANSLATE_SECTOR] = { NULL, CFA_OK },
- [WIN_DIAGNOSE] = { NULL, ALL_OK },
+ [WIN_DIAGNOSE] = { cmd_exec_dev_diagnostic, ALL_OK },
[WIN_SPECIFY] = { cmd_nop, HD_CFA_OK | SET_DSC },
[WIN_STANDBYNOW2] = { cmd_nop, ALL_OK },
[WIN_IDLEIMMEDIATE2] = { cmd_nop, ALL_OK },
@@ -1338,8 +1395,8 @@ static const struct {
[WIN_SETIDLE2] = { cmd_nop, ALL_OK },
[WIN_CHECKPOWERMODE2] = { cmd_check_power_mode, ALL_OK | SET_DSC },
[WIN_SLEEPNOW2] = { cmd_nop, ALL_OK },
- [WIN_PACKETCMD] = { NULL, CD_OK },
- [WIN_PIDENTIFY] = { NULL, CD_OK },
+ [WIN_PACKETCMD] = { cmd_packet, CD_OK },
+ [WIN_PIDENTIFY] = { cmd_identify_packet, CD_OK },
[WIN_SMART] = { NULL, HD_CFA_OK },
[CFA_ACCESS_METADATA_STORAGE] = { NULL, CFA_OK },
[CFA_ERASE_SECTORS] = { NULL, CFA_OK },
@@ -1415,41 +1472,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
}
switch(val) {
- /* ATAPI commands */
- case WIN_PIDENTIFY:
- ide_atapi_identify(s);
- s->status = READY_STAT | SEEK_STAT;
- ide_transfer_start(s, s->io_buffer, 512, ide_transfer_stop);
- 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_DEVICE_RESET:
- ide_set_signature(s);
- s->status = 0x00; /* NOTE: READY is _not_ set */
- s->error = 0x01;
- break;
- case WIN_PACKETCMD:
- /* 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:
s->error = 0x09; /* miscellaneous error */
--
1.8.1.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [Qemu-devel] [PATCH 15/23] ide: Convert CF-ATA commands to ide_cmd_table handler
2013-06-24 9:10 [Qemu-devel] [PULL 00/23] Block patches Stefan Hajnoczi
` (13 preceding siblings ...)
2013-06-24 9:10 ` [Qemu-devel] [PATCH 14/23] ide: Convert ATAPI commands " Stefan Hajnoczi
@ 2013-06-24 9:10 ` Stefan Hajnoczi
2013-06-24 9:10 ` [Qemu-devel] [PATCH 16/23] ide: Convert SMART " Stefan Hajnoczi
` (7 subsequent siblings)
22 siblings, 0 replies; 35+ messages in thread
From: Stefan Hajnoczi @ 2013-06-24 9:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi
From: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/ide/core.c | 170 ++++++++++++++++++++++++++++++++--------------------------
1 file changed, 95 insertions(+), 75 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index eebd5d9..a563f6e 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1349,6 +1349,95 @@ static bool cmd_packet(IDEState *s, uint8_t cmd)
return false;
}
+
+/*** CF-ATA commands ***/
+
+static bool cmd_cfa_req_ext_error_code(IDEState *s, uint8_t cmd)
+{
+ s->error = 0x09; /* miscellaneous error */
+ s->status = READY_STAT | SEEK_STAT;
+ ide_set_irq(s->bus);
+
+ return false;
+}
+
+static bool cmd_cfa_erase_sectors(IDEState *s, uint8_t cmd)
+{
+ /* WIN_SECURITY_FREEZE_LOCK has the same ID as CFA_WEAR_LEVEL and is
+ * required for Windows 8 to work with AHCI */
+
+ if (cmd == CFA_WEAR_LEVEL) {
+ s->nsector = 0;
+ }
+
+ if (cmd == CFA_ERASE_SECTORS) {
+ s->media_changed = 1;
+ }
+
+ return true;
+}
+
+static bool cmd_cfa_translate_sector(IDEState *s, uint8_t cmd)
+{
+ 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);
+
+ return false;
+}
+
+static bool cmd_cfa_access_metadata_storage(IDEState *s, uint8_t 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:
+ ide_abort_command(s);
+ return true;
+ }
+
+ ide_transfer_start(s, s->io_buffer, 0x200, ide_transfer_stop);
+ s->status = 0x00; /* NOTE: READY is _not_ set */
+ ide_set_irq(s->bus);
+
+ return false;
+}
+
+static bool cmd_ibm_sense_condition(IDEState *s, uint8_t cmd)
+{
+ switch (s->feature) {
+ case 0x01: /* sense temperature in device */
+ s->nsector = 0x50; /* +20 C */
+ break;
+ default:
+ ide_abort_command(s);
+ return true;
+ }
+
+ return true;
+}
+
#define HD_OK (1u << IDE_HD)
#define CD_OK (1u << IDE_CD)
#define CFA_OK (1u << IDE_CFATA)
@@ -1365,7 +1454,7 @@ static const struct {
int flags;
} ide_cmd_table[0x100] = {
/* NOP not implemented, mandatory for CD */
- [CFA_REQ_EXT_ERROR_CODE] = { NULL, CFA_OK },
+ [CFA_REQ_EXT_ERROR_CODE] = { cmd_cfa_req_ext_error_code, CFA_OK },
[WIN_DSM] = { cmd_data_set_management, ALL_OK },
[WIN_DEVICE_RESET] = { cmd_device_reset, CD_OK },
[WIN_RECAL] = { cmd_nop, HD_CFA_OK | SET_DSC},
@@ -1386,7 +1475,7 @@ static const struct {
[WIN_VERIFY_ONCE] = { cmd_verify, HD_CFA_OK | SET_DSC },
[WIN_VERIFY_EXT] = { cmd_verify, HD_CFA_OK | SET_DSC },
[WIN_SEEK] = { cmd_seek, HD_CFA_OK | SET_DSC },
- [CFA_TRANSLATE_SECTOR] = { NULL, CFA_OK },
+ [CFA_TRANSLATE_SECTOR] = { cmd_cfa_translate_sector, CFA_OK },
[WIN_DIAGNOSE] = { cmd_exec_dev_diagnostic, ALL_OK },
[WIN_SPECIFY] = { cmd_nop, HD_CFA_OK | SET_DSC },
[WIN_STANDBYNOW2] = { cmd_nop, ALL_OK },
@@ -1398,8 +1487,8 @@ static const struct {
[WIN_PACKETCMD] = { cmd_packet, CD_OK },
[WIN_PIDENTIFY] = { cmd_identify_packet, CD_OK },
[WIN_SMART] = { NULL, HD_CFA_OK },
- [CFA_ACCESS_METADATA_STORAGE] = { NULL, CFA_OK },
- [CFA_ERASE_SECTORS] = { NULL, CFA_OK },
+ [CFA_ACCESS_METADATA_STORAGE] = { cmd_cfa_access_metadata_storage, CFA_OK },
+ [CFA_ERASE_SECTORS] = { cmd_cfa_erase_sectors, CFA_OK | SET_DSC },
[WIN_MULTREAD] = { cmd_read_multiple, HD_CFA_OK },
[WIN_MULTWRITE] = { cmd_write_multiple, HD_CFA_OK },
[WIN_SETMULT] = { cmd_set_multiple_mode, HD_CFA_OK | SET_DSC },
@@ -1418,8 +1507,8 @@ static const struct {
[WIN_FLUSH_CACHE_EXT] = { cmd_flush_cache, HD_CFA_OK },
[WIN_IDENTIFY] = { cmd_identify, ALL_OK },
[WIN_SETFEATURES] = { cmd_set_features, ALL_OK | SET_DSC },
- [IBM_SENSE_CONDITION] = { NULL, CFA_OK },
- [CFA_WEAR_LEVEL] = { NULL, HD_CFA_OK },
+ [IBM_SENSE_CONDITION] = { cmd_ibm_sense_condition, CFA_OK | SET_DSC },
+ [CFA_WEAR_LEVEL] = { cmd_cfa_erase_sectors, HD_CFA_OK | SET_DSC },
[WIN_READ_NATIVE_MAX] = { cmd_read_native_max, ALL_OK | SET_DSC },
};
@@ -1472,75 +1561,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
}
switch(val) {
- /* CF-ATA commands */
- case CFA_REQ_EXT_ERROR_CODE:
- 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 0
- /* This one has the same ID as CFA_WEAR_LEVEL and is required for
- Windows 8 to work with AHCI */
- case WIN_SECURITY_FREEZE_LOCK:
-#endif
- 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:
- 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:
- 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:
- 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->hcyl != 0xc2 || s->lcyl != 0x4f)
goto abort_cmd;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [Qemu-devel] [PATCH 16/23] ide: Convert SMART commands to ide_cmd_table handler
2013-06-24 9:10 [Qemu-devel] [PULL 00/23] Block patches Stefan Hajnoczi
` (14 preceding siblings ...)
2013-06-24 9:10 ` [Qemu-devel] [PATCH 15/23] ide: Convert CF-ATA " Stefan Hajnoczi
@ 2013-06-24 9:10 ` Stefan Hajnoczi
2013-06-24 9:10 ` [Qemu-devel] [PATCH 17/23] ide: Clean up ide_exec_cmd() Stefan Hajnoczi
` (6 subsequent siblings)
22 siblings, 0 replies; 35+ messages in thread
From: Stefan Hajnoczi @ 2013-06-24 9:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi
From: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/ide/core.c | 325 +++++++++++++++++++++++++++++++---------------------------
1 file changed, 174 insertions(+), 151 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index a563f6e..1c8f414 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1438,6 +1438,179 @@ static bool cmd_ibm_sense_condition(IDEState *s, uint8_t cmd)
return true;
}
+
+/*** SMART commands ***/
+
+static bool cmd_smart(IDEState *s, uint8_t cmd)
+{
+ int n;
+
+ if (s->hcyl != 0xc2 || s->lcyl != 0x4f) {
+ goto abort_cmd;
+ }
+
+ if (!s->smart_enabled && s->feature != SMART_ENABLE) {
+ goto abort_cmd;
+ }
+
+ switch (s->feature) {
+ case SMART_DISABLE:
+ s->smart_enabled = 0;
+ return true;
+
+ case SMART_ENABLE:
+ s->smart_enabled = 1;
+ return true;
+
+ case SMART_ATTR_AUTOSAVE:
+ switch (s->sector) {
+ case 0x00:
+ s->smart_autosave = 0;
+ break;
+ case 0xf1:
+ s->smart_autosave = 1;
+ break;
+ default:
+ goto abort_cmd;
+ }
+ return true;
+
+ case SMART_STATUS:
+ if (!s->smart_errors) {
+ s->hcyl = 0xc2;
+ s->lcyl = 0x4f;
+ } else {
+ s->hcyl = 0x2c;
+ s->lcyl = 0xf4;
+ }
+ return true;
+
+ case SMART_READ_THRESH:
+ memset(s->io_buffer, 0, 0x200);
+ s->io_buffer[0] = 0x01; /* smart struct version */
+
+ for (n = 0; n < ARRAY_SIZE(smart_attributes); n++) {
+ s->io_buffer[2 + 0 + (n * 12)] = smart_attributes[n][0];
+ s->io_buffer[2 + 1 + (n * 12)] = smart_attributes[n][11];
+ }
+
+ /* checksum */
+ for (n = 0; n < 511; 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);
+ return false;
+
+ case SMART_READ_DATA:
+ memset(s->io_buffer, 0, 0x200);
+ s->io_buffer[0] = 0x01; /* smart struct version */
+
+ for (n = 0; n < ARRAY_SIZE(smart_attributes); n++) {
+ int i;
+ for (i = 0; i < 11; i++) {
+ s->io_buffer[2 + i + (n * 12)] = smart_attributes[n][i];
+ }
+ }
+
+ s->io_buffer[362] = 0x02 | (s->smart_autosave ? 0x80 : 0x00);
+ if (s->smart_selftest_count == 0) {
+ s->io_buffer[363] = 0;
+ } else {
+ s->io_buffer[363] =
+ s->smart_selftest_data[3 +
+ (s->smart_selftest_count - 1) *
+ 24];
+ }
+ s->io_buffer[364] = 0x20;
+ s->io_buffer[365] = 0x01;
+ /* offline data collection capacity: execute + self-test*/
+ s->io_buffer[367] = (1 << 4 | 1 << 3 | 1);
+ s->io_buffer[368] = 0x03; /* smart capability (1) */
+ s->io_buffer[369] = 0x00; /* smart capability (2) */
+ s->io_buffer[370] = 0x01; /* error logging supported */
+ s->io_buffer[372] = 0x02; /* minutes for poll short test */
+ s->io_buffer[373] = 0x36; /* minutes for poll ext test */
+ 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] = 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);
+ return false;
+
+ 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;
+
+ for (n = 0; n < 511; n++) {
+ s->io_buffer[511] += s->io_buffer[n];
+ }
+ 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) {
+ s->io_buffer[508] = 0;
+ } 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[511] += s->io_buffer[n];
+ }
+ s->io_buffer[511] = 0x100 - s->io_buffer[511];
+ break;
+ default:
+ 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);
+ return false;
+
+ 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 = 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 */
+ break;
+ default:
+ goto abort_cmd;
+ }
+ return true;
+ }
+
+abort_cmd:
+ ide_abort_command(s);
+ return true;
+}
+
#define HD_OK (1u << IDE_HD)
#define CD_OK (1u << IDE_CD)
#define CFA_OK (1u << IDE_CFATA)
@@ -1486,7 +1659,7 @@ static const struct {
[WIN_SLEEPNOW2] = { cmd_nop, ALL_OK },
[WIN_PACKETCMD] = { cmd_packet, CD_OK },
[WIN_PIDENTIFY] = { cmd_identify_packet, CD_OK },
- [WIN_SMART] = { NULL, HD_CFA_OK },
+ [WIN_SMART] = { cmd_smart, HD_CFA_OK | SET_DSC },
[CFA_ACCESS_METADATA_STORAGE] = { cmd_cfa_access_metadata_storage, CFA_OK },
[CFA_ERASE_SECTORS] = { cmd_cfa_erase_sectors, CFA_OK | SET_DSC },
[WIN_MULTREAD] = { cmd_read_multiple, HD_CFA_OK },
@@ -1521,7 +1694,6 @@ static bool ide_cmd_permitted(IDEState *s, uint32_t cmd)
void ide_exec_cmd(IDEBus *bus, uint32_t val)
{
IDEState *s;
- int n;
#if defined(DEBUG_IDE)
printf("ide: CMD=%02x\n", val);
@@ -1561,155 +1733,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
}
switch(val) {
- case WIN_SMART:
- if (s->hcyl != 0xc2 || s->lcyl != 0x4f)
- goto abort_cmd;
- if (!s->smart_enabled && s->feature != SMART_ENABLE)
- goto abort_cmd;
- 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:
- s->smart_enabled = 1;
- s->status = READY_STAT | SEEK_STAT;
- ide_set_irq(s->bus);
- break;
- case SMART_ATTR_AUTOSAVE:
- switch (s->sector) {
- case 0x00:
- s->smart_autosave = 0;
- break;
- case 0xf1:
- s->smart_autosave = 1;
- break;
- default:
- goto abort_cmd;
- }
- s->status = READY_STAT | SEEK_STAT;
- ide_set_irq(s->bus);
- break;
- case SMART_STATUS:
- if (!s->smart_errors) {
- s->hcyl = 0xc2;
- s->lcyl = 0x4f;
- } else {
- s->hcyl = 0x2c;
- s->lcyl = 0xf4;
- }
- s->status = READY_STAT | SEEK_STAT;
- ide_set_irq(s->bus);
- break;
- case SMART_READ_THRESH:
- memset(s->io_buffer, 0, 0x200);
- s->io_buffer[0] = 0x01; /* smart struct version */
- for (n = 0; n < ARRAY_SIZE(smart_attributes); n++) {
- s->io_buffer[2+0+(n*12)] = smart_attributes[n][0];
- s->io_buffer[2+1+(n*12)] = smart_attributes[n][11];
- }
- for (n=0; n<511; n++) /* checksum */
- 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:
- memset(s->io_buffer, 0, 0x200);
- s->io_buffer[0] = 0x01; /* smart struct version */
- for (n = 0; n < ARRAY_SIZE(smart_attributes); n++) {
- int i;
- for(i = 0; i < 11; i++) {
- s->io_buffer[2+i+(n*12)] = smart_attributes[n][i];
- }
- }
- s->io_buffer[362] = 0x02 | (s->smart_autosave?0x80:0x00);
- if (s->smart_selftest_count == 0) {
- s->io_buffer[363] = 0;
- } else {
- s->io_buffer[363] =
- s->smart_selftest_data[3 +
- (s->smart_selftest_count - 1) *
- 24];
- }
- s->io_buffer[364] = 0x20;
- s->io_buffer[365] = 0x01;
- /* offline data collection capacity: execute + self-test*/
- s->io_buffer[367] = (1<<4 | 1<<3 | 1);
- s->io_buffer[368] = 0x03; /* smart capability (1) */
- s->io_buffer[369] = 0x00; /* smart capability (2) */
- s->io_buffer[370] = 0x01; /* error logging supported */
- s->io_buffer[372] = 0x02; /* minutes for poll short test */
- s->io_buffer[373] = 0x36; /* minutes for poll ext test */
- 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] = 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:
- 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;
-
- for (n=0; n<511; n++)
- s->io_buffer[511] += s->io_buffer[n];
- 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) {
- s->io_buffer[508] = 0;
- } 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[511] += s->io_buffer[n];
- s->io_buffer[511] = 0x100 - s->io_buffer[511];
- break;
- default:
- 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:
- 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 = 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;
- default:
- goto abort_cmd;
- }
- break;
- default:
- goto abort_cmd;
- }
- break;
default:
/* should not be reachable */
abort_cmd:
--
1.8.1.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [Qemu-devel] [PATCH 17/23] ide: Clean up ide_exec_cmd()
2013-06-24 9:10 [Qemu-devel] [PULL 00/23] Block patches Stefan Hajnoczi
` (15 preceding siblings ...)
2013-06-24 9:10 ` [Qemu-devel] [PATCH 16/23] ide: Convert SMART " Stefan Hajnoczi
@ 2013-06-24 9:10 ` Stefan Hajnoczi
2013-06-24 9:10 ` [Qemu-devel] [PATCH 18/23] Revert "block: Disable driver-specific options for 1.5" Stefan Hajnoczi
` (5 subsequent siblings)
22 siblings, 0 replies; 35+ messages in thread
From: Stefan Hajnoczi @ 2013-06-24 9:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi
From: Kevin Wolf <kwolf@redhat.com>
All commands are now converted to ide_cmd_table handlers, so it can be
unconditional now and the old switch block can go.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/ide/core.c | 36 ++++++++++++------------------------
1 file changed, 12 insertions(+), 24 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 1c8f414..03d1cfa 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1694,6 +1694,7 @@ static bool ide_cmd_permitted(IDEState *s, uint32_t cmd)
void ide_exec_cmd(IDEBus *bus, uint32_t val)
{
IDEState *s;
+ bool complete;
#if defined(DEBUG_IDE)
printf("ide: CMD=%02x\n", val);
@@ -1708,37 +1709,24 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
return;
if (!ide_cmd_permitted(s, val)) {
- goto abort_cmd;
+ ide_abort_command(s);
+ ide_set_irq(s->bus);
+ return;
}
- if (ide_cmd_table[val].handler != NULL) {
- bool complete;
-
- s->status = READY_STAT | BUSY_STAT;
- s->error = 0;
-
- complete = ide_cmd_table[val].handler(s, val);
- if (complete) {
- s->status &= ~BUSY_STAT;
- assert(!!s->error == !!(s->status & ERR_STAT));
+ s->status = READY_STAT | BUSY_STAT;
+ s->error = 0;
- if ((ide_cmd_table[val].flags & SET_DSC) && !s->error) {
- s->status |= SEEK_STAT;
- }
+ complete = ide_cmd_table[val].handler(s, val);
+ if (complete) {
+ s->status &= ~BUSY_STAT;
+ assert(!!s->error == !!(s->status & ERR_STAT));
- ide_set_irq(s->bus);
+ if ((ide_cmd_table[val].flags & SET_DSC) && !s->error) {
+ s->status |= SEEK_STAT;
}
- return;
- }
-
- switch(val) {
- default:
- /* should not be reachable */
- abort_cmd:
- ide_abort_command(s);
ide_set_irq(s->bus);
- break;
}
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [Qemu-devel] [PATCH 18/23] Revert "block: Disable driver-specific options for 1.5"
2013-06-24 9:10 [Qemu-devel] [PULL 00/23] Block patches Stefan Hajnoczi
` (16 preceding siblings ...)
2013-06-24 9:10 ` [Qemu-devel] [PATCH 17/23] ide: Clean up ide_exec_cmd() Stefan Hajnoczi
@ 2013-06-24 9:10 ` Stefan Hajnoczi
2013-06-24 9:10 ` [Qemu-devel] [PATCH 19/23] qcow2: Add refcount update reason to all callers Stefan Hajnoczi
` (4 subsequent siblings)
22 siblings, 0 replies; 35+ messages in thread
From: Stefan Hajnoczi @ 2013-06-24 9:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi
From: Kevin Wolf <kwolf@redhat.com>
This reverts commit 8ec7d390b0d50b5e5b4b1d8dba7ba40d64a70875.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
blockdev.c | 118 ++---------------------------------------------
tests/qemu-iotests/group | 2 +-
2 files changed, 5 insertions(+), 115 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 5975dde..459a87e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1737,120 +1737,10 @@ QemuOptsList qemu_drive_opts = {
.name = "drive",
.head = QTAILQ_HEAD_INITIALIZER(qemu_drive_opts.head),
.desc = {
- {
- .name = "bus",
- .type = QEMU_OPT_NUMBER,
- .help = "bus number",
- },{
- .name = "unit",
- .type = QEMU_OPT_NUMBER,
- .help = "unit number (i.e. lun for scsi)",
- },{
- .name = "if",
- .type = QEMU_OPT_STRING,
- .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)",
- },{
- .name = "index",
- .type = QEMU_OPT_NUMBER,
- .help = "index number",
- },{
- .name = "cyls",
- .type = QEMU_OPT_NUMBER,
- .help = "number of cylinders (ide disk geometry)",
- },{
- .name = "heads",
- .type = QEMU_OPT_NUMBER,
- .help = "number of heads (ide disk geometry)",
- },{
- .name = "secs",
- .type = QEMU_OPT_NUMBER,
- .help = "number of sectors (ide disk geometry)",
- },{
- .name = "trans",
- .type = QEMU_OPT_STRING,
- .help = "chs translation (auto, lba. none)",
- },{
- .name = "media",
- .type = QEMU_OPT_STRING,
- .help = "media type (disk, cdrom)",
- },{
- .name = "snapshot",
- .type = QEMU_OPT_BOOL,
- .help = "enable/disable snapshot mode",
- },{
- .name = "file",
- .type = QEMU_OPT_STRING,
- .help = "disk image",
- },{
- .name = "discard",
- .type = QEMU_OPT_STRING,
- .help = "discard operation (ignore/off, unmap/on)",
- },{
- .name = "cache",
- .type = QEMU_OPT_STRING,
- .help = "host cache usage (none, writeback, writethrough, "
- "directsync, unsafe)",
- },{
- .name = "aio",
- .type = QEMU_OPT_STRING,
- .help = "host AIO implementation (threads, native)",
- },{
- .name = "format",
- .type = QEMU_OPT_STRING,
- .help = "disk format (raw, qcow2, ...)",
- },{
- .name = "serial",
- .type = QEMU_OPT_STRING,
- .help = "disk serial number",
- },{
- .name = "rerror",
- .type = QEMU_OPT_STRING,
- .help = "read error action",
- },{
- .name = "werror",
- .type = QEMU_OPT_STRING,
- .help = "write error action",
- },{
- .name = "addr",
- .type = QEMU_OPT_STRING,
- .help = "pci address (virtio only)",
- },{
- .name = "readonly",
- .type = QEMU_OPT_BOOL,
- .help = "open drive file as read-only",
- },{
- .name = "iops",
- .type = QEMU_OPT_NUMBER,
- .help = "limit total I/O operations per second",
- },{
- .name = "iops_rd",
- .type = QEMU_OPT_NUMBER,
- .help = "limit read operations per second",
- },{
- .name = "iops_wr",
- .type = QEMU_OPT_NUMBER,
- .help = "limit write operations per second",
- },{
- .name = "bps",
- .type = QEMU_OPT_NUMBER,
- .help = "limit total bytes per second",
- },{
- .name = "bps_rd",
- .type = QEMU_OPT_NUMBER,
- .help = "limit read bytes per second",
- },{
- .name = "bps_wr",
- .type = QEMU_OPT_NUMBER,
- .help = "limit write bytes per second",
- },{
- .name = "copy-on-read",
- .type = QEMU_OPT_BOOL,
- .help = "copy read data from backing file into image file",
- },{
- .name = "boot",
- .type = QEMU_OPT_BOOL,
- .help = "(deprecated, ignored)",
- },
+ /*
+ * no elements => accept any params
+ * validation will happen later
+ */
{ /* end of list */ }
},
};
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 387b050..f487a8f 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -57,7 +57,7 @@
048 img auto quick
049 rw auto
050 rw auto backing quick
-#051 rw auto
+051 rw auto
052 rw auto backing
053 rw auto
054 rw auto
--
1.8.1.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [Qemu-devel] [PATCH 19/23] qcow2: Add refcount update reason to all callers
2013-06-24 9:10 [Qemu-devel] [PULL 00/23] Block patches Stefan Hajnoczi
` (17 preceding siblings ...)
2013-06-24 9:10 ` [Qemu-devel] [PATCH 18/23] Revert "block: Disable driver-specific options for 1.5" Stefan Hajnoczi
@ 2013-06-24 9:10 ` Stefan Hajnoczi
2013-06-24 9:10 ` [Qemu-devel] [PATCH 20/23] qcow2: Options to enable discard for freed clusters Stefan Hajnoczi
` (3 subsequent siblings)
22 siblings, 0 replies; 35+ messages in thread
From: Stefan Hajnoczi @ 2013-06-24 9:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi
From: Kevin Wolf <kwolf@redhat.com>
This adds a refcount update reason to all callers of update_refcounts(),
so that a follow-up patch can use this information to decide whether
clusters that reach a refcount of 0 should be discarded in the image
file.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/qcow2-cluster.c | 19 +++++++++++------
block/qcow2-refcount.c | 55 +++++++++++++++++++++++++++++++-------------------
block/qcow2-snapshot.c | 6 ++++--
block/qcow2.c | 3 ++-
block/qcow2.h | 16 ++++++++++++---
5 files changed, 66 insertions(+), 33 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 76f30e5..3191d6b 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -98,14 +98,16 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
goto fail;
}
g_free(s->l1_table);
- qcow2_free_clusters(bs, s->l1_table_offset, s->l1_size * sizeof(uint64_t));
+ qcow2_free_clusters(bs, s->l1_table_offset, s->l1_size * sizeof(uint64_t),
+ QCOW2_DISCARD_OTHER);
s->l1_table_offset = new_l1_table_offset;
s->l1_table = new_l1_table;
s->l1_size = new_l1_size;
return 0;
fail:
g_free(new_l1_table);
- qcow2_free_clusters(bs, new_l1_table_offset, new_l1_size2);
+ qcow2_free_clusters(bs, new_l1_table_offset, new_l1_size2,
+ QCOW2_DISCARD_OTHER);
return ret;
}
@@ -548,7 +550,8 @@ static int get_cluster_table(BlockDriverState *bs, uint64_t offset,
/* Then decrease the refcount of the old table */
if (l2_offset) {
- qcow2_free_clusters(bs, l2_offset, s->l2_size * sizeof(uint64_t));
+ qcow2_free_clusters(bs, l2_offset, s->l2_size * sizeof(uint64_t),
+ QCOW2_DISCARD_OTHER);
}
}
@@ -715,10 +718,14 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
/*
* If this was a COW, we need to decrease the refcount of the old cluster.
* Also flush bs->file to get the right order for L2 and refcount update.
+ *
+ * Don't discard clusters that reach a refcount of 0 (e.g. compressed
+ * clusters), the next write will reuse them anyway.
*/
if (j != 0) {
for (i = 0; i < j; i++) {
- qcow2_free_any_clusters(bs, be64_to_cpu(old_cluster[i]), 1);
+ qcow2_free_any_clusters(bs, be64_to_cpu(old_cluster[i]), 1,
+ QCOW2_DISCARD_NEVER);
}
}
@@ -1339,7 +1346,7 @@ static int discard_single_l2(BlockDriverState *bs, uint64_t offset,
l2_table[l2_index + i] = cpu_to_be64(0);
/* Then decrease the refcount */
- qcow2_free_any_clusters(bs, old_offset, 1);
+ qcow2_free_any_clusters(bs, old_offset, 1, QCOW2_DISCARD_REQUEST);
}
ret = qcow2_cache_put(bs, s->l2_table_cache, (void**) &l2_table);
@@ -1415,7 +1422,7 @@ static int zero_single_l2(BlockDriverState *bs, uint64_t offset,
qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_table);
if (old_offset & QCOW_OFLAG_COMPRESSED) {
l2_table[l2_index + i] = cpu_to_be64(QCOW_OFLAG_ZERO);
- qcow2_free_any_clusters(bs, old_offset, 1);
+ qcow2_free_any_clusters(bs, old_offset, 1, QCOW2_DISCARD_REQUEST);
} else {
l2_table[l2_index + i] |= cpu_to_be64(QCOW_OFLAG_ZERO);
}
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index b32738f..6d35e49 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -29,7 +29,7 @@
static int64_t alloc_clusters_noref(BlockDriverState *bs, int64_t size);
static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
int64_t offset, int64_t length,
- int addend);
+ int addend, enum qcow2_discard_type type);
/*********************************************************/
@@ -235,7 +235,8 @@ static int alloc_refcount_block(BlockDriverState *bs,
} else {
/* Described somewhere else. This can recurse at most twice before we
* arrive at a block that describes itself. */
- ret = update_refcount(bs, new_block, s->cluster_size, 1);
+ ret = update_refcount(bs, new_block, s->cluster_size, 1,
+ QCOW2_DISCARD_NEVER);
if (ret < 0) {
goto fail_block;
}
@@ -399,7 +400,8 @@ static int alloc_refcount_block(BlockDriverState *bs,
/* Free old table. Remember, we must not change free_cluster_index */
uint64_t old_free_cluster_index = s->free_cluster_index;
- qcow2_free_clusters(bs, old_table_offset, old_table_size * sizeof(uint64_t));
+ qcow2_free_clusters(bs, old_table_offset, old_table_size * sizeof(uint64_t),
+ QCOW2_DISCARD_OTHER);
s->free_cluster_index = old_free_cluster_index;
ret = load_refcount_block(bs, new_block, (void**) refcount_block);
@@ -420,7 +422,7 @@ fail_block:
/* XXX: cache several refcount block clusters ? */
static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
- int64_t offset, int64_t length, int addend)
+ int64_t offset, int64_t length, int addend, enum qcow2_discard_type type)
{
BDRVQcowState *s = bs->opaque;
int64_t start, last, cluster_offset;
@@ -506,7 +508,8 @@ fail:
*/
if (ret < 0) {
int dummy;
- dummy = update_refcount(bs, offset, cluster_offset - offset, -addend);
+ dummy = update_refcount(bs, offset, cluster_offset - offset, -addend,
+ QCOW2_DISCARD_NEVER);
(void)dummy;
}
@@ -522,12 +525,14 @@ fail:
*/
static int update_cluster_refcount(BlockDriverState *bs,
int64_t cluster_index,
- int addend)
+ int addend,
+ enum qcow2_discard_type type)
{
BDRVQcowState *s = bs->opaque;
int ret;
- ret = update_refcount(bs, cluster_index << s->cluster_bits, 1, addend);
+ ret = update_refcount(bs, cluster_index << s->cluster_bits, 1, addend,
+ type);
if (ret < 0) {
return ret;
}
@@ -579,7 +584,7 @@ int64_t qcow2_alloc_clusters(BlockDriverState *bs, int64_t size)
return offset;
}
- ret = update_refcount(bs, offset, size, 1);
+ ret = update_refcount(bs, offset, size, 1, QCOW2_DISCARD_NEVER);
if (ret < 0) {
return ret;
}
@@ -611,7 +616,8 @@ int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset,
old_free_cluster_index = s->free_cluster_index;
s->free_cluster_index = cluster_index + i;
- ret = update_refcount(bs, offset, i << s->cluster_bits, 1);
+ ret = update_refcount(bs, offset, i << s->cluster_bits, 1,
+ QCOW2_DISCARD_NEVER);
if (ret < 0) {
return ret;
}
@@ -649,7 +655,8 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
if (free_in_cluster == 0)
s->free_byte_offset = 0;
if ((offset & (s->cluster_size - 1)) != 0)
- update_cluster_refcount(bs, offset >> s->cluster_bits, 1);
+ update_cluster_refcount(bs, offset >> s->cluster_bits, 1,
+ QCOW2_DISCARD_NEVER);
} else {
offset = qcow2_alloc_clusters(bs, s->cluster_size);
if (offset < 0) {
@@ -659,7 +666,8 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
if ((cluster_offset + s->cluster_size) == offset) {
/* we are lucky: contiguous data */
offset = s->free_byte_offset;
- update_cluster_refcount(bs, offset >> s->cluster_bits, 1);
+ update_cluster_refcount(bs, offset >> s->cluster_bits, 1,
+ QCOW2_DISCARD_NEVER);
s->free_byte_offset += size;
} else {
s->free_byte_offset = offset;
@@ -676,12 +684,13 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
}
void qcow2_free_clusters(BlockDriverState *bs,
- int64_t offset, int64_t size)
+ int64_t offset, int64_t size,
+ enum qcow2_discard_type type)
{
int ret;
BLKDBG_EVENT(bs->file, BLKDBG_CLUSTER_FREE);
- ret = update_refcount(bs, offset, size, -1);
+ ret = update_refcount(bs, offset, size, -1, type);
if (ret < 0) {
fprintf(stderr, "qcow2_free_clusters failed: %s\n", strerror(-ret));
/* TODO Remember the clusters to free them later and avoid leaking */
@@ -692,8 +701,8 @@ void qcow2_free_clusters(BlockDriverState *bs,
* Free a cluster using its L2 entry (handles clusters of all types, e.g.
* normal cluster, compressed cluster, etc.)
*/
-void qcow2_free_any_clusters(BlockDriverState *bs,
- uint64_t l2_entry, int nb_clusters)
+void qcow2_free_any_clusters(BlockDriverState *bs, uint64_t l2_entry,
+ int nb_clusters, enum qcow2_discard_type type)
{
BDRVQcowState *s = bs->opaque;
@@ -705,12 +714,12 @@ void qcow2_free_any_clusters(BlockDriverState *bs,
s->csize_mask) + 1;
qcow2_free_clusters(bs,
(l2_entry & s->cluster_offset_mask) & ~511,
- nb_csectors * 512);
+ nb_csectors * 512, type);
}
break;
case QCOW2_CLUSTER_NORMAL:
qcow2_free_clusters(bs, l2_entry & L2E_OFFSET_MASK,
- nb_clusters << s->cluster_bits);
+ nb_clusters << s->cluster_bits, type);
break;
case QCOW2_CLUSTER_UNALLOCATED:
case QCOW2_CLUSTER_ZERO:
@@ -785,7 +794,8 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
int ret;
ret = update_refcount(bs,
(offset & s->cluster_offset_mask) & ~511,
- nb_csectors * 512, addend);
+ nb_csectors * 512, addend,
+ QCOW2_DISCARD_SNAPSHOT);
if (ret < 0) {
goto fail;
}
@@ -795,7 +805,8 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
} else {
uint64_t cluster_index = (offset & L2E_OFFSET_MASK) >> s->cluster_bits;
if (addend != 0) {
- refcount = update_cluster_refcount(bs, cluster_index, addend);
+ refcount = update_cluster_refcount(bs, cluster_index, addend,
+ QCOW2_DISCARD_SNAPSHOT);
} else {
refcount = get_refcount(bs, cluster_index);
}
@@ -827,7 +838,8 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
if (addend != 0) {
- refcount = update_cluster_refcount(bs, l2_offset >> s->cluster_bits, addend);
+ refcount = update_cluster_refcount(bs, l2_offset >> s->cluster_bits, addend,
+ QCOW2_DISCARD_SNAPSHOT);
} else {
refcount = get_refcount(bs, l2_offset >> s->cluster_bits);
}
@@ -1253,7 +1265,8 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
if (num_fixed) {
ret = update_refcount(bs, i << s->cluster_bits, 1,
- refcount2 - refcount1);
+ refcount2 - refcount1,
+ QCOW2_DISCARD_ALWAYS);
if (ret >= 0) {
(*num_fixed)++;
continue;
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 992a5c8..0caac90 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -262,7 +262,8 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
}
/* free the old snapshot table */
- qcow2_free_clusters(bs, s->snapshots_offset, s->snapshots_size);
+ qcow2_free_clusters(bs, s->snapshots_offset, s->snapshots_size,
+ QCOW2_DISCARD_SNAPSHOT);
s->snapshots_offset = snapshots_offset;
s->snapshots_size = snapshots_size;
return 0;
@@ -569,7 +570,8 @@ int qcow2_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
if (ret < 0) {
return ret;
}
- qcow2_free_clusters(bs, sn.l1_table_offset, sn.l1_size * sizeof(uint64_t));
+ qcow2_free_clusters(bs, sn.l1_table_offset, sn.l1_size * sizeof(uint64_t),
+ QCOW2_DISCARD_SNAPSHOT);
/* must update the copied flag on the current cluster offsets */
ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset, s->l1_size, 0);
diff --git a/block/qcow2.c b/block/qcow2.c
index 0fa5cb2..e28ea47 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1196,7 +1196,8 @@ static int preallocate(BlockDriverState *bs)
ret = qcow2_alloc_cluster_link_l2(bs, meta);
if (ret < 0) {
- qcow2_free_any_clusters(bs, meta->alloc_offset, meta->nb_clusters);
+ qcow2_free_any_clusters(bs, meta->alloc_offset, meta->nb_clusters,
+ QCOW2_DISCARD_NEVER);
return ret;
}
diff --git a/block/qcow2.h b/block/qcow2.h
index 6959c6a..64a6479 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -129,6 +129,15 @@ enum {
QCOW2_COMPAT_FEAT_MASK = QCOW2_COMPAT_LAZY_REFCOUNTS,
};
+enum qcow2_discard_type {
+ QCOW2_DISCARD_NEVER = 0,
+ QCOW2_DISCARD_ALWAYS,
+ QCOW2_DISCARD_REQUEST,
+ QCOW2_DISCARD_SNAPSHOT,
+ QCOW2_DISCARD_OTHER,
+ QCOW2_DISCARD_MAX
+};
+
typedef struct Qcow2Feature {
uint8_t type;
uint8_t bit;
@@ -349,9 +358,10 @@ int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset,
int nb_clusters);
int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size);
void qcow2_free_clusters(BlockDriverState *bs,
- int64_t offset, int64_t size);
-void qcow2_free_any_clusters(BlockDriverState *bs,
- uint64_t cluster_offset, int nb_clusters);
+ int64_t offset, int64_t size,
+ enum qcow2_discard_type type);
+void qcow2_free_any_clusters(BlockDriverState *bs, uint64_t l2_entry,
+ int nb_clusters, enum qcow2_discard_type type);
int qcow2_update_snapshot_refcount(BlockDriverState *bs,
int64_t l1_table_offset, int l1_size, int addend);
--
1.8.1.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [Qemu-devel] [PATCH 20/23] qcow2: Options to enable discard for freed clusters
2013-06-24 9:10 [Qemu-devel] [PULL 00/23] Block patches Stefan Hajnoczi
` (18 preceding siblings ...)
2013-06-24 9:10 ` [Qemu-devel] [PATCH 19/23] qcow2: Add refcount update reason to all callers Stefan Hajnoczi
@ 2013-06-24 9:10 ` Stefan Hajnoczi
2013-06-24 9:10 ` [Qemu-devel] [PATCH 21/23] qcow2: Batch discards Stefan Hajnoczi
` (2 subsequent siblings)
22 siblings, 0 replies; 35+ messages in thread
From: Stefan Hajnoczi @ 2013-06-24 9:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi
From: Kevin Wolf <kwolf@redhat.com>
Deleted snapshots are discarded in the image file by default, discard
requests take their default from the -drive discard=... option and other
places that free clusters must always be enabled explicitly.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/qcow2-refcount.c | 5 +++++
block/qcow2.c | 26 ++++++++++++++++++++++++++
block/qcow2.h | 5 +++++
3 files changed, 36 insertions(+)
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 6d35e49..7488988 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -488,6 +488,11 @@ static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
s->free_cluster_index = cluster_index;
}
refcount_block[block_index] = cpu_to_be16(refcount);
+ if (refcount == 0 && s->discard_passthrough[type]) {
+ /* Try discarding, ignore errors */
+ /* FIXME Doing this cluster by cluster will be painfully slow */
+ bdrv_discard(bs->file, cluster_offset, 1);
+ }
}
ret = 0;
diff --git a/block/qcow2.c b/block/qcow2.c
index e28ea47..ef8a2ca 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -295,6 +295,22 @@ static QemuOptsList qcow2_runtime_opts = {
.type = QEMU_OPT_BOOL,
.help = "Postpone refcount updates",
},
+ {
+ .name = QCOW2_OPT_DISCARD_REQUEST,
+ .type = QEMU_OPT_BOOL,
+ .help = "Pass guest discard requests to the layer below",
+ },
+ {
+ .name = QCOW2_OPT_DISCARD_SNAPSHOT,
+ .type = QEMU_OPT_BOOL,
+ .help = "Generate discard requests when snapshot related space "
+ "is freed",
+ },
+ {
+ .name = QCOW2_OPT_DISCARD_OTHER,
+ .type = QEMU_OPT_BOOL,
+ .help = "Generate discard requests when other clusters are freed",
+ },
{ /* end of list */ }
},
};
@@ -532,6 +548,16 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags)
s->use_lazy_refcounts = qemu_opt_get_bool(opts, QCOW2_OPT_LAZY_REFCOUNTS,
(s->compatible_features & QCOW2_COMPAT_LAZY_REFCOUNTS));
+ s->discard_passthrough[QCOW2_DISCARD_NEVER] = false;
+ s->discard_passthrough[QCOW2_DISCARD_ALWAYS] = true;
+ s->discard_passthrough[QCOW2_DISCARD_REQUEST] =
+ qemu_opt_get_bool(opts, QCOW2_OPT_DISCARD_REQUEST,
+ flags & BDRV_O_UNMAP);
+ s->discard_passthrough[QCOW2_DISCARD_SNAPSHOT] =
+ qemu_opt_get_bool(opts, QCOW2_OPT_DISCARD_SNAPSHOT, true);
+ s->discard_passthrough[QCOW2_DISCARD_OTHER] =
+ qemu_opt_get_bool(opts, QCOW2_OPT_DISCARD_OTHER, false);
+
qemu_opts_del(opts);
if (s->use_lazy_refcounts && s->qcow_version < 3) {
diff --git a/block/qcow2.h b/block/qcow2.h
index 64a6479..6f91b9a 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -60,6 +60,9 @@
#define QCOW2_OPT_LAZY_REFCOUNTS "lazy_refcounts"
+#define QCOW2_OPT_DISCARD_REQUEST "pass_discard_request"
+#define QCOW2_OPT_DISCARD_SNAPSHOT "pass_discard_snapshot"
+#define QCOW2_OPT_DISCARD_OTHER "pass_discard_other"
typedef struct QCowHeader {
uint32_t magic;
@@ -187,6 +190,8 @@ typedef struct BDRVQcowState {
int qcow_version;
bool use_lazy_refcounts;
+ bool discard_passthrough[QCOW2_DISCARD_MAX];
+
uint64_t incompatible_features;
uint64_t compatible_features;
uint64_t autoclear_features;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [Qemu-devel] [PATCH 21/23] qcow2: Batch discards
2013-06-24 9:10 [Qemu-devel] [PULL 00/23] Block patches Stefan Hajnoczi
` (19 preceding siblings ...)
2013-06-24 9:10 ` [Qemu-devel] [PATCH 20/23] qcow2: Options to enable discard for freed clusters Stefan Hajnoczi
@ 2013-06-24 9:10 ` Stefan Hajnoczi
2013-06-24 9:10 ` [Qemu-devel] [PATCH 22/23] block: Always enable discard on the protocol level Stefan Hajnoczi
2013-06-24 9:10 ` [Qemu-devel] [PATCH 23/23] vmdk: refuse to open higher version than supported Stefan Hajnoczi
22 siblings, 0 replies; 35+ messages in thread
From: Stefan Hajnoczi @ 2013-06-24 9:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi
From: Kevin Wolf <kwolf@redhat.com>
This optimises the discard operation for freed clusters by batching
discard requests (both snapshot deletion and bdrv_discard end up
updating the refcounts cluster by cluster).
Note that we don't discard asynchronously, but keep s->lock held. This
is to avoid that a freed cluster is reallocated and written to while the
discard is still in flight.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/qcow2-cluster.c | 22 +++++++++++---
block/qcow2-refcount.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++--
block/qcow2.c | 1 +
block/qcow2.h | 11 +++++++
4 files changed, 109 insertions(+), 7 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 3191d6b..cca76d4 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1377,18 +1377,25 @@ int qcow2_discard_clusters(BlockDriverState *bs, uint64_t offset,
nb_clusters = size_to_clusters(s, end_offset - offset);
+ s->cache_discards = true;
+
/* Each L2 table is handled by its own loop iteration */
while (nb_clusters > 0) {
ret = discard_single_l2(bs, offset, nb_clusters);
if (ret < 0) {
- return ret;
+ goto fail;
}
nb_clusters -= ret;
offset += (ret * s->cluster_size);
}
- return 0;
+ ret = 0;
+fail:
+ s->cache_discards = false;
+ qcow2_process_discards(bs, ret);
+
+ return ret;
}
/*
@@ -1450,15 +1457,22 @@ int qcow2_zero_clusters(BlockDriverState *bs, uint64_t offset, int nb_sectors)
/* Each L2 table is handled by its own loop iteration */
nb_clusters = size_to_clusters(s, nb_sectors << BDRV_SECTOR_BITS);
+ s->cache_discards = true;
+
while (nb_clusters > 0) {
ret = zero_single_l2(bs, offset, nb_clusters);
if (ret < 0) {
- return ret;
+ goto fail;
}
nb_clusters -= ret;
offset += (ret * s->cluster_size);
}
- return 0;
+ ret = 0;
+fail:
+ s->cache_discards = false;
+ qcow2_process_discards(bs, ret);
+
+ return ret;
}
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 7488988..1244693 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -420,6 +420,74 @@ fail_block:
return ret;
}
+void qcow2_process_discards(BlockDriverState *bs, int ret)
+{
+ BDRVQcowState *s = bs->opaque;
+ Qcow2DiscardRegion *d, *next;
+
+ QTAILQ_FOREACH_SAFE(d, &s->discards, next, next) {
+ QTAILQ_REMOVE(&s->discards, d, next);
+
+ /* Discard is optional, ignore the return value */
+ if (ret >= 0) {
+ bdrv_discard(bs->file,
+ d->offset >> BDRV_SECTOR_BITS,
+ d->bytes >> BDRV_SECTOR_BITS);
+ }
+
+ g_free(d);
+ }
+}
+
+static void update_refcount_discard(BlockDriverState *bs,
+ uint64_t offset, uint64_t length)
+{
+ BDRVQcowState *s = bs->opaque;
+ Qcow2DiscardRegion *d, *p, *next;
+
+ QTAILQ_FOREACH(d, &s->discards, next) {
+ uint64_t new_start = MIN(offset, d->offset);
+ uint64_t new_end = MAX(offset + length, d->offset + d->bytes);
+
+ if (new_end - new_start <= length + d->bytes) {
+ /* There can't be any overlap, areas ending up here have no
+ * references any more and therefore shouldn't get freed another
+ * time. */
+ assert(d->bytes + length == new_end - new_start);
+ d->offset = new_start;
+ d->bytes = new_end - new_start;
+ goto found;
+ }
+ }
+
+ d = g_malloc(sizeof(*d));
+ *d = (Qcow2DiscardRegion) {
+ .bs = bs,
+ .offset = offset,
+ .bytes = length,
+ };
+ QTAILQ_INSERT_TAIL(&s->discards, d, next);
+
+found:
+ /* Merge discard requests if they are adjacent now */
+ QTAILQ_FOREACH_SAFE(p, &s->discards, next, next) {
+ if (p == d
+ || p->offset > d->offset + d->bytes
+ || d->offset > p->offset + p->bytes)
+ {
+ continue;
+ }
+
+ /* Still no overlap possible */
+ assert(p->offset == d->offset + d->bytes
+ || d->offset == p->offset + p->bytes);
+
+ QTAILQ_REMOVE(&s->discards, p, next);
+ d->offset = MIN(d->offset, p->offset);
+ d->bytes += p->bytes;
+ }
+}
+
/* XXX: cache several refcount block clusters ? */
static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
int64_t offset, int64_t length, int addend, enum qcow2_discard_type type)
@@ -488,15 +556,18 @@ static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
s->free_cluster_index = cluster_index;
}
refcount_block[block_index] = cpu_to_be16(refcount);
+
if (refcount == 0 && s->discard_passthrough[type]) {
- /* Try discarding, ignore errors */
- /* FIXME Doing this cluster by cluster will be painfully slow */
- bdrv_discard(bs->file, cluster_offset, 1);
+ update_refcount_discard(bs, cluster_offset, s->cluster_size);
}
}
ret = 0;
fail:
+ if (!s->cache_discards) {
+ qcow2_process_discards(bs, ret);
+ }
+
/* Write last changed block to disk */
if (refcount_block) {
int wret;
@@ -755,6 +826,8 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
l1_table = NULL;
l1_size2 = l1_size * sizeof(uint64_t);
+ s->cache_discards = true;
+
/* WARNING: qcow2_snapshot_goto relies on this function not using the
* l1_table_offset when it is the current s->l1_table_offset! Be careful
* when changing this! */
@@ -867,6 +940,9 @@ fail:
qcow2_cache_put(bs, s->l2_table_cache, (void**) &l2_table);
}
+ s->cache_discards = false;
+ qcow2_process_discards(bs, ret);
+
/* Update L1 only if it isn't deleted anyway (addend = -1) */
if (ret == 0 && addend >= 0 && l1_modified) {
for (i = 0; i < l1_size; i++) {
diff --git a/block/qcow2.c b/block/qcow2.c
index ef8a2ca..9383990 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -486,6 +486,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags)
}
QLIST_INIT(&s->cluster_allocs);
+ QTAILQ_INIT(&s->discards);
/* read qcow2 extensions */
if (qcow2_read_extensions(bs, header.header_length, ext_end, NULL)) {
diff --git a/block/qcow2.h b/block/qcow2.h
index 6f91b9a..3b2d5cd 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -147,6 +147,13 @@ typedef struct Qcow2Feature {
char name[46];
} QEMU_PACKED Qcow2Feature;
+typedef struct Qcow2DiscardRegion {
+ BlockDriverState *bs;
+ uint64_t offset;
+ uint64_t bytes;
+ QTAILQ_ENTRY(Qcow2DiscardRegion) next;
+} Qcow2DiscardRegion;
+
typedef struct BDRVQcowState {
int cluster_bits;
int cluster_size;
@@ -199,6 +206,8 @@ typedef struct BDRVQcowState {
size_t unknown_header_fields_size;
void* unknown_header_fields;
QLIST_HEAD(, Qcow2UnknownHeaderExtension) unknown_header_ext;
+ QTAILQ_HEAD (, Qcow2DiscardRegion) discards;
+ bool cache_discards;
} BDRVQcowState;
/* XXX: use std qcow open function ? */
@@ -374,6 +383,8 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
BdrvCheckMode fix);
+void qcow2_process_discards(BlockDriverState *bs, int ret);
+
/* qcow2-cluster.c functions */
int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
bool exact_size);
--
1.8.1.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [Qemu-devel] [PATCH 22/23] block: Always enable discard on the protocol level
2013-06-24 9:10 [Qemu-devel] [PULL 00/23] Block patches Stefan Hajnoczi
` (20 preceding siblings ...)
2013-06-24 9:10 ` [Qemu-devel] [PATCH 21/23] qcow2: Batch discards Stefan Hajnoczi
@ 2013-06-24 9:10 ` Stefan Hajnoczi
2013-06-24 9:10 ` [Qemu-devel] [PATCH 23/23] vmdk: refuse to open higher version than supported Stefan Hajnoczi
22 siblings, 0 replies; 35+ messages in thread
From: Stefan Hajnoczi @ 2013-06-24 9:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi
From: Kevin Wolf <kwolf@redhat.com>
Turning on discard options in qcow2 doesn't help a lot when the discard
requests that it issues are thrown away by the raw-posix layer. This
patch always enables discard functionality on the protocol level so that
it's the image format's responsibility to send (or not) discard
requests. Requests sent by the guest will be allowed or ignored by the
top level BlockDriverState, which depends on the discard=... option like
before.
In particular, this means that even without specifying options, the
qcow2 default of discarding deleted snapshots actually takes effect now,
both for qemu and qemu-img.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block.c b/block.c
index b88ad2f..8e77d46 100644
--- a/block.c
+++ b/block.c
@@ -1045,7 +1045,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
extract_subqdict(options, &file_options, "file.");
ret = bdrv_file_open(&file, filename, file_options,
- bdrv_open_flags(bs, flags));
+ bdrv_open_flags(bs, flags | BDRV_O_UNMAP));
if (ret < 0) {
goto fail;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [Qemu-devel] [PATCH 23/23] vmdk: refuse to open higher version than supported
2013-06-24 9:10 [Qemu-devel] [PULL 00/23] Block patches Stefan Hajnoczi
` (21 preceding siblings ...)
2013-06-24 9:10 ` [Qemu-devel] [PATCH 22/23] block: Always enable discard on the protocol level Stefan Hajnoczi
@ 2013-06-24 9:10 ` Stefan Hajnoczi
22 siblings, 0 replies; 35+ messages in thread
From: Stefan Hajnoczi @ 2013-06-24 9:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Anthony Liguori, Fam Zheng, Stefan Hajnoczi
From: Fam Zheng <famz@redhat.com>
Refuse to open higher version for safety.
Although we try to be compatible with published VMDK spec, VMware has
newer version from ESXi 5.1 exported OVF/OVA, which we have no knowledge
what's changed in it. And it is very likely to have more new versions in
the future, so it's not safe to open them blindly.
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/vmdk.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/block/vmdk.c b/block/vmdk.c
index 65ae011..975e1d4 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -561,6 +561,15 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
header = footer.header;
}
+ if (le32_to_cpu(header.version) >= 3) {
+ char buf[64];
+ snprintf(buf, sizeof(buf), "VMDK version %d",
+ le32_to_cpu(header.version));
+ qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
+ bs->device_name, "vmdk", buf);
+ return -ENOTSUP;
+ }
+
l1_entry_sectors = le32_to_cpu(header.num_gtes_per_gte)
* le64_to_cpu(header.granularity);
if (l1_entry_sectors == 0) {
--
1.8.1.4
^ permalink raw reply related [flat|nested] 35+ messages in thread