* [PATCH] esp.c: remove transfer size check from DMA DATA IN and DATA OUT transfers
@ 2024-07-13 22:42 Mark Cave-Ayland
2024-07-15 6:48 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 6+ messages in thread
From: Mark Cave-Ayland @ 2024-07-13 22:42 UTC (permalink / raw)
To: qemu-devel, pbonzini, fam
The transfer size check was originally added to prevent consecutive DMA TI
commands from causing an assert() due to an existing SCSI request being in
progress, but since the last set of updates this is no longer required.
Remove the transfer size check from DMA DATA IN and DATA OUT transfers so
that issuing a DMA TI command when there is no data left to transfer does
not cause an assert() due to an existing SCSI request being in progress.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2415
---
hw/scsi/esp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 5d9b52632e..8504dd30a0 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -594,7 +594,7 @@ static void esp_do_dma(ESPState *s)
if (!s->current_req) {
return;
}
- if (s->async_len == 0 && esp_get_tc(s) && s->ti_size) {
+ if (s->async_len == 0 && esp_get_tc(s)) {
/* Defer until data is available. */
return;
}
@@ -647,7 +647,7 @@ static void esp_do_dma(ESPState *s)
if (!s->current_req) {
return;
}
- if (s->async_len == 0 && esp_get_tc(s) && s->ti_size) {
+ if (s->async_len == 0 && esp_get_tc(s)) {
/* Defer until data is available. */
return;
}
--
2.39.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] esp.c: remove transfer size check from DMA DATA IN and DATA OUT transfers
2024-07-13 22:42 [PATCH] esp.c: remove transfer size check from DMA DATA IN and DATA OUT transfers Mark Cave-Ayland
@ 2024-07-15 6:48 ` Philippe Mathieu-Daudé
2024-07-15 22:01 ` Mark Cave-Ayland
0 siblings, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-07-15 6:48 UTC (permalink / raw)
To: Mark Cave-Ayland, qemu-devel, pbonzini, fam
On 14/7/24 00:42, Mark Cave-Ayland wrote:
> The transfer size check was originally added to prevent consecutive DMA TI
> commands from causing an assert() due to an existing SCSI request being in
> progress, but since the last set of updates
[*]
> this is no longer required.
>
> Remove the transfer size check from DMA DATA IN and DATA OUT transfers so
> that issuing a DMA TI command when there is no data left to transfer does
> not cause an assert() due to an existing SCSI request being in progress.
>
[*] See commits f3ace75be8..78d68f312a
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2415
> ---
> hw/scsi/esp.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Queued adding [*], thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] esp.c: remove transfer size check from DMA DATA IN and DATA OUT transfers
2024-07-15 6:48 ` Philippe Mathieu-Daudé
@ 2024-07-15 22:01 ` Mark Cave-Ayland
2024-07-16 6:46 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 6+ messages in thread
From: Mark Cave-Ayland @ 2024-07-15 22:01 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel, pbonzini, fam
On 15/07/2024 07:48, Philippe Mathieu-Daudé wrote:
> On 14/7/24 00:42, Mark Cave-Ayland wrote:
>> The transfer size check was originally added to prevent consecutive DMA TI
>> commands from causing an assert() due to an existing SCSI request being in
>> progress, but since the last set of updates
>
> [*]
>
>> this is no longer required.
>>
>> Remove the transfer size check from DMA DATA IN and DATA OUT transfers so
>> that issuing a DMA TI command when there is no data left to transfer does
>> not cause an assert() due to an existing SCSI request being in progress.
>>
>
> [*] See commits f3ace75be8..78d68f312a
>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2415
>> ---
>> hw/scsi/esp.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> Queued adding [*], thanks.
Awesome, thanks Phil!
ATB,
Mark.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] esp.c: remove transfer size check from DMA DATA IN and DATA OUT transfers
2024-07-15 22:01 ` Mark Cave-Ayland
@ 2024-07-16 6:46 ` Philippe Mathieu-Daudé
2024-07-16 8:56 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-07-16 6:46 UTC (permalink / raw)
To: Mark Cave-Ayland, qemu-devel, pbonzini, fam; +Cc: Thomas Huth
On 16/7/24 00:01, Mark Cave-Ayland wrote:
> On 15/07/2024 07:48, Philippe Mathieu-Daudé wrote:
>
>> On 14/7/24 00:42, Mark Cave-Ayland wrote:
>>> The transfer size check was originally added to prevent consecutive
>>> DMA TI
>>> commands from causing an assert() due to an existing SCSI request
>>> being in
>>> progress, but since the last set of updates
>>
>> [*]
>>
>>> this is no longer required.
>>>
>>> Remove the transfer size check from DMA DATA IN and DATA OUT
>>> transfers so
>>> that issuing a DMA TI command when there is no data left to transfer
>>> does
>>> not cause an assert() due to an existing SCSI request being in progress.
>>>
>>
>> [*] See commits f3ace75be8..78d68f312a
>>
>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2415
>>> ---
>>> hw/scsi/esp.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>
>> Queued adding [*], thanks.
>
> Awesome, thanks Phil!
I'm getting dubious timeout on the msys2 build on the SPARC target:
https://gitlab.com/philmd/qemu/-/jobs/7347774958
qemu:qtest+qtest-sparc / qtest-sparc/qom-test time out (After 900.0 seconds)
1/151 qemu:qtest+qtest-sparc / qtest-sparc/qom-test
TIMEOUT 900.38s exit status 1
qemu:qtest+qtest-sparc / qtest-sparc/device-introspect-test time out
(After 720.0 seconds)
2/151 qemu:qtest+qtest-sparc / qtest-sparc/device-introspect-test
TIMEOUT 720.23s exit status 1
qemu:qtest+qtest-sparc / qtest-sparc/prom-env-test time out (After 360.0
seconds)
4/151 qemu:qtest+qtest-sparc / qtest-sparc/prom-env-test
TIMEOUT 360.17s exit status 1
Not sure this patch is the culprit, but since only SPARC is affected,
likely. I'll retest without this patch.
Regards,
Phil.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] esp.c: remove transfer size check from DMA DATA IN and DATA OUT transfers
2024-07-16 6:46 ` Philippe Mathieu-Daudé
@ 2024-07-16 8:56 ` Philippe Mathieu-Daudé
2024-07-16 13:30 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-07-16 8:56 UTC (permalink / raw)
To: Mark Cave-Ayland, qemu-devel, pbonzini, fam; +Cc: Thomas Huth
On 16/7/24 08:46, Philippe Mathieu-Daudé wrote:
> On 16/7/24 00:01, Mark Cave-Ayland wrote:
>> On 15/07/2024 07:48, Philippe Mathieu-Daudé wrote:
>>
>>> On 14/7/24 00:42, Mark Cave-Ayland wrote:
>>>> The transfer size check was originally added to prevent consecutive
>>>> DMA TI
>>>> commands from causing an assert() due to an existing SCSI request
>>>> being in
>>>> progress, but since the last set of updates
>>>
>>> [*]
>>>
>>>> this is no longer required.
>>>>
>>>> Remove the transfer size check from DMA DATA IN and DATA OUT
>>>> transfers so
>>>> that issuing a DMA TI command when there is no data left to transfer
>>>> does
>>>> not cause an assert() due to an existing SCSI request being in
>>>> progress.
>>>>
>>>
>>> [*] See commits f3ace75be8..78d68f312a
>>>
>>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2415
>>>> ---
>>>> hw/scsi/esp.c | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>
>>> Queued adding [*], thanks.
>>
>> Awesome, thanks Phil!
>
> I'm getting dubious timeout on the msys2 build on the SPARC target:
> https://gitlab.com/philmd/qemu/-/jobs/7347774958
>
> qemu:qtest+qtest-sparc / qtest-sparc/qom-test time out (After 900.0
> seconds)
> 1/151 qemu:qtest+qtest-sparc / qtest-sparc/qom-test TIMEOUT
> 900.38s exit status 1
> qemu:qtest+qtest-sparc / qtest-sparc/device-introspect-test time out
> (After 720.0 seconds)
> 2/151 qemu:qtest+qtest-sparc / qtest-sparc/device-introspect-test
> TIMEOUT 720.23s exit status 1
> qemu:qtest+qtest-sparc / qtest-sparc/prom-env-test time out (After 360.0
> seconds)
> 4/151 qemu:qtest+qtest-sparc / qtest-sparc/prom-env-test
> TIMEOUT 360.17s exit status 1
>
> Not sure this patch is the culprit, but since only SPARC is affected,
> likely. I'll retest without this patch.
Same failure without this patch, so not this patch fault ;)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] esp.c: remove transfer size check from DMA DATA IN and DATA OUT transfers
2024-07-16 8:56 ` Philippe Mathieu-Daudé
@ 2024-07-16 13:30 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-07-16 13:30 UTC (permalink / raw)
To: Mark Cave-Ayland, qemu-devel, pbonzini, fam
Cc: Thomas Huth, Richard Henderson
On 16/7/24 10:56, Philippe Mathieu-Daudé wrote:
> On 16/7/24 08:46, Philippe Mathieu-Daudé wrote:
>> On 16/7/24 00:01, Mark Cave-Ayland wrote:
>>> On 15/07/2024 07:48, Philippe Mathieu-Daudé wrote:
>>>
>>>> On 14/7/24 00:42, Mark Cave-Ayland wrote:
>>>>> The transfer size check was originally added to prevent consecutive
>>>>> DMA TI
>>>>> commands from causing an assert() due to an existing SCSI request
>>>>> being in
>>>>> progress, but since the last set of updates
>>>>
>>>> [*]
>>>>
>>>>> this is no longer required.
>>>>>
>>>>> Remove the transfer size check from DMA DATA IN and DATA OUT
>>>>> transfers so
>>>>> that issuing a DMA TI command when there is no data left to
>>>>> transfer does
>>>>> not cause an assert() due to an existing SCSI request being in
>>>>> progress.
>>>>>
>>>>
>>>> [*] See commits f3ace75be8..78d68f312a
>>>>
>>>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2415
>>>>> ---
>>>>> hw/scsi/esp.c | 4 ++--
>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>>
>>>> Queued adding [*], thanks.
>>>
>>> Awesome, thanks Phil!
>>
>> I'm getting dubious timeout on the msys2 build on the SPARC target:
>> https://gitlab.com/philmd/qemu/-/jobs/7347774958
>>
>> qemu:qtest+qtest-sparc / qtest-sparc/qom-test time out (After 900.0
>> seconds)
>> 1/151 qemu:qtest+qtest-sparc / qtest-sparc/qom-test TIMEOUT
>> 900.38s exit status 1
>> qemu:qtest+qtest-sparc / qtest-sparc/device-introspect-test time out
>> (After 720.0 seconds)
>> 2/151 qemu:qtest+qtest-sparc / qtest-sparc/device-introspect-test
>> TIMEOUT 720.23s exit status 1
>> qemu:qtest+qtest-sparc / qtest-sparc/prom-env-test time out (After
>> 360.0 seconds)
>> 4/151 qemu:qtest+qtest-sparc / qtest-sparc/prom-env-test
>> TIMEOUT 360.17s exit status 1
>>
>> Not sure this patch is the culprit, but since only SPARC is affected,
>> likely. I'll retest without this patch.
>
> Same failure without this patch, so not this patch fault ;)
Actually I couldn't find any patch in my PR triggering this,
then noticed it is also happening on the main branch:
https://gitlab.com/qemu-project/qemu/-/jobs/7347517442
:/
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-07-16 13:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-13 22:42 [PATCH] esp.c: remove transfer size check from DMA DATA IN and DATA OUT transfers Mark Cave-Ayland
2024-07-15 6:48 ` Philippe Mathieu-Daudé
2024-07-15 22:01 ` Mark Cave-Ayland
2024-07-16 6:46 ` Philippe Mathieu-Daudé
2024-07-16 8:56 ` Philippe Mathieu-Daudé
2024-07-16 13:30 ` Philippe Mathieu-Daudé
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).