From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45805) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZzmEY-0003qz-Im for qemu-devel@nongnu.org; Fri, 20 Nov 2015 09:00:19 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZzmES-0000XU-O8 for qemu-devel@nongnu.org; Fri, 20 Nov 2015 09:00:14 -0500 Received: from mx-v6.kamp.de ([2a02:248:0:51::16]:47105 helo=mx01.kamp.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZzmES-0000We-F8 for qemu-devel@nongnu.org; Fri, 20 Nov 2015 09:00:08 -0500 Message-ID: <564F2761.8000200@kamp.de> Date: Fri, 20 Nov 2015 15:00:01 +0100 From: Peter Lieven MIME-Version: 1.0 References: <1447932069-2772-1-git-send-email-quintela@redhat.com> <20151119151627.GH2653@work-vm> <564EEA17.9080006@kamp.de> <564F23AC.5030903@kamp.de> <20151120135355.GB4130@noname.redhat.com> In-Reply-To: <20151120135355.GB4130@noname.redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PULL 00/14] Migration pull request List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Peter Maydell , Juan Quintela , QEMU Developers , "Dr. David Alan Gilbert" , Amit Shah , John Snow Am 20.11.2015 um 14:53 schrieb Kevin Wolf: > Am 20.11.2015 um 14:44 hat Peter Lieven geschrieben: >> Am 20.11.2015 um 12:33 schrieb Peter Maydell: >>> On 20 November 2015 at 09:38, Peter Lieven wrote: >>>> I wonder if there is a glitch in the PIO implementation of test-ide.c. As far as I understand the specs >>>> it is not allowed to read data while the BSY flag is set. With the following change to the test-ide script >>>> the test does not race: >>>> >>>> diff --git a/tests/ide-test.c b/tests/ide-test.c >>>> index d1014bb..ab0489e 100644 >>>> --- a/tests/ide-test.c >>>> +++ b/tests/ide-test.c >>>> @@ -728,6 +728,7 @@ static void cdrom_pio_impl(int nblocks) >>>> for (i = 0; i < DIV_ROUND_UP(rxsize, limit); i++) { >>>> size_t offset = i * (limit / 2); >>>> size_t rem = (rxsize / 2) - offset; >>>> + ide_wait_clear(BSY); >>>> for (j = 0; j < MIN((limit / 2), rem); j++) { >>>> rx[offset + j] = le16_to_cpu(inw(IDE_BASE + reg_data)); >>>> } >>>> >>>> Note: in the old sync version of the ATAPI PIO implementation this could not happen. >>> This certainly fixes the stalls for me, though I don't know enough >>> IDE to say whether it is the correct fix. >> Thanks for testing. >> >> I hope that John or Kevin can verify this fix? > The fix looks correct to me. > > While you're improving the test, you could also add an assertion that > DRQ is set after BSY has been cleared. I would actually move the check (which is already there) into the loop: diff --git a/tests/ide-test.c b/tests/ide-test.c index d1014bb..d7ee376 100644 --- a/tests/ide-test.c +++ b/tests/ide-test.c @@ -712,11 +712,6 @@ static void cdrom_pio_impl(int nblocks) /* HPD3: INTRQ_Wait */ ide_wait_intr(IDE_PRIMARY_IRQ); - /* HPD2: Check_Status_B */ - data = ide_wait_clear(BSY); - assert_bit_set(data, DRQ | DRDY); - assert_bit_clear(data, ERR | DF | BSY); - /* Read data back: occurs in bursts of 'BYTE_COUNT_LIMIT' bytes. * If BYTE_COUNT_LIMIT is odd, we transfer BYTE_COUNT_LIMIT - 1 bytes. * We allow an odd limit only when the remaining transfer size is @@ -728,6 +723,10 @@ static void cdrom_pio_impl(int nblocks) for (i = 0; i < DIV_ROUND_UP(rxsize, limit); i++) { size_t offset = i * (limit / 2); size_t rem = (rxsize / 2) - offset; + /* HPD2: Check_Status_B */ + data = ide_wait_clear(BSY); + assert_bit_set(data, DRQ | DRDY); + assert_bit_clear(data, ERR | DF | BSY); for (j = 0; j < MIN((limit / 2), rem); j++) { rx[offset + j] = le16_to_cpu(inw(IDE_BASE + reg_data)); } Are you okay with that? @John, you also? Peter