* [Qemu-devel] [PULL 01/19] configure: require glib 2.22
2015-05-22 19:59 [Qemu-devel] [PULL 00/19] Ide patches John Snow
@ 2015-05-22 19:59 ` John Snow
2015-05-22 19:59 ` [Qemu-devel] [PULL 02/19] glib: remove stale compat functions John Snow
` (18 subsequent siblings)
19 siblings, 0 replies; 32+ messages in thread
From: John Snow @ 2015-05-22 19:59 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow, Paolo Bonzini
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 <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
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 f758f32..9852aef 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"
--
2.1.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PULL 02/19] glib: remove stale compat functions
2015-05-22 19:59 [Qemu-devel] [PULL 00/19] Ide patches John Snow
2015-05-22 19:59 ` [Qemu-devel] [PULL 01/19] configure: require glib 2.22 John Snow
@ 2015-05-22 19:59 ` John Snow
2015-05-22 19:59 ` [Qemu-devel] [PULL 03/19] libqos/ahci: Add halted command helpers John Snow
` (17 subsequent siblings)
19 siblings, 0 replies; 32+ messages in thread
From: John Snow @ 2015-05-22 19:59 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow
Since we're bumping the version to 2.22+,
remove the now-stale compat functions.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
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 28d9f15..318e000 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)
--
2.1.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PULL 03/19] libqos/ahci: Add halted command helpers
2015-05-22 19:59 [Qemu-devel] [PULL 00/19] Ide patches John Snow
2015-05-22 19:59 ` [Qemu-devel] [PULL 01/19] configure: require glib 2.22 John Snow
2015-05-22 19:59 ` [Qemu-devel] [PULL 02/19] glib: remove stale compat functions John Snow
@ 2015-05-22 19:59 ` John Snow
2015-05-22 19:59 ` [Qemu-devel] [PULL 04/19] libqos/ahci: Fix sector set method John Snow
` (16 subsequent siblings)
19 siblings, 0 replies; 32+ messages in thread
From: John Snow @ 2015-05-22 19:59 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow
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 <jsnow@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
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 843cf72..229409b 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 40e8ca4..779e812 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);
--
2.1.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PULL 04/19] libqos/ahci: Fix sector set method
2015-05-22 19:59 [Qemu-devel] [PULL 00/19] Ide patches John Snow
` (2 preceding siblings ...)
2015-05-22 19:59 ` [Qemu-devel] [PULL 03/19] libqos/ahci: Add halted command helpers John Snow
@ 2015-05-22 19:59 ` John Snow
2015-05-22 19:59 ` [Qemu-devel] [PULL 05/19] libqos: Add migration helpers John Snow
` (15 subsequent siblings)
19 siblings, 0 replies; 32+ messages in thread
From: John Snow @ 2015-05-22 19:59 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow
|| 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 <jsnow@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
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 229409b..8c8fd89 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);
--
2.1.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PULL 05/19] libqos: Add migration helpers
2015-05-22 19:59 [Qemu-devel] [PULL 00/19] Ide patches John Snow
` (3 preceding siblings ...)
2015-05-22 19:59 ` [Qemu-devel] [PULL 04/19] libqos/ahci: Fix sector set method John Snow
@ 2015-05-22 19:59 ` John Snow
2015-05-22 19:59 ` [Qemu-devel] [PULL 06/19] ich9/ahci: Enable Migration John Snow
` (14 subsequent siblings)
19 siblings, 0 replies; 32+ messages in thread
From: John Snow @ 2015-05-22 19:59 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow
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 <jsnow@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
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 7e72078..fce625b 100644
--- a/tests/libqos/libqos.c
+++ b/tests/libqos/libqos.c
@@ -1,5 +1,6 @@
#include <stdio.h>
#include <stdlib.h>
+#include <string.h>
#include <glib.h>
#include <unistd.h>
#include <fcntl.h>
@@ -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 f57362b..e1f14ea 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 67f3190..8276130 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 71ac407..0c6c9b7 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,
--
2.1.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PULL 06/19] ich9/ahci: Enable Migration
2015-05-22 19:59 [Qemu-devel] [PULL 00/19] Ide patches John Snow
` (4 preceding siblings ...)
2015-05-22 19:59 ` [Qemu-devel] [PULL 05/19] libqos: Add migration helpers John Snow
@ 2015-05-22 19:59 ` John Snow
2015-05-22 19:59 ` [Qemu-devel] [PULL 07/19] qtest/ahci: Add migration test John Snow
` (13 subsequent siblings)
19 siblings, 0 replies; 32+ messages in thread
From: John Snow @ 2015-05-22 19:59 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow
Lift the flag preventing the migration of the ICH9/AHCI devices.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
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 833fd45..8e36dec 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 b1d8874..350c7f1 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),
--
2.1.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PULL 07/19] qtest/ahci: Add migration test
2015-05-22 19:59 [Qemu-devel] [PULL 00/19] Ide patches John Snow
` (5 preceding siblings ...)
2015-05-22 19:59 ` [Qemu-devel] [PULL 06/19] ich9/ahci: Enable Migration John Snow
@ 2015-05-22 19:59 ` John Snow
2015-11-05 15:26 ` Peter Maydell
2015-05-22 19:59 ` [Qemu-devel] [PULL 08/19] qtest/ahci: add migrate dma test John Snow
` (12 subsequent siblings)
19 siblings, 1 reply; 32+ messages in thread
From: John Snow @ 2015-05-22 19:59 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow
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 <jsnow@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
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 7c23bb2..9fccafc 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 */
--
2.1.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PULL 07/19] qtest/ahci: Add migration test
2015-05-22 19:59 ` [Qemu-devel] [PULL 07/19] qtest/ahci: Add migration test John Snow
@ 2015-11-05 15:26 ` Peter Maydell
2015-11-05 16:52 ` John Snow
0 siblings, 1 reply; 32+ messages in thread
From: Peter Maydell @ 2015-11-05 15:26 UTC (permalink / raw)
To: John Snow; +Cc: QEMU Developers
On 22 May 2015 at 20:59, John Snow <jsnow@redhat.com> wrote:
> 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.
> +static void test_migrate_sanity(void)
> +{
> + AHCIQState *src, *dst;
> + const char *uri = "tcp:127.0.0.1:1234";
Hi. I've just noticed that these migration tests are hardcoded
to port 1234. The reason I noticed is that this is also the default
port used for the gdbstub if you use QEMU's "-s" argument, so
a 'make check' running in a completely different QEMU tree failed
because I happened to be running a QEMU with the gdbstub at the
same time.
Could we at least switch to a different port number, please?
(Ideally we would want to be able to pick a port such that
you could do 'make check' in two different qemu trees without
them interfering with each other, but that's harder.)
thanks
-- PMM
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PULL 07/19] qtest/ahci: Add migration test
2015-11-05 15:26 ` Peter Maydell
@ 2015-11-05 16:52 ` John Snow
0 siblings, 0 replies; 32+ messages in thread
From: John Snow @ 2015-11-05 16:52 UTC (permalink / raw)
To: Peter Maydell; +Cc: QEMU Developers
On 11/05/2015 10:26 AM, Peter Maydell wrote:
> On 22 May 2015 at 20:59, John Snow <jsnow@redhat.com> wrote:
>> 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.
>
>> +static void test_migrate_sanity(void)
>> +{
>> + AHCIQState *src, *dst;
>> + const char *uri = "tcp:127.0.0.1:1234";
>
> Hi. I've just noticed that these migration tests are hardcoded
> to port 1234. The reason I noticed is that this is also the default
> port used for the gdbstub if you use QEMU's "-s" argument, so
> a 'make check' running in a completely different QEMU tree failed
> because I happened to be running a QEMU with the gdbstub at the
> same time.
>
> Could we at least switch to a different port number, please?
> (Ideally we would want to be able to pick a port such that
> you could do 'make check' in two different qemu trees without
> them interfering with each other, but that's harder.)
>
> thanks
> -- PMM
>
I'll just have it use something that isn't TCP instead, since it's not
really vital to the test.
--js
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 08/19] qtest/ahci: add migrate dma test
2015-05-22 19:59 [Qemu-devel] [PULL 00/19] Ide patches John Snow
` (6 preceding siblings ...)
2015-05-22 19:59 ` [Qemu-devel] [PULL 07/19] qtest/ahci: Add migration test John Snow
@ 2015-05-22 19:59 ` John Snow
2015-05-22 19:59 ` [Qemu-devel] [PULL 09/19] qtest/ahci: add flush migrate test John Snow
` (11 subsequent siblings)
19 siblings, 0 replies; 32+ messages in thread
From: John Snow @ 2015-05-22 19:59 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow
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 <jsnow@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
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 9fccafc..6513330 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 8c8fd89..95bfb3d 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);
--
2.1.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PULL 09/19] qtest/ahci: add flush migrate test
2015-05-22 19:59 [Qemu-devel] [PULL 00/19] Ide patches John Snow
` (7 preceding siblings ...)
2015-05-22 19:59 ` [Qemu-devel] [PULL 08/19] qtest/ahci: add migrate dma test John Snow
@ 2015-05-22 19:59 ` John Snow
2015-05-22 19:59 ` [Qemu-devel] [PULL 10/19] qtest/ahci: add halted dma test John Snow
` (10 subsequent siblings)
19 siblings, 0 replies; 32+ messages in thread
From: John Snow @ 2015-05-22 19:59 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow
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 <jsnow@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
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 6513330..f6de355 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);
--
2.1.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PULL 10/19] qtest/ahci: add halted dma test
2015-05-22 19:59 [Qemu-devel] [PULL 00/19] Ide patches John Snow
` (8 preceding siblings ...)
2015-05-22 19:59 ` [Qemu-devel] [PULL 09/19] qtest/ahci: add flush migrate test John Snow
@ 2015-05-22 19:59 ` John Snow
2015-05-22 19:59 ` [Qemu-devel] [PULL 11/19] qtest/ahci: add migrate " John Snow
` (9 subsequent siblings)
19 siblings, 0 replies; 32+ messages in thread
From: John Snow @ 2015-05-22 19:59 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow
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 <jsnow@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
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 f6de355..4eb1105 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -1155,6 +1155,65 @@ static void test_migrate_dma(void)
}
/**
+ * 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.
*/
static void test_flush_migrate(void)
@@ -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();
--
2.1.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PULL 11/19] qtest/ahci: add migrate halted dma test
2015-05-22 19:59 [Qemu-devel] [PULL 00/19] Ide patches John Snow
` (9 preceding siblings ...)
2015-05-22 19:59 ` [Qemu-devel] [PULL 10/19] qtest/ahci: add halted dma test John Snow
@ 2015-05-22 19:59 ` John Snow
2015-05-22 19:59 ` [Qemu-devel] [PULL 12/19] qtest: allow arbitrarily long sends John Snow
` (8 subsequent siblings)
19 siblings, 0 replies; 32+ messages in thread
From: John Snow @ 2015-05-22 19:59 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow
Test migrating a halted DMA transaction.
Resume, then test data integrity.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
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 4eb1105..1a967c3 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -1214,6 +1214,78 @@ static void test_halted_dma(void)
}
/**
+ * 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.
*/
static void test_flush_migrate(void)
@@ -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();
--
2.1.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PULL 12/19] qtest: allow arbitrarily long sends
2015-05-22 19:59 [Qemu-devel] [PULL 00/19] Ide patches John Snow
` (10 preceding siblings ...)
2015-05-22 19:59 ` [Qemu-devel] [PULL 11/19] qtest/ahci: add migrate " John Snow
@ 2015-05-22 19:59 ` John Snow
2015-05-22 19:59 ` [Qemu-devel] [PULL 13/19] qtest: Add base64 encoded read/write John Snow
` (7 subsequent siblings)
19 siblings, 0 replies; 32+ messages in thread
From: John Snow @ 2015-05-22 19:59 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow
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 <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
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 8d1e66c..3738eae 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]);
}
}
--
2.1.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PULL 13/19] qtest: Add base64 encoded read/write
2015-05-22 19:59 [Qemu-devel] [PULL 00/19] Ide patches John Snow
` (11 preceding siblings ...)
2015-05-22 19:59 ` [Qemu-devel] [PULL 12/19] qtest: allow arbitrarily long sends John Snow
@ 2015-05-22 19:59 ` John Snow
2015-05-22 19:59 ` [Qemu-devel] [PULL 14/19] qtest: add memset to qtest protocol John Snow
` (6 subsequent siblings)
19 siblings, 0 replies; 32+ messages in thread
From: John Snow @ 2015-05-22 19:59 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow
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 <jsnow@redhat.com>
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 3738eae..73b7a0f 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;
@@ -432,6 +473,34 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
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) {
int64_t ns;
diff --git a/tests/libqtest.c b/tests/libqtest.c
index a525dc5..5f57005 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 4b54b5d..ec42031 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -301,6 +301,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.
* @addr: Guest address to write to.
@@ -312,6 +323,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.
* @addr: Guest address to write to.
@@ -699,6 +722,19 @@ static inline void memread(uint64_t addr, void *data, size_t 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.
* @data: Pointer to the bytes that will be written to guest memory.
@@ -712,6 +748,19 @@ static inline void memwrite(uint64_t addr, const void *data, size_t 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.
* @patt: Byte pattern to fill the guest memory region with.
--
2.1.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PULL 14/19] qtest: add memset to qtest protocol
2015-05-22 19:59 [Qemu-devel] [PULL 00/19] Ide patches John Snow
` (12 preceding siblings ...)
2015-05-22 19:59 ` [Qemu-devel] [PULL 13/19] qtest: Add base64 encoded read/write John Snow
@ 2015-05-22 19:59 ` John Snow
2015-05-22 19:59 ` [Qemu-devel] [PULL 15/19] libqos/ahci: Swap memread/write with bufread/write John Snow
` (5 subsequent siblings)
19 siblings, 0 replies; 32+ messages in thread
From: John Snow @ 2015-05-22 19:59 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow
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 <jsnow@redhat.com>
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 73b7a0f..04412dd 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
@@ -473,6 +476,23 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
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) {
uint64_t addr, len;
uint8_t *data;
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 5f57005..055aad6 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);
}
--
2.1.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PULL 15/19] libqos/ahci: Swap memread/write with bufread/write
2015-05-22 19:59 [Qemu-devel] [PULL 00/19] Ide patches John Snow
` (13 preceding siblings ...)
2015-05-22 19:59 ` [Qemu-devel] [PULL 14/19] qtest: add memset to qtest protocol John Snow
@ 2015-05-22 19:59 ` John Snow
2015-05-22 19:59 ` [Qemu-devel] [PULL 16/19] qtest: pre-buffer hex nibs John Snow
` (4 subsequent siblings)
19 siblings, 0 replies; 32+ messages in thread
From: John Snow @ 2015-05-22 19:59 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow
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 <jsnow@redhat.com>
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 1a967c3..6e3fa81 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 95bfb3d..7e17bb6 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);
--
2.1.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PULL 16/19] qtest: pre-buffer hex nibs
2015-05-22 19:59 [Qemu-devel] [PULL 00/19] Ide patches John Snow
` (14 preceding siblings ...)
2015-05-22 19:59 ` [Qemu-devel] [PULL 15/19] libqos/ahci: Swap memread/write with bufread/write John Snow
@ 2015-05-22 19:59 ` John Snow
2015-05-22 19:59 ` [Qemu-devel] [PULL 17/19] macio: move unaligned DMA read code into separate pmac_dma_read() function John Snow
` (3 subsequent siblings)
19 siblings, 0 replies; 32+ messages in thread
From: John Snow @ 2015-05-22 19:59 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow
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 <jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
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 04412dd..05cefd2 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 055aad6..e5188e0 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)
--
2.1.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PULL 17/19] macio: move unaligned DMA read code into separate pmac_dma_read() function
2015-05-22 19:59 [Qemu-devel] [PULL 00/19] Ide patches John Snow
` (15 preceding siblings ...)
2015-05-22 19:59 ` [Qemu-devel] [PULL 16/19] qtest: pre-buffer hex nibs John Snow
@ 2015-05-22 19:59 ` John Snow
2015-05-22 19:59 ` [Qemu-devel] [PULL 18/19] macio: move unaligned DMA write code into separate pmac_dma_write() function John Snow
` (2 subsequent siblings)
19 siblings, 0 replies; 32+ messages in thread
From: John Snow @ 2015-05-22 19:59 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Mark Cave-Ayland, jsnow, Mark Cave-Ayland
From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
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 <mark.cave-ayland@ailande.co.uk>
Acked-by: John Snow <jsnow@redhat.com>
Message-id: 1425939893-14404-2-git-send-email-mark.cave-ayland@ilande.co.uk
Signed-off-by: John Snow <jsnow@redhat.com>
---
hw/ide/macio.c | 257 +++++++++++++++++++++++++++++++++------------------------
1 file changed, 149 insertions(+), 108 deletions(-)
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index a009674..037db8b 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -51,18 +51,118 @@ static const int debug_macio = 0;
#define MACIO_PAGE_SIZE 4096
+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);
+ dma_addr_t dma_addr, dma_len;
+ void *mem;
+ int nsector, remainder;
+
+ 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 (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 + 0x1ff) >> 9);
+ remainder = (nsector << 9) - io->len;
+
+ 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);
+
+ 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);
+
+ io->remainder_len = remainder;
+ }
+
+ s->io_buffer_size -= io->len;
+ s->io_buffer_index += io->len;
+
+ io->len = 0;
+
+ MACIO_DPRINTF("--- Block read transfer - sector_num: %"PRIx64" "
+ "nsector: %x\n",
+ sector_num, nsector);
+
+ m->aiocb = blk_aio_readv(blk, sector_num, &io->iov, nsector, cb, io);
+}
+
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);
- int unaligned;
+ 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) {
- m->aiocb = NULL;
- qemu_sglist_destroy(&s->sg);
+ MACIO_DPRINTF("THERE WAS AN ERROR! %d\n", ret);
ide_atapi_io_error(s, ret);
- io->remainder_len = 0;
goto done;
}
@@ -74,105 +174,43 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, int ret)
return;
}
- 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);
-
- s->packet_transfer_size -= s->io_buffer_size;
-
- s->io_buffer_index += s->io_buffer_size;
- s->lba += s->io_buffer_index >> 11;
- s->io_buffer_index &= 0x7ff;
- }
-
- s->io_buffer_size = MIN(io->len, s->packet_transfer_size);
-
- 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);
-
- MACIO_DPRINTF("copying remainder %d bytes\n", remainder_len);
-
- cpu_physical_memory_write(io->addr, io->remainder + 0x200 -
- remainder_len, remainder_len);
-
- 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);
- 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;
--
2.1.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PULL 18/19] macio: move unaligned DMA write code into separate pmac_dma_write() function
2015-05-22 19:59 [Qemu-devel] [PULL 00/19] Ide patches John Snow
` (16 preceding siblings ...)
2015-05-22 19:59 ` [Qemu-devel] [PULL 17/19] macio: move unaligned DMA read code into separate pmac_dma_read() function John Snow
@ 2015-05-22 19:59 ` John Snow
2015-07-27 22:00 ` Aurelien Jarno
2015-05-22 19:59 ` [Qemu-devel] [PULL 19/19] ahci: do not remap clb/fis unconditionally John Snow
2015-05-26 11:54 ` [Qemu-devel] [PULL 00/19] Ide patches Peter Maydell
19 siblings, 1 reply; 32+ messages in thread
From: John Snow @ 2015-05-22 19:59 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Mark Cave-Ayland, jsnow
From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
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 <mark.cave-ayland@ilande.co.uk>
Acked-by: John Snow <jsnow@redhat.com>
Message-id: 1425939893-14404-3-git-send-email-mark.cave-ayland@ilande.co.uk
Signed-off-by: John Snow <jsnow@redhat.com>
---
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 037db8b..585a27b 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 */
+ /* Calculate number of sectors */
+ sector_num = ide_get_sector(s) + (s->io_buffer_index >> 9);
+ nsector = (io->len + 0x1ff) >> 9;
+ remainder = io->len & 0x1ff;
- s->io_buffer_index = 0;
- s->io_buffer_size = MIN(io->len, s->nsector * 512);
+ s->nsector -= nsector;
- /* 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;
- }
-
- io->len = 0;
-
- 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 d7db06c..c580327 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;
};
/*
--
2.1.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PULL 18/19] macio: move unaligned DMA write code into separate pmac_dma_write() function
2015-05-22 19:59 ` [Qemu-devel] [PULL 18/19] macio: move unaligned DMA write code into separate pmac_dma_write() function John Snow
@ 2015-07-27 22:00 ` Aurelien Jarno
2015-07-28 8:52 ` Mark Cave-Ayland
0 siblings, 1 reply; 32+ messages in thread
From: Aurelien Jarno @ 2015-07-27 22:00 UTC (permalink / raw)
To: John Snow; +Cc: peter.maydell, Mark Cave-Ayland, qemu-devel
On 2015-05-22 15:59, John Snow wrote:
> From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>
> 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 <mark.cave-ayland@ilande.co.uk>
> Acked-by: John Snow <jsnow@redhat.com>
> Message-id: 1425939893-14404-3-git-send-email-mark.cave-ayland@ilande.co.uk
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> hw/ide/macio.c | 268 +++++++++++++++++++++------------------------
> include/hw/ppc/mac_dbdma.h | 4 -
> 2 files changed, 125 insertions(+), 147 deletions(-)
This patch has removed TRIM support without any obvious reason, and
without mentioning it in the changelog. As a consequence guests with
TRIM enabled now fail to boot:
| [ 46.916047] ata1.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x0
| [ 46.916545] ata1.00: failed command: DATA SET MANAGEMENT
| [ 46.916794] ata1.00: cmd 06/01:01:00:00:00/00:00:00:00:00/a0 tag 0 dma 512 out
| [ 46.916794] res 40/00:01:00:00:00/00:00:00:00:00/e0 Emask 0x20 (host bus error)
| [ 46.917219] ata1.00: status: { DRDY }
| [ 51.957389] ata1.00: qc timeout (cmd 0xec)
| [ 51.958076] ata1.00: failed to IDENTIFY (I/O error, err_mask=0x4)
| [ 51.958551] ata1.00: revalidation failed (errno=-5)
| [ 56.996713] ata1: link is slow to respond, please be patient (ready=0)
| [ 61.981042] ata1: device not ready (errno=-16), forcing hardreset
| [ 61.981669] ata1: soft resetting link
| [ 62.137894] ata1.00: configured for MWDMA2
| [ 62.138294] ata1.00: device reported invalid CHS sector 0
| [ 62.139045] sd 0:0:0:0: [sda]
| [ 62.139128] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
| [ 62.139243] sd 0:0:0:0: [sda]
| [ 62.139346] Sense Key : Aborted Command [current] [descriptor]
| [ 62.139581] Descriptor sense data with sense descriptors (in hex):
| [ 62.139670] 72 0b 00 00 00 00 00 0c 00 0a 80 00 00 00 00 00
| [ 62.139812] 00 00 00 00
| [ 62.139897] sd 0:0:0:0: [sda]
| [ 62.140009] Add. Sense: No additional sense information
| [ 62.140115] sd 0:0:0:0: [sda] CDB:
| [ 62.140204] Write same(16): 93 08 00 00 00 00 03 c0 00 48 00 3f ff b8 00 00
| [ 62.140661] end_request: I/O error, dev sda, sector 62914632
| [ 62.141270] ata1: EH complete
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PULL 18/19] macio: move unaligned DMA write code into separate pmac_dma_write() function
2015-07-27 22:00 ` Aurelien Jarno
@ 2015-07-28 8:52 ` Mark Cave-Ayland
2015-07-28 18:02 ` Aurelien Jarno
` (2 more replies)
0 siblings, 3 replies; 32+ messages in thread
From: Mark Cave-Ayland @ 2015-07-28 8:52 UTC (permalink / raw)
To: Aurelien Jarno, John Snow; +Cc: peter.maydell, qemu-devel
On 27/07/15 23:00, Aurelien Jarno wrote:
> On 2015-05-22 15:59, John Snow wrote:
>> From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>
>> 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 <mark.cave-ayland@ilande.co.uk>
>> Acked-by: John Snow <jsnow@redhat.com>
>> Message-id: 1425939893-14404-3-git-send-email-mark.cave-ayland@ilande.co.uk
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>> hw/ide/macio.c | 268 +++++++++++++++++++++------------------------
>> include/hw/ppc/mac_dbdma.h | 4 -
>> 2 files changed, 125 insertions(+), 147 deletions(-)
>
> This patch has removed TRIM support without any obvious reason, and
> without mentioning it in the changelog. As a consequence guests with
> TRIM enabled now fail to boot:
>
> | [ 46.916047] ata1.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x0
> | [ 46.916545] ata1.00: failed command: DATA SET MANAGEMENT
> | [ 46.916794] ata1.00: cmd 06/01:01:00:00:00/00:00:00:00:00/a0 tag 0 dma 512 out
> | [ 46.916794] res 40/00:01:00:00:00/00:00:00:00:00/e0 Emask 0x20 (host bus error)
> | [ 46.917219] ata1.00: status: { DRDY }
> | [ 51.957389] ata1.00: qc timeout (cmd 0xec)
> | [ 51.958076] ata1.00: failed to IDENTIFY (I/O error, err_mask=0x4)
> | [ 51.958551] ata1.00: revalidation failed (errno=-5)
> | [ 56.996713] ata1: link is slow to respond, please be patient (ready=0)
> | [ 61.981042] ata1: device not ready (errno=-16), forcing hardreset
> | [ 61.981669] ata1: soft resetting link
> | [ 62.137894] ata1.00: configured for MWDMA2
> | [ 62.138294] ata1.00: device reported invalid CHS sector 0
> | [ 62.139045] sd 0:0:0:0: [sda]
> | [ 62.139128] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
> | [ 62.139243] sd 0:0:0:0: [sda]
> | [ 62.139346] Sense Key : Aborted Command [current] [descriptor]
> | [ 62.139581] Descriptor sense data with sense descriptors (in hex):
> | [ 62.139670] 72 0b 00 00 00 00 00 0c 00 0a 80 00 00 00 00 00
> | [ 62.139812] 00 00 00 00
> | [ 62.139897] sd 0:0:0:0: [sda]
> | [ 62.140009] Add. Sense: No additional sense information
> | [ 62.140115] sd 0:0:0:0: [sda] CDB:
> | [ 62.140204] Write same(16): 93 08 00 00 00 00 03 c0 00 48 00 3f ff b8 00 00
> | [ 62.140661] end_request: I/O error, dev sda, sector 62914632
> | [ 62.141270] ata1: EH complete
Hi Aurelien,
Thanks for the heads-up. I have a fairly comprehensive suite of various
OS test images I use for OpenBIOS testing and evidently not a single one
of them issues a TRIM command as I don't see any regressions here. Can
you point me towards the particular test image you are using?
ATB,
Mark.
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PULL 18/19] macio: move unaligned DMA write code into separate pmac_dma_write() function
2015-07-28 8:52 ` Mark Cave-Ayland
@ 2015-07-28 18:02 ` Aurelien Jarno
2015-08-01 11:21 ` Mark Cave-Ayland
2015-07-30 10:10 ` Kevin Wolf
2015-07-31 20:37 ` John Snow
2 siblings, 1 reply; 32+ messages in thread
From: Aurelien Jarno @ 2015-07-28 18:02 UTC (permalink / raw)
To: Mark Cave-Ayland; +Cc: peter.maydell, John Snow, qemu-devel
On 2015-07-28 09:52, Mark Cave-Ayland wrote:
> On 27/07/15 23:00, Aurelien Jarno wrote:
>
> > On 2015-05-22 15:59, John Snow wrote:
> >> From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> >>
> >> 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 <mark.cave-ayland@ilande.co.uk>
> >> Acked-by: John Snow <jsnow@redhat.com>
> >> Message-id: 1425939893-14404-3-git-send-email-mark.cave-ayland@ilande.co.uk
> >> Signed-off-by: John Snow <jsnow@redhat.com>
> >> ---
> >> hw/ide/macio.c | 268 +++++++++++++++++++++------------------------
> >> include/hw/ppc/mac_dbdma.h | 4 -
> >> 2 files changed, 125 insertions(+), 147 deletions(-)
> >
> > This patch has removed TRIM support without any obvious reason, and
> > without mentioning it in the changelog. As a consequence guests with
> > TRIM enabled now fail to boot:
> >
> > | [ 46.916047] ata1.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x0
> > | [ 46.916545] ata1.00: failed command: DATA SET MANAGEMENT
> > | [ 46.916794] ata1.00: cmd 06/01:01:00:00:00/00:00:00:00:00/a0 tag 0 dma 512 out
> > | [ 46.916794] res 40/00:01:00:00:00/00:00:00:00:00/e0 Emask 0x20 (host bus error)
> > | [ 46.917219] ata1.00: status: { DRDY }
> > | [ 51.957389] ata1.00: qc timeout (cmd 0xec)
> > | [ 51.958076] ata1.00: failed to IDENTIFY (I/O error, err_mask=0x4)
> > | [ 51.958551] ata1.00: revalidation failed (errno=-5)
> > | [ 56.996713] ata1: link is slow to respond, please be patient (ready=0)
> > | [ 61.981042] ata1: device not ready (errno=-16), forcing hardreset
> > | [ 61.981669] ata1: soft resetting link
> > | [ 62.137894] ata1.00: configured for MWDMA2
> > | [ 62.138294] ata1.00: device reported invalid CHS sector 0
> > | [ 62.139045] sd 0:0:0:0: [sda]
> > | [ 62.139128] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
> > | [ 62.139243] sd 0:0:0:0: [sda]
> > | [ 62.139346] Sense Key : Aborted Command [current] [descriptor]
> > | [ 62.139581] Descriptor sense data with sense descriptors (in hex):
> > | [ 62.139670] 72 0b 00 00 00 00 00 0c 00 0a 80 00 00 00 00 00
> > | [ 62.139812] 00 00 00 00
> > | [ 62.139897] sd 0:0:0:0: [sda]
> > | [ 62.140009] Add. Sense: No additional sense information
> > | [ 62.140115] sd 0:0:0:0: [sda] CDB:
> > | [ 62.140204] Write same(16): 93 08 00 00 00 00 03 c0 00 48 00 3f ff b8 00 00
> > | [ 62.140661] end_request: I/O error, dev sda, sector 62914632
> > | [ 62.141270] ata1: EH complete
>
> Hi Aurelien,
>
> Thanks for the heads-up. I have a fairly comprehensive suite of various
> OS test images I use for OpenBIOS testing and evidently not a single one
> of them issues a TRIM command as I don't see any regressions here. Can
> you point me towards the particular test image you are using?
You need a Linux based image, with a relatively recent kernel (3.2 is
ok). The disk image has to be created with -drive file=...,discard=unmap
for the TRIM feature to be advertised in the guest. Finally you need to
run the command "fstrim /" or mount a partition or the swap with the
option discard.
As a summary:
- wget https://people.debian.org/~aurel32/qemu/powerpc/debian_wheezy_powerpc_standard.qcow2
- qemu-system-ppc -snapshot -drive file=debian_wheezy_powerpc_standard.qcow2,discard=unmap
- in the image, login as root (password root) and run fstrim /.
I have looked a bit at the code, and it looks like we have to provide a
function similar to pmac_dma_read/write for the TRIM. I'll look at it a
bit more in details and I will try to provide a patch, but it's probably
won't be before the week-end.
Aurelien
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PULL 18/19] macio: move unaligned DMA write code into separate pmac_dma_write() function
2015-07-28 18:02 ` Aurelien Jarno
@ 2015-08-01 11:21 ` Mark Cave-Ayland
2015-08-01 12:37 ` Aurelien Jarno
0 siblings, 1 reply; 32+ messages in thread
From: Mark Cave-Ayland @ 2015-08-01 11:21 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: peter.maydell, John Snow, qemu-devel
On 28/07/15 19:02, Aurelien Jarno wrote:
>> Thanks for the heads-up. I have a fairly comprehensive suite of various
>> OS test images I use for OpenBIOS testing and evidently not a single one
>> of them issues a TRIM command as I don't see any regressions here. Can
>> you point me towards the particular test image you are using?
>
> You need a Linux based image, with a relatively recent kernel (3.2 is
> ok). The disk image has to be created with -drive file=...,discard=unmap
> for the TRIM feature to be advertised in the guest. Finally you need to
> run the command "fstrim /" or mount a partition or the swap with the
> option discard.
>
> As a summary:
> - wget https://people.debian.org/~aurel32/qemu/powerpc/debian_wheezy_powerpc_standard.qcow2
> - qemu-system-ppc -snapshot -drive file=debian_wheezy_powerpc_standard.qcow2,discard=unmap
> - in the image, login as root (password root) and run fstrim /.
>
> I have looked a bit at the code, and it looks like we have to provide a
> function similar to pmac_dma_read/write for the TRIM. I'll look at it a
> bit more in details and I will try to provide a patch, but it's probably
> won't be before the week-end.
I should add that regardless of whether the TRIM patch is applied or
not, the wheezy image reports DMA timeout errors which I don't see in
the squeeze or lenny images:
[ 5.420949] ata2.00: configured for MWDMA2
[ 5.451070] rtc-generic rtc-generic: setting system clock to
2015-08-01 11:13:25 UTC (1438427605)
[ 5.461100] Initializing network drop monitor service
[ 5.480460] scsi 1:0:0:0: CD-ROM QEMU QEMU DVD-ROM
2.3. PQ: 0 ANSI: 5
[ 5.540013] Freeing unused kernel memory: 244k freed
[ 6.677779] udevd[46]: starting version 175
[ 8.548044] ne2k-pci.c:v1.03 9/22/2003 D. Becker/P. Gortmaker
[ 8.761179] eth0: RealTek RTL-8029 found at 0x400, IRQ 23,
52:54:00:12:34:56.
[ 10.300003] sd 0:0:0:0: [sda] 52428800 512-byte logical blocks: (26.8
GB/25.0 GiB)
[ 10.359219] pata-macio 0.00021000:ata-4: timeout flushing DMA
[ 10.400935] ata2.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x0
[ 10.412768] ata2.00: BMDMA stat 0x6
[ 10.421732] sr 1:0:0:0: CDB: Mode Sense(10): 5a 00 2a 00 00 00 00 00
80 00
[ 10.430301] ata2.00: cmd a0/01:00:00:80:00/00:00:00:00:00/a0 tag 0
dma 16512 in
[ 10.430334] res 50/00:03:00:80:00/00:00:00:00:00/a0 Emask
0x20 (host bus error)
[ 10.459348] ata2.00: status: { DRDY }
[ 10.516892] sd 0:0:0:0: [sda] Write Protect is off
[ 10.524614] sd 0:0:0:0: [sda] Mode Sense: 00 3a 00 00
[ 10.545353] sd 0:0:0:0: [sda] Write cache: enabled, read cache:
enabled, doesn't support DPO or FUA
[ 10.655885] ata2.00: configured for MWDMA2
[ 10.665041] ata2: EH complete
and:
[ 14.016326] PM: Hibernation image partition 8:4 present
[ 14.016380] PM: Looking for hibernation image.
[ 14.020555] PM: Image not found (code -22)
[ 14.020642] PM: Hibernation image not present or could not be loaded.
[ 14.513667] EXT4-fs (sda3): mounted filesystem with ordered data
mode. Opts: (null)
[ 24.750778] udevd[252]: starting version 175
[ 31.040517] pata-macio 0.00021000:ata-4: timeout flushing DMA
[ 31.528077] ata2.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x0
[ 31.546115] ata2.00: BMDMA stat 0x6
[ 31.562905] sr 1:0:0:0: [sr0] CDB: Get configuration: 46 00 00 00 00
00 00 00 10 00
[ 31.572557] ata2.00: cmd a0/01:00:00:10:00/00:00:00:00:00/a0 tag 0
dma 16400 in
[ 31.572589] res 50/00:03:00:10:00/00:00:00:00:00/a0 Emask
0x20 (host bus error)
[ 31.597940] ata2.00: status: { DRDY }
[ 32.139342] ata2.00: configured for MWDMA2
[ 32.148769] ata2: EH complete
I'll have a quick look with debugging enabled to see if I can spot
anything obvious.
ATB,
Mark.
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PULL 18/19] macio: move unaligned DMA write code into separate pmac_dma_write() function
2015-08-01 11:21 ` Mark Cave-Ayland
@ 2015-08-01 12:37 ` Aurelien Jarno
2015-08-01 16:09 ` Mark Cave-Ayland
0 siblings, 1 reply; 32+ messages in thread
From: Aurelien Jarno @ 2015-08-01 12:37 UTC (permalink / raw)
To: Mark Cave-Ayland; +Cc: peter.maydell, John Snow, qemu-devel
On 2015-08-01 12:21, Mark Cave-Ayland wrote:
> On 28/07/15 19:02, Aurelien Jarno wrote:
>
> >> Thanks for the heads-up. I have a fairly comprehensive suite of various
> >> OS test images I use for OpenBIOS testing and evidently not a single one
> >> of them issues a TRIM command as I don't see any regressions here. Can
> >> you point me towards the particular test image you are using?
> >
> > You need a Linux based image, with a relatively recent kernel (3.2 is
> > ok). The disk image has to be created with -drive file=...,discard=unmap
> > for the TRIM feature to be advertised in the guest. Finally you need to
> > run the command "fstrim /" or mount a partition or the swap with the
> > option discard.
> >
> > As a summary:
> > - wget https://people.debian.org/~aurel32/qemu/powerpc/debian_wheezy_powerpc_standard.qcow2
> > - qemu-system-ppc -snapshot -drive file=debian_wheezy_powerpc_standard.qcow2,discard=unmap
> > - in the image, login as root (password root) and run fstrim /.
> >
> > I have looked a bit at the code, and it looks like we have to provide a
> > function similar to pmac_dma_read/write for the TRIM. I'll look at it a
> > bit more in details and I will try to provide a patch, but it's probably
> > won't be before the week-end.
>
> I should add that regardless of whether the TRIM patch is applied or
> not, the wheezy image reports DMA timeout errors which I don't see in
> the squeeze or lenny images:
>
>
> [ 5.420949] ata2.00: configured for MWDMA2
> [ 5.451070] rtc-generic rtc-generic: setting system clock to
> 2015-08-01 11:13:25 UTC (1438427605)
> [ 5.461100] Initializing network drop monitor service
> [ 5.480460] scsi 1:0:0:0: CD-ROM QEMU QEMU DVD-ROM
> 2.3. PQ: 0 ANSI: 5
> [ 5.540013] Freeing unused kernel memory: 244k freed
> [ 6.677779] udevd[46]: starting version 175
> [ 8.548044] ne2k-pci.c:v1.03 9/22/2003 D. Becker/P. Gortmaker
> [ 8.761179] eth0: RealTek RTL-8029 found at 0x400, IRQ 23,
> 52:54:00:12:34:56.
> [ 10.300003] sd 0:0:0:0: [sda] 52428800 512-byte logical blocks: (26.8
> GB/25.0 GiB)
> [ 10.359219] pata-macio 0.00021000:ata-4: timeout flushing DMA
> [ 10.400935] ata2.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x0
> [ 10.412768] ata2.00: BMDMA stat 0x6
> [ 10.421732] sr 1:0:0:0: CDB: Mode Sense(10): 5a 00 2a 00 00 00 00 00
> 80 00
> [ 10.430301] ata2.00: cmd a0/01:00:00:80:00/00:00:00:00:00/a0 tag 0
> dma 16512 in
> [ 10.430334] res 50/00:03:00:80:00/00:00:00:00:00/a0 Emask
> 0x20 (host bus error)
> [ 10.459348] ata2.00: status: { DRDY }
> [ 10.516892] sd 0:0:0:0: [sda] Write Protect is off
> [ 10.524614] sd 0:0:0:0: [sda] Mode Sense: 00 3a 00 00
> [ 10.545353] sd 0:0:0:0: [sda] Write cache: enabled, read cache:
> enabled, doesn't support DPO or FUA
> [ 10.655885] ata2.00: configured for MWDMA2
> [ 10.665041] ata2: EH complete
>
> and:
>
> [ 14.016326] PM: Hibernation image partition 8:4 present
> [ 14.016380] PM: Looking for hibernation image.
> [ 14.020555] PM: Image not found (code -22)
> [ 14.020642] PM: Hibernation image not present or could not be loaded.
> [ 14.513667] EXT4-fs (sda3): mounted filesystem with ordered data
> mode. Opts: (null)
> [ 24.750778] udevd[252]: starting version 175
> [ 31.040517] pata-macio 0.00021000:ata-4: timeout flushing DMA
> [ 31.528077] ata2.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x0
> [ 31.546115] ata2.00: BMDMA stat 0x6
> [ 31.562905] sr 1:0:0:0: [sr0] CDB: Get configuration: 46 00 00 00 00
> 00 00 00 10 00
> [ 31.572557] ata2.00: cmd a0/01:00:00:10:00/00:00:00:00:00/a0 tag 0
> dma 16400 in
> [ 31.572589] res 50/00:03:00:10:00/00:00:00:00:00/a0 Emask
> 0x20 (host bus error)
> [ 31.597940] ata2.00: status: { DRDY }
> [ 32.139342] ata2.00: configured for MWDMA2
> [ 32.148769] ata2: EH complete
>
>
> I'll have a quick look with debugging enabled to see if I can spot
> anything obvious.
ata2.00 is actually the CD-ROM drive. I have the same issue there,
except here, and there are even more messages when mounting the drive.
That said, it seems to work correctly anyway.
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PULL 18/19] macio: move unaligned DMA write code into separate pmac_dma_write() function
2015-08-01 12:37 ` Aurelien Jarno
@ 2015-08-01 16:09 ` Mark Cave-Ayland
0 siblings, 0 replies; 32+ messages in thread
From: Mark Cave-Ayland @ 2015-08-01 16:09 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: peter.maydell, John Snow, qemu-devel
On 01/08/15 13:37, Aurelien Jarno wrote:
> On 2015-08-01 12:21, Mark Cave-Ayland wrote:
>> On 28/07/15 19:02, Aurelien Jarno wrote:
>>
>>>> Thanks for the heads-up. I have a fairly comprehensive suite of various
>>>> OS test images I use for OpenBIOS testing and evidently not a single one
>>>> of them issues a TRIM command as I don't see any regressions here. Can
>>>> you point me towards the particular test image you are using?
>>>
>>> You need a Linux based image, with a relatively recent kernel (3.2 is
>>> ok). The disk image has to be created with -drive file=...,discard=unmap
>>> for the TRIM feature to be advertised in the guest. Finally you need to
>>> run the command "fstrim /" or mount a partition or the swap with the
>>> option discard.
>>>
>>> As a summary:
>>> - wget https://people.debian.org/~aurel32/qemu/powerpc/debian_wheezy_powerpc_standard.qcow2
>>> - qemu-system-ppc -snapshot -drive file=debian_wheezy_powerpc_standard.qcow2,discard=unmap
>>> - in the image, login as root (password root) and run fstrim /.
>>>
>>> I have looked a bit at the code, and it looks like we have to provide a
>>> function similar to pmac_dma_read/write for the TRIM. I'll look at it a
>>> bit more in details and I will try to provide a patch, but it's probably
>>> won't be before the week-end.
>>
>> I should add that regardless of whether the TRIM patch is applied or
>> not, the wheezy image reports DMA timeout errors which I don't see in
>> the squeeze or lenny images:
>>
>>
>> [ 5.420949] ata2.00: configured for MWDMA2
>> [ 5.451070] rtc-generic rtc-generic: setting system clock to
>> 2015-08-01 11:13:25 UTC (1438427605)
>> [ 5.461100] Initializing network drop monitor service
>> [ 5.480460] scsi 1:0:0:0: CD-ROM QEMU QEMU DVD-ROM
>> 2.3. PQ: 0 ANSI: 5
>> [ 5.540013] Freeing unused kernel memory: 244k freed
>> [ 6.677779] udevd[46]: starting version 175
>> [ 8.548044] ne2k-pci.c:v1.03 9/22/2003 D. Becker/P. Gortmaker
>> [ 8.761179] eth0: RealTek RTL-8029 found at 0x400, IRQ 23,
>> 52:54:00:12:34:56.
>> [ 10.300003] sd 0:0:0:0: [sda] 52428800 512-byte logical blocks: (26.8
>> GB/25.0 GiB)
>> [ 10.359219] pata-macio 0.00021000:ata-4: timeout flushing DMA
>> [ 10.400935] ata2.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x0
>> [ 10.412768] ata2.00: BMDMA stat 0x6
>> [ 10.421732] sr 1:0:0:0: CDB: Mode Sense(10): 5a 00 2a 00 00 00 00 00
>> 80 00
>> [ 10.430301] ata2.00: cmd a0/01:00:00:80:00/00:00:00:00:00/a0 tag 0
>> dma 16512 in
>> [ 10.430334] res 50/00:03:00:80:00/00:00:00:00:00/a0 Emask
>> 0x20 (host bus error)
>> [ 10.459348] ata2.00: status: { DRDY }
>> [ 10.516892] sd 0:0:0:0: [sda] Write Protect is off
>> [ 10.524614] sd 0:0:0:0: [sda] Mode Sense: 00 3a 00 00
>> [ 10.545353] sd 0:0:0:0: [sda] Write cache: enabled, read cache:
>> enabled, doesn't support DPO or FUA
>> [ 10.655885] ata2.00: configured for MWDMA2
>> [ 10.665041] ata2: EH complete
>>
>> and:
>>
>> [ 14.016326] PM: Hibernation image partition 8:4 present
>> [ 14.016380] PM: Looking for hibernation image.
>> [ 14.020555] PM: Image not found (code -22)
>> [ 14.020642] PM: Hibernation image not present or could not be loaded.
>> [ 14.513667] EXT4-fs (sda3): mounted filesystem with ordered data
>> mode. Opts: (null)
>> [ 24.750778] udevd[252]: starting version 175
>> [ 31.040517] pata-macio 0.00021000:ata-4: timeout flushing DMA
>> [ 31.528077] ata2.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x0
>> [ 31.546115] ata2.00: BMDMA stat 0x6
>> [ 31.562905] sr 1:0:0:0: [sr0] CDB: Get configuration: 46 00 00 00 00
>> 00 00 00 10 00
>> [ 31.572557] ata2.00: cmd a0/01:00:00:10:00/00:00:00:00:00/a0 tag 0
>> dma 16400 in
>> [ 31.572589] res 50/00:03:00:10:00/00:00:00:00:00/a0 Emask
>> 0x20 (host bus error)
>> [ 31.597940] ata2.00: status: { DRDY }
>> [ 32.139342] ata2.00: configured for MWDMA2
>> [ 32.148769] ata2: EH complete
>>
>>
>> I'll have a quick look with debugging enabled to see if I can spot
>> anything obvious.
>
> ata2.00 is actually the CD-ROM drive. I have the same issue there,
> except here, and there are even more messages when mounting the drive.
> That said, it seems to work correctly anyway.
I've worked out what is going on here (it's the non-block transfers) and
have a fix that looks good on my test images here. John/Aurelien, please
can you test on your setups to make sure that everything still works for
you?
ATB,
Mark.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PULL 18/19] macio: move unaligned DMA write code into separate pmac_dma_write() function
2015-07-28 8:52 ` Mark Cave-Ayland
2015-07-28 18:02 ` Aurelien Jarno
@ 2015-07-30 10:10 ` Kevin Wolf
2015-07-31 20:37 ` John Snow
2 siblings, 0 replies; 32+ messages in thread
From: Kevin Wolf @ 2015-07-30 10:10 UTC (permalink / raw)
To: Mark Cave-Ayland; +Cc: peter.maydell, John Snow, qemu-devel, Aurelien Jarno
Am 28.07.2015 um 10:52 hat Mark Cave-Ayland geschrieben:
> On 27/07/15 23:00, Aurelien Jarno wrote:
>
> > On 2015-05-22 15:59, John Snow wrote:
> >> From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> >>
> >> 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 <mark.cave-ayland@ilande.co.uk>
> >> Acked-by: John Snow <jsnow@redhat.com>
> >> Message-id: 1425939893-14404-3-git-send-email-mark.cave-ayland@ilande.co.uk
> >> Signed-off-by: John Snow <jsnow@redhat.com>
> >> ---
> >> hw/ide/macio.c | 268 +++++++++++++++++++++------------------------
> >> include/hw/ppc/mac_dbdma.h | 4 -
> >> 2 files changed, 125 insertions(+), 147 deletions(-)
> >
> > This patch has removed TRIM support without any obvious reason, and
> > without mentioning it in the changelog. As a consequence guests with
> > TRIM enabled now fail to boot:
> >
> > | [ 46.916047] ata1.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x0
> > | [ 46.916545] ata1.00: failed command: DATA SET MANAGEMENT
> > | [ 46.916794] ata1.00: cmd 06/01:01:00:00:00/00:00:00:00:00/a0 tag 0 dma 512 out
> > | [ 46.916794] res 40/00:01:00:00:00/00:00:00:00:00/e0 Emask 0x20 (host bus error)
> > | [ 46.917219] ata1.00: status: { DRDY }
> > | [ 51.957389] ata1.00: qc timeout (cmd 0xec)
> > | [ 51.958076] ata1.00: failed to IDENTIFY (I/O error, err_mask=0x4)
> > | [ 51.958551] ata1.00: revalidation failed (errno=-5)
> > | [ 56.996713] ata1: link is slow to respond, please be patient (ready=0)
> > | [ 61.981042] ata1: device not ready (errno=-16), forcing hardreset
> > | [ 61.981669] ata1: soft resetting link
> > | [ 62.137894] ata1.00: configured for MWDMA2
> > | [ 62.138294] ata1.00: device reported invalid CHS sector 0
> > | [ 62.139045] sd 0:0:0:0: [sda]
> > | [ 62.139128] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
> > | [ 62.139243] sd 0:0:0:0: [sda]
> > | [ 62.139346] Sense Key : Aborted Command [current] [descriptor]
> > | [ 62.139581] Descriptor sense data with sense descriptors (in hex):
> > | [ 62.139670] 72 0b 00 00 00 00 00 0c 00 0a 80 00 00 00 00 00
> > | [ 62.139812] 00 00 00 00
> > | [ 62.139897] sd 0:0:0:0: [sda]
> > | [ 62.140009] Add. Sense: No additional sense information
> > | [ 62.140115] sd 0:0:0:0: [sda] CDB:
> > | [ 62.140204] Write same(16): 93 08 00 00 00 00 03 c0 00 48 00 3f ff b8 00 00
> > | [ 62.140661] end_request: I/O error, dev sda, sector 62914632
> > | [ 62.141270] ata1: EH complete
>
> Hi Aurelien,
>
> Thanks for the heads-up. I have a fairly comprehensive suite of various
> OS test images I use for OpenBIOS testing and evidently not a single one
> of them issues a TRIM command as I don't see any regressions here. Can
> you point me towards the particular test image you are using?
It would probably be useful to add qtest cases for this controller so
that you don't have to run all of these test images. Generally, I won't
run your OS test images, but it's unlikely that anything covered by
qtest breaks accidentally as all subsystem maintainers routinely run it.
Kevin
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PULL 18/19] macio: move unaligned DMA write code into separate pmac_dma_write() function
2015-07-28 8:52 ` Mark Cave-Ayland
2015-07-28 18:02 ` Aurelien Jarno
2015-07-30 10:10 ` Kevin Wolf
@ 2015-07-31 20:37 ` John Snow
2015-08-02 13:02 ` Mark Cave-Ayland
2 siblings, 1 reply; 32+ messages in thread
From: John Snow @ 2015-07-31 20:37 UTC (permalink / raw)
To: Mark Cave-Ayland, Aurelien Jarno; +Cc: peter.maydell, qemu-devel
On 07/28/2015 04:52 AM, Mark Cave-Ayland wrote:
> On 27/07/15 23:00, Aurelien Jarno wrote:
>
>> On 2015-05-22 15:59, John Snow wrote:
>>> From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>
>>> 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 <mark.cave-ayland@ilande.co.uk>
>>> Acked-by: John Snow <jsnow@redhat.com>
>>> Message-id: 1425939893-14404-3-git-send-email-mark.cave-ayland@ilande.co.uk
>>> Signed-off-by: John Snow <jsnow@redhat.com>
>>> ---
>>> hw/ide/macio.c | 268 +++++++++++++++++++++------------------------
>>> include/hw/ppc/mac_dbdma.h | 4 -
>>> 2 files changed, 125 insertions(+), 147 deletions(-)
>>
>> This patch has removed TRIM support without any obvious reason, and
>> without mentioning it in the changelog. As a consequence guests with
>> TRIM enabled now fail to boot:
>>
>> | [ 46.916047] ata1.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x0
>> | [ 46.916545] ata1.00: failed command: DATA SET MANAGEMENT
>> | [ 46.916794] ata1.00: cmd 06/01:01:00:00:00/00:00:00:00:00/a0 tag 0 dma 512 out
>> | [ 46.916794] res 40/00:01:00:00:00/00:00:00:00:00/e0 Emask 0x20 (host bus error)
>> | [ 46.917219] ata1.00: status: { DRDY }
>> | [ 51.957389] ata1.00: qc timeout (cmd 0xec)
>> | [ 51.958076] ata1.00: failed to IDENTIFY (I/O error, err_mask=0x4)
>> | [ 51.958551] ata1.00: revalidation failed (errno=-5)
>> | [ 56.996713] ata1: link is slow to respond, please be patient (ready=0)
>> | [ 61.981042] ata1: device not ready (errno=-16), forcing hardreset
>> | [ 61.981669] ata1: soft resetting link
>> | [ 62.137894] ata1.00: configured for MWDMA2
>> | [ 62.138294] ata1.00: device reported invalid CHS sector 0
>> | [ 62.139045] sd 0:0:0:0: [sda]
>> | [ 62.139128] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
>> | [ 62.139243] sd 0:0:0:0: [sda]
>> | [ 62.139346] Sense Key : Aborted Command [current] [descriptor]
>> | [ 62.139581] Descriptor sense data with sense descriptors (in hex):
>> | [ 62.139670] 72 0b 00 00 00 00 00 0c 00 0a 80 00 00 00 00 00
>> | [ 62.139812] 00 00 00 00
>> | [ 62.139897] sd 0:0:0:0: [sda]
>> | [ 62.140009] Add. Sense: No additional sense information
>> | [ 62.140115] sd 0:0:0:0: [sda] CDB:
>> | [ 62.140204] Write same(16): 93 08 00 00 00 00 03 c0 00 48 00 3f ff b8 00 00
>> | [ 62.140661] end_request: I/O error, dev sda, sector 62914632
>> | [ 62.141270] ata1: EH complete
>
> Hi Aurelien,
>
> Thanks for the heads-up. I have a fairly comprehensive suite of various
> OS test images I use for OpenBIOS testing and evidently not a single one
> of them issues a TRIM command as I don't see any regressions here. Can
> you point me towards the particular test image you are using?
>
>
> ATB,
>
> Mark.
>
Hi Mark:
This series also exposes to me (unfortunately) the fact that we aren't
unmapping the memory space we're picking up for the DMA.
It wouldn't be too hard to unmap in the pmac_ide_transfer_cb with
something like ...
dma_memory_unmap(&address_space_memory, XXXX, io->len, s->dma_cmd ==
IDE_DMA_READ ? DMA_DIRECTION_FROM_DEVICE : DMA_DIRECTION_TO_DEVICE, io->len)
As the XXXX is the dead giveaway, though, we've actually leaked the
pointer -- we've given it to qemu_iovec_add, but I don't think there's a
way to get it back, so we'll need to stash it /somewhere/.
In DBDMA_io ?
--js
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PULL 18/19] macio: move unaligned DMA write code into separate pmac_dma_write() function
2015-07-31 20:37 ` John Snow
@ 2015-08-02 13:02 ` Mark Cave-Ayland
0 siblings, 0 replies; 32+ messages in thread
From: Mark Cave-Ayland @ 2015-08-02 13:02 UTC (permalink / raw)
To: John Snow, Aurelien Jarno; +Cc: peter.maydell, qemu-devel
On 31/07/15 21:37, John Snow wrote:
> On 07/28/2015 04:52 AM, Mark Cave-Ayland wrote:
>> On 27/07/15 23:00, Aurelien Jarno wrote:
>>
>>> On 2015-05-22 15:59, John Snow wrote:
>>>> From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>>
>>>> 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 <mark.cave-ayland@ilande.co.uk>
>>>> Acked-by: John Snow <jsnow@redhat.com>
>>>> Message-id: 1425939893-14404-3-git-send-email-mark.cave-ayland@ilande.co.uk
>>>> Signed-off-by: John Snow <jsnow@redhat.com>
>>>> ---
>>>> hw/ide/macio.c | 268 +++++++++++++++++++++------------------------
>>>> include/hw/ppc/mac_dbdma.h | 4 -
>>>> 2 files changed, 125 insertions(+), 147 deletions(-)
>>>
>>> This patch has removed TRIM support without any obvious reason, and
>>> without mentioning it in the changelog. As a consequence guests with
>>> TRIM enabled now fail to boot:
>>>
>>> | [ 46.916047] ata1.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x0
>>> | [ 46.916545] ata1.00: failed command: DATA SET MANAGEMENT
>>> | [ 46.916794] ata1.00: cmd 06/01:01:00:00:00/00:00:00:00:00/a0 tag 0 dma 512 out
>>> | [ 46.916794] res 40/00:01:00:00:00/00:00:00:00:00/e0 Emask 0x20 (host bus error)
>>> | [ 46.917219] ata1.00: status: { DRDY }
>>> | [ 51.957389] ata1.00: qc timeout (cmd 0xec)
>>> | [ 51.958076] ata1.00: failed to IDENTIFY (I/O error, err_mask=0x4)
>>> | [ 51.958551] ata1.00: revalidation failed (errno=-5)
>>> | [ 56.996713] ata1: link is slow to respond, please be patient (ready=0)
>>> | [ 61.981042] ata1: device not ready (errno=-16), forcing hardreset
>>> | [ 61.981669] ata1: soft resetting link
>>> | [ 62.137894] ata1.00: configured for MWDMA2
>>> | [ 62.138294] ata1.00: device reported invalid CHS sector 0
>>> | [ 62.139045] sd 0:0:0:0: [sda]
>>> | [ 62.139128] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
>>> | [ 62.139243] sd 0:0:0:0: [sda]
>>> | [ 62.139346] Sense Key : Aborted Command [current] [descriptor]
>>> | [ 62.139581] Descriptor sense data with sense descriptors (in hex):
>>> | [ 62.139670] 72 0b 00 00 00 00 00 0c 00 0a 80 00 00 00 00 00
>>> | [ 62.139812] 00 00 00 00
>>> | [ 62.139897] sd 0:0:0:0: [sda]
>>> | [ 62.140009] Add. Sense: No additional sense information
>>> | [ 62.140115] sd 0:0:0:0: [sda] CDB:
>>> | [ 62.140204] Write same(16): 93 08 00 00 00 00 03 c0 00 48 00 3f ff b8 00 00
>>> | [ 62.140661] end_request: I/O error, dev sda, sector 62914632
>>> | [ 62.141270] ata1: EH complete
>>
>> Hi Aurelien,
>>
>> Thanks for the heads-up. I have a fairly comprehensive suite of various
>> OS test images I use for OpenBIOS testing and evidently not a single one
>> of them issues a TRIM command as I don't see any regressions here. Can
>> you point me towards the particular test image you are using?
>>
>>
>> ATB,
>>
>> Mark.
>>
>
> Hi Mark:
>
> This series also exposes to me (unfortunately) the fact that we aren't
> unmapping the memory space we're picking up for the DMA.
Indeed I think you're right - it seems that for unaligned cases
dma_memory_map() can create a bounce region rather than providing a
pointer to the memory directly. I'm not exactly sure that we're
triggering this at the moment since I don't see memory usage ballooning
during use, but it's something that should be done for consistency (not
least that the unmap call marks the DMA memory as dirty).
> It wouldn't be too hard to unmap in the pmac_ide_transfer_cb with
> something like ...
>
> dma_memory_unmap(&address_space_memory, XXXX, io->len, s->dma_cmd ==
> IDE_DMA_READ ? DMA_DIRECTION_FROM_DEVICE : DMA_DIRECTION_TO_DEVICE, io->len)
>
> As the XXXX is the dead giveaway, though, we've actually leaked the
> pointer -- we've given it to qemu_iovec_add, but I don't think there's a
> way to get it back, so we'll need to stash it /somewhere/.
>
> In DBDMA_io ?
That sounds like a reasonable approach. I'm extremely low on QEMU cycle
for the next 2 weeks or so, but if you post something I'll try my best
to review it.
ATB,
Mark.
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 19/19] ahci: do not remap clb/fis unconditionally
2015-05-22 19:59 [Qemu-devel] [PULL 00/19] Ide patches John Snow
` (17 preceding siblings ...)
2015-05-22 19:59 ` [Qemu-devel] [PULL 18/19] macio: move unaligned DMA write code into separate pmac_dma_write() function John Snow
@ 2015-05-22 19:59 ` John Snow
2015-05-26 11:54 ` [Qemu-devel] [PULL 00/19] Ide patches Peter Maydell
19 siblings, 0 replies; 32+ messages in thread
From: John Snow @ 2015-05-22 19:59 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow
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 <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
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 8e36dec..9e5d862 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
--
2.1.0
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PULL 00/19] Ide patches
2015-05-22 19:59 [Qemu-devel] [PULL 00/19] Ide patches John Snow
` (18 preceding siblings ...)
2015-05-22 19:59 ` [Qemu-devel] [PULL 19/19] ahci: do not remap clb/fis unconditionally John Snow
@ 2015-05-26 11:54 ` Peter Maydell
19 siblings, 0 replies; 32+ messages in thread
From: Peter Maydell @ 2015-05-26 11:54 UTC (permalink / raw)
To: John Snow; +Cc: QEMU Developers
On 22 May 2015 at 20:59, John Snow <jsnow@redhat.com> wrote:
> The following changes since commit bb2fa17f182ee0b45b53474f76679944fc891f04:
>
> Merge remote-tracking branch 'remotes/bkoppelmann/tags/pull-tricore-20150522' into staging (2015-05-22 16:22:42 +0100)
>
> are available in the git repository at:
>
> https://github.com/jnsnow/qemu.git tags/ide-pull-request
>
> for you to fetch changes up to cd6cb73beb63e5fa62ca8ed540b9d54063b15c44:
>
> ahci: do not remap clb/fis unconditionally (2015-05-22 15:58:22 -0400)
>
> ----------------------------------------------------------------
Applied, thanks (and dropped Centos5 from my build test setup!)
-- PMM
^ permalink raw reply [flat|nested] 32+ messages in thread