READ ID on a drive with no medium terminates normally and returns the made-up sector ID left over from the "Pretend we are spinning" emulation. The only error path is a data rate mismatch, and media_rate is assigned solely by pick_geometry(); it is never reset when the medium is removed. A guest that has just ejected a diskette is therefore told that one is still present.
READ, WRITE and FORMAT have a related problem: fd_seek() answers 2 both for "track/head out of range" and for "no medium", so the callers report ST0 = ABNTERM with ST1 = 0x00 either way. Without ST1.MA the guest cannot tell an absent diskette from a transient error. Give fd_seek() a return code of its own for an absent medium, and let both switch statements report the missing address mark for it. The comments on the two switches were swapped: fd_seek() answers 2 for a bad track or head and 3 for a sector past last_sect, but case 2 read "sect too big" and case 3 "track too big". Both now say what they mean. This is a behaviour change for FORMAT TRACK on an empty drive as well, which now answers ST1.MA rather than ST1 = 0x00. None of the guests tested reaches that path -- DOS gives up during media sensing and never issues the command -- but it seemed wrong to leave fdctrl_format_sector() falling through to "default" for a case fd_seek() now reports explicitly. Failing READ ID does not make guests detect the removal: real hardware never completes the command on an empty drive, because there are no index pulses, and OS/2 for one relies on that timeout. It does stop the controller from claiming a diskette that is not there. tests/qtest/fdc-test.c starts QEMU with "-device floppy,id=floppy0" and no medium, and test_read_id asserts a normal termination with a made-up cylinder 8 / head 1. That contradicts its neighbours test_no_media_on_start and test_media_change, which state that DSKCHG signals an absent medium. Insert a medium before READ ID and eject it afterwards -- the rewritten test passes before and after this change -- and add test_read_id_no_media for the empty drive. Guests checked, reading and writing, with and without a medium: Linux 2.0.34 and 7.0, PC-DOS 7, IBM DOS 5.02, Windows for Workgroups 3.11 and OS/2 2.11. None changes behaviour. No version of the Linux floppy driver from 1.2.13 to master issues READ ID at all -- FD_READID is defined in the uapi header for FDRAWCMD users and the driver never sends it -- so Linux detects an empty drive by stepping the head and reading DSKCHG instead. Buglink: https://gitlab.com/qemu-project/qemu/-/issues/3971 Signed-off-by: Christian Quante <[email protected]> --- hw/block/fdc.c | 49 ++++++++++++++++++++++++----- tests/qtest/fdc-test.c | 70 +++++++++++++++++++++++++++++++++++++----- 2 files changed, 105 insertions(+), 14 deletions(-) diff --git a/hw/block/fdc.c b/hw/block/fdc.c index 9b2409cfa4..04750f7310 100644 --- a/hw/block/fdc.c +++ b/hw/block/fdc.c @@ -196,6 +196,12 @@ static void fd_init(FDrive *drv) #define NUM_SIDES(drv) ((drv)->flags & FDISK_DBL_SIDES ? 2 : 1) +/* Is a diskette present in the drive? */ +static bool fd_media_present(FDrive *drv) +{ + return drv->blk != NULL && blk_is_inserted(drv->blk); +} + static int fd_sector_calc(uint8_t head, uint8_t track, uint8_t sect, uint8_t last_sect, uint8_t num_sides) { @@ -258,7 +264,7 @@ static int fd_seek(FDrive *drv, uint8_t head, uint8_t track, uint8_t sect, #endif drv->head = head; if (drv->track != track) { - if (drv->blk != NULL && blk_is_inserted(drv->blk)) { + if (fd_media_present(drv)) { drv->media_changed = 0; } ret = 1; @@ -267,8 +273,8 @@ static int fd_seek(FDrive *drv, uint8_t head, uint8_t track, uint8_t sect, drv->sect = sect; } - if (drv->blk == NULL || !blk_is_inserted(drv->blk)) { - ret = 2; + if (!fd_media_present(drv)) { + ret = 5; } return ret; @@ -1476,14 +1482,24 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int direction) NUM_SIDES(cur_drv))); switch (fd_seek(cur_drv, kh, kt, ks, fdctrl->config & FD_CONFIG_EIS)) { case 2: - /* sect too big */ + /* track/head out of range */ fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, 0x00, 0x00); fdctrl->fifo[3] = kt; fdctrl->fifo[4] = kh; fdctrl->fifo[5] = ks; return; + case 5: + /* + * No medium: there is no address mark to be found. Guests that tell + * an absent diskette from an unreadable one rely on ST1.MA. + */ + fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_MA, 0x00); + fdctrl->fifo[3] = kt; + fdctrl->fifo[4] = kh; + fdctrl->fifo[5] = ks; + return; case 3: - /* track too big */ + /* sector too big */ fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_EC, 0x00); fdctrl->fifo[3] = kt; fdctrl->fifo[4] = kh; @@ -1791,14 +1807,21 @@ static void fdctrl_format_sector(FDCtrl *fdctrl) NUM_SIDES(cur_drv))); switch (fd_seek(cur_drv, kh, kt, ks, fdctrl->config & FD_CONFIG_EIS)) { case 2: - /* sect too big */ + /* track/head out of range */ fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, 0x00, 0x00); fdctrl->fifo[3] = kt; fdctrl->fifo[4] = kh; fdctrl->fifo[5] = ks; return; + case 5: + /* no medium */ + fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_MA, 0x00); + fdctrl->fifo[3] = kt; + fdctrl->fifo[4] = kh; + fdctrl->fifo[5] = ks; + return; case 3: - /* track too big */ + /* sector too big */ fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_EC, 0x00); fdctrl->fifo[3] = kt; fdctrl->fifo[4] = kh; @@ -2306,6 +2329,18 @@ static void fdctrl_result_timer(void *opaque) FDCtrl *fdctrl = opaque; FDrive *cur_drv = get_cur_drv(fdctrl); + /* + * An empty drive has no address marks to read. Completing READ ID + * successfully, with the made-up sector ID left over from the "spinning" + * emulation below, tells the guest that a diskette is still present after + * it has been ejected. The only error path left was a data rate mismatch, + * and media_rate is never reset when the medium is removed. + */ + if (!fd_media_present(cur_drv)) { + FLOPPY_DPRINTF("read id on empty drive\n"); + fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_MA, 0x00); + return; + } /* Pretend we are spinning. * This is needed for Coherent, which uses READ ID to check for * sector interleaving. diff --git a/tests/qtest/fdc-test.c b/tests/qtest/fdc-test.c index 1b37a8a4d2..1e1dd8659d 100644 --- a/tests/qtest/fdc-test.c +++ b/tests/qtest/fdc-test.c @@ -64,6 +64,12 @@ enum { DSKCHG = 0x80, }; +enum { + ST0_IC_MASK = 0xc0, /* interrupt code */ + ST0_IC_ABNTERM = 0x40, /* abnormal termination */ + + ST1_MA = 0x01, /* missing address mark */ +}; static char *test_image; @@ -270,6 +276,21 @@ static void test_cmos(void) g_assert(cmos == 0x40 || cmos == 0x50); } +static void media_insert(void) +{ + qtest_qmp_assert_success(global_qtest, + "{'execute':'blockdev-change-medium', 'arguments':{" + " 'id':'floppy0', 'filename': %s, 'format': 'raw' }}", + test_image); +} + +static void media_eject(void) +{ + qtest_qmp_assert_success(global_qtest, + "{'execute':'eject', 'arguments':{" + " 'id':'floppy0' }}"); +} + static void test_no_media_on_start(void) { uint8_t dir; @@ -301,10 +322,7 @@ static void test_media_insert(void) /* Insert media in drive. DSKCHK should not be reset until a step pulse * is sent. */ - qtest_qmp_assert_success(global_qtest, - "{'execute':'blockdev-change-medium', 'arguments':{" - " 'id':'floppy0', 'filename': %s, 'format': 'raw' }}", - test_image); + media_insert(); dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); @@ -333,9 +351,7 @@ static void test_media_change(void) /* Eject the floppy and check that DSKCHG is set. Reading it out doesn't * reset the bit. */ - qtest_qmp_assert_success(global_qtest, - "{'execute':'eject', 'arguments':{" - " 'id':'floppy0' }}"); + media_eject(); dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); @@ -414,6 +430,9 @@ static void test_read_id(void) uint8_t st0; uint8_t msr; + /* READ ID reads an address mark, so it needs a medium in the drive. */ + media_insert(); + /* Seek to track 0 and check with READ ID */ send_seek(0); @@ -491,6 +510,42 @@ static void test_read_id(void) g_assert_cmpint(cyl, ==, 8); g_assert_cmpint(head, ==, 1); g_assert_cmpint(st0, ==, head << 2); + + /* Leave the drive empty, the way the machine starts up. */ + media_eject(); +} + +/* + * An empty drive spins no diskette, so READ ID finds no address mark and must + * terminate abnormally. Reporting success (with a made-up sector ID) would + * tell the guest that a medium is still present after it has been ejected. + */ +static void test_read_id_no_media(void) +{ + uint8_t drive = 0; + uint8_t head = 0; + uint8_t st0, st1; + + floppy_send(CMD_READ_ID); + g_assert(!get_irq(FLOPPY_IRQ)); + floppy_send(head << 2 | drive); + + while (!get_irq(FLOPPY_IRQ)) { + clock_step(1000000000LL / 50); + } + + st0 = floppy_recv(); + st1 = floppy_recv(); + floppy_recv(); /* ST2 */ + floppy_recv(); /* cylinder */ + floppy_recv(); /* head */ + floppy_recv(); /* sector */ + g_assert(get_irq(FLOPPY_IRQ)); + floppy_recv(); /* sector size */ + g_assert(!get_irq(FLOPPY_IRQ)); + + g_assert_cmpint(st0 & ST0_IC_MASK, ==, ST0_IC_ABNTERM); + g_assert_cmpint(st1 & ST1_MA, ==, ST1_MA); } static void test_read_no_dma_1(void) @@ -625,6 +680,7 @@ int main(int argc, char **argv) qtest_add_func("/fdc/sense_interrupt", test_sense_interrupt); qtest_add_func("/fdc/relative_seek", test_relative_seek); qtest_add_func("/fdc/read_id", test_read_id); + qtest_add_func("/fdc/read_id_no_media", test_read_id_no_media); qtest_add_func("/fdc/verify", test_verify); qtest_add_func("/fdc/media_insert", test_media_insert); qtest_add_func("/fdc/read_no_dma_1", test_read_no_dma_1); -- 2.53.0
