Re: [PATCH] tests/qtest: add one more test for the am53c974

[Mark Cave-Ayland]

On 03/04/2021 15:38, Mark Cave-Ayland wrote:

> On 02/04/2021 17:20, Alexander Bulekov wrote:
>
> > Original crash:
> > qemu-fuzz-i386: ../hw/scsi/esp.c:791: void esp_transfer_data(SCSIRequest *, 
> > uint32_t): Assertion `!s->do_cmd' failed.
> > ==257532== ERROR: libFuzzer: deadly signal
> > __assert_fail assert/assert.c:101:3
> > esp_transfer_data hw/scsi/esp.c:791:5
> > scsi_req_data hw/scsi/scsi-bus.c:1412:9
> > scsi_disk_emulate_read_data hw/scsi/scsi-disk.c:1407:9
> > scsi_req_continue hw/scsi/scsi-bus.c:1394:9
> > do_busid_cmd hw/scsi/esp.c:317:9
> > handle_s_without_atn hw/scsi/esp.c:393:9
> > esp_reg_write hw/scsi/esp.c:1029:13
> > esp_pci_io_write hw/scsi/esp-pci.c:215:9
> > memory_region_write_accessor softmmu/memory.c:491:5
> > access_with_adjusted_size softmmu/memory.c:552:18
> > memory_region_dispatch_write softmmu/memory.c:1502:16
> > flatview_write_continue softmmu/physmem.c:2746:23
> > flatview_write softmmu/physmem.c:2786:14
> > address_space_write softmmu/physmem.c:2878:18
> > cpu_outl softmmu/ioport.c:80:5
> >
> > Based-on: <20210401074933.9923-1-mark.cave-ayland@ilande.co.uk>
> > Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> > ---
> >   tests/qtest/am53c974-test.c | 18 ++++++++++++++++++
> >   1 file changed, 18 insertions(+)
> >
> > The patch took care of the handle_satn_stop assert. Here's a test case
> > for the other assert.
>
> Great! I've squashed the get_cmd() changes into a v4 version of the patchset.
>
> > Pasteable:
> >
> > cat << EOF | ./qemu-system-i386 -display none -machine accel=qtest, -m \
> > 512M -device am53c974,id=scsi -device scsi-hd,drive=disk0 -drive \
> > id=disk0,if=none,file=null-co://,format=raw -nodefaults -qtest stdio
> > outl 0xcf8 0x80001010
> > outl 0xcfc 0xc000
> > outl 0xcf8 0x80001004
> > outw 0xcfc 0x01
> > outl 0xc00b 0x4100
> > outb 0xc008 0x42
> > outw 0xc03f 0x0300
> > outl 0xc00b 0xc100
> > EOF
> >
> >
> > diff --git a/tests/qtest/am53c974-test.c b/tests/qtest/am53c974-test.c
> > index 9c4285d0c0..506276677a 100644
> > --- a/tests/qtest/am53c974-test.c
> > +++ b/tests/qtest/am53c974-test.c
> > @@ -9,6 +9,22 @@
> >   #include "libqos/libqtest.h"
> > +static void test_do_cmd_assert(void)
> > +{
> > +    QTestState *s = qtest_init(
> > +        "-device am53c974,id=scsi "
> > +        "-device scsi-hd,drive=disk0 -drive "
> > +        "id=disk0,if=none,file=null-co://,format=raw -nodefaults");
> > +    qtest_outl(s, 0xcf8, 0x80001010);
> > +    qtest_outl(s, 0xcfc, 0xc000);
> > +    qtest_outl(s, 0xcf8, 0x80001004);
> > +    qtest_outw(s, 0xcfc, 0x01);
> > +    qtest_outl(s, 0xc00b, 0x4100);
> > +    qtest_outb(s, 0xc008, 0x42);
> > +    qtest_outw(s, 0xc03f, 0x0300);
> > +    qtest_outl(s, 0xc00b, 0xc100);
> > +    qtest_quit(s);
> > +}
> >   static void test_cmdfifo_underflow_ok(void)
> >   {
> > @@ -194,6 +210,8 @@ int main(int argc, char **argv)
> >       g_test_init(&argc, &argv, NULL);
> >       if (strcmp(arch, "i386") == 0) {
> > +        qtest_add_func("am53c974/test_do_cmd_asser",
> > +                       test_do_cmd_assert);
> >           qtest_add_func("am53c974/test_cmdfifo_underflow_ok",
> >                          test_cmdfifo_underflow_ok);
> >           qtest_add_func("am53c974/test_cmdfifo_underflow2_ok",
>
> When I try this patch on top of the v4 patchset I don't get an assert() in 
> esp_transfer_data here?

If I also revert the suggested updates for get_cmd() then I don't get the assert() 
either, so I believe this particular case is already covered by the existing tests. 
I'll drop this change for v4.


ATB,

Mark.