From: Fiona Ebner <f.ebner@proxmox.com>
To: qemu-devel@nongnu.org
Cc: thuth@redhat.com, lvivier@redhat.com, pbonzini@redhat.com,
srowe@mose.org.uk, mike.maslenkin@gmail.com,
qemu-block@nongnu.org, t.lamprecht@proxmox.com,
a.lauterer@proxmox.com
Subject: [POC 2/2] add test exposing AHCI reset issue
Date: Thu, 24 Aug 2023 15:38:31 +0200 [thread overview]
Message-ID: <20230824133831.617833-2-f.ebner@proxmox.com> (raw)
In-Reply-To: <20230824133831.617833-1-f.ebner@proxmox.com>
Fails without the previous commit "hw/ide: reset: cancel async DMA
operation before reseting state".
I haven't ever written such a test before, but I wanted something to
expose the problem more easily. It hardcodes the behavior that the
pending write actually is done during reset, which might not be ideal.
It could just check that the first sector is still intact instead.
If I should make this a proper test, I'd be happy about some guidance,
but not sure if required for such a specific one-off issue. After all,
a different variation of the bug might have written to some other
sector not covered by this test.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
tests/qtest/ahci-test.c | 81 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 81 insertions(+)
diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
index abab761c26..3ebeb4e255 100644
--- a/tests/qtest/ahci-test.c
+++ b/tests/qtest/ahci-test.c
@@ -1401,6 +1401,84 @@ static void test_max(void)
ahci_shutdown(ahci);
}
+static void test_reset_with_pending_callback(void)
+{
+ AHCIQState *ahci;
+
+ ahci = ahci_boot(NULL);
+ ahci_test_pci_spec(ahci);
+ ahci_pci_enable(ahci);
+
+ int bufsize = 512 * 1024;
+ int offset1 = 0;
+ int offset2 = bufsize / AHCI_SECTOR_SIZE;
+
+ ahci_test_hba_spec(ahci);
+ ahci_hba_enable(ahci);
+ ahci_test_identify(ahci);
+
+ uint8_t port = ahci_port_select(ahci);
+ ahci_port_clear(ahci, port);
+
+ unsigned char *tx1 = g_malloc(bufsize);
+ unsigned char *tx2 = g_malloc(bufsize);
+ unsigned char *rx1 = g_malloc0(bufsize);
+ unsigned char *rx2 = g_malloc0(bufsize);
+ uint64_t ptr1 = ahci_alloc(ahci, bufsize);
+ uint64_t ptr2 = ahci_alloc(ahci, bufsize);
+
+ g_assert(ptr1 && ptr2);
+
+ /* Need two different patterns. */
+ do {
+ generate_pattern(tx1, bufsize, AHCI_SECTOR_SIZE);
+ generate_pattern(tx2, bufsize, AHCI_SECTOR_SIZE);
+ } while (memcmp(tx1, tx2, bufsize) == 0);
+
+ qtest_bufwrite(ahci->parent->qts, ptr1, tx1, bufsize);
+ qtest_bufwrite(ahci->parent->qts, ptr2, tx2, bufsize);
+
+ /* Write to beginning of disk to check it wasn't overwritten later. */
+ ahci_guest_io(ahci, port, CMD_WRITE_DMA_EXT, ptr1, bufsize, offset1);
+
+ /* Issue asynchronously to get a pending callback during reset. */
+ AHCICommand *cmd = ahci_command_create(CMD_WRITE_DMA_EXT);
+ ahci_command_adjust(cmd, offset2, ptr2, bufsize, 0);
+ ahci_command_commit(ahci, cmd, port);
+ ahci_command_issue_async(ahci, cmd);
+
+ ahci_set(ahci, AHCI_GHC, AHCI_GHC_HR);
+
+ ahci_command_free(cmd);
+
+ /* Start again. */
+ ahci_clean_mem(ahci);
+ ahci_pci_enable(ahci);
+ ahci_hba_enable(ahci);
+ port = ahci_port_select(ahci);
+ ahci_port_clear(ahci, port);
+
+ /* Read and verify. */
+ ahci_guest_io(ahci, port, CMD_READ_DMA_EXT, ptr1, bufsize, offset1);
+ qtest_bufread(ahci->parent->qts, ptr1, rx1, bufsize);
+ g_assert_cmphex(memcmp(tx1, rx1, bufsize), ==, 0);
+
+ ahci_guest_io(ahci, port, CMD_READ_DMA_EXT, ptr2, bufsize, offset2);
+ qtest_bufread(ahci->parent->qts, ptr2, rx2, bufsize);
+ g_assert_cmphex(memcmp(tx2, rx2, bufsize), ==, 0);
+
+ ahci_free(ahci, ptr1);
+ ahci_free(ahci, ptr2);
+ g_free(tx1);
+ g_free(tx2);
+ g_free(rx1);
+ g_free(rx2);
+
+ ahci_clean_mem(ahci);
+
+ ahci_shutdown(ahci);
+}
+
static void test_reset(void)
{
AHCIQState *ahci;
@@ -1915,6 +1993,9 @@ int main(int argc, char **argv)
g_assert(fd >= 0);
close(fd);
+ qtest_add_func("/ahci/reset_with_pending_callback",
+ test_reset_with_pending_callback);
+
/* Run the tests */
qtest_add_func("/ahci/sanity", test_sanity);
qtest_add_func("/ahci/pci_spec", test_pci_spec);
--
2.39.2
next prev parent reply other threads:[~2023-08-24 13:39 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-24 13:38 [PATCH 1/2] hw/ide: reset: cancel async DMA operation before reseting state Fiona Ebner
2023-08-24 13:38 ` Fiona Ebner [this message]
2023-08-24 15:09 ` [POC 2/2] add test exposing AHCI reset issue Philippe Mathieu-Daudé
2023-08-24 15:52 ` Thomas Huth
2023-08-25 10:17 ` Fiona Ebner
2023-09-04 9:26 ` Kevin Wolf
2023-08-24 15:07 ` [PATCH 1/2] hw/ide: reset: cancel async DMA operation before reseting state Philippe Mathieu-Daudé
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230824133831.617833-2-f.ebner@proxmox.com \
--to=f.ebner@proxmox.com \
--cc=a.lauterer@proxmox.com \
--cc=lvivier@redhat.com \
--cc=mike.maslenkin@gmail.com \
--cc=pbonzini@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=srowe@mose.org.uk \
--cc=t.lamprecht@proxmox.com \
--cc=thuth@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).