[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 00/14] Migration pull request
From: |
Peter Lieven |
Subject: |
Re: [Qemu-devel] [PULL 00/14] Migration pull request |
Date: |
Fri, 20 Nov 2015 15:00:01 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 |
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 <address@hidden> 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
- Re: [Qemu-devel] [PULL 00/14] Migration pull request, (continued)
- Re: [Qemu-devel] [PULL 00/14] Migration pull request, Peter Maydell, 2015/11/19
- Re: [Qemu-devel] [PULL 00/14] Migration pull request, Peter Maydell, 2015/11/19
- Re: [Qemu-devel] [PULL 00/14] Migration pull request, Peter Maydell, 2015/11/19
- Re: [Qemu-devel] [PULL 00/14] Migration pull request, Peter Maydell, 2015/11/19
- Re: [Qemu-devel] [PULL 00/14] Migration pull request, Dr. David Alan Gilbert, 2015/11/19
- Re: [Qemu-devel] [PULL 00/14] Migration pull request, Peter Lieven, 2015/11/20
- Re: [Qemu-devel] [PULL 00/14] Migration pull request, Peter Lieven, 2015/11/20
- Re: [Qemu-devel] [PULL 00/14] Migration pull request, Peter Maydell, 2015/11/20
- Re: [Qemu-devel] [PULL 00/14] Migration pull request, Peter Lieven, 2015/11/20
- Re: [Qemu-devel] [PULL 00/14] Migration pull request, Kevin Wolf, 2015/11/20
- Re: [Qemu-devel] [PULL 00/14] Migration pull request,
Peter Lieven <=
- Re: [Qemu-devel] [PULL 00/14] Migration pull request, Kevin Wolf, 2015/11/20
- Re: [Qemu-devel] [PULL 00/14] Migration pull request, John Snow, 2015/11/19
Re: [Qemu-devel] [PULL 00/14] Migration pull request, Peter Maydell, 2015/11/19