From e7bd708ec85e40fd51569bb90c52d6613ffd8f45 Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 14 Nov 2016 11:15:54 -0500 Subject: [PATCH 1/9] atapi: classify read_cd as conditionally returning data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For the purposes of byte_count_limit verification, add a new flag that identifies read_cd as sometimes returning data, then check the BCL in its command handler after we know that it will indeed return data. Reported-by: Hervé Poussineau Signed-off-by: John Snow Reviewed-by: Kevin Wolf Message-id: 1477970211-25754-2-git-send-email-jsnow@redhat.com Signed-off-by: John Snow --- hw/ide/atapi.c | 51 +++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 40 insertions(+), 11 deletions(-) diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c index 6189675036..fc1d19c6d4 100644 --- a/hw/ide/atapi.c +++ b/hw/ide/atapi.c @@ -637,6 +637,23 @@ static unsigned int event_status_media(IDEState *s, return 8; /* We wrote to 4 extra bytes from the header */ } +/* + * Before transferring data or otherwise signalling acceptance of a command + * marked CONDDATA, we must check the validity of the byte_count_limit. + */ +static bool validate_bcl(IDEState *s) +{ + /* TODO: Check IDENTIFY data word 125 for defacult BCL (currently 0) */ + if (s->atapi_dma || atapi_byte_count_limit(s)) { + return true; + } + + /* TODO: Move abort back into core.c and introduce proper error flow between + * ATAPI layer and IDE core layer */ + ide_abort_command(s); + return false; +} + static void cmd_get_event_status_notification(IDEState *s, uint8_t *buf) { @@ -1028,12 +1045,19 @@ static void cmd_read_cd(IDEState *s, uint8_t* buf) return; } - transfer_request = buf[9]; - switch(transfer_request & 0xf8) { - case 0x00: + transfer_request = buf[9] & 0xf8; + if (transfer_request == 0x00) { /* nothing */ ide_atapi_cmd_ok(s); - break; + return; + } + + /* Check validity of BCL before transferring data */ + if (!validate_bcl(s)) { + return; + } + + switch (transfer_request) { case 0x10: /* normal read */ ide_atapi_cmd_read(s, lba, nb_sectors, 2048); @@ -1266,6 +1290,14 @@ enum { * See ATA8-ACS3 "7.21.5 Byte Count Limit" */ NONDATA = 0x04, + + /* + * CONDDATA implies a command that transfers data only conditionally based + * on the presence of suboptions. It should be exempt from the BCL check at + * command validation time, but it needs to be checked at the command + * handler level instead. + */ + CONDDATA = 0x08, }; static const struct AtapiCmd { @@ -1289,7 +1321,7 @@ static const struct AtapiCmd { [ 0xad ] = { cmd_read_dvd_structure, CHECK_READY }, [ 0xbb ] = { cmd_set_speed, NONDATA }, [ 0xbd ] = { cmd_mechanism_status, 0 }, - [ 0xbe ] = { cmd_read_cd, CHECK_READY }, + [ 0xbe ] = { cmd_read_cd, CHECK_READY | CONDDATA }, /* [1] handler detects and reports not ready condition itself */ }; @@ -1348,15 +1380,12 @@ void ide_atapi_cmd(IDEState *s) return; } - /* Nondata commands permit the byte_count_limit to be 0. + /* Commands that don't transfer DATA permit the byte_count_limit to be 0. * If this is a data-transferring PIO command and BCL is 0, * we abort at the /ATA/ level, not the ATAPI level. * See ATA8 ACS3 section 7.17.6.49 and 7.21.5 */ - if (cmd->handler && !(cmd->flags & NONDATA)) { - /* TODO: Check IDENTIFY data word 125 for default BCL (currently 0) */ - if (!(atapi_byte_count_limit(s) || s->atapi_dma)) { - /* TODO: Move abort back into core.c and make static inline again */ - ide_abort_command(s); + if (cmd->handler && !(cmd->flags & (NONDATA | CONDDATA))) { + if (!validate_bcl(s)) { return; } } From 53c05e6c20509adcff40fe1c7a02210354fe53f7 Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 14 Nov 2016 11:15:54 -0500 Subject: [PATCH 2/9] ahci-test: Create smaller test ISO images These can simply be the size of the number of sectors we're reading, plus one for a buffer. We don't need them to be any larger. Signed-off-by: John Snow Reviewed-by: Kevin Wolf Message-id: 1477970211-25754-3-git-send-email-jsnow@redhat.com Signed-off-by: John Snow --- tests/ahci-test.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/ahci-test.c b/tests/ahci-test.c index 70bcafa9e4..a271cad7ab 100644 --- a/tests/ahci-test.c +++ b/tests/ahci-test.c @@ -1494,9 +1494,10 @@ static void ahci_test_cdrom(int nsectors, bool dma) .atapi_dma = dma, .post_cb = ahci_cb_cmp_buff, }; + uint64_t iso_size = ATAPI_SECTOR_SIZE * (nsectors + 1); /* Prepare ISO and fill 'tx' buffer */ - fd = prepare_iso(1024 * 1024, &tx, &iso); + fd = prepare_iso(iso_size, &tx, &iso); opts.opaque = tx; /* Standard startup wonkery, but use ide-cd and our special iso file */ From ebde93bf9a13f2e0a853eac8fb4f33c9ecd74baf Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 14 Nov 2016 11:15:54 -0500 Subject: [PATCH 3/9] ahci-test: test atapi read_cd with bcl, nb_sectors = 0 Commit 9ef2e93f introduced the concept of tagging ATAPI commands as NONDATA, but this introduced a regression for certain commands better described as CONDDATA. read_cd is such a command that both requires a non-zero BCL if a transfer size is set, but is perfectly content to accept a zero BCL if the transfer size is 0. This test adds a regression test for the case where BCL and nb_sectors are both 0. Flesh out the CDROM tests by: (1) Allowing the test to specify a BCL (2) Allowing the buffer comparison test to compare a 0-size buffer (3) Fix the BCL specification in libqos (It is LE, not BE) (4) Add a nice human-readable message for future SCSI command additions Signed-off-by: John Snow Reviewed-by: Kevin Wolf Message-id: 1477970211-25754-4-git-send-email-jsnow@redhat.com [Line length edit --js] Signed-off-by: John Snow --- tests/ahci-test.c | 36 +++++++++++++++++++++++++++++------- tests/libqos/ahci.c | 28 +++++++++++++++++++++------- tests/libqos/ahci.h | 17 ++++++++++------- 3 files changed, 60 insertions(+), 21 deletions(-) diff --git a/tests/ahci-test.c b/tests/ahci-test.c index a271cad7ab..0b1b6f7201 100644 --- a/tests/ahci-test.c +++ b/tests/ahci-test.c @@ -1473,8 +1473,13 @@ static int ahci_cb_cmp_buff(AHCIQState *ahci, AHCICommand *cmd, const AHCIOpts *opts) { unsigned char *tx = opts->opaque; - unsigned char *rx = g_malloc0(opts->size); + unsigned char *rx; + if (!opts->size) { + return 0; + } + + rx = g_malloc0(opts->size); bufread(opts->buffer, rx, opts->size); g_assert_cmphex(memcmp(tx, rx, opts->size), ==, 0); g_free(rx); @@ -1482,7 +1487,8 @@ static int ahci_cb_cmp_buff(AHCIQState *ahci, AHCICommand *cmd, return 0; } -static void ahci_test_cdrom(int nsectors, bool dma) +static void ahci_test_cdrom(int nsectors, bool dma, uint8_t cmd, + bool override_bcl, uint16_t bcl) { AHCIQState *ahci; unsigned char *tx; @@ -1493,6 +1499,8 @@ static void ahci_test_cdrom(int nsectors, bool dma) .atapi = true, .atapi_dma = dma, .post_cb = ahci_cb_cmp_buff, + .set_bcl = override_bcl, + .bcl = bcl, }; uint64_t iso_size = ATAPI_SECTOR_SIZE * (nsectors + 1); @@ -1506,7 +1514,7 @@ static void ahci_test_cdrom(int nsectors, bool dma) "-device ide-cd,drive=drive0 ", iso); /* Build & Send AHCI command */ - ahci_exec(ahci, ahci_port_select(ahci), CMD_ATAPI_READ_10, &opts); + ahci_exec(ahci, ahci_port_select(ahci), cmd, &opts); /* Cleanup */ g_free(tx); @@ -1514,24 +1522,36 @@ static void ahci_test_cdrom(int nsectors, bool dma) remove_iso(fd, iso); } +static void ahci_test_cdrom_read10(int nsectors, bool dma) +{ + ahci_test_cdrom(nsectors, dma, CMD_ATAPI_READ_10, false, 0); +} + static void test_cdrom_dma(void) { - ahci_test_cdrom(1, true); + ahci_test_cdrom_read10(1, true); } static void test_cdrom_dma_multi(void) { - ahci_test_cdrom(3, true); + ahci_test_cdrom_read10(3, true); } static void test_cdrom_pio(void) { - ahci_test_cdrom(1, false); + ahci_test_cdrom_read10(1, false); } static void test_cdrom_pio_multi(void) { - ahci_test_cdrom(3, false); + ahci_test_cdrom_read10(3, false); +} + +/* Regression test: Test that a READ_CD command with a BCL of 0 but a size of 0 + * completes as a NOP instead of erroring out. */ +static void test_atapi_bcl(void) +{ + ahci_test_cdrom(0, false, CMD_ATAPI_READ_CD, true, 0); } /******************************************************************************/ @@ -1823,6 +1843,8 @@ int main(int argc, char **argv) qtest_add_func("/ahci/cdrom/pio/single", test_cdrom_pio); qtest_add_func("/ahci/cdrom/pio/multi", test_cdrom_pio_multi); + qtest_add_func("/ahci/cdrom/pio/bcl", test_atapi_bcl); + ret = g_test_run(); /* Cleanup */ diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c index 5180d65279..0e9354bf13 100644 --- a/tests/libqos/ahci.c +++ b/tests/libqos/ahci.c @@ -633,7 +633,8 @@ void ahci_exec(AHCIQState *ahci, uint8_t port, /* Command creation */ if (opts->atapi) { - cmd = ahci_atapi_command_create(op); + uint16_t bcl = opts->set_bcl ? opts->bcl : ATAPI_SECTOR_SIZE; + cmd = ahci_atapi_command_create(op, bcl); if (opts->atapi_dma) { ahci_command_enable_atapi_dma(cmd); } @@ -864,16 +865,12 @@ AHCICommand *ahci_command_create(uint8_t command_name) return cmd; } -AHCICommand *ahci_atapi_command_create(uint8_t scsi_cmd) +AHCICommand *ahci_atapi_command_create(uint8_t scsi_cmd, uint16_t bcl) { AHCICommand *cmd = ahci_command_create(CMD_PACKET); cmd->atapi_cmd = g_malloc0(16); cmd->atapi_cmd[0] = scsi_cmd; - /* ATAPI needs a PIO transfer chunk size set inside of the LBA registers. - * The block/sector size is a natural default. */ - cmd->fis.lba_lo[1] = ATAPI_SECTOR_SIZE >> 8 & 0xFF; - cmd->fis.lba_lo[2] = ATAPI_SECTOR_SIZE & 0xFF; - + stw_le_p(&cmd->fis.lba_lo[1], bcl); return cmd; } @@ -901,12 +898,17 @@ static void ahci_atapi_command_set_offset(AHCICommand *cmd, uint64_t lba) switch (cbd[0]) { case CMD_ATAPI_READ_10: + case CMD_ATAPI_READ_CD: g_assert_cmpuint(lba, <=, UINT32_MAX); stl_be_p(&cbd[2], lba); break; default: /* SCSI doesn't have uniform packet formats, * so you have to add support for it manually. Sorry! */ + fprintf(stderr, "The Libqos AHCI driver does not support the " + "set_offset operation for ATAPI command 0x%02x, " + "please add support.\n", + cbd[0]); g_assert_not_reached(); } } @@ -951,6 +953,7 @@ static void ahci_atapi_set_size(AHCICommand *cmd, uint64_t xbytes) { unsigned char *cbd = cmd->atapi_cmd; uint64_t nsectors = xbytes / 2048; + uint32_t tmp; g_assert(cbd); switch (cbd[0]) { @@ -958,9 +961,20 @@ static void ahci_atapi_set_size(AHCICommand *cmd, uint64_t xbytes) g_assert_cmpuint(nsectors, <=, UINT16_MAX); stw_be_p(&cbd[7], nsectors); break; + case CMD_ATAPI_READ_CD: + /* 24bit BE store */ + g_assert_cmpuint(nsectors, <, 1ULL << 24); + tmp = nsectors; + cbd[6] = (tmp & 0xFF0000) >> 16; + cbd[7] = (tmp & 0xFF00) >> 8; + cbd[8] = (tmp & 0xFF); + break; default: /* SCSI doesn't have uniform packet formats, * so you have to add support for it manually. Sorry! */ + fprintf(stderr, "The Libqos AHCI driver does not support the set_size " + "operation for ATAPI command 0x%02x, please add support.\n", + cbd[0]); g_assert_not_reached(); } } diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h index caaafe3fdf..f144fab37a 100644 --- a/tests/libqos/ahci.h +++ b/tests/libqos/ahci.h @@ -288,6 +288,7 @@ enum { /* ATAPI Commands */ enum { CMD_ATAPI_READ_10 = 0x28, + CMD_ATAPI_READ_CD = 0xbe, }; /* AHCI Command Header Flags & Masks*/ @@ -462,12 +463,14 @@ typedef struct AHCICommand AHCICommand; /* Options to ahci_exec */ typedef struct AHCIOpts { - size_t size; - unsigned prd_size; - uint64_t lba; - uint64_t buffer; - bool atapi; - bool atapi_dma; + size_t size; /* Size of transfer */ + unsigned prd_size; /* Size per-each PRD */ + bool set_bcl; /* Override the default BCL of ATAPI_SECTOR_SIZE */ + unsigned bcl; /* Byte Count Limit, for ATAPI PIO */ + uint64_t lba; /* Starting LBA offset */ + uint64_t buffer; /* Pointer to source or destination guest buffer */ + bool atapi; /* ATAPI command? */ + bool atapi_dma; /* Use DMA for ATAPI? */ bool error; int (*pre_cb)(AHCIQState*, AHCICommand*, const struct AHCIOpts *); int (*mid_cb)(AHCIQState*, AHCICommand*, const struct AHCIOpts *); @@ -599,7 +602,7 @@ void ahci_exec(AHCIQState *ahci, uint8_t port, /* Command: Fine-grained lifecycle */ AHCICommand *ahci_command_create(uint8_t command_name); -AHCICommand *ahci_atapi_command_create(uint8_t scsi_cmd); +AHCICommand *ahci_atapi_command_create(uint8_t scsi_cmd, uint16_t bcl); void ahci_command_commit(AHCIQState *ahci, AHCICommand *cmd, uint8_t port); void ahci_command_issue(AHCIQState *ahci, AHCICommand *cmd); void ahci_command_issue_async(AHCIQState *ahci, AHCICommand *cmd); From c47ee043dc2cc85da710e87524144a720598c096 Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 14 Nov 2016 11:15:54 -0500 Subject: [PATCH 4/9] block-backend: Always notify on blk_eject blk_eject is only used by scsi-disk and atapi, and in both cases we only attempt to invoke blk_eject if we have a bona-fide change in tray state. The "issue" here is that the tray state does not generate a QMP event unless there is a medium/BDS attached to the device, so if libvirt et al are waiting for a tray event to occur from an empty-but-closed drive, software opening that drive will not emit an event and libvirt will wait forever. Change this by modifying blk_eject to always emit an event, instead of conditionally on a "real" backend eject. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1373264 Reported-by: Peter Krempa Signed-off-by: John Snow Reviewed-by: Eric Blake Reviewed-by: Kevin Wolf Message-id: 1478553214-497-2-git-send-email-jsnow@redhat.com Signed-off-by: John Snow --- block/block-backend.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/block/block-backend.c b/block/block-backend.c index 27a7f6f523..efbf398bb5 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -1393,13 +1393,14 @@ void blk_eject(BlockBackend *blk, bool eject_flag) if (bs) { bdrv_eject(bs, eject_flag); - - id = blk_get_attached_dev_id(blk); - qapi_event_send_device_tray_moved(blk_name(blk), id, - eject_flag, &error_abort); - g_free(id); - } + + /* Whether or not we ejected on the backend, + * the frontend experienced a tray event. */ + id = blk_get_attached_dev_id(blk); + qapi_event_send_device_tray_moved(blk_name(blk), id, + eject_flag, &error_abort); + g_free(id); } int blk_get_flags(BlockBackend *blk) From 7ffe3124edd4cfb68870f62263cd9830b68a7a46 Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 14 Nov 2016 11:15:54 -0500 Subject: [PATCH 5/9] libqtest: add qmp_eventwait_ref Wait for an event, but return a copy so we can investigate parameters. Signed-off-by: John Snow Reviewed-by: Eric Blake Reviewed-by: Kevin Wolf Message-id: 1478553214-497-3-git-send-email-jsnow@redhat.com Signed-off-by: John Snow --- tests/libqtest.c | 13 ++++++++++--- tests/libqtest.h | 22 ++++++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/tests/libqtest.c b/tests/libqtest.c index d4e6bff121..6f6975248f 100644 --- a/tests/libqtest.c +++ b/tests/libqtest.c @@ -533,7 +533,7 @@ void qtest_qmp_discard_response(QTestState *s, const char *fmt, ...) QDECREF(response); } -void qtest_qmp_eventwait(QTestState *s, const char *event) +QDict *qtest_qmp_eventwait_ref(QTestState *s, const char *event) { QDict *response; @@ -541,13 +541,20 @@ void qtest_qmp_eventwait(QTestState *s, const char *event) response = qtest_qmp_receive(s); if ((qdict_haskey(response, "event")) && (strcmp(qdict_get_str(response, "event"), event) == 0)) { - QDECREF(response); - break; + return response; } QDECREF(response); } } +void qtest_qmp_eventwait(QTestState *s, const char *event) +{ + QDict *response; + + response = qtest_qmp_eventwait_ref(s, event); + QDECREF(response); +} + char *qtest_hmpv(QTestState *s, const char *fmt, va_list ap) { char *cmd; diff --git a/tests/libqtest.h b/tests/libqtest.h index 0224f06d65..90f182e1d8 100644 --- a/tests/libqtest.h +++ b/tests/libqtest.h @@ -113,6 +113,16 @@ QDict *qtest_qmp_receive(QTestState *s); */ void qtest_qmp_eventwait(QTestState *s, const char *event); +/** + * qtest_qmp_eventwait_ref: + * @s: #QTestState instance to operate on. + * @s: #event event to wait for. + * + * Continuosly polls for QMP responses until it receives the desired event. + * Returns a copy of the event for further investigation. + */ +QDict *qtest_qmp_eventwait_ref(QTestState *s, const char *event); + /** * qtest_hmpv: * @s: #QTestState instance to operate on. @@ -558,6 +568,18 @@ static inline void qmp_eventwait(const char *event) return qtest_qmp_eventwait(global_qtest, event); } +/** + * qmp_eventwait_ref: + * @s: #event event to wait for. + * + * Continuosly polls for QMP responses until it receives the desired event. + * Returns a copy of the event for further investigation. + */ +static inline QDict *qmp_eventwait_ref(const char *event) +{ + return qtest_qmp_eventwait_ref(global_qtest, event); +} + /** * hmp: * @fmt...: HMP command to send to QEMU From f697b0edea426da261bff7541a66f36266d8edb0 Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 14 Nov 2016 11:15:54 -0500 Subject: [PATCH 6/9] libqos/ahci: Support expected errors Sometimes we know we'll get back an error, so let's have the test framework understand that. Signed-off-by: John Snow Reviewed-by: Kevin Wolf Message-id: 1478553214-497-4-git-send-email-jsnow@redhat.com Signed-off-by: John Snow --- tests/libqos/ahci.c | 16 ++++++++++++---- tests/libqos/ahci.h | 3 ++- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c index 0e9354bf13..99e85d5b31 100644 --- a/tests/libqos/ahci.c +++ b/tests/libqos/ahci.c @@ -86,6 +86,7 @@ struct AHCICommand { uint8_t name; uint8_t port; uint8_t slot; + uint8_t errors; uint32_t interrupts; uint64_t xbytes; uint32_t prd_size; @@ -402,12 +403,14 @@ void ahci_port_clear(AHCIQState *ahci, uint8_t port) /** * Check a port for errors. */ -void ahci_port_check_error(AHCIQState *ahci, uint8_t port) +void ahci_port_check_error(AHCIQState *ahci, uint8_t port, + uint32_t imask, uint8_t emask) { uint32_t reg; /* The upper 9 bits of the IS register all indicate errors. */ reg = ahci_px_rreg(ahci, port, AHCI_PX_IS); + reg &= ~imask; reg >>= 23; g_assert_cmphex(reg, ==, 0); @@ -417,8 +420,13 @@ void ahci_port_check_error(AHCIQState *ahci, uint8_t port) /* The TFD also has two error sections. */ reg = ahci_px_rreg(ahci, port, AHCI_PX_TFD); - ASSERT_BIT_CLEAR(reg, AHCI_PX_TFD_STS_ERR); - ASSERT_BIT_CLEAR(reg, AHCI_PX_TFD_ERR); + if (!emask) { + ASSERT_BIT_CLEAR(reg, AHCI_PX_TFD_STS_ERR); + } else { + ASSERT_BIT_SET(reg, AHCI_PX_TFD_STS_ERR); + } + ASSERT_BIT_CLEAR(reg, AHCI_PX_TFD_ERR & (~emask << 8)); + ASSERT_BIT_SET(reg, AHCI_PX_TFD_ERR & (emask << 8)); } void ahci_port_check_interrupts(AHCIQState *ahci, uint8_t port, @@ -1119,7 +1127,7 @@ void ahci_command_verify(AHCIQState *ahci, AHCICommand *cmd) uint8_t slot = cmd->slot; uint8_t port = cmd->port; - ahci_port_check_error(ahci, port); + ahci_port_check_error(ahci, port, cmd->interrupts, cmd->errors); ahci_port_check_interrupts(ahci, port, cmd->interrupts); ahci_port_check_nonbusy(ahci, port, slot); ahci_port_check_cmd_sanity(ahci, cmd); diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h index f144fab37a..bbe04f834a 100644 --- a/tests/libqos/ahci.h +++ b/tests/libqos/ahci.h @@ -576,7 +576,8 @@ void ahci_set_command_header(AHCIQState *ahci, uint8_t port, void ahci_destroy_command(AHCIQState *ahci, uint8_t port, uint8_t slot); /* AHCI sanity check routines */ -void ahci_port_check_error(AHCIQState *ahci, uint8_t port); +void ahci_port_check_error(AHCIQState *ahci, uint8_t port, + uint32_t imask, uint8_t emask); void ahci_port_check_interrupts(AHCIQState *ahci, uint8_t port, uint32_t intr_mask); void ahci_port_check_nonbusy(AHCIQState *ahci, uint8_t port, uint8_t slot); From 48cde0913203079036f2785b6bb274873a1a1db2 Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 14 Nov 2016 11:15:55 -0500 Subject: [PATCH 7/9] libqos/ahci: Add ATAPI tray macros (1) Add START_STOP_UNIT command to ahci-test suite (2) Add eject/start macro commands; this is not a data transfer command so it is not well-served by the existing generic pipeline. Signed-off-by: John Snow Reviewed-by: Kevin Wolf Message-id: 1478553214-497-5-git-send-email-jsnow@redhat.com Signed-off-by: John Snow --- tests/libqos/ahci.c | 30 ++++++++++++++++++++++++++++++ tests/libqos/ahci.h | 7 +++++-- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c index 99e85d5b31..79398b4085 100644 --- a/tests/libqos/ahci.c +++ b/tests/libqos/ahci.c @@ -882,6 +882,30 @@ AHCICommand *ahci_atapi_command_create(uint8_t scsi_cmd, uint16_t bcl) return cmd; } +void ahci_atapi_eject(AHCIQState *ahci, uint8_t port) +{ + AHCICommand *cmd = ahci_atapi_command_create(CMD_ATAPI_START_STOP_UNIT, 0); + ahci_command_set_size(cmd, 0); + + cmd->atapi_cmd[4] = 0x02; /* loej = true */ + ahci_command_commit(ahci, cmd, port); + ahci_command_issue(ahci, cmd); + ahci_command_verify(ahci, cmd); + ahci_command_free(cmd); +} + +void ahci_atapi_load(AHCIQState *ahci, uint8_t port) +{ + AHCICommand *cmd = ahci_atapi_command_create(CMD_ATAPI_START_STOP_UNIT, 0); + ahci_command_set_size(cmd, 0); + + cmd->atapi_cmd[4] = 0x03; /* loej,start = true */ + ahci_command_commit(ahci, cmd, port); + ahci_command_issue(ahci, cmd); + ahci_command_verify(ahci, cmd); + ahci_command_free(cmd); +} + void ahci_command_free(AHCICommand *cmd) { g_free(cmd->atapi_cmd); @@ -910,6 +934,9 @@ static void ahci_atapi_command_set_offset(AHCICommand *cmd, uint64_t lba) g_assert_cmpuint(lba, <=, UINT32_MAX); stl_be_p(&cbd[2], lba); break; + case CMD_ATAPI_START_STOP_UNIT: + g_assert_cmpuint(lba, ==, 0x00); + break; default: /* SCSI doesn't have uniform packet formats, * so you have to add support for it manually. Sorry! */ @@ -977,6 +1004,9 @@ static void ahci_atapi_set_size(AHCICommand *cmd, uint64_t xbytes) cbd[7] = (tmp & 0xFF00) >> 8; cbd[8] = (tmp & 0xFF); break; + case CMD_ATAPI_START_STOP_UNIT: + g_assert_cmpuint(xbytes, ==, 0); + break; default: /* SCSI doesn't have uniform packet formats, * so you have to add support for it manually. Sorry! */ diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h index bbe04f834a..05ce3de47f 100644 --- a/tests/libqos/ahci.h +++ b/tests/libqos/ahci.h @@ -287,8 +287,9 @@ enum { /* ATAPI Commands */ enum { - CMD_ATAPI_READ_10 = 0x28, - CMD_ATAPI_READ_CD = 0xbe, + CMD_ATAPI_START_STOP_UNIT = 0x1b, + CMD_ATAPI_READ_10 = 0x28, + CMD_ATAPI_READ_CD = 0xbe, }; /* AHCI Command Header Flags & Masks*/ @@ -600,6 +601,8 @@ void ahci_io(AHCIQState *ahci, uint8_t port, uint8_t ide_cmd, void *buffer, size_t bufsize, uint64_t sector); void ahci_exec(AHCIQState *ahci, uint8_t port, uint8_t op, const AHCIOpts *opts); +void ahci_atapi_eject(AHCIQState *ahci, uint8_t port); +void ahci_atapi_load(AHCIQState *ahci, uint8_t port); /* Command: Fine-grained lifecycle */ AHCICommand *ahci_command_create(uint8_t command_name); From e0a4cb2c7da23c2f0e6364214de5d84f35ce4d5d Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 14 Nov 2016 11:15:55 -0500 Subject: [PATCH 8/9] libqos/ahci: Add get_sense and test_ready Required for tray tests once a medium may have changed. Signed-off-by: John Snow Reviewed-by: Kevin Wolf Message-id: 1478553214-497-6-git-send-email-jsnow@redhat.com [Line length edit --js] Signed-off-by: John Snow --- tests/libqos/ahci.c | 50 +++++++++++++++++++++++++++++++++++++++++++++ tests/libqos/ahci.h | 17 +++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c index 79398b4085..1ca7f456b5 100644 --- a/tests/libqos/ahci.c +++ b/tests/libqos/ahci.c @@ -882,6 +882,49 @@ AHCICommand *ahci_atapi_command_create(uint8_t scsi_cmd, uint16_t bcl) return cmd; } +void ahci_atapi_test_ready(AHCIQState *ahci, uint8_t port, + bool ready, uint8_t expected_sense) +{ + AHCICommand *cmd = ahci_atapi_command_create(CMD_ATAPI_TEST_UNIT_READY, 0); + ahci_command_set_size(cmd, 0); + if (!ready) { + cmd->interrupts |= AHCI_PX_IS_TFES; + cmd->errors |= expected_sense << 4; + } + ahci_command_commit(ahci, cmd, port); + ahci_command_issue(ahci, cmd); + ahci_command_verify(ahci, cmd); + ahci_command_free(cmd); +} + +static int copy_buffer(AHCIQState *ahci, AHCICommand *cmd, + const AHCIOpts *opts) +{ + unsigned char *rx = opts->opaque; + bufread(opts->buffer, rx, opts->size); + return 0; +} + +void ahci_atapi_get_sense(AHCIQState *ahci, uint8_t port, + uint8_t *sense, uint8_t *asc) +{ + unsigned char *rx; + AHCIOpts opts = { + .size = 18, + .atapi = true, + .post_cb = copy_buffer, + }; + rx = g_malloc(18); + opts.opaque = rx; + + ahci_exec(ahci, port, CMD_ATAPI_REQUEST_SENSE, &opts); + + *sense = rx[2]; + *asc = rx[12]; + + g_free(rx); +} + void ahci_atapi_eject(AHCIQState *ahci, uint8_t port) { AHCICommand *cmd = ahci_atapi_command_create(CMD_ATAPI_START_STOP_UNIT, 0); @@ -934,6 +977,8 @@ static void ahci_atapi_command_set_offset(AHCICommand *cmd, uint64_t lba) g_assert_cmpuint(lba, <=, UINT32_MAX); stl_be_p(&cbd[2], lba); break; + case CMD_ATAPI_REQUEST_SENSE: + case CMD_ATAPI_TEST_UNIT_READY: case CMD_ATAPI_START_STOP_UNIT: g_assert_cmpuint(lba, ==, 0x00); break; @@ -1004,6 +1049,11 @@ static void ahci_atapi_set_size(AHCICommand *cmd, uint64_t xbytes) cbd[7] = (tmp & 0xFF00) >> 8; cbd[8] = (tmp & 0xFF); break; + case CMD_ATAPI_REQUEST_SENSE: + g_assert_cmpuint(xbytes, <=, UINT8_MAX); + cbd[4] = (uint8_t)xbytes; + break; + case CMD_ATAPI_TEST_UNIT_READY: case CMD_ATAPI_START_STOP_UNIT: g_assert_cmpuint(xbytes, ==, 0); break; diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h index 05ce3de47f..5f9627bb0f 100644 --- a/tests/libqos/ahci.h +++ b/tests/libqos/ahci.h @@ -287,11 +287,24 @@ enum { /* ATAPI Commands */ enum { + CMD_ATAPI_TEST_UNIT_READY = 0x00, + CMD_ATAPI_REQUEST_SENSE = 0x03, CMD_ATAPI_START_STOP_UNIT = 0x1b, CMD_ATAPI_READ_10 = 0x28, CMD_ATAPI_READ_CD = 0xbe, }; +enum { + SENSE_NO_SENSE = 0x00, + SENSE_NOT_READY = 0x02, + SENSE_UNIT_ATTENTION = 0x06, +}; + +enum { + ASC_MEDIUM_MAY_HAVE_CHANGED = 0x28, + ASC_MEDIUM_NOT_PRESENT = 0x3a, +}; + /* AHCI Command Header Flags & Masks*/ #define CMDH_CFL (0x1F) #define CMDH_ATAPI (0x20) @@ -601,6 +614,10 @@ void ahci_io(AHCIQState *ahci, uint8_t port, uint8_t ide_cmd, void *buffer, size_t bufsize, uint64_t sector); void ahci_exec(AHCIQState *ahci, uint8_t port, uint8_t op, const AHCIOpts *opts); +void ahci_atapi_test_ready(AHCIQState *ahci, uint8_t port, bool ready, + uint8_t expected_sense); +void ahci_atapi_get_sense(AHCIQState *ahci, uint8_t port, + uint8_t *sense, uint8_t *asc); void ahci_atapi_eject(AHCIQState *ahci, uint8_t port); void ahci_atapi_load(AHCIQState *ahci, uint8_t port); From 22381d4180fa71614ad195b54fe81e0ffb77b01e Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 14 Nov 2016 11:15:55 -0500 Subject: [PATCH 9/9] ahci-test: add QMP tray test for ATAPI Test QMP events for a CDROM device with or without a media inserted, including both guest-initiated and hw-initiated eject/load requests. Signed-off-by: John Snow Reviewed-by: Kevin Wolf Message-id: 1478553214-497-7-git-send-email-jsnow@redhat.com Signed-off-by: John Snow --- tests/ahci-test.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/tests/ahci-test.c b/tests/ahci-test.c index 0b1b6f7201..ef17629345 100644 --- a/tests/ahci-test.c +++ b/tests/ahci-test.c @@ -1554,6 +1554,103 @@ static void test_atapi_bcl(void) ahci_test_cdrom(0, false, CMD_ATAPI_READ_CD, true, 0); } + +static void atapi_wait_tray(bool open) +{ + QDict *rsp = qmp_eventwait_ref("DEVICE_TRAY_MOVED"); + QDict *data = qdict_get_qdict(rsp, "data"); + if (open) { + g_assert(qdict_get_bool(data, "tray-open")); + } else { + g_assert(!qdict_get_bool(data, "tray-open")); + } + QDECREF(rsp); +} + +static void test_atapi_tray(void) +{ + AHCIQState *ahci; + unsigned char *tx; + char *iso; + int fd; + uint8_t port, sense, asc; + uint64_t iso_size = ATAPI_SECTOR_SIZE; + QDict *rsp; + + fd = prepare_iso(iso_size, &tx, &iso); + ahci = ahci_boot_and_enable("-drive if=none,id=drive0,file=%s,format=raw " + "-M q35 " + "-device ide-cd,drive=drive0 ", iso); + port = ahci_port_select(ahci); + + ahci_atapi_eject(ahci, port); + atapi_wait_tray(true); + + ahci_atapi_load(ahci, port); + atapi_wait_tray(false); + + /* Remove media */ + qmp_async("{'execute': 'blockdev-open-tray', " + "'arguments': {'device': 'drive0'}}"); + atapi_wait_tray(true); + rsp = qmp_receive(); + QDECREF(rsp); + + qmp_discard_response("{'execute': 'x-blockdev-remove-medium', " + "'arguments': {'device': 'drive0'}}"); + + /* Test the tray without a medium */ + ahci_atapi_load(ahci, port); + atapi_wait_tray(false); + + ahci_atapi_eject(ahci, port); + atapi_wait_tray(true); + + /* Re-insert media */ + qmp_discard_response("{'execute': 'blockdev-add', " + "'arguments': {'node-name': 'node0', " + "'driver': 'raw', " + "'file': { 'driver': 'file', " + "'filename': %s }}}", iso); + qmp_discard_response("{'execute': 'x-blockdev-insert-medium'," + "'arguments': { 'device': 'drive0', " + "'node-name': 'node0' }}"); + + /* Again, the event shows up first */ + qmp_async("{'execute': 'blockdev-close-tray', " + "'arguments': {'device': 'drive0'}}"); + atapi_wait_tray(false); + rsp = qmp_receive(); + QDECREF(rsp); + + /* Now, to convince ATAPI we understand the media has changed... */ + ahci_atapi_test_ready(ahci, port, false, SENSE_NOT_READY); + ahci_atapi_get_sense(ahci, port, &sense, &asc); + g_assert_cmpuint(sense, ==, SENSE_NOT_READY); + g_assert_cmpuint(asc, ==, ASC_MEDIUM_NOT_PRESENT); + + ahci_atapi_test_ready(ahci, port, false, SENSE_UNIT_ATTENTION); + ahci_atapi_get_sense(ahci, port, &sense, &asc); + g_assert_cmpuint(sense, ==, SENSE_UNIT_ATTENTION); + g_assert_cmpuint(asc, ==, ASC_MEDIUM_MAY_HAVE_CHANGED); + + ahci_atapi_test_ready(ahci, port, true, SENSE_NO_SENSE); + ahci_atapi_get_sense(ahci, port, &sense, &asc); + g_assert_cmpuint(sense, ==, SENSE_NO_SENSE); + + /* Final tray test. */ + ahci_atapi_eject(ahci, port); + atapi_wait_tray(true); + + ahci_atapi_load(ahci, port); + atapi_wait_tray(false); + + /* Cleanup */ + g_free(tx); + ahci_shutdown(ahci); + remove_iso(fd, iso); +} + /******************************************************************************/ /* AHCI I/O Test Matrix Definitions */ @@ -1844,6 +1941,7 @@ int main(int argc, char **argv) qtest_add_func("/ahci/cdrom/pio/multi", test_cdrom_pio_multi); qtest_add_func("/ahci/cdrom/pio/bcl", test_atapi_bcl); + qtest_add_func("/ahci/cdrom/eject", test_atapi_tray); ret = g_test_run();