From f40685c62b802c8c3f5c914e8d357dd5c4d4f9cc Mon Sep 17 00:00:00 2001 From: John Snow Date: Fri, 22 May 2015 14:13:42 -0400 Subject: [PATCH 01/19] configure: require glib 2.22 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This provides g_ptr_array_new_with_free_func, as well as a few other functions that we've been hacking around in glib-compat.h. Cleaning up the compatibility headers will come later. Signed-off-by: Paolo Bonzini Signed-off-by: John Snow Reviewed-by: Markus Armbruster Reviewed-by: Alex Bennée Message-id: 1431469140-22208-2-git-send-email-jsnow@redhat.com --- configure | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/configure b/configure index f758f320615f..9852aef52626 100755 --- a/configure +++ b/configure @@ -2779,12 +2779,7 @@ fi ########################################## # glib support probe -if test "$mingw32" = yes; then - # g_poll is required in order to integrate with the glib main loop. - glib_req_ver=2.20 -else - glib_req_ver=2.12 -fi +glib_req_ver=2.22 glib_modules=gthread-2.0 if test "$modules" = yes; then glib_modules="$glib_modules gmodule-2.0" From 62754b157156c2cd26a00ce534aeec9e74619d95 Mon Sep 17 00:00:00 2001 From: John Snow Date: Fri, 22 May 2015 14:13:42 -0400 Subject: [PATCH 02/19] glib: remove stale compat functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since we're bumping the version to 2.22+, remove the now-stale compat functions. Signed-off-by: John Snow Reviewed-by: Markus Armbruster Reviewed-by: Alex Bennée Message-id: 1431469140-22208-2-git-send-email-jsnow@redhat.com --- include/glib-compat.h | 35 ----------------------------------- 1 file changed, 35 deletions(-) diff --git a/include/glib-compat.h b/include/glib-compat.h index 28d9f15bd309..318e0003689d 100644 --- a/include/glib-compat.h +++ b/include/glib-compat.h @@ -23,14 +23,6 @@ #define G_TIME_SPAN_SECOND (G_GINT64_CONSTANT(1000000)) #endif -#if !GLIB_CHECK_VERSION(2, 14, 0) -static inline guint g_timeout_add_seconds(guint interval, GSourceFunc function, - gpointer data) -{ - return g_timeout_add(interval * 1000, function, data); -} -#endif - #if !GLIB_CHECK_VERSION(2, 28, 0) static inline gint64 qemu_g_get_monotonic_time(void) { @@ -47,23 +39,6 @@ static inline gint64 qemu_g_get_monotonic_time(void) #define g_get_monotonic_time() qemu_g_get_monotonic_time() #endif -#if !GLIB_CHECK_VERSION(2, 16, 0) -static inline int g_strcmp0(const char *str1, const char *str2) -{ - int result; - - if (!str1) { - result = -(str1 != str2); - } else if (!str2) { - result = (str1 != str2); - } else { - result = strcmp(str1, str2); - } - - return result; -} -#endif - #ifdef _WIN32 /* * g_poll has a problem on Windows when using @@ -71,16 +46,6 @@ static inline int g_strcmp0(const char *str1, const char *str2) */ #define g_poll(fds, nfds, timeout) g_poll_fixed(fds, nfds, timeout) gint g_poll_fixed(GPollFD *fds, guint nfds, gint timeout); -#elif !GLIB_CHECK_VERSION(2, 20, 0) -/* - * Glib before 2.20.0 doesn't implement g_poll, so wrap it to compile properly - * on older systems. - */ -static inline gint g_poll(GPollFD *fds, guint nfds, gint timeout) -{ - GMainContext *ctx = g_main_context_default(); - return g_main_context_get_poll_func(ctx)(fds, nfds, timeout); -} #endif #if !GLIB_CHECK_VERSION(2, 31, 0) From 008b6e123ff2ee421112768e838b0b44bc7f6c45 Mon Sep 17 00:00:00 2001 From: John Snow Date: Fri, 22 May 2015 14:13:42 -0400 Subject: [PATCH 03/19] libqos/ahci: Add halted command helpers Sometimes we want a command to halt the VM instead of complete successfully, so it'd be nice to let the libqos/ahci functions cope with such scenarios. Signed-off-by: John Snow Reviewed-by: Kevin Wolf Message-id: 1430417242-11859-2-git-send-email-jsnow@redhat.com --- tests/libqos/ahci.c | 27 +++++++++++++++++++++++++++ tests/libqos/ahci.h | 3 +++ 2 files changed, 30 insertions(+) diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c index 843cf72980e6..229409b353ca 100644 --- a/tests/libqos/ahci.c +++ b/tests/libqos/ahci.c @@ -566,6 +566,33 @@ inline unsigned size_to_prdtl(unsigned bytes, unsigned bytes_per_prd) return (bytes + bytes_per_prd - 1) / bytes_per_prd; } +/* Issue a command, expecting it to fail and STOP the VM */ +AHCICommand *ahci_guest_io_halt(AHCIQState *ahci, uint8_t port, + uint8_t ide_cmd, uint64_t buffer, + size_t bufsize, uint64_t sector) +{ + AHCICommand *cmd; + + cmd = ahci_command_create(ide_cmd); + ahci_command_adjust(cmd, sector, buffer, bufsize, 0); + ahci_command_commit(ahci, cmd, port); + ahci_command_issue_async(ahci, cmd); + qmp_eventwait("STOP"); + + return cmd; +} + +/* Resume a previously failed command and verify/finalize */ +void ahci_guest_io_resume(AHCIQState *ahci, AHCICommand *cmd) +{ + /* Complete the command */ + qmp_async("{'execute':'cont' }"); + qmp_eventwait("RESUME"); + ahci_command_wait(ahci, cmd); + ahci_command_verify(ahci, cmd); + ahci_command_free(cmd); +} + /* Given a guest buffer address, perform an IO operation */ void ahci_guest_io(AHCIQState *ahci, uint8_t port, uint8_t ide_cmd, uint64_t buffer, size_t bufsize, uint64_t sector) diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h index 40e8ca48ba85..779e812400ac 100644 --- a/tests/libqos/ahci.h +++ b/tests/libqos/ahci.h @@ -524,6 +524,9 @@ unsigned ahci_pick_cmd(AHCIQState *ahci, uint8_t port); unsigned size_to_prdtl(unsigned bytes, unsigned bytes_per_prd); void ahci_guest_io(AHCIQState *ahci, uint8_t port, uint8_t ide_cmd, uint64_t gbuffer, size_t size, uint64_t sector); +AHCICommand *ahci_guest_io_halt(AHCIQState *ahci, uint8_t port, uint8_t ide_cmd, + uint64_t gbuffer, size_t size, uint64_t sector); +void ahci_guest_io_resume(AHCIQState *ahci, AHCICommand *cmd); void ahci_io(AHCIQState *ahci, uint8_t port, uint8_t ide_cmd, void *buffer, size_t bufsize, uint64_t sector); From 455e861cc625891baacf74e66c31a914883b80ca Mon Sep 17 00:00:00 2001 From: John Snow Date: Fri, 22 May 2015 14:13:42 -0400 Subject: [PATCH 04/19] libqos/ahci: Fix sector set method || probably does not mean the same thing as |. Additionally, allow users to submit a prd_size of 0 to indicate that they'd like to continue using the default. Signed-off-by: John Snow Reviewed-by: Kevin Wolf Message-id: 1430417242-11859-3-git-send-email-jsnow@redhat.com --- tests/libqos/ahci.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c index 229409b353ca..8c8fd8967d08 100644 --- a/tests/libqos/ahci.c +++ b/tests/libqos/ahci.c @@ -769,7 +769,7 @@ void ahci_command_set_offset(AHCICommand *cmd, uint64_t lba_sect) fis->lba_lo[1] = (lba_sect >> 8) & 0xFF; fis->lba_lo[2] = (lba_sect >> 16) & 0xFF; if (cmd->props->lba28) { - fis->device = (fis->device & 0xF0) || (lba_sect >> 24) & 0x0F; + fis->device = (fis->device & 0xF0) | ((lba_sect >> 24) & 0x0F); } fis->lba_hi[0] = (lba_sect >> 24) & 0xFF; fis->lba_hi[1] = (lba_sect >> 32) & 0xFF; @@ -787,7 +787,9 @@ void ahci_command_set_sizes(AHCICommand *cmd, uint64_t xbytes, /* Each PRD can describe up to 4MiB, and must not be odd. */ g_assert_cmphex(prd_size, <=, 4096 * 1024); g_assert_cmphex(prd_size & 0x01, ==, 0x00); - cmd->prd_size = prd_size; + if (prd_size) { + cmd->prd_size = prd_size; + } cmd->xbytes = xbytes; cmd->fis.count = (cmd->xbytes / AHCI_SECTOR_SIZE); cmd->header.prdtl = size_to_prdtl(cmd->xbytes, cmd->prd_size); From 085248ae87704f1c1e4e1f929f58beca3ba294a2 Mon Sep 17 00:00:00 2001 From: John Snow Date: Fri, 22 May 2015 14:13:43 -0400 Subject: [PATCH 05/19] libqos: Add migration helpers libqos.c: -set_context for addressing which commands go where -migrate performs the actual migration malloc.c: - Structure of the allocator is adjusted slightly with a second-tier malloc to make swapping around the allocators easy when we "migrate" the lists from the source to the destination. Signed-off-by: John Snow Reviewed-by: Kevin Wolf Message-id: 1430417242-11859-4-git-send-email-jsnow@redhat.com --- tests/libqos/libqos.c | 85 +++++++++++++++++++++++++++++++++++++++++++ tests/libqos/libqos.h | 2 + tests/libqos/malloc.c | 74 ++++++++++++++++++++++++++++--------- tests/libqos/malloc.h | 1 + 4 files changed, 145 insertions(+), 17 deletions(-) diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c index 7e7207856ef6..fce625b18ada 100644 --- a/tests/libqos/libqos.c +++ b/tests/libqos/libqos.c @@ -1,5 +1,6 @@ #include #include +#include #include #include #include @@ -62,6 +63,90 @@ void qtest_shutdown(QOSState *qs) g_free(qs); } +void set_context(QOSState *s) +{ + global_qtest = s->qts; +} + +static QDict *qmp_execute(const char *command) +{ + char *fmt; + QDict *rsp; + + fmt = g_strdup_printf("{ 'execute': '%s' }", command); + rsp = qmp(fmt); + g_free(fmt); + + return rsp; +} + +void migrate(QOSState *from, QOSState *to, const char *uri) +{ + const char *st; + char *s; + QDict *rsp, *sub; + bool running; + + set_context(from); + + /* Is the machine currently running? */ + rsp = qmp_execute("query-status"); + g_assert(qdict_haskey(rsp, "return")); + sub = qdict_get_qdict(rsp, "return"); + g_assert(qdict_haskey(sub, "running")); + running = qdict_get_bool(sub, "running"); + QDECREF(rsp); + + /* Issue the migrate command. */ + s = g_strdup_printf("{ 'execute': 'migrate'," + "'arguments': { 'uri': '%s' } }", + uri); + rsp = qmp(s); + g_free(s); + g_assert(qdict_haskey(rsp, "return")); + QDECREF(rsp); + + /* Wait for STOP event, but only if we were running: */ + if (running) { + qmp_eventwait("STOP"); + } + + /* If we were running, we can wait for an event. */ + if (running) { + migrate_allocator(from->alloc, to->alloc); + set_context(to); + qmp_eventwait("RESUME"); + return; + } + + /* Otherwise, we need to wait: poll until migration is completed. */ + while (1) { + rsp = qmp_execute("query-migrate"); + g_assert(qdict_haskey(rsp, "return")); + sub = qdict_get_qdict(rsp, "return"); + g_assert(qdict_haskey(sub, "status")); + st = qdict_get_str(sub, "status"); + + /* "setup", "active", "completed", "failed", "cancelled" */ + if (strcmp(st, "completed") == 0) { + QDECREF(rsp); + break; + } + + if ((strcmp(st, "setup") == 0) || (strcmp(st, "active") == 0)) { + QDECREF(rsp); + g_usleep(5000); + continue; + } + + fprintf(stderr, "Migration did not complete, status: %s\n", st); + g_assert_not_reached(); + } + + migrate_allocator(from->alloc, to->alloc); + set_context(to); +} + void mkimg(const char *file, const char *fmt, unsigned size_mb) { gchar *cli; diff --git a/tests/libqos/libqos.h b/tests/libqos/libqos.h index f57362b68819..e1f14ea6fbfe 100644 --- a/tests/libqos/libqos.h +++ b/tests/libqos/libqos.h @@ -21,6 +21,8 @@ QOSState *qtest_boot(QOSOps *ops, const char *cmdline_fmt, ...); void qtest_shutdown(QOSState *qs); void mkimg(const char *file, const char *fmt, unsigned size_mb); void mkqcow2(const char *file, unsigned size_mb); +void set_context(QOSState *s); +void migrate(QOSState *from, QOSState *to, const char *uri); void prepare_blkdebug_script(const char *debug_fn, const char *event); static inline uint64_t qmalloc(QOSState *q, size_t bytes) diff --git a/tests/libqos/malloc.c b/tests/libqos/malloc.c index 67f31902fdf9..827613005a75 100644 --- a/tests/libqos/malloc.c +++ b/tests/libqos/malloc.c @@ -30,8 +30,8 @@ struct QGuestAllocator { uint64_t end; uint32_t page_size; - MemList used; - MemList free; + MemList *used; + MemList *free; }; #define DEFAULT_PAGE_SIZE 4096 @@ -150,7 +150,7 @@ static uint64_t mlist_fulfill(QGuestAllocator *s, MemBlock *freenode, addr = freenode->addr; if (freenode->size == size) { /* re-use this freenode as our used node */ - QTAILQ_REMOVE(&s->free, freenode, MLIST_ENTNAME); + QTAILQ_REMOVE(s->free, freenode, MLIST_ENTNAME); usednode = freenode; } else { /* adjust the free node and create a new used node */ @@ -159,7 +159,7 @@ static uint64_t mlist_fulfill(QGuestAllocator *s, MemBlock *freenode, usednode = mlist_new(addr, size); } - mlist_sort_insert(&s->used, usednode); + mlist_sort_insert(s->used, usednode); return addr; } @@ -171,7 +171,7 @@ static void mlist_check(QGuestAllocator *s) uint64_t addr = s->start > 0 ? s->start - 1 : 0; uint64_t next = s->start; - QTAILQ_FOREACH(node, &s->free, MLIST_ENTNAME) { + QTAILQ_FOREACH(node, s->free, MLIST_ENTNAME) { g_assert_cmpint(node->addr, >, addr); g_assert_cmpint(node->addr, >=, next); addr = node->addr; @@ -180,7 +180,7 @@ static void mlist_check(QGuestAllocator *s) addr = s->start > 0 ? s->start - 1 : 0; next = s->start; - QTAILQ_FOREACH(node, &s->used, MLIST_ENTNAME) { + QTAILQ_FOREACH(node, s->used, MLIST_ENTNAME) { g_assert_cmpint(node->addr, >, addr); g_assert_cmpint(node->addr, >=, next); addr = node->addr; @@ -192,7 +192,7 @@ static uint64_t mlist_alloc(QGuestAllocator *s, uint64_t size) { MemBlock *node; - node = mlist_find_space(&s->free, size); + node = mlist_find_space(s->free, size); if (!node) { fprintf(stderr, "Out of guest memory.\n"); g_assert_not_reached(); @@ -208,7 +208,7 @@ static void mlist_free(QGuestAllocator *s, uint64_t addr) return; } - node = mlist_find_key(&s->used, addr); + node = mlist_find_key(s->used, addr); if (!node) { fprintf(stderr, "Error: no record found for an allocation at " "0x%016" PRIx64 ".\n", @@ -217,9 +217,9 @@ static void mlist_free(QGuestAllocator *s, uint64_t addr) } /* Rip it out of the used list and re-insert back into the free list. */ - QTAILQ_REMOVE(&s->used, node, MLIST_ENTNAME); - mlist_sort_insert(&s->free, node); - mlist_coalesce(&s->free, node); + QTAILQ_REMOVE(s->used, node, MLIST_ENTNAME); + mlist_sort_insert(s->free, node); + mlist_coalesce(s->free, node); } /* @@ -233,7 +233,7 @@ void alloc_uninit(QGuestAllocator *allocator) QAllocOpts mask; /* Check for guest leaks, and destroy the list. */ - QTAILQ_FOREACH_SAFE(node, &allocator->used, MLIST_ENTNAME, tmp) { + QTAILQ_FOREACH_SAFE(node, allocator->used, MLIST_ENTNAME, tmp) { if (allocator->opts & (ALLOC_LEAK_WARN | ALLOC_LEAK_ASSERT)) { fprintf(stderr, "guest malloc leak @ 0x%016" PRIx64 "; " "size 0x%016" PRIx64 ".\n", @@ -248,7 +248,7 @@ void alloc_uninit(QGuestAllocator *allocator) /* If we have previously asserted that there are no leaks, then there * should be only one node here with a specific address and size. */ mask = ALLOC_LEAK_ASSERT | ALLOC_PARANOID; - QTAILQ_FOREACH_SAFE(node, &allocator->free, MLIST_ENTNAME, tmp) { + QTAILQ_FOREACH_SAFE(node, allocator->free, MLIST_ENTNAME, tmp) { if ((allocator->opts & mask) == mask) { if ((node->addr != allocator->start) || (node->size != allocator->end - allocator->start)) { @@ -260,6 +260,8 @@ void alloc_uninit(QGuestAllocator *allocator) g_free(node); } + g_free(allocator->used); + g_free(allocator->free); g_free(allocator); } @@ -297,11 +299,13 @@ QGuestAllocator *alloc_init(uint64_t start, uint64_t end) s->start = start; s->end = end; - QTAILQ_INIT(&s->used); - QTAILQ_INIT(&s->free); + s->used = g_malloc(sizeof(MemList)); + s->free = g_malloc(sizeof(MemList)); + QTAILQ_INIT(s->used); + QTAILQ_INIT(s->free); node = mlist_new(s->start, s->end - s->start); - QTAILQ_INSERT_HEAD(&s->free, node, MLIST_ENTNAME); + QTAILQ_INSERT_HEAD(s->free, node, MLIST_ENTNAME); s->page_size = DEFAULT_PAGE_SIZE; @@ -319,7 +323,7 @@ QGuestAllocator *alloc_init_flags(QAllocOpts opts, void alloc_set_page_size(QGuestAllocator *allocator, size_t page_size) { /* Can't alter the page_size for an allocator in-use */ - g_assert(QTAILQ_EMPTY(&allocator->used)); + g_assert(QTAILQ_EMPTY(allocator->used)); g_assert(is_power_of_2(page_size)); allocator->page_size = page_size; @@ -329,3 +333,39 @@ void alloc_set_flags(QGuestAllocator *allocator, QAllocOpts opts) { allocator->opts |= opts; } + +void migrate_allocator(QGuestAllocator *src, + QGuestAllocator *dst) +{ + MemBlock *node, *tmp; + MemList *tmpused, *tmpfree; + + /* The general memory layout should be equivalent, + * though opts can differ. */ + g_assert_cmphex(src->start, ==, dst->start); + g_assert_cmphex(src->end, ==, dst->end); + + /* Destroy (silently, regardless of options) the dest-list: */ + QTAILQ_FOREACH_SAFE(node, dst->used, MLIST_ENTNAME, tmp) { + g_free(node); + } + QTAILQ_FOREACH_SAFE(node, dst->free, MLIST_ENTNAME, tmp) { + g_free(node); + } + + tmpused = dst->used; + tmpfree = dst->free; + + /* Inherit the lists of the source allocator: */ + dst->used = src->used; + dst->free = src->free; + + /* Source is now re-initialized, the source memory is 'invalid' now: */ + src->used = tmpused; + src->free = tmpfree; + QTAILQ_INIT(src->used); + QTAILQ_INIT(src->free); + node = mlist_new(src->start, src->end - src->start); + QTAILQ_INSERT_HEAD(src->free, node, MLIST_ENTNAME); + return; +} diff --git a/tests/libqos/malloc.h b/tests/libqos/malloc.h index 71ac407dcdc0..0c6c9b7f3075 100644 --- a/tests/libqos/malloc.h +++ b/tests/libqos/malloc.h @@ -31,6 +31,7 @@ void alloc_uninit(QGuestAllocator *allocator); /* Always returns page aligned values */ uint64_t guest_alloc(QGuestAllocator *allocator, size_t size); void guest_free(QGuestAllocator *allocator, uint64_t addr); +void migrate_allocator(QGuestAllocator *src, QGuestAllocator *dst); QGuestAllocator *alloc_init(uint64_t start, uint64_t end); QGuestAllocator *alloc_init_flags(QAllocOpts flags, From 04329029a8c539eb5f75dcb6d8b016f0c53a031a Mon Sep 17 00:00:00 2001 From: John Snow Date: Fri, 22 May 2015 14:13:43 -0400 Subject: [PATCH 06/19] ich9/ahci: Enable Migration Lift the flag preventing the migration of the ICH9/AHCI devices. Signed-off-by: John Snow Reviewed-by: Kevin Wolf Message-id: 1430417242-11859-5-git-send-email-jsnow@redhat.com --- hw/ide/ahci.c | 1 - hw/ide/ich.c | 1 - 2 files changed, 2 deletions(-) diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 833fd45faf6d..8e36dec5a9e2 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -1461,7 +1461,6 @@ typedef struct SysbusAHCIState { static const VMStateDescription vmstate_sysbus_ahci = { .name = "sysbus-ahci", - .unmigratable = 1, /* Still buggy under I/O load */ .fields = (VMStateField[]) { VMSTATE_AHCI(ahci, SysbusAHCIState), VMSTATE_END_OF_LIST() diff --git a/hw/ide/ich.c b/hw/ide/ich.c index b1d8874671ad..350c7f1c7531 100644 --- a/hw/ide/ich.c +++ b/hw/ide/ich.c @@ -82,7 +82,6 @@ static const VMStateDescription vmstate_ich9_ahci = { .name = "ich9_ahci", - .unmigratable = 1, /* Still buggy under I/O load */ .version_id = 1, .fields = (VMStateField[]) { VMSTATE_PCI_DEVICE(parent_obj, AHCIPCIState), From 278128ab06c36341edb2c8b0bfcfd92760f4db52 Mon Sep 17 00:00:00 2001 From: John Snow Date: Fri, 22 May 2015 14:13:43 -0400 Subject: [PATCH 07/19] qtest/ahci: Add migration test Notes: * The migration is performed on QOSState objects. * The migration is performed in such a way that it does not assume consistency between the allocators attached to each. That is to say, you can use each QOSState object completely independently and then at an arbitrary point decide to migrate, and the destination object will now be consistent with the memory within the source guest. The source object that was migrated from will have a completely blank allocator. ahci-test.c: - verify_state is added - ahci_migrate is added as a frontend to migrate - test_migrate_sanity test case is added. Signed-off-by: John Snow Reviewed-by: Kevin Wolf Message-id: 1430417242-11859-6-git-send-email-jsnow@redhat.com --- tests/ahci-test.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/tests/ahci-test.c b/tests/ahci-test.c index 7c23bb2180aa..9fccafcbd851 100644 --- a/tests/ahci-test.c +++ b/tests/ahci-test.c @@ -97,6 +97,72 @@ static void generate_pattern(void *buffer, size_t len, size_t cycle_len) } } +/** + * Verify that the transfer did not corrupt our state at all. + */ +static void verify_state(AHCIQState *ahci) +{ + int i, j; + uint32_t ahci_fingerprint; + uint64_t hba_base; + uint64_t hba_stored; + AHCICommandHeader cmd; + + ahci_fingerprint = qpci_config_readl(ahci->dev, PCI_VENDOR_ID); + g_assert_cmphex(ahci_fingerprint, ==, ahci->fingerprint); + + /* If we haven't initialized, this is as much as can be validated. */ + if (!ahci->hba_base) { + return; + } + + hba_base = (uint64_t)qpci_config_readl(ahci->dev, PCI_BASE_ADDRESS_5); + hba_stored = (uint64_t)(uintptr_t)ahci->hba_base; + g_assert_cmphex(hba_base, ==, hba_stored); + + g_assert_cmphex(ahci_rreg(ahci, AHCI_CAP), ==, ahci->cap); + g_assert_cmphex(ahci_rreg(ahci, AHCI_CAP2), ==, ahci->cap2); + + for (i = 0; i < 32; i++) { + g_assert_cmphex(ahci_px_rreg(ahci, i, AHCI_PX_FB), ==, + ahci->port[i].fb); + g_assert_cmphex(ahci_px_rreg(ahci, i, AHCI_PX_CLB), ==, + ahci->port[i].clb); + for (j = 0; j < 32; j++) { + ahci_get_command_header(ahci, i, j, &cmd); + g_assert_cmphex(cmd.prdtl, ==, ahci->port[i].prdtl[j]); + g_assert_cmphex(cmd.ctba, ==, ahci->port[i].ctba[j]); + } + } +} + +static void ahci_migrate(AHCIQState *from, AHCIQState *to, const char *uri) +{ + QOSState *tmp = to->parent; + QPCIDevice *dev = to->dev; + if (uri == NULL) { + uri = "tcp:127.0.0.1:1234"; + } + + /* context will be 'to' after completion. */ + migrate(from->parent, to->parent, uri); + + /* We'd like for the AHCIState objects to still point + * to information specific to its specific parent + * instance, but otherwise just inherit the new data. */ + memcpy(to, from, sizeof(AHCIQState)); + to->parent = tmp; + to->dev = dev; + + tmp = from->parent; + dev = from->dev; + memset(from, 0x00, sizeof(AHCIQState)); + from->parent = tmp; + from->dev = dev; + + verify_state(to); +} + /*** Test Setup & Teardown ***/ /** @@ -146,6 +212,8 @@ static AHCIQState *ahci_boot(const char *cli, ...) static void ahci_shutdown(AHCIQState *ahci) { QOSState *qs = ahci->parent; + + set_context(qs); ahci_clean_mem(ahci); free_ahci_device(ahci->dev); g_free(ahci); @@ -1022,6 +1090,26 @@ static void test_flush_retry(void) ahci_shutdown(ahci); } +/** + * Basic sanity test to boot a machine, find an AHCI device, and shutdown. + */ +static void test_migrate_sanity(void) +{ + AHCIQState *src, *dst; + const char *uri = "tcp:127.0.0.1:1234"; + + src = ahci_boot("-m 1024 -M q35 " + "-hda %s ", tmp_path); + dst = ahci_boot("-m 1024 -M q35 " + "-hda %s " + "-incoming %s", tmp_path, uri); + + ahci_migrate(src, dst, uri); + + ahci_shutdown(src); + ahci_shutdown(dst); +} + /******************************************************************************/ /* AHCI I/O Test Matrix Definitions */ @@ -1271,6 +1359,8 @@ int main(int argc, char **argv) qtest_add_func("/ahci/flush/simple", test_flush); qtest_add_func("/ahci/flush/retry", test_flush_retry); + qtest_add_func("/ahci/migrate/sanity", test_migrate_sanity); + ret = g_test_run(); /* Cleanup */ From 88e21f9485f0a41603f0af3483ff3f11c95979ab Mon Sep 17 00:00:00 2001 From: John Snow Date: Fri, 22 May 2015 14:13:43 -0400 Subject: [PATCH 08/19] qtest/ahci: add migrate dma test Write to one guest, migrate, and then read from the other. adjust ahci_io to clear any buffers it creates, so that we can use ahci_io safely on both guests knowing we are using empty buffers and not accidentally re-using data. Signed-off-by: John Snow Reviewed-by: Kevin Wolf Message-id: 1430417242-11859-7-git-send-email-jsnow@redhat.com --- tests/ahci-test.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ tests/libqos/ahci.c | 1 + 2 files changed, 46 insertions(+) diff --git a/tests/ahci-test.c b/tests/ahci-test.c index 9fccafcbd851..6513330296aa 100644 --- a/tests/ahci-test.c +++ b/tests/ahci-test.c @@ -1110,6 +1110,50 @@ static void test_migrate_sanity(void) ahci_shutdown(dst); } +/** + * DMA Migration test: Write a pattern, migrate, then read. + */ +static void test_migrate_dma(void) +{ + AHCIQState *src, *dst; + uint8_t px; + size_t bufsize = 4096; + unsigned char *tx = g_malloc(bufsize); + unsigned char *rx = g_malloc0(bufsize); + unsigned i; + const char *uri = "tcp:127.0.0.1:1234"; + + src = ahci_boot_and_enable("-m 1024 -M q35 " + "-hda %s ", tmp_path); + dst = ahci_boot("-m 1024 -M q35 " + "-hda %s " + "-incoming %s", tmp_path, uri); + + set_context(src->parent); + + /* initialize */ + px = ahci_port_select(src); + ahci_port_clear(src, px); + + /* create pattern */ + for (i = 0; i < bufsize; i++) { + tx[i] = (bufsize - i); + } + + /* Write, migrate, then read. */ + ahci_io(src, px, CMD_WRITE_DMA, tx, bufsize, 0); + ahci_migrate(src, dst, uri); + ahci_io(dst, px, CMD_READ_DMA, rx, bufsize, 0); + + /* Verify pattern */ + g_assert_cmphex(memcmp(tx, rx, bufsize), ==, 0); + + ahci_shutdown(src); + ahci_shutdown(dst); + g_free(rx); + g_free(tx); +} + /******************************************************************************/ /* AHCI I/O Test Matrix Definitions */ @@ -1360,6 +1404,7 @@ int main(int argc, char **argv) qtest_add_func("/ahci/flush/retry", test_flush_retry); qtest_add_func("/ahci/migrate/sanity", test_migrate_sanity); + qtest_add_func("/ahci/migrate/dma", test_migrate_dma); ret = g_test_run(); diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c index 8c8fd8967d08..95bfb3dac8c6 100644 --- a/tests/libqos/ahci.c +++ b/tests/libqos/ahci.c @@ -650,6 +650,7 @@ void ahci_io(AHCIQState *ahci, uint8_t port, uint8_t ide_cmd, g_assert(props); ptr = ahci_alloc(ahci, bufsize); g_assert(ptr); + qmemset(ptr, 0x00, bufsize); if (props->write) { memwrite(ptr, buffer, bufsize); From a606ce50c29561b567995ab663cbb40067623e82 Mon Sep 17 00:00:00 2001 From: John Snow Date: Fri, 22 May 2015 14:13:43 -0400 Subject: [PATCH 09/19] qtest/ahci: add flush migrate test Use blkdebug to inject an error on first flush, then attempt to flush on the first guest. When the error halts the VM, migrate to the second VM, and attempt to resume the command. Signed-off-by: John Snow Reviewed-by: Kevin Wolf Message-id: 1430417242-11859-8-git-send-email-jsnow@redhat.com --- tests/ahci-test.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/tests/ahci-test.c b/tests/ahci-test.c index 6513330296aa..f6de355fb7b2 100644 --- a/tests/ahci-test.c +++ b/tests/ahci-test.c @@ -1071,7 +1071,7 @@ static void test_flush_retry(void) debug_path, tmp_path); - /* Issue Flush Command */ + /* Issue Flush Command and wait for error */ port = ahci_port_select(ahci); ahci_port_clear(ahci, port); cmd = ahci_command_create(CMD_FLUSH_CACHE); @@ -1154,6 +1154,55 @@ static void test_migrate_dma(void) g_free(tx); } +/** + * Migration test: Try to flush, migrate, then resume. + */ +static void test_flush_migrate(void) +{ + AHCIQState *src, *dst; + AHCICommand *cmd; + uint8_t px; + const char *s; + const char *uri = "tcp:127.0.0.1:1234"; + + prepare_blkdebug_script(debug_path, "flush_to_disk"); + + src = ahci_boot_and_enable("-drive file=blkdebug:%s:%s,if=none,id=drive0," + "cache=writeback,rerror=stop,werror=stop " + "-M q35 " + "-device ide-hd,drive=drive0 ", + debug_path, tmp_path); + dst = ahci_boot("-drive file=%s,if=none,id=drive0," + "cache=writeback,rerror=stop,werror=stop " + "-M q35 " + "-device ide-hd,drive=drive0 " + "-incoming %s", tmp_path, uri); + + set_context(src->parent); + + /* Issue Flush Command */ + px = ahci_port_select(src); + ahci_port_clear(src, px); + cmd = ahci_command_create(CMD_FLUSH_CACHE); + ahci_command_commit(src, cmd, px); + ahci_command_issue_async(src, cmd); + qmp_eventwait("STOP"); + + /* Migrate over */ + ahci_migrate(src, dst, uri); + + /* Complete the command */ + s = "{'execute':'cont' }"; + qmp_async(s); + qmp_eventwait("RESUME"); + ahci_command_wait(dst, cmd); + ahci_command_verify(dst, cmd); + + ahci_command_free(cmd); + ahci_shutdown(src); + ahci_shutdown(dst); +} + /******************************************************************************/ /* AHCI I/O Test Matrix Definitions */ @@ -1402,6 +1451,7 @@ int main(int argc, char **argv) qtest_add_func("/ahci/flush/simple", test_flush); qtest_add_func("/ahci/flush/retry", test_flush_retry); + qtest_add_func("/ahci/flush/migrate", test_flush_migrate); qtest_add_func("/ahci/migrate/sanity", test_migrate_sanity); qtest_add_func("/ahci/migrate/dma", test_migrate_dma); From 189d1b6126625212fbb50ed3a30a3e9e7ba7ca37 Mon Sep 17 00:00:00 2001 From: John Snow Date: Fri, 22 May 2015 14:13:43 -0400 Subject: [PATCH 10/19] qtest/ahci: add halted dma test If we're going to test the migration of halted DMA jobs, we should probably check to make sure we can resume them locally as a first step. Signed-off-by: John Snow Reviewed-by: Kevin Wolf Message-id: 1430417242-11859-9-git-send-email-jsnow@redhat.com --- tests/ahci-test.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/tests/ahci-test.c b/tests/ahci-test.c index f6de355fb7b2..4eb110508bc8 100644 --- a/tests/ahci-test.c +++ b/tests/ahci-test.c @@ -1154,6 +1154,65 @@ static void test_migrate_dma(void) g_free(tx); } +/** + * DMA Error Test + * + * Simulate an error on first write, Try to write a pattern, + * Confirm the VM has stopped, resume the VM, verify command + * has completed, then read back the data and verify. + */ +static void test_halted_dma(void) +{ + AHCIQState *ahci; + uint8_t port; + size_t bufsize = 4096; + unsigned char *tx = g_malloc(bufsize); + unsigned char *rx = g_malloc0(bufsize); + unsigned i; + uint64_t ptr; + AHCICommand *cmd; + + prepare_blkdebug_script(debug_path, "write_aio"); + + ahci = ahci_boot_and_enable("-drive file=blkdebug:%s:%s,if=none,id=drive0," + "format=qcow2,cache=writeback," + "rerror=stop,werror=stop " + "-M q35 " + "-device ide-hd,drive=drive0 ", + debug_path, + tmp_path); + + /* Initialize and prepare */ + port = ahci_port_select(ahci); + ahci_port_clear(ahci, port); + + for (i = 0; i < bufsize; i++) { + tx[i] = (bufsize - i); + } + + /* create DMA source buffer and write pattern */ + ptr = ahci_alloc(ahci, bufsize); + g_assert(ptr); + memwrite(ptr, tx, bufsize); + + /* Attempt to write (and fail) */ + cmd = ahci_guest_io_halt(ahci, port, CMD_WRITE_DMA, + ptr, bufsize, 0); + + /* Attempt to resume the command */ + ahci_guest_io_resume(ahci, cmd); + ahci_free(ahci, ptr); + + /* Read back and verify */ + ahci_io(ahci, port, CMD_READ_DMA, rx, bufsize, 0); + g_assert_cmphex(memcmp(tx, rx, bufsize), ==, 0); + + /* Cleanup and go home */ + ahci_shutdown(ahci); + g_free(rx); + g_free(tx); +} + /** * Migration test: Try to flush, migrate, then resume. */ @@ -1455,6 +1514,7 @@ int main(int argc, char **argv) qtest_add_func("/ahci/migrate/sanity", test_migrate_sanity); qtest_add_func("/ahci/migrate/dma", test_migrate_dma); + qtest_add_func("/ahci/io/dma/lba28/retry", test_halted_dma); ret = g_test_run(); From 5d1cf0917b4f1282ac81bb72697713d14c98a876 Mon Sep 17 00:00:00 2001 From: John Snow Date: Fri, 22 May 2015 14:13:43 -0400 Subject: [PATCH 11/19] qtest/ahci: add migrate halted dma test Test migrating a halted DMA transaction. Resume, then test data integrity. Signed-off-by: John Snow Reviewed-by: Kevin Wolf Message-id: 1430417242-11859-10-git-send-email-jsnow@redhat.com --- tests/ahci-test.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 74 insertions(+), 1 deletion(-) diff --git a/tests/ahci-test.c b/tests/ahci-test.c index 4eb110508bc8..1a967c372415 100644 --- a/tests/ahci-test.c +++ b/tests/ahci-test.c @@ -1213,6 +1213,78 @@ static void test_halted_dma(void) g_free(tx); } +/** + * DMA Error Migration Test + * + * Simulate an error on first write, Try to write a pattern, + * Confirm the VM has stopped, migrate, resume the VM, + * verify command has completed, then read back the data and verify. + */ +static void test_migrate_halted_dma(void) +{ + AHCIQState *src, *dst; + uint8_t port; + size_t bufsize = 4096; + unsigned char *tx = g_malloc(bufsize); + unsigned char *rx = g_malloc0(bufsize); + unsigned i; + uint64_t ptr; + AHCICommand *cmd; + const char *uri = "tcp:127.0.0.1:1234"; + + prepare_blkdebug_script(debug_path, "write_aio"); + + src = ahci_boot_and_enable("-drive file=blkdebug:%s:%s,if=none,id=drive0," + "format=qcow2,cache=writeback," + "rerror=stop,werror=stop " + "-M q35 " + "-device ide-hd,drive=drive0 ", + debug_path, + tmp_path); + + dst = ahci_boot("-drive file=%s,if=none,id=drive0," + "format=qcow2,cache=writeback," + "rerror=stop,werror=stop " + "-M q35 " + "-device ide-hd,drive=drive0 " + "-incoming %s", + tmp_path, uri); + + set_context(src->parent); + + /* Initialize and prepare */ + port = ahci_port_select(src); + ahci_port_clear(src, port); + + for (i = 0; i < bufsize; i++) { + tx[i] = (bufsize - i); + } + + /* create DMA source buffer and write pattern */ + ptr = ahci_alloc(src, bufsize); + g_assert(ptr); + memwrite(ptr, tx, bufsize); + + /* Write, trigger the VM to stop, migrate, then resume. */ + cmd = ahci_guest_io_halt(src, port, CMD_WRITE_DMA, + ptr, bufsize, 0); + ahci_migrate(src, dst, uri); + ahci_guest_io_resume(dst, cmd); + ahci_free(dst, ptr); + + /* Read back */ + ahci_io(dst, port, CMD_READ_DMA, rx, bufsize, 0); + + /* Verify TX and RX are identical */ + g_assert_cmphex(memcmp(tx, rx, bufsize), ==, 0); + + /* Cleanup and go home. */ + ahci_shutdown(src); + ahci_shutdown(dst); + g_free(rx); + g_free(tx); +} + /** * Migration test: Try to flush, migrate, then resume. */ @@ -1513,8 +1585,9 @@ int main(int argc, char **argv) qtest_add_func("/ahci/flush/migrate", test_flush_migrate); qtest_add_func("/ahci/migrate/sanity", test_migrate_sanity); - qtest_add_func("/ahci/migrate/dma", test_migrate_dma); + qtest_add_func("/ahci/migrate/dma/simple", test_migrate_dma); qtest_add_func("/ahci/io/dma/lba28/retry", test_halted_dma); + qtest_add_func("/ahci/migrate/dma/halted", test_migrate_halted_dma); ret = g_test_run(); From 332cc7e9b39ddb2feacb4c71dcd18c3e5b0c3147 Mon Sep 17 00:00:00 2001 From: John Snow Date: Fri, 22 May 2015 14:13:43 -0400 Subject: [PATCH 12/19] qtest: allow arbitrarily long sends qtest currently has a static buffer of size 1024 that if we overflow, ignores the additional data silently which leads to hangs or stream failures. Use glib's string facilities to allow arbitrarily long data, but split this off into a new function, qtest_sendf. Static data can still be sent using qtest_send, which avoids the malloc/copy overhead. Signed-off-by: John Snow Reviewed-by: Eric Blake Message-id: 1430864578-22072-2-git-send-email-jsnow@redhat.com --- qtest.c | 46 ++++++++++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/qtest.c b/qtest.c index 8d1e66caa316..3738eae37789 100644 --- a/qtest.c +++ b/qtest.c @@ -182,21 +182,29 @@ static void qtest_send_prefix(CharDriverState *chr) (long) tv.tv_sec, (long) tv.tv_usec); } -static void GCC_FMT_ATTR(2, 3) qtest_send(CharDriverState *chr, - const char *fmt, ...) +static void do_qtest_send(CharDriverState *chr, const char *str, size_t len) +{ + qemu_chr_fe_write_all(chr, (uint8_t *)str, len); + if (qtest_log_fp && qtest_opened) { + fprintf(qtest_log_fp, "%s", str); + } +} + +static void qtest_send(CharDriverState *chr, const char *str) +{ + do_qtest_send(chr, str, strlen(str)); +} + +static void GCC_FMT_ATTR(2, 3) qtest_sendf(CharDriverState *chr, + const char *fmt, ...) { va_list ap; - char buffer[1024]; - size_t len; + gchar *buffer; va_start(ap, fmt); - len = vsnprintf(buffer, sizeof(buffer), fmt, ap); + buffer = g_strdup_vprintf(fmt, ap); + qtest_send(chr, buffer); va_end(ap); - - qemu_chr_fe_write_all(chr, (uint8_t *)buffer, len); - if (qtest_log_fp && qtest_opened) { - fprintf(qtest_log_fp, "%s", buffer); - } } static void qtest_irq_handler(void *opaque, int n, int level) @@ -208,8 +216,8 @@ static void qtest_irq_handler(void *opaque, int n, int level) CharDriverState *chr = qtest_chr; irq_levels[n] = level; qtest_send_prefix(chr); - qtest_send(chr, "IRQ %s %d\n", - level ? "raise" : "lower", n); + qtest_sendf(chr, "IRQ %s %d\n", + level ? "raise" : "lower", n); } } @@ -318,7 +326,7 @@ static void qtest_process_command(CharDriverState *chr, gchar **words) value = cpu_inl(addr); } qtest_send_prefix(chr); - qtest_send(chr, "OK 0x%04x\n", value); + qtest_sendf(chr, "OK 0x%04x\n", value); } else if (strcmp(words[0], "writeb") == 0 || strcmp(words[0], "writew") == 0 || strcmp(words[0], "writel") == 0 || @@ -375,7 +383,7 @@ static void qtest_process_command(CharDriverState *chr, gchar **words) tswap64s(&value); } qtest_send_prefix(chr); - qtest_send(chr, "OK 0x%016" PRIx64 "\n", value); + qtest_sendf(chr, "OK 0x%016" PRIx64 "\n", value); } else if (strcmp(words[0], "read") == 0) { uint64_t addr, len, i; uint8_t *data; @@ -390,7 +398,7 @@ static void qtest_process_command(CharDriverState *chr, gchar **words) qtest_send_prefix(chr); qtest_send(chr, "OK 0x"); for (i = 0; i < len; i++) { - qtest_send(chr, "%02x", data[i]); + qtest_sendf(chr, "%02x", data[i]); } qtest_send(chr, "\n"); @@ -434,7 +442,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words) } qtest_clock_warp(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + ns); qtest_send_prefix(chr); - qtest_send(chr, "OK %"PRIi64"\n", (int64_t)qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)); + qtest_sendf(chr, "OK %"PRIi64"\n", + (int64_t)qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)); } else if (qtest_enabled() && strcmp(words[0], "clock_set") == 0) { int64_t ns; @@ -442,10 +451,11 @@ static void qtest_process_command(CharDriverState *chr, gchar **words) ns = strtoll(words[1], NULL, 0); qtest_clock_warp(ns); qtest_send_prefix(chr); - qtest_send(chr, "OK %"PRIi64"\n", (int64_t)qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)); + qtest_sendf(chr, "OK %"PRIi64"\n", + (int64_t)qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)); } else { qtest_send_prefix(chr); - qtest_send(chr, "FAIL Unknown command `%s'\n", words[0]); + qtest_sendf(chr, "FAIL Unknown command '%s'\n", words[0]); } } From 7a6a740d8dcc02f5693315d7935b5de9b963bb96 Mon Sep 17 00:00:00 2001 From: John Snow Date: Fri, 22 May 2015 14:13:44 -0400 Subject: [PATCH 13/19] qtest: Add base64 encoded read/write For larger pieces of data that won't need to be debugged and viewing the hex nibbles is unlikely to be useful, we can encode data using base64 instead of encoding each byte as %02x, which leads to some space savings and faster reads/writes. For now, the default is left as hex nibbles in memwrite() and memread(). For the purposes of making qtest io easier to read and debug, some callers may want to specify using the old encoding format for small patches of data where the savings from base64 wouldn't be that profound. memwrite/memread use a data encoding that takes 2x the size of the original buffer, but base64 uses "only" (4/3)x, so for larger buffers we can save a decent amount of time and space. Signed-off-by: John Snow Message-id: 1430864578-22072-3-git-send-email-jsnow@redhat.com --- qtest.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++ tests/libqtest.c | 31 ++++++++++++++++++++++ tests/libqtest.h | 49 ++++++++++++++++++++++++++++++++++ 3 files changed, 149 insertions(+) diff --git a/qtest.c b/qtest.c index 3738eae37789..73b7a0f9b81f 100644 --- a/qtest.c +++ b/qtest.c @@ -119,12 +119,21 @@ static bool qtest_opened; * > write ADDR SIZE DATA * < OK * + * > b64read ADDR SIZE + * < OK B64_DATA + * + * > b64write ADDR SIZE B64_DATA + * < OK + * * ADDR, SIZE, VALUE are all integers parsed with strtoul() with a base of 0. * * DATA is an arbitrarily long hex number prefixed with '0x'. If it's smaller * than the expected size, the value will be zero filled at the end of the data * sequence. * + * B64_DATA is an arbitrarily long base64 encoded string. + * If the sizes do not match, the data will be truncated. + * * IRQ management: * * > irq_intercept_in QOM-PATH @@ -182,6 +191,21 @@ static void qtest_send_prefix(CharDriverState *chr) (long) tv.tv_sec, (long) tv.tv_usec); } +static void GCC_FMT_ATTR(1, 2) qtest_log_send(const char *fmt, ...) +{ + va_list ap; + + if (!qtest_log_fp || !qtest_opened) { + return; + } + + qtest_send_prefix(NULL); + + va_start(ap, fmt); + vfprintf(qtest_log_fp, fmt, ap); + va_end(ap); +} + static void do_qtest_send(CharDriverState *chr, const char *str, size_t len) { qemu_chr_fe_write_all(chr, (uint8_t *)str, len); @@ -403,6 +427,23 @@ static void qtest_process_command(CharDriverState *chr, gchar **words) qtest_send(chr, "\n"); g_free(data); + } else if (strcmp(words[0], "b64read") == 0) { + uint64_t addr, len; + uint8_t *data; + gchar *b64_data; + + g_assert(words[1] && words[2]); + addr = strtoull(words[1], NULL, 0); + len = strtoull(words[2], NULL, 0); + + data = g_malloc(len); + cpu_physical_memory_read(addr, data, len); + b64_data = g_base64_encode(data, len); + qtest_send_prefix(chr); + qtest_sendf(chr, "OK %s\n", b64_data); + + g_free(data); + g_free(b64_data); } else if (strcmp(words[0], "write") == 0) { uint64_t addr, len, i; uint8_t *data; @@ -430,6 +471,34 @@ static void qtest_process_command(CharDriverState *chr, gchar **words) cpu_physical_memory_write(addr, data, len); g_free(data); + qtest_send_prefix(chr); + qtest_send(chr, "OK\n"); + } else if (strcmp(words[0], "b64write") == 0) { + uint64_t addr, len; + uint8_t *data; + size_t data_len; + gsize out_len; + + g_assert(words[1] && words[2] && words[3]); + addr = strtoull(words[1], NULL, 0); + len = strtoull(words[2], NULL, 0); + + data_len = strlen(words[3]); + if (data_len < 3) { + qtest_send(chr, "ERR invalid argument size\n"); + return; + } + + data = g_base64_decode_inplace(words[3], &out_len); + if (out_len != len) { + qtest_log_send("b64write: data length mismatch (told %"PRIu64", " + "found %zu)\n", + len, out_len); + out_len = MIN(out_len, len); + } + + cpu_physical_memory_write(addr, data, out_len); + qtest_send_prefix(chr); qtest_send(chr, "OK\n"); } else if (qtest_enabled() && strcmp(words[0], "clock_step") == 0) { diff --git a/tests/libqtest.c b/tests/libqtest.c index a525dc532c48..5f5700544729 100644 --- a/tests/libqtest.c +++ b/tests/libqtest.c @@ -695,6 +695,37 @@ void qtest_add_data_func(const char *str, const void *data, void (*fn)) g_free(path); } +void qtest_bufwrite(QTestState *s, uint64_t addr, const void *data, size_t size) +{ + gchar *bdata; + + bdata = g_base64_encode(data, size); + qtest_sendf(s, "b64write 0x%" PRIx64 " 0x%zx ", addr, size); + socket_send(s->fd, bdata, strlen(bdata)); + socket_send(s->fd, "\n", 1); + qtest_rsp(s, 0); + g_free(bdata); +} + +void qtest_bufread(QTestState *s, uint64_t addr, void *data, size_t size) +{ + gchar **args; + size_t len; + + qtest_sendf(s, "b64read 0x%" PRIx64 " 0x%zx\n", addr, size); + args = qtest_rsp(s, 2); + + g_base64_decode_inplace(args[1], &len); + if (size != len) { + fprintf(stderr, "bufread: asked for %zu bytes but decoded %zu\n", + size, len); + len = MIN(len, size); + } + + memcpy(data, args[1], len); + g_strfreev(args); +} + void qtest_memwrite(QTestState *s, uint64_t addr, const void *data, size_t size) { const uint8_t *ptr = data; diff --git a/tests/libqtest.h b/tests/libqtest.h index 4b54b5da9eac..ec4203152333 100644 --- a/tests/libqtest.h +++ b/tests/libqtest.h @@ -300,6 +300,17 @@ uint64_t qtest_readq(QTestState *s, uint64_t addr); */ void qtest_memread(QTestState *s, uint64_t addr, void *data, size_t size); +/** + * qtest_bufread: + * @s: #QTestState instance to operate on. + * @addr: Guest address to read from. + * @data: Pointer to where memory contents will be stored. + * @size: Number of bytes to read. + * + * Read guest memory into a buffer and receive using a base64 encoding. + */ +void qtest_bufread(QTestState *s, uint64_t addr, void *data, size_t size); + /** * qtest_memwrite: * @s: #QTestState instance to operate on. @@ -311,6 +322,18 @@ void qtest_memread(QTestState *s, uint64_t addr, void *data, size_t size); */ void qtest_memwrite(QTestState *s, uint64_t addr, const void *data, size_t size); +/** + * qtest_bufwrite: + * @s: #QTestState instance to operate on. + * @addr: Guest address to write to. + * @data: Pointer to the bytes that will be written to guest memory. + * @size: Number of bytes to write. + * + * Write a buffer to guest memory and transmit using a base64 encoding. + */ +void qtest_bufwrite(QTestState *s, uint64_t addr, + const void *data, size_t size); + /** * qtest_memset: * @s: #QTestState instance to operate on. @@ -698,6 +721,19 @@ static inline void memread(uint64_t addr, void *data, size_t size) qtest_memread(global_qtest, addr, data, size); } +/** + * bufread: + * @addr: Guest address to read from. + * @data: Pointer to where memory contents will be stored. + * @size: Number of bytes to read. + * + * Read guest memory into a buffer, receive using a base64 encoding. + */ +static inline void bufread(uint64_t addr, void *data, size_t size) +{ + qtest_bufread(global_qtest, addr, data, size); +} + /** * memwrite: * @addr: Guest address to write to. @@ -711,6 +747,19 @@ static inline void memwrite(uint64_t addr, const void *data, size_t size) qtest_memwrite(global_qtest, addr, data, size); } +/** + * bufwrite: + * @addr: Guest address to write to. + * @data: Pointer to the bytes that will be written to guest memory. + * @size: Number of bytes to write. + * + * Write a buffer to guest memory, transmit using a base64 encoding. + */ +static inline void bufwrite(uint64_t addr, const void *data, size_t size) +{ + qtest_bufwrite(global_qtest, addr, data, size); +} + /** * qmemset: * @addr: Guest address to write to. From 4d00796364ec4edab86b08abc38fd644d5e3c0ad Mon Sep 17 00:00:00 2001 From: John Snow Date: Fri, 22 May 2015 14:13:44 -0400 Subject: [PATCH 14/19] qtest: add memset to qtest protocol Previously, memset was just a frontend to write() and only stupidly sent the pattern many times across the wire. Let's not discuss who stupidly wrote it like that in the first place. (Hint: It was me.) Signed-off-by: John Snow Message-id: 1430864578-22072-4-git-send-email-jsnow@redhat.com --- qtest.c | 20 ++++++++++++++++++++ tests/libqtest.c | 8 +------- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/qtest.c b/qtest.c index 73b7a0f9b81f..04412ddde1b8 100644 --- a/qtest.c +++ b/qtest.c @@ -125,6 +125,9 @@ static bool qtest_opened; * > b64write ADDR SIZE B64_DATA * < OK * + * > memset ADDR SIZE VALUE + * < OK + * * ADDR, SIZE, VALUE are all integers parsed with strtoul() with a base of 0. * * DATA is an arbitrarily long hex number prefixed with '0x'. If it's smaller @@ -471,6 +474,23 @@ static void qtest_process_command(CharDriverState *chr, gchar **words) cpu_physical_memory_write(addr, data, len); g_free(data); + qtest_send_prefix(chr); + qtest_send(chr, "OK\n"); + } else if (strcmp(words[0], "memset") == 0) { + uint64_t addr, len; + uint8_t *data; + uint8_t pattern; + + g_assert(words[1] && words[2] && words[3]); + addr = strtoull(words[1], NULL, 0); + len = strtoull(words[2], NULL, 0); + pattern = strtoull(words[3], NULL, 0); + + data = g_malloc(len); + memset(data, pattern, len); + cpu_physical_memory_write(addr, data, len); + g_free(data); + qtest_send_prefix(chr); qtest_send(chr, "OK\n"); } else if (strcmp(words[0], "b64write") == 0) { diff --git a/tests/libqtest.c b/tests/libqtest.c index 5f5700544729..055aad69e0f0 100644 --- a/tests/libqtest.c +++ b/tests/libqtest.c @@ -741,13 +741,7 @@ void qtest_memwrite(QTestState *s, uint64_t addr, const void *data, size_t size) void qtest_memset(QTestState *s, uint64_t addr, uint8_t pattern, size_t size) { - size_t i; - - qtest_sendf(s, "write 0x%" PRIx64 " 0x%zx 0x", addr, size); - for (i = 0; i < size; i++) { - qtest_sendf(s, "%02x", pattern); - } - qtest_sendf(s, "\n"); + qtest_sendf(s, "memset 0x%" PRIx64 " 0x%zx 0x%02x\n", addr, size, pattern); qtest_rsp(s, 0); } From 91d0374a7ffbd6a9cd0ba159c9160d9f26220cf5 Mon Sep 17 00:00:00 2001 From: John Snow Date: Fri, 22 May 2015 14:13:44 -0400 Subject: [PATCH 15/19] libqos/ahci: Swap memread/write with bufread/write Where it makes sense, use the new faster primitives. For generally small reads/writes such as for the PRDT and FIS packets, stick with the more wasteful but easier to debug memread/memwrite. For ahci-test (before migration tests): With this patch: real 0m3.675s user 0m2.582s sys 0m1.718s Without any qtest protocol improvements: real 0m14.171s user 0m12.072s sys 0m12.527s Signed-off-by: John Snow Message-id: 1430864578-22072-6-git-send-email-jsnow@redhat.com --- tests/ahci-test.c | 8 ++++---- tests/libqos/ahci.c | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/ahci-test.c b/tests/ahci-test.c index 1a967c372415..6e3fa819e0cd 100644 --- a/tests/ahci-test.c +++ b/tests/ahci-test.c @@ -874,7 +874,7 @@ static void ahci_test_io_rw_simple(AHCIQState *ahci, unsigned bufsize, /* Write some indicative pattern to our buffer. */ generate_pattern(tx, bufsize, AHCI_SECTOR_SIZE); - memwrite(ptr, tx, bufsize); + bufwrite(ptr, tx, bufsize); /* Write this buffer to disk, then read it back to the DMA buffer. */ ahci_guest_io(ahci, port, write_cmd, ptr, bufsize, sector); @@ -882,7 +882,7 @@ static void ahci_test_io_rw_simple(AHCIQState *ahci, unsigned bufsize, ahci_guest_io(ahci, port, read_cmd, ptr, bufsize, sector); /*** Read back the Data ***/ - memread(ptr, rx, bufsize); + bufread(ptr, rx, bufsize); g_assert_cmphex(memcmp(tx, rx, bufsize), ==, 0); ahci_free(ahci, ptr); @@ -1018,7 +1018,7 @@ static void test_dma_fragmented(void) /* Create a DMA buffer in guest memory, and write our pattern to it. */ ptr = guest_alloc(ahci->parent->alloc, bufsize); g_assert(ptr); - memwrite(ptr, tx, bufsize); + bufwrite(ptr, tx, bufsize); cmd = ahci_command_create(CMD_WRITE_DMA); ahci_command_adjust(cmd, 0, ptr, bufsize, 32); @@ -1035,7 +1035,7 @@ static void test_dma_fragmented(void) g_free(cmd); /* Read back the guest's receive buffer into local memory */ - memread(ptr, rx, bufsize); + bufread(ptr, rx, bufsize); guest_free(ahci->parent->alloc, ptr); g_assert_cmphex(memcmp(tx, rx, bufsize), ==, 0); diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c index 95bfb3dac8c6..7e17bb691e3b 100644 --- a/tests/libqos/ahci.c +++ b/tests/libqos/ahci.c @@ -653,13 +653,13 @@ void ahci_io(AHCIQState *ahci, uint8_t port, uint8_t ide_cmd, qmemset(ptr, 0x00, bufsize); if (props->write) { - memwrite(ptr, buffer, bufsize); + bufwrite(ptr, buffer, bufsize); } ahci_guest_io(ahci, port, ide_cmd, ptr, bufsize, sector); if (props->read) { - memread(ptr, buffer, bufsize); + bufread(ptr, buffer, bufsize); } ahci_free(ahci, ptr); From 5560b85a31e6f15a8841b66620d9497943094ee4 Mon Sep 17 00:00:00 2001 From: John Snow Date: Fri, 22 May 2015 14:13:44 -0400 Subject: [PATCH 16/19] qtest: pre-buffer hex nibs Instead of converting each byte one-at-a-time and then sending each byte over the wire, use sprintf() to pre-compute all of the hex nibs into a single buffer, then send the entire buffer all at once. This gives a moderate speed boost to memread() and memwrite() functions. Signed-off-by: John Snow Reviewed-by: Markus Armbruster Message-id: 1431021095-7558-2-git-send-email-jsnow@redhat.com --- qtest.c | 11 +++++++---- tests/libqtest.c | 8 +++++--- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/qtest.c b/qtest.c index 04412ddde1b8..05cefd280072 100644 --- a/qtest.c +++ b/qtest.c @@ -414,6 +414,7 @@ static void qtest_process_command(CharDriverState *chr, gchar **words) } else if (strcmp(words[0], "read") == 0) { uint64_t addr, len, i; uint8_t *data; + char *enc; g_assert(words[1] && words[2]); addr = strtoull(words[1], NULL, 0); @@ -422,14 +423,16 @@ static void qtest_process_command(CharDriverState *chr, gchar **words) data = g_malloc(len); cpu_physical_memory_read(addr, data, len); - qtest_send_prefix(chr); - qtest_send(chr, "OK 0x"); + enc = g_malloc(2 * len + 1); for (i = 0; i < len; i++) { - qtest_sendf(chr, "%02x", data[i]); + sprintf(&enc[i * 2], "%02x", data[i]); } - qtest_send(chr, "\n"); + + qtest_send_prefix(chr); + qtest_sendf(chr, "OK 0x%s\n", enc); g_free(data); + g_free(enc); } else if (strcmp(words[0], "b64read") == 0) { uint64_t addr, len; uint8_t *data; diff --git a/tests/libqtest.c b/tests/libqtest.c index 055aad69e0f0..e5188e0327a2 100644 --- a/tests/libqtest.c +++ b/tests/libqtest.c @@ -730,13 +730,15 @@ void qtest_memwrite(QTestState *s, uint64_t addr, const void *data, size_t size) { const uint8_t *ptr = data; size_t i; + char *enc = g_malloc(2 * size + 1); - qtest_sendf(s, "write 0x%" PRIx64 " 0x%zx 0x", addr, size); for (i = 0; i < size; i++) { - qtest_sendf(s, "%02x", ptr[i]); + sprintf(&enc[i * 2], "%02x", ptr[i]); } - qtest_sendf(s, "\n"); + + qtest_sendf(s, "write 0x%" PRIx64 " 0x%zx 0x%s\n", addr, size, enc); qtest_rsp(s, 0); + g_free(enc); } void qtest_memset(QTestState *s, uint64_t addr, uint8_t pattern, size_t size) From 4827ac1e8f8306b24e61b44ea1f2082ea08099bb Mon Sep 17 00:00:00 2001 From: Mark Cave-Ayland Date: Fri, 22 May 2015 14:13:44 -0400 Subject: [PATCH 17/19] macio: move unaligned DMA read code into separate pmac_dma_read() function This considerably helps simplify the complexity of the macio read routines and by switching macio CDROM accesses to use the new code, fixes the issue with the CDROM device being detected intermittently by Darwin/OS X. [Maintainer edit: printf format codes adjusted for 32/64bit. --js] Signed-off-by: Mark Cave-Ayland Acked-by: John Snow Message-id: 1425939893-14404-2-git-send-email-mark.cave-ayland@ilande.co.uk Signed-off-by: John Snow --- hw/ide/macio.c | 253 ++++++++++++++++++++++++++++--------------------- 1 file changed, 147 insertions(+), 106 deletions(-) diff --git a/hw/ide/macio.c b/hw/ide/macio.c index a009674f48aa..037db8b1d836 100644 --- a/hw/ide/macio.c +++ b/hw/ide/macio.c @@ -51,128 +51,166 @@ static const int debug_macio = 0; #define MACIO_PAGE_SIZE 4096 -static void pmac_ide_atapi_transfer_cb(void *opaque, int ret) +static void pmac_dma_read(BlockBackend *blk, + int64_t sector_num, int nb_sectors, + void (*cb)(void *opaque, int ret), void *opaque) { DBDMA_io *io = opaque; MACIOIDEState *m = io->opaque; IDEState *s = idebus_active_if(&m->bus); - int unaligned; + dma_addr_t dma_addr, dma_len; + void *mem; + int nsector, remainder; - if (ret < 0) { - m->aiocb = NULL; - qemu_sglist_destroy(&s->sg); - ide_atapi_io_error(s, ret); - io->remainder_len = 0; - goto done; + qemu_iovec_destroy(&io->iov); + qemu_iovec_init(&io->iov, io->len / MACIO_PAGE_SIZE + 1); + + if (io->remainder_len > 0) { + /* Return remainder of request */ + int transfer = MIN(io->remainder_len, io->len); + + MACIO_DPRINTF("--- DMA read pop - bounce addr: %p addr: %" + HWADDR_PRIx " remainder_len: %x\n", + &io->remainder + (0x200 - transfer), io->addr, + io->remainder_len); + + cpu_physical_memory_write(io->addr, + &io->remainder + (0x200 - transfer), + transfer); + + io->remainder_len -= transfer; + io->len -= transfer; + io->addr += transfer; + + s->io_buffer_index += transfer; + s->io_buffer_size -= transfer; + + if (io->remainder_len != 0) { + /* Still waiting for remainder */ + return; + } + + if (io->len == 0) { + MACIO_DPRINTF("--- finished all read processing; go and finish\n"); + cb(opaque, 0); + return; + } } - if (!m->dma_active) { - MACIO_DPRINTF("waiting for data (%#x - %#x - %x)\n", - s->nsector, io->len, s->status); - /* data not ready yet, wait for the channel to get restarted */ - io->processing = false; - return; + if (s->drive_kind == IDE_CD) { + sector_num = (int64_t)(s->lba << 2) + (s->io_buffer_index >> 9); + } else { + sector_num = ide_get_sector(s) + (s->io_buffer_index >> 9); } - MACIO_DPRINTF("io_buffer_size = %#x\n", s->io_buffer_size); + nsector = ((io->len + 0x1ff) >> 9); + remainder = (nsector << 9) - io->len; - if (s->io_buffer_size > 0) { - m->aiocb = NULL; - qemu_sglist_destroy(&s->sg); + MACIO_DPRINTF("--- DMA read transfer - addr: %" HWADDR_PRIx " len: %x\n", + io->addr, io->len); + + dma_addr = io->addr; + dma_len = io->len; + mem = dma_memory_map(&address_space_memory, dma_addr, &dma_len, + DMA_DIRECTION_FROM_DEVICE); + + if (!remainder) { + MACIO_DPRINTF("--- DMA read aligned - addr: %" HWADDR_PRIx + " len: %x\n", io->addr, io->len); + qemu_iovec_add(&io->iov, mem, io->len); + } else { + MACIO_DPRINTF("--- DMA read unaligned - addr: %" HWADDR_PRIx + " len: %x\n", io->addr, io->len); + qemu_iovec_add(&io->iov, mem, io->len); - s->packet_transfer_size -= s->io_buffer_size; + MACIO_DPRINTF("--- DMA read push - bounce addr: %p " + "remainder_len: %x\n", + &io->remainder + 0x200 - remainder, remainder); + qemu_iovec_add(&io->iov, &io->remainder + 0x200 - remainder, + remainder); - s->io_buffer_index += s->io_buffer_size; - s->lba += s->io_buffer_index >> 11; - s->io_buffer_index &= 0x7ff; + io->remainder_len = remainder; } - s->io_buffer_size = MIN(io->len, s->packet_transfer_size); + s->io_buffer_size -= io->len; + s->io_buffer_index += io->len; - MACIO_DPRINTF("remainder: %d io->len: %d size: %d\n", io->remainder_len, - io->len, s->packet_transfer_size); - if (io->remainder_len && io->len) { - /* guest wants the rest of its previous transfer */ - int remainder_len = MIN(io->remainder_len, io->len); + io->len = 0; - MACIO_DPRINTF("copying remainder %d bytes\n", remainder_len); + MACIO_DPRINTF("--- Block read transfer - sector_num: %"PRIx64" " + "nsector: %x\n", + sector_num, nsector); - cpu_physical_memory_write(io->addr, io->remainder + 0x200 - - remainder_len, remainder_len); + m->aiocb = blk_aio_readv(blk, sector_num, &io->iov, nsector, cb, io); +} - io->addr += remainder_len; - io->len -= remainder_len; - s->io_buffer_size = remainder_len; - io->remainder_len -= remainder_len; - /* treat remainder as individual transfer, start again */ - qemu_sglist_init(&s->sg, DEVICE(m), io->len / MACIO_PAGE_SIZE + 1, - &address_space_memory); - pmac_ide_atapi_transfer_cb(opaque, 0); +static void pmac_ide_atapi_transfer_cb(void *opaque, int ret) +{ + DBDMA_io *io = opaque; + MACIOIDEState *m = io->opaque; + IDEState *s = idebus_active_if(&m->bus); + int64_t sector_num; + int nsector, remainder; + + MACIO_DPRINTF("\ns is %p\n", s); + MACIO_DPRINTF("io_buffer_index: %x\n", s->io_buffer_index); + MACIO_DPRINTF("io_buffer_size: %x packet_transfer_size: %x\n", + s->io_buffer_size, s->packet_transfer_size); + MACIO_DPRINTF("lba: %x\n", s->lba); + MACIO_DPRINTF("io_addr: %" HWADDR_PRIx " io_len: %x\n", io->addr, + io->len); + + if (ret < 0) { + MACIO_DPRINTF("THERE WAS AN ERROR! %d\n", ret); + ide_atapi_io_error(s, ret); + goto done; + } + + if (!m->dma_active) { + MACIO_DPRINTF("waiting for data (%#x - %#x - %x)\n", + s->nsector, io->len, s->status); + /* data not ready yet, wait for the channel to get restarted */ + io->processing = false; return; } - if (!s->packet_transfer_size) { - MACIO_DPRINTF("end of transfer\n"); + if (s->io_buffer_size <= 0) { ide_atapi_cmd_ok(s); m->dma_active = false; + goto done; } if (io->len == 0) { - MACIO_DPRINTF("end of DMA\n"); + MACIO_DPRINTF("End of DMA transfer\n"); goto done; } - /* launch next transfer */ - - /* handle unaligned accesses first, get them over with and only do the - remaining bulk transfer using our async DMA helpers */ - unaligned = io->len & 0x1ff; - if (unaligned) { - int sector_num = (s->lba << 2) + (s->io_buffer_index >> 9); - int nsector = io->len >> 9; - - MACIO_DPRINTF("precopying unaligned %d bytes to %#" HWADDR_PRIx "\n", - unaligned, io->addr + io->len - unaligned); - - blk_read(s->blk, sector_num + nsector, io->remainder, 1); - cpu_physical_memory_write(io->addr + io->len - unaligned, - io->remainder, unaligned); - - io->len -= unaligned; - } - - MACIO_DPRINTF("io->len = %#x\n", io->len); - - qemu_sglist_init(&s->sg, DEVICE(m), io->len / MACIO_PAGE_SIZE + 1, - &address_space_memory); - qemu_sglist_add(&s->sg, io->addr, io->len); - io->addr += s->io_buffer_size; - io->remainder_len = MIN(s->packet_transfer_size - s->io_buffer_size, - (0x200 - unaligned) & 0x1ff); - MACIO_DPRINTF("set remainder to: %d\n", io->remainder_len); - - /* We would read no data from the block layer, thus not get a callback. - Just fake completion manually. */ - if (!io->len) { - pmac_ide_atapi_transfer_cb(opaque, 0); - return; + if (s->lba == -1) { + /* Non-block ATAPI transfer - just copy to RAM */ + s->io_buffer_size = MIN(s->io_buffer_size, io->len); + cpu_physical_memory_write(io->addr, s->io_buffer, s->io_buffer_size); + ide_atapi_cmd_ok(s); + m->dma_active = false; + goto done; } - io->len = 0; + /* Calculate number of sectors */ + sector_num = (int64_t)(s->lba << 2) + (s->io_buffer_index >> 9); + nsector = (io->len + 0x1ff) >> 9; + remainder = io->len & 0x1ff; - MACIO_DPRINTF("sector_num=%d size=%d, cmd_cmd=%d\n", - (s->lba << 2) + (s->io_buffer_index >> 9), - s->packet_transfer_size, s->dma_cmd); + MACIO_DPRINTF("nsector: %d remainder: %x\n", nsector, remainder); + MACIO_DPRINTF("sector: %"PRIx64" %zx\n", sector_num, io->iov.size / 512); - m->aiocb = dma_blk_read(s->blk, &s->sg, - (int64_t)(s->lba << 2) + (s->io_buffer_index >> 9), - pmac_ide_atapi_transfer_cb, io); + pmac_dma_read(s->blk, sector_num, nsector, pmac_ide_atapi_transfer_cb, io); return; done: - MACIO_DPRINTF("done DMA\n"); + MACIO_DPRINTF("done DMA\n\n"); block_acct_done(blk_get_stats(s->blk), &s->acct); io->dma_end(opaque); + + return; } static void pmac_ide_transfer_cb(void *opaque, int ret) @@ -364,33 +402,14 @@ static void pmac_ide_transfer(DBDMA_io *io) MACIO_DPRINTF("\n"); - s->io_buffer_size = 0; if (s->drive_kind == IDE_CD) { - - /* Handle non-block ATAPI DMA transfers */ - if (s->lba == -1) { - s->io_buffer_size = MIN(io->len, s->packet_transfer_size); - block_acct_start(blk_get_stats(s->blk), &s->acct, s->io_buffer_size, - BLOCK_ACCT_READ); - MACIO_DPRINTF("non-block ATAPI DMA transfer size: %d\n", - s->io_buffer_size); - - /* Copy ATAPI buffer directly to RAM and finish */ - cpu_physical_memory_write(io->addr, s->io_buffer, - s->io_buffer_size); - ide_atapi_cmd_ok(s); - m->dma_active = false; - - MACIO_DPRINTF("end of non-block ATAPI DMA transfer\n"); - block_acct_done(blk_get_stats(s->blk), &s->acct); - io->dma_end(io); - return; - } - block_acct_start(blk_get_stats(s->blk), &s->acct, io->len, BLOCK_ACCT_READ); + pmac_ide_atapi_transfer_cb(io, 0); return; + } else { + s->io_buffer_size = 0; } switch (s->dma_cmd) { @@ -562,6 +581,28 @@ static void ide_dbdma_start(IDEDMA *dma, IDEState *s, BlockCompletionFunc *cb) { MACIOIDEState *m = container_of(dma, MACIOIDEState, dma); + DBDMAState *dbdma = m->dbdma; + DBDMA_io *io; + int i; + + if (s->drive_kind == IDE_CD) { + s->io_buffer_index = 0; + s->io_buffer_size = s->packet_transfer_size; + + MACIO_DPRINTF("\n\n------------ IDE transfer\n"); + MACIO_DPRINTF("buffer_size: %x buffer_index: %x\n", + s->io_buffer_size, s->io_buffer_index); + MACIO_DPRINTF("lba: %x size: %x\n", s->lba, s->io_buffer_size); + MACIO_DPRINTF("-------------------------\n"); + + for (i = 0; i < DBDMA_CHANNELS; i++) { + io = &dbdma->channels[i].io; + + if (io->opaque == m) { + io->remainder_len = 0; + } + } + } MACIO_DPRINTF("\n"); m->dma_active = true; From bd4214fc92090694aefa17882815c6109f0fd70c Mon Sep 17 00:00:00 2001 From: Mark Cave-Ayland Date: Fri, 22 May 2015 14:13:44 -0400 Subject: [PATCH 18/19] macio: move unaligned DMA write code into separate pmac_dma_write() function Similarly switch the macio IDE routines over to use the new function and tidy-up the remaining code as required. [Maintainer edit: printf format codes adjusted for 32/64bit. --js] Signed-off-by: Mark Cave-Ayland Acked-by: John Snow Message-id: 1425939893-14404-3-git-send-email-mark.cave-ayland@ilande.co.uk Signed-off-by: John Snow --- hw/ide/macio.c | 268 +++++++++++++++++-------------------- include/hw/ppc/mac_dbdma.h | 4 - 2 files changed, 125 insertions(+), 147 deletions(-) diff --git a/hw/ide/macio.c b/hw/ide/macio.c index 037db8b1d836..585a27bd6c7f 100644 --- a/hw/ide/macio.c +++ b/hw/ide/macio.c @@ -144,6 +144,101 @@ static void pmac_dma_read(BlockBackend *blk, m->aiocb = blk_aio_readv(blk, sector_num, &io->iov, nsector, cb, io); } +static void pmac_dma_write(BlockBackend *blk, + int64_t sector_num, int nb_sectors, + void (*cb)(void *opaque, int ret), void *opaque) +{ + DBDMA_io *io = opaque; + MACIOIDEState *m = io->opaque; + IDEState *s = idebus_active_if(&m->bus); + dma_addr_t dma_addr, dma_len; + void *mem; + int nsector, remainder; + int extra = 0; + + qemu_iovec_destroy(&io->iov); + qemu_iovec_init(&io->iov, io->len / MACIO_PAGE_SIZE + 1); + + if (io->remainder_len > 0) { + /* Return remainder of request */ + int transfer = MIN(io->remainder_len, io->len); + + MACIO_DPRINTF("--- processing write remainder %x\n", transfer); + cpu_physical_memory_read(io->addr, + &io->remainder + (0x200 - transfer), + transfer); + + io->remainder_len -= transfer; + io->len -= transfer; + io->addr += transfer; + + s->io_buffer_index += transfer; + s->io_buffer_size -= transfer; + + if (io->remainder_len != 0) { + /* Still waiting for remainder */ + return; + } + + MACIO_DPRINTF("--> prepending bounce buffer with size 0x200\n"); + + /* Sector transfer complete - prepend to request */ + qemu_iovec_add(&io->iov, &io->remainder, 0x200); + extra = 1; + } + + if (s->drive_kind == IDE_CD) { + sector_num = (int64_t)(s->lba << 2) + (s->io_buffer_index >> 9); + } else { + sector_num = ide_get_sector(s) + (s->io_buffer_index >> 9); + } + + nsector = (io->len >> 9); + remainder = io->len - (nsector << 9); + + MACIO_DPRINTF("--- DMA write transfer - addr: %" HWADDR_PRIx " len: %x\n", + io->addr, io->len); + MACIO_DPRINTF("xxx remainder: %x\n", remainder); + MACIO_DPRINTF("xxx sector_num: %"PRIx64" nsector: %x\n", + sector_num, nsector); + + dma_addr = io->addr; + dma_len = io->len; + mem = dma_memory_map(&address_space_memory, dma_addr, &dma_len, + DMA_DIRECTION_TO_DEVICE); + + if (!remainder) { + MACIO_DPRINTF("--- DMA write aligned - addr: %" HWADDR_PRIx + " len: %x\n", io->addr, io->len); + qemu_iovec_add(&io->iov, mem, io->len); + } else { + /* Write up to last complete sector */ + MACIO_DPRINTF("--- DMA write unaligned - addr: %" HWADDR_PRIx + " len: %x\n", io->addr, (nsector << 9)); + qemu_iovec_add(&io->iov, mem, (nsector << 9)); + + MACIO_DPRINTF("--- DMA write read - bounce addr: %p " + "remainder_len: %x\n", &io->remainder, remainder); + cpu_physical_memory_read(io->addr + (nsector << 9), &io->remainder, + remainder); + + io->remainder_len = 0x200 - remainder; + + MACIO_DPRINTF("xxx remainder_len: %x\n", io->remainder_len); + } + + s->io_buffer_size -= ((nsector + extra) << 9); + s->io_buffer_index += ((nsector + extra) << 9); + + io->len = 0; + + MACIO_DPRINTF("--- Block write transfer - sector_num: %"PRIx64" " + "nsector: %x\n", sector_num, nsector + extra); + + m->aiocb = blk_aio_writev(blk, sector_num, &io->iov, nsector + extra, cb, + io); +} + static void pmac_ide_atapi_transfer_cb(void *opaque, int ret) { DBDMA_io *io = opaque; @@ -218,24 +313,19 @@ static void pmac_ide_transfer_cb(void *opaque, int ret) DBDMA_io *io = opaque; MACIOIDEState *m = io->opaque; IDEState *s = idebus_active_if(&m->bus); - int n = 0; int64_t sector_num; - int unaligned; + int nsector, remainder; + + MACIO_DPRINTF("pmac_ide_transfer_cb\n"); if (ret < 0) { MACIO_DPRINTF("DMA error\n"); m->aiocb = NULL; - qemu_sglist_destroy(&s->sg); ide_dma_error(s); io->remainder_len = 0; goto done; } - if (--io->requests) { - /* More requests still in flight */ - return; - } - if (!m->dma_active) { MACIO_DPRINTF("waiting for data (%#x - %#x - %x)\n", s->nsector, io->len, s->status); @@ -244,155 +334,48 @@ static void pmac_ide_transfer_cb(void *opaque, int ret) return; } - sector_num = ide_get_sector(s); - MACIO_DPRINTF("io_buffer_size = %#x\n", s->io_buffer_size); - if (s->io_buffer_size > 0) { - m->aiocb = NULL; - qemu_sglist_destroy(&s->sg); - n = (s->io_buffer_size + 0x1ff) >> 9; - sector_num += n; - ide_set_sector(s, sector_num); - s->nsector -= n; - } - - if (io->finish_remain_read) { - /* Finish a stale read from the last iteration */ - io->finish_remain_read = false; - cpu_physical_memory_write(io->finish_addr, io->remainder, - io->finish_len); - } - - MACIO_DPRINTF("remainder: %d io->len: %d nsector: %d " - "sector_num: %" PRId64 "\n", - io->remainder_len, io->len, s->nsector, sector_num); - if (io->remainder_len && io->len) { - /* guest wants the rest of its previous transfer */ - int remainder_len = MIN(io->remainder_len, io->len); - uint8_t *p = &io->remainder[0x200 - remainder_len]; - - MACIO_DPRINTF("copying remainder %d bytes at %#" HWADDR_PRIx "\n", - remainder_len, io->addr); - - switch (s->dma_cmd) { - case IDE_DMA_READ: - cpu_physical_memory_write(io->addr, p, remainder_len); - break; - case IDE_DMA_WRITE: - cpu_physical_memory_read(io->addr, p, remainder_len); - break; - case IDE_DMA_TRIM: - break; - } - io->addr += remainder_len; - io->len -= remainder_len; - io->remainder_len -= remainder_len; - - if (s->dma_cmd == IDE_DMA_WRITE && !io->remainder_len) { - io->requests++; - qemu_iovec_reset(&io->iov); - qemu_iovec_add(&io->iov, io->remainder, 0x200); - - m->aiocb = blk_aio_writev(s->blk, sector_num - 1, &io->iov, 1, - pmac_ide_transfer_cb, io); - } - } - - if (s->nsector == 0 && !io->remainder_len) { + if (s->io_buffer_size <= 0) { MACIO_DPRINTF("end of transfer\n"); s->status = READY_STAT | SEEK_STAT; ide_set_irq(s->bus); m->dma_active = false; + goto done; } if (io->len == 0) { - MACIO_DPRINTF("end of DMA\n"); + MACIO_DPRINTF("End of DMA transfer\n"); goto done; } - /* launch next transfer */ - - s->io_buffer_index = 0; - s->io_buffer_size = MIN(io->len, s->nsector * 512); - - /* handle unaligned accesses first, get them over with and only do the - remaining bulk transfer using our async DMA helpers */ - unaligned = io->len & 0x1ff; - if (unaligned) { - int nsector = io->len >> 9; - - MACIO_DPRINTF("precopying unaligned %d bytes to %#" HWADDR_PRIx "\n", - unaligned, io->addr + io->len - unaligned); - - switch (s->dma_cmd) { - case IDE_DMA_READ: - io->requests++; - io->finish_addr = io->addr + io->len - unaligned; - io->finish_len = unaligned; - io->finish_remain_read = true; - qemu_iovec_reset(&io->iov); - qemu_iovec_add(&io->iov, io->remainder, 0x200); - - m->aiocb = blk_aio_readv(s->blk, sector_num + nsector, &io->iov, 1, - pmac_ide_transfer_cb, io); - break; - case IDE_DMA_WRITE: - /* cache the contents in our io struct */ - cpu_physical_memory_read(io->addr + io->len - unaligned, - io->remainder + io->remainder_len, - unaligned); - break; - case IDE_DMA_TRIM: - break; - } - } - - MACIO_DPRINTF("io->len = %#x\n", io->len); - - qemu_sglist_init(&s->sg, DEVICE(m), io->len / MACIO_PAGE_SIZE + 1, - &address_space_memory); - qemu_sglist_add(&s->sg, io->addr, io->len); - io->addr += io->len + unaligned; - io->remainder_len = (0x200 - unaligned) & 0x1ff; - MACIO_DPRINTF("set remainder to: %d\n", io->remainder_len); - - /* Only subsector reads happening */ - if (!io->len) { - if (!io->requests) { - io->requests++; - pmac_ide_transfer_cb(opaque, ret); - } - return; - } + /* Calculate number of sectors */ + sector_num = ide_get_sector(s) + (s->io_buffer_index >> 9); + nsector = (io->len + 0x1ff) >> 9; + remainder = io->len & 0x1ff; - io->len = 0; + s->nsector -= nsector; - MACIO_DPRINTF("sector_num=%" PRId64 " n=%d, nsector=%d, cmd_cmd=%d\n", - sector_num, n, s->nsector, s->dma_cmd); + MACIO_DPRINTF("nsector: %d remainder: %x\n", nsector, remainder); + MACIO_DPRINTF("sector: %"PRIx64" %x\n", sector_num, nsector); switch (s->dma_cmd) { case IDE_DMA_READ: - m->aiocb = dma_blk_read(s->blk, &s->sg, sector_num, - pmac_ide_transfer_cb, io); + pmac_dma_read(s->blk, sector_num, nsector, pmac_ide_transfer_cb, io); break; case IDE_DMA_WRITE: - m->aiocb = dma_blk_write(s->blk, &s->sg, sector_num, - pmac_ide_transfer_cb, io); + pmac_dma_write(s->blk, sector_num, nsector, pmac_ide_transfer_cb, io); break; case IDE_DMA_TRIM: - m->aiocb = dma_blk_io(s->blk, &s->sg, sector_num, - ide_issue_trim, pmac_ide_transfer_cb, io, - DMA_DIRECTION_TO_DEVICE); + MACIO_DPRINTF("TRIM command issued!"); break; } - io->requests++; return; done: if (s->dma_cmd == IDE_DMA_READ || s->dma_cmd == IDE_DMA_WRITE) { block_acct_done(blk_get_stats(s->blk), &s->acct); } - io->dma_end(io); + io->dma_end(opaque); } static void pmac_ide_transfer(DBDMA_io *io) @@ -408,8 +391,6 @@ static void pmac_ide_transfer(DBDMA_io *io) pmac_ide_atapi_transfer_cb(io, 0); return; - } else { - s->io_buffer_size = 0; } switch (s->dma_cmd) { @@ -425,7 +406,6 @@ static void pmac_ide_transfer(DBDMA_io *io) break; } - io->requests++; pmac_ide_transfer_cb(io, 0); } @@ -585,22 +565,24 @@ static void ide_dbdma_start(IDEDMA *dma, IDEState *s, DBDMA_io *io; int i; + s->io_buffer_index = 0; if (s->drive_kind == IDE_CD) { - s->io_buffer_index = 0; s->io_buffer_size = s->packet_transfer_size; + } else { + s->io_buffer_size = s->nsector * 0x200; + } - MACIO_DPRINTF("\n\n------------ IDE transfer\n"); - MACIO_DPRINTF("buffer_size: %x buffer_index: %x\n", - s->io_buffer_size, s->io_buffer_index); - MACIO_DPRINTF("lba: %x size: %x\n", s->lba, s->io_buffer_size); - MACIO_DPRINTF("-------------------------\n"); + MACIO_DPRINTF("\n\n------------ IDE transfer\n"); + MACIO_DPRINTF("buffer_size: %x buffer_index: %x\n", + s->io_buffer_size, s->io_buffer_index); + MACIO_DPRINTF("lba: %x size: %x\n", s->lba, s->io_buffer_size); + MACIO_DPRINTF("-------------------------\n"); - for (i = 0; i < DBDMA_CHANNELS; i++) { - io = &dbdma->channels[i].io; + for (i = 0; i < DBDMA_CHANNELS; i++) { + io = &dbdma->channels[i].io; - if (io->opaque == m) { - io->remainder_len = 0; - } + if (io->opaque == m) { + io->remainder_len = 0; } } diff --git a/include/hw/ppc/mac_dbdma.h b/include/hw/ppc/mac_dbdma.h index d7db06c03128..c5803279da6f 100644 --- a/include/hw/ppc/mac_dbdma.h +++ b/include/hw/ppc/mac_dbdma.h @@ -43,10 +43,6 @@ struct DBDMA_io { uint8_t remainder[0x200]; int remainder_len; QEMUIOVector iov; - bool finish_remain_read; - hwaddr finish_addr; - hwaddr finish_len; - int requests; }; /* From cd6cb73beb63e5fa62ca8ed540b9d54063b15c44 Mon Sep 17 00:00:00 2001 From: John Snow Date: Fri, 22 May 2015 14:13:44 -0400 Subject: [PATCH 19/19] ahci: do not remap clb/fis unconditionally This continues the IOMMU fix from 2.3, where we should not attempt to remap the CLB or FIS RX buffers if the AHCI device is currently running. The same applies to migration: keep our mitts off these registers unless the device is supposed to be on. Does not impact backwards compatibility for the AHCI device. Signed-off-by: John Snow Reviewed-by: Stefan Hajnoczi Message-id: 1431470173-30847-2-git-send-email-jsnow@redhat.com --- hw/ide/ahci.c | 88 ++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 63 insertions(+), 25 deletions(-) diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 8e36dec5a9e2..9e5d86297cc4 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -198,6 +198,61 @@ static void map_page(AddressSpace *as, uint8_t **ptr, uint64_t addr, } } +/** + * Check the cmd register to see if we should start or stop + * the DMA or FIS RX engines. + * + * @ad: Device to engage. + * @allow_stop: Allow device to transition from started to stopped? + * 'no' is useful for migration post_load, which does not expect a transition. + * + * @return 0 on success, -1 on error. + */ +static int ahci_cond_start_engines(AHCIDevice *ad, bool allow_stop) +{ + AHCIPortRegs *pr = &ad->port_regs; + + if (pr->cmd & PORT_CMD_START) { + if (ahci_map_clb_address(ad)) { + pr->cmd |= PORT_CMD_LIST_ON; + } else { + error_report("AHCI: Failed to start DMA engine: " + "bad command list buffer address"); + return -1; + } + } else if (pr->cmd & PORT_CMD_LIST_ON) { + if (allow_stop) { + ahci_unmap_clb_address(ad); + pr->cmd = pr->cmd & ~(PORT_CMD_LIST_ON); + } else { + error_report("AHCI: DMA engine should be off, " + "but appears to still be running"); + return -1; + } + } + + if (pr->cmd & PORT_CMD_FIS_RX) { + if (ahci_map_fis_address(ad)) { + pr->cmd |= PORT_CMD_FIS_ON; + } else { + error_report("AHCI: Failed to start FIS receive engine: " + "bad FIS receive buffer address"); + return -1; + } + } else if (pr->cmd & PORT_CMD_FIS_ON) { + if (allow_stop) { + ahci_unmap_fis_address(ad); + pr->cmd = pr->cmd & ~(PORT_CMD_FIS_ON); + } else { + error_report("AHCI: FIS receive engine should be off, " + "but appears to still be running"); + return -1; + } + } + + return 0; +} + static void ahci_port_write(AHCIState *s, int port, int offset, uint32_t val) { AHCIPortRegs *pr = &s->dev[port].port_regs; @@ -229,29 +284,8 @@ static void ahci_port_write(AHCIState *s, int port, int offset, uint32_t val) * including LIST_ON and FIS_ON. */ pr->cmd = (pr->cmd & PORT_CMD_RO_MASK) | (val & ~PORT_CMD_RO_MASK); - if (pr->cmd & PORT_CMD_START) { - if (ahci_map_clb_address(&s->dev[port])) { - pr->cmd |= PORT_CMD_LIST_ON; - } else { - error_report("AHCI: Failed to start DMA engine: " - "bad command list buffer address"); - } - } else if (pr->cmd & PORT_CMD_LIST_ON) { - ahci_unmap_clb_address(&s->dev[port]); - pr->cmd = pr->cmd & ~(PORT_CMD_LIST_ON); - } - - if (pr->cmd & PORT_CMD_FIS_RX) { - if (ahci_map_fis_address(&s->dev[port])) { - pr->cmd |= PORT_CMD_FIS_ON; - } else { - error_report("AHCI: Failed to start FIS receive engine: " - "bad FIS receive buffer address"); - } - } else if (pr->cmd & PORT_CMD_FIS_ON) { - ahci_unmap_fis_address(&s->dev[port]); - pr->cmd = pr->cmd & ~(PORT_CMD_FIS_ON); - } + /* Check FIS RX and CLB engines, allow transition to false: */ + ahci_cond_start_engines(&s->dev[port], true); /* XXX usually the FIS would be pending on the bus here and issuing deferred until the OS enables FIS receival. @@ -1404,8 +1438,12 @@ static int ahci_state_post_load(void *opaque, int version_id) for (i = 0; i < s->ports; i++) { ad = &s->dev[i]; - ahci_map_clb_address(ad); - ahci_map_fis_address(ad); + /* Only remap the CLB address if appropriate, disallowing a state + * transition from 'on' to 'off' it should be consistent here. */ + if (ahci_cond_start_engines(ad, false) != 0) { + return -1; + } + /* * If an error is present, ad->busy_slot will be valid and not -1. * In this case, an operation is waiting to resume and will re-check